linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling
@ 2026-09-08 16:43 Ovidiu Panait
  2026-09-08 16:43 ` [PATCH net v4 1/7] net: stmmac: Remove VLAN perfect matching dead code Ovidiu Panait
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Ovidiu Panait @ 2026-09-08 16:43 UTC (permalink / raw)
  To: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest, 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 --

Note that this series only fixes the issue on dwmac4. dwxgmac2 has the same
issue but I do not have access to hw to test on.

Since dwmac4 does not expose the tag type in the RDES3 descriptor, it
cannot support hardware S-Tag stripping correctly. This series
disables S-tag stripping for it, so the 802.1ad tags are left in
place and are handled by the software VLAN path.

v4:
- Reworked the S-VLAN patches based on Joseph's feedback: EDVLP is no
  longer toggled, the "double VLAN" naming was dropped from everywhere
  along with the is_double implementation, and ESVL/DOVLTC now
  follow the advertised NETIF_F_HW_VLAN_STAG_* features.
- Added a patch to stop advertising S-VLAN stripping when
  rx-vlan-offload is disabled.
- The VLAN selftest now also toggles rx-vlan-stag-hw-parse for the
  802.1ad variants.

v3: https://lore.kernel.org/all/20260825164522.4244-1-ovidiu.panait.rb@renesas.com/
- Dropped patch "net: stmmac: Enable double VLAN processing only when
  supported" from this series, as Joseph reported that double VLAN and
  S-Tag are separate features, so it would not be correct to gate
  everything behind dma_cap.dvlan.
- Moved the selftest to drivers/net/hw, as the _hw test variants couldn't
  be run with netdevsim.
- Fixed "ruff check" and "pylint --disable=R" for the VLAN selftest.
- Picked up "Reviewed-by" tags from Maxime.

v2: https://lore.kernel.org/all/20260821170959.79708-1-ovidiu.panait.rb@renesas.com/
- Added a net selftest for VLAN.
- Dropped patch "net: stmmac: Fix double VLAN stripping on dwxgmac2" from
  this series, as it was compile tested only and Nazim reported that it
  doesn't actually fix the issue on his board. More investigation is
  needed on dwxgmac2, but it should not block this series.
- Addressed some issues reported by Sashiko, to fix the advertised STAG
  offload features mismatch.

v1: https://lore.kernel.org/all/20260729095110.164157-1-ovidiu.panait.rb@renesas.com/

Ovidiu Panait (7):
  net: stmmac: Remove VLAN perfect matching dead code
  net: stmmac: Stop toggling the EDVLP bit
  net: stmmac: Rework S-VLAN handling
  net: stmmac: Do not advertise S-VLAN stripping when it is disabled
  net: stmmac: Disable S-Tag processing on dwmac4
  selftests: drv-net: Move _set_ethtool_feat() into lib
  selftests: drv-net: Add VLAN test

 drivers/net/ethernet/stmicro/stmmac/common.h  |   1 +
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |   3 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |   1 -
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  58 ++++-----
 .../net/ethernet/stmicro/stmmac/stmmac_vlan.c |  89 ++++----------
 tools/testing/selftests/drivers/net/gro.py    |  65 ++++------
 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 tools/testing/selftests/drivers/net/hw/config |   1 +
 .../drivers/net/hw/lib/py/__init__.py         |   3 +-
 .../testing/selftests/drivers/net/hw/vlan.py  | 112 ++++++++++++++++++
 .../selftests/drivers/net/lib/py/__init__.py  |   3 +-
 .../selftests/drivers/net/lib/py/feat.py      |  35 ++++++
 12 files changed, 222 insertions(+), 150 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/hw/vlan.py
 create mode 100644 tools/testing/selftests/drivers/net/lib/py/feat.py

-- 
2.34.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net v4 1/7] net: stmmac: Remove VLAN perfect matching dead code
  2026-09-08 16:43 [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling Ovidiu Panait
@ 2026-09-08 16:43 ` Ovidiu Panait
  2026-09-10 23:45   ` netdev-bot+sashiko
  2026-09-08 16:43 ` [PATCH net v4 2/7] net: stmmac: Stop toggling the EDVLP bit Ovidiu Panait
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Ovidiu Panait @ 2026-09-08 16:43 UTC (permalink / raw)
  To: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest, 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>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
v4 changes: None.

v3 changes:
- Added "Reviewed-by" tag from Maxime.

v2 changes: None.

 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 24656b35350b..24d64cce1d87 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6816,29 +6816,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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net v4 2/7] net: stmmac: Stop toggling the EDVLP bit
  2026-09-08 16:43 [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling Ovidiu Panait
  2026-09-08 16:43 ` [PATCH net v4 1/7] net: stmmac: Remove VLAN perfect matching dead code Ovidiu Panait
@ 2026-09-08 16:43 ` Ovidiu Panait
  2026-09-10 23:45   ` netdev-bot+sashiko
  2026-09-08 16:43 ` [PATCH net v4 3/7] net: stmmac: Rework S-VLAN handling Ovidiu Panait
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Ovidiu Panait @ 2026-09-08 16:43 UTC (permalink / raw)
  To: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest, Ovidiu Panait

Currently, the EDVLP bit is toggled whenever an 802.1ad VLAN is
registered. This bit enables the double VLAN feature, which provides
a way to insert, extract and filter an additional inner VLAN tag, and
has nothing to do with S-Tag handling.

Move EDVLP handling into vlan_set_hw_mode() instead, and keep it always
enabled, so that COE can work for packets with an inner VLAN header.
Add a dedicated callback for dwxlgmac2, as it doesn't implement the
set_hw_vlan_mode callback, like the other cores.

Suggested-by: Joseph Steel <recv.jo@gmail.com>
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
v4 changes:
- New patch.

 .../net/ethernet/stmicro/stmmac/stmmac_vlan.c | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
index 983a90cb9767..200b34588c7f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
@@ -174,19 +174,16 @@ 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_VTHM | VLAN_ETV | VLAN_ESVL);
 		value &= ~VLAN_DOVLTC;
 		value &= ~VLAN_VID;
 
@@ -222,6 +219,7 @@ static void vlan_set_hw_mode(struct mac_device_info *hw)
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value = readl(ioaddr + VLAN_TAG);
 
+	value |= VLAN_EDVLP;
 	value &= ~VLAN_TAG_CTRL_EVLS_MASK;
 
 	if (hw->hw_vlan_en)
@@ -254,11 +252,9 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 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;
 		}
@@ -274,8 +270,7 @@ 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_VTHM | VLAN_ETV | VLAN_ESVL);
 		value &= ~VLAN_DOVLTC;
 		value &= ~VLAN_VID;
 
@@ -283,6 +278,14 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
 	}
 }
 
+static void dwxlgmac2_set_hw_vlan_mode(struct mac_device_info *hw)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + VLAN_TAG);
+
+	writel(value | VLAN_EDVLP, ioaddr + VLAN_TAG);
+}
+
 const struct stmmac_vlan_ops dwmac_vlan_ops = {
 	.update_vlan_hash = vlan_update_hash,
 	.enable_vlan = vlan_enable,
@@ -296,6 +299,7 @@ const struct stmmac_vlan_ops dwmac_vlan_ops = {
 const struct stmmac_vlan_ops dwxlgmac2_vlan_ops = {
 	.update_vlan_hash = dwxgmac2_update_vlan_hash,
 	.enable_vlan = vlan_enable,
+	.set_hw_vlan_mode = dwxlgmac2_set_hw_vlan_mode,
 };
 
 const struct stmmac_vlan_ops dwxgmac210_vlan_ops = {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net v4 3/7] net: stmmac: Rework S-VLAN handling
  2026-09-08 16:43 [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling Ovidiu Panait
  2026-09-08 16:43 ` [PATCH net v4 1/7] net: stmmac: Remove VLAN perfect matching dead code Ovidiu Panait
  2026-09-08 16:43 ` [PATCH net v4 2/7] net: stmmac: Stop toggling the EDVLP bit Ovidiu Panait
@ 2026-09-08 16:43 ` Ovidiu Panait
  2026-09-10 23:46   ` netdev-bot+sashiko
  2026-09-08 16:43 ` [PATCH net v4 4/7] net: stmmac: Do not advertise S-VLAN stripping when it is disabled Ovidiu Panait
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Ovidiu Panait @ 2026-09-08 16:43 UTC (permalink / raw)
  To: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest, Ovidiu Panait

The ESVL and DOVLTC bits control S-VLAN tag processing and have
nothing to do with the double VLAN feature, which only provides a way
to process an additional inner VLAN tag. However, the driver code
that handles them always refers to "double VLAN", which is unrelated
and makes the implementation confusing. The driver does not use any
of the inner VLAN tag features, and the networking core does not
support offloads for the inner tag anyway.

To simplify the logic and to reduce the confusion regarding S-Tag vs
double VLAN handling, drop the is_double logic and add a hw_svlan_en
flag that is set when S-Tag hardware handling is enabled.

Suggested-by: Joseph Steel <recv.jo@gmail.com>
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
v4 changes:
- New patch.

 drivers/net/ethernet/stmicro/stmmac/common.h  |  1 +
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  3 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  1 -
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 31 ++++----------
 .../net/ethernet/stmicro/stmmac/stmmac_vlan.c | 42 ++++++++-----------
 5 files changed, 28 insertions(+), 50 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 927ea6230073..1dd4fc7e7a96 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -634,6 +634,7 @@ struct mac_device_info {
 	bool vlan_fail_q_en;
 	u8 vlan_fail_q;
 	bool hw_vlan_en;
+	bool hw_svlan_en;
 	bool reverse_sgmii_enable;
 
 	/* This spinlock protects read-modify-write of the interrupt
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 6f26dbf95ce1..bfd68f8460c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -632,8 +632,7 @@ 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 (*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.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 7582fca63741..7520bdcb7c6b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -344,7 +344,6 @@ struct stmmac_priv {
 	void __iomem *ptpaddr;
 	void __iomem *estaddr;
 	unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
-	unsigned int num_double_vlans;
 	int sfty_irq;
 	struct stmmac_msi *msi;
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 24d64cce1d87..cd9671493b41 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6813,7 +6813,7 @@ static u32 stmmac_vid_crc32_le(__le16 vid_le)
 	return crc;
 }
 
-static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double)
+static int stmmac_vlan_update(struct stmmac_priv *priv)
 {
 	u32 crc, hash = 0;
 	u16 vid = 0;
@@ -6827,7 +6827,7 @@ static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double)
 	if (!netif_running(priv->dev))
 		return 0;
 
-	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
@@ -6836,20 +6836,14 @@ static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double)
 static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid)
 {
 	struct stmmac_priv *priv = netdev_priv(ndev);
-	unsigned int num_double_vlans;
-	bool is_double = false;
 	int ret;
 
 	ret = pm_runtime_resume_and_get(priv->device);
 	if (ret < 0)
 		return ret;
 
-	if (be16_to_cpu(proto) == ETH_P_8021AD)
-		is_double = true;
-
 	set_bit(vid, priv->active_vlans);
-	num_double_vlans = priv->num_double_vlans + is_double;
-	ret = stmmac_vlan_update(priv, num_double_vlans);
+	ret = stmmac_vlan_update(priv);
 	if (ret) {
 		clear_bit(vid, priv->active_vlans);
 		goto err_pm_put;
@@ -6859,13 +6853,11 @@ static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid
 		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);
+			stmmac_vlan_update(priv);
 			goto err_pm_put;
 		}
 	}
 
-	priv->num_double_vlans = num_double_vlans;
-
 err_pm_put:
 	pm_runtime_put(priv->device);
 
@@ -6878,20 +6870,14 @@ static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid
 static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vid)
 {
 	struct stmmac_priv *priv = netdev_priv(ndev);
-	unsigned int num_double_vlans;
-	bool is_double = false;
 	int ret;
 
 	ret = pm_runtime_resume_and_get(priv->device);
 	if (ret < 0)
 		return ret;
 
-	if (be16_to_cpu(proto) == ETH_P_8021AD)
-		is_double = true;
-
 	clear_bit(vid, priv->active_vlans);
-	num_double_vlans = priv->num_double_vlans - is_double;
-	ret = stmmac_vlan_update(priv, num_double_vlans);
+	ret = stmmac_vlan_update(priv);
 	if (ret) {
 		set_bit(vid, priv->active_vlans);
 		goto del_vlan_error;
@@ -6901,13 +6887,11 @@ static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vi
 		ret = stmmac_del_hw_vlan_rx_fltr(priv, ndev, priv->hw, proto, vid);
 		if (ret) {
 			set_bit(vid, priv->active_vlans);
-			stmmac_vlan_update(priv, priv->num_double_vlans);
+			stmmac_vlan_update(priv);
 			goto del_vlan_error;
 		}
 	}
 
-	priv->num_double_vlans = num_double_vlans;
-
 del_vlan_error:
 	pm_runtime_put(priv->device);
 
@@ -6922,7 +6906,7 @@ static void stmmac_vlan_restore(struct stmmac_priv *priv)
 	if (priv->hw->num_vlan)
 		stmmac_restore_hw_vlan_rx_fltr(priv, priv->dev, priv->hw);
 
-	stmmac_vlan_update(priv, priv->num_double_vlans);
+	stmmac_vlan_update(priv);
 }
 
 static int stmmac_bpf(struct net_device *dev, struct netdev_bpf *bpf)
@@ -7962,6 +7946,7 @@ static int __stmmac_dvr_probe(struct device *device,
 #ifdef STMMAC_VLAN_TAG_USED
 	/* Both mac100 and gmac support receive VLAN tag detection */
 	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX;
+	priv->hw->hw_svlan_en = true;
 	if (dwmac_is_xmac(priv->plat->core_type)) {
 		ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
 		priv->hw->hw_vlan_en = true;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
index 200b34588c7f..fbb99b70ac27 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
@@ -161,8 +161,7 @@ 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_hash(struct mac_device_info *hw, u32 hash)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value;
@@ -173,18 +172,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_ESVL;
-			value |= VLAN_DOVLTC;
-		} else {
-			value &= ~VLAN_ESVL;
-			value &= ~VLAN_DOVLTC;
-		}
-
 		writel(value, ioaddr + VLAN_TAG);
 	} else {
-		value &= ~(VLAN_VTHM | VLAN_ETV | VLAN_ESVL);
-		value &= ~VLAN_DOVLTC;
+		value &= ~(VLAN_VTHM | VLAN_ETV);
 		value &= ~VLAN_VID;
 
 		writel(value, ioaddr + VLAN_TAG);
@@ -220,6 +210,12 @@ static void vlan_set_hw_mode(struct mac_device_info *hw)
 	u32 value = readl(ioaddr + VLAN_TAG);
 
 	value |= VLAN_EDVLP;
+
+	if (hw->hw_svlan_en)
+		value |= VLAN_ESVL | VLAN_DOVLTC;
+	else
+		value &= ~(VLAN_ESVL | VLAN_DOVLTC);
+
 	value &= ~VLAN_TAG_CTRL_EVLS_MASK;
 
 	if (hw->hw_vlan_en)
@@ -234,8 +230,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;
 
@@ -251,13 +246,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_ESVL;
-			value |= VLAN_DOVLTC;
-		} else {
-			value &= ~VLAN_ESVL;
-			value &= ~VLAN_DOVLTC;
-		}
 
 		value &= ~VLAN_VID;
 		writel(value, ioaddr + VLAN_TAG);
@@ -270,8 +258,7 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
 
 		value = readl(ioaddr + VLAN_TAG);
 
-		value &= ~(VLAN_VTHM | VLAN_ETV | VLAN_ESVL);
-		value &= ~VLAN_DOVLTC;
+		value &= ~(VLAN_VTHM | VLAN_ETV);
 		value &= ~VLAN_VID;
 
 		writel(value, ioaddr + VLAN_TAG);
@@ -283,7 +270,14 @@ static void dwxlgmac2_set_hw_vlan_mode(struct mac_device_info *hw)
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value = readl(ioaddr + VLAN_TAG);
 
-	writel(value | VLAN_EDVLP, ioaddr + VLAN_TAG);
+	value |= VLAN_EDVLP;
+
+	if (hw->hw_svlan_en)
+		value |= VLAN_ESVL | VLAN_DOVLTC;
+	else
+		value &= ~(VLAN_ESVL | VLAN_DOVLTC);
+
+	writel(value, ioaddr + VLAN_TAG);
 }
 
 const struct stmmac_vlan_ops dwmac_vlan_ops = {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net v4 4/7] net: stmmac: Do not advertise S-VLAN stripping when it is disabled
  2026-09-08 16:43 [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling Ovidiu Panait
                   ` (2 preceding siblings ...)
  2026-09-08 16:43 ` [PATCH net v4 3/7] net: stmmac: Rework S-VLAN handling Ovidiu Panait
@ 2026-09-08 16:43 ` Ovidiu Panait
  2026-09-10 23:46   ` netdev-bot+sashiko
  2026-09-08 16:43 ` [PATCH net v4 5/7] net: stmmac: Disable S-Tag processing on dwmac4 Ovidiu Panait
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Ovidiu Panait @ 2026-09-08 16:43 UTC (permalink / raw)
  To: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest, Ovidiu Panait

C-VLAN and S-VLAN tag stripping are both controlled by the EVLS bit,
so disabling rx-vlan-offload also disables S-VLAN tag stripping.
However, rx-vlan-stag-hw-parse keeps being advertised as enabled:

root@rzv2h-evk:~# ethtool -K end1 rx-vlan-offload off
root@rzv2h-evk:~# ethtool -k end1 | grep -i vlan
rx-vlan-offload: off
tx-vlan-offload: off [fixed]
rx-vlan-filter: on [fixed]
vlan-challenged: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: on [fixed]
rx-vlan-stag-filter: on [fixed]

Fix this inconsistency by making NETIF_F_HW_VLAN_STAG_RX follow
NETIF_F_HW_VLAN_CTAG_RX.

Fixes: 750011e239a5 ("net: stmmac: Add support for HW-accelerated VLAN stripping")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
v4 changes:
- New patch.

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index cd9671493b41..c81c5bb5b075 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6189,6 +6189,13 @@ static netdev_features_t stmmac_fix_features(struct net_device *dev,
 	if (priv->plat->bugged_jumbo && (dev->mtu > ETH_DATA_LEN))
 		features &= ~NETIF_F_CSUM_MASK;
 
+	if (priv->hw->hw_svlan_en) {
+		if (features & NETIF_F_HW_VLAN_CTAG_RX)
+			features |= NETIF_F_HW_VLAN_STAG_RX;
+		else
+			features &= ~NETIF_F_HW_VLAN_STAG_RX;
+	}
+
 	return features;
 }
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net v4 5/7] net: stmmac: Disable S-Tag processing on dwmac4
  2026-09-08 16:43 [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling Ovidiu Panait
                   ` (3 preceding siblings ...)
  2026-09-08 16:43 ` [PATCH net v4 4/7] net: stmmac: Do not advertise S-VLAN stripping when it is disabled Ovidiu Panait
@ 2026-09-08 16:43 ` Ovidiu Panait
  2026-09-09 12:19   ` Maxime Chevallier
  2026-09-10 23:46   ` netdev-bot+sashiko
  2026-09-08 16:43 ` [PATCH net v4 6/7] selftests: drv-net: Move _set_ethtool_feat() into lib Ovidiu Panait
  2026-09-08 16:43 ` [PATCH net v4 7/7] selftests: drv-net: Add VLAN test Ovidiu Panait
  6 siblings, 2 replies; 17+ messages in thread
From: Ovidiu Panait @ 2026-09-08 16:43 UTC (permalink / raw)
  To: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest, 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 S-Tag stripping correctly. Therefore, restrict the
NETIF_F_HW_VLAN_STAG_RX and NETIF_F_HW_VLAN_STAG_FILTER advertisement
to dwxgmac2 only.

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>
---
v4 changes:
- Dropped all "double VLAN" references from the commit title/message.
- Rebased on top of the S-VLAN rework.

v3 changes:
- Rebased after dropping the dma_cap.dvlan patch.

v2 changes:
- Advertised NETIF_F_HW_VLAN_STAG_RX and NETIF_F_HW_VLAN_STAG_FILTER only
  for XGMAC (reported by Sashiko).

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c81c5bb5b075..eab2903a66e4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7952,15 +7952,20 @@ static int __stmmac_dvr_probe(struct device *device,
 	ndev->watchdog_timeo = msecs_to_jiffies(watchdog);
 #ifdef STMMAC_VLAN_TAG_USED
 	/* Both mac100 and gmac support receive VLAN tag detection */
-	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX;
-	priv->hw->hw_svlan_en = true;
+	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
+	if (priv->plat->core_type == DWMAC_CORE_XGMAC) {
+		ndev->features |= NETIF_F_HW_VLAN_STAG_RX;
+		priv->hw->hw_svlan_en = true;
+	}
+
 	if (dwmac_is_xmac(priv->plat->core_type)) {
 		ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
 		priv->hw->hw_vlan_en = true;
 	}
 	if (priv->dma_cap.vlhash) {
 		ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
-		ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
+		if (priv->plat->core_type == DWMAC_CORE_XGMAC)
+			ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
 	}
 	if (priv->dma_cap.vlins)
 		ndev->features |= NETIF_F_HW_VLAN_CTAG_TX;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net v4 6/7] selftests: drv-net: Move _set_ethtool_feat() into lib
  2026-09-08 16:43 [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling Ovidiu Panait
                   ` (4 preceding siblings ...)
  2026-09-08 16:43 ` [PATCH net v4 5/7] net: stmmac: Disable S-Tag processing on dwmac4 Ovidiu Panait
@ 2026-09-08 16:43 ` Ovidiu Panait
  2026-09-10 23:42   ` Jakub Kicinski
  2026-09-08 16:43 ` [PATCH net v4 7/7] selftests: drv-net: Add VLAN test Ovidiu Panait
  6 siblings, 1 reply; 17+ messages in thread
From: Ovidiu Panait @ 2026-09-08 16:43 UTC (permalink / raw)
  To: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest, Ovidiu Panait

Move the _set_ethtool_feat() helper from gro.py into lib, so that it can
be reused by the VLAN test added in the next commit. Drop the leading
underscore, now that the helper is exported.

Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
v4 changes: None.

v3 changes:
- Fixed "ruff check" and "pylint --disable=R" warnings reported for
  tools/testing/selftests/drivers/net/lib/py/feat.py.

v2 changes:
- New patch.

 tools/testing/selftests/drivers/net/gro.py    | 65 ++++++-------------
 .../drivers/net/hw/lib/py/__init__.py         |  3 +-
 .../selftests/drivers/net/lib/py/__init__.py  |  3 +-
 .../selftests/drivers/net/lib/py/feat.py      | 35 ++++++++++
 4 files changed, 59 insertions(+), 47 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/lib/py/feat.py

diff --git a/tools/testing/selftests/drivers/net/gro.py b/tools/testing/selftests/drivers/net/gro.py
index 6ab8c97880d1..051a3b2d0ead 100755
--- a/tools/testing/selftests/drivers/net/gro.py
+++ b/tools/testing/selftests/drivers/net/gro.py
@@ -42,7 +42,7 @@ import re
 from lib.py import ksft_run, ksft_exit, ksft_pr
 from lib.py import NetDrvEpEnv, KsftFailEx, KsftXfailEx
 from lib.py import NetdevFamily, EthtoolFamily
-from lib.py import bkg, cmd, defer, ethtool, ip
+from lib.py import bkg, cmd, defer, ethtool, ip, set_ethtool_feat
 from lib.py import ksft_variants, KsftNamedVariant
 
 
@@ -96,31 +96,6 @@ def _set_mtu_restore(dev, mtu, host):
         defer(ip, f"link set dev {dev['ifname']} mtu {dev['mtu']}", host=host)
 
 
-def _set_ethtool_feat(dev, current, feats, host=None):
-    s2n = {True: "on", False: "off"}
-
-    new = ["-K", dev]
-    old = ["-K", dev]
-    no_change = True
-    for name, state in feats.items():
-        new += [name, s2n[state]]
-        old += [name, s2n[current[name]["active"]]]
-
-        if current[name]["active"] != state:
-            no_change = False
-            if current[name]["fixed"]:
-                raise KsftXfailEx(f"Device does not support {name}")
-    if no_change:
-        return
-
-    eth_cmd = ethtool(" ".join(new), host=host)
-    defer(ethtool, " ".join(old), host=host)
-
-    # If ethtool printed something kernel must have modified some features
-    if eth_cmd.stdout:
-        ksft_pr(eth_cmd)
-
-
 def _get_queue_stats(cfg, queue_id):
     """Get stats for a specific Rx queue."""
     cfg.wait_hw_stats_settle()
@@ -247,15 +222,15 @@ def _setup(cfg, mode, test_name):
         _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
         _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
 
-        _set_ethtool_feat(cfg.ifname, cfg.feat,
-                          {"generic-receive-offload": True,
-                           "rx-gro-hw": False,
-                           "large-receive-offload": False})
+        set_ethtool_feat(cfg.ifname, cfg.feat,
+                         {"generic-receive-offload": True,
+                          "rx-gro-hw": False,
+                          "large-receive-offload": False})
     elif mode == "hw":
-        _set_ethtool_feat(cfg.ifname, cfg.feat,
-                          {"generic-receive-offload": False,
-                           "rx-gro-hw": True,
-                           "large-receive-offload": False})
+        set_ethtool_feat(cfg.ifname, cfg.feat,
+                         {"generic-receive-offload": False,
+                          "rx-gro-hw": True,
+                          "large-receive-offload": False})
 
         # Some NICs treat HW GRO as a GRO sub-feature so disabling GRO
         # will also clear HW GRO. Use a hack of installing XDP generic
@@ -270,27 +245,27 @@ def _setup(cfg, mode, test_name):
             # Attaching XDP may change features, fetch the latest state
             feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
 
-            _set_ethtool_feat(cfg.ifname, feat,
-                              {"generic-receive-offload": True,
-                               "rx-gro-hw": True,
-                               "large-receive-offload": False})
+            set_ethtool_feat(cfg.ifname, feat,
+                             {"generic-receive-offload": True,
+                              "rx-gro-hw": True,
+                              "large-receive-offload": False})
     elif mode == "lro":
         # netdevsim advertises LRO for feature inheritance testing with
         # bonding/team tests but it doesn't actually perform the offload
         cfg.require_nsim(nsim_test=False)
 
-        _set_ethtool_feat(cfg.ifname, cfg.feat,
-                          {"generic-receive-offload": False,
-                           "rx-gro-hw": False,
-                           "large-receive-offload": True})
+        set_ethtool_feat(cfg.ifname, cfg.feat,
+                         {"generic-receive-offload": False,
+                          "rx-gro-hw": False,
+                          "large-receive-offload": True})
 
     try:
         # Disable TSO for local tests
         cfg.require_nsim()  # will raise KsftXfailEx if not running on nsim
 
-        _set_ethtool_feat(cfg.remote_ifname, cfg.remote_feat,
-                          {"tcp-segmentation-offload": False},
-                          host=cfg.remote)
+        set_ethtool_feat(cfg.remote_ifname, cfg.remote_feat,
+                         {"tcp-segmentation-offload": False},
+                         host=cfg.remote)
     except KsftXfailEx:
         pass
 
diff --git a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
index 8a58cb17cc06..eea45d01cf7b 100644
--- a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
@@ -31,6 +31,7 @@ try:
         ksft_setup, ksft_variants, KsftNamedVariant
     from net.lib.py import ksft_eq, ksft_ge, ksft_in, ksft_is, ksft_lt, \
         ksft_ne, ksft_not_in, ksft_raises, ksft_true, ksft_gt, ksft_not_none
+    from drivers.net.lib.py import set_ethtool_feat
     from drivers.net.lib.py import GenerateTraffic, Remote, Iperf3Runner
     from drivers.net.lib.py import NetDrvEnv, NetDrvEpEnv, NetDrvContEnv
 
@@ -49,7 +50,7 @@ try:
                "ksft_ne", "ksft_not_in", "ksft_raises", "ksft_true", "ksft_gt",
                "ksft_not_none", "ksft_not_none",
                "NetDrvEnv", "NetDrvEpEnv", "NetDrvContEnv", "GenerateTraffic",
-               "Remote", "Iperf3Runner"]
+               "Remote", "Iperf3Runner", "set_ethtool_feat"]
 except ModuleNotFoundError as e:
     print("Failed importing `net` library from kernel sources")
     print(str(e))
diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py
index ee903bcf3207..c261aac2b976 100644
--- a/tools/testing/selftests/drivers/net/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py
@@ -48,11 +48,12 @@ try:
                "ksft_not_none", "ksft_not_none"]
 
     from .env import NetDrvEnv, NetDrvEpEnv, NetDrvContEnv
+    from .feat import set_ethtool_feat
     from .load import GenerateTraffic, Iperf3Runner
     from .remote import Remote
 
     __all__ += ["NetDrvEnv", "NetDrvEpEnv", "NetDrvContEnv", "GenerateTraffic",
-                "Remote", "Iperf3Runner"]
+                "Remote", "Iperf3Runner", "set_ethtool_feat"]
 except ModuleNotFoundError as e:
     print("Failed importing `net` library from kernel sources")
     print(str(e))
diff --git a/tools/testing/selftests/drivers/net/lib/py/feat.py b/tools/testing/selftests/drivers/net/lib/py/feat.py
new file mode 100644
index 000000000000..014971c82b4b
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/lib/py/feat.py
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+Netdev feature helper utilities for kernel selftests.
+
+Provides common operations for changing device features via ethtool.
+"""
+
+from lib.py import KsftXfailEx, defer, ethtool, ksft_pr
+
+
+def set_ethtool_feat(dev, current, feats, host=None):
+    """Set ethtool features with defer to restore original state."""
+    s2n = {True: "on", False: "off"}
+
+    new = ["-K", dev]
+    old = ["-K", dev]
+    no_change = True
+    for name, state in feats.items():
+        new += [name, s2n[state]]
+        old += [name, s2n[current[name]["active"]]]
+
+        if current[name]["active"] != state:
+            no_change = False
+            if current[name]["fixed"]:
+                raise KsftXfailEx(f"Device does not support {name}")
+    if no_change:
+        return
+
+    eth_cmd = ethtool(" ".join(new), host=host)
+    defer(ethtool, " ".join(old), host=host)
+
+    # If ethtool printed something kernel must have modified some features
+    if eth_cmd.stdout:
+        ksft_pr(eth_cmd)
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net v4 7/7] selftests: drv-net: Add VLAN test
  2026-09-08 16:43 [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling Ovidiu Panait
                   ` (5 preceding siblings ...)
  2026-09-08 16:43 ` [PATCH net v4 6/7] selftests: drv-net: Move _set_ethtool_feat() into lib Ovidiu Panait
@ 2026-09-08 16:43 ` Ovidiu Panait
  2026-09-10 23:46   ` netdev-bot+sashiko
  6 siblings, 1 reply; 17+ messages in thread
From: Ovidiu Panait @ 2026-09-08 16:43 UTC (permalink / raw)
  To: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest, Ovidiu Panait

Add a test that validates ping traffic over VLAN interfaces. It aims
to catch drivers which mishandle hardware VLAN tag stripping, in
particular QinQ.

Three VLAN configurations are covered, each with hardware RX VLAN
stripping enabled and disabled (via the rx-vlan-offload and
rx-vlan-stag-hw-parse features):
- a single 802.1q VLAN interface
- a single 802.1ad VLAN interface
- an 802.1q VLAN stacked on top of an 802.1ad interface

The test is xfailed when a specific feature cannot be changed.

VLAN insertion offloads are not tested for now.

NETIF=end1 LOCAL_V4=172.16.0.2 REMOTE_V4=172.16.0.3 \
REMOTE_TYPE=ssh REMOTE_ARGS=root@172.16.0.3 \
run_kselftest.sh -t drivers/net/hw:vlan.py
 TAP version 13
 1..1
 # timeout set to 0
 # selftests: drivers/net/hw: vlan.py
 # # Interface: end1, driver: st_gmac
 # TAP version 13
 # 1..6
 # ok 1 vlan.test.8021q_hw
 # ok 2 vlan.test.8021q_sw
 # ok 3 vlan.test.8021ad_hw # XFAIL Device does not support rx-vlan-stag-hw-parse
 # ok 4 vlan.test.8021ad_sw
 # ok 5 vlan.test.qinq_hw # XFAIL Device does not support rx-vlan-stag-hw-parse
 # ok 6 vlan.test.qinq_sw
 # # Totals: pass:4 fail:0 xfail:2 xpass:0 skip:0 error:0
 ok 1 selftests: drivers/net/hw: vlan.py
 # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
v4 changes:
- Toggle rx-vlan-stag-hw-parse for the 802.1ad and QinQ variants.
- Updated the commit message and docstrings.

v3 changes:
- Moved the selftest to drivers/net/hw, as the _hw variants couldn't run
  under netdevsim.
- Fixed "ruff check" and "pylint --disable=R" warnings.
- Added CONFIG_VLAN_8021Q=m to configs file.

v2 changes:
- New patch.

 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 tools/testing/selftests/drivers/net/hw/config |   1 +
 .../testing/selftests/drivers/net/hw/vlan.py  | 112 ++++++++++++++++++
 3 files changed, 114 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/hw/vlan.py

diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index 78bb0169350b..886a4222c6cf 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -49,6 +49,7 @@ TEST_PROGS = \
 	tso.py \
 	userns_devmem.py \
 	uso.py \
+	vlan.py \
 	xdp_metadata.py \
 	xsk_reconfig.py \
 	#
diff --git a/tools/testing/selftests/drivers/net/hw/config b/tools/testing/selftests/drivers/net/hw/config
index d89a9ba17655..c6c2b64bb712 100644
--- a/tools/testing/selftests/drivers/net/hw/config
+++ b/tools/testing/selftests/drivers/net/hw/config
@@ -24,5 +24,6 @@ CONFIG_NET_SCH_INGRESS=y
 CONFIG_SYNC_FILE=y
 CONFIG_UDMABUF=y
 CONFIG_USER_NS=y
+CONFIG_VLAN_8021Q=m
 CONFIG_VXLAN=y
 CONFIG_XFRM_USER=y
diff --git a/tools/testing/selftests/drivers/net/hw/vlan.py b/tools/testing/selftests/drivers/net/hw/vlan.py
new file mode 100755
index 000000000000..61fa452b95bd
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/vlan.py
@@ -0,0 +1,112 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+VLAN tests.
+
+Validates that ping traffic is sent and received correctly over 802.1q
+and 802.1ad VLAN interfaces, with hardware RX VLAN stripping enabled and
+disabled on the local interface.
+
+Test cases:
+  - 8021q_hw:  Traffic over a single 802.1q VLAN, HW stripping on
+  - 8021q_sw:  Traffic over a single 802.1q VLAN, HW stripping off
+  - 8021ad_hw: Traffic over a single 802.1ad VLAN, HW stripping on
+  - 8021ad_sw: Traffic over a single 802.1ad VLAN, HW stripping off
+  - qinq_hw:   Traffic over an 802.1q VLAN stacked on an 802.1ad VLAN,
+               HW stripping on
+  - qinq_sw:   Traffic over an 802.1q VLAN stacked on an 802.1ad VLAN,
+               HW stripping off
+"""
+
+import os
+
+from lib.py import (
+    KsftNamedVariant,
+    NetDrvEpEnv,
+    cmd,
+    defer,
+    ethtool,
+    ip,
+    ksft_exit,
+    ksft_run,
+    ksft_variants,
+    set_ethtool_feat,
+)
+
+OUTER_DEV = f"vlout{os.getpid()}"
+INNER_DEV = f"vlin{os.getpid()}"
+
+OUTER_VID = 100
+INNER_VID = 200
+
+LOCAL_IP = "198.51.100.1"
+REMOTE_IP = "198.51.100.2"
+
+
+def _vlan_add(base, name, proto, vid, host=None):
+    """Create a VLAN device on top of base and bring it up."""
+
+    ip(f"link add link {base} name {name} type vlan proto {proto} id {vid}",
+       host=host)
+    defer(ip, f"link del {name}", host=host)
+    ip(f"link set {name} up", host=host)
+
+
+def _vlan_setup(base, addr, outer_proto, inner_proto, host=None):
+    """Create VLAN interfaces on base and set an IP on the innermost one."""
+
+    _vlan_add(base, OUTER_DEV, outer_proto, OUTER_VID, host=host)
+    if inner_proto:
+        _vlan_add(OUTER_DEV, INNER_DEV, inner_proto, INNER_VID, host=host)
+
+    dev = INNER_DEV if inner_proto else OUTER_DEV
+    ip(f"addr add {addr}/24 dev {dev}", host=host)
+
+
+def _setup(cfg, outer_proto, inner_proto, hw_strip):
+    """Configure VLAN stripping and create the VLAN interfaces."""
+
+    feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
+    feats = {"rx-vlan-offload": hw_strip}
+    if outer_proto == "802.1ad":
+        feats["rx-vlan-stag-hw-parse"] = hw_strip
+    set_ethtool_feat(cfg.ifname, feat, feats)
+
+    _vlan_setup(cfg.ifname, LOCAL_IP, outer_proto, inner_proto)
+    _vlan_setup(cfg.remote_ifname, REMOTE_IP, outer_proto, inner_proto,
+                host=cfg.remote)
+
+
+def _vlan_variants():
+    """Generator that yields the VLAN protocols and the stripping mode."""
+
+    yield KsftNamedVariant("8021q_hw", "802.1q", None, True)
+    yield KsftNamedVariant("8021q_sw", "802.1q", None, False)
+    yield KsftNamedVariant("8021ad_hw", "802.1ad", None, True)
+    yield KsftNamedVariant("8021ad_sw", "802.1ad", None, False)
+    yield KsftNamedVariant("qinq_hw", "802.1ad", "802.1q", True)
+    yield KsftNamedVariant("qinq_sw", "802.1ad", "802.1q", False)
+
+
+@ksft_variants(_vlan_variants())
+def test(cfg, outer_proto, inner_proto, hw_strip):
+    """Run a single VLAN test"""
+
+    cfg.require_ipver("4")
+
+    _setup(cfg, outer_proto, inner_proto, hw_strip)
+
+    cmd(f"ping -c 1 -W 5 {REMOTE_IP}")
+
+
+def main() -> None:
+    """ Ksft boiler plate main """
+
+    with NetDrvEpEnv(__file__) as cfg:
+        ksft_run(cases=[test], args=(cfg, ))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 5/7] net: stmmac: Disable S-Tag processing on dwmac4
  2026-09-08 16:43 ` [PATCH net v4 5/7] net: stmmac: Disable S-Tag processing on dwmac4 Ovidiu Panait
@ 2026-09-09 12:19   ` Maxime Chevallier
  2026-09-10 23:46   ` netdev-bot+sashiko
  1 sibling, 0 replies; 17+ messages in thread
From: Maxime Chevallier @ 2026-09-09 12:19 UTC (permalink / raw)
  To: Ovidiu Panait, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai
  Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel,
	linux-kselftest

Hi,

On 9/8/26 18:43, 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 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 S-Tag stripping correctly. Therefore, restrict the
> NETIF_F_HW_VLAN_STAG_RX and NETIF_F_HW_VLAN_STAG_FILTER advertisement
> to dwxgmac2 only.
> 
> 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>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 6/7] selftests: drv-net: Move _set_ethtool_feat() into lib
  2026-09-08 16:43 ` [PATCH net v4 6/7] selftests: drv-net: Move _set_ethtool_feat() into lib Ovidiu Panait
@ 2026-09-10 23:42   ` Jakub Kicinski
  0 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2026-09-10 23:42 UTC (permalink / raw)
  To: Ovidiu Panait
  Cc: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai, linux-kernel, netdev, linux-stm32,
	linux-arm-kernel, linux-kselftest

On Tue,  8 Sep 2026 16:43:08 +0000 Ovidiu Panait wrote:
> +from lib.py import KsftXfailEx, defer, ethtool, ksft_pr

tools/testing/selftests/drivers/net/lib/py/feat.py:9:0: E0401: Unable to import 'lib.py' (import-error)

do:

from . import KsftXfailEx, defer, ethtool, ksft_pr

?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 1/7] net: stmmac: Remove VLAN perfect matching dead code
  2026-09-08 16:43 ` [PATCH net v4 1/7] net: stmmac: Remove VLAN perfect matching dead code Ovidiu Panait
@ 2026-09-10 23:45   ` netdev-bot+sashiko
  0 siblings, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10 23:45 UTC (permalink / raw)
  To: ovidiu.panait.rb
  Cc: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai, linux-kernel, netdev, linux-stm32,
	linux-arm-kernel, linux-kselftest

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

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

- [Medium] Incomplete removal: the two in-tree self-tests whose only
  purpose was to exercise the deleted VLAN "perfect matching" fallback…
- [Low] The commit carries `Fixes: c7ab0b8088d7 ("net: stmmac: Fallback
  to VLAN Perfect filtering if HASH is not available")` while its own…

--

net: stmmac: Remove VLAN perfect matching dead code

This removes the "perfect matching" fallback that stmmac_vlan_update() used
when priv->dma_cap.vlhash was clear, along with the perfect_match argument
of the ->update_vlan_hash callback and the corresponding branches in
vlan_update_hash() and dwxgmac2_update_vlan_hash().

No Link: tags were present in the commit header.

>     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.
>
>     Fixes: c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if HASH is not available")

[Severity: Low]
Is the Fixes: tag intended here, given the message states the removed code
"has been unreachable in normal operation since its introduction"?

The unreachability claim does check out: NETIF_F_HW_VLAN_CTAG_FILTER and
NETIF_F_HW_VLAN_STAG_FILTER are only set when priv->dma_cap.vlhash is true
in stmmac_hw_init():

	if (priv->dma_cap.vlhash) {
		ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
		ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
	}

and the VLAN core only calls ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid for
devices advertising hardware filtering (vlan_hw_filter_capable() in
net/8021q/vlan_core.c).

So no user-visible misbehaviour appears to be corrected, while the patch does
change the ->update_vlan_hash signature in hwif.h, stmmac_main.c and
stmmac_vlan.c. Would it be better to drop the Fixes: tag and target net-next,
or alternatively describe a concretely reachable failure in the message?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 24656b35350b..24d64cce1d87 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6816,29 +6816,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;
>  	}

[Severity: Medium]
Should the two self-tests that exist only to exercise this fallback be
removed in the same patch?

After this hunk, stmmac_vlan_update() no longer reads priv->dma_cap.vlhash,
but stmmac_selftests.c still overrides the capability around the ordinary
test bodies:

stmmac_test_vlanfilt_perfect():
	int ret, prev_cap = priv->dma_cap.vlhash;

	if (!(priv->dev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
		return -EOPNOTSUPP;

	priv->dma_cap.vlhash = 0;
	ret = __stmmac_test_vlanfilt(priv);
	priv->dma_cap.vlhash = prev_cap;

stmmac_test_dvlanfilt_perfect() does the same around
__stmmac_test_dvlanfilt().

Since nothing under test inspects dma_cap.vlhash any more, do these two
wrappers now behave exactly like stmmac_test_vlanfilt() and
stmmac_test_dvlanfilt()?

That would make the "VLAN Filtering (perf)" and "Double VLAN Filter (perf)"
entries in stmmac_selftests[] report PASS from ethtool -t for a filtering
mode the driver no longer implements, and they could never fail for the
behaviour they name.

Verified that both wrappers are still unchanged at the end of the series
(bb70e68e6ebc), so this does not look like a later cleanup either.

> @@ -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);

With only the hash and no-hash branches left in vlan_update_hash() and in
dwxgmac2_update_vlan_hash(), a nonzero hash always enables hash matching, so
the "perf" self-tests end up programming the plain hash filter. Would
dropping those two test functions and their stmmac_selftests[] entries (or
re-pointing them at the vlan_add_hw_rx_fltr() path) be the right follow-up
inside this patch?

[ ... remaining hunks snipped ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908164309.59282-1-ovidiu.panait.rb%40renesas.com


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 2/7] net: stmmac: Stop toggling the EDVLP bit
  2026-09-08 16:43 ` [PATCH net v4 2/7] net: stmmac: Stop toggling the EDVLP bit Ovidiu Panait
@ 2026-09-10 23:45   ` netdev-bot+sashiko
  0 siblings, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10 23:45 UTC (permalink / raw)
  To: ovidiu.panait.rb
  Cc: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai, linux-kernel, netdev, linux-stm32,
	linux-arm-kernel, linux-kselftest

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

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

- [Medium] The in-kernel selftests "VLAN Filtering (perf)"
  (stmmac_test_vlanfilt_perfect,…

Pre-existing issues:
- [Medium] XLGMAC Rx VLAN offload state is never programmed into the MAC:
  dwxlgmac2_set_hw_vlan_mode()…

--

net: stmmac: Stop toggling the EDVLP bit

The EDVLP bit is no longer toggled per 802.1ad VLAN registration. It is
now set once from vlan_set_hw_mode() and kept enabled, and a new
set_hw_vlan_mode callback is added for dwxlgmac2, which previously had
none.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
> index 983a90cb9767..200b34588c7f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c

[ ... ]

> @@ -222,6 +219,7 @@ static void vlan_set_hw_mode(struct mac_device_info *hw)
>  	void __iomem *ioaddr = hw->pcsr;
>  	u32 value = readl(ioaddr + VLAN_TAG);
>  
> +	value |= VLAN_EDVLP;
>  	value &= ~VLAN_TAG_CTRL_EVLS_MASK;
>  
>  	if (hw->hw_vlan_en)

[ ... ]

> @@ -283,6 +278,14 @@ static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
>  	}
>  }
>  
> +static void dwxlgmac2_set_hw_vlan_mode(struct mac_device_info *hw)
> +{
> +	void __iomem *ioaddr = hw->pcsr;
> +	u32 value = readl(ioaddr + VLAN_TAG);
> +
> +	writel(value | VLAN_EDVLP, ioaddr + VLAN_TAG);
> +}
> +

[Severity: Medium]

This isn't a bug introduced by this patch, but now that dwxlgmac2 gains a
set_hw_vlan_mode callback, should it also honour hw->hw_vlan_en?

The other implementation of the same ops slot, vlan_set_hw_mode(), programs
the Rx strip mode and the descriptor report bit:

	value &= ~VLAN_TAG_CTRL_EVLS_MASK;

	if (hw->hw_vlan_en)
		value |= VLAN_TAG_STRIP_ALL;
	else
		value |= VLAN_TAG_STRIP_NONE;

	value |= VLAN_TAG_CTRL_EVLRXS;

dwxlgmac2_set_hw_vlan_mode() only ORs VLAN_EDVLP, so EVLS and EVLRXS keep
whatever value they had.

At the same time stmmac_dvr_probe() forces the flag on and advertises the
offload for every core matched by dwmac_is_xmac(), which includes the
XLGMAC hwif entry (.core_type = DWMAC_CORE_XGMAC, .vlan =
&dwxlgmac2_vlan_ops):

	if (dwmac_is_xmac(priv->plat->core_type)) {
		ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
		priv->hw->hw_vlan_en = true;
	}

So on XLGMAC, ethtool -K <if> rxvlan on|off goes stmmac_set_features() ->
stmmac_set_hw_vlan_mode() -> dwxlgmac2_set_hw_vlan_mode(), flips
priv->hw->hw_vlan_en and returns success, while no strip or report bit is
written to MAC_VLAN_Tag.

The Rx path then takes the MAC level branch in stmmac_rx():

	if (priv->hw->hw_vlan_en)
		/* MAC level stripping. */
		stmmac_rx_hw_vlan(priv, priv->hw, p, skb);
	else
		/* Driver level stripping. */
		stmmac_rx_vlan(priv->dev, skb);

and dwxlgmac2_vlan_ops has no .rx_hw_vlan handler, so that call resolves to
nothing.

To be clear, this is not packet or tag loss: with EVLS unprogrammed the tag
stays in-band and skb_vlan_untag() in the core Rx path still de-tags the
frame. The point is that driver bookkeeping and the actual MAC VLAN
configuration diverge, and the advertised NETIF_F_HW_VLAN_CTAG_RX does
nothing on this core. This is still the case at the end of the series
(bb70e68e6ebc): dwxlgmac2_vlan_ops has no .rx_hw_vlan and
dwxlgmac2_set_hw_vlan_mode() never programs EVLS/EVLRXS.

>  const struct stmmac_vlan_ops dwmac_vlan_ops = {
>  	.update_vlan_hash = vlan_update_hash,
>  	.enable_vlan = vlan_enable,
> @@ -296,6 +299,7 @@ const struct stmmac_vlan_ops dwmac_vlan_ops = {
>  const struct stmmac_vlan_ops dwxlgmac2_vlan_ops = {
>  	.update_vlan_hash = dwxgmac2_update_vlan_hash,
>  	.enable_vlan = vlan_enable,
> +	.set_hw_vlan_mode = dwxlgmac2_set_hw_vlan_mode,
>  };

A separate question, about the preceding patch in the series rather than
this one:

[Severity: Medium]

After "net: stmmac: Remove VLAN perfect matching dead code", do the two
"(perf)" selftests still exercise a different path?

stmmac_test_vlanfilt_perfect() and stmmac_test_dvlanfilt_perfect() in
stmmac_selftests.c select the perfect matching path only by clearing the
capability:

	priv->dma_cap.vlhash = 0;
	ret = __stmmac_test_vlanfilt(priv);
	priv->dma_cap.vlhash = prev_cap;

but stmmac_vlan_update() no longer consults priv->dma_cap.vlhash:

	if (!netif_running(priv->dev))
		return 0;

	return stmmac_update_vlan_hash(priv, priv->hw, hash);

and vlan_update_hash()/dwxgmac2_update_vlan_hash() no longer have the
perfect_match branch.

That makes the override inert, so hash filtering (VTHM/VTFE) stays enabled
during the "(perf)" runs and the entries named "VLAN Filtering (perf)" and
"Double VLAN Filter (perf)" in stmmac_selftests[] run the same path as their
non-perf counterparts.

A broken perfect match filter, that is vlan_add_hw_rx_fltr() and the
VLAN_TAG_DATA table, would be masked by the still active hash filter while
ethtool -t reports these tests as passing. Both tests and their names are
unchanged at the end of the series (bb70e68e6ebc).

Should these two tests be removed, or reworked to actually disable hash
filtering?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908164309.59282-1-ovidiu.panait.rb%40renesas.com


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 3/7] net: stmmac: Rework S-VLAN handling
  2026-09-08 16:43 ` [PATCH net v4 3/7] net: stmmac: Rework S-VLAN handling Ovidiu Panait
@ 2026-09-10 23:46   ` netdev-bot+sashiko
  0 siblings, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10 23:46 UTC (permalink / raw)
  To: ovidiu.panait.rb
  Cc: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai, linux-kernel, netdev, linux-stm32,
	linux-arm-kernel, linux-kselftest

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

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

- [High] Unconditional S-VLAN enablement: `__stmmac_dvr_probe()` sets
  `priv->hw->hw_svlan_en = true` for every core…
- [Medium] The in-kernel stmmac selftests named "VLAN Filtering (perf)"
  and "Double VLAN Filter (perf)" no longer exercise perfect (VID-…

--

net: stmmac: Rework S-VLAN handling

This patch removes the is_double / num_double_vlans tracking from the
stmmac VLAN paths and replaces it with a new mac_device_info flag,
hw_svlan_en, which decides whether the ESVL and DOVLTC bits are set. The
bit programming moves out of the vlan hash update callbacks and into
vlan_set_hw_mode() and dwxlgmac2_set_hw_vlan_mode().

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 24d64cce1d87..cd9671493b41 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6813,7 +6813,7 @@ static u32 stmmac_vid_crc32_le(__le16 vid_le)
>  	return crc;
>  }
>  
> -static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double)
> +static int stmmac_vlan_update(struct stmmac_priv *priv)
>  {
>  	u32 crc, hash = 0;
>  	u16 vid = 0;
> @@ -6827,7 +6827,7 @@ static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double)
>  	if (!netif_running(priv->dev))
>  		return 0;
>  
> -	return stmmac_update_vlan_hash(priv, priv->hw, hash, is_double);
> +	return stmmac_update_vlan_hash(priv, priv->hw, hash);
>  }

[Severity: Medium]

With the pmatch argument and the !priv->dma_cap.vlhash branch now gone from
stmmac_vlan_update() (removed by the earlier series commit "net: stmmac:
Remove VLAN perfect matching dead code"), do the selftests reported as
"VLAN Filtering (perf)" and "Double VLAN Filter (perf)" still exercise
perfect matching?

stmmac_test_vlanfilt_perfect() and stmmac_test_dvlanfilt_perfect() select
the perfect-match implementation by forcing the capability to zero:

drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:stmmac_test_vlanfilt_perfect() {
	priv->dma_cap.vlhash = 0;
	ret = __stmmac_test_vlanfilt(priv);
	priv->dma_cap.vlhash = prev_cap;
}

stmmac_vlan_update() no longer reads dma_cap.vlhash, so it always computes a
non-zero hash, and vlan_update_hash() / dwxgmac2_update_vlan_hash() then
program VLAN_HASH_TABLE and set VLAN_VTHM | VLAN_ETV regardless. Do both
"perf" tests now run the same hardware configuration as their hash
counterparts, so they can pass while the perfect MAC_VLAN_Tag_Filter
admission path is broken?

stmmac_selftests.c is unchanged at the end of the series (bb70e68e6ebc).
Should the tests be updated, or the reported names adjusted?

[ ... ]

> @@ -7962,6 +7946,7 @@ static int __stmmac_dvr_probe(struct device *device,
>  #ifdef STMMAC_VLAN_TAG_USED
>  	/* Both mac100 and gmac support receive VLAN tag detection */
>  	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX;
> +	priv->hw->hw_svlan_en = true;
>  	if (dwmac_is_xmac(priv->plat->core_type)) {
>  		ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
>  		priv->hw->hw_vlan_en = true;

[Severity: High]

hw_svlan_en is set here for every core, unlike hw_vlan_en just below which
is limited to xmac cores. Together with the move of the ESVL and DOVLTC
programming into vlan_set_hw_mode(), does this change when those two bits
are asserted?

Before this patch they were only set while an 802.1ad VID was registered
(is_double / num_double_vlans in vlan_update_hash() and
dwxgmac2_update_vlan_hash()) and were explicitly cleared otherwise. Now
they are programmed on every open and resume through
stmmac_hw_setup()->stmmac_set_hw_vlan_mode()->vlan_set_hw_mode(), and on
every feature change through stmmac_set_features().

With ESVL set the MAC treats 0x88A8 as a valid VLAN tag, and the same
register write selects VLAN_TAG_STRIP_ALL when hw_vlan_en is set, so
S-tags get stripped in hardware for all traffic. The receive path then
reinserts the TCI with a hardcoded protocol:

drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c:vlan_rx_hw() {
	if (hw->desc->get_rx_vlan_valid(rx_desc)) {
		u16 vid = hw->desc->get_rx_vlan_tci(rx_desc);

		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
	}
}

Does an 802.1ad frame therefore reach the stack labelled 802.1Q and get
demuxed onto the C-VLAN device with the same VID, while 802.1ad VLAN
interfaces stop receiving? The descriptor already carries the tag type -
dwxgmac2_wrback_get_rx_vlan_valid() decodes XGMAC_RDES3_ET_LT and
distinguishes S-tag from C-tag:

drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:dwxgmac2_wrback_get_rx_vlan_valid() {
	return et_lt >= XGMAC_ET_LT_VLAN_STAG &&
	       et_lt <= XGMAC_ET_LT_DVLAN_STAG_CTAG;
}

Separately, DOVLTC now permanently disables the hardware VLAN type
comparison. Does that mean a filter entry installed for a C-VLAN VID also
admits S-tagged frames with the same VID, and that S-tagged frames whose
VID is not registered are now dropped in hardware, where previously with
ESVL clear they were not treated as VLAN at all? Probe still advertises
NETIF_F_HW_VLAN_CTAG_FILTER and NETIF_F_HW_VLAN_STAG_FILTER as separate
offloads, and stmmac_vlan_rx_add_vid() / stmmac_vlan_rx_kill_vid() no
longer pass proto down to the hash configuration at all.

The later series commit "net: stmmac: Disable S-Tag processing on dwmac4"
describes this failure mode for GMAC4 and narrows hw_svlan_en to
core_type == DWMAC_CORE_XGMAC. At the end of the series (bb70e68e6ebc),
ESVL and DOVLTC still end up unconditionally set for dwxgmac2 and
dwxlgmac2, and vlan_rx_hw() still hardcodes ETH_P_8021Q, so does the same
mislabeling remain reachable on those cores?

The commit message presents this as dropping the is_double logic and adding
a hw_svlan_en flag. Could it also describe the receive stripping and
receive filtering behaviour changes?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
> index 200b34588c7f..fbb99b70ac27 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c

[ ... ]

> @@ -173,18 +172,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_ESVL;
> -			value |= VLAN_DOVLTC;
> -		} else {
> -			value &= ~VLAN_ESVL;
> -			value &= ~VLAN_DOVLTC;
> -		}
> -
>  		writel(value, ioaddr + VLAN_TAG);
>  	} else {
> -		value &= ~(VLAN_VTHM | VLAN_ETV | VLAN_ESVL);
> -		value &= ~VLAN_DOVLTC;
> +		value &= ~(VLAN_VTHM | VLAN_ETV);
>  		value &= ~VLAN_VID;
>  
>  		writel(value, ioaddr + VLAN_TAG);
> @@ -220,6 +210,12 @@ static void vlan_set_hw_mode(struct mac_device_info *hw)
>  	u32 value = readl(ioaddr + VLAN_TAG);
>  
>  	value |= VLAN_EDVLP;
> +
> +	if (hw->hw_svlan_en)
> +		value |= VLAN_ESVL | VLAN_DOVLTC;
> +	else
> +		value &= ~(VLAN_ESVL | VLAN_DOVLTC);
> +
>  	value &= ~VLAN_TAG_CTRL_EVLS_MASK;
>  
>  	if (hw->hw_vlan_en)

Since hw_svlan_en is true for every core and hw_vlan_en is true for every
xmac core, both branches below end up in the same register write, enabling
S-tag recognition and strip-all at once. Is that the intended combination
given vlan_rx_hw() cannot report the tag type?

[ ... ]

> @@ -283,7 +270,14 @@ static void dwxlgmac2_set_hw_vlan_mode(struct mac_device_info *hw)
>  	void __iomem *ioaddr = hw->pcsr;
>  	u32 value = readl(ioaddr + VLAN_TAG);
>  
> -	writel(value | VLAN_EDVLP, ioaddr + VLAN_TAG);
> +	value |= VLAN_EDVLP;
> +
> +	if (hw->hw_svlan_en)
> +		value |= VLAN_ESVL | VLAN_DOVLTC;
> +	else
> +		value &= ~(VLAN_ESVL | VLAN_DOVLTC);
> +
> +	writel(value, ioaddr + VLAN_TAG);
>  }

This path previously only set EDVLP and never touched ESVL or DOVLTC. Does
XLGMAC now also get S-VLAN classification and the disabled tag type
comparison enabled on every open, resume and feature change? XLGMAC shares
core_type DWMAC_CORE_XGMAC, so it keeps hw_svlan_en set even after the
later dwmac4 fix in the series.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908164309.59282-1-ovidiu.panait.rb%40renesas.com


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 4/7] net: stmmac: Do not advertise S-VLAN stripping when it is disabled
  2026-09-08 16:43 ` [PATCH net v4 4/7] net: stmmac: Do not advertise S-VLAN stripping when it is disabled Ovidiu Panait
@ 2026-09-10 23:46   ` netdev-bot+sashiko
  0 siblings, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10 23:46 UTC (permalink / raw)
  To: ovidiu.panait.rb
  Cc: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai, linux-kernel, netdev, linux-stm32,
	linux-arm-kernel, linux-kselftest

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

Critical: 0 · High: 0 (1) · Medium: 0 · Low: 2

- [Low] Commit-message premise vs. actual code: with rx-vlan-offload off
  (priv->hw->hw_vlan_en == false) the driver did not stop stripping…
- [Low] The patch carries 'Fixes: 750011e239a5' yet its hunk depends on
  priv->hw->hw_svlan_en, a struct member added by the immediately…

Pre-existing issues:
- [High] Hardware-stripped S-VLAN tags are handed to the stack with the
  wrong TPID.

--

net: stmmac: Do not advertise S-VLAN stripping when it is disabled

The patch makes NETIF_F_HW_VLAN_STAG_RX follow NETIF_F_HW_VLAN_CTAG_RX in
stmmac_fix_features(), so that rx-vlan-stag-hw-parse is no longer reported
as enabled after rx-vlan-offload is turned off.  Both are driven by the same
EVLS field, so the two features are tied together in hardware.

A few questions below.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index cd9671493b41..c81c5bb5b075 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6189,6 +6189,13 @@ static netdev_features_t stmmac_fix_features(struct net_device *dev,
>  	if (priv->plat->bugged_jumbo && (dev->mtu > ETH_DATA_LEN))
>  		features &= ~NETIF_F_CSUM_MASK;
>  
> +	if (priv->hw->hw_svlan_en) {
> +		if (features & NETIF_F_HW_VLAN_CTAG_RX)
> +			features |= NETIF_F_HW_VLAN_STAG_RX;
> +		else
> +			features &= ~NETIF_F_HW_VLAN_STAG_RX;
> +	}
> +

[Severity: Low]
The commit message describes the problem purely as a stale advertisement
("rx-vlan-stag-hw-parse keeps being advertised as enabled"), but clearing
NETIF_F_HW_VLAN_STAG_RX here also turns off the software S-tag stripper in
the same file:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_rx_vlan() {
	if ((vlan_proto == htons(ETH_P_8021Q) &&
	     dev->features & NETIF_F_HW_VLAN_CTAG_RX) ||
	    (vlan_proto == htons(ETH_P_8021AD) &&
	     dev->features & NETIF_F_HW_VLAN_STAG_RX)) {
	...
}

With rx-vlan-offload off, hw_vlan_en is false and stmmac_rx() takes the
stmmac_rx_vlan() path, so before this change 802.1ad tags were still popped
into skb metadata by the driver; afterwards they are left in the linear
frame.

This looks harmless in practice, since __netif_receive_skb_core() calls
skb_vlan_untag() for any eth_type_vlan() protocol and re-establishes the tag
with the correct ETH_P_8021AD TPID.  Would it be worth mentioning the
datapath side effect in the changelog, so it is clear that only the place
where the S-tag is moved into skb metadata changes?

[Severity: Low]
The commit carries Fixes: 750011e239a5 but the hunk tests
priv->hw->hw_svlan_en, which is a struct mac_device_info member added by the
preceding, non-Fixes patch of this series ("net: stmmac: Rework S-VLAN
handling").  A backport of this patch alone onto a kernel that only has
750011e239a5 would not build.

Could a note about that dependency be added for stable backporters?

[Severity: High]
This isn't a bug introduced by this patch, but keeping
NETIF_F_HW_VLAN_STAG_RX advertised whenever NETIF_F_HW_VLAN_CTAG_RX is on
also keeps advertising an S-VLAN parse offload whose result is labelled with
the wrong TPID.

In the hardware path, the stripped outer tag is always handed up as 802.1Q:

drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c:vlan_rx_hw() {
	if (hw->desc->get_rx_vlan_valid(rx_desc)) {
		u16 vid = hw->desc->get_rx_vlan_tci(rx_desc);

		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
	}
}

while the descriptor helper deliberately accepts S-tagged and double-tagged
frames:

drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:dwxgmac2_wrback_get_rx_vlan_valid() {
	et_lt = FIELD_GET(XGMAC_RDES3_ET_LT, le32_to_cpu(p->des3));

	return et_lt >= XGMAC_ET_LT_VLAN_STAG &&
	       et_lt <= XGMAC_ET_LT_DVLAN_STAG_CTAG;
}

So on a dwxgmac2 core with rx-vlan-offload on, an 802.1ad frame is delivered
as 802.1Q: an 8021ad upper device never sees it, a same-VID 8021Q upper
device does, and a forwarded frame is re-tagged with the wrong TPID.

This appears to date back to 750011e239a5 rather than to this patch, and
vlan_rx_hw() is untouched by the rest of the series: the later "net: stmmac:
Disable S-Tag processing on dwmac4" only drops the STAG advertisement for
non-XGMAC cores, and dwxgmac210_vlan_ops still points .rx_hw_vlan at
vlan_rx_hw().  Since the ET/LT field already distinguishes S-tag, C-tag and
double tag, could vlan_rx_hw() use it to pick between ETH_P_8021Q and
ETH_P_8021AD?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908164309.59282-1-ovidiu.panait.rb%40renesas.com


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 5/7] net: stmmac: Disable S-Tag processing on dwmac4
  2026-09-08 16:43 ` [PATCH net v4 5/7] net: stmmac: Disable S-Tag processing on dwmac4 Ovidiu Panait
  2026-09-09 12:19   ` Maxime Chevallier
@ 2026-09-10 23:46   ` netdev-bot+sashiko
  1 sibling, 0 replies; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10 23:46 UTC (permalink / raw)
  To: ovidiu.panait.rb
  Cc: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai, linux-kernel, netdev, linux-stm32,
	linux-arm-kernel, linux-kselftest

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

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

- [High] Incomplete fix: the bug class the commit message names (a
  hardware-stripped 802.1ad S-Tag being handed to the stack as…
- [Low] The in-kernel selftest stmmac_test_dvlanfilt()
  (drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1069-1076)…

--

net: stmmac: Disable S-Tag processing on dwmac4

This restricts NETIF_F_HW_VLAN_STAG_RX, NETIF_F_HW_VLAN_STAG_FILTER and
priv->hw->hw_svlan_en to DWMAC_CORE_XGMAC, on the grounds that dwmac4
cannot report which tag type the hardware stripped.  The stated goal is
that 802.1ad tags are then left in place and handled by the software VLAN
path.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index c81c5bb5b075..eab2903a66e4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7952,15 +7952,20 @@ static int __stmmac_dvr_probe(struct device *device,
>  	ndev->watchdog_timeo = msecs_to_jiffies(watchdog);
>  #ifdef STMMAC_VLAN_TAG_USED
>  	/* Both mac100 and gmac support receive VLAN tag detection */
> -	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX;
> -	priv->hw->hw_svlan_en = true;
> +	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
> +	if (priv->plat->core_type == DWMAC_CORE_XGMAC) {
> +		ndev->features |= NETIF_F_HW_VLAN_STAG_RX;
> +		priv->hw->hw_svlan_en = true;
> +	}

[Severity: High]
Does this leave the misreported tag protocol reachable on dwxgmac2?

The commit message describes the bug as vlan_rx_hw() hardcoding
ETH_P_8021Q, but vlan_rx_hw() is unchanged and is still the .rx_hw_vlan
callback in dwxgmac210_vlan_ops:

drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c:vlan_rx_hw() {
	if (hw->desc->get_rx_vlan_valid(rx_desc)) {
		u16 vid = hw->desc->get_rx_vlan_tci(rx_desc);

		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
	}
}

On DWMAC_CORE_XGMAC hw_svlan_en stays true here, and dwmac_is_xmac()
below sets hw_vlan_en, so vlan_set_hw_mode() still programs the MAC to
recognise and strip S-Tags:

drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c:vlan_set_hw_mode() {
	if (hw->hw_svlan_en)
		value |= VLAN_ESVL | VLAN_DOVLTC;
	...
	if (hw->hw_vlan_en)
		/* Always strip VLAN on Receive */
		value |= VLAN_TAG_STRIP_ALL;
}

And dwxgmac2_wrback_get_rx_vlan_valid() collapses the ET_LT information
the commit message points at into a boolean, so vlan_rx_hw() has no way
to tell an S-Tag from a C-Tag:

drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:dwxgmac2_wrback_get_rx_vlan_valid() {
	return et_lt >= XGMAC_ET_LT_VLAN_STAG &&
	       et_lt <= XGMAC_ET_LT_DVLAN_STAG_CTAG;
}

With that, doesn't a received 802.1ad frame on a dwxgmac2 interface with
default features still reach the stack with skb->vlan_proto ==
ETH_P_8021Q, so the 802.1ad VLAN device never sees it and an 802.1Q
sub-interface or bridge VLAN with the same VID does?  That is the same
QinQ symptom the commit says it fixes.

Would it make sense to either derive the protocol in vlan_rx_hw() from
ET_LT, or state in the commit message that the dwxgmac2 side is handled
by a follow-up?

A second question on the gate itself: the DWXLGMAC entry in
drivers/net/ethernet/stmicro/stmmac/hwif.c also uses
core_type = DWMAC_CORE_XGMAC, but dwxlgmac2_vlan_ops has no .rx_hw_vlan
callback at all:

drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c {
const struct stmmac_vlan_ops dwxlgmac2_vlan_ops = {
	.update_vlan_hash = dwxgmac2_update_vlan_hash,
	.enable_vlan = vlan_enable,
	.set_hw_vlan_mode = dwxlgmac2_set_hw_vlan_mode,
};
}

Should the check be narrower than core_type == DWMAC_CORE_XGMAC so
NETIF_F_HW_VLAN_STAG_RX is not advertised on DWXLGMAC?

>  	if (dwmac_is_xmac(priv->plat->core_type)) {
>  		ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
>  		priv->hw->hw_vlan_en = true;
>  	}
>  	if (priv->dma_cap.vlhash) {
>  		ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> -		ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
> +		if (priv->plat->core_type == DWMAC_CORE_XGMAC)
> +			ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
>  	}

[Severity: Low]
Should the selftest gate be updated to match this?

stmmac_test_dvlanfilt() gates on priv->dma_cap.vlhash, while its peer
stmmac_test_dvlanfilt_perfect() gates on the feature bit:

drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c {
static int stmmac_test_dvlanfilt(struct stmmac_priv *priv)
{
	if (!priv->dma_cap.vlhash)
		return -EOPNOTSUPP;
	...
}

static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
{
	...
	if (!(priv->dev->features & NETIF_F_HW_VLAN_STAG_FILTER))
		return -EOPNOTSUPP;
	...
}
}

Before this change both gates were equivalent, since STAG_FILTER was set
whenever vlhash was set.  On a dwmac4 core with vlhash, does
stmmac_test_dvlanfilt() now still run while exercising no hardware?  The
vlan_vid_add(priv->dev, htons(ETH_P_8021AD), ...) in
__stmmac_test_dvlanfilt() returns success without reaching
ndo_vlan_rx_add_vid():

net/8021q/vlan_core.c:vlan_add_rx_filter_info() {
	if (!vlan_hw_filter_capable(dev, proto))
		return 0;
}

so no VLAN hash or perfect filter entry is programmed and the frames are
validated through the software VLAN path instead.  Would ethtool -t then
report the double-VLAN hash filter test as passing on a core where the
feature is no longer advertised, while the perfect-filter test is
correctly skipped?

>  	if (priv->dma_cap.vlins)
>  		ndev->features |= NETIF_F_HW_VLAN_CTAG_TX;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908164309.59282-1-ovidiu.panait.rb%40renesas.com


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 7/7] selftests: drv-net: Add VLAN test
  2026-09-08 16:43 ` [PATCH net v4 7/7] selftests: drv-net: Add VLAN test Ovidiu Panait
@ 2026-09-10 23:46   ` netdev-bot+sashiko
  2026-09-10 23:52     ` Jakub Kicinski
  0 siblings, 1 reply; 17+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10 23:46 UTC (permalink / raw)
  To: ovidiu.panait.rb
  Cc: maxime.chevallier, recv.jo, andrew+netdev, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, alexandre.torgue, shuah, joabreu,
	yi.fang.gan, jun.ann.lai, linux-kernel, netdev, linux-stm32,
	linux-arm-kernel, linux-kselftest

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

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

- [Medium] vlan.py's 802.1ad/QinQ matrix never requests the one feature
  combination in which the QinQ mis-stripping bug the commit says it…
- [Low] test() calls cfg.require_ipver("4")
  (tools/testing/selftests/drivers/net/hw/vlan.py:94) although the test…

--

selftests: drv-net: Add VLAN test

Adds tools/testing/selftests/drivers/net/hw/vlan.py, which pings over
802.1q, 802.1ad and QinQ VLAN interfaces with hardware RX VLAN stripping
toggled on and off, plus the Makefile and config entries for it.

A couple of questions about the variant matrix and the environment
requirement below.

> diff --git a/tools/testing/selftests/drivers/net/hw/vlan.py b/tools/testing/selftests/drivers/net/hw/vlan.py
> new file mode 100755
> index 000000000000..61fa452b95bd
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/vlan.py
> @@ -0,0 +1,112 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +"""
> +VLAN tests.
> +
> +Validates that ping traffic is sent and received correctly over 802.1q
> +and 802.1ad VLAN interfaces, with hardware RX VLAN stripping enabled and
> +disabled on the local interface.
> +

[ ... ]

> +def _vlan_setup(base, addr, outer_proto, inner_proto, host=None):
> +    """Create VLAN interfaces on base and set an IP on the innermost one."""
> +
> +    _vlan_add(base, OUTER_DEV, outer_proto, OUTER_VID, host=host)
> +    if inner_proto:
> +        _vlan_add(OUTER_DEV, INNER_DEV, inner_proto, INNER_VID, host=host)
> +
> +    dev = INNER_DEV if inner_proto else OUTER_DEV
> +    ip(f"addr add {addr}/24 dev {dev}", host=host)
> +
> +
> +def _setup(cfg, outer_proto, inner_proto, hw_strip):
> +    """Configure VLAN stripping and create the VLAN interfaces."""
> +
> +    feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
> +    feats = {"rx-vlan-offload": hw_strip}
> +    if outer_proto == "802.1ad":
> +        feats["rx-vlan-stag-hw-parse"] = hw_strip
> +    set_ethtool_feat(cfg.ifname, feat, feats)

[Severity: Medium]

Does this combination actually reach the QinQ mis-stripping case the commit
message says the test aims to catch?

In _setup() both rx-vlan-offload and rx-vlan-stag-hw-parse are driven to the
same hw_strip value, and _vlan_variants() only yields all-on and all-off
combinations. The combination rx-vlan-offload=on with
rx-vlan-stag-hw-parse=off is never requested.

On a device where NETIF_F_HW_VLAN_STAG_RX is fixed off, set_ethtool_feat()
raises before running any ethtool -K:

tools/testing/selftests/drivers/net/lib/py/feat.py:set_ethtool_feat() {
	...
		if current[name]["active"] != state:
			no_change = False
			if current[name]["fixed"]:
				raise KsftXfailEx(f"Device does not support {name}")
	...
}

So the whole 8021ad_hw / qinq_hw variant is skipped, including the
rx-vlan-offload=on part which is settable on those devices. The surviving
8021ad_sw / qinq_sw variants then force rx-vlan-offload off.

That leaves no 802.1ad or QinQ ping running with hardware C-tag stripping
active. Wouldn't a driver that strips the first VLAN tag regardless of TPID,
reporting an 0x88a8 tag as a C-tag, still report "ok 8021ad_sw" and
"ok qinq_sw" here? The sample TAP output in the commit message shows exactly
this on st_gmac, and netdevsim likewise advertises only
NETIF_F_HW_VLAN_STAG_FILTER and not NETIF_F_HW_VLAN_STAG_RX in nsim_setup().

Should the 802.1ad/QinQ variants instead be split so that rx-vlan-offload=on
is requested independently of rx-vlan-stag-hw-parse, so a fixed
rx-vlan-stag-hw-parse does not remove the hardware stripping coverage?

Related, the docstring says:

  Validates that ping traffic is sent and received correctly over 802.1q
  and 802.1ad VLAN interfaces, with hardware RX VLAN stripping enabled and
  disabled on the local interface.

and the commit message says the three configurations are covered "each with
hardware RX VLAN stripping enabled and disabled". Could these be reworded to
match what happens on devices where rx-vlan-stag-hw-parse cannot be changed?

> +
> +    _vlan_setup(cfg.ifname, LOCAL_IP, outer_proto, inner_proto)
> +    _vlan_setup(cfg.remote_ifname, REMOTE_IP, outer_proto, inner_proto,
> +                host=cfg.remote)
> +
> +
> +def _vlan_variants():
> +    """Generator that yields the VLAN protocols and the stripping mode."""
> +
> +    yield KsftNamedVariant("8021q_hw", "802.1q", None, True)
> +    yield KsftNamedVariant("8021q_sw", "802.1q", None, False)
> +    yield KsftNamedVariant("8021ad_hw", "802.1ad", None, True)
> +    yield KsftNamedVariant("8021ad_sw", "802.1ad", None, False)
> +    yield KsftNamedVariant("qinq_hw", "802.1ad", "802.1q", True)
> +    yield KsftNamedVariant("qinq_sw", "802.1ad", "802.1q", False)
> +
> +
> +@ksft_variants(_vlan_variants())
> +def test(cfg, outer_proto, inner_proto, hw_strip):
> +    """Run a single VLAN test"""
> +
> +    cfg.require_ipver("4")

[Severity: Low]

Is this requirement needed? The test never uses the environment's IPv4
endpoint addresses: _vlan_setup() installs its own 198.51.100.1/24 and
198.51.100.2/24 on the newly created VLAN devices and the ping goes between
those.

NetDrvEpEnv.require_ipver() skips unless both the local and remote address
for that version were configured:

tools/testing/selftests/drivers/net/lib/py/env.py:require_ipver() {
	if not self.addr_v[ipver] or not self.remote_addr_v[ipver]:
		raise KsftSkipEx(f"Test requires IPv{ipver} connectivity")
}

So an environment configured with only LOCAL_V6/REMOTE_V6, which
_check_env() accepts, skips all six variants even though the base ifname,
the remote ifname and the test's own VLAN addressing are all available.

drivers/net/macsec.py does the same 198.51.100.x-over-VLAN ping in
_setup_vlan_ips() without calling require_ipver(). Could the call be dropped
here as well?

> +
> +    _setup(cfg, outer_proto, inner_proto, hw_strip)
> +
> +    cmd(f"ping -c 1 -W 5 {REMOTE_IP}")

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908164309.59282-1-ovidiu.panait.rb%40renesas.com


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net v4 7/7] selftests: drv-net: Add VLAN test
  2026-09-10 23:46   ` netdev-bot+sashiko
@ 2026-09-10 23:52     ` Jakub Kicinski
  0 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2026-09-10 23:52 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: ovidiu.panait.rb, maxime.chevallier, recv.jo, andrew+netdev,
	davem, edumazet, pabeni, mcoquelin.stm32, alexandre.torgue, shuah,
	joabreu, yi.fang.gan, jun.ann.lai, linux-kernel, netdev,
	linux-stm32, linux-arm-kernel, linux-kselftest

On Thu, 10 Sep 2026 23:46:05 +0000 netdev-bot+sashiko@kernel.org wrote:
> > +@ksft_variants(_vlan_variants())
> > +def test(cfg, outer_proto, inner_proto, hw_strip):
> > +    """Run a single VLAN test"""
> > +
> > +    cfg.require_ipver("4")  
> 
> [Severity: Low]
> 
> Is this requirement needed? The test never uses the environment's IPv4
> endpoint addresses: _vlan_setup() installs its own 198.51.100.1/24 and
> 198.51.100.2/24 on the newly created VLAN devices and the ping goes between
> those.

AI seems to be right, what we'd actually need here is 
"require l2 connectivity" Let's leave it be for now,
just drop the require_ipver("4") and maybe we'll revisit
this when we also have the PHY related tests.

Could you break out the selftests to a separate series
for net-next? This posting seems to have conflicted with
other stmmac patches on the list and it'd be great to
run them a few times on our CI NICs before merging.


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-09-10 23:52 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08 16:43 [PATCH net v4 0/7] net: stmmac: Fix double VLAN 802.1ad tag handling Ovidiu Panait
2026-09-08 16:43 ` [PATCH net v4 1/7] net: stmmac: Remove VLAN perfect matching dead code Ovidiu Panait
2026-09-10 23:45   ` netdev-bot+sashiko
2026-09-08 16:43 ` [PATCH net v4 2/7] net: stmmac: Stop toggling the EDVLP bit Ovidiu Panait
2026-09-10 23:45   ` netdev-bot+sashiko
2026-09-08 16:43 ` [PATCH net v4 3/7] net: stmmac: Rework S-VLAN handling Ovidiu Panait
2026-09-10 23:46   ` netdev-bot+sashiko
2026-09-08 16:43 ` [PATCH net v4 4/7] net: stmmac: Do not advertise S-VLAN stripping when it is disabled Ovidiu Panait
2026-09-10 23:46   ` netdev-bot+sashiko
2026-09-08 16:43 ` [PATCH net v4 5/7] net: stmmac: Disable S-Tag processing on dwmac4 Ovidiu Panait
2026-09-09 12:19   ` Maxime Chevallier
2026-09-10 23:46   ` netdev-bot+sashiko
2026-09-08 16:43 ` [PATCH net v4 6/7] selftests: drv-net: Move _set_ethtool_feat() into lib Ovidiu Panait
2026-09-10 23:42   ` Jakub Kicinski
2026-09-08 16:43 ` [PATCH net v4 7/7] selftests: drv-net: Add VLAN test Ovidiu Panait
2026-09-10 23:46   ` netdev-bot+sashiko
2026-09-10 23:52     ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).