From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

This patch aims to fix unexpected negotiation features for
vhost-user netdev interface.

When openvswitch reconnect Qemu after an unexpected disconnection
and Qemu therefore start the vhost_dev, acked_features field in
vhost_dev is initialized with value fetched from acked_features
field in NetVhostUserState, which should be up-to-date at that
moment but Qemu could not make it actually during the time window
of virtio features negotiation.

So we save the acked_features right after being configured by
guest virtio driver so it can be used to restore acked_features
field in vhost_dev correctly.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
---
 hw/net/vhost_net-stub.c | 5 +++++
 hw/net/vhost_net.c      | 6 ++++++
 hw/net/virtio-net.c     | 6 ++++++
 include/net/vhost_net.h | 2 ++
 4 files changed, 19 insertions(+)

diff --git a/hw/net/vhost_net-stub.c b/hw/net/vhost_net-stub.c
index 9f7daae..66ed5f0 100644
--- a/hw/net/vhost_net-stub.c
+++ b/hw/net/vhost_net-stub.c
@@ -113,3 +113,8 @@ int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
 {
     return 0;
 }
+
+void vhost_net_save_acked_features(NetClientState *nc)
+{
+
+}
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index feda448..ceb962c 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -143,6 +143,12 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net)
     return net->dev.acked_features;
 }
 
+void vhost_net_save_acked_features(NetClientState *nc)
+{
+    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_USER);
+    vhost_user_save_acked_features(nc);
+}
+
 static int vhost_net_get_fd(NetClientState *backend)
 {
     switch (backend->info->type) {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index aba1275..91cbd0c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -981,6 +981,12 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
             continue;
         }
         vhost_net_ack_features(get_vhost_net(nc->peer), features);
+
+        /*
+         * keep acked_features in NetVhostUserState up-to-date so it
+         * can't miss any features configured by guest virtio driver.
+         */
+        vhost_net_save_acked_features(nc->peer);
     }
 
     if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 40b9a40..dfb1375 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -52,4 +52,6 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
                                int vq_index);
 int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
                                 int vq_index);
+
+void vhost_net_save_acked_features(NetClientState *nc);
 #endif
-- 
1.8.3.1

QEMU will coredump when vm starts.

Using command line:

./build/qemu-system-x86_64 \
     -nodefaults \
     -m 4G \
     -machine pc-i440fx-4.1 \
     -accel kvm \
     -cpu host \
     -smp 4 \
     -device qemu-xhci -device usb-kbd -device usb-tablet \
     -drive if=none,id=linux,file=test.img,format=raw \
     -device virtio-blk-pci,drive=linux,disable-legacy=on \
     -vnc :0 \
     -d all \
     -D %dlog \
     -netdev tap,id=hostnet0,ifname=tap0,vhost=on,script=no,downscript=no \
     -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:6b:0d:a1 \
     -device cirrus-vga \
     -msg timestamp=on


And then:
qemu-system-x86_64: ../hw/net/vhost_net.c:149: 
vhost_net_save_acked_features: Assertion `nc->info->type == 
NET_CLIENT_DRIVER_VHOST_USER' failed.
Aborted (core dumped)

Because it may be a tap or vdpa besides vhost user when function 
"get_vhost_net(nc->peer)" returns ture.





From: liuxiangdong <liuxiangdong5@huawei.com>
Date: Mon, 5 Dec 2022 07:11:28 +0800
Subject: [PATCH] vhost_net: keep acked_feature only for
  NET_CLIENT_DRIVER_VHOST_USER

Keep acked_features in NetVhostUserState up-to-date by function 
vhost_net_save_acked_features
in function virtio_net_set_features. But nc->peer->info->type maybe 
NET_CLIENT_DRIVER_TAP or
NET_CLIENT_DRIVER_VHOST_VDPA besides NET_CLIENT_DRIVER_VHOST_USER.

Don't keep acked_features in other type now except 
NET_CLIENT_DRIVER_VHOST_USER

Fix:  vhost-user: Fix the virtio features negotiation flaw

Signed-off-by: liuxiangdong <liuxiangdong5@huawei.com>
---
  hw/net/vhost_net.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index a98575ffbc..bea053a742 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -143,8 +143,9 @@ uint64_t vhost_net_get_acked_features(VHostNetState 
*net)

  void vhost_net_save_acked_features(NetClientState *nc)
  {
-    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_USER);
-    vhost_user_save_acked_features(nc);
+    if (nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
+        vhost_user_save_acked_features(nc);
+    }
  }

  static int vhost_net_get_fd(NetClientState *backend)
-- 
Gitee

Thanks xiangdong for the testing and reporting, indeed, vhost-user 
negotiaion features saving only apply on dpdk interface, vhost-net and 
vdpa interface can be ingored. I'll apply it next version.

Yong

在 2022/12/14 16:15, Liuxiangdong 写道:
> QEMU will coredump when vm starts.
> 
> Using command line:
> 
> ./build/qemu-system-x86_64 \
>      -nodefaults \
>      -m 4G \
>      -machine pc-i440fx-4.1 \
>      -accel kvm \
>      -cpu host \
>      -smp 4 \
>      -device qemu-xhci -device usb-kbd -device usb-tablet \
>      -drive if=none,id=linux,file=test.img,format=raw \
>      -device virtio-blk-pci,drive=linux,disable-legacy=on \
>      -vnc :0 \
>      -d all \
>      -D %dlog \
>      -netdev tap,id=hostnet0,ifname=tap0,vhost=on,script=no,downscript=no \
>      -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:6b:0d:a1 \
>      -device cirrus-vga \
>      -msg timestamp=on
> 
> 
> And then:
> qemu-system-x86_64: ../hw/net/vhost_net.c:149: 
> vhost_net_save_acked_features: Assertion `nc->info->type == 
> NET_CLIENT_DRIVER_VHOST_USER' failed.
> Aborted (core dumped)
> 
> Because it may be a tap or vdpa besides vhost user when function 
> "get_vhost_net(nc->peer)" returns ture.
> 
> 
> 
> 
> 
> From: liuxiangdong <liuxiangdong5@huawei.com>
> Date: Mon, 5 Dec 2022 07:11:28 +0800
> Subject: [PATCH] vhost_net: keep acked_feature only for
>   NET_CLIENT_DRIVER_VHOST_USER
> 
> Keep acked_features in NetVhostUserState up-to-date by function 
> vhost_net_save_acked_features
> in function virtio_net_set_features. But nc->peer->info->type maybe 
> NET_CLIENT_DRIVER_TAP or
> NET_CLIENT_DRIVER_VHOST_VDPA besides NET_CLIENT_DRIVER_VHOST_USER.
> 
> Don't keep acked_features in other type now except 
> NET_CLIENT_DRIVER_VHOST_USER
> 
> Fix:  vhost-user: Fix the virtio features negotiation flaw
> 
> Signed-off-by: liuxiangdong <liuxiangdong5@huawei.com>
> ---
>   hw/net/vhost_net.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index a98575ffbc..bea053a742 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -143,8 +143,9 @@ uint64_t vhost_net_get_acked_features(VHostNetState 
> *net)
> 
>   void vhost_net_save_acked_features(NetClientState *nc)
>   {
> -    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_USER);
> -    vhost_user_save_acked_features(nc);
> +    if (nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> +        vhost_user_save_acked_features(nc);
> +    }
>   }
> 
>   static int vhost_net_get_fd(NetClientState *backend)

-- 
Best regard

Hyman Huang(黄勇)