[PATCH v2] net: dsa: push hwaccel VLAN tag into payload in dsa_user_xmit()

[Amitesh Singh]

When a socket buffer has a hardware-accelerated VLAN tag (skb->vlan_tci
set), the conduit NIC inserts the 802.1Q header after whatever the DSA
tagger prepends, producing the wrong on-wire ordering:

  [tagger header][802.1Q VID]

instead of the correct:

  [802.1Q VID][tagger header]    (for taggers that follow the VLAN)
  [tagger header][802.1Q VID]    (for taggers that precede the VLAN — broken)

This affects any platform where tx-vlan-offload is enabled or fixed:on
and cannot be disabled (e.g. imx-dwmac). tag_rtl8_4 and tag_sja1105
both carried open-coded workarounds for this; rather than let each new
tagger rediscover the problem, handle it once in dsa_user_xmit() before
the skb is handed to any tagger.

__vlan_hwaccel_push_inside() frees the skb internally on allocation
failure, so returning NETDEV_TX_OK on NULL is correct. Remove the
now-redundant per-tagger copies from tag_rtl8_4.c and tag_sja1105.c.

Note: tag_lan9303.c is unaffected. Its xmit path never inspects
skb->vlan_tci — it always writes its own fixed [8100 | port_index]
header regardless. The skb_vlan_tag_present() call in lan9303_rcv is
on the RX path, which this change does not touch.

Signed-off-by: Amitesh Singh <singh.amitesh@xxxxxxxxx>
---
 net/dsa/tag_sja1105.c | 10 ----------
 net/dsa/user.c        | 13 +++++++++++++
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index bfe1f746f55b..57bb9360f9d2 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -244,16 +244,6 @@ static struct sk_buff *sja1105_pvid_tag_control_pkt(struct dsa_port *dp,
 	__be16 xmit_tpid = htons(sja1105_xmit_tpid(dp));
 	struct vlan_ethhdr *hdr;
 
-	/* If VLAN tag is in hwaccel area, move it to the payload
-	 * to deal with both cases uniformly and to ensure that
-	 * the VLANs are added in the right order.
-	 */
-	if (unlikely(skb_vlan_tag_present(skb))) {
-		skb = __vlan_hwaccel_push_inside(skb);
-		if (!skb)
-			return NULL;
-	}
-
 	hdr = skb_vlan_eth_hdr(skb);
 
 	/* If skb is already VLAN-tagged, leave that VLAN ID in place */
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 041f9060c8ef..7c178634228c 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -20,6 +20,7 @@
 #include <net/tc_act/tc_mirred.h>
 #include <linux/if_bridge.h>
 #include <linux/if_hsr.h>
+#include <linux/if_vlan.h>
 #include <net/dcbnl.h>
 #include <linux/netpoll.h>
 #include <linux/string.h>
@@ -935,6 +936,18 @@ static netdev_tx_t dsa_user_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (dev->needed_tailroom)
 		eth_skb_pad(skb);
 
+	/* If the conduit NIC has tx-vlan-offload enabled (or it is fixed:on and
+	 * cannot be turned off, e.g. imx-dwmac), it will insert the 802.1Q
+	 * header *after* whatever the tagger prepends, producing the wrong
+	 * on-wire ordering. Materialise any hwaccel VLAN tag into the payload
+	 * here, once, before handing the skb to the tagger.
+	 */
+	if (skb_vlan_tag_present(skb)) {
+		skb = __vlan_hwaccel_push_inside(skb);
+		if (!skb)
+			return NETDEV_TX_OK;
+	}
+
 	/* Transmit function may have to reallocate the original SKB,
 	 * in which case it must have freed it. Taggers will drop the
 	 * passed skb on error.
-- 
2.43.0