[PULL 03/23] hw/net/virtio-net: make VirtIONet.vlans an array instead of a pointer

Philippe Mathieu-Daudé posted 23 patches 3 months, 2 weeks ago
Maintainers: "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, Richard Henderson <richard.henderson@linaro.org>, Peter Maydell <peter.maydell@linaro.org>, Antony Pavlov <antonynpavlov@gmail.com>, Rob Herring <robh@kernel.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Alberto Garcia <berto@igalia.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Alistair Francis <alistair@alistair23.me>, "Alex Bennée" <alex.bennee@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Helge Deller <deller@gmx.de>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Corey Minyard <minyard@acm.org>, Song Gao <gaosong@loongson.cn>, Bibo Mao <maobibo@loongson.cn>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Thomas Huth <huth@tuxfamily.org>, Laurent Vivier <laurent@vivier.eu>, Paul Burton <paulburton@kernel.org>, Aleksandar Rikalo <arikalo@gmail.com>, Huacai Chen <chenhuacai@kernel.org>, "Hervé Poussineau" <hpoussin@reactos.org>, Aurelien Jarno <aurelien@aurel32.net>, Bernhard Beschow <shentey@gmail.com>, Jason Wang <jasowang@redhat.com>, Stafford Horne <shorne@gmail.com>, BALATON Zoltan <balaton@eik.bme.hu>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Aditya Gupta <adityag@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>, Palmer Dabbelt <palmer@dabbelt.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Jared Rossi <jrossi@linux.ibm.com>, Zhuoying Cai <zycai@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Magnus Damm <magnus.damm@gmail.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, "Clément Chigot" <chigot@adacore.com>, Frederic Konrad <konrad.frederic@yahoo.fr>, Artyom Tarasenko <atar4qemu@gmail.com>, Max Filippov <jcmvbkbc@gmail.com>, Yi Liu <yi.l.liu@intel.com>, "Clément Mathieu--Drif" <clement.mathieu--drif@eviden.com>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Fam Zheng <fam@euphon.net>, Gerd Hoffmann <kraxel@redhat.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Amit Shah <amit@kernel.org>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Pavel Pisa <pisa@cmp.felk.cvut.cz>, Francisco Iglesias <francisco.iglesias@amd.com>, Vikram Garhwal <vikram.garhwal@bytedance.com>, David Gibson <david@gibson.dropbear.id.au>
[PULL 03/23] hw/net/virtio-net: make VirtIONet.vlans an array instead of a pointer
Posted by Philippe Mathieu-Daudé 3 months, 2 weeks ago
From: Michael Tokarev <mjt@tls.msk.ru>

This field is a fixed-size buffer (number of elements is MAX_VLAN,
known at build time).  There's no need to allocate it dynamically,
it can be made an integral part of VirtIONet structure.

This field is the only user of VMSTATE_BUFFER_POINTER_UNSAFE() macro.

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Tested-by: Lei Yang <leiyang@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Message-ID: <20251023135316.31128-2-mjt@tls.msk.ru>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/virtio/virtio-net.h | 2 +-
 hw/net/virtio-net.c            | 9 ++++-----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 5b8ab7bda79..f7083553068 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -202,7 +202,7 @@ struct VirtIONet {
         uint8_t uni_overflow;
         uint8_t *macs;
     } mac_table;
-    uint32_t *vlans;
+    uint32_t vlans[MAX_VLAN];
     virtio_net_conf net_conf;
     NICConf nic_conf;
     DeviceState *qdev;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 33116712eb4..17ed0ef9190 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -986,7 +986,7 @@ static void virtio_net_set_features(VirtIODevice *vdev,
         virtio_has_feature_ex(vdev->guest_features_ex,
                               VIRTIO_NET_F_CTRL_VLAN)) {
         bool vlan = virtio_has_feature_ex(features, VIRTIO_NET_F_CTRL_VLAN);
-        memset(n->vlans, vlan ? 0 : 0xff, MAX_VLAN >> 3);
+        memset(n->vlans, vlan ? 0 : 0xff, sizeof(n->vlans));
     }
 
     if (virtio_has_feature_ex(features, VIRTIO_NET_F_STANDBY)) {
@@ -3600,7 +3600,8 @@ static const VMStateDescription vmstate_virtio_net_device = {
          * buffer; hold onto your endiannesses; it's actually used as a bitmap
          * but based on the uint.
          */
-        VMSTATE_BUFFER_POINTER_UNSAFE(vlans, VirtIONet, 0, MAX_VLAN >> 3),
+        VMSTATE_BUFFER_UNSAFE(vlans, VirtIONet, 0,
+                              sizeof(typeof_field(VirtIONet, vlans))),
         VMSTATE_WITH_TMP(VirtIONet, struct VirtIONetMigTmp,
                          vmstate_virtio_net_has_vnet),
         VMSTATE_UINT8(mac_table.multi_overflow, VirtIONet),
@@ -4018,8 +4019,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
 
     n->mac_table.macs = g_malloc0(MAC_TABLE_ENTRIES * ETH_ALEN);
 
-    n->vlans = g_malloc0(MAX_VLAN >> 3);
-    memset(n->vlans, 0xff, MAX_VLAN >> 3);
+    memset(n->vlans, 0xff, sizeof(n->vlans));
 
     nc = qemu_get_queue(n->nic);
     nc->rxfilter_notify_enabled = 1;
@@ -4068,7 +4068,6 @@ static void virtio_net_device_unrealize(DeviceState *dev)
     n->netclient_type = NULL;
 
     g_free(n->mac_table.macs);
-    g_free(n->vlans);
 
     if (n->failover) {
         qobject_unref(n->primary_opts);
-- 
2.51.0


Re: [PULL 03/23] hw/net/virtio-net: make VirtIONet.vlans an array instead of a pointer
Posted by Fiona Ebner 2 months ago
Hi,

Am 28.10.25 um 11:19 AM schrieb Philippe Mathieu-Daudé:
> From: Michael Tokarev <mjt@tls.msk.ru>
> 
> This field is a fixed-size buffer (number of elements is MAX_VLAN,
> known at build time).  There's no need to allocate it dynamically,
> it can be made an integral part of VirtIONet structure.
> 
> This field is the only user of VMSTATE_BUFFER_POINTER_UNSAFE() macro.
> 
> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Tested-by: Lei Yang <leiyang@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> Message-ID: <20251023135316.31128-2-mjt@tls.msk.ru>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

unfortunately, after this commit, loading a VM state taken with v10.1.2
or older doesn't work anymore:

> qemu-system-x86_64: Missing section footer for 0000:00:13.0/virtio-net
> qemu-system-x86_64: Section footer error, section_id: 41

Reproducer below [0].


Reverting

> 58341158d0 migration/vmstate: remove VMSTATE_BUFFER_POINTER_UNSAFE macro
> 3a9cd2a4a1 hw/net/virtio-net: make VirtIONet.vlans an array instead of a pointer

in current master makes it work again.


I'm also seeing the following when a guest is actually running:

> kvm: VQ 1 size 0x100 < last_avail_idx 0x9 - used_idx 0x3e30
> kvm: load of migration failed: Operation not permitted: error while loading state for instance 0x0 of device '0000:00:13.0/virtio-net': Failed to load element of type virtio for virtio: -1

and here too a revert of the two commits seems to help :)


Best Regards,
Fiona


[0]:

> [I] root@pve9a1 ~# cat snapshot-virtio-net.sh
> #!/bin/bash
> rm /tmp/disk.qcow2
> args="
>   -netdev type=tap,id=net1,ifname=tap104i1,script=/usr/libexec/qemu-server/pve-bridge,downscript=/usr/libexec/qemu-server/pve-bridgedown,vhost=on
>   -device virtio-net-pci,mac=BC:24:11:32:3C:69,netdev=net1,bus=pci.0,addr=0x13,id=net1
>   -machine type=pc-i440fx-10.1
> "
> $1/qemu-img create -f qcow2 /tmp/disk.qcow2 1G
> $1/qemu-system-x86_64 --qmp stdio --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 $args <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "snapshot-save", "arguments": { "job-id": "save0", "tag": "snap", "vmstate": "node0", "devices": ["node0"] } }
> {"execute": "quit"}
> EOF
> $2/qemu-system-x86_64 --qmp stdio --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 $args -loadvm snap



Re: [PULL 03/23] hw/net/virtio-net: make VirtIONet.vlans an array instead of a pointer
Posted by Philippe Mathieu-Daudé 2 months ago
Hi Fiona,

On 9/12/25 13:15, Fiona Ebner wrote:
> Hi,
> 
> Am 28.10.25 um 11:19 AM schrieb Philippe Mathieu-Daudé:
>> From: Michael Tokarev <mjt@tls.msk.ru>
>>
>> This field is a fixed-size buffer (number of elements is MAX_VLAN,
>> known at build time).  There's no need to allocate it dynamically,
>> it can be made an integral part of VirtIONet structure.
>>
>> This field is the only user of VMSTATE_BUFFER_POINTER_UNSAFE() macro.
>>
>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> Tested-by: Lei Yang <leiyang@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
>> Message-ID: <20251023135316.31128-2-mjt@tls.msk.ru>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> unfortunately, after this commit, loading a VM state taken with v10.1.2
> or older doesn't work anymore:
> 
>> qemu-system-x86_64: Missing section footer for 0000:00:13.0/virtio-net
>> qemu-system-x86_64: Section footer error, section_id: 41
> 
> Reproducer below [0].
> 
> 
> Reverting
> 
>> 58341158d0 migration/vmstate: remove VMSTATE_BUFFER_POINTER_UNSAFE macro
>> 3a9cd2a4a1 hw/net/virtio-net: make VirtIONet.vlans an array instead of a pointer
> 
> in current master makes it work again.

Thanks for the report. Commits reverted.

> 
> 
> I'm also seeing the following when a guest is actually running:
> 
>> kvm: VQ 1 size 0x100 < last_avail_idx 0x9 - used_idx 0x3e30
>> kvm: load of migration failed: Operation not permitted: error while loading state for instance 0x0 of device '0000:00:13.0/virtio-net': Failed to load element of type virtio for virtio: -1
> 
> and here too a revert of the two commits seems to help :)
> 
> 
> Best Regards,
> Fiona
> 
> 
> [0]:
> 
>> [I] root@pve9a1 ~# cat snapshot-virtio-net.sh
>> #!/bin/bash
>> rm /tmp/disk.qcow2
>> args="
>>    -netdev type=tap,id=net1,ifname=tap104i1,script=/usr/libexec/qemu-server/pve-bridge,downscript=/usr/libexec/qemu-server/pve-bridgedown,vhost=on
>>    -device virtio-net-pci,mac=BC:24:11:32:3C:69,netdev=net1,bus=pci.0,addr=0x13,id=net1
>>    -machine type=pc-i440fx-10.1
>> "
>> $1/qemu-img create -f qcow2 /tmp/disk.qcow2 1G
>> $1/qemu-system-x86_64 --qmp stdio --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 $args <<EOF
>> {"execute": "qmp_capabilities"}
>> {"execute": "snapshot-save", "arguments": { "job-id": "save0", "tag": "snap", "vmstate": "node0", "devices": ["node0"] } }
>> {"execute": "quit"}
>> EOF
>> $2/qemu-system-x86_64 --qmp stdio --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 $args -loadvm snap
> 
>