virtio_net: Incorrect VLAN filter configuration on boot

virtio_net: Incorrect VLAN filter configuration on boot

[Konstantin Shkolnyy]

Symptom:
On boot, if VIRTIO_NET_F_CTRL_VLAN is negotiated, QEMU adds all 4096 
VLANs to the VLAN filter table. But, according to the virtio spec, the 
table should be empty in this case. (Observed when using VDPA.)

Investigation:
------------
commit 06b636a1e2ad12ab130edcbb0ccf995118440706
Author: Hawkins Jiawei <yin31149@gmail.com>
Date:   Sun Jul 23 20:09:11 2023 +0800

     virtio-net: do not reset vlan filtering at set_features

     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.

@@ -1029,9 +1029,7 @@ static void virtio_net_set_features(VirtIODevice 
*vdev, uint64_t features)

-    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);
      }
------------
After the above commit, virtio_net_set_features() can only set all bits 
in vlans[], but never clear them.
It expects virtio_net_reset() to clear the bits. I guess, this worked at 
the time because "set features" and "reset" were called in a favorable 
order. However, then this changed:
------------
commit 0caed25cd171c611781589b5402161d27d57229c
Author: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Date:   Mon Apr 21 21:17:20 2025 +0900

     virtio: Call set_features during reset

     virtio-net expects set_features() will be called when the feature set
     used by the guest changes to update the number of virtqueues but it is
     not called during reset, which will clear all features, leaving the
     queues added for VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS. Not only these
     extra queues are visible to the guest, they will cause segmentation
     fault during migration.

     Call set_features() during reset to remove those queues for virtio-net
     as we call set_status(). It will also prevent similar bugs for
     virtio-net and other devices in the future.

@@ -2350,7 +2352,7 @@ void virtio_reset(void *opaque)
      vdev->broken = false;
-    vdev->guest_features = 0;
+    virtio_set_features_nocheck(vdev, 0);
      vdev->queue_sel = 0;
------------
Now virtio_reset() first clears all bits in vlans[], and then calls 
virtio_net_set_features(0) which sets all bits in vlans[]. And that 
causes all VLANs to be added to the filter table on boot.

I'm not sure how to fix this correctly. The QEMU boot is complex - the 
"clear vlans[], set vlans[]" sequence is repeated 5 times during the boot...

Re: virtio_net: Incorrect VLAN filter configuration on boot

[Michael S. Tsirkin]

On Fri, Jul 11, 2025 at 11:11:21AM -0500, Konstantin Shkolnyy wrote:
> Symptom:
> On boot, if VIRTIO_NET_F_CTRL_VLAN is negotiated, QEMU adds all 4096 VLANs
> to the VLAN filter table. But, according to the virtio spec, the table
> should be empty in this case. (Observed when using VDPA.)

I think you want to repost this to the QEMU ML:
qemu-devel@nongnu.org

> Investigation:
> ------------
> commit 06b636a1e2ad12ab130edcbb0ccf995118440706
> Author: Hawkins Jiawei <yin31149@gmail.com>
> Date:   Sun Jul 23 20:09:11 2023 +0800
> 
>     virtio-net: do not reset vlan filtering at set_features
> 
>     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.
> 
> @@ -1029,9 +1029,7 @@ static void virtio_net_set_features(VirtIODevice
> *vdev, uint64_t features)
> 
> -    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);
>      }
> ------------
> After the above commit, virtio_net_set_features() can only set all bits in
> vlans[], but never clear them.
> It expects virtio_net_reset() to clear the bits. I guess, this worked at the
> time because "set features" and "reset" were called in a favorable order.
> However, then this changed:
> ------------
> commit 0caed25cd171c611781589b5402161d27d57229c
> Author: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Date:   Mon Apr 21 21:17:20 2025 +0900
> 
>     virtio: Call set_features during reset
> 
>     virtio-net expects set_features() will be called when the feature set
>     used by the guest changes to update the number of virtqueues but it is
>     not called during reset, which will clear all features, leaving the
>     queues added for VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS. Not only these
>     extra queues are visible to the guest, they will cause segmentation
>     fault during migration.
> 
>     Call set_features() during reset to remove those queues for virtio-net
>     as we call set_status(). It will also prevent similar bugs for
>     virtio-net and other devices in the future.
> 
> @@ -2350,7 +2352,7 @@ void virtio_reset(void *opaque)
>      vdev->broken = false;
> -    vdev->guest_features = 0;
> +    virtio_set_features_nocheck(vdev, 0);
>      vdev->queue_sel = 0;
> ------------
> Now virtio_reset() first clears all bits in vlans[], and then calls
> virtio_net_set_features(0) which sets all bits in vlans[]. And that causes
> all VLANs to be added to the filter table on boot.
> 
> I'm not sure how to fix this correctly. The QEMU boot is complex - the
> "clear vlans[], set vlans[]" sequence is repeated 5 times during the boot...