[PATCH net] vlan: do not insert the vlan header in vlan_dev_hard_header()

[PATCH net] vlan: do not insert the vlan header in vlan_dev_hard_header()

[Eric Dumazet]

Commit 447cbe95ebb9 ("vlan: fix skb_under_panic and races when toggling
HW VLAN offload") replaced vlan_passthru_header_ops with vlan_header_ops
unconditionally to avoid lockless data races when modifying
hard_header_len dynamically under RTNL.

Prior to that commit, vlan devices on top of a real device capable of
HW VLAN TX offload used vlan_passthru_header_ops, which never inserted
the 802.1Q header in the frame. vlan_header_ops was only used when the
real device had no such offload.

However, vlan_dev_hard_header() retained its legacy path:

	if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
		...
		vhdr = skb_push(skb, VLAN_HLEN);
		...
	}

With vlan_header_ops used unconditionally, creating a VLAN with
reorder_hdr=off (as done by Home Assistant OS / NetworkManager when
flags are omitted over D-Bus) causes vlan_dev_hard_header() to push
an in-band 802.1Q header in software, even if the real device supports
HW VLAN TX offload.

This is a regression for r8169 users (Realtek RTL8168h), who report
transmit queue timeouts (NETDEV WATCHDOG): this NIC stalls when asked
to offload checksum and/or TSO for frames carrying an in-band tag.

Reverting the blamed commit, disabling TX checksum offload (which
also disables TSO), or setting reorder_hdr=on were all reported to
work around the issue.

This insertion is also inconsistent with dev->hard_header_len, which
does not account for VLAN_HLEN.

The in-band insertion is not needed: vlan_dev_hard_start_xmit() sets
the hwaccel tag on frames not already carrying the vlan protocol, and
validate_xmit_vlan() inserts it in software right before
ndo_start_xmit() when the real device lacks HW VLAN TX offload.

Remove it, so that vlan_dev_hard_header() behaves like the former
vlan_passthru_hard_header() for all real devices.

Frames sent on the wire are unchanged for real devices without
HW VLAN TX offload.

The (veth->h_vlan_proto != vlan->vlan_proto) test in
vlan_dev_hard_start_xmit() is left unchanged, so that frames already
carrying a vlan header (e.g. sent through AF_PACKET sockets) are
handled as before.

Fixes: 447cbe95ebb9 ("vlan: fix skb_under_panic and races when toggling HW VLAN offload")
Reported-by: sairon <sairon@users.noreply.github.com>
Reported-by: hitchin999 <hitchin999@users.noreply.github.com>
Reported-by: Stefan Agner <stefan@agner.ch>
Closes: https://github.com/home-assistant/operating-system/issues/5019
Closes: https://lore.kernel.org/netdev/CAPa5EdCj3v17tB-SF2JNecq5Q8s1Pranr-XzAFWNpNTBPgPG7w@mail.gmail.com/
Assisted-by: LLM
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
Cc: Heiner Kallweit <hkallweit1@gmail.com>
---
 net/8021q/vlan_dev.c | 43 +++++++------------------------------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index c949c6a829456c2f75d6c514b35a85ff64c493d8..c3db1ac23e6a8e0a9eb655ccb547db6cd8139c1d 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -35,11 +35,15 @@
 #include <linux/netpoll.h>
 
 /*
- *	Create the VLAN header for an arbitrary protocol layer
+ *	Create the hard header for an arbitrary protocol layer
  *
  *	saddr=NULL	means use device source address
  *	daddr=NULL	means leave destination address (eg unresolved arp)
  *
+ *  The VLAN tag is not inserted here: vlan_dev_hard_start_xmit() sets
+ *  the hwaccel tag, and the core inserts it in software if the real
+ *  device can not.
+ *
  *  This is called when the SKB is moving down the stack towards the
  *  physical devices.
  */
@@ -49,47 +53,14 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 				unsigned int len)
 {
 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
-	struct vlan_hdr *vhdr;
-	unsigned int vhdrlen = 0;
-	u16 vlan_tci = 0;
-	int rc;
-
-	if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
-		unsigned int hlen = READ_ONCE(dev->hard_header_len) +
-				    READ_ONCE(dev->needed_headroom);
-
-		if (skb_cow_head(skb, hlen) < 0)
-			return -ENOMEM;
-		vhdr = skb_push(skb, VLAN_HLEN);
-
-		vlan_tci = vlan->vlan_id;
-		vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
-		vhdr->h_vlan_TCI = htons(vlan_tci);
-
-		/*
-		 *  Set the protocol type. For a packet of type ETH_P_802_3/2 we
-		 *  put the length in here instead.
-		 */
-		if (type != ETH_P_802_3 && type != ETH_P_802_2)
-			vhdr->h_vlan_encapsulated_proto = htons(type);
-		else
-			vhdr->h_vlan_encapsulated_proto = htons(len);
-
-		skb->protocol = vlan->vlan_proto;
-		type = ntohs(vlan->vlan_proto);
-		vhdrlen = VLAN_HLEN;
-	}
+	struct net_device *real_dev = vlan->real_dev;
 
 	/* Before delegating work to the lower layer, enter our MAC-address */
 	if (saddr == NULL)
 		saddr = dev->dev_addr;
 
 	/* Now make the underlying real hard header */
-	dev = vlan->real_dev;
-	rc = dev_hard_header(skb, dev, type, daddr, saddr, len + vhdrlen);
-	if (rc > 0)
-		rc += vhdrlen;
-	return rc;
+	return dev_hard_header(skb, real_dev, type, daddr, saddr, len);
 }
 
 static inline netdev_tx_t vlan_netpoll_send_skb(struct vlan_dev_priv *vlan, struct sk_buff *skb)
-- 
2.53.0

Re: [PATCH net] vlan: do not insert the vlan header in vlan_dev_hard_header()

[Eric Dumazet]

Le mer. 7 oct. 2026 à 09:30, Eric Dumazet <edumazet@kernel.org> a écrit :
>
> Commit 447cbe95ebb9 ("vlan: fix skb_under_panic and races when toggling
> HW VLAN offload") replaced vlan_passthru_header_ops with vlan_header_ops
> unconditionally to avoid lockless data races when modifying
> hard_header_len dynamically under RTNL.
>
> Prior to that commit, vlan devices on top of a real device capable of
> HW VLAN TX offload used vlan_passthru_header_ops, which never inserted
> the 802.1Q header in the frame. vlan_header_ops was only used when the
> real device had no such offload.
>
> However, vlan_dev_hard_header() retained its legacy path:
>
>         if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
>                 ...
>                 vhdr = skb_push(skb, VLAN_HLEN);
>                 ...
>         }
>
> With vlan_header_ops used unconditionally, creating a VLAN with
> reorder_hdr=off (as done by Home Assistant OS / NetworkManager when
> flags are omitted over D-Bus) causes vlan_dev_hard_header() to push
> an in-band 802.1Q header in software, even if the real device supports
> HW VLAN TX offload.
>
> This is a regression for r8169 users (Realtek RTL8168h), who report
> transmit queue timeouts (NETDEV WATCHDOG): this NIC stalls when asked
> to offload checksum and/or TSO for frames carrying an in-band tag.

A V2 is coming, addressing Sashiko's feedback.

pw-bot: cr

Re: [PATCH net] vlan: do not insert the vlan header in vlan_dev_hard_header()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Stacked VLANs of the same protocol lose their outer tag when
  reorder_hdr=off.
- [Low] On non-offload real devices, the patch removes the transmit-side
  meaning of reorder_hdr=off.

--

vlan: do not insert the vlan header in vlan_dev_hard_header()

This removes the in-band 802.1Q push that vlan_dev_hard_header() did for
reorder_hdr=off. Tagging is left to vlan_dev_hard_start_xmit(), which sets
the hwaccel tag, and to validate_xmit_vlan(), which inserts the tag in
software when the device can't. The goal is to fix NETDEV WATCHDOG stalls
on r8169 that started after commit 447cbe95ebb9.

Closes: https://github.com/home-assistant/operating-system/issues/5019
Closes: https://lore.kernel.org/netdev/CAPa5EdCj3v17tB-SF2JNecq5Q8s1Pranr-XzAFWNpNTBPgPG7w@mail.gmail.com/

> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index c949c6a829456..c3db1ac23e6a8 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c

[ ... ]

> @@ -49,47 +53,14 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
>  				unsigned int len)
>  {
>  	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> -	struct vlan_hdr *vhdr;
> -	unsigned int vhdrlen = 0;
> -	u16 vlan_tci = 0;
> -	int rc;
> -
> -	if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
> -		unsigned int hlen = READ_ONCE(dev->hard_header_len) +
> -				    READ_ONCE(dev->needed_headroom);
> -
> -		if (skb_cow_head(skb, hlen) < 0)
> -			return -ENOMEM;
> -		vhdr = skb_push(skb, VLAN_HLEN);

[Severity: High]
With this push gone, can stacked VLANs of the same protocol drop their
outer tag when reorder_hdr=off? Take eth0.200.100 on top of eth0.200, both
802.1Q with reorder_hdr=off. The same applies to 802.1ad over 802.1ad.

Here is the transmit path after this patch:

eth0.200.100: vlan_dev_hard_header()
  dev_hard_header() -> eth_header() writes the payload ethertype (0x0800)

eth0.200.100: vlan_dev_hard_start_xmit()
  0x0800 != vlan_proto, so __vlan_hwaccel_put_tag(skb, 0x8100, 100)
  dev_queue_xmit() on eth0.200

eth0.200: __dev_queue_xmit()->validate_xmit_skb()->validate_xmit_vlan()
  __vlan_hwaccel_push_inside() puts tag 100 in-band

VLAN devices never advertise NETIF_F_HW_VLAN_CTAG_TX or STAG_TX.
vlan_dev_init() leaves them out of hw_features, and vlan_dev_fix_features()
only intersects. So the core always inserts the tag in software here.

Then in vlan_dev_hard_start_xmit() for eth0.200:

	if (vlan->flags & VLAN_FLAG_REORDER_HDR ||
	    veth->h_vlan_proto != vlan->vlan_proto) {

REORDER_HDR is clear and h_vlan_proto is now 0x8100, so eth0.200 treats
the upper device's tag as its own. It never adds tag 200, and eth0 sends a
frame tagged only with VLAN 100.

Before this patch, both hard_header calls pushed their tags, so the wire
carried 8100/200 8100/100.

The commit message says:

  "Frames sent on the wire are unchanged for real devices without
  HW VLAN TX offload."

Isn't the lower VLAN device itself a real device without HW VLAN TX
offload in this setup? The result also doesn't depend on what eth0
supports.

Could this also bring back the r8169 stall for stacked setups? Before,
eth0 received a double-tagged frame, and vlan_features_check() masks
csum/TSO for multi-tagged frames. Now eth0 gets a frame with a single
in-band tag, so csum/TSO stay enabled through vlan_features.

A smaller variant seems to exist with AF_PACKET SOCK_DGRAM and protocol ==
vlan_proto on a single reorder_hdr=off VLAN. Before, the device's own tag
was pushed in front of the frame. Now the same h_vlan_proto test skips it.

The new comment above vlan_dev_hard_header() says that
vlan_dev_hard_start_xmit() sets the hwaccel tag. That only happens when the
outer ethertype differs from vlan_proto. The h_vlan_proto test was the
counterpart of the in-band insertion removed here. Does the lower VLAN need
some way to still tag frames whose in-band header came from an upper VLAN's
hwaccel push?

> -
> -		vlan_tci = vlan->vlan_id;
> -		vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
> -		vhdr->h_vlan_TCI = htons(vlan_tci);
> -

[ ... ]

> -		skb->protocol = vlan->vlan_proto;

[Severity: Low]
This isn't about the wire format, but removing this also changes what
observers on the VLAN device itself see when reorder_hdr=off. ip-link(8)
says that with reorder_hdr off, the VLAN header is inserted immediately.

Before, the tagged frame with skb->protocol = vlan_proto was visible on the
VLAN device to egress tc/clsact, netfilter egress, an attached qdisc and
packet taps (for example tcpdump -i eth0.100).

Now the tag only exists as hwaccel metadata, set later inside
vlan_dev_hard_start_xmit(). Those observers see an untagged frame with
skb->protocol set to the payload protocol. A filter such as "protocol
802.1q" on the VLAN device would stop matching.

Should the commit message mention this user-visible change? It currently
only covers frames on the wire.

> -		type = ntohs(vlan->vlan_proto);
> -		vhdrlen = VLAN_HLEN;
> -	}
> +	struct net_device *real_dev = vlan->real_dev;
>  
>  	/* Before delegating work to the lower layer, enter our MAC-address */
>  	if (saddr == NULL)
>  		saddr = dev->dev_addr;
>  
>  	/* Now make the underlying real hard header */
> -	dev = vlan->real_dev;
> -	rc = dev_hard_header(skb, dev, type, daddr, saddr, len + vhdrlen);
> -	if (rc > 0)
> -		rc += vhdrlen;
> -	return rc;
> +	return dev_hard_header(skb, real_dev, type, daddr, saddr, len);
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007073027.459868-1-edumazet%40kernel.org