All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konstantin Shkolnyy <kshk@linux.ibm.com>
To: yin31149@gmail.com, odaki@rsg.ci.i.u-tokyo.ac.jp
Cc: eperezma@redhat.com, mst@redhat.com, jasowang@redhat.com,
	virtualization@lists.linux.dev
Subject: virtio_net: Incorrect VLAN filter configuration on boot
Date: Fri, 11 Jul 2025 11:11:21 -0500	[thread overview]
Message-ID: <2be77146-dcbe-4370-b5e3-dfec9f4cf335@linux.ibm.com> (raw)

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

             reply	other threads:[~2025-07-11 16:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-11 16:11 Konstantin Shkolnyy [this message]
2025-07-13  8:36 ` virtio_net: Incorrect VLAN filter configuration on boot Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2be77146-dcbe-4370-b5e3-dfec9f4cf335@linux.ibm.com \
    --to=kshk@linux.ibm.com \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=odaki@rsg.ci.i.u-tokyo.ac.jp \
    --cc=virtualization@lists.linux.dev \
    --cc=yin31149@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.