[PATCH 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: 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