* [PATCH net v8 0/2] net: stmmac: remove software VLAN tag stripping
@ 2026-09-19 12:54 peilinhe2020
2026-09-19 12:54 ` [PATCH net v8 1/2] net: stmmac: selftests: Prevent RXC from being disabled in LPI peilinhe2020
2026-09-19 12:54 ` [PATCH net v8 2/2] net: stmmac: remove software VLAN tag stripping peilinhe2020
0 siblings, 2 replies; 4+ messages in thread
From: peilinhe2020 @ 2026-09-19 12:54 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Peilin He
From: Peilin He <he.peilin@zte.com.cn>
This series removes the driver-level software VLAN stripping in stmmac.
Patch 1 is a prerequisite selftests fix from Maxime Chevallier to keep
the ethtool selftests working on platforms where NETIF_F_VLAN_FEATURES
may be empty after patch 2.
Patch 2 removes stmmac_rx_vlan() and the related VLAN RX feature
advertisement.
Changes in v8:
- Patch 1: fix inverted true/false description in commit log and mention
CONFIG_VLAN_8021Q=n builds.
- Patch 2: drop Fixes tag, fix trailers, document user-visible feature
changes and EEE side effect, tolerate in-payload VLAN tag in
stmmac_has_ip_ethertype(), and gate NETIF_F_HW_VLAN_CTAG_RX on the
presence of VLAN ops callbacks.
Maxime Chevallier (1):
net: stmmac: selftests: Prevent RXC from being disabled in LPI
Peilin He (1):
net: stmmac: remove software VLAN tag stripping
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 38 +++++--------------
.../stmicro/stmmac/stmmac_selftests.c | 2 +
2 files changed, 12 insertions(+), 28 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net v8 1/2] net: stmmac: selftests: Prevent RXC from being disabled in LPI
2026-09-19 12:54 [PATCH net v8 0/2] net: stmmac: remove software VLAN tag stripping peilinhe2020
@ 2026-09-19 12:54 ` peilinhe2020
2026-09-19 12:54 ` [PATCH net v8 2/2] net: stmmac: remove software VLAN tag stripping peilinhe2020
1 sibling, 0 replies; 4+ messages in thread
From: peilinhe2020 @ 2026-09-19 12:54 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Peilin He
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
RXC is critical to the RX path and some register banks in stmmac, so
it is blocked from being disabled in LPI when we know we're going
to need that clock.
During selftests, we're in MAC loopback so the PHY may enter LPI and
stop its RXC. This will prevent the RX path from properly receiving
the looped-back packets, hence failing the selftests.
As NETIF_F_VLAN_FEATURES may be empty on some platforms once the
software VLAN stripping is removed, the phylink config attribute
'eee_rx_clk_stop_enable' may no longer be unconditionally set to
false, hence allowing RXC to be disabled during LPI on platforms
that don't have any VLAN features. This is already the case for
CONFIG_VLAN_8021Q=n builds.
Fixes: 091810dbded9 ("net: stmmac: Introduce selftests support")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Peilin He <he.peilin@zte.com.cn>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index 664d7cfde2f1..4f4d61a9c098 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -1847,12 +1847,14 @@ void stmmac_selftest_run(struct net_device *dev,
return;
}
+ phylink_rx_clk_stop_block(priv->phylink);
for (i = 0; i < count; i++) {
ret = stmmac_selftests[i].fn(priv);
if (ret && (ret != -EOPNOTSUPP))
etest->flags |= ETH_TEST_FL_FAILED;
buf[i] = ret;
}
+ phylink_rx_clk_stop_unblock(priv->phylink);
stmmac_set_mac_loopback(priv, priv->ioaddr, false);
}
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net v8 2/2] net: stmmac: remove software VLAN tag stripping
2026-09-19 12:54 [PATCH net v8 0/2] net: stmmac: remove software VLAN tag stripping peilinhe2020
2026-09-19 12:54 ` [PATCH net v8 1/2] net: stmmac: selftests: Prevent RXC from being disabled in LPI peilinhe2020
@ 2026-09-19 12:54 ` peilinhe2020
2026-09-23 13:18 ` netdev-bot+sashiko
1 sibling, 1 reply; 4+ messages in thread
From: peilinhe2020 @ 2026-09-19 12:54 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Peilin He,
xu xin, Jiang Kun
From: Peilin He <he.peilin@zte.com.cn>
The software VLAN stripping logic in stmmac_rx_vlan() was originally
introduced in 2014 by commit b93819854d6e ("stmmac: Add vlan rx for
better GRO performance.") as a workaround to improve GRO performance,
since at that time GRO could not handle frames with VLAN tags. However,
this limitation was resolved in 2015 by commit 66e5133f19e9 ("vlan: Add
GRO support for non hardware accelerated vlan"), which added GRO support
for non-hardware-accelerated VLAN frames. Keeping a software fallback
path for VLAN stripping is no longer necessary and only adds complexity.
After removing it, VLAN tags remain in the packet. Adjust
stmmac_has_ip_ethertype() to tolerate a single in-payload VLAN tag so
that the MAC RX checksum offload result is still used for tagged
IPv4/IPv6 frames on cores without MAC-level stripping.
This intentionally changes the advertised features:
- dwmac100/dwmac1000 no longer advertise NETIF_F_HW_VLAN_CTAG_RX;
they have no hardware VLAN stripping.
- NETIF_F_HW_VLAN_STAG_RX is no longer advertised, as the hardware
reports stripped S-tags as C-tags.
- On xmac cores NETIF_F_HW_VLAN_CTAG_RX is advertised only when the
VLAN ops provide .set_hw_vlan_mode and .rx_hw_vlan, so DWXLGMAC no
longer falsely advertises RX VLAN offload.
With NETIF_F_VLAN_FEATURES possibly empty, EEE RX clock stop may be
enabled on some platforms; the preceding selftests patch blocks RX
clock stop while the ethtool selftests run.
This depends on the preceding selftests fix.
Suggested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Peilin He <he.peilin@zte.com.cn>
Reviewed-by: xu xin <xu.xin16@zte.com.cn>
Reviewed-by: Jiang Kun <jiang.kun2@zte.com.cn>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 38 +++++--------------
1 file changed, 10 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9c9e95d66b09..6d4716b1a7ad 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4752,7 +4752,11 @@ static bool stmmac_has_ip_ethertype(struct sk_buff *skb)
proto = __vlan_get_protocol(skb, eth_header_parse_protocol(skb),
&depth);
- return (depth <= ETH_HLEN) &&
+ /* Software VLAN stripping is gone, so a single VLAN tag may
+ * still be present. The MAC RX checksum result still applies
+ * to the inner IP packet.
+ */
+ return (depth <= VLAN_ETH_HLEN) &&
(proto == htons(ETH_P_IP) || proto == htons(ETH_P_IPV6));
}
@@ -5032,24 +5036,6 @@ static netdev_features_t stmmac_features_check(struct sk_buff *skb,
return vlan_features_check(skb, features);
}
-static void stmmac_rx_vlan(struct net_device *dev, struct sk_buff *skb)
-{
- struct vlan_ethhdr *veth = skb_vlan_eth_hdr(skb);
- __be16 vlan_proto = veth->h_vlan_proto;
- u16 vlanid;
-
- 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)) {
- /* pop the vlan tag */
- vlanid = ntohs(veth->h_vlan_TCI);
- memmove(skb->data + VLAN_HLEN, veth, ETH_ALEN * 2);
- skb_pull(skb, VLAN_HLEN);
- __vlan_hwaccel_put_tag(skb, vlan_proto, vlanid);
- }
-}
-
/**
* stmmac_rx_refill - refill used skb preallocated buffers
* @priv: driver private structure
@@ -5418,9 +5404,7 @@ static void stmmac_dispatch_skb_zc(struct stmmac_priv *priv, u32 queue,
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);
+
skb->protocol = eth_type_trans(skb, priv->dev);
if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb))
@@ -5912,9 +5896,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
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);
skb->protocol = eth_type_trans(skb, priv->dev);
@@ -7995,9 +7976,10 @@ static int __stmmac_dvr_probe(struct device *device,
ndev->features |= ndev->hw_features | NETIF_F_HIGHDMA;
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;
- if (dwmac_is_xmac(priv->plat->core_type)) {
+ if (priv->hw->vlan &&
+ priv->hw->vlan->set_hw_vlan_mode &&
+ priv->hw->vlan->rx_hw_vlan) {
+ ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
priv->hw->hw_vlan_en = true;
}
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v8 2/2] net: stmmac: remove software VLAN tag stripping
2026-09-19 12:54 ` [PATCH net v8 2/2] net: stmmac: remove software VLAN tag stripping peilinhe2020
@ 2026-09-23 13:18 ` netdev-bot+sashiko
0 siblings, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 13:18 UTC (permalink / raw)
To: peilinhe2020
Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, alexandre.torgue, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, he.peilin
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 3 · Medium: 0 (1) · Low: 1 (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] stmmac_has_ip_ethertype() parses the VLAN chain at the wrong
offset on RX, so widening the bound from `depth <= ETH_HLEN` to `depth…
- [High] The same helper is the TX gate: stmmac_features_check()
(ndo_features_check) clears NETIF_F_IP_CSUM/NETIF_F_IPV6_CSUM when…
- [High] The widened `depth <= VLAN_ETH_HLEN` bound removes the last
software guard that kept the driver from claiming CHECKSUM_UNNECESSARY…
- [Low] The patch silently fixes a live, user-visible feature-
advertisement bug on DWXLGMAC but carries no Fixes: tag.
Pre-existing issues:
- [Medium] Removing NETIF_F_HW_VLAN_STAG_RX from the advertised features
does not stop the MAC from stripping S-tags and reporting them as…
- [Low] priv->hw->hw_vlan_en is a plain bool flipped from process context
in stmmac_set_features() (under rtnl) while it is read concurrently by…
--
net: stmmac: remove software VLAN tag stripping
This removes stmmac_rx_vlan() and its two call sites, relaxes the depth
check in stmmac_has_ip_ethertype() so a single in-payload VLAN tag is
tolerated, and changes the RX VLAN feature advertisement in
__stmmac_dvr_probe() to depend on the presence of .set_hw_vlan_mode and
.rx_hw_vlan.
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 1fb5f804ea23..232b1e7e031c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4769,7 +4769,11 @@ static bool stmmac_has_ip_ethertype(struct sk_buff *skb)
> proto = __vlan_get_protocol(skb, eth_header_parse_protocol(skb),
> &depth);
>
> - return (depth <= ETH_HLEN) &&
> + /* Software VLAN stripping is gone, so a single VLAN tag may
> + * still be present. The MAC RX checksum result still applies
> + * to the inner IP packet.
> + */
> + return (depth <= VLAN_ETH_HLEN) &&
> (proto == htons(ETH_P_IP) || proto == htons(ETH_P_IPV6));
> }
[Severity: High]
On the RX side, is the VLAN chain parsed at the right offset here? Both
RX callers advance skb->data before calling this helper:
skb->protocol = eth_type_trans(skb, priv->dev);
if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb) ||
(status & csum_none))
eth_type_trans() calls eth_skb_pull_mac(), which is just:
static inline struct ethhdr *eth_skb_pull_mac(struct sk_buff *skb)
{
struct ethhdr *eth = (struct ethhdr *)skb->data;
skb_pull_inline(skb, ETH_HLEN);
return eth;
}
and it never assigns skb->mac_len. The skb is built with
napi_build_skb()/napi_alloc_skb() so mac_len is still 0 at this point;
skb_reset_mac_len() only runs later in __netif_receive_skb_core(), after
napi_gro_receive().
__vlan_get_protocol() passes mac_offset 0, and the walk then does:
net/core/skbuff.c:__vlan_get_protocol_offset() {
unsigned int vlan_depth = skb->mac_len, parse_depth = VLAN_MAX_DEPTH;
if (vlan_depth) {
...
vlan_depth -= VLAN_HLEN;
} else {
vlan_depth = ETH_HLEN;
}
vh = skb_header_pointer(skb, mac_offset + vlan_depth,
sizeof(vhdr), &vhdr);
...
}
With mac_len == 0 the first VLAN header is expected at skb->data +
ETH_HLEN, but after the pull the real tag sits at skb->data + 0, so the
read lands 14 bytes too far. For a tagged IPv4 frame
h_vlan_encapsulated_proto is then read from IP header bytes 12..13, i.e.
the first two bytes of the source address; for IPv6 it is source address
bytes 4..5, and for a tagged ARP frame it is sender-MAC bytes 4..5.
That leads to two effects. First, the ordinary tagged IPv4/IPv6 frame
fails the proto comparison, so the MAC RX checksum result is still
discarded and dwmac100/dwmac1000 lose CHECKSUM_UNNECESSARY for the VLAN
traffic that previously got it via stmmac_rx_vlan() with depth 0. Does
that contradict the commit message claim:
"Adjust stmmac_has_ip_ethertype() to tolerate a single in-payload
VLAN tag so that the MAC RX checksum offload result is still used
for tagged IPv4/IPv6 frames on cores without MAC-level stripping."
Second, when those mis-read bytes do happen to equal 0x0800 or 0x86DD
(for instance a tagged frame with a source address in 8.0.0.0/16, which
the sender chooses), depth becomes 18 and the new bound accepts it, so
CHECKSUM_UNNECESSARY is set for frames whose real EtherType is not IP.
The old depth <= ETH_HLEN bound could not reach that state because a
VLAN walk always yields 18.
Would it work to run the helper before eth_type_trans(), or to pass an
explicit mac_offset that accounts for the ETH_HLEN pull?
> @@ -5049,24 +5053,6 @@ static netdev_features_t stmmac_features_check(struct sk_buff *skb,
> return vlan_features_check(skb, features);
> }
[Severity: High]
stmmac_has_ip_ethertype() is also the TX gate in stmmac_features_check():
if (priv->plat->tx_queues_cfg[queue].coe_unsupported ||
!stmmac_has_ip_ethertype(skb))
features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
Unlike the RX side, the parse offset is correct there, because
__dev_queue_xmit() has done skb_reset_mac_header() and mac_len is either
0 or 18, so depth 18 is genuinely accepted after this change. That
re-enables hardware checksum (and therefore TSO, via
harmonize_features()) for frames that already carry one VLAN tag in the
payload when ndo_features_check runs: DSA tag_8021q frames such as
sja1105 in VLAN-aware mode, and bridged/forwarded tagged
CHECKSUM_PARTIAL frames from tap/vhost guests. vlan_features_check()
only masks multi-tagged frames, so a single in-payload tag passes
through.
Isn't that the class of frames that commit c2945c435c99 ("net: stmmac:
Prevent DSA tags from breaking COE") excluded with depth <= ETH_HLEN, on
the grounds that "on TX these tagged frames are ignored by the checksum
offload engine and IP header checker of some stmmac cores"? If the COE
does not skip the tag, the descriptor CIC request is ignored and the
frame goes out with the pseudo-header sum still in the L4 checksum
field, and there is no TX-side fallback.
Was this TX effect intended? The changelog only describes the RX
rationale, and c2945c435c99 carried a Fixes: tag and went to stable.
>
> -static void stmmac_rx_vlan(struct net_device *dev, struct sk_buff *skb)
> -{
> - struct vlan_ethhdr *veth = skb_vlan_eth_hdr(skb);
> - __be16 vlan_proto = veth->h_vlan_proto;
> - u16 vlanid;
> -
> - 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)) {
> - /* pop the vlan tag */
> - vlanid = ntohs(veth->h_vlan_TCI);
> - memmove(skb->data + VLAN_HLEN, veth, ETH_ALEN * 2);
> - skb_pull(skb, VLAN_HLEN);
> - __vlan_hwaccel_put_tag(skb, vlan_proto, vlanid);
> - }
> -}
> -
> /**
> * stmmac_rx_refill - refill used skb preallocated buffers
> * @priv: driver private structure
> @@ -5435,9 +5421,7 @@ static void stmmac_dispatch_skb_zc(struct stmmac_priv *priv, u32 queue,
> 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);
> +
> skb->protocol = eth_type_trans(skb, priv->dev);
>
> if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb))
[Severity: High]
Can the relaxed bound make the driver claim CHECKSUM_UNNECESSARY for
frames the COE never validated? In this zero-copy path the descriptor
status is not consulted at all:
if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb))
skb_checksum_none_assert(skb);
else
skb->ip_summed = CHECKSUM_UNNECESSARY;
and in stmmac_rx() the extra status term cannot express "not checked":
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:dwmac4_wrback_get_rx_status() {
if (rdes1 & RDES1_IP_HDR_ERROR) { x->ip_hdr_err++; ret |= csum_none; }
if (rdes1 & RDES1_IP_CSUM_BYPASSED)
x->ip_csum_bypassed++;
...
if (rdes1 & RDES1_IP_PAYLOAD_ERROR) { x->ip_payload_err++; ret |= csum_none; }
}
RDES1_IP_CSUM_BYPASSED is only counted, never mapped to csum_none, so
status & csum_none stays clear for a frame the engine skipped. Before
this change, any frame whose payload still began with a VLAN header gave
depth 18 > ETH_HLEN and was downgraded to CHECKSUM_NONE; now depth 18 is
accepted. Cases where the COE legitimately bypasses include an
802.1ad/QinQ frame whose inner tag remains after the outer one is
stripped, and IP fragments.
Combined with the offset question above, the accept on RX is reached via
the mis-read EtherType, i.e. from payload bytes the sender picks, so a
corrupt L4 checksum can be handed to the stack as verified. Should the
descriptor bypass bit be consulted before setting
CHECKSUM_UNNECESSARY, at least in the zero-copy path?
> @@ -5929,9 +5913,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
> 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);
>
> skb->protocol = eth_type_trans(skb, priv->dev);
>
[Severity: Low]
This is a pre-existing issue and not introduced by this patch, but after
the else branch is gone, priv->hw->hw_vlan_en becomes the only gate for
recovering the VLAN ID from the descriptor. The flag is a plain bool
written from process context in stmmac_set_features():
if (features & NETIF_F_HW_VLAN_CTAG_RX)
priv->hw->hw_vlan_en = true;
else
priv->hw->hw_vlan_en = false;
phylink_rx_clk_stop_block(priv->phylink);
stmmac_set_hw_vlan_mode(priv, priv->hw);
and read from softirq context in stmmac_rx() and
stmmac_dispatch_skb_zc(), with no READ_ONCE/WRITE_ONCE, no barrier and
no NAPI quiescing between the flag update and the MAC strip-mode
register write. On the disable transition the flag is cleared first, so
frames whose tag the MAC already stripped are delivered with
stmmac_rx_hw_vlan() skipped and the VLAN ID lost. Is it worth making
the flag update and the register write ordered while this code is being
touched?
> @@ -7990,9 +7971,10 @@ static int __stmmac_dvr_probe(struct device *device,
> ndev->features |= ndev->hw_features | NETIF_F_HIGHDMA;
> 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;
> - if (dwmac_is_xmac(priv->plat->core_type)) {
> + if (priv->hw->vlan &&
> + priv->hw->vlan->set_hw_vlan_mode &&
> + priv->hw->vlan->rx_hw_vlan) {
> + ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
> ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
> priv->hw->hw_vlan_en = true;
> }
[Severity: Low]
This ops-presence check also fixes a live advertisement bug on DWXLGMAC.
The ops table there provides neither callback:
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,
};
so with the old dwmac_is_xmac() test that core advertised
rx-vlan-offload and set hw_vlan_en while rx_hw_vlan was a no-op and the
MAC was never programmed to strip. That is the same problem
8eb301bd7b0f4 ("net: stmmac: enable HW-accelerated VLAN stripping for
gmac4 only") addressed for gmac/xgmac, and 534df0c1724b7 ("net: stmmac:
dwxgmac2: Add support for HW-accelerated VLAN stripping") reintroduced
for XLGMAC by widening the condition to all xmac cores. Should this
carry a Fixes: 534df0c1724b7 tag, or be split into its own patch so it
can be backported separately from the cleanup?
[Severity: Medium]
This is a pre-existing issue rather than something this patch
introduces, but does dropping NETIF_F_HW_VLAN_STAG_RX actually change
what the hardware does? hw_vlan_en is still set here, and
stmmac_set_features() derives it from the single C-tag bit, after which:
drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c:vlan_set_hw_mode() {
if (hw->hw_vlan_en)
/* Always strip VLAN on Receive */
value |= VLAN_TAG_STRIP_ALL;
...
}
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);
}
}
Once an 802.1ad VLAN is configured, vlan_update_hash() sets VLAN_ESVL /
VLAN_EDVLP / VLAN_DOVLTC, so an outer S-tag is recognised, stripped by
VLAN_TAG_STRIP_ALL and then reported as ETH_P_8021Q on an interface that
now declares S-tag RX offload unsupported. The S-VID can then land on an
802.1Q upper device with the same VID. Since the commit message gives
this as a motivation:
"NETIF_F_HW_VLAN_STAG_RX is no longer advertised, as the hardware
reports stripped S-tags as C-tags."
would it be clearer to state that the strip/report behaviour itself is
unchanged and only the feature bit is removed? The driver selftest
validator still expects ETH_P_8021AD metadata for an outer S-tag.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260919125413.18058-1-peilinhe2020%40163.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-23 13:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-19 12:54 [PATCH net v8 0/2] net: stmmac: remove software VLAN tag stripping peilinhe2020
2026-09-19 12:54 ` [PATCH net v8 1/2] net: stmmac: selftests: Prevent RXC from being disabled in LPI peilinhe2020
2026-09-19 12:54 ` [PATCH net v8 2/2] net: stmmac: remove software VLAN tag stripping peilinhe2020
2026-09-23 13:18 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®