[PATCH v4 0/6] Vhost-vdpa Shadow Virtqueue Offloads support

[PATCH v4 0/6] Vhost-vdpa Shadow Virtqueue Offloads support

[Hawkins Jiawei]

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

Changelog
=========
v4:
  - refactor the commit message suggested by Eugenio in patch#4
"virtio-net: expose virtio_net_supported_guest_offloads()"
  - fix the wrong "cpu_to_le64()" pointed out by Eugenio in patch$5
"vdpa: Add vhost_vdpa_net_load_offloads()"
  - refactor the comment in patch#5
"vdpa: Add vhost_vdpa_net_load_offloads()"

v3: https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00206.html

v2: https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00044.html

v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg07198.html

Hawkins Jiawei (6):
  include/hw/virtio: make some VirtIODevice const
  vdpa: reuse virtio_vdev_has_feature()
  hw/net/virtio-net: make some VirtIONet const
  virtio-net: expose virtio_net_supported_guest_offloads()
  vdpa: Add vhost_vdpa_net_load_offloads()
  vdpa: Allow VIRTIO_NET_F_CTRL_GUEST_OFFLOADS in SVQ

 hw/net/virtio-net.c            |  2 +-
 include/hw/virtio/virtio-net.h |  1 +
 include/hw/virtio/virtio.h     |  2 +-
 net/vhost-vdpa.c               | 49 +++++++++++++++++++++++++++++++---
 4 files changed, 48 insertions(+), 6 deletions(-)

-- 
2.25.1

[PATCH v4 1/6] include/hw/virtio: make some VirtIODevice const

[Hawkins Jiawei]

The VirtIODevice structure is not modified in
virtio_vdev_has_feature(). Therefore, make it const
to allow this function to accept const variables.

Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
Reviewed-by: Eugenio Pérez Martin <eperezma@redhat.com>
---
 include/hw/virtio/virtio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index af86ed7249..0492d26900 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -402,7 +402,7 @@ static inline bool virtio_has_feature(uint64_t features, unsigned int fbit)
     return !!(features & (1ULL << fbit));
 }
 
-static inline bool virtio_vdev_has_feature(VirtIODevice *vdev,
+static inline bool virtio_vdev_has_feature(const VirtIODevice *vdev,
                                            unsigned int fbit)
 {
     return virtio_has_feature(vdev->guest_features, fbit);
-- 
2.25.1

[PATCH v4 2/6] vdpa: reuse virtio_vdev_has_feature()

[Hawkins Jiawei]

We can use virtio_vdev_has_feature() instead of manually
accessing the features.

Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
Acked-by: Eugenio Pérez <eperezma@redhat.com>
---
 net/vhost-vdpa.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 37cdc84562..e907a3c792 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -643,8 +643,7 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, uint8_t class,
 
 static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
 {
-    uint64_t features = n->parent_obj.guest_features;
-    if (features & BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+    if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
         ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_MAC,
                                                   VIRTIO_NET_CTRL_MAC_ADDR_SET,
                                                   n->mac, sizeof(n->mac));
@@ -662,10 +661,9 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
                                   const VirtIONet *n)
 {
     struct virtio_net_ctrl_mq mq;
-    uint64_t features = n->parent_obj.guest_features;
     ssize_t dev_written;
 
-    if (!(features & BIT_ULL(VIRTIO_NET_F_MQ))) {
+    if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_MQ)) {
         return 0;
     }
 
-- 
2.25.1

[PATCH v4 3/6] hw/net/virtio-net: make some VirtIONet const

[Hawkins Jiawei]

The VirtIONet structure is not modified in
virtio_net_supported_guest_offloads().
Therefore, make it const to allow this function to
accept const variables.

Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/net/virtio-net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 6df6b7329d..7b27dad6c4 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -874,7 +874,7 @@ static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
     return guest_offloads_mask & features;
 }
 
-static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
+static inline uint64_t virtio_net_supported_guest_offloads(const VirtIONet *n)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
     return virtio_net_guest_offloads_by_features(vdev->guest_features);
-- 
2.25.1

[PATCH v4 4/6] virtio-net: expose virtio_net_supported_guest_offloads()

[Hawkins Jiawei]

To support restoring offloads state in vdpa, it is necessary to
expose the function virtio_net_supported_guest_offloads().

According to VirtIO standard, "Upon feature negotiation
corresponding offload gets enabled to preserve backward compatibility.".
Therefore, QEMU uses this function to get the device supported offloads.
This allows QEMU to know the device's defaults and skip the control
message sending if these defaults align with the driver's configuration.

Note that the device's defaults can mismatch the driver's configuration
only at live migration.

Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 hw/net/virtio-net.c            | 2 +-
 include/hw/virtio/virtio-net.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 7b27dad6c4..7e8897a8bc 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -874,7 +874,7 @@ static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
     return guest_offloads_mask & features;
 }
 
-static inline uint64_t virtio_net_supported_guest_offloads(const VirtIONet *n)
+uint64_t virtio_net_supported_guest_offloads(const VirtIONet *n)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
     return virtio_net_guest_offloads_by_features(vdev->guest_features);
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index ef234ffe7e..5f5dcb4572 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -227,5 +227,6 @@ size_t virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
                                   unsigned out_num);
 void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
                                    const char *type);
+uint64_t virtio_net_supported_guest_offloads(const VirtIONet *n);
 
 #endif
-- 
2.25.1

[PATCH v4 5/6] vdpa: Add vhost_vdpa_net_load_offloads()

[Hawkins Jiawei]

This patch introduces vhost_vdpa_net_load_offloads() to
restore offloads state at device's startup.

Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 net/vhost-vdpa.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index e907a3c792..1a295af51d 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -678,6 +678,44 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
     return *s->status != VIRTIO_NET_OK;
 }
 
+static int vhost_vdpa_net_load_offloads(VhostVDPAState *s,
+                                        const VirtIONet *n)
+{
+    uint64_t offloads;
+    ssize_t dev_written;
+
+    if (!virtio_vdev_has_feature(&n->parent_obj,
+                                 VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) {
+        return 0;
+    }
+
+    if (n->curr_guest_offloads == virtio_net_supported_guest_offloads(n)) {
+        /*
+         * According to VirtIO standard, "Upon feature negotiation
+         * corresponding offload gets enabled to preserve
+         * backward compatibility.".
+         *
+         * Therefore, there is no need to send this CVQ command if the
+         * driver also enables all supported offloads, which aligns with
+         * the device's defaults.
+         *
+         * Note that the device's defaults can mismatch the driver's configuration
+         * only at live migration.
+         */
+        return 0;
+    }
+
+    offloads = cpu_to_le64(n->curr_guest_offloads);
+    dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
+                                          VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET,
+                                          &offloads, sizeof(offloads));
+    if (unlikely(dev_written < 0)) {
+        return dev_written;
+    }
+
+    return *s->status != VIRTIO_NET_OK;
+}
+
 static int vhost_vdpa_net_load(NetClientState *nc)
 {
     VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
@@ -700,6 +738,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
     if (unlikely(r)) {
         return r;
     }
+    r = vhost_vdpa_net_load_offloads(s, n);
+    if (unlikely(r)) {
+        return r;
+    }
 
     return 0;
 }
-- 
2.25.1

[PATCH v4 6/6] vdpa: Allow VIRTIO_NET_F_CTRL_GUEST_OFFLOADS in SVQ

[Hawkins Jiawei]

Enable SVQ with VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature.

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

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 1a295af51d..5a72204899 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -85,6 +85,7 @@ const int vdpa_feature_bits[] = {
 static const uint64_t vdpa_svq_device_features =
     BIT_ULL(VIRTIO_NET_F_CSUM) |
     BIT_ULL(VIRTIO_NET_F_GUEST_CSUM) |
+    BIT_ULL(VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) |
     BIT_ULL(VIRTIO_NET_F_MTU) |
     BIT_ULL(VIRTIO_NET_F_MAC) |
     BIT_ULL(VIRTIO_NET_F_GUEST_TSO4) |
-- 
2.25.1

Re: [PATCH v4 0/6] Vhost-vdpa Shadow Virtqueue Offloads support

[Lei Yang]

QE tested this series v4 on the vp_vdpa device with the following
scenarios: reboot/shutdown, hotplug/unplug, nic driver load/unload,
ping, vDPA control virtqueue(changed the mac address), everything are
working fine. And L1 guest will not appear  error messages "vdpa svq
does not work with features 0x4" after applied this series of patches.

Tested-by: Lei Yang <leiyang@redhat.com>


On Fri, Jun 2, 2023 at 7:55 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> This series enables shadowed CVQ to intercept Offloads commands
> through shadowed CVQ, update the virtio NIC device model so qemu
> send it in a migration, and the restore of that Offloads state
> in the destination.
>
> Changelog
> =========
> v4:
>   - refactor the commit message suggested by Eugenio in patch#4
> "virtio-net: expose virtio_net_supported_guest_offloads()"
>   - fix the wrong "cpu_to_le64()" pointed out by Eugenio in patch$5
> "vdpa: Add vhost_vdpa_net_load_offloads()"
>   - refactor the comment in patch#5
> "vdpa: Add vhost_vdpa_net_load_offloads()"
>
> v3: https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00206.html
>
> v2: https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00044.html
>
> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg07198.html
>
> Hawkins Jiawei (6):
>   include/hw/virtio: make some VirtIODevice const
>   vdpa: reuse virtio_vdev_has_feature()
>   hw/net/virtio-net: make some VirtIONet const
>   virtio-net: expose virtio_net_supported_guest_offloads()
>   vdpa: Add vhost_vdpa_net_load_offloads()
>   vdpa: Allow VIRTIO_NET_F_CTRL_GUEST_OFFLOADS in SVQ
>
>  hw/net/virtio-net.c            |  2 +-
>  include/hw/virtio/virtio-net.h |  1 +
>  include/hw/virtio/virtio.h     |  2 +-
>  net/vhost-vdpa.c               | 49 +++++++++++++++++++++++++++++++---
>  4 files changed, 48 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>
>

Re: [PATCH v4 0/6] Vhost-vdpa Shadow Virtqueue Offloads support

[Eugenio Perez Martin]

On Fri, Jun 2, 2023 at 1:52 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> This series enables shadowed CVQ to intercept Offloads commands
> through shadowed CVQ, update the virtio NIC device model so qemu
> send it in a migration, and the restore of that Offloads state
> in the destination.
>
> Changelog
> =========
> v4:
>   - refactor the commit message suggested by Eugenio in patch#4
> "virtio-net: expose virtio_net_supported_guest_offloads()"
>   - fix the wrong "cpu_to_le64()" pointed out by Eugenio in patch$5
> "vdpa: Add vhost_vdpa_net_load_offloads()"
>   - refactor the comment in patch#5
> "vdpa: Add vhost_vdpa_net_load_offloads()"
>
> v3: https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00206.html
>
> v2: https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00044.html
>
> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg07198.html
>

Please keep all the v2, v3 etc changes here too. A reviewer may see
this newly from an older revision.

Apart from that,
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Tested-by: Eugenio Pérez <eperezma@redhat.com>

I tested it by tracing the value it sends at destination when the
guest enable or disable hw GRO offloads with:
ethtool -K rx-gro-hw off.

As you point in previous series, a migration blocker needs to be
conditionally added. This is done in [1].

While investigating this I discovered two related issues, it might be
in handy if other reviewer wants to test the changes [2][3].

Thanks!

[1] https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00512.html
[2] https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00601.html
[3] https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00604.html

> Hawkins Jiawei (6):
>   include/hw/virtio: make some VirtIODevice const
>   vdpa: reuse virtio_vdev_has_feature()
>   hw/net/virtio-net: make some VirtIONet const
>   virtio-net: expose virtio_net_supported_guest_offloads()
>   vdpa: Add vhost_vdpa_net_load_offloads()
>   vdpa: Allow VIRTIO_NET_F_CTRL_GUEST_OFFLOADS in SVQ
>
>  hw/net/virtio-net.c            |  2 +-
>  include/hw/virtio/virtio-net.h |  1 +
>  include/hw/virtio/virtio.h     |  2 +-
>  net/vhost-vdpa.c               | 49 +++++++++++++++++++++++++++++++---
>  4 files changed, 48 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>

Re: [PATCH v4 0/6] Vhost-vdpa Shadow Virtqueue Offloads support

[Hawkins Jiawei]

On 2023/6/3 1:47, Eugenio Perez Martin wrote:
> On Fri, Jun 2, 2023 at 1:52 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>>
>> This series enables shadowed CVQ to intercept Offloads commands
>> through shadowed CVQ, update the virtio NIC device model so qemu
>> send it in a migration, and the restore of that Offloads state
>> in the destination.
>>
>> Changelog
>> =========
>> v4:
>>    - refactor the commit message suggested by Eugenio in patch#4
>> "virtio-net: expose virtio_net_supported_guest_offloads()"
>>    - fix the wrong "cpu_to_le64()" pointed out by Eugenio in patch$5
>> "vdpa: Add vhost_vdpa_net_load_offloads()"
>>    - refactor the comment in patch#5
>> "vdpa: Add vhost_vdpa_net_load_offloads()"
>>
>> v3: https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00206.html
>>
>> v2: https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00044.html
>>
>> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg07198.html
>>
>
> Please keep all the v2, v3 etc changes here too. A reviewer may see
> this newly from an older revision.

Thank you for the reminder, I will include all change log in the
upcoming patches.

>
> Apart from that,
> Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
> Tested-by: Eugenio Pérez <eperezma@redhat.com>
>
> I tested it by tracing the value it sends at destination when the
> guest enable or disable hw GRO offloads with:
> ethtool -K rx-gro-hw off.
>
> As you point in previous series, a migration blocker needs to be
> conditionally added. This is done in [1].
>
> While investigating this I discovered two related issues, it might be
> in handy if other reviewer wants to test the changes [2][3].
>
> Thanks!
>
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00512.html
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00601.html
> [3] https://lists.nongnu.org/archive/html/qemu-devel/2023-06/msg00604.html

Thanks for your explanation!


>
>> Hawkins Jiawei (6):
>>    include/hw/virtio: make some VirtIODevice const
>>    vdpa: reuse virtio_vdev_has_feature()
>>    hw/net/virtio-net: make some VirtIONet const
>>    virtio-net: expose virtio_net_supported_guest_offloads()
>>    vdpa: Add vhost_vdpa_net_load_offloads()
>>    vdpa: Allow VIRTIO_NET_F_CTRL_GUEST_OFFLOADS in SVQ
>>
>>   hw/net/virtio-net.c            |  2 +-
>>   include/hw/virtio/virtio-net.h |  1 +
>>   include/hw/virtio/virtio.h     |  2 +-
>>   net/vhost-vdpa.c               | 49 +++++++++++++++++++++++++++++++---
>>   4 files changed, 48 insertions(+), 6 deletions(-)
>>
>> --
>> 2.25.1
>>
>