Re: virtio_net: Incorrect VLAN filter configuration on boot

["Michael S. Tsirkin" <mst@redhat.com>]

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...