[PATCH v2 0/4] Vhost-vdpa Shadow Virtqueue VLAN support

[PATCH v2 0/4] Vhost-vdpa Shadow Virtqueue VLAN support

[Hawkins Jiawei]

This series enables shadowed CVQ to intercept VLAN commands
through shadowed CVQ, update the virtio NIC device model
so qemu send it in a migration, and the restore of that
VLAN state in the destination.

ChangeLog
=========
v2:
 - remove the extra line pointed out by Eugenio in patch 3
"vdpa: Restore vlan filtering state"

v1: https://lore.kernel.org/all/cover.1689690854.git.yin31149@gmail.com/
 - based on patch "[PATCH 0/3] Vhost-vdpa Shadow Virtqueue VLAN support"
at https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg01016.html
 - move `MAX_VLAN` macro to include/hw/virtio/virtio-net.h
instead of net/vhost-vdpa.c
 - fix conflicts with the master branch


TestStep
========
1. test the migration using vp-vdpa device
  - For L0 guest, boot QEMU with two virtio-net-pci net device with
`ctrl_vq`, `ctrl_vlan` features on, command line like:
      -device virtio-net-pci,disable-legacy=on,disable-modern=off,
iommu_platform=on,mq=on,ctrl_vq=on,guest_announce=off,
indirect_desc=off,queue_reset=off,ctrl_vlan=on,...

  - For L1 guest, apply the patch series and compile the source code,
start QEMU with two vdpa device with svq mode on, enable the `ctrl_vq`,
`ctrl_vlan` features on, command line like:
      -netdev type=vhost-vdpa,x-svq=true,...
      -device virtio-net-pci,mq=on,guest_announce=off,ctrl_vq=on,
ctrl_vlan=on,...

  - For L2 source guest, run the following bash command:
```bash
#!/bin/sh

for idx in {1..4094}
do
  ip link add link eth0 name vlan$idx type vlan id $idx
done
```

  - gdb attaches the L2 dest VM and break at the
vhost_vdpa_net_load_single_vlan(), and execute the following
gdbscript
```gdbscript
ignore 1 4094
c
```

  - Execute the live migration in L2 source monitor

  - Result
    * with this series, gdb can hit the breakpoint and continue
the executing without triggering any error or warning.

Eugenio Pérez (1):
  virtio-net: do not reset vlan filtering at set_features

Hawkins Jiawei (3):
  virtio-net: Expose MAX_VLAN
  vdpa: Restore vlan filtering state
  vdpa: Allow VIRTIO_NET_F_CTRL_VLAN in SVQ

 hw/net/virtio-net.c            |  6 +----
 include/hw/virtio/virtio-net.h |  6 +++++
 net/vhost-vdpa.c               | 49 ++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] virtio-net: do not reset vlan filtering at set_features

[Hawkins Jiawei]

From: Eugenio Pérez <eperezma@redhat.com>

This function is called after virtio_load, so all vlan configuration is
lost in migration case.

Just allow all the vlan-tagged packets if vlan is not configured, and
trust device reset to clear all filtered vlans.

Fixes: 0b1eaa8803 ("virtio-net: Do not filter VLANs without F_CTRL_VLAN")
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Reviewed-by: Hawkins Jiawei <yin31149@gmail.com>
Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 hw/net/virtio-net.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 7102ec4817..d20d5a63cd 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1006,9 +1006,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
         vhost_net_save_acked_features(nc->peer);
     }
 
-    if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
-        memset(n->vlans, 0, MAX_VLAN >> 3);
-    } else {
+    if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
         memset(n->vlans, 0xff, MAX_VLAN >> 3);
     }
 
-- 
2.25.1

[PATCH v2 2/4] virtio-net: Expose MAX_VLAN

[Hawkins Jiawei]

vhost-vdpa shadowed CVQ needs to know the maximum number of
vlans supported by the virtio-net device, so QEMU can restore
the VLAN state in a migration.

Co-developed-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 hw/net/virtio-net.c            | 2 --
 include/hw/virtio/virtio-net.h | 6 ++++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d20d5a63cd..a32672039d 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -49,8 +49,6 @@
 
 #define VIRTIO_NET_VM_VERSION    11
 
-#define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
-
 /* previously fixed value */
 #define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256
 #define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 5f5dcb4572..93f3bb5d97 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -38,6 +38,12 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIONet, VIRTIO_NET)
 /* Maximum VIRTIO_NET_CTRL_MAC_TABLE_SET unicast + multicast entries. */
 #define MAC_TABLE_ENTRIES    64
 
+/*
+ * The maximum number of VLANs in the VLAN filter table
+ * added by VIRTIO_NET_CTRL_VLAN_ADD
+ */
+#define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
+
 typedef struct virtio_net_conf
 {
     uint32_t txtimer;
-- 
2.25.1

[PATCH v2 3/4] vdpa: Restore vlan filtering state

[Hawkins Jiawei]

This patch introduces vhost_vdpa_net_load_single_vlan()
and vhost_vdpa_net_load_vlan() to restore the vlan
filtering state at device's startup.

Co-developed-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
v2:
 - remove the extra line pointed out by Eugenio

v1: https://lore.kernel.org/all/0a568cc8a8d2b750c2e09b2237e9f05cece07c3f.1689690854.git.yin31149@gmail.com/

 net/vhost-vdpa.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 9795306742..347241796d 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -965,6 +965,50 @@ static int vhost_vdpa_net_load_rx(VhostVDPAState *s,
     return 0;
 }
 
+static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
+                                           const VirtIONet *n,
+                                           uint16_t vid)
+{
+    const struct iovec data = {
+        .iov_base = &vid,
+        .iov_len = sizeof(vid),
+    };
+    ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN,
+                                                  VIRTIO_NET_CTRL_VLAN_ADD,
+                                                  &data, 1);
+    if (unlikely(dev_written < 0)) {
+        return dev_written;
+    }
+    if (unlikely(*s->status != VIRTIO_NET_OK)) {
+        return -EIO;
+    }
+
+    return 0;
+}
+
+static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
+                                    const VirtIONet *n)
+{
+    int r;
+
+    if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_VLAN)) {
+        return 0;
+    }
+
+    for (int i = 0; i < MAX_VLAN >> 5; i++) {
+        for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
+            if (n->vlans[i] & (1U << j)) {
+                r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j);
+                if (unlikely(r != 0)) {
+                    return r;
+                }
+            }
+        }
+    }
+
+    return 0;
+}
+
 static int vhost_vdpa_net_load(NetClientState *nc)
 {
     VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
@@ -995,6 +1039,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
     if (unlikely(r)) {
         return r;
     }
+    r = vhost_vdpa_net_load_vlan(s, n);
+    if (unlikely(r)) {
+        return r;
+    }
 
     return 0;
 }
-- 
2.25.1

[PATCH v2 4/4] vdpa: Allow VIRTIO_NET_F_CTRL_VLAN in SVQ

[Hawkins Jiawei]

Enable SVQ with VIRTIO_NET_F_CTRL_VLAN feature.

Co-developed-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 net/vhost-vdpa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 347241796d..73e9063fa0 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -111,6 +111,7 @@ static const uint64_t vdpa_svq_device_features =
     BIT_ULL(VIRTIO_NET_F_STATUS) |
     BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |
     BIT_ULL(VIRTIO_NET_F_CTRL_RX) |
+    BIT_ULL(VIRTIO_NET_F_CTRL_VLAN) |
     BIT_ULL(VIRTIO_NET_F_CTRL_RX_EXTRA) |
     BIT_ULL(VIRTIO_NET_F_MQ) |
     BIT_ULL(VIRTIO_F_ANY_LAYOUT) |
-- 
2.25.1

Re: [PATCH v2 1/4] virtio-net: do not reset vlan filtering at set_features

[Hawkins Jiawei]

On 2023/7/23 17:26, Hawkins Jiawei wrote:
> From: Eugenio Pérez <eperezma@redhat.com>

There was a wrong "From" line by mistake, I will send the v3 patch to
fix this.

Thanks!


>
> This function is called after virtio_load, so all vlan configuration is
> lost in migration case.
>
> Just allow all the vlan-tagged packets if vlan is not configured, and
> trust device reset to clear all filtered vlans.
>
> Fixes: 0b1eaa8803 ("virtio-net: Do not filter VLANs without F_CTRL_VLAN")
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Reviewed-by: Hawkins Jiawei <yin31149@gmail.com>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
> ---
>   hw/net/virtio-net.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 7102ec4817..d20d5a63cd 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1006,9 +1006,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
>           vhost_net_save_acked_features(nc->peer);
>       }
>
> -    if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
> -        memset(n->vlans, 0, MAX_VLAN >> 3);
> -    } else {
> +    if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
>           memset(n->vlans, 0xff, MAX_VLAN >> 3);
>       }
>

Re: [PATCH v2 1/4] virtio-net: do not reset vlan filtering at set_features

[Jason Wang]

On Sun, Jul 23, 2023 at 5:27 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> From: Eugenio Pérez <eperezma@redhat.com>
>
> This function is called after virtio_load, so all vlan configuration is
> lost in migration case.
>
> Just allow all the vlan-tagged packets if vlan is not configured, and
> trust device reset to clear all filtered vlans.
>
> Fixes: 0b1eaa8803 ("virtio-net: Do not filter VLANs without F_CTRL_VLAN")
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Reviewed-by: Hawkins Jiawei <yin31149@gmail.com>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>

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

Thanks

> ---
>  hw/net/virtio-net.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 7102ec4817..d20d5a63cd 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1006,9 +1006,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
>          vhost_net_save_acked_features(nc->peer);
>      }
>
> -    if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
> -        memset(n->vlans, 0, MAX_VLAN >> 3);
> -    } else {
> +    if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
>          memset(n->vlans, 0xff, MAX_VLAN >> 3);
>      }
>
> --
> 2.25.1
>

Re: [PATCH v2 3/4] vdpa: Restore vlan filtering state

[Jason Wang]

On Sun, Jul 23, 2023 at 5:28 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> This patch introduces vhost_vdpa_net_load_single_vlan()
> and vhost_vdpa_net_load_vlan() to restore the vlan
> filtering state at device's startup.
>
> Co-developed-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>

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

But this seems to be a source of latency killer as it may at most send
1024 commands.

As discussed in the past, we need a better cvq command to do this: for
example, a single command to carray a bitmap.

Thanks

> ---
> v2:
>  - remove the extra line pointed out by Eugenio
>
> v1: https://lore.kernel.org/all/0a568cc8a8d2b750c2e09b2237e9f05cece07c3f.1689690854.git.yin31149@gmail.com/
>
>  net/vhost-vdpa.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 9795306742..347241796d 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -965,6 +965,50 @@ static int vhost_vdpa_net_load_rx(VhostVDPAState *s,
>      return 0;
>  }
>
> +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
> +                                           const VirtIONet *n,
> +                                           uint16_t vid)
> +{
> +    const struct iovec data = {
> +        .iov_base = &vid,
> +        .iov_len = sizeof(vid),
> +    };
> +    ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN,
> +                                                  VIRTIO_NET_CTRL_VLAN_ADD,
> +                                                  &data, 1);
> +    if (unlikely(dev_written < 0)) {
> +        return dev_written;
> +    }
> +    if (unlikely(*s->status != VIRTIO_NET_OK)) {
> +        return -EIO;
> +    }
> +
> +    return 0;
> +}
> +
> +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
> +                                    const VirtIONet *n)
> +{
> +    int r;
> +
> +    if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_VLAN)) {
> +        return 0;
> +    }
> +
> +    for (int i = 0; i < MAX_VLAN >> 5; i++) {
> +        for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
> +            if (n->vlans[i] & (1U << j)) {
> +                r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j);
> +                if (unlikely(r != 0)) {
> +                    return r;
> +                }
> +            }
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  static int vhost_vdpa_net_load(NetClientState *nc)
>  {
>      VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> @@ -995,6 +1039,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
>      if (unlikely(r)) {
>          return r;
>      }
> +    r = vhost_vdpa_net_load_vlan(s, n);
> +    if (unlikely(r)) {
> +        return r;
> +    }
>
>      return 0;
>  }
> --
> 2.25.1
>

Re: [PATCH v2 2/4] virtio-net: Expose MAX_VLAN

[Jason Wang]

On Sun, Jul 23, 2023 at 5:27 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> vhost-vdpa shadowed CVQ needs to know the maximum number of
> vlans supported by the virtio-net device, so QEMU can restore
> the VLAN state in a migration.
>
> Co-developed-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>

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

Thanks

> ---
>  hw/net/virtio-net.c            | 2 --
>  include/hw/virtio/virtio-net.h | 6 ++++++
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index d20d5a63cd..a32672039d 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -49,8 +49,6 @@
>
>  #define VIRTIO_NET_VM_VERSION    11
>
> -#define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
> -
>  /* previously fixed value */
>  #define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256
>  #define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index 5f5dcb4572..93f3bb5d97 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -38,6 +38,12 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIONet, VIRTIO_NET)
>  /* Maximum VIRTIO_NET_CTRL_MAC_TABLE_SET unicast + multicast entries. */
>  #define MAC_TABLE_ENTRIES    64
>
> +/*
> + * The maximum number of VLANs in the VLAN filter table
> + * added by VIRTIO_NET_CTRL_VLAN_ADD
> + */
> +#define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
> +
>  typedef struct virtio_net_conf
>  {
>      uint32_t txtimer;
> --
> 2.25.1
>

Re: [PATCH v2 4/4] vdpa: Allow VIRTIO_NET_F_CTRL_VLAN in SVQ

[Jason Wang]

On Sun, Jul 23, 2023 at 5:27 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> Enable SVQ with VIRTIO_NET_F_CTRL_VLAN feature.
>
> Co-developed-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>

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

Thanks

> ---
>  net/vhost-vdpa.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 347241796d..73e9063fa0 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -111,6 +111,7 @@ static const uint64_t vdpa_svq_device_features =
>      BIT_ULL(VIRTIO_NET_F_STATUS) |
>      BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |
>      BIT_ULL(VIRTIO_NET_F_CTRL_RX) |
> +    BIT_ULL(VIRTIO_NET_F_CTRL_VLAN) |
>      BIT_ULL(VIRTIO_NET_F_CTRL_RX_EXTRA) |
>      BIT_ULL(VIRTIO_NET_F_MQ) |
>      BIT_ULL(VIRTIO_F_ANY_LAYOUT) |
> --
> 2.25.1
>

Re: [PATCH v2 3/4] vdpa: Restore vlan filtering state

[Hawkins Jiawei]

On 2023/7/25 14:47, Jason Wang wrote:
> On Sun, Jul 23, 2023 at 5:28 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>>
>> This patch introduces vhost_vdpa_net_load_single_vlan()
>> and vhost_vdpa_net_load_vlan() to restore the vlan
>> filtering state at device's startup.
>>
>> Co-developed-by: Eugenio Pérez <eperezma@redhat.com>
>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
>
> Acked-by: Jason Wang <jasowang@redhat.com>
>
> But this seems to be a source of latency killer as it may at most send
> 1024 commands.
>
> As discussed in the past, we need a better cvq command to do this: for
> example, a single command to carray a bitmap.

Hi Jason,

Thanks for your review.

You are right, we need some improvement here.

Therefore, I have submitted another patch series titled "vdpa: Send all
CVQ state load commands in parallel" at [1], which allows QEMU to delay
polling and checking the device used buffer until either the SVQ is full
or control commands shadow buffers are full, so that QEMU can send all
the SVQ control commands in parallel, which has better performance
improvement.

To test that patch series, I created 4094 VLANS in guest to build an
environment for sending multiple CVQ state load commands. According to
the result on the real vdpa device at [2], this patch series can improve
latency from 23296 us to 6539 us.

Thanks!

[1]. https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg03726.html
[2]. https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg03947.html


>
> Thanks
>
>> ---
>> v2:
>>   - remove the extra line pointed out by Eugenio
>>
>> v1: https://lore.kernel.org/all/0a568cc8a8d2b750c2e09b2237e9f05cece07c3f.1689690854.git.yin31149@gmail.com/
>>
>>   net/vhost-vdpa.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 48 insertions(+)
>>
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index 9795306742..347241796d 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -965,6 +965,50 @@ static int vhost_vdpa_net_load_rx(VhostVDPAState *s,
>>       return 0;
>>   }
>>
>> +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
>> +                                           const VirtIONet *n,
>> +                                           uint16_t vid)
>> +{
>> +    const struct iovec data = {
>> +        .iov_base = &vid,
>> +        .iov_len = sizeof(vid),
>> +    };
>> +    ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN,
>> +                                                  VIRTIO_NET_CTRL_VLAN_ADD,
>> +                                                  &data, 1);
>> +    if (unlikely(dev_written < 0)) {
>> +        return dev_written;
>> +    }
>> +    if (unlikely(*s->status != VIRTIO_NET_OK)) {
>> +        return -EIO;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
>> +                                    const VirtIONet *n)
>> +{
>> +    int r;
>> +
>> +    if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_VLAN)) {
>> +        return 0;
>> +    }
>> +
>> +    for (int i = 0; i < MAX_VLAN >> 5; i++) {
>> +        for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
>> +            if (n->vlans[i] & (1U << j)) {
>> +                r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j);
>> +                if (unlikely(r != 0)) {
>> +                    return r;
>> +                }
>> +            }
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int vhost_vdpa_net_load(NetClientState *nc)
>>   {
>>       VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
>> @@ -995,6 +1039,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
>>       if (unlikely(r)) {
>>           return r;
>>       }
>> +    r = vhost_vdpa_net_load_vlan(s, n);
>> +    if (unlikely(r)) {
>> +        return r;
>> +    }
>>
>>       return 0;
>>   }
>> --
>> 2.25.1
>>
>

Re: [PATCH v2 3/4] vdpa: Restore vlan filtering state

[Jason Wang]

On Tue, Jul 25, 2023 at 3:48 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> On 2023/7/25 14:47, Jason Wang wrote:
> > On Sun, Jul 23, 2023 at 5:28 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
> >>
> >> This patch introduces vhost_vdpa_net_load_single_vlan()
> >> and vhost_vdpa_net_load_vlan() to restore the vlan
> >> filtering state at device's startup.
> >>
> >> Co-developed-by: Eugenio Pérez <eperezma@redhat.com>
> >> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> >> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
> >
> > Acked-by: Jason Wang <jasowang@redhat.com>
> >
> > But this seems to be a source of latency killer as it may at most send
> > 1024 commands.
> >
> > As discussed in the past, we need a better cvq command to do this: for
> > example, a single command to carray a bitmap.
>
> Hi Jason,
>
> Thanks for your review.
>
> You are right, we need some improvement here.
>
> Therefore, I have submitted another patch series titled "vdpa: Send all
> CVQ state load commands in parallel" at [1], which allows QEMU to delay
> polling and checking the device used buffer until either the SVQ is full
> or control commands shadow buffers are full, so that QEMU can send all
> the SVQ control commands in parallel, which has better performance
> improvement.
>
> To test that patch series, I created 4094 VLANS in guest to build an
> environment for sending multiple CVQ state load commands. According to
> the result on the real vdpa device at [2], this patch series can improve
> latency from 23296 us to 6539 us.
>
> Thanks!
>
> [1]. https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg03726.html
> [2]. https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg03947.html
>

That's great, and if we can use a single command to get/set vid it
would be still helpful (I meant we can extend the virtio spec).

Thanks

>
> >
> > Thanks
> >
> >> ---
> >> v2:
> >>   - remove the extra line pointed out by Eugenio
> >>
> >> v1: https://lore.kernel.org/all/0a568cc8a8d2b750c2e09b2237e9f05cece07c3f.1689690854.git.yin31149@gmail.com/
> >>
> >>   net/vhost-vdpa.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 48 insertions(+)
> >>
> >> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> >> index 9795306742..347241796d 100644
> >> --- a/net/vhost-vdpa.c
> >> +++ b/net/vhost-vdpa.c
> >> @@ -965,6 +965,50 @@ static int vhost_vdpa_net_load_rx(VhostVDPAState *s,
> >>       return 0;
> >>   }
> >>
> >> +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
> >> +                                           const VirtIONet *n,
> >> +                                           uint16_t vid)
> >> +{
> >> +    const struct iovec data = {
> >> +        .iov_base = &vid,
> >> +        .iov_len = sizeof(vid),
> >> +    };
> >> +    ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN,
> >> +                                                  VIRTIO_NET_CTRL_VLAN_ADD,
> >> +                                                  &data, 1);
> >> +    if (unlikely(dev_written < 0)) {
> >> +        return dev_written;
> >> +    }
> >> +    if (unlikely(*s->status != VIRTIO_NET_OK)) {
> >> +        return -EIO;
> >> +    }
> >> +
> >> +    return 0;
> >> +}
> >> +
> >> +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
> >> +                                    const VirtIONet *n)
> >> +{
> >> +    int r;
> >> +
> >> +    if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_VLAN)) {
> >> +        return 0;
> >> +    }
> >> +
> >> +    for (int i = 0; i < MAX_VLAN >> 5; i++) {
> >> +        for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
> >> +            if (n->vlans[i] & (1U << j)) {
> >> +                r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j);
> >> +                if (unlikely(r != 0)) {
> >> +                    return r;
> >> +                }
> >> +            }
> >> +        }
> >> +    }
> >> +
> >> +    return 0;
> >> +}
> >> +
> >>   static int vhost_vdpa_net_load(NetClientState *nc)
> >>   {
> >>       VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> >> @@ -995,6 +1039,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
> >>       if (unlikely(r)) {
> >>           return r;
> >>       }
> >> +    r = vhost_vdpa_net_load_vlan(s, n);
> >> +    if (unlikely(r)) {
> >> +        return r;
> >> +    }
> >>
> >>       return 0;
> >>   }
> >> --
> >> 2.25.1
> >>
> >
>