[PATCH RFC] virtio-net: Fix VLAN filter table initialization timing

[PATCH RFC] virtio-net: Fix VLAN filter table initialization timing

[Akihiko Odaki]

The VLAN filter table was not being properly initialized when the driver
sets the DRIVER_OK status bit, causing incorrect VLAN filtering behavior
that could allow unintended VLAN packets to pass through. Correct the
table initialization timing to fix the issue.

Problem
-------

The expected initial state of the table depends on feature negotiation:

With VIRTIO_NET_F_CTRL_VLAN:
  The table must be empty in accordance with the specification.
Without VIRTIO_NET_F_CTRL_VLAN:
  The table should be filled to permit all VLAN traffic.

Prior to commit  ("virtio-net: do not reset vlan filtering at
set_features"), virtio_net_set_features() always initialized the VLAN
table. That commit changed the behavior to skip initialization when
VIRTIO_NET_F_CTRL_VLAN was negotiated, assuming the table would be
properly cleared during reset and remain stable.

However, this assumption breaks when a driver renegotiates features:
1. Initial negotiation without VIRTIO_NET_F_CTRL_VLAN (table filled)
2. Renegotiation with VIRTIO_NET_F_CTRL_VLAN (table will not be cleared)

The problem was exacerbated by commit 0caed25cd171 ("virtio: Call
set_features during reset"), which triggered virtio_net_set_features()
during reset, exposing the bug whenever VIRTIO_NET_F_CTRL_VLAN was
negotiated after a reset.

Solution
--------

Fix by moving VLAN table initialization to virtio_net_set_status(),
which sets the DRIVER_OK status bit. This ensures proper table
initialization regardless of feature negotiation sequence.

Fixes: 06b636a1e2ad ("virtio-net: do not reset vlan filtering at set_features")
Reported-by: Konstantin Shkolnyy <kshk@linux.ibm.com>
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
Not tested.

Konstantin, please see if this patch fixes your workload.
---
 hw/net/virtio-net.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 221252e00a50a46033d7ec8d18936e7c8196a6ca..6d2a67471e570e1e9b4b4fb5338d87c30e23ae36 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -389,6 +389,12 @@ static int virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
     int i;
     uint8_t queue_status;
 
+    if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+        !(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        bool vlan = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VLAN);
+        memset(n->vlans, vlan ? 0 : 0xff, MAX_VLAN >> 3);
+    }
+
     virtio_net_vnet_endian_status(n, status);
     virtio_net_vhost_status(n, status);
 
@@ -998,10 +1004,6 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
         vhost_net_save_acked_features(nc->peer);
     }
 
-    if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
-        memset(n->vlans, 0xff, MAX_VLAN >> 3);
-    }
-
     if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
         qapi_event_send_failover_negotiated(n->netclient_name);
         qatomic_set(&n->failover_primary_hidden, false);
@@ -3990,7 +3992,6 @@ static void virtio_net_reset(VirtIODevice *vdev)
     memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
     memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
     qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
-    memset(n->vlans, 0, MAX_VLAN >> 3);
 
     /* Flush any async TX */
     for (i = 0;  i < n->max_queue_pairs; i++) {

---
base-commit: f0737158b483e7ec2b2512145aeab888b85cc1f7
change-id: 20250713-vlan-8c107a65ad91

Best regards,
-- 
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Re: [PATCH RFC] virtio-net: Fix VLAN filter table initialization timing

[Michael S. Tsirkin]

On Sun, Jul 13, 2025 at 08:52:43PM +0900, Akihiko Odaki wrote:
> The VLAN filter table was not being properly initialized when the driver
> sets the DRIVER_OK status bit, causing incorrect VLAN filtering behavior
> that could allow unintended VLAN packets to pass through. Correct the
> table initialization timing to fix the issue.
> 
> Problem
> -------
> 
> The expected initial state of the table depends on feature negotiation:
> 
> With VIRTIO_NET_F_CTRL_VLAN:
>   The table must be empty in accordance with the specification.
> Without VIRTIO_NET_F_CTRL_VLAN:
>   The table should be filled to permit all VLAN traffic.
> 
> Prior to commit  ("virtio-net: do not reset vlan filtering at
> set_features"), virtio_net_set_features() always initialized the VLAN
> table. That commit changed the behavior to skip initialization when
> VIRTIO_NET_F_CTRL_VLAN was negotiated, assuming the table would be
> properly cleared during reset and remain stable.
> 
> However, this assumption breaks when a driver renegotiates features:
> 1. Initial negotiation without VIRTIO_NET_F_CTRL_VLAN (table filled)
> 2. Renegotiation with VIRTIO_NET_F_CTRL_VLAN (table will not be cleared)
> 
> The problem was exacerbated by commit 0caed25cd171 ("virtio: Call
> set_features during reset"), which triggered virtio_net_set_features()
> during reset, exposing the bug whenever VIRTIO_NET_F_CTRL_VLAN was
> negotiated after a reset.
> 
> Solution
> --------
> 
> Fix by moving VLAN table initialization to virtio_net_set_status(),
> which sets the DRIVER_OK status bit. This ensures proper table
> initialization regardless of feature negotiation sequence.
> 
> Fixes: 06b636a1e2ad ("virtio-net: do not reset vlan filtering at set_features")
> Reported-by: Konstantin Shkolnyy <kshk@linux.ibm.com>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

A bit worried about legacy drivers that might use device before
DRIVER_OK, is that a problem?


> ---
> Not tested.
> 
> Konstantin, please see if this patch fixes your workload.
> ---
>  hw/net/virtio-net.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 221252e00a50a46033d7ec8d18936e7c8196a6ca..6d2a67471e570e1e9b4b4fb5338d87c30e23ae36 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -389,6 +389,12 @@ static int virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>      int i;
>      uint8_t queue_status;
>  
> +    if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
> +        !(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> +        bool vlan = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VLAN);
> +        memset(n->vlans, vlan ? 0 : 0xff, MAX_VLAN >> 3);
> +    }
> +
>      virtio_net_vnet_endian_status(n, status);
>      virtio_net_vhost_status(n, status);
>  
> @@ -998,10 +1004,6 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
>          vhost_net_save_acked_features(nc->peer);
>      }
>  
> -    if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
> -        memset(n->vlans, 0xff, MAX_VLAN >> 3);
> -    }
> -
>      if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
>          qapi_event_send_failover_negotiated(n->netclient_name);
>          qatomic_set(&n->failover_primary_hidden, false);
> @@ -3990,7 +3992,6 @@ static void virtio_net_reset(VirtIODevice *vdev)
>      memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
>      memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
>      qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
> -    memset(n->vlans, 0, MAX_VLAN >> 3);
>  
>      /* Flush any async TX */
>      for (i = 0;  i < n->max_queue_pairs; i++) {
> 
> ---
> base-commit: f0737158b483e7ec2b2512145aeab888b85cc1f7
> change-id: 20250713-vlan-8c107a65ad91
> 
> Best regards,
> -- 
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Re: [PATCH RFC] virtio-net: Fix VLAN filter table initialization timing

[Konstantin Shkolnyy]

On 13-Jul-25 06:52, Akihiko Odaki wrote:
> Konstantin, please see if this patch fixes your workload.
Yes, it does. It delays vlans[] initialization until 
virtio_net_set_status(0xf) which later also calls vhost_net_start() 
which is where the NIC is actually programmed.

Re: [PATCH RFC] virtio-net: Fix VLAN filter table initialization timing

[Lei Yang]

Tested this patch with virtio-net regression tests, everything works fine.

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

On Mon, Jul 14, 2025 at 5:34 AM Konstantin Shkolnyy <kshk@linux.ibm.com> wrote:
>
> On 13-Jul-25 06:52, Akihiko Odaki wrote:
> > Konstantin, please see if this patch fixes your workload.
> Yes, it does. It delays vlans[] initialization until
> virtio_net_set_status(0xf) which later also calls vhost_net_start()
> which is where the NIC is actually programmed.
>
>