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

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

[Ovidiu Panait]

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

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

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

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

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.

v6:
- Rebased on latest net.

v5: https://lore.kernel.org/all/20260920153516.103461-1-ovidiu.panait.rb@renesas.com/
- Split out the selftest part and sent it separately for net-next:
  https://lore.kernel.org/all/20260918112529.96039-1-ovidiu.panait.rb@renesas.com/
- Dropped the S-VLAN rework patch and replaced it with a plain
  double -> svlan rename. Rebased the patches that depended on it.
- Added Reviewed-by tag from Maxime on patch 5/5.

v4: https://lore.kernel.org/all/20260908164309.59282-1-ovidiu.panait.rb@renesas.com/
- 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.

Ovidiu Panait (5):
  net: stmmac: Remove VLAN perfect matching dead code
  net: stmmac: Stop toggling the EDVLP bit
  net: stmmac: Rename double VLAN references to svlan
  net: stmmac: Do not advertise S-VLAN stripping when it is disabled
  net: stmmac: Disable S-Tag processing on dwmac4

 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 60 ++++++++---------
 .../stmicro/stmmac/stmmac_selftests.c         | 30 ++++-----
 .../net/ethernet/stmicro/stmmac/stmmac_vlan.c | 65 +++++--------------
 5 files changed, 63 insertions(+), 96 deletions(-)

-- 
2.34.1

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

[Ovidiu Panait]

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

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

Therefore, remove it.

Fixes: c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if HASH is not available")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
v6 changes: None.

v5 changes: None

v4 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 857f7562c6c6..df2126d71c2f 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 3f34d491c959..14183f92663a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6838,29 +6838,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

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch removes the only code that ever read
  priv->dma_cap.vlhash inside stmmac_vlan_update().
- [Low] The commit message's second reason for removal is wrong for
  XLGMAC.

--

net: stmmac: Remove VLAN perfect matching dead code

This commit removes the perfect-matching fallback in stmmac_vlan_update().
That fallback was used when the VLAN hash filter is unavailable
(!priv->dma_cap.vlhash). The commit also drops the perfect_match argument
from the update_vlan_hash() callbacks, together with the matching branches
in vlan_update_hash() and dwxgmac2_update_vlan_hash().

> 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.

[Severity: Low]
Does this hold for XLGMAC? hwif.c maps XLGMAC cores to dwxlgmac2_vlan_ops,
which has no add, del or restore filter callbacks:

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,
};

dwxlgmac2_setup() also never calls stmmac_get_num_vlan(). Only
dwmac4_setup() and dwxgmac2_setup() do. mac_device_info is zero-allocated,
so hw->num_vlan stays 0 on XLGMAC, and stmmac_vlan_rx_add_vid() skips the
num_vlan block.

On XLGMAC, the perfect_match branch removed from
dwxgmac2_update_vlan_hash() seems to be the only non-hash VLAN filter code.
It is not a duplicate of vlan_add_hw_rx_fltr().

The removal itself still looks safe. The first reason in the commit message
holds, since the filter features are only advertised when
dma_cap.vlhash is set. Also, the removed "pmatch = vid" ran after the
for_each_set_bit() loop, so it only ever programmed VID 4096.

Could this paragraph be corrected? Otherwise reviewers and backporters may
assume XLGMAC keeps a perfect-filter path.

[ ... ]

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index ec62fa7418f4..bdb856291340 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6853,29 +6853,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);
>  }

[Severity: Low]
This removes the only check of priv->dma_cap.vlhash in
stmmac_vlan_update(). Do the "(perf)" ethtool selftests still test anything
different from the hash tests now?

stmmac_test_vlanfilt_perfect() and stmmac_test_dvlanfilt_perfect() in
stmmac_selftests.c still toggle that capability to force the perfect-match
path:

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;
    ...
}

With this patch, adding VID 0x123 makes stmmac_vlan_update() compute a
non-zero hash and always pass it to update_vlan_hash(). As a result,
vlan_update_hash() and dwxgmac2_update_vlan_hash() set
VLAN_VTHM | VLAN_ETV (and XGMAC_FILTER_VTFE on XGMAC), whatever the vlhash
toggle says.

Before this patch, the fallback forced hash = 0 and cleared VTHM. That left
only the perfect filters programmed by vlan_add_hw_rx_fltr() active.

The "VLAN Filtering (perf)" and "Double VLAN Filter (perf)" entries are
still registered in stmmac_selftests[]. They now pass on the hash filter
alone, because the test VIDs 0x124..0x126 fall into different hash bins. On
XLGMAC (num_vlan == 0, no add_hw_vlan_rx_fltr), they don't exercise perfect
filtering at all.

These selftests also seem to stay as they are later in the series. The
later rename only changes the dvlanfilt names to svlanfilt.

Should these two selftests be removed or reworked as part of this change?
Otherwise ethtool -t reports PASS for perfect filtering that is no longer
tested on its own.

[ ... ]

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

[PATCH net v6 2/5] net: stmmac: Stop toggling the EDVLP bit

[Ovidiu Panait]

Currently, the double VLAN EDVLP bit is toggled whenever an 802.1ad VLAN
is registered. This bit enables processing of the inner VLAN tag, which
is completely unrelated to S-Tag VLAN 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>
---
v6 changes: None.

v5 changes: None.

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

Re: [PATCH net v6 2/5] net: stmmac: Stop toggling the EDVLP bit

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The COE benefit that the commit message gives as its reason
  for this change never shows up in this tree.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is still
  there at the end of the series for XGMAC 2.10.

--

net: stmmac: Stop toggling the EDVLP bit

The EDVLP bit is no longer set and cleared in the VLAN hash update paths
when an 802.1ad VLAN is registered. It is now always set from
vlan_set_hw_mode(), and a new dwxlgmac2_set_hw_vlan_mode() callback sets it
on dwxlgmac2 as well.

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

[Severity: Medium]
The commit message says EDVLP is kept always enabled "so that COE can work
for packets with an inner VLAN header". Does the driver actually use the
checksum offload result for those packets?

stmmac_has_ip_ethertype() in stmmac_main.c rejects any frame that still has
a VLAN header in the payload:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_has_ip_ethertype() {
    ...
	proto = __vlan_get_protocol(skb, eth_header_parse_protocol(skb),
				    &depth);

	return (depth <= ETH_HLEN) &&
		(proto == htons(ETH_P_IP) || proto == htons(ETH_P_IPV6));
}

On RX, EVLS=STRIP_ALL strips only the outer tag and EIVLS is never
programmed, so the inner 802.1Q header stays in-band. Both stmmac_rx() and
stmmac_dispatch_skb_zc() then drop the hardware result:

	if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb) ||
	    (status & csum_none))
		skb_checksum_none_assert(skb);

On TX, stmmac_features_check() clears the checksum features for the same
skbs. STAG_TX is not advertised, so a QinQ frame always has at least one
in-band tag:

	if (priv->plat->tx_queues_cfg[queue].coe_unsupported ||
	    !stmmac_has_ip_ethertype(skb))
		features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);

This holds whether or not EDVLP is set. No later patch in the series changes
stmmac_has_ip_ethertype(). The later patch "net: stmmac: Rename double VLAN
references to svlan" also says "the networking core does not support
offloads for the inner tag anyway".

This patch changes VLAN_TAG programming on all DWMAC4/5, XGMAC and XLGMAC
cores. Could the COE rationale be dropped or corrected in the commit
message? If COE for inner-VLAN frames is the goal, does
stmmac_has_ip_ethertype() also need updating, along with a Fixes: tag?

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

[Severity: High]
This problem was not introduced by this patch. It is still present on
XGMAC 2.10 cores at the end of the series, though.

Registering an 802.1ad VID follows this call chain, with is_double set:

stmmac_vlan_rx_add_vid()->stmmac_vlan_update()->dwxgmac2_update_vlan_hash()

That sets ESVL and DOVLTC. With hw_vlan_en, which is the default on XGMAC,
vlan_set_hw_mode() also programs EVLS=STRIP_ALL and EVLRXS. The MAC then
strips 0x88A8 outer tags and writes the TCI into RDES0.

dwxgmac2_wrback_get_rx_vlan_valid() accepts the S-tag packet types:

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

However, vlan_rx_hw() always reports the stripped tag as a C-tag:

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

Won't S-tagged frames then go to an 802.1Q upper device with the same VID,
or be dropped, and never reach the 802.1ad upper device? The software path
in stmmac_rx_vlan() does tell ETH_P_8021AD apart, so the two paths disagree.

The last patch in the series, "net: stmmac: Disable S-Tag processing on
dwmac4", stops advertising STAG_RX/STAG_FILTER only on dwmac4. Its commit
message says the XGMAC tag type can be derived from ET_LT. Yet vlan_rx_hw()
is unchanged, and XGMAC still advertises the S-tag features in
__stmmac_dvr_probe():

	if (priv->plat->core_type == DWMAC_CORE_XGMAC)
		ndev->features |= NETIF_F_HW_VLAN_STAG_RX;

On XGMAC, should vlan_rx_hw() use ET_LT to report ETH_P_8021AD, or should
XGMAC also stop advertising the S-tag features?

S-tag recognition appears to be controlled by ESVL/DOVLTC, not EDVLP.
Setting EDVLP unconditionally here does not seem to make this worse.

[ ... ]

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

[PATCH net v6 3/5] net: stmmac: Rename double VLAN references to svlan

[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 reduce the confusion regarding S-Tag vs double VLAN handling,
rename double -> svlan.

No functional change intended.

Suggested-by: Joseph Steel <recv.jo@gmail.com>
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
v6 changes:
- Rebased on top of latest net.

v5 changes: New patch.

 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 34 +++++++++----------
 .../stmicro/stmmac/stmmac_selftests.c         | 30 ++++++++--------
 .../net/ethernet/stmicro/stmmac/stmmac_vlan.c |  8 ++---
 5 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index df2126d71c2f..5e2654c91b41 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,
-				 bool is_double);
+				 bool is_svlan);
 	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..d2d387f45c10 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -344,7 +344,7 @@ struct stmmac_priv {
 	void __iomem *ptpaddr;
 	void __iomem *estaddr;
 	unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
-	unsigned int num_double_vlans;
+	unsigned int num_svlans;
 	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 14183f92663a..0d70eb452af7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6835,7 +6835,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, bool is_svlan)
 {
 	u32 crc, hash = 0;
 	u16 vid = 0;
@@ -6849,7 +6849,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, is_svlan);
 }
 
 /* FIXME: This may need RXC to be running, but it may be called with BH
@@ -6858,8 +6858,8 @@ 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;
+	unsigned int num_svlans;
+	bool is_svlan = false;
 	int ret;
 
 	ret = pm_runtime_resume_and_get(priv->device);
@@ -6867,11 +6867,11 @@ static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid
 		return ret;
 
 	if (be16_to_cpu(proto) == ETH_P_8021AD)
-		is_double = true;
+		is_svlan = true;
 
 	set_bit(vid, priv->active_vlans);
-	num_double_vlans = priv->num_double_vlans + is_double;
-	ret = stmmac_vlan_update(priv, num_double_vlans);
+	num_svlans = priv->num_svlans + is_svlan;
+	ret = stmmac_vlan_update(priv, num_svlans);
 	if (ret) {
 		clear_bit(vid, priv->active_vlans);
 		goto err_pm_put;
@@ -6881,12 +6881,12 @@ 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, priv->num_svlans);
 			goto err_pm_put;
 		}
 	}
 
-	priv->num_double_vlans = num_double_vlans;
+	priv->num_svlans = num_svlans;
 
 err_pm_put:
 	pm_runtime_put(priv->device);
@@ -6900,8 +6900,8 @@ 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;
+	unsigned int num_svlans;
+	bool is_svlan = false;
 	int ret;
 
 	ret = pm_runtime_resume_and_get(priv->device);
@@ -6909,11 +6909,11 @@ static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vi
 		return ret;
 
 	if (be16_to_cpu(proto) == ETH_P_8021AD)
-		is_double = true;
+		is_svlan = true;
 
 	clear_bit(vid, priv->active_vlans);
-	num_double_vlans = priv->num_double_vlans - is_double;
-	ret = stmmac_vlan_update(priv, num_double_vlans);
+	num_svlans = priv->num_svlans - is_svlan;
+	ret = stmmac_vlan_update(priv, num_svlans);
 	if (ret) {
 		set_bit(vid, priv->active_vlans);
 		goto del_vlan_error;
@@ -6923,12 +6923,12 @@ 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, priv->num_svlans);
 			goto del_vlan_error;
 		}
 	}
 
-	priv->num_double_vlans = num_double_vlans;
+	priv->num_svlans = num_svlans;
 
 del_vlan_error:
 	pm_runtime_put(priv->device);
@@ -6944,7 +6944,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, priv->num_svlans);
 }
 
 static int stmmac_bpf(struct net_device *dev, struct netdev_bpf *bpf)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index c25dc9f89270..c485217ba880 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -243,7 +243,7 @@ struct stmmac_test_priv {
 	int (*func)(struct sk_buff *skb, struct net_device *ndev,
 		    struct packet_type *pt, struct net_device *orig_ndev);
 	bool capture_all;
-	int double_vlan;
+	int svlan;
 	int vlan_id;
 	int ok;
 };
@@ -285,7 +285,7 @@ static int stmmac_test_loopback_validate(struct sk_buff *skb,
 	}
 
 	ihdr = ip_hdr(skb);
-	if (tpriv->double_vlan)
+	if (tpriv->svlan)
 		ihdr = (struct iphdr *)(skb_network_header(skb) + 4);
 
 	if (tpriv->packet->tcp) {
@@ -936,7 +936,7 @@ static int stmmac_test_vlan_validate(struct sk_buff *skb,
 	struct iphdr *ihdr;
 	u16 proto;
 
-	proto = tpriv->double_vlan ? ETH_P_8021AD : ETH_P_8021Q;
+	proto = tpriv->svlan ? ETH_P_8021AD : ETH_P_8021Q;
 
 	skb = skb_unshare(skb, GFP_ATOMIC);
 	if (!skb)
@@ -963,7 +963,7 @@ static int stmmac_test_vlan_validate(struct sk_buff *skb,
 	}
 
 	ihdr = ip_hdr(skb);
-	if (tpriv->double_vlan)
+	if (tpriv->svlan)
 		ihdr = (struct iphdr *)(skb_network_header(skb) + 4);
 	if (ihdr->protocol != IPPROTO_UDP)
 		goto out;
@@ -1080,7 +1080,7 @@ static int stmmac_test_vlanfilt_perfect(struct stmmac_priv *priv)
 	return ret;
 }
 
-static int __stmmac_test_dvlanfilt(struct stmmac_priv *priv)
+static int __stmmac_test_svlanfilt(struct stmmac_priv *priv)
 {
 	struct stmmac_packet_attrs attr = { };
 	struct stmmac_test_priv *tpriv;
@@ -1092,7 +1092,7 @@ static int __stmmac_test_dvlanfilt(struct stmmac_priv *priv)
 		return -ENOMEM;
 
 	tpriv->ok = false;
-	tpriv->double_vlan = true;
+	tpriv->svlan = true;
 	init_completion(&tpriv->comp);
 
 	tpriv->pt.type = htons(ETH_P_8021Q);
@@ -1155,15 +1155,15 @@ static int __stmmac_test_dvlanfilt(struct stmmac_priv *priv)
 	return ret;
 }
 
-static int stmmac_test_dvlanfilt(struct stmmac_priv *priv)
+static int stmmac_test_svlanfilt(struct stmmac_priv *priv)
 {
 	if (!priv->dma_cap.vlhash)
 		return -EOPNOTSUPP;
 
-	return __stmmac_test_dvlanfilt(priv);
+	return __stmmac_test_svlanfilt(priv);
 }
 
-static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
+static int stmmac_test_svlanfilt_perfect(struct stmmac_priv *priv)
 {
 	int ret, prev_cap = priv->dma_cap.vlhash;
 
@@ -1171,7 +1171,7 @@ static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
 		return -EOPNOTSUPP;
 
 	priv->dma_cap.vlhash = 0;
-	ret = __stmmac_test_dvlanfilt(priv);
+	ret = __stmmac_test_svlanfilt(priv);
 	priv->dma_cap.vlhash = prev_cap;
 
 	return ret;
@@ -1372,7 +1372,7 @@ static int stmmac_test_vlanoff_common(struct stmmac_priv *priv, bool svlan)
 	proto = svlan ? ETH_P_8021AD : ETH_P_8021Q;
 
 	tpriv->ok = false;
-	tpriv->double_vlan = svlan;
+	tpriv->svlan = svlan;
 	init_completion(&tpriv->comp);
 
 	tpriv->pt.type = svlan ? htons(ETH_P_8021Q) : htons(ETH_P_IP);
@@ -1960,11 +1960,11 @@ static const struct stmmac_test {
 		.name = "VLAN Filtering (perf)      ",
 		.fn = stmmac_test_vlanfilt_perfect,
 	}, {
-		.name = "Double VLAN Filter         ",
-		.fn = stmmac_test_dvlanfilt,
+		.name = "SVLAN Filtering            ",
+		.fn = stmmac_test_svlanfilt,
 	}, {
-		.name = "Double VLAN Filter (perf)  ",
-		.fn = stmmac_test_dvlanfilt_perfect,
+		.name = "SVLAN Filtering (perf)     ",
+		.fn = stmmac_test_svlanfilt_perfect,
 	}, {
 		.name = "Flexible RX Parser         ",
 		.fn = stmmac_test_rxp,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
index 200b34588c7f..fb9aad748cb3 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,
-			     bool is_double)
+			     bool is_svlan)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value;
@@ -173,7 +173,7 @@ static void vlan_update_hash(struct mac_device_info *hw, u32 hash,
 
 	if (hash) {
 		value |= VLAN_VTHM | VLAN_ETV;
-		if (is_double) {
+		if (is_svlan) {
 			value |= VLAN_ESVL;
 			value |= VLAN_DOVLTC;
 		} else {
@@ -235,7 +235,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,
-				      bool is_double)
+				      bool is_svlan)
 {
 	void __iomem *ioaddr = hw->pcsr;
 
@@ -251,7 +251,7 @@ 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) {
+		if (is_svlan) {
 			value |= VLAN_ESVL;
 			value |= VLAN_DOVLTC;
 		} else {
-- 
2.34.1

Re: [PATCH net v6 3/5] net: stmmac: Rename double VLAN references to svlan

[Maxime Chevallier]

Hi,

On 9/28/26 22:34, Ovidiu Panait wrote:
> 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 reduce the confusion regarding S-Tag vs double VLAN handling,
> rename double -> svlan.
> 
> No functional change intended.
> 
> Suggested-by: Joseph Steel <recv.jo@gmail.com>
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>

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

Maxime

> ---
> v6 changes:
> - Rebased on top of latest net.
> 
> v5 changes: New patch.
> 
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  2 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  2 +-
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 34 +++++++++----------
>  .../stmicro/stmmac/stmmac_selftests.c         | 30 ++++++++--------
>  .../net/ethernet/stmicro/stmmac/stmmac_vlan.c |  8 ++---
>  5 files changed, 38 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index df2126d71c2f..5e2654c91b41 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,
> -				 bool is_double);
> +				 bool is_svlan);
>  	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..d2d387f45c10 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -344,7 +344,7 @@ struct stmmac_priv {
>  	void __iomem *ptpaddr;
>  	void __iomem *estaddr;
>  	unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
> -	unsigned int num_double_vlans;
> +	unsigned int num_svlans;
>  	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 14183f92663a..0d70eb452af7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6835,7 +6835,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, bool is_svlan)
>  {
>  	u32 crc, hash = 0;
>  	u16 vid = 0;
> @@ -6849,7 +6849,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, is_svlan);
>  }
>  
>  /* FIXME: This may need RXC to be running, but it may be called with BH
> @@ -6858,8 +6858,8 @@ 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;
> +	unsigned int num_svlans;
> +	bool is_svlan = false;
>  	int ret;
>  
>  	ret = pm_runtime_resume_and_get(priv->device);
> @@ -6867,11 +6867,11 @@ static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid
>  		return ret;
>  
>  	if (be16_to_cpu(proto) == ETH_P_8021AD)
> -		is_double = true;
> +		is_svlan = true;
>  
>  	set_bit(vid, priv->active_vlans);
> -	num_double_vlans = priv->num_double_vlans + is_double;
> -	ret = stmmac_vlan_update(priv, num_double_vlans);
> +	num_svlans = priv->num_svlans + is_svlan;
> +	ret = stmmac_vlan_update(priv, num_svlans);
>  	if (ret) {
>  		clear_bit(vid, priv->active_vlans);
>  		goto err_pm_put;
> @@ -6881,12 +6881,12 @@ 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, priv->num_svlans);
>  			goto err_pm_put;
>  		}
>  	}
>  
> -	priv->num_double_vlans = num_double_vlans;
> +	priv->num_svlans = num_svlans;
>  
>  err_pm_put:
>  	pm_runtime_put(priv->device);
> @@ -6900,8 +6900,8 @@ 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;
> +	unsigned int num_svlans;
> +	bool is_svlan = false;
>  	int ret;
>  
>  	ret = pm_runtime_resume_and_get(priv->device);
> @@ -6909,11 +6909,11 @@ static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vi
>  		return ret;
>  
>  	if (be16_to_cpu(proto) == ETH_P_8021AD)
> -		is_double = true;
> +		is_svlan = true;
>  
>  	clear_bit(vid, priv->active_vlans);
> -	num_double_vlans = priv->num_double_vlans - is_double;
> -	ret = stmmac_vlan_update(priv, num_double_vlans);
> +	num_svlans = priv->num_svlans - is_svlan;
> +	ret = stmmac_vlan_update(priv, num_svlans);
>  	if (ret) {
>  		set_bit(vid, priv->active_vlans);
>  		goto del_vlan_error;
> @@ -6923,12 +6923,12 @@ 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, priv->num_svlans);
>  			goto del_vlan_error;
>  		}
>  	}
>  
> -	priv->num_double_vlans = num_double_vlans;
> +	priv->num_svlans = num_svlans;
>  
>  del_vlan_error:
>  	pm_runtime_put(priv->device);
> @@ -6944,7 +6944,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, priv->num_svlans);
>  }
>  
>  static int stmmac_bpf(struct net_device *dev, struct netdev_bpf *bpf)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> index c25dc9f89270..c485217ba880 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> @@ -243,7 +243,7 @@ struct stmmac_test_priv {
>  	int (*func)(struct sk_buff *skb, struct net_device *ndev,
>  		    struct packet_type *pt, struct net_device *orig_ndev);
>  	bool capture_all;
> -	int double_vlan;
> +	int svlan;
>  	int vlan_id;
>  	int ok;
>  };
> @@ -285,7 +285,7 @@ static int stmmac_test_loopback_validate(struct sk_buff *skb,
>  	}
>  
>  	ihdr = ip_hdr(skb);
> -	if (tpriv->double_vlan)
> +	if (tpriv->svlan)
>  		ihdr = (struct iphdr *)(skb_network_header(skb) + 4);
>  
>  	if (tpriv->packet->tcp) {
> @@ -936,7 +936,7 @@ static int stmmac_test_vlan_validate(struct sk_buff *skb,
>  	struct iphdr *ihdr;
>  	u16 proto;
>  
> -	proto = tpriv->double_vlan ? ETH_P_8021AD : ETH_P_8021Q;
> +	proto = tpriv->svlan ? ETH_P_8021AD : ETH_P_8021Q;
>  
>  	skb = skb_unshare(skb, GFP_ATOMIC);
>  	if (!skb)
> @@ -963,7 +963,7 @@ static int stmmac_test_vlan_validate(struct sk_buff *skb,
>  	}
>  
>  	ihdr = ip_hdr(skb);
> -	if (tpriv->double_vlan)
> +	if (tpriv->svlan)
>  		ihdr = (struct iphdr *)(skb_network_header(skb) + 4);
>  	if (ihdr->protocol != IPPROTO_UDP)
>  		goto out;
> @@ -1080,7 +1080,7 @@ static int stmmac_test_vlanfilt_perfect(struct stmmac_priv *priv)
>  	return ret;
>  }
>  
> -static int __stmmac_test_dvlanfilt(struct stmmac_priv *priv)
> +static int __stmmac_test_svlanfilt(struct stmmac_priv *priv)
>  {
>  	struct stmmac_packet_attrs attr = { };
>  	struct stmmac_test_priv *tpriv;
> @@ -1092,7 +1092,7 @@ static int __stmmac_test_dvlanfilt(struct stmmac_priv *priv)
>  		return -ENOMEM;
>  
>  	tpriv->ok = false;
> -	tpriv->double_vlan = true;
> +	tpriv->svlan = true;
>  	init_completion(&tpriv->comp);
>  
>  	tpriv->pt.type = htons(ETH_P_8021Q);
> @@ -1155,15 +1155,15 @@ static int __stmmac_test_dvlanfilt(struct stmmac_priv *priv)
>  	return ret;
>  }
>  
> -static int stmmac_test_dvlanfilt(struct stmmac_priv *priv)
> +static int stmmac_test_svlanfilt(struct stmmac_priv *priv)
>  {
>  	if (!priv->dma_cap.vlhash)
>  		return -EOPNOTSUPP;
>  
> -	return __stmmac_test_dvlanfilt(priv);
> +	return __stmmac_test_svlanfilt(priv);
>  }
>  
> -static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
> +static int stmmac_test_svlanfilt_perfect(struct stmmac_priv *priv)
>  {
>  	int ret, prev_cap = priv->dma_cap.vlhash;
>  
> @@ -1171,7 +1171,7 @@ static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
>  		return -EOPNOTSUPP;
>  
>  	priv->dma_cap.vlhash = 0;
> -	ret = __stmmac_test_dvlanfilt(priv);
> +	ret = __stmmac_test_svlanfilt(priv);
>  	priv->dma_cap.vlhash = prev_cap;
>  
>  	return ret;
> @@ -1372,7 +1372,7 @@ static int stmmac_test_vlanoff_common(struct stmmac_priv *priv, bool svlan)
>  	proto = svlan ? ETH_P_8021AD : ETH_P_8021Q;
>  
>  	tpriv->ok = false;
> -	tpriv->double_vlan = svlan;
> +	tpriv->svlan = svlan;
>  	init_completion(&tpriv->comp);
>  
>  	tpriv->pt.type = svlan ? htons(ETH_P_8021Q) : htons(ETH_P_IP);
> @@ -1960,11 +1960,11 @@ static const struct stmmac_test {
>  		.name = "VLAN Filtering (perf)      ",
>  		.fn = stmmac_test_vlanfilt_perfect,
>  	}, {
> -		.name = "Double VLAN Filter         ",
> -		.fn = stmmac_test_dvlanfilt,
> +		.name = "SVLAN Filtering            ",
> +		.fn = stmmac_test_svlanfilt,
>  	}, {
> -		.name = "Double VLAN Filter (perf)  ",
> -		.fn = stmmac_test_dvlanfilt_perfect,
> +		.name = "SVLAN Filtering (perf)     ",
> +		.fn = stmmac_test_svlanfilt_perfect,
>  	}, {
>  		.name = "Flexible RX Parser         ",
>  		.fn = stmmac_test_rxp,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
> index 200b34588c7f..fb9aad748cb3 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,
> -			     bool is_double)
> +			     bool is_svlan)
>  {
>  	void __iomem *ioaddr = hw->pcsr;
>  	u32 value;
> @@ -173,7 +173,7 @@ static void vlan_update_hash(struct mac_device_info *hw, u32 hash,
>  
>  	if (hash) {
>  		value |= VLAN_VTHM | VLAN_ETV;
> -		if (is_double) {
> +		if (is_svlan) {
>  			value |= VLAN_ESVL;
>  			value |= VLAN_DOVLTC;
>  		} else {
> @@ -235,7 +235,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,
> -				      bool is_double)
> +				      bool is_svlan)
>  {
>  	void __iomem *ioaddr = hw->pcsr;
>  
> @@ -251,7 +251,7 @@ 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) {
> +		if (is_svlan) {
>  			value |= VLAN_ESVL;
>  			value |= VLAN_DOVLTC;
>  		} else {

Re: [PATCH net v6 3/5] net: stmmac: Rename double VLAN references to svlan

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The self-test renamed to "SVLAN Filtering (perf)"
  (stmmac_test_svlanfilt_perfect), and likewise…

--

net: stmmac: Rename double VLAN references to svlan

This patch renames the "double VLAN" identifiers in the stmmac driver to
"svlan". It covers the VLAN hash callbacks, the num_double_vlans counter
and the selftests. The ESVL and DOVLTC bits control S-VLAN tag
processing, not the inner-tag double VLAN feature. No functional change
is intended.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> index c25dc9f89270..c485217ba880 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c

[ ... ]

> -static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
> +static int stmmac_test_svlanfilt_perfect(struct stmmac_priv *priv)
>  {
>  	int ret, prev_cap = priv->dma_cap.vlhash;
>  
> @@ -1171,7 +1171,7 @@ static int stmmac_test_dvlanfilt_perfect(struct stmmac_priv *priv)
>  		return -EOPNOTSUPP;
>  
>  	priv->dma_cap.vlhash = 0;
> -	ret = __stmmac_test_dvlanfilt(priv);
> +	ret = __stmmac_test_svlanfilt(priv);
>  	priv->dma_cap.vlhash = prev_cap;

[Severity: Low]
Does the "SVLAN Filtering (perf)" test still exercise perfect VLAN
filtering?

This test forces perfect-match mode by clearing priv->dma_cap.vlhash
before it calls __stmmac_test_svlanfilt(). An earlier patch in this
series, "net: stmmac: Remove VLAN perfect matching dead code", removed
the fallback in stmmac_vlan_update() that used to read vlhash:

    if (!priv->dma_cap.vlhash) {
        pmatch = vid;
        hash = 0;
    }

At this commit, the only reads of dma_cap.vlhash outside the selftests
are the probe-time feature check in __stmmac_dvr_probe() and the
debugfs dump. The filtering path never reads it:

__stmmac_test_svlanfilt()
  vlan_vid_add()
    stmmac_vlan_rx_add_vid()
      stmmac_vlan_update()
        stmmac_update_vlan_hash(priv, priv->hw, hash, is_svlan);

stmmac_vlan_update() always computes a non-zero hash for the active VID.
vlan_update_hash() and dwxgmac2_update_vlan_hash() then do:

    value |= VLAN_VTHM | VLAN_ETV;

so hash matching stays enabled throughout the "(perf)" run.

That seems to make stmmac_test_svlanfilt_perfect() a repeat of
stmmac_test_svlanfilt(). It could pass even if perfect filtering via
stmmac_add_hw_vlan_rx_fltr() were broken. The same applies to
stmmac_test_vlanfilt_perfect().

This patch renames the test but keeps it in this state, and the
remaining patches in the series do not touch the selftests. Should
these two "(perf)" tests be reworked or dropped?

>  
>  	return ret;

[ ... ]

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

[PATCH net v6 4/5] net: stmmac: Do not advertise S-VLAN stripping when it is disabled

[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>
---
v6 changes: None.

v5 changes:
- Rebased on top of the rename patch: used dwmac_is_xmac() instead of
  the dropped hw_svlan_en flag.

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 0d70eb452af7..b007ff120471 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6211,6 +6211,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 (dwmac_is_xmac(priv->plat->core_type)) {
+		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

Re: [PATCH net v6 4/5] net: stmmac: Do not advertise S-VLAN stripping when it is disabled

[Maxime Chevallier]

Hi,

On 9/28/26 22:34, Ovidiu Panait wrote:
> 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>

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

Maxime


> ---
> v6 changes: None.
> 
> v5 changes:
> - Rebased on top of the rename patch: used dwmac_is_xmac() instead of
>   the dropped hw_svlan_en flag.
> 
> 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 0d70eb452af7..b007ff120471 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6211,6 +6211,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 (dwmac_is_xmac(priv->plat->core_type)) {
> +		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;
>  }
>  

[PATCH net v6 5/5] net: stmmac: Disable S-Tag processing on dwmac4

[Ovidiu Panait]

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

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

Since dwmac4 cannot report the stripped tag type, it cannot support
hardware 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>
---
v6 changes: None.

v5 changes:
- Rebased on top of latest changes.
- Added Reviewed-by tag from Maxime.

v4 changes:
- Dropped all "double VLAN" references from the commit title/message.
- Rebased on top of the S-VLAN rework.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b007ff120471..5af245fd9ba0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6211,7 +6211,7 @@ 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 (dwmac_is_xmac(priv->plat->core_type)) {
+	if (priv->plat->core_type == DWMAC_CORE_XGMAC) {
 		if (features & NETIF_F_HW_VLAN_CTAG_RX)
 			features |= NETIF_F_HW_VLAN_STAG_RX;
 		else
@@ -7990,14 +7990,18 @@ 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;
+	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
+	if (priv->plat->core_type == DWMAC_CORE_XGMAC)
+		ndev->features |= NETIF_F_HW_VLAN_STAG_RX;
+
 	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

Re: [PATCH net v6 5/5] net: stmmac: Disable S-Tag processing on dwmac4

[Maxime Chevallier]

Hi,

On 9/28/26 22:34, 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>

I ran your proposed QinQ selftests on stm32mp157a (dwmac4), and it correctly
fixes the problem on that core.

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

Maxime

> ---
> v6 changes: None.
> 
> v5 changes:
> - Rebased on top of latest changes.
> - Added Reviewed-by tag from Maxime.
> 
> v4 changes:
> - Dropped all "double VLAN" references from the commit title/message.
> - Rebased on top of the S-VLAN rework.
> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b007ff120471..5af245fd9ba0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6211,7 +6211,7 @@ 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 (dwmac_is_xmac(priv->plat->core_type)) {
> +	if (priv->plat->core_type == DWMAC_CORE_XGMAC) {
>  		if (features & NETIF_F_HW_VLAN_CTAG_RX)
>  			features |= NETIF_F_HW_VLAN_STAG_RX;
>  		else
> @@ -7990,14 +7990,18 @@ 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;
> +	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
> +	if (priv->plat->core_type == DWMAC_CORE_XGMAC)
> +		ndev->features |= NETIF_F_HW_VLAN_STAG_RX;
> +
>  	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;

Re: [PATCH net v6 5/5] net: stmmac: Disable S-Tag processing on dwmac4

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete: XGMAC still passes hardware-stripped
  802.1ad tags up the stack as 802.1Q.
- [Low] After this patch, the two S-VLAN selftests check for different
  things.

--

net: stmmac: Disable S-Tag processing on dwmac4

This commit limits NETIF_F_HW_VLAN_STAG_RX and NETIF_F_HW_VLAN_STAG_FILTER
to DWMAC_CORE_XGMAC. The reason given is that dwmac4 cannot report which
tag type was stripped, and vlan_rx_hw() hardcodes ETH_P_8021Q. On dwmac4,
802.1ad tags are then left in the packet for 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 2ce6e2724bcb..b837e8e27a35 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -8005,14 +8005,18 @@ 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;
> +	ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
> +	if (priv->plat->core_type == DWMAC_CORE_XGMAC)
> +		ndev->features |= NETIF_F_HW_VLAN_STAG_RX;
> +
>  	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]
Won't XGMAC still pass hardware-stripped 802.1ad tags up the stack as
802.1Q after this change?

The commit message says that on dwxgmac2 "the outer tag type can be
determined based on that info". However, nothing in this patch or the rest
of the series actually does that.

dwxgmac210_vlan_ops still uses vlan_rx_hw() as .rx_hw_vlan, and that
function is unchanged:

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

dwxgmac2_wrback_get_rx_vlan_valid() accepts the ET_LT values with an S-tag
outer tag, but it returns only a bool, so the tag type is lost:

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

On XGMAC, hw_vlan_en is still set and STAG_RX/STAG_FILTER are still
advertised. stmmac_fix_features() also still ties STAG_RX to CTAG_RX.
Creating an 802.1ad VLAN on an XGMAC with vlhash then leads to:

stmmac_vlan_rx_add_vid(is_svlan = true)
  dwxgmac2_update_vlan_hash()    /* sets VLAN_ESVL */

and on receive:

stmmac_rx()
  stmmac_rx_hw_vlan()
    vlan_rx_hw()                 /* tags the skb as ETH_P_8021Q */

vlan_do_receive() then looks up (ETH_P_8021Q, vid), so the 802.1ad VLAN
device never gets the frame. Isn't this the same QinQ ping failure the
commit message describes?

Also, if an 802.1Q VLAN with the same VID exists, would S-tagged frames
from the wire be delivered to that interface instead?

Should vlan_rx_hw() take the protocol from ET_LT on XGMAC? Or should S-tag
offload be turned off on XGMAC as well?

>  	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]
On dwmac4, vlhash no longer implies NETIF_F_HW_VLAN_STAG_FILTER. Should
stmmac_test_svlanfilt() in stmmac_selftests.c be updated to match? It
still checks only:

	if (!priv->dma_cap.vlhash)
		return -EOPNOTSUPP;

Its sibling, stmmac_test_svlanfilt_perfect(), checks the feature flag:

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

On a dwmac4 with vlhash, __stmmac_test_svlanfilt() calls
vlan_vid_add(priv->dev, htons(ETH_P_8021AD), tpriv->vlan_id). Without
STAG_FILTER, vlan_hw_filter_capable() returns false for 802.1ad. As a
result, stmmac_vlan_rx_add_vid() is never called and no S-VLAN filter is
programmed.

The test still runs, and for i > 0 the loop treats a missing tpriv->ok as
success:

		ret = tpriv->ok ? 0 : -ETIMEDOUT;
		if (ret && !i) {
			goto vlan_del;
		} else if (!ret && i) {
			ret = -EINVAL;
			goto vlan_del;
		} else {
			ret = 0;
		}

Can "SVLAN Filtering" then report a pass on hardware that no longer uses
S-VLAN filtering, while the perfect variant is skipped?

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