[PATCH net 0/5] net: stmmac: Fix double VLAN 802.1ad tag handling

[PATCH net 0/5] net: stmmac: Fix double VLAN 802.1ad tag handling

[Ovidiu Panait]

Currently, hardware VLAN stripping is broken for 802.1ad tags. vlan_rx_hw()
hardcodes ETH_P_8021Q when putting the hardware tag into the skb, rather
than using the actual protocol from the packet. Because of this, packets
that contain an 802.1ad outer tag are incorrectly passed up the stack as
having an 802.1Q tag.

This issue was observed on the Renesas RZ/V2H platform (which has a dwmac4
IP), when testing QinQ ping:

  # DUT
  ip link add link end0 name end0.100 type vlan proto 802.1ad id 100
  ip link add link end0.100 name end0.100.200 type vlan proto 802.1q id 200
  ip addr add 172.16.3.2/24 dev end0.100.200
  ip link set end0 up
  ip link set end0.100 up
  ip link set end0.100.200 up

  # Peer
  ip link add link eth0 name eth0.100 type vlan proto 802.1ad id 100
  ip link add link eth0.100 name eth0.100.200 type vlan proto 802.1q id 200
  ip addr add 172.16.3.1/24 dev eth0.100.200
  ip link set eth0 up
  ip link set eth0.100 up
  ip link set eth0.100.200 up
  ping 172.16.3.2
    -- FAIL --

This series addresses the issue for both dwmac4 and dwxgmac2:
- dwmac4: since dwmac4 does not expose the tag type in the RDES3
  descriptor, it cannot support hardware double VLAN stripping correctly.
  This series disables double VLAN stripping for it, so the 802.1ad tags
  are left in place and are handled by the software VLAN path.

- dwxgmac2: the RDES3 descriptor does report the type, so the code was
  updated to read it instead of hardcoding ETH_P_8021Q.

Note: patch 5/5 was compile tested only, as I do not have the hardware.
However, it uses the same codepath as dwmac4, so it has the same issue.

Ovidiu Panait (5):
  net: stmmac: Remove VLAN perfect matching dead code
  net: stmmac: Enable double VLAN processing only when supported
  net: stmmac: Move double VLAN handling to a dedicated op
  net: stmmac: Disable double VLAN stripping on dwmac4
  net: stmmac: Fix double VLAN stripping on dwxgmac2

 .../ethernet/stmicro/stmmac/dwxgmac2_descs.c  | 15 ++++
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  8 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 16 +---
 .../net/ethernet/stmicro/stmmac/stmmac_vlan.c | 87 +++++--------------
 4 files changed, 47 insertions(+), 79 deletions(-)

-- 
2.34.1

[PATCH net 1/5] net: stmmac: Remove VLAN perfect matching dead code

[Ovidiu Panait]

stmmac_vlan_update() falls back to "perfect matching" when the VLAN hash
filter is unavailable (!priv->dma_cap.vlhash). This fallback has been
unreachable in normal operation since its introduction in
commit c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if
HASH is not available") because the NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER
features are advertised only when priv->dma_cap.vlhash is true.

The fallback is also duplicating the code in vlan_add_hw_rx_fltr(), which
is always available since stmmac_get_num_vlan() returns at least 1.

Therefore, remove it.

Fixes: c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if HASH is not available")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +-----
 .../net/ethernet/stmicro/stmmac/stmmac_vlan.c | 41 +------------------
 3 files changed, 4 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 04dafec021b4..6f26dbf95ce1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -633,7 +633,7 @@ struct stmmac_est_ops {
 struct stmmac_vlan_ops {
 	/* VLAN */
 	void (*update_vlan_hash)(struct mac_device_info *hw, u32 hash,
-				 u16 perfect_match, bool is_double);
+				 bool is_double);
 	void (*enable_vlan)(struct mac_device_info *hw, u32 type);
 	void (*rx_hw_vlan)(struct mac_device_info *hw, struct dma_desc *rx_desc,
 			   struct sk_buff *skb);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 0de4bc949913..fc9f1342ea45 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6791,29 +6791,18 @@ static u32 stmmac_vid_crc32_le(__le16 vid_le)
 static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double)
 {
 	u32 crc, hash = 0;
-	u16 pmatch = 0;
-	int count = 0;
 	u16 vid = 0;
 
 	for_each_set_bit(vid, priv->active_vlans, VLAN_N_VID) {
 		__le16 vid_le = cpu_to_le16(vid);
 		crc = bitrev32(~stmmac_vid_crc32_le(vid_le)) >> 28;
 		hash |= (1 << crc);
-		count++;
-	}
-
-	if (!priv->dma_cap.vlhash) {
-		if (count > 2) /* VID = 0 always passes filter */
-			return -EOPNOTSUPP;
-
-		pmatch = vid;
-		hash = 0;
 	}
 
 	if (!netif_running(priv->dev))
 		return 0;
 
-	return stmmac_update_vlan_hash(priv, priv->hw, hash, pmatch, is_double);
+	return stmmac_update_vlan_hash(priv, priv->hw, hash, is_double);
 }
 
 /* FIXME: This may need RXC to be running, but it may be called with BH
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
index e24efe3bfedb..983a90cb9767 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
@@ -162,7 +162,7 @@ static void vlan_restore_hw_rx_fltr(struct net_device *dev,
 }
 
 static void vlan_update_hash(struct mac_device_info *hw, u32 hash,
-			     u16 perfect_match, bool is_double)
+			     bool is_double)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value;
@@ -184,20 +184,6 @@ static void vlan_update_hash(struct mac_device_info *hw, u32 hash,
 		}
 
 		writel(value, ioaddr + VLAN_TAG);
-	} else if (perfect_match) {
-		u32 value = VLAN_ETV;
-
-		if (is_double) {
-			value |= VLAN_EDVLP;
-			value |= VLAN_ESVL;
-			value |= VLAN_DOVLTC;
-		} else {
-			value &= ~VLAN_EDVLP;
-			value &= ~VLAN_ESVL;
-			value &= ~VLAN_DOVLTC;
-		}
-
-		writel(value | perfect_match, ioaddr + VLAN_TAG);
 	} else {
 		value &= ~(VLAN_VTHM | VLAN_ETV);
 		value &= ~(VLAN_EDVLP | VLAN_ESVL);
@@ -251,7 +237,7 @@ static void vlan_set_hw_mode(struct mac_device_info *hw)
 }
 
 static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
-				      u16 perfect_match, bool is_double)
+				      bool is_double)
 {
 	void __iomem *ioaddr = hw->pcsr;
 
@@ -279,29 +265,6 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
 
 		value &= ~VLAN_VID;
 		writel(value, ioaddr + VLAN_TAG);
-	} else if (perfect_match) {
-		u32 value = readl(ioaddr + XGMAC_PACKET_FILTER);
-
-		value |= XGMAC_FILTER_VTFE;
-
-		writel(value, ioaddr + XGMAC_PACKET_FILTER);
-
-		value = readl(ioaddr + VLAN_TAG);
-
-		value &= ~VLAN_VTHM;
-		value |= VLAN_ETV;
-		if (is_double) {
-			value |= VLAN_EDVLP;
-			value |= VLAN_ESVL;
-			value |= VLAN_DOVLTC;
-		} else {
-			value &= ~VLAN_EDVLP;
-			value &= ~VLAN_ESVL;
-			value &= ~VLAN_DOVLTC;
-		}
-
-		value &= ~VLAN_VID;
-		writel(value | perfect_match, ioaddr + VLAN_TAG);
 	} else {
 		u32 value = readl(ioaddr + XGMAC_PACKET_FILTER);
 
-- 
2.34.1

[PATCH net 2/5] net: stmmac: Enable double VLAN processing only when supported

[Ovidiu Panait]

stmmac_vlan_update() turns on double/S-VLAN processing whenever an
802.1ad VLAN is registered, without checking whether the MAC actually
supports double VLAN processing. That capability is reported in
dma_cap.dvlan.

This was found while investigating a separate bug in the double VLAN RX
stripping path. The outer 802.1ad tags were unexpectedly stripped by the
MAC (because the ESVL bit was set).

Check dma_cap.dvlan before enabling EDVLP/ESVL/DOVLTC bits, so that double
VLAN processing is enabled only on supported hardware.

Fixes: 3cd1cfcba26e ("net: stmmac: Implement VLAN Hash Filtering in XGMAC")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fc9f1342ea45..ecb8eb7e29ab 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6802,6 +6802,9 @@ static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double)
 	if (!netif_running(priv->dev))
 		return 0;
 
+	if (!priv->dma_cap.dvlan)
+		is_double = false;
+
 	return stmmac_update_vlan_hash(priv, priv->hw, hash, is_double);
 }
 
-- 
2.34.1

[PATCH net 3/5] net: stmmac: Move double VLAN handling to a dedicated op

[Ovidiu Panait]

The double VLAN bits (EDVLP, ESVL, DOVLTC) are currently handled inside
update_vlan_hash(). This ties the double VLAN state to the hash filter
update, even though the two features are independent: hash filtering is
controlled by dma_cap.vlhash and double vlan by dma_cap.dvlan.

In preparation for removing double vlan support from dwmac4, move the
double vlan logic into a separate update_dvlan_state() VLAN operation.

Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  6 ++-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  6 +--
 .../net/ethernet/stmicro/stmmac/stmmac_vlan.c | 45 ++++++++-----------
 3 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 6f26dbf95ce1..66837caafa84 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -632,8 +632,8 @@ struct stmmac_est_ops {
 
 struct stmmac_vlan_ops {
 	/* VLAN */
-	void (*update_vlan_hash)(struct mac_device_info *hw, u32 hash,
-				 bool is_double);
+	void (*update_vlan_hash)(struct mac_device_info *hw, u32 hash);
+	void (*update_dvlan_state)(struct mac_device_info *hw, bool enable);
 	void (*enable_vlan)(struct mac_device_info *hw, u32 type);
 	void (*rx_hw_vlan)(struct mac_device_info *hw, struct dma_desc *rx_desc,
 			   struct sk_buff *skb);
@@ -650,6 +650,8 @@ struct stmmac_vlan_ops {
 
 #define stmmac_update_vlan_hash(__priv, __args...) \
 	stmmac_do_void_callback(__priv, vlan, update_vlan_hash, __args)
+#define stmmac_update_dvlan_state(__priv, __args...) \
+	stmmac_do_void_callback(__priv, vlan, update_dvlan_state, __args)
 #define stmmac_enable_vlan(__priv, __args...) \
 	stmmac_do_void_callback(__priv, vlan, enable_vlan, __args)
 #define stmmac_rx_hw_vlan(__priv, __args...) \
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ecb8eb7e29ab..ee75cbf6e935 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6802,10 +6802,10 @@ static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double)
 	if (!netif_running(priv->dev))
 		return 0;
 
-	if (!priv->dma_cap.dvlan)
-		is_double = false;
+	if (priv->dma_cap.dvlan)
+		stmmac_update_dvlan_state(priv, priv->hw, is_double);
 
-	return stmmac_update_vlan_hash(priv, priv->hw, hash, is_double);
+	return stmmac_update_vlan_hash(priv, priv->hw, hash);
 }
 
 /* FIXME: This may need RXC to be running, but it may be called with BH
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
index 983a90cb9767..1e47ae62093e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
@@ -161,8 +161,20 @@ static void vlan_restore_hw_rx_fltr(struct net_device *dev,
 		vlan_write_filter(dev, hw, i, hw->vlan_filter[i]);
 }
 
-static void vlan_update_hash(struct mac_device_info *hw, u32 hash,
-			     bool is_double)
+static void vlan_update_dvlan_state(struct mac_device_info *hw, bool enable)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value;
+
+	value = readl(ioaddr + VLAN_TAG);
+	if (enable)
+		value |= VLAN_EDVLP | VLAN_ESVL | VLAN_DOVLTC;
+	else
+		value &= ~(VLAN_EDVLP | VLAN_ESVL | VLAN_DOVLTC);
+	writel(value, ioaddr + VLAN_TAG);
+}
+
+static void vlan_update_hash(struct mac_device_info *hw, u32 hash)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value;
@@ -173,21 +185,9 @@ static void vlan_update_hash(struct mac_device_info *hw, u32 hash,
 
 	if (hash) {
 		value |= VLAN_VTHM | VLAN_ETV;
-		if (is_double) {
-			value |= VLAN_EDVLP;
-			value |= VLAN_ESVL;
-			value |= VLAN_DOVLTC;
-		} else {
-			value &= ~VLAN_EDVLP;
-			value &= ~VLAN_ESVL;
-			value &= ~VLAN_DOVLTC;
-		}
-
 		writel(value, ioaddr + VLAN_TAG);
 	} else {
 		value &= ~(VLAN_VTHM | VLAN_ETV);
-		value &= ~(VLAN_EDVLP | VLAN_ESVL);
-		value &= ~VLAN_DOVLTC;
 		value &= ~VLAN_VID;
 
 		writel(value, ioaddr + VLAN_TAG);
@@ -236,8 +236,7 @@ static void vlan_set_hw_mode(struct mac_device_info *hw)
 	writel(value, ioaddr + VLAN_TAG);
 }
 
-static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
-				      bool is_double)
+static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash)
 {
 	void __iomem *ioaddr = hw->pcsr;
 
@@ -253,15 +252,6 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
 		value = readl(ioaddr + VLAN_TAG);
 
 		value |= VLAN_VTHM | VLAN_ETV;
-		if (is_double) {
-			value |= VLAN_EDVLP;
-			value |= VLAN_ESVL;
-			value |= VLAN_DOVLTC;
-		} else {
-			value &= ~VLAN_EDVLP;
-			value &= ~VLAN_ESVL;
-			value &= ~VLAN_DOVLTC;
-		}
 
 		value &= ~VLAN_VID;
 		writel(value, ioaddr + VLAN_TAG);
@@ -275,8 +265,6 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
 		value = readl(ioaddr + VLAN_TAG);
 
 		value &= ~(VLAN_VTHM | VLAN_ETV);
-		value &= ~(VLAN_EDVLP | VLAN_ESVL);
-		value &= ~VLAN_DOVLTC;
 		value &= ~VLAN_VID;
 
 		writel(value, ioaddr + VLAN_TAG);
@@ -285,6 +273,7 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
 
 const struct stmmac_vlan_ops dwmac_vlan_ops = {
 	.update_vlan_hash = vlan_update_hash,
+	.update_dvlan_state = vlan_update_dvlan_state,
 	.enable_vlan = vlan_enable,
 	.add_hw_vlan_rx_fltr = vlan_add_hw_rx_fltr,
 	.del_hw_vlan_rx_fltr = vlan_del_hw_rx_fltr,
@@ -295,11 +284,13 @@ const struct stmmac_vlan_ops dwmac_vlan_ops = {
 
 const struct stmmac_vlan_ops dwxlgmac2_vlan_ops = {
 	.update_vlan_hash = dwxgmac2_update_vlan_hash,
+	.update_dvlan_state = vlan_update_dvlan_state,
 	.enable_vlan = vlan_enable,
 };
 
 const struct stmmac_vlan_ops dwxgmac210_vlan_ops = {
 	.update_vlan_hash = dwxgmac2_update_vlan_hash,
+	.update_dvlan_state = vlan_update_dvlan_state,
 	.enable_vlan = vlan_enable,
 	.add_hw_vlan_rx_fltr = vlan_add_hw_rx_fltr,
 	.del_hw_vlan_rx_fltr = vlan_del_hw_rx_fltr,
-- 
2.34.1

[PATCH net 4/5] net: stmmac: Disable double VLAN stripping on dwmac4

[Ovidiu Panait]

Currently, hardware VLAN stripping is broken for 802.1ad tags. vlan_rx_hw()
hardcodes ETH_P_8021Q when putting the hardware tag into the skb, rather
than using the actual protocol from the packet. Because of this, packets
that contain a 802.1ad outer tag are incorrectly passed up the stack as
having an 802.1Q tag. This causes QinQ ping between two hosts to fail.

vlan_rx_hw() is shared by dwxgmac2 and dwmac4: on dwxgmac2 the tag type
is available in the RDES3 write-back descriptor (the ET_LT field), so the
outer tag type can be determined based on that info. However, dwmac4
doesn't seem to provide the tag type. The Length/Type field in RDES3 only
indicates whether the packet is single or double-tagged, not which tag
type was stripped.

Since dwmac4 cannot report the stripped tag type, it cannot support
hardware double VLAN stripping correctly. Disable it by dropping
update_dvlan_state from dwmac_vlan_ops. With this, 802.1ad tags are
left in place and handled by the software VLAN path.

Fixes: 750011e239a5 ("net: stmmac: Add support for HW-accelerated VLAN stripping")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
index 1e47ae62093e..9b5b3f11f699 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
@@ -273,7 +273,6 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash)
 
 const struct stmmac_vlan_ops dwmac_vlan_ops = {
 	.update_vlan_hash = vlan_update_hash,
-	.update_dvlan_state = vlan_update_dvlan_state,
 	.enable_vlan = vlan_enable,
 	.add_hw_vlan_rx_fltr = vlan_add_hw_rx_fltr,
 	.del_hw_vlan_rx_fltr = vlan_del_hw_rx_fltr,
-- 
2.34.1

[PATCH net 5/5] net: stmmac: Fix double VLAN stripping on dwxgmac2

[Ovidiu Panait]

Currently, vlan_rx_hw() hardcodes ETH_P_8021Q when passing stripped
VLAN tags up the stack. If the MAC strips an 802.1ad outer tag, it is
incorrectly reported as an 802.1Q tag, breaking QinQ.

Fix this by adding a new get_rx_vlan_type() descriptor op so the driver
can read the actual type of the stripped tag. Implement this for dwxgmac2
only, which exposes the outer tag type in the RDES3 descriptor.

dwmac4 does not expose the tag type in the descriptor, so it does not
implement the op and keeps the ETH_P_8021Q fallback, as before.

Fixes: 534df0c1724b ("net: stmmac: dwxgmac2: Add support for HW-accelerated VLAN stripping")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 .../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c  | 15 +++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/hwif.h        |  2 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c |  6 +++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
index 6719ac6e395b..4fbd2442d918 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
@@ -61,6 +61,20 @@ static void dwxgmac2_set_rx_owner(struct dma_desc *p, int disable_rx_ic)
 	p->des3 |= cpu_to_le32(flags);
 }
 
+static u16 dwxgmac2_wrback_get_rx_vlan_type(struct dma_desc *p)
+{
+	u32 et_lt = FIELD_GET(XGMAC_RDES3_ET_LT, le32_to_cpu(p->des3));
+
+	switch (et_lt) {
+	case XGMAC_ET_LT_VLAN_STAG:
+	case XGMAC_ET_LT_DVLAN_STAG_STAG:
+	case XGMAC_ET_LT_DVLAN_STAG_CTAG:
+		return ETH_P_8021AD;
+	default:
+		return ETH_P_8021Q;
+	}
+}
+
 static u16 dwxgmac2_wrback_get_rx_vlan_tci(struct dma_desc *p)
 {
 	return le32_to_cpu(p->des0) & XGMAC_RDES0_VLAN_TAG_MASK;
@@ -350,6 +364,7 @@ const struct stmmac_desc_ops dwxgmac210_desc_ops = {
 	.rx_status = dwxgmac2_get_rx_status,
 	.set_tx_owner = dwxgmac2_set_tx_owner,
 	.set_rx_owner = dwxgmac2_set_rx_owner,
+	.get_rx_vlan_type = dwxgmac2_wrback_get_rx_vlan_type,
 	.get_rx_vlan_tci = dwxgmac2_wrback_get_rx_vlan_tci,
 	.get_rx_vlan_valid = dwxgmac2_wrback_get_rx_vlan_valid,
 	.get_rx_frame_len = dwxgmac2_get_rx_frame_len,
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 66837caafa84..eb6aacc0dfbc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -56,6 +56,8 @@ struct stmmac_desc_ops {
 	/* Clear interrupt on tx frame completion. When this bit is
 	 * set an interrupt happens as soon as the frame is transmitted */
 	void (*set_tx_ic)(struct dma_desc *p);
+	/* Get the VLAN tag type of the descriptor */
+	u16 (*get_rx_vlan_type)(struct dma_desc *p);
 	/* Get the tag of the descriptor */
 	u16 (*get_rx_vlan_tci)(struct dma_desc *p);
 	/* Get the valid status of descriptor */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
index 9b5b3f11f699..4bb639c717f8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
@@ -212,8 +212,12 @@ static void vlan_rx_hw(struct mac_device_info *hw,
 {
 	if (hw->desc->get_rx_vlan_valid(rx_desc)) {
 		u16 vid = hw->desc->get_rx_vlan_tci(rx_desc);
+		u16 type = ETH_P_8021Q;
 
-		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
+		if (hw->desc->get_rx_vlan_type)
+			type = hw->desc->get_rx_vlan_type(rx_desc);
+
+		__vlan_hwaccel_put_tag(skb, htons(type), vid);
 	}
 }
 
-- 
2.34.1

Re: [PATCH net 1/5] net: stmmac: Remove VLAN perfect matching dead code

[Maxime Chevallier]

Hi Ovidiu,

On 7/29/26 11:51, Ovidiu Panait wrote:
> stmmac_vlan_update() falls back to "perfect matching" when the VLAN hash
> filter is unavailable (!priv->dma_cap.vlhash). This fallback has been
> unreachable in normal operation since its introduction in
> commit c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if
> HASH is not available") because the NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER
> features are advertised only when priv->dma_cap.vlhash is true.
> 
> The fallback is also duplicating the code in vlan_add_hw_rx_fltr(), which
> is always available since stmmac_get_num_vlan() returns at least 1.

It's not exactly dead code, there's an stmmac selftest for that, see :

https://elixir.bootlin.com/linux/v7.2-rc4/source/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c#L973

It does exactly what you say, it fakes the fact that we don't support vlhash,
then exercise the perfect matching path.

But is it correct to drop that feature altogether ? Or should we just
relax the conditions for NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER to be
advertised ?

Maxime

Re: [PATCH net 0/5] net: stmmac: Fix double VLAN 802.1ad tag handling

[Maxime Chevallier]

Hi Ovidiu,

On 7/29/26 11:51, Ovidiu Panait wrote:
> Currently, hardware VLAN stripping is broken for 802.1ad tags. vlan_rx_hw()
> hardcodes ETH_P_8021Q when putting the hardware tag into the skb, rather
> than using the actual protocol from the packet. Because of this, packets
> that contain an 802.1ad outer tag are incorrectly passed up the stack as
> having an 802.1Q tag.
> 
> This issue was observed on the Renesas RZ/V2H platform (which has a dwmac4
> IP), when testing QinQ ping:
> 
>   # DUT
>   ip link add link end0 name end0.100 type vlan proto 802.1ad id 100
>   ip link add link end0.100 name end0.100.200 type vlan proto 802.1q id 200
>   ip addr add 172.16.3.2/24 dev end0.100.200
>   ip link set end0 up
>   ip link set end0.100 up
>   ip link set end0.100.200 up
> 
>   # Peer
>   ip link add link eth0 name eth0.100 type vlan proto 802.1ad id 100
>   ip link add link eth0.100 name eth0.100.200 type vlan proto 802.1q id 200
>   ip addr add 172.16.3.1/24 dev eth0.100.200
>   ip link set eth0 up
>   ip link set eth0.100 up
>   ip link set eth0.100.200 up
>   ping 172.16.3.2
>     -- FAIL --
> 
> This series addresses the issue for both dwmac4 and dwxgmac2:
> - dwmac4: since dwmac4 does not expose the tag type in the RDES3
>   descriptor, it cannot support hardware double VLAN stripping correctly.
>   This series disables double VLAN stripping for it, so the 802.1ad tags
>   are left in place and are handled by the software VLAN path.
> 
> - dwxgmac2: the RDES3 descriptor does report the type, so the code was
>   updated to read it instead of hardcoding ETH_P_8021Q.
> 
> Note: patch 5/5 was compile tested only, as I do not have the hardware.
> However, it uses the same codepath as dwmac4, so it has the same issue.

I was able to reproduce the issue on imx8mp, and indeed this series fixes it.

I'll try to do a more thorough review soon, but this is a good finding, thanks
for fixing that :)

Maxime

Re: [PATCH net 0/5] net: stmmac: Fix double VLAN 802.1ad tag handling

[Maxime Chevallier]

+ Nazim Amirul

On 7/29/26 11:51, Ovidiu Panait wrote:
> Currently, hardware VLAN stripping is broken for 802.1ad tags. vlan_rx_hw()
> hardcodes ETH_P_8021Q when putting the hardware tag into the skb, rather
> than using the actual protocol from the packet. Because of this, packets
> that contain an 802.1ad outer tag are incorrectly passed up the stack as
> having an 802.1Q tag.
> 
> This issue was observed on the Renesas RZ/V2H platform (which has a dwmac4
> IP), when testing QinQ ping:
> 
>   # DUT
>   ip link add link end0 name end0.100 type vlan proto 802.1ad id 100
>   ip link add link end0.100 name end0.100.200 type vlan proto 802.1q id 200
>   ip addr add 172.16.3.2/24 dev end0.100.200
>   ip link set end0 up
>   ip link set end0.100 up
>   ip link set end0.100.200 up
> 
>   # Peer
>   ip link add link eth0 name eth0.100 type vlan proto 802.1ad id 100
>   ip link add link eth0.100 name eth0.100.200 type vlan proto 802.1q id 200
>   ip addr add 172.16.3.1/24 dev eth0.100.200
>   ip link set eth0 up
>   ip link set eth0.100 up
>   ip link set eth0.100.200 up
>   ping 172.16.3.2
>     -- FAIL --
> 
> This series addresses the issue for both dwmac4 and dwxgmac2:
> - dwmac4: since dwmac4 does not expose the tag type in the RDES3
>   descriptor, it cannot support hardware double VLAN stripping correctly.
>   This series disables double VLAN stripping for it, so the 802.1ad tags
>   are left in place and are handled by the software VLAN path.
> 
> - dwxgmac2: the RDES3 descriptor does report the type, so the code was
>   updated to read it instead of hardcoding ETH_P_8021Q.
> 
> Note: patch 5/5 was compile tested only, as I do not have the hardware.
> However, it uses the same codepath as dwmac4, so it has the same issue.
Nazim Amirul, as you seem to have access to some AgileX5 with XGMAC, can you
see if you can reproduce the issue on your side and if this series solves it ?

Thanks :)

Maxime

RE: [PATCH net 1/5] net: stmmac: Remove VLAN perfect matching dead code

[Ovidiu Panait]

Hi Maxime,

> 
> Hi Ovidiu,
> 
> On 7/29/26 11:51, Ovidiu Panait wrote:
> > stmmac_vlan_update() falls back to "perfect matching" when the VLAN hash
> > filter is unavailable (!priv->dma_cap.vlhash). This fallback has been
> > unreachable in normal operation since its introduction in
> > commit c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if
> > HASH is not available") because the NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER
> > features are advertised only when priv->dma_cap.vlhash is true.
> >
> > The fallback is also duplicating the code in vlan_add_hw_rx_fltr(),
> which
> > is always available since stmmac_get_num_vlan() returns at least 1.
> 
> It's not exactly dead code, there's an stmmac selftest for that, see :
> 
> https://elixir.bootlin.com/linux/v7.2-
> rc4/source/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c#L973
> 
> It does exactly what you say, it fakes the fact that we don't support
> vlhash,
> then exercise the perfect matching path.
> 
> But is it correct to drop that feature altogether ? Or should we just
> relax the conditions for NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER to be
> advertised ?
> 

We are not dropping the perfect matching feature by removing this code.
Basically, there are two perfect matching code paths that duplicate the
same logic and which diverged over time:

- vlan_update_hash() has the "if (perfect_match)" path that has never run on real
  hardware (unless running the selftest) because NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER
  is only advertised if the core supports VLAN hash filtering.

- vlan_add_hw_rx_fltr() also has a perfect matching codepath, which calls into
  vlan_write_single(). This runs unconditionally and overwrites what
  vlan_update_hash() did anyway, as it runs after stmmac_vlan_update():

stmmac_vlan_rx_add_vid():
...
	ret = stmmac_vlan_update(priv, num_double_vlans);
	if (ret) {
		clear_bit(vid, priv->active_vlans);
		goto err_pm_put;
	}

	if (priv->hw->num_vlan) {
		ret = stmmac_add_hw_vlan_rx_fltr(priv, ndev, priv->hw, proto, vid);
		if (ret) {
			clear_bit(vid, priv->active_vlans);
			stmmac_vlan_update(priv, priv->num_double_vlans);
			goto err_pm_put;
		}
	}

This path is also more correct because it takes into account cores that support
extended VLAN filters.

Therefore, I think removing the perfect matching code in vlan_update_hash() is
the right choice here.

Ovidiu

> Maxime

Re: [PATCH net 1/5] net: stmmac: Remove VLAN perfect matching dead code

[Maxime Chevallier]

Hi,

On 7/29/26 17:02, Ovidiu Panait wrote:
> Hi Maxime,
> 
>>
>> Hi Ovidiu,
>>
>> On 7/29/26 11:51, Ovidiu Panait wrote:
>>> stmmac_vlan_update() falls back to "perfect matching" when the VLAN hash
>>> filter is unavailable (!priv->dma_cap.vlhash). This fallback has been
>>> unreachable in normal operation since its introduction in
>>> commit c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if
>>> HASH is not available") because the NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER
>>> features are advertised only when priv->dma_cap.vlhash is true.
>>>
>>> The fallback is also duplicating the code in vlan_add_hw_rx_fltr(),
>> which
>>> is always available since stmmac_get_num_vlan() returns at least 1.
>>
>> It's not exactly dead code, there's an stmmac selftest for that, see :
>>
>> https://elixir.bootlin.com/linux/v7.2-
>> rc4/source/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c#L973
>>
>> It does exactly what you say, it fakes the fact that we don't support
>> vlhash,
>> then exercise the perfect matching path.
>>
>> But is it correct to drop that feature altogether ? Or should we just
>> relax the conditions for NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER to be
>> advertised ?
>>
> 
> We are not dropping the perfect matching feature by removing this code.
> Basically, there are two perfect matching code paths that duplicate the
> same logic and which diverged over time:
> 
> - vlan_update_hash() has the "if (perfect_match)" path that has never run on real
>   hardware (unless running the selftest) because NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER
>   is only advertised if the core supports VLAN hash filtering.
> 
> - vlan_add_hw_rx_fltr() also has a perfect matching codepath, which calls into
>   vlan_write_single(). This runs unconditionally and overwrites what
>   vlan_update_hash() did anyway, as it runs after stmmac_vlan_update():
> 
> stmmac_vlan_rx_add_vid():
> ...
> 	ret = stmmac_vlan_update(priv, num_double_vlans);
> 	if (ret) {
> 		clear_bit(vid, priv->active_vlans);
> 		goto err_pm_put;
> 	}
> 
> 	if (priv->hw->num_vlan) {
> 		ret = stmmac_add_hw_vlan_rx_fltr(priv, ndev, priv->hw, proto, vid);
> 		if (ret) {
> 			clear_bit(vid, priv->active_vlans);
> 			stmmac_vlan_update(priv, priv->num_double_vlans);
> 			goto err_pm_put;
> 		}
> 	}
> 
> This path is also more correct because it takes into account cores that support
> extended VLAN filters.
> 
> Therefore, I think removing the perfect matching code in vlan_update_hash() is
> the right choice here.

OK makes sense. Then the sensible followup (not necessarily for -net) would be to
update the selftests, as there's no point messing with the vlhash cap then, if I'm
getting this right.

Thanks :)

Maxime

Re: [PATCH net 0/5] net: stmmac: Fix double VLAN 802.1ad tag handling

[Andrew Lunn]

On Wed, Jul 29, 2026 at 09:51:04AM +0000, Ovidiu Panait wrote:
> Currently, hardware VLAN stripping is broken for 802.1ad tags. vlan_rx_hw()
> hardcodes ETH_P_8021Q when putting the hardware tag into the skb, rather
> than using the actual protocol from the packet. Because of this, packets
> that contain an 802.1ad outer tag are incorrectly passed up the stack as
> having an 802.1Q tag.
> 
> This issue was observed on the Renesas RZ/V2H platform (which has a dwmac4
> IP), when testing QinQ ping:
> 
>   # DUT
>   ip link add link end0 name end0.100 type vlan proto 802.1ad id 100
>   ip link add link end0.100 name end0.100.200 type vlan proto 802.1q id 200
>   ip addr add 172.16.3.2/24 dev end0.100.200
>   ip link set end0 up
>   ip link set end0.100 up
>   ip link set end0.100.200 up
> 
>   # Peer
>   ip link add link eth0 name eth0.100 type vlan proto 802.1ad id 100
>   ip link add link eth0.100 name eth0.100.200 type vlan proto 802.1q id 200
>   ip addr add 172.16.3.1/24 dev eth0.100.200
>   ip link set eth0 up
>   ip link set eth0.100 up
>   ip link set eth0.100.200 up
>   ping 172.16.3.2
>     -- FAIL --

I stretch goal, if you are interested and have time. Can you turn this
into a test somewhere in tools/testing/selftests/net ? If one MAC
driver has it wrong, there are probably other MAC drivers which are
also wrong.

	Andrew