[PATCH v2 0/6] Support VIRTIO_F_RING_RESET for vhost-user in virtio pci-modern

[PATCH v2 0/6] Support VIRTIO_F_RING_RESET for vhost-user in virtio pci-modern

[Kangjie Xu]

This patch set is based on the patch set that supports VIRTIO_F_RING_RESET for vhost-kernel:
    https://lore.kernel.org/qemu-devel/cover.1662916759.git.kangjie.xu@linux.alibaba.com/

The virtio queue reset function has already been defined in the virtio spec 1.2.
The relevant virtio spec information is here:
    https://github.com/oasis-tcs/virtio-spec/issues/124
    https://github.com/oasis-tcs/virtio-spec/issues/139

The process of virtqueue reset can be concluded as:
1. The virtqueue is disabled when VIRTIO_PCI_COMMON_Q_RESET is written.
2. Then the virtqueue can be optionally restarted(re-enabled).

The detailed process is listed below:
1. VIRTIO_PCI_COMMON_Q_RESET is written [virtio-pci]
    -> virtio_queue_reset() [virtio]
        -> virtio_net_queue_reset() [virtio-net]
            -> vhost_net_virtqueue_reset() [vhost-net]
                -> vhost_virtqueue_stop()
        -> __virtio_queue_reset()
2. VIRTIO_PCI_COMMON_Q_ENABLE is written [virtio-pci]
    -> virtio_queue_enable() [virtio]
        -> virtio_net_queue_enable() [virtio-net]
            -> vhost_net_virtqueue_restart() [vhost-net]
                -> vhost_virtqueue_start()
                -> vhost_user_set_vring_enable [vhost-user]
                    -> send VHOST_USER_SET_VRING_ENABLE to the device
    -> set enabled, reset status of vq.

changelog:
  1. Reuse the vhost_virtqueue_stop() when resetting a vq.

Test environment:
    Qemu: QEMU emulator version 7.1.50
    Guest: 5.19.0-rc3 (With vq reset support)
    DPDK: 22.11-rc0 (With vq reset support)
    Test Cmd: ethtool -g eth1; ethtool -G eth1 rx $1 tx $2; ethtool -g eth1;

    The drvier can resize the virtio queue, then virtio queue reset function should
    be triggered.

    The default is split mode, modify Qemu virtio-net to add PACKED feature to 
    test packed mode.

Guest Kernel Patch:
    https://lore.kernel.org/bpf/20220801063902.129329-1-xuanzhuo@linux.alibaba.com/

DPDK Patch:
    https://github.com/middaywords/dpdk/compare/72206323a5dd3182b13f61b25a64abdddfee595c...080d9d5a6bba8ff8dce5186e2c6cb35cabef77c9

Kangjie Xu (6):
  net: virtio: rename vhost_set_vring_enable to vhost_set_dev_enable
  vhost-user: add op to enable or disable a single vring
  vhost-net: vhost-user: update vhost_net_virtqueue_reset()
  vhost-net: vhost-user: update vhost_net_virtqueue_restart()
  virtio-net: vhost-user: update queue_reset and queue_enable
  vhost: vhost-user: enable vq reset feature

 backends/cryptodev-vhost.c        | 12 ++++++------
 hw/net/vhost_net-stub.c           |  2 +-
 hw/net/vhost_net.c                | 15 +++++++++++----
 hw/net/virtio-net.c               | 10 ++++++----
 hw/virtio/vhost-user.c            | 27 +++++++++++++++++++--------
 include/hw/virtio/vhost-backend.h |  5 ++++-
 include/net/vhost_net.h           |  2 +-
 7 files changed, 48 insertions(+), 25 deletions(-)

-- 
2.32.0

[PATCH v2 1/6] net: virtio: rename vhost_set_vring_enable to vhost_set_dev_enable

[Kangjie Xu]

Previously, vhost_set_vring_enable will enable/disable all vrings
in a device, which causes ambiguity. So we rename it to
vhost_set_dev_enable.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 backends/cryptodev-vhost.c        | 12 ++++++------
 hw/net/vhost_net-stub.c           |  2 +-
 hw/net/vhost_net.c                |  8 ++++----
 hw/net/virtio-net.c               |  4 ++--
 hw/virtio/vhost-user.c            |  4 ++--
 include/hw/virtio/vhost-backend.h |  6 +++---
 include/net/vhost_net.h           |  2 +-
 7 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
index bc13e466b4..b83e939760 100644
--- a/backends/cryptodev-vhost.c
+++ b/backends/cryptodev-vhost.c
@@ -147,9 +147,9 @@ cryptodev_vhost_set_vq_index(CryptoDevBackendVhost *crypto,
 }
 
 static int
-vhost_set_vring_enable(CryptoDevBackendClient *cc,
-                            CryptoDevBackend *b,
-                            uint16_t queue, int enable)
+vhost_set_dev_enable(CryptoDevBackendClient *cc,
+                     CryptoDevBackend *b,
+                     uint16_t queue, int enable)
 {
     CryptoDevBackendVhost *crypto =
                        cryptodev_get_vhost(cc, b, queue);
@@ -162,8 +162,8 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
     }
 
     vhost_ops = crypto->dev.vhost_ops;
-    if (vhost_ops->vhost_set_vring_enable) {
-        return vhost_ops->vhost_set_vring_enable(&crypto->dev, enable);
+    if (vhost_ops->vhost_set_dev_enable) {
+        return vhost_ops->vhost_set_dev_enable(&crypto->dev, enable);
     }
 
     return 0;
@@ -219,7 +219,7 @@ int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
 
         if (cc->vring_enable) {
             /* restore vring enable state */
-            r = vhost_set_vring_enable(cc, b, i, cc->vring_enable);
+            r = vhost_set_dev_enable(cc, b, i, cc->vring_enable);
 
             if (r < 0) {
                 goto err_start;
diff --git a/hw/net/vhost_net-stub.c b/hw/net/vhost_net-stub.c
index 89d71cfb8e..ac5f217dc1 100644
--- a/hw/net/vhost_net-stub.c
+++ b/hw/net/vhost_net-stub.c
@@ -92,7 +92,7 @@ VHostNetState *get_vhost_net(NetClientState *nc)
     return 0;
 }
 
-int vhost_set_vring_enable(NetClientState *nc, int enable)
+int vhost_set_dev_enable(NetClientState *nc, int enable)
 {
     return 0;
 }
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 97cdf9280b..ea896ea75b 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -396,7 +396,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
 
         if (peer->vring_enable) {
             /* restore vring enable state */
-            r = vhost_set_vring_enable(peer, peer->vring_enable);
+            r = vhost_set_dev_enable(peer, peer->vring_enable);
 
             if (r < 0) {
                 vhost_net_stop_one(get_vhost_net(peer), dev);
@@ -508,15 +508,15 @@ VHostNetState *get_vhost_net(NetClientState *nc)
     return vhost_net;
 }
 
-int vhost_set_vring_enable(NetClientState *nc, int enable)
+int vhost_set_dev_enable(NetClientState *nc, int enable)
 {
     VHostNetState *net = get_vhost_net(nc);
     const VhostOps *vhost_ops = net->dev.vhost_ops;
 
     nc->vring_enable = enable;
 
-    if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
-        return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
+    if (vhost_ops && vhost_ops->vhost_set_dev_enable) {
+        return vhost_ops->vhost_set_dev_enable(&net->dev, enable);
     }
 
     return 0;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 7817206596..6ab796b399 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -696,7 +696,7 @@ static int peer_attach(VirtIONet *n, int index)
     }
 
     if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
-        vhost_set_vring_enable(nc->peer, 1);
+        vhost_set_dev_enable(nc->peer, 1);
     }
 
     if (nc->peer->info->type != NET_CLIENT_DRIVER_TAP) {
@@ -719,7 +719,7 @@ static int peer_detach(VirtIONet *n, int index)
     }
 
     if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
-        vhost_set_vring_enable(nc->peer, 0);
+        vhost_set_dev_enable(nc->peer, 0);
     }
 
     if (nc->peer->info->type !=  NET_CLIENT_DRIVER_TAP) {
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index bd24741be8..794519359b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1198,7 +1198,7 @@ static int vhost_user_set_vring_base(struct vhost_dev *dev,
     return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
 }
 
-static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
+static int vhost_user_set_dev_enable(struct vhost_dev *dev, int enable)
 {
     int i;
 
@@ -2627,7 +2627,7 @@ const VhostOps user_ops = {
         .vhost_set_owner = vhost_user_set_owner,
         .vhost_reset_device = vhost_user_reset_device,
         .vhost_get_vq_index = vhost_user_get_vq_index,
-        .vhost_set_vring_enable = vhost_user_set_vring_enable,
+        .vhost_set_dev_enable = vhost_user_set_dev_enable,
         .vhost_requires_shm_log = vhost_user_requires_shm_log,
         .vhost_migration_done = vhost_user_migration_done,
         .vhost_backend_can_merge = vhost_user_can_merge,
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index eab46d7f0b..b49432045f 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -81,8 +81,8 @@ typedef int (*vhost_set_backend_cap_op)(struct vhost_dev *dev);
 typedef int (*vhost_set_owner_op)(struct vhost_dev *dev);
 typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
 typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
-typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
-                                         int enable);
+typedef int (*vhost_set_dev_enable_op)(struct vhost_dev *dev,
+                                       int enable);
 typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
 typedef int (*vhost_migration_done_op)(struct vhost_dev *dev,
                                        char *mac_addr);
@@ -155,7 +155,7 @@ typedef struct VhostOps {
     vhost_set_owner_op vhost_set_owner;
     vhost_reset_device_op vhost_reset_device;
     vhost_get_vq_index_op vhost_get_vq_index;
-    vhost_set_vring_enable_op vhost_set_vring_enable;
+    vhost_set_dev_enable_op vhost_set_dev_enable;
     vhost_requires_shm_log_op vhost_requires_shm_log;
     vhost_migration_done_op vhost_migration_done;
     vhost_backend_can_merge_op vhost_backend_can_merge;
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 40b9a40074..22a1fcd39e 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -42,7 +42,7 @@ void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
 int vhost_net_notify_migration_done(VHostNetState *net, char* mac_addr);
 VHostNetState *get_vhost_net(NetClientState *nc);
 
-int vhost_set_vring_enable(NetClientState * nc, int enable);
+int vhost_set_dev_enable(NetClientState *nc, int enable);
 
 uint64_t vhost_net_get_acked_features(VHostNetState *net);
 
-- 
2.32.0

[PATCH v2 2/6] vhost-user: add op to enable or disable a single vring

[Kangjie Xu]

There is only vhost_set_dev_enable op in VhostOps. Thus, we introduce
the interface vhost_set_vring_enable to set the enable status for a
single vring.

Resetting a single vq will rely on this interface.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/virtio/vhost-user.c            | 25 ++++++++++++++++++-------
 include/hw/virtio/vhost-backend.h |  3 +++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 794519359b..3f140d5085 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1198,6 +1198,22 @@ static int vhost_user_set_vring_base(struct vhost_dev *dev,
     return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
 }
 
+static int vhost_user_set_vring_enable(struct vhost_dev *dev,
+                                       int index,
+                                       int enable)
+{
+    if (index < dev->vq_index || index >= dev->vq_index + dev->nvqs) {
+        return -EINVAL;
+    }
+
+    struct vhost_vring_state state = {
+        .index = index,
+        .num   = enable,
+    };
+
+    return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state);
+}
+
 static int vhost_user_set_dev_enable(struct vhost_dev *dev, int enable)
 {
     int i;
@@ -1207,13 +1223,7 @@ static int vhost_user_set_dev_enable(struct vhost_dev *dev, int enable)
     }
 
     for (i = 0; i < dev->nvqs; ++i) {
-        int ret;
-        struct vhost_vring_state state = {
-            .index = dev->vq_index + i,
-            .num   = enable,
-        };
-
-        ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state);
+        int ret = vhost_user_set_vring_enable(dev, dev->vq_index + i, enable);
         if (ret < 0) {
             /*
              * Restoring the previous state is likely infeasible, as well as
@@ -2627,6 +2637,7 @@ const VhostOps user_ops = {
         .vhost_set_owner = vhost_user_set_owner,
         .vhost_reset_device = vhost_user_reset_device,
         .vhost_get_vq_index = vhost_user_get_vq_index,
+        .vhost_set_vring_enable = vhost_user_set_vring_enable,
         .vhost_set_dev_enable = vhost_user_set_dev_enable,
         .vhost_requires_shm_log = vhost_user_requires_shm_log,
         .vhost_migration_done = vhost_user_migration_done,
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index b49432045f..dad7191bac 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -81,6 +81,8 @@ typedef int (*vhost_set_backend_cap_op)(struct vhost_dev *dev);
 typedef int (*vhost_set_owner_op)(struct vhost_dev *dev);
 typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
 typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
+typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
+                                         int index, int enable);
 typedef int (*vhost_set_dev_enable_op)(struct vhost_dev *dev,
                                        int enable);
 typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
@@ -155,6 +157,7 @@ typedef struct VhostOps {
     vhost_set_owner_op vhost_set_owner;
     vhost_reset_device_op vhost_reset_device;
     vhost_get_vq_index_op vhost_get_vq_index;
+    vhost_set_vring_enable_op vhost_set_vring_enable;
     vhost_set_dev_enable_op vhost_set_dev_enable;
     vhost_requires_shm_log_op vhost_requires_shm_log;
     vhost_migration_done_op vhost_migration_done;
-- 
2.32.0

[PATCH v2 3/6] vhost-net: vhost-user: update vhost_net_virtqueue_reset()

[Kangjie Xu]

Update vhost_net_virtqueue_reset() for vhost-user scenario.

In order to reuse some functions, we process the idx for
vhost-user scenario because vhost_get_vq_index behave
differently for vhost-user.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/vhost_net.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ea896ea75b..25e5665489 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -545,6 +545,9 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
     assert(vhost_ops);
 
     idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
+    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
+        idx -= net->dev.vq_index;
+    }
 
     if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
         file.index = idx;
-- 
2.32.0

[PATCH v2 4/6] vhost-net: vhost-user: update vhost_net_virtqueue_restart()

[Kangjie Xu]

Update vhost_net_virtqueue_restart() for vhost-user scenario.

In order to reuse some functions, we process the idx for
vhost-user case. It is because vhost_get_vq_index behave
differently in vhost-user.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/vhost_net.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 25e5665489..8b80942e7c 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -577,6 +577,9 @@ int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
     assert(vhost_ops);
 
     idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
+    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
+        idx -= net->dev.vq_index;
+    }
 
     r = vhost_virtqueue_start(&net->dev,
                               vdev,
-- 
2.32.0

[PATCH v2 5/6] virtio-net: vhost-user: update queue_reset and queue_enable

[Kangjie Xu]

Update virtio_net_queue_reset() and virtio_net_queue_enable()
for vhost-user scenario.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/virtio-net.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 6ab796b399..19a2132180 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -550,7 +550,8 @@ static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
     }
 
     if (get_vhost_net(nc->peer) &&
-        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
+        (nc->peer->info->type == NET_CLIENT_DRIVER_TAP ||
+         nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER)) {
         vhost_net_virtqueue_reset(vdev, nc, queue_index);
     }
 
@@ -568,7 +569,8 @@ static void virtio_net_queue_enable(VirtIODevice *vdev, uint32_t queue_index)
     }
 
     if (get_vhost_net(nc->peer) &&
-        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
+        (nc->peer->info->type == NET_CLIENT_DRIVER_TAP ||
+         nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER)) {
         r = vhost_net_virtqueue_restart(vdev, nc, queue_index);
         if (r < 0) {
             error_report("unable to restart vhost net virtqueue: %d, "
-- 
2.32.0

[PATCH v2 6/6] vhost: vhost-user: enable vq reset feature

[Kangjie Xu]

Add virtqueue reset feature for vhost-user.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/vhost_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 8b80942e7c..ad319faee8 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -74,6 +74,7 @@ static const int user_feature_bits[] = {
     VIRTIO_NET_F_MTU,
     VIRTIO_F_IOMMU_PLATFORM,
     VIRTIO_F_RING_PACKED,
+    VIRTIO_F_RING_RESET,
     VIRTIO_NET_F_RSS,
     VIRTIO_NET_F_HASH_REPORT,
 
-- 
2.32.0

Re: [PATCH v2 1/6] net: virtio: rename vhost_set_vring_enable to vhost_set_dev_enable

[Jason Wang]

在 2022/9/12 11:10, Kangjie Xu 写道:
> Previously, vhost_set_vring_enable will enable/disable all vrings
> in a device,


Nit: It would be more clear to say to disable all vrings for a specific 
vhost device.

Since in the case of multiqueue virtio-net, a single virtio-net device 
may have multiple vhost devices.

Thanks


> which causes ambiguity. So we rename it to
> vhost_set_dev_enable.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   backends/cryptodev-vhost.c        | 12 ++++++------
>   hw/net/vhost_net-stub.c           |  2 +-
>   hw/net/vhost_net.c                |  8 ++++----
>   hw/net/virtio-net.c               |  4 ++--
>   hw/virtio/vhost-user.c            |  4 ++--
>   include/hw/virtio/vhost-backend.h |  6 +++---
>   include/net/vhost_net.h           |  2 +-
>   7 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
> index bc13e466b4..b83e939760 100644
> --- a/backends/cryptodev-vhost.c
> +++ b/backends/cryptodev-vhost.c
> @@ -147,9 +147,9 @@ cryptodev_vhost_set_vq_index(CryptoDevBackendVhost *crypto,
>   }
>   
>   static int
> -vhost_set_vring_enable(CryptoDevBackendClient *cc,
> -                            CryptoDevBackend *b,
> -                            uint16_t queue, int enable)
> +vhost_set_dev_enable(CryptoDevBackendClient *cc,
> +                     CryptoDevBackend *b,
> +                     uint16_t queue, int enable)
>   {
>       CryptoDevBackendVhost *crypto =
>                          cryptodev_get_vhost(cc, b, queue);
> @@ -162,8 +162,8 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
>       }
>   
>       vhost_ops = crypto->dev.vhost_ops;
> -    if (vhost_ops->vhost_set_vring_enable) {
> -        return vhost_ops->vhost_set_vring_enable(&crypto->dev, enable);
> +    if (vhost_ops->vhost_set_dev_enable) {
> +        return vhost_ops->vhost_set_dev_enable(&crypto->dev, enable);
>       }
>   
>       return 0;
> @@ -219,7 +219,7 @@ int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
>   
>           if (cc->vring_enable) {
>               /* restore vring enable state */
> -            r = vhost_set_vring_enable(cc, b, i, cc->vring_enable);
> +            r = vhost_set_dev_enable(cc, b, i, cc->vring_enable);
>   
>               if (r < 0) {
>                   goto err_start;
> diff --git a/hw/net/vhost_net-stub.c b/hw/net/vhost_net-stub.c
> index 89d71cfb8e..ac5f217dc1 100644
> --- a/hw/net/vhost_net-stub.c
> +++ b/hw/net/vhost_net-stub.c
> @@ -92,7 +92,7 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>       return 0;
>   }
>   
> -int vhost_set_vring_enable(NetClientState *nc, int enable)
> +int vhost_set_dev_enable(NetClientState *nc, int enable)
>   {
>       return 0;
>   }
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 97cdf9280b..ea896ea75b 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -396,7 +396,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
>   
>           if (peer->vring_enable) {
>               /* restore vring enable state */
> -            r = vhost_set_vring_enable(peer, peer->vring_enable);
> +            r = vhost_set_dev_enable(peer, peer->vring_enable);
>   
>               if (r < 0) {
>                   vhost_net_stop_one(get_vhost_net(peer), dev);
> @@ -508,15 +508,15 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>       return vhost_net;
>   }
>   
> -int vhost_set_vring_enable(NetClientState *nc, int enable)
> +int vhost_set_dev_enable(NetClientState *nc, int enable)
>   {
>       VHostNetState *net = get_vhost_net(nc);
>       const VhostOps *vhost_ops = net->dev.vhost_ops;
>   
>       nc->vring_enable = enable;
>   
> -    if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
> -        return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
> +    if (vhost_ops && vhost_ops->vhost_set_dev_enable) {
> +        return vhost_ops->vhost_set_dev_enable(&net->dev, enable);
>       }
>   
>       return 0;
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 7817206596..6ab796b399 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -696,7 +696,7 @@ static int peer_attach(VirtIONet *n, int index)
>       }
>   
>       if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> -        vhost_set_vring_enable(nc->peer, 1);
> +        vhost_set_dev_enable(nc->peer, 1);
>       }
>   
>       if (nc->peer->info->type != NET_CLIENT_DRIVER_TAP) {
> @@ -719,7 +719,7 @@ static int peer_detach(VirtIONet *n, int index)
>       }
>   
>       if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> -        vhost_set_vring_enable(nc->peer, 0);
> +        vhost_set_dev_enable(nc->peer, 0);
>       }
>   
>       if (nc->peer->info->type !=  NET_CLIENT_DRIVER_TAP) {
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index bd24741be8..794519359b 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1198,7 +1198,7 @@ static int vhost_user_set_vring_base(struct vhost_dev *dev,
>       return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
>   }
>   
> -static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
> +static int vhost_user_set_dev_enable(struct vhost_dev *dev, int enable)
>   {
>       int i;
>   
> @@ -2627,7 +2627,7 @@ const VhostOps user_ops = {
>           .vhost_set_owner = vhost_user_set_owner,
>           .vhost_reset_device = vhost_user_reset_device,
>           .vhost_get_vq_index = vhost_user_get_vq_index,
> -        .vhost_set_vring_enable = vhost_user_set_vring_enable,
> +        .vhost_set_dev_enable = vhost_user_set_dev_enable,
>           .vhost_requires_shm_log = vhost_user_requires_shm_log,
>           .vhost_migration_done = vhost_user_migration_done,
>           .vhost_backend_can_merge = vhost_user_can_merge,
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index eab46d7f0b..b49432045f 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -81,8 +81,8 @@ typedef int (*vhost_set_backend_cap_op)(struct vhost_dev *dev);
>   typedef int (*vhost_set_owner_op)(struct vhost_dev *dev);
>   typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
>   typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
> -typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
> -                                         int enable);
> +typedef int (*vhost_set_dev_enable_op)(struct vhost_dev *dev,
> +                                       int enable);
>   typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
>   typedef int (*vhost_migration_done_op)(struct vhost_dev *dev,
>                                          char *mac_addr);
> @@ -155,7 +155,7 @@ typedef struct VhostOps {
>       vhost_set_owner_op vhost_set_owner;
>       vhost_reset_device_op vhost_reset_device;
>       vhost_get_vq_index_op vhost_get_vq_index;
> -    vhost_set_vring_enable_op vhost_set_vring_enable;
> +    vhost_set_dev_enable_op vhost_set_dev_enable;
>       vhost_requires_shm_log_op vhost_requires_shm_log;
>       vhost_migration_done_op vhost_migration_done;
>       vhost_backend_can_merge_op vhost_backend_can_merge;
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index 40b9a40074..22a1fcd39e 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -42,7 +42,7 @@ void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
>   int vhost_net_notify_migration_done(VHostNetState *net, char* mac_addr);
>   VHostNetState *get_vhost_net(NetClientState *nc);
>   
> -int vhost_set_vring_enable(NetClientState * nc, int enable);
> +int vhost_set_dev_enable(NetClientState *nc, int enable);
>   
>   uint64_t vhost_net_get_acked_features(VHostNetState *net);
>   

Re: [PATCH v2 2/6] vhost-user: add op to enable or disable a single vring

[Jason Wang]

在 2022/9/12 11:10, Kangjie Xu 写道:
> There is only vhost_set_dev_enable op in VhostOps. Thus, we introduce
> the interface vhost_set_vring_enable to set the enable status for a
> single vring.
>
> Resetting a single vq will rely on this interface.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   hw/virtio/vhost-user.c            | 25 ++++++++++++++++++-------
>   include/hw/virtio/vhost-backend.h |  3 +++
>   2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 794519359b..3f140d5085 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1198,6 +1198,22 @@ static int vhost_user_set_vring_base(struct vhost_dev *dev,
>       return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
>   }
>   
> +static int vhost_user_set_vring_enable(struct vhost_dev *dev,
> +                                       int index,
> +                                       int enable)
> +{
> +    if (index < dev->vq_index || index >= dev->vq_index + dev->nvqs) {
> +        return -EINVAL;
> +    }
> +
> +    struct vhost_vring_state state = {
> +        .index = index,
> +        .num   = enable,
> +    };
> +
> +    return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state);
> +}
> +
>   static int vhost_user_set_dev_enable(struct vhost_dev *dev, int enable)
>   {
>       int i;
> @@ -1207,13 +1223,7 @@ static int vhost_user_set_dev_enable(struct vhost_dev *dev, int enable)
>       }
>   
>       for (i = 0; i < dev->nvqs; ++i) {
> -        int ret;
> -        struct vhost_vring_state state = {
> -            .index = dev->vq_index + i,
> -            .num   = enable,
> -        };
> -
> -        ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state);
> +        int ret = vhost_user_set_vring_enable(dev, dev->vq_index + i, enable);
>           if (ret < 0) {
>               /*
>                * Restoring the previous state is likely infeasible, as well as
> @@ -2627,6 +2637,7 @@ const VhostOps user_ops = {
>           .vhost_set_owner = vhost_user_set_owner,
>           .vhost_reset_device = vhost_user_reset_device,
>           .vhost_get_vq_index = vhost_user_get_vq_index,
> +        .vhost_set_vring_enable = vhost_user_set_vring_enable,
>           .vhost_set_dev_enable = vhost_user_set_dev_enable,
>           .vhost_requires_shm_log = vhost_user_requires_shm_log,
>           .vhost_migration_done = vhost_user_migration_done,
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index b49432045f..dad7191bac 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -81,6 +81,8 @@ typedef int (*vhost_set_backend_cap_op)(struct vhost_dev *dev);
>   typedef int (*vhost_set_owner_op)(struct vhost_dev *dev);
>   typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
>   typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
> +typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
> +                                         int index, int enable);
>   typedef int (*vhost_set_dev_enable_op)(struct vhost_dev *dev,
>                                          int enable);
>   typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
> @@ -155,6 +157,7 @@ typedef struct VhostOps {
>       vhost_set_owner_op vhost_set_owner;
>       vhost_reset_device_op vhost_reset_device;
>       vhost_get_vq_index_op vhost_get_vq_index;
> +    vhost_set_vring_enable_op vhost_set_vring_enable;
>       vhost_set_dev_enable_op vhost_set_dev_enable;
>       vhost_requires_shm_log_op vhost_requires_shm_log;
>       vhost_migration_done_op vhost_migration_done;

Re: [PATCH v2 3/6] vhost-net: vhost-user: update vhost_net_virtqueue_reset()

[Jason Wang]

在 2022/9/12 11:10, Kangjie Xu 写道:
> Update vhost_net_virtqueue_reset() for vhost-user scenario.
>
> In order to reuse some functions, we process the idx for
> vhost-user scenario because vhost_get_vq_index behave
> differently for vhost-user.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   hw/net/vhost_net.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index ea896ea75b..25e5665489 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -545,6 +545,9 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
>       assert(vhost_ops);
>   
>       idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> +    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> +        idx -= net->dev.vq_index;
> +    }


This looks tricky. Any reason we can't simply use vq_index for both 
vhost-user and vhost-net?

Thanks


>   
>       if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
>           file.index = idx;

Re: [PATCH v2 5/6] virtio-net: vhost-user: update queue_reset and queue_enable

[Jason Wang]

在 2022/9/12 11:10, Kangjie Xu 写道:
> Update virtio_net_queue_reset() and virtio_net_queue_enable()
> for vhost-user scenario.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   hw/net/virtio-net.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 6ab796b399..19a2132180 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -550,7 +550,8 @@ static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
>       }
>   
>       if (get_vhost_net(nc->peer) &&
> -        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
> +        (nc->peer->info->type == NET_CLIENT_DRIVER_TAP ||
> +         nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER)) {
>           vhost_net_virtqueue_reset(vdev, nc, queue_index);
>       }
>   
> @@ -568,7 +569,8 @@ static void virtio_net_queue_enable(VirtIODevice *vdev, uint32_t queue_index)
>       }
>   
>       if (get_vhost_net(nc->peer) &&
> -        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
> +        (nc->peer->info->type == NET_CLIENT_DRIVER_TAP ||
> +         nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER)) {
>           r = vhost_net_virtqueue_restart(vdev, nc, queue_index);
>           if (r < 0) {
>               error_report("unable to restart vhost net virtqueue: %d, "

Re: [PATCH v2 6/6] vhost: vhost-user: enable vq reset feature

[Jason Wang]

在 2022/9/12 11:10, Kangjie Xu 写道:
> Add virtqueue reset feature for vhost-user.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   hw/net/vhost_net.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 8b80942e7c..ad319faee8 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -74,6 +74,7 @@ static const int user_feature_bits[] = {
>       VIRTIO_NET_F_MTU,
>       VIRTIO_F_IOMMU_PLATFORM,
>       VIRTIO_F_RING_PACKED,
> +    VIRTIO_F_RING_RESET,
>       VIRTIO_NET_F_RSS,
>       VIRTIO_NET_F_HASH_REPORT,
>   

Re: [PATCH v2 3/6] vhost-net: vhost-user: update vhost_net_virtqueue_reset()

[Xuan Zhuo]

On Wed, 14 Sep 2022 11:13:29 +0800, Jason Wang <jasowang@redhat.com> wrote:
>
> 在 2022/9/12 11:10, Kangjie Xu 写道:
> > Update vhost_net_virtqueue_reset() for vhost-user scenario.
> >
> > In order to reuse some functions, we process the idx for
> > vhost-user scenario because vhost_get_vq_index behave
> > differently for vhost-user.
> >
> > Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >   hw/net/vhost_net.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index ea896ea75b..25e5665489 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -545,6 +545,9 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> >       assert(vhost_ops);
> >
> >       idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> > +    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > +        idx -= net->dev.vq_index;
> > +    }
>
>
> This looks tricky. Any reason we can't simply use vq_index for both
> vhost-user and vhost-net?


static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
{
    assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);

    return idx;
}


static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
{
    assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);

    return idx - dev->vq_index;
}

The implementation of these two callbacks is different. The structure of the two
scenarios is different. We may need to do some optimizations in the future.

Thanks.


>
> Thanks
>
>
> >
> >       if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> >           file.index = idx;
>

Re: [PATCH v2 3/6] vhost-net: vhost-user: update vhost_net_virtqueue_reset()

[Jason Wang]

On Wed, Sep 14, 2022 at 2:21 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Wed, 14 Sep 2022 11:13:29 +0800, Jason Wang <jasowang@redhat.com> wrote:
> >
> > 在 2022/9/12 11:10, Kangjie Xu 写道:
> > > Update vhost_net_virtqueue_reset() for vhost-user scenario.
> > >
> > > In order to reuse some functions, we process the idx for
> > > vhost-user scenario because vhost_get_vq_index behave
> > > differently for vhost-user.
> > >
> > > Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > ---
> > >   hw/net/vhost_net.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index ea896ea75b..25e5665489 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -545,6 +545,9 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> > >       assert(vhost_ops);
> > >
> > >       idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> > > +    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > > +        idx -= net->dev.vq_index;
> > > +    }
> >
> >
> > This looks tricky. Any reason we can't simply use vq_index for both
> > vhost-user and vhost-net?
>
>
> static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
> {
>     assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
>
>     return idx;
> }
>
>
> static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> {
>     assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
>
>     return idx - dev->vq_index;
> }
>
> The implementation of these two callbacks is different. The structure of the two
> scenarios is different. We may need to do some optimizations in the future.

Yes, but what I meant is, you do

idx -= net->dev.vq_index;

and then

net->dev.vq_index + idx

This is a hint that we should have a better organization of the code.

Thanks

>
> Thanks.
>
>
> >
> > Thanks
> >
> >
> > >
> > >       if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> > >           file.index = idx;
> >
>

Re: [PATCH v2 3/6] vhost-net: vhost-user: update vhost_net_virtqueue_reset()

[Xuan Zhuo]

On Thu, 15 Sep 2022 10:12:11 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Wed, Sep 14, 2022 at 2:21 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Wed, 14 Sep 2022 11:13:29 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > 在 2022/9/12 11:10, Kangjie Xu 写道:
> > > > Update vhost_net_virtqueue_reset() for vhost-user scenario.
> > > >
> > > > In order to reuse some functions, we process the idx for
> > > > vhost-user scenario because vhost_get_vq_index behave
> > > > differently for vhost-user.
> > > >
> > > > Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > ---
> > > >   hw/net/vhost_net.c | 3 +++
> > > >   1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > index ea896ea75b..25e5665489 100644
> > > > --- a/hw/net/vhost_net.c
> > > > +++ b/hw/net/vhost_net.c
> > > > @@ -545,6 +545,9 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> > > >       assert(vhost_ops);
> > > >
> > > >       idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> > > > +    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > > > +        idx -= net->dev.vq_index;
> > > > +    }
> > >
> > >
> > > This looks tricky. Any reason we can't simply use vq_index for both
> > > vhost-user and vhost-net?
> >
> >
> > static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
> > {
> >     assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> >
> >     return idx;
> > }
> >
> >
> > static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> > {
> >     assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> >
> >     return idx - dev->vq_index;
> > }
> >
> > The implementation of these two callbacks is different. The structure of the two
> > scenarios is different. We may need to do some optimizations in the future.
>
> Yes, but what I meant is, you do
>
> idx -= net->dev.vq_index;
>
> and then
>
> net->dev.vq_index + idx
>
> This is a hint that we should have a better organization of the code.

I see.

Will fix this.

Thanks.


>
> Thanks
>
> >
> > Thanks.
> >
> >
> > >
> > > Thanks
> > >
> > >
> > > >
> > > >       if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> > > >           file.index = idx;
> > >
> >
>

Re: [PATCH v2 3/6] vhost-net: vhost-user: update vhost_net_virtqueue_reset()

[Xuan Zhuo]

On Thu, 15 Sep 2022 10:12:11 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Wed, Sep 14, 2022 at 2:21 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Wed, 14 Sep 2022 11:13:29 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > 在 2022/9/12 11:10, Kangjie Xu 写道:
> > > > Update vhost_net_virtqueue_reset() for vhost-user scenario.
> > > >
> > > > In order to reuse some functions, we process the idx for
> > > > vhost-user scenario because vhost_get_vq_index behave
> > > > differently for vhost-user.
> > > >
> > > > Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > ---
> > > >   hw/net/vhost_net.c | 3 +++
> > > >   1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > index ea896ea75b..25e5665489 100644
> > > > --- a/hw/net/vhost_net.c
> > > > +++ b/hw/net/vhost_net.c
> > > > @@ -545,6 +545,9 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> > > >       assert(vhost_ops);
> > > >
> > > >       idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> > > > +    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > > > +        idx -= net->dev.vq_index;
> > > > +    }
> > >
> > >
> > > This looks tricky. Any reason we can't simply use vq_index for both
> > > vhost-user and vhost-net?
> >
> >
> > static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
> > {
> >     assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> >
> >     return idx;
> > }
> >
> >
> > static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> > {
> >     assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> >
> >     return idx - dev->vq_index;
> > }
> >
> > The implementation of these two callbacks is different. The structure of the two
> > scenarios is different. We may need to do some optimizations in the future.
>
> Yes, but what I meant is, you do
>
> idx -= net->dev.vq_index;
>
> and then
>
> net->dev.vq_index + idx
>
> This is a hint that we should have a better organization of the code.

Rethink about this.

If I don't do this "idx -= net->dev.vq_index".

Then, it is necessary to call vhost_virtqueue_stop() separately and once again
"net->dev.vqs + idx - net->dev.vq_index"

    vhost_virtqueue_stop(&net->dev,
                         vdev,
                         net->dev.vqs + idx - net->dev.vq_index,
                         idx);

Thanks.


>
> Thanks
>
> >
> > Thanks.
> >
> >
> > >
> > > Thanks
> > >
> > >
> > > >
> > > >       if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> > > >           file.index = idx;
> > >
> >
>