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

[Eric Dumazet <edumazet@kernel.org>]

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: validate_xmit_vlan() inserts the
tag in software right before ndo_start_xmit() when the real device
lacks HW VLAN TX offload.

Remove it, and make vlan_dev_hard_start_xmit() always set the hwaccel
tag. The (veth->h_vlan_proto != vlan->vlan_proto) test it used to skip
tagging with reorder_hdr=off only made sense together with the
insertion in vlan_dev_hard_header(). Without it, stacked VLANs (Q-in-Q)
and AF_PACKET SOCK_DGRAM frames using ETH_P_8021Q would lose our tag.

The TX path now behaves as it always did with reorder_hdr=on (the
default). VLAN_FLAG_REORDER_HDR only controls whether vlan_do_receive()
reinserts the tag on RX.

Note that AF_PACKET SOCK_RAW users of a reorder_hdr=off VLAN building
frames that already carry our tag will now send them double tagged,
as they would with reorder_hdr=on.

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>
---
v2: Always set the hwaccel tag in vlan_dev_hard_start_xmit(): with the
    (veth->h_vlan_proto != vlan->vlan_proto) test kept, the outer tag was
    lost for Q-in-Q and SOCK_DGRAM ETH_P_8021Q with reorder_hdr=off
    (reported by sashiko).
v1: https://lore.kernel.org/netdev/20261007073027.459868-1-edumazet@kernel.org/

 net/8021q/vlan_dev.c | 60 ++++++++------------------------------------
 1 file changed, 11 insertions(+), 49 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index c949c6a829456c2f75d6c514b35a85ff64c493d8..bd98c191a66f96acc958f52e6fae2337b410ef3b 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)
@@ -106,22 +77,13 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 					    struct net_device *dev)
 {
 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
-	struct vlan_ethhdr *veth = (struct vlan_ethhdr *)(skb->data);
 	unsigned int len;
+	u16 vlan_tci;
 	int ret;
 
-	/* Handle non-VLAN frames if they are sent to us, for example by DHCP.
-	 *
-	 * NOTE: THIS ASSUMES DIX ETHERNET, SPECIFICALLY NOT SUPPORTING
-	 * OTHER THINGS LIKE FDDI/TokenRing/802.3 SNAPs...
-	 */
-	if (vlan->flags & VLAN_FLAG_REORDER_HDR ||
-	    veth->h_vlan_proto != vlan->vlan_proto) {
-		u16 vlan_tci;
-		vlan_tci = vlan->vlan_id;
-		vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
-		__vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
-	}
+	vlan_tci = vlan->vlan_id;
+	vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
+	__vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
 
 	skb->dev = vlan->real_dev;
 	len = skb->len;
-- 
2.53.0