[RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[Yuri Benditovich]

Allow fallback to userspace only upon migration, only for specific features
and only if 'vhostforce' is not requested.

Changes from v1:
Patch 1 dropeed (will be submitted in another series)
Added device callback in case the migration should fail due to missing features

Yuri Benditovich (3):
  net: add ability to hide (disable) vhost_net
  virtio: introduce 'missing_features_migrated' device callback
  virtio-net: implement missing_features_migrated callback

 hw/net/vhost_net.c         |  4 ++-
 hw/net/virtio-net.c        | 51 ++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio.c         |  8 ++++++
 include/hw/virtio/virtio.h |  8 ++++++
 include/net/net.h          |  1 +
 5 files changed, 71 insertions(+), 1 deletion(-)

-- 
2.26.2

[RFC PATCH v2 1/3] net: add ability to hide (disable) vhost_net

[Yuri Benditovich]

If 'vhost_net_disabled' in the NetClientState of the
net device, get_vhost_net for TAP returns NULL. Network adapters
can use this ability to hide the vhost_net temporary between
resets in case some active features contradict with vhost.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/net/vhost_net.c | 4 +++-
 include/net/net.h  | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 24d555e764..6660efd9ea 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -436,7 +436,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
 
     switch (nc->info->type) {
     case NET_CLIENT_DRIVER_TAP:
-        vhost_net = tap_get_vhost_net(nc);
+        if (!nc->vhost_net_disabled) {
+            vhost_net = tap_get_vhost_net(nc);
+        }
         break;
 #ifdef CONFIG_VHOST_NET_USER
     case NET_CLIENT_DRIVER_VHOST_USER:
diff --git a/include/net/net.h b/include/net/net.h
index a02949f6db..a938211524 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -103,6 +103,7 @@ struct NetClientState {
     int vring_enable;
     int vnet_hdr_len;
     bool is_netdev;
+    bool vhost_net_disabled;
     QTAILQ_HEAD(, NetFilterState) filters;
 };
 
-- 
2.26.2

[RFC PATCH v2 2/3] virtio: introduce 'missing_features_migrated' device callback

[Yuri Benditovich]

This optional callback addresses migration problem in case
some of negotiated features not present on the destination
system. The device has a chance to avoid migration failure.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/virtio/virtio.c         | 8 ++++++++
 include/hw/virtio/virtio.h | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 07f4e60b30..36dcac75e5 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3107,6 +3107,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
         vdev->device_endian = virtio_default_endian();
     }
 
+    if (vdc->missing_features_migrated) {
+        uint64_t missing = (vdev->guest_features & ~(vdev->host_features));
+        if (missing && vdc->missing_features_migrated(vdev, missing)) {
+            vdev->host_features =
+                vdc->get_features(vdev, vdev->host_features, NULL);
+        }
+    }
+
     if (virtio_64bit_features_needed(vdev)) {
         /*
          * Subsection load filled vdev->guest_features.  Run them
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b7ece7a6a8..fbfbec6ef2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -158,6 +158,14 @@ struct VirtioDeviceClass {
      * processed, e.g. for bounds checking.
      */
     int (*post_load)(VirtIODevice *vdev);
+    /* In case when some of negotiated features are missing on the destination
+       system, the migration is expected to fail. To avoid such failure, the
+       device may implement this callback and apply graceful configuration
+       change to extend host features (for example, disable vhost).
+       If the device returns true the virtio reinitializes the host features
+       and further set_features call may succeed.
+     */
+    bool (*missing_features_migrated)(VirtIODevice *vdev, uint64_t val);
     const VMStateDescription *vmsd;
     bool (*primary_unplug_pending)(void *opaque);
 };
-- 
2.26.2

[RFC PATCH v2 3/3] virtio-net: implement missing_features_migrated callback

[Yuri Benditovich]

Graceful drop to userspace virtio in case selected features
are missing on the destination system. Currently used for
3 features that might be supported by the vhost kernel on
the source machine and not supported on the destination machine:
rss, hash reporting, packed ring.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/net/virtio-net.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 96a3cc8357..97afca34e7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -527,6 +527,15 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
     return info;
 }
 
+static void virtio_net_allow_vhost(VirtIONet *n, bool allow)
+{
+    int i;
+    for (i = 0; i < n->max_queues; i++) {
+        NetClientState *nc = qemu_get_subqueue(n->nic, i)->peer;
+        nc->vhost_net_disabled = !allow;
+    }
+}
+
 static void virtio_net_reset(VirtIODevice *vdev)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -564,6 +573,7 @@ static void virtio_net_reset(VirtIODevice *vdev)
             assert(!virtio_net_get_subqueue(nc)->async_tx.elem);
         }
     }
+    virtio_net_allow_vhost(n, true);
 }
 
 static void peer_test_vnet_hdr(VirtIONet *n)
@@ -701,6 +711,27 @@ static void virtio_net_set_queues(VirtIONet *n)
     }
 }
 
+static bool can_disable_vhost(VirtIONet *n)
+{
+    NetClientState *peer = qemu_get_queue(n->nic)->peer;
+    NetdevInfo *ndi;
+    if (!get_vhost_net(peer)) {
+        return false;
+    }
+    if (!peer) {
+        return true;
+    }
+    if (peer->info->type != NET_CLIENT_DRIVER_TAP) {
+        return false;
+    }
+    ndi = peer->stored_config;
+    if (ndi && ndi->u.tap.has_vhostforce && ndi->u.tap.vhostforce) {
+        printf("vhost forced, can't drop it\n");
+        return false;
+    }
+    return true;
+}
+
 static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue);
 
 static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
@@ -3433,6 +3464,25 @@ static bool dev_unplug_pending(void *opaque)
     return vdc->primary_unplug_pending(dev);
 }
 
+static bool virtio_net_missing_features_migrated(VirtIODevice *vdev,
+                                                 uint64_t missing)
+{
+    VirtIONet *n = VIRTIO_NET(vdev);
+    bool disable_vhost = false;
+    if (virtio_has_feature(missing, VIRTIO_NET_F_HASH_REPORT) ||
+        virtio_has_feature(missing, VIRTIO_NET_F_RSS) ||
+        virtio_has_feature(missing, VIRTIO_F_RING_PACKED)) {
+        disable_vhost = true;
+    }
+    disable_vhost = disable_vhost && can_disable_vhost(n);
+    if (disable_vhost) {
+        warn_report("falling back to userspace virtio due to missing"
+                    " features %lx", missing);
+        virtio_net_allow_vhost(n, false);
+    }
+    return disable_vhost;
+}
+
 static const VMStateDescription vmstate_virtio_net = {
     .name = "virtio-net",
     .minimum_version_id = VIRTIO_NET_VM_VERSION,
@@ -3527,6 +3577,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
     vdc->get_features = virtio_net_get_features;
     vdc->set_features = virtio_net_set_features;
     vdc->bad_features = virtio_net_bad_features;
+    vdc->missing_features_migrated = virtio_net_missing_features_migrated;
     vdc->reset = virtio_net_reset;
     vdc->set_status = virtio_net_set_status;
     vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
-- 
2.26.2

Re: [RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[Jason Wang]

在 2021/3/22 下午8:24, Yuri Benditovich 写道:
> Allow fallback to userspace only upon migration, only for specific features
> and only if 'vhostforce' is not requested.
>
> Changes from v1:
> Patch 1 dropeed (will be submitted in another series)
> Added device callback in case the migration should fail due to missing features


Hi Yuri:

Have a quick glance at the series. A questions is why we need to do the 
fallback only during load?

I think we should do it in the device initializating. E.g when the vhost 
features can not satisfy, we should disable vhost since there.

Thanks


>
> Yuri Benditovich (3):
>    net: add ability to hide (disable) vhost_net
>    virtio: introduce 'missing_features_migrated' device callback
>    virtio-net: implement missing_features_migrated callback
>
>   hw/net/vhost_net.c         |  4 ++-
>   hw/net/virtio-net.c        | 51 ++++++++++++++++++++++++++++++++++++++
>   hw/virtio/virtio.c         |  8 ++++++
>   include/hw/virtio/virtio.h |  8 ++++++
>   include/net/net.h          |  1 +
>   5 files changed, 71 insertions(+), 1 deletion(-)
>

Re: [RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[Yuri Benditovich]

Hi Jason,

This was discussed earlier on the previous series of patches.
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg01829.html
There were strong objections from both Daniel and Michael and I feel
that the series was rejected.
There was Michael's claim:
"We did what this patch is trying to change for years now, in
particular KVM also seems to happily disable CPU features not supported
by kernel so I wonder why we can't keep doing it, with tweaks for some
corner cases."
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03187.html
And it was Michael's question:
"Can we limit the change to when a VM is migrated in?"
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03163.html
So I'm trying to suggest another approach:
- In case of conflicting features (for example RSS and vhost) we in
qemu we do not have enough information to prefer one or another.
- If we drop to userspace in the first set_features we say: "vhost is
less important than other requested features"
- This series keeps backward compatibility, i.e. if you start with
vhost and some features are not available - they are silently cleared.
- But in case the features are available on source machine - they are used
- In case of migration this series says: "We prefer successful
migration even if for that we need to drop to userspace"
- On the migration back to the 1st system we again work with all the
features and with vhost as all the features are available.

Thanks,
Yuri



On Thu, Mar 25, 2021 at 8:59 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/3/22 下午8:24, Yuri Benditovich 写道:
> > Allow fallback to userspace only upon migration, only for specific features
> > and only if 'vhostforce' is not requested.
> >
> > Changes from v1:
> > Patch 1 dropeed (will be submitted in another series)
> > Added device callback in case the migration should fail due to missing features
>
>
> Hi Yuri:
>
> Have a quick glance at the series. A questions is why we need to do the
> fallback only during load?
>
> I think we should do it in the device initializating. E.g when the vhost
> features can not satisfy, we should disable vhost since there.
>
> Thanks
>
>
> >
> > Yuri Benditovich (3):
> >    net: add ability to hide (disable) vhost_net
> >    virtio: introduce 'missing_features_migrated' device callback
> >    virtio-net: implement missing_features_migrated callback
> >
> >   hw/net/vhost_net.c         |  4 ++-
> >   hw/net/virtio-net.c        | 51 ++++++++++++++++++++++++++++++++++++++
> >   hw/virtio/virtio.c         |  8 ++++++
> >   include/hw/virtio/virtio.h |  8 ++++++
> >   include/net/net.h          |  1 +
> >   5 files changed, 71 insertions(+), 1 deletion(-)
> >
>

Re: [RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[Jason Wang]

在 2021/3/25 下午5:00, Yuri Benditovich 写道:
> Hi Jason,
>
> This was discussed earlier on the previous series of patches.
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg01829.html
> There were strong objections from both Daniel and Michael and I feel
> that the series was rejected.
> There was Michael's claim:
> "We did what this patch is trying to change for years now, in
> particular KVM also seems to happily disable CPU features not supported
> by kernel so I wonder why we can't keep doing it, with tweaks for some
> corner cases."


So for cpu feautres, it works since the management have other tool to 
the cpuid. Then management will make sure the migration happens amongs 
the hosts that is compatibile with the same cpuid sets.

For vhost, we don't have such capabilities, that's why I think we need 
to have fallback.


> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03187.html
> And it was Michael's question:
> "Can we limit the change to when a VM is migrated in?"
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03163.html
> So I'm trying to suggest another approach:
> - In case of conflicting features (for example RSS and vhost) we in
> qemu we do not have enough information to prefer one or another.
> - If we drop to userspace in the first set_features we say: "vhost is
> less important than other requested features"
> - This series keeps backward compatibility, i.e. if you start with
> vhost and some features are not available - they are silently cleared.
> - But in case the features are available on source machine - they are used
> - In case of migration this series says: "We prefer successful
> migration even if for that we need to drop to userspace"
> - On the migration back to the 1st system we again work with all the
> features and with vhost as all the features are available.


One issue for this approach is that. Consider we had two drivers:

1) Driver A that supports split only
2) Driver B that supports packed

Consider src support packed but dest doesn't

So switching driver A to driver B works without migration. But if we 
switch driver from A to B after migration it won't work?

Thanks


>
> Thanks,
> Yuri
>
>
>
> On Thu, Mar 25, 2021 at 8:59 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>> 在 2021/3/22 下午8:24, Yuri Benditovich 写道:
>>> Allow fallback to userspace only upon migration, only for specific features
>>> and only if 'vhostforce' is not requested.
>>>
>>> Changes from v1:
>>> Patch 1 dropeed (will be submitted in another series)
>>> Added device callback in case the migration should fail due to missing features
>>
>> Hi Yuri:
>>
>> Have a quick glance at the series. A questions is why we need to do the
>> fallback only during load?
>>
>> I think we should do it in the device initializating. E.g when the vhost
>> features can not satisfy, we should disable vhost since there.
>>
>> Thanks
>>
>>
>>> Yuri Benditovich (3):
>>>     net: add ability to hide (disable) vhost_net
>>>     virtio: introduce 'missing_features_migrated' device callback
>>>     virtio-net: implement missing_features_migrated callback
>>>
>>>    hw/net/vhost_net.c         |  4 ++-
>>>    hw/net/virtio-net.c        | 51 ++++++++++++++++++++++++++++++++++++++
>>>    hw/virtio/virtio.c         |  8 ++++++
>>>    include/hw/virtio/virtio.h |  8 ++++++
>>>    include/net/net.h          |  1 +
>>>    5 files changed, 71 insertions(+), 1 deletion(-)
>>>

Re: [RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[Yuri Benditovich]

On Fri, Mar 26, 2021 at 10:51 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/3/25 下午5:00, Yuri Benditovich 写道:
> > Hi Jason,
> >
> > This was discussed earlier on the previous series of patches.
> > https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg01829.html
> > There were strong objections from both Daniel and Michael and I feel
> > that the series was rejected.
> > There was Michael's claim:
> > "We did what this patch is trying to change for years now, in
> > particular KVM also seems to happily disable CPU features not supported
> > by kernel so I wonder why we can't keep doing it, with tweaks for some
> > corner cases."
>
>
> So for cpu feautres, it works since the management have other tool to
> the cpuid. Then management will make sure the migration happens amongs
> the hosts that is compatibile with the same cpuid sets.
>
> For vhost, we don't have such capabilities, that's why I think we need
> to have fallback.
>
Hi Jason,
What, from your POV was the result of v1 discussion?
IMO, there was one critical comment that the patch does not address
'forcevhost' properly (indeed).
IMO, there are many comments from Daniel and Michael that in the sum
say that this change is not what they would like.
If I'm mistaken please let me know.

I have no problem to send v3 = v1 + handling of ''forcevhost'
If this is what you want, please let me know also.

>
> > https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03187.html
> > And it was Michael's question:
> > "Can we limit the change to when a VM is migrated in?"
> > https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03163.html
> > So I'm trying to suggest another approach:
> > - In case of conflicting features (for example RSS and vhost) we in
> > qemu we do not have enough information to prefer one or another.
> > - If we drop to userspace in the first set_features we say: "vhost is
> > less important than other requested features"
> > - This series keeps backward compatibility, i.e. if you start with
> > vhost and some features are not available - they are silently cleared.
> > - But in case the features are available on source machine - they are used
> > - In case of migration this series says: "We prefer successful
> > migration even if for that we need to drop to userspace"
> > - On the migration back to the 1st system we again work with all the
> > features and with vhost as all the features are available.
>
>
> One issue for this approach is that. Consider we had two drivers:
>
> 1) Driver A that supports split only
> 2) Driver B that supports packed
>
> Consider src support packed but dest doesn't
>
> So switching driver A to driver B works without migration. But if we
> switch driver from A to B after migration it won't work?

I assume that  both src and dest started with vhost=on.

As driver B supports both packed and split, you can switch from driver
A to driver B after migration
and driver B will work with split. Exactly as it does today.

The key question is what is more important - vhost or features that
vhost does not support?
current code says: vhost is more important always
v1 patch says: features are more important always.
v2 patch says: vhost is more important at init time, features are more
important at migration time.
Because we are able to drop vhost but we can't drop features when we
have a running driver.
Do you agree?

>
> Thanks
>
>
> >
> > Thanks,
> > Yuri
> >
> >
> >
> > On Thu, Mar 25, 2021 at 8:59 AM Jason Wang <jasowang@redhat.com> wrote:
> >>
> >> 在 2021/3/22 下午8:24, Yuri Benditovich 写道:
> >>> Allow fallback to userspace only upon migration, only for specific features
> >>> and only if 'vhostforce' is not requested.
> >>>
> >>> Changes from v1:
> >>> Patch 1 dropeed (will be submitted in another series)
> >>> Added device callback in case the migration should fail due to missing features
> >>
> >> Hi Yuri:
> >>
> >> Have a quick glance at the series. A questions is why we need to do the
> >> fallback only during load?
> >>
> >> I think we should do it in the device initializating. E.g when the vhost
> >> features can not satisfy, we should disable vhost since there.
> >>
> >> Thanks
> >>
> >>
> >>> Yuri Benditovich (3):
> >>>     net: add ability to hide (disable) vhost_net
> >>>     virtio: introduce 'missing_features_migrated' device callback
> >>>     virtio-net: implement missing_features_migrated callback
> >>>
> >>>    hw/net/vhost_net.c         |  4 ++-
> >>>    hw/net/virtio-net.c        | 51 ++++++++++++++++++++++++++++++++++++++
> >>>    hw/virtio/virtio.c         |  8 ++++++
> >>>    include/hw/virtio/virtio.h |  8 ++++++
> >>>    include/net/net.h          |  1 +
> >>>    5 files changed, 71 insertions(+), 1 deletion(-)
> >>>
>

Re: [RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[Jason Wang]

在 2021/3/26 下午5:09, Yuri Benditovich 写道:
> On Fri, Mar 26, 2021 at 10:51 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>> 在 2021/3/25 下午5:00, Yuri Benditovich 写道:
>>> Hi Jason,
>>>
>>> This was discussed earlier on the previous series of patches.
>>> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg01829.html
>>> There were strong objections from both Daniel and Michael and I feel
>>> that the series was rejected.
>>> There was Michael's claim:
>>> "We did what this patch is trying to change for years now, in
>>> particular KVM also seems to happily disable CPU features not supported
>>> by kernel so I wonder why we can't keep doing it, with tweaks for some
>>> corner cases."
>>
>> So for cpu feautres, it works since the management have other tool to
>> the cpuid. Then management will make sure the migration happens amongs
>> the hosts that is compatibile with the same cpuid sets.
>>
>> For vhost, we don't have such capabilities, that's why I think we need
>> to have fallback.
>>
> Hi Jason,
> What, from your POV was the result of v1 discussion?


It looks to me we don't have an agreement on that, sorry.


> IMO, there was one critical comment that the patch does not address
> 'forcevhost' properly (indeed).
> IMO, there are many comments from Daniel and Michael that in the sum
> say that this change is not what they would like.
> If I'm mistaken please let me know.


I think I will open a new thread and summarize the different approaches 
and then we can come a conclusion.


>
> I have no problem to send v3 = v1 + handling of ''forcevhost'
> If this is what you want, please let me know also.
>
>>> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03187.html
>>> And it was Michael's question:
>>> "Can we limit the change to when a VM is migrated in?"
>>> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03163.html
>>> So I'm trying to suggest another approach:
>>> - In case of conflicting features (for example RSS and vhost) we in
>>> qemu we do not have enough information to prefer one or another.
>>> - If we drop to userspace in the first set_features we say: "vhost is
>>> less important than other requested features"
>>> - This series keeps backward compatibility, i.e. if you start with
>>> vhost and some features are not available - they are silently cleared.
>>> - But in case the features are available on source machine - they are used
>>> - In case of migration this series says: "We prefer successful
>>> migration even if for that we need to drop to userspace"
>>> - On the migration back to the 1st system we again work with all the
>>> features and with vhost as all the features are available.
>>
>> One issue for this approach is that. Consider we had two drivers:
>>
>> 1) Driver A that supports split only
>> 2) Driver B that supports packed
>>
>> Consider src support packed but dest doesn't
>>
>> So switching driver A to driver B works without migration. But if we
>> switch driver from A to B after migration it won't work?
> I assume that  both src and dest started with vhost=on.
>
> As driver B supports both packed and split, you can switch from driver
> A to driver B after migration
> and driver B will work with split. Exactly as it does today.
>
> The key question is what is more important - vhost or features that
> vhost does not support?
> current code says: vhost is more important always
> v1 patch says: features are more important always.
> v2 patch says: vhost is more important at init time, features are more
> important at migration time.
> Because we are able to drop vhost but we can't drop features when we
> have a running driver.
> Do you agree?


I think what came from cli is the most important. So if I understand 
correclty:

- vhost=on means "turn on vhost when possible" it implies that fallback 
is allowed (we had already had fallback codes)
- vhostforce=on means "turn on vhost unconditonally" it implies that we 
can't do fallback

So my understanding is that:

- "vhost=on, packed=on", we can fallback to userspace but must keep 
packed virtqueue works
- "vhost=on,vhostforce=on,packed=on", we can't fallback and must keep 
both vhost and packed virtqueue work, if we can't we need to fail

Thanks


>
>> Thanks
>>
>>
>>> Thanks,
>>> Yuri
>>>
>>>
>>>
>>> On Thu, Mar 25, 2021 at 8:59 AM Jason Wang <jasowang@redhat.com> wrote:
>>>> 在 2021/3/22 下午8:24, Yuri Benditovich 写道:
>>>>> Allow fallback to userspace only upon migration, only for specific features
>>>>> and only if 'vhostforce' is not requested.
>>>>>
>>>>> Changes from v1:
>>>>> Patch 1 dropeed (will be submitted in another series)
>>>>> Added device callback in case the migration should fail due to missing features
>>>> Hi Yuri:
>>>>
>>>> Have a quick glance at the series. A questions is why we need to do the
>>>> fallback only during load?
>>>>
>>>> I think we should do it in the device initializating. E.g when the vhost
>>>> features can not satisfy, we should disable vhost since there.
>>>>
>>>> Thanks
>>>>
>>>>
>>>>> Yuri Benditovich (3):
>>>>>      net: add ability to hide (disable) vhost_net
>>>>>      virtio: introduce 'missing_features_migrated' device callback
>>>>>      virtio-net: implement missing_features_migrated callback
>>>>>
>>>>>     hw/net/vhost_net.c         |  4 ++-
>>>>>     hw/net/virtio-net.c        | 51 ++++++++++++++++++++++++++++++++++++++
>>>>>     hw/virtio/virtio.c         |  8 ++++++
>>>>>     include/hw/virtio/virtio.h |  8 ++++++
>>>>>     include/net/net.h          |  1 +
>>>>>     5 files changed, 71 insertions(+), 1 deletion(-)
>>>>>

Re: [RFC PATCH v2 0/3] virtio-net: graceful drop of vhost for TAP

[Jason Wang]

在 2021/3/26 下午5:18, Jason Wang 写道:
>>> ?
>> I assume that  both src and dest started with vhost=on.
>>
>> As driver B supports both packed and split, you can switch from driver
>> A to driver B after migration
>> and driver B will work with split. Exactly as it does today.
>>
>> The key question is what is more important - vhost or features that
>> vhost does not support?
>> current code says: vhost is more important always
>> v1 patch says: features are more important always.
>> v2 patch says: vhost is more important at init time, features are more
>> important at migration time.
>> Because we are able to drop vhost but we can't drop features when we
>> have a running driver.
>> Do you agree?
>
>
> I think what came from cli is the most important. So if I understand 
> correclty:
>
> - vhost=on means "turn on vhost when possible" it implies that 
> fallback is allowed (we had already had fallback codes)
> - vhostforce=on means "turn on vhost unconditonally" it implies that 
> we can't do fallback
>
> So my understanding is that:
>
> - "vhost=on, packed=on", we can fallback to userspace but must keep 
> packed virtqueue works
> - "vhost=on,vhostforce=on,packed=on", we can't fallback and must keep 
> both vhost and packed virtqueue work, if we can't we need to fail
>
> Thanks


Daniel and Michael, am I right here?

We need some inputs to move forward to fix the migration compatibility 
issue.

Thanks