Netdev List
 help / color / mirror / Atom feed
* [PATCH net] vlan: fix skb_under_panic() and races when toggling HW VLAN offload
@ 2026-07-20  7:26 Eric Dumazet
  2026-07-23 16:22 ` Jakub Kicinski
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2026-07-20  7:26 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, netdev, eric.dumazet, Eric Dumazet, Tangxin Xie

Toggling hardware VLAN TX offload (NETIF_F_HW_VLAN_CTAG_TX or
NETIF_F_HW_VLAN_STAG_TX) on a lower device invokes vlan_transfer_features(),
which dynamically changes vlandev->hard_header_len.

This causes two issues:
1. Lockless TX paths (e.g. packet_snd in af_packet.c, ip6_finish_output2)
   read dev->hard_header_len without holding RTNL lock. Mutating
   hard_header_len dynamically under RTNL creates a data race where upper
   layers reserve insufficient headroom based on a stale hard_header_len,
   resulting in skb_under_panic when vlan_dev_hard_header() is called.
2. In addition, vlan_transfer_features() updated hard_header_len without
   updating header_ops, causing a mismatch between allocated headroom
   and header creation.

A VLAN interface may require software VLAN header insertion at any point
(e.g., if reorder_hdr is disabled or HW offload is unavailable). Always
setting hard_header_len = real_dev->hard_header_len + VLAN_HLEN and using
vlan_header_ops unconditionally ensures sufficient headroom is reserved
by all upper layers and avoids any data race on hard_header_len during ETHTOOL
feature changes.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Tangxin Xie <xietangxin@h-partners.com>
Closes: https://lore.kernel.org/netdev/99d678ae-c7b2-4b44-b534-b8320679deb3@h-partners.com/
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/8021q/vlan_dev.c | 34 +++-------------------------------
 1 file changed, 3 insertions(+), 31 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index ec2569b3f8dac629027b4344bc89402decf026d1..acf7f52d1eb2191ec0387dc29e2e9eb34124aa9d 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -502,26 +502,6 @@ static const struct header_ops vlan_header_ops = {
 	.parse_protocol = vlan_parse_protocol,
 };
 
-static int vlan_passthru_hard_header(struct sk_buff *skb, struct net_device *dev,
-				     unsigned short type,
-				     const void *daddr, const void *saddr,
-				     unsigned int len)
-{
-	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
-	struct net_device *real_dev = vlan->real_dev;
-
-	if (saddr == NULL)
-		saddr = dev->dev_addr;
-
-	return dev_hard_header(skb, real_dev, type, daddr, saddr, len);
-}
-
-static const struct header_ops vlan_passthru_header_ops = {
-	.create	 = vlan_passthru_hard_header,
-	.parse	 = eth_header_parse,
-	.parse_protocol = vlan_parse_protocol,
-};
-
 static const struct device_type vlan_type = {
 	.name	= "vlan",
 };
@@ -581,13 +561,8 @@ static int vlan_dev_init(struct net_device *dev)
 #endif
 
 	dev->needed_headroom = real_dev->needed_headroom;
-	if (vlan_hw_offload_capable(real_dev->features, vlan->vlan_proto)) {
-		dev->header_ops      = &vlan_passthru_header_ops;
-		dev->hard_header_len = real_dev->hard_header_len;
-	} else {
-		dev->header_ops      = &vlan_header_ops;
-		dev->hard_header_len = real_dev->hard_header_len + VLAN_HLEN;
-	}
+	dev->header_ops      = &vlan_header_ops;
+	dev->hard_header_len = real_dev->hard_header_len + VLAN_HLEN;
 
 	dev->netdev_ops = &vlan_netdev_ops;
 
@@ -1029,10 +1004,7 @@ static void vlan_transfer_features(struct net_device *dev,
 
 	netif_inherit_tso_max(vlandev, dev);
 
-	if (vlan_hw_offload_capable(dev->features, vlan->vlan_proto))
-		vlandev->hard_header_len = dev->hard_header_len;
-	else
-		vlandev->hard_header_len = dev->hard_header_len + VLAN_HLEN;
+	vlandev->hard_header_len = dev->hard_header_len + VLAN_HLEN;
 
 #if IS_ENABLED(CONFIG_FCOE)
 	vlandev->fcoe_ddp_xid = dev->fcoe_ddp_xid;
-- 
2.55.0.229.g6434b31f56-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] vlan: fix skb_under_panic() and races when toggling HW VLAN offload
  2026-07-20  7:26 [PATCH net] vlan: fix skb_under_panic() and races when toggling HW VLAN offload Eric Dumazet
@ 2026-07-23 16:22 ` Jakub Kicinski
  2026-07-23 18:20   ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2026-07-23 16:22 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Paolo Abeni, Simon Horman, netdev, eric.dumazet,
	Tangxin Xie

On Mon, 20 Jul 2026 07:26:22 +0000 Eric Dumazet wrote:
> Toggling hardware VLAN TX offload (NETIF_F_HW_VLAN_CTAG_TX or
> NETIF_F_HW_VLAN_STAG_TX) on a lower device invokes vlan_transfer_features(),
> which dynamically changes vlandev->hard_header_len.
> 
> This causes two issues:
> 1. Lockless TX paths (e.g. packet_snd in af_packet.c, ip6_finish_output2)
>    read dev->hard_header_len without holding RTNL lock. Mutating
>    hard_header_len dynamically under RTNL creates a data race where upper
>    layers reserve insufficient headroom based on a stale hard_header_len,
>    resulting in skb_under_panic when vlan_dev_hard_header() is called.
> 2. In addition, vlan_transfer_features() updated hard_header_len without
>    updating header_ops, causing a mismatch between allocated headroom
>    and header creation.
> 
> A VLAN interface may require software VLAN header insertion at any point
> (e.g., if reorder_hdr is disabled or HW offload is unavailable). Always
> setting hard_header_len = real_dev->hard_header_len + VLAN_HLEN and using
> vlan_header_ops unconditionally ensures sufficient headroom is reserved
> by all upper layers and avoids any data race on hard_header_len during ETHTOOL
> feature changes.

Hi Eric,

Looks like this broke VXLAN tests:

https://netdev-ctrl.bots.linux.dev/logview.html?f=/logs/vmksft/net/results/742304/2-test-vxlan-mdb-sh/stdout

AI bot says:

The NIPA CI detected a regression introduced by this patch in the VXLAN
MDB selftest (tools/testing/selftests/net/test_vxlan_mdb.sh).

All data path forwarding sub-tests fail on both normal and debug kernel
builds, while the control path (MDB entry add/replace/delete) tests
continue to pass. The failure is 100% reproducible across both runs.

The test topology uses VLAN subinterfaces (br0.10, br0.20) on a bridge
that has a VXLAN device as a member. mausezahn injects multicast frames
from the VLAN subinterface and tc ingress filters on the peer namespace
count packet arrivals to verify forwarding.

With this patch, `vlan_passthru_header_ops` has been removed and all
VLAN devices now unconditionally use `vlan_header_ops`, which calls
`vlan_dev_hard_header()` and inserts an 802.1Q VLAN header in software.
Previously, when the underlying device had `NETIF_F_HW_VLAN_CTAG_TX`
set (as veth does), the passthrough header ops were used and no software
VLAN tag was inserted.

This change causes frames injected from VLAN subinterfaces in the test
to arrive at the peer with an unexpected software-inserted VLAN tag,
breaking the tc filter matches used to verify delivery. Tests that check
frames are *dropped* still pass (the frames never arrive regardless),
but tests that check frames are *forwarded* all fail.

Failing test output excerpt:
  TEST: Destination IP - match                                        [FAIL]
  TEST: Destination IP - no match                                     [FAIL]
  TEST: Forward valid source - first VTEP                             [FAIL]
  TEST: Forward valid source - second VTEP                            [FAIL]
  ...

The pattern holds for all four overlay/underlay combinations (IPv4/IPv6
× IPv4/IPv6).

Could you please take a look? It seems the unconditional software VLAN
insertion breaks the VXLAN MDB data path when veth (or similarly capable)
devices are used as the underlay.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] vlan: fix skb_under_panic() and races when toggling HW VLAN offload
  2026-07-23 16:22 ` Jakub Kicinski
@ 2026-07-23 18:20   ` Eric Dumazet
  0 siblings, 0 replies; 3+ messages in thread
From: Eric Dumazet @ 2026-07-23 18:20 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S . Miller, Paolo Abeni, Simon Horman, netdev, eric.dumazet,
	Tangxin Xie

On Thu, Jul 23, 2026 at 6:22 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Hi Eric,
>
> Looks like this broke VXLAN tests:
>
> https://netdev-ctrl.bots.linux.dev/logview.html?f=/logs/vmksft/net/results/742304/2-test-vxlan-mdb-sh/stdout
>
> AI bot says:
>
> The NIPA CI detected a regression introduced by this patch in the VXLAN
> MDB selftest (tools/testing/selftests/net/test_vxlan_mdb.sh).
>
> All data path forwarding sub-tests fail on both normal and debug kernel
> builds, while the control path (MDB entry add/replace/delete) tests
> continue to pass. The failure is 100% reproducible across both runs.
>
> The test topology uses VLAN subinterfaces (br0.10, br0.20) on a bridge
> that has a VXLAN device as a member. mausezahn injects multicast frames
> from the VLAN subinterface and tc ingress filters on the peer namespace
> count packet arrivals to verify forwarding.
>
> With this patch, `vlan_passthru_header_ops` has been removed and all
> VLAN devices now unconditionally use `vlan_header_ops`, which calls
> `vlan_dev_hard_header()` and inserts an 802.1Q VLAN header in software.
> Previously, when the underlying device had `NETIF_F_HW_VLAN_CTAG_TX`
> set (as veth does), the passthrough header ops were used and no software
> VLAN tag was inserted.
>
> This change causes frames injected from VLAN subinterfaces in the test
> to arrive at the peer with an unexpected software-inserted VLAN tag,
> breaking the tc filter matches used to verify delivery. Tests that check
> frames are *dropped* still pass (the frames never arrive regardless),
> but tests that check frames are *forwarded* all fail.
>
> Failing test output excerpt:
>   TEST: Destination IP - match                                        [FAIL]
>   TEST: Destination IP - no match                                     [FAIL]
>   TEST: Forward valid source - first VTEP                             [FAIL]
>   TEST: Forward valid source - second VTEP                            [FAIL]
>   ...
>
> The pattern holds for all four overlay/underlay combinations (IPv4/IPv6
> × IPv4/IPv6).
>
> Could you please take a look? It seems the unconditional software VLAN
> insertion breaks the VXLAN MDB data path when veth (or similarly capable)
> devices are used as the underlay.

Ah right, we probably want to keep hard_header_len as is, and instead
inflate needed_headroom by 4 bytes.

ie in vlan_transfer_features() have somethng like:


        netif_inherit_tso_max(vlandev, dev);

-       if (vlan_hw_offload_capable(dev->features, vlan->vlan_proto))
-               vlandev->hard_header_len = dev->hard_header_len;
-       else
-               vlandev->hard_header_len = dev->hard_header_len + VLAN_HLEN;
+       vlandev->needed_headroom = dev->needed_headroom + VLAN_HLEN;
+       vlandev->hard_header_len = dev->hard_header_len;

Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-23 18:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20  7:26 [PATCH net] vlan: fix skb_under_panic() and races when toggling HW VLAN offload Eric Dumazet
2026-07-23 16:22 ` Jakub Kicinski
2026-07-23 18:20   ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox