[PATCH v2] virtio-net: Add queues for RSS during migration

Akihiko Odaki posted 1 patch 8 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250530-n-v2-1-b1be546ca586@daynix.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
include/hw/virtio/virtio.h | 10 ++++++++--
hw/net/virtio-net.c        | 11 ++++-------
hw/virtio/virtio.c         | 14 +++++++-------
3 files changed, 19 insertions(+), 16 deletions(-)
[PATCH v2] virtio-net: Add queues for RSS during migration
Posted by Akihiko Odaki 8 months, 2 weeks ago
virtio_net_pre_load_queues() inspects vdev->guest_features to tell if
VIRTIO_NET_F_RSS or VIRTIO_NET_F_MQ is enabled to infer the required
number of queues. This works for VIRTIO_NET_F_MQ but it doesn't for
VIRTIO_NET_F_RSS because only the lowest 32 bits of vdev->guest_features
is set at the point and VIRTIO_NET_F_RSS uses bit 60 while
VIRTIO_NET_F_MQ uses bit 22.

Instead of inferring the required number of queues from
vdev->guest_features, use the number loaded from the vm state. This
change also has a nice side effect to remove a duplicate peer queue
pair change by circumventing virtio_net_set_multiqueue().

Also update the comment in include/hw/virtio/virtio.h to prevent an
implementation of pre_load_queues() from refering to any fields being
loaded during migration by accident in the future.

Fixes: 8c49756825da ("virtio-net: Add only one queue pair when realizing")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v2:
- Updated a documentation comment of pre_load_queues() for clarity.
- Link to v1: https://lore.kernel.org/qemu-devel/20250510-n-v1-1-19ee26ac3ca6@daynix.com
---
 include/hw/virtio/virtio.h | 10 ++++++++--
 hw/net/virtio-net.c        | 11 ++++-------
 hw/virtio/virtio.c         | 14 +++++++-------
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 214d4a77e932..c594764f23f4 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -210,8 +210,14 @@ struct VirtioDeviceClass {
     void (*guest_notifier_mask)(VirtIODevice *vdev, int n, bool mask);
     int (*start_ioeventfd)(VirtIODevice *vdev);
     void (*stop_ioeventfd)(VirtIODevice *vdev);
-    /* Called before loading queues. Useful to add queues before loading. */
-    int (*pre_load_queues)(VirtIODevice *vdev);
+    /*
+     * Called before loading queues.
+     * If the number of queues change at runtime, use @n to know the
+     * number and add or remove queues accordingly.
+     * Note that this function is called in the middle of loading vmsd;
+     * no assumption should be made on states being loaded from vmsd.
+     */
+    int (*pre_load_queues)(VirtIODevice *vdev, uint32_t n);
     /* Saving and loading of a device; trying to deprecate save/load
      * use vmsd for new devices.
      */
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 221252e00a50..5a1aead4e70e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3022,11 +3022,10 @@ static void virtio_net_del_queue(VirtIONet *n, int index)
     virtio_del_queue(vdev, index * 2 + 1);
 }
 
-static void virtio_net_change_num_queue_pairs(VirtIONet *n, int new_max_queue_pairs)
+static void virtio_net_change_num_queues(VirtIONet *n, int new_num_queues)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
     int old_num_queues = virtio_get_num_queues(vdev);
-    int new_num_queues = new_max_queue_pairs * 2 + 1;
     int i;
 
     assert(old_num_queues >= 3);
@@ -3062,16 +3061,14 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue)
     int max = multiqueue ? n->max_queue_pairs : 1;
 
     n->multiqueue = multiqueue;
-    virtio_net_change_num_queue_pairs(n, max);
+    virtio_net_change_num_queues(n, max * 2 + 1);
 
     virtio_net_set_queue_pairs(n);
 }
 
-static int virtio_net_pre_load_queues(VirtIODevice *vdev)
+static int virtio_net_pre_load_queues(VirtIODevice *vdev, uint32_t n)
 {
-    virtio_net_set_multiqueue(VIRTIO_NET(vdev),
-                              virtio_has_feature(vdev->guest_features, VIRTIO_NET_F_RSS) ||
-                              virtio_has_feature(vdev->guest_features, VIRTIO_NET_F_MQ));
+    virtio_net_change_num_queues(VIRTIO_NET(vdev), n);
 
     return 0;
 }
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 2e98cecf64df..bbeccccf6d77 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3259,13 +3259,6 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
         config_len--;
     }
 
-    if (vdc->pre_load_queues) {
-        ret = vdc->pre_load_queues(vdev);
-        if (ret) {
-            return ret;
-        }
-    }
-
     num = qemu_get_be32(f);
 
     if (num > VIRTIO_QUEUE_MAX) {
@@ -3273,6 +3266,13 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
         return -1;
     }
 
+    if (vdc->pre_load_queues) {
+        ret = vdc->pre_load_queues(vdev, num);
+        if (ret) {
+            return ret;
+        }
+    }
+
     for (i = 0; i < num; i++) {
         vdev->vq[i].vring.num = qemu_get_be32(f);
         if (k->has_variable_vring_alignment) {

---
base-commit: f0737158b483e7ec2b2512145aeab888b85cc1f7
change-id: 20250406-n-ae7be0389382

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>
Re: [PATCH v2] virtio-net: Add queues for RSS during migration
Posted by Jason Wang 8 months, 1 week ago
On Fri, May 30, 2025 at 1:19 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> virtio_net_pre_load_queues() inspects vdev->guest_features to tell if
> VIRTIO_NET_F_RSS or VIRTIO_NET_F_MQ is enabled to infer the required
> number of queues. This works for VIRTIO_NET_F_MQ but it doesn't for
> VIRTIO_NET_F_RSS because only the lowest 32 bits of vdev->guest_features
> is set at the point and VIRTIO_NET_F_RSS uses bit 60 while
> VIRTIO_NET_F_MQ uses bit 22.
>
> Instead of inferring the required number of queues from
> vdev->guest_features, use the number loaded from the vm state. This
> change also has a nice side effect to remove a duplicate peer queue
> pair change by circumventing virtio_net_set_multiqueue().
>
> Also update the comment in include/hw/virtio/virtio.h to prevent an
> implementation of pre_load_queues() from refering to any fields being
> loaded during migration by accident in the future.
>
> Fixes: 8c49756825da ("virtio-net: Add only one queue pair when realizing")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> Changes in v2:
> - Updated a documentation comment of pre_load_queues() for clarity.
> - Link to v1: https://lore.kernel.org/qemu-devel/20250510-n-v1-1-19ee26ac3ca6@daynix.com

Queued.

Thanks
Re: [PATCH v2] virtio-net: Add queues for RSS during migration
Posted by Lei Yang 8 months, 1 week ago
Tested this series of patches with virtio-net regression tests,
everything works fine.

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

On Tue, Jun 3, 2025 at 11:01 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, May 30, 2025 at 1:19 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> >
> > virtio_net_pre_load_queues() inspects vdev->guest_features to tell if
> > VIRTIO_NET_F_RSS or VIRTIO_NET_F_MQ is enabled to infer the required
> > number of queues. This works for VIRTIO_NET_F_MQ but it doesn't for
> > VIRTIO_NET_F_RSS because only the lowest 32 bits of vdev->guest_features
> > is set at the point and VIRTIO_NET_F_RSS uses bit 60 while
> > VIRTIO_NET_F_MQ uses bit 22.
> >
> > Instead of inferring the required number of queues from
> > vdev->guest_features, use the number loaded from the vm state. This
> > change also has a nice side effect to remove a duplicate peer queue
> > pair change by circumventing virtio_net_set_multiqueue().
> >
> > Also update the comment in include/hw/virtio/virtio.h to prevent an
> > implementation of pre_load_queues() from refering to any fields being
> > loaded during migration by accident in the future.
> >
> > Fixes: 8c49756825da ("virtio-net: Add only one queue pair when realizing")
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > ---
> > Changes in v2:
> > - Updated a documentation comment of pre_load_queues() for clarity.
> > - Link to v1: https://lore.kernel.org/qemu-devel/20250510-n-v1-1-19ee26ac3ca6@daynix.com
>
> Queued.
>
> Thanks
>
>