[PATCH net v2 0/2] net: dsa: mt7530: fix trapped frame forwarding and egress tagging

[PATCH net v2 0/2] net: dsa: mt7530: fix trapped frame forwarding and egress tagging

[Tao Gong]

This series resolves two issues in the MT7530 DSA switch driver preventing
trapped link-local control frames (such as STP BPDUs) from reaching the CPU
host when bridge VLAN filtering is enabled:

1. Commit 3ac85bcfd404 ("net: dsa: mt7530: preserve VLAN tags on trapped
   link-local frames") set MT7530_VLAN_EG_DISABLED to preserve VLAN tags,
   but in hardware this disables the per-packet override and causes
   trapped frames to follow the CPU port's active VLAN table (VAWD2) rule.
   When bridge VLAN filtering is active, the CPU port is in stack mode,
   causing incoming untagged BPDUs (01:80:C2:00:00:00) to be egressed with
   a VLAN tag, which userspace STP daemons (such as ustpd) and the kernel
   bridge drop. MT7530_VLAN_EG_CONSISTENT correctly preserves wire state
   (untagged frames stay untagged, tagged frames stay tagged).

2. On Airoha EN7581 and AN7583 switch IP, setting MT7531_CFC (0x04)
   CPU_PMAP alone is insufficient; the switch MAC forwarding control
   (MT753X_MFC, 0x10) must also have CPU_EN and CPU_PORT configured for
   trapped frames to exit to the CPU conduit.

Changes in v2:
  - Rebased onto net/main
    (git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git) adapting
    to mt7530_rmw() instead of net-next's regmap helpers, resolving the
    patch application failure on net-0 noted by Jakub Kicinski.
  - Carried over Lorenzo Bianconi's Acked-by on patch 2 as the underlying
    logic is unchanged.

Tao Gong (2):
  net: dsa: mt7530: use consistent egress tag for trapped link-local
    frames
  net: dsa: mt7530: trap frames to conduit in MFC on EN7581 and AN7583

 drivers/net/dsa/mt7530.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)


base-commit: 9c572a83037a7dcd653ba3a9cc468c16b857d0c9
-- 
2.55.0

[PATCH net v2 1/2] net: dsa: mt7530: use consistent egress tag for trapped link-local frames

[Tao Gong]

commit 3ac85bcfd404 ("net: dsa: mt7530: preserve VLAN tags on trapped
link-local frames") changed the egress VLAN tag attribute for trapped
link-local frames in BPC, RGAC1, and RGAC2 from MT7530_VLAN_EG_UNTAGGED
to MT7530_VLAN_EG_DISABLED, with the intention of passing frames
through without VLAN tag modification.

However, in MT7530 hardware, MT7530_VLAN_EG_DISABLED disables the
protocol-level egress tagging override, causing trapped frames to
fall back to the egress port's active VLAN table (VAWD2) rule. When a
bridge with VLAN filtering is active, the CPU port is configured with
stack mode, causing the switch to insert a VLAN tag into incoming
untagged BPDUs (01:80:C2:00:00:00) and other link-local frames before
delivering them to the CPU conduit.

Because BPDUs arrive at the CPU with a VLAN tag (EtherType 0x8100
at offset 12 instead of 802.3 length <= 1500), userspace STP daemons
(such as ustpd) and the kernel bridge drop or ignore them, breaking
STP convergence.

Use MT7530_VLAN_EG_CONSISTENT instead. This instructs the hardware to
keep egress tagging consistent with the ingress wire format: frames
received untagged on the wire remain untagged when forwarded to the
CPU, while VLAN-tagged frames preserve their original VLAN tag.

Fixes: 3ac85bcfd404 ("net: dsa: mt7530: preserve VLAN tags on trapped link-local frames")
Signed-off-by: Tao Gong <gongtao0607@gmail.com>
---
 drivers/net/dsa/mt7530.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 3e61eb3c2b1e..20bb07fac0f7 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1309,39 +1309,39 @@ static void
 mt753x_trap_frames(struct mt7530_priv *priv)
 {
 	/* Trap 802.1X PAE frames and BPDUs to the CPU port(s) and egress
-	 * them with the EG_TAG attribute set to disabled (system default)
-	 * so that any VLAN tags in the frame are not modified by the
-	 * switch egress VLAN tag processing. This preserves VLAN tags
-	 * for reception on VLAN sub-interfaces.
+	 * them with the EG_TAG attribute set to consistent so that any
+	 * VLAN tags in the frame are not modified by the switch egress
+	 * VLAN tag processing. This preserves VLAN tags for reception
+	 * on VLAN sub-interfaces.
 	 */
 	mt7530_rmw(priv, MT753X_BPC,
 		   PAE_BPDU_FR | PAE_EG_TAG_MASK | PAE_PORT_FW_MASK |
 			   BPDU_EG_TAG_MASK | BPDU_PORT_FW_MASK,
-		   PAE_BPDU_FR | PAE_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+		   PAE_BPDU_FR | PAE_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
 			   PAE_PORT_FW(TO_CPU_FW_CPU_ONLY) |
-			   BPDU_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+			   BPDU_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
 			   TO_CPU_FW_CPU_ONLY);
 
 	/* Trap frames with :01 and :02 MAC DAs to the CPU port(s) and
-	 * egress them with EG_TAG disabled.
+	 * egress them with EG_TAG consistent.
 	 */
 	mt7530_rmw(priv, MT753X_RGAC1,
 		   R02_BPDU_FR | R02_EG_TAG_MASK | R02_PORT_FW_MASK |
 			   R01_BPDU_FR | R01_EG_TAG_MASK | R01_PORT_FW_MASK,
-		   R02_BPDU_FR | R02_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+		   R02_BPDU_FR | R02_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
 			   R02_PORT_FW(TO_CPU_FW_CPU_ONLY) | R01_BPDU_FR |
-			   R01_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+			   R01_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
 			   TO_CPU_FW_CPU_ONLY);
 
 	/* Trap frames with :03 and :0E MAC DAs to the CPU port(s) and
-	 * egress them with EG_TAG disabled.
+	 * egress them with EG_TAG consistent.
 	 */
 	mt7530_rmw(priv, MT753X_RGAC2,
 		   R0E_BPDU_FR | R0E_EG_TAG_MASK | R0E_PORT_FW_MASK |
 			   R03_BPDU_FR | R03_EG_TAG_MASK | R03_PORT_FW_MASK,
-		   R0E_BPDU_FR | R0E_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+		   R0E_BPDU_FR | R0E_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
 			   R0E_PORT_FW(TO_CPU_FW_CPU_ONLY) | R03_BPDU_FR |
-			   R03_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+			   R03_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
 			   TO_CPU_FW_CPU_ONLY);
 }
 
-- 
2.55.0

Re: [PATCH net v2 1/2] net: dsa: mt7530: use consistent egress tag for trapped link-local frames

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · 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 commit message's justification is contradicted by code
  already in the tree.
- [Medium] Setting BPC, RGAC1 and RGAC2 EG_TAG to
  MT7530_VLAN_EG_CONSISTENT in mt753x_trap_frames() probably breaks…

--

net: dsa: mt7530: use consistent egress tag for trapped link-local frames

This commit changes the EG_TAG attribute for trapped link-local frames in
the BPC, RGAC1 and RGAC2 registers from MT7530_VLAN_EG_DISABLED to
MT7530_VLAN_EG_CONSISTENT. The goal is to keep untagged BPDUs from
reaching the CPU with the PVID tag stacked on when a VLAN-aware bridge is
in use, while still preserving the VLAN tags of frames that arrive tagged.

> Because BPDUs arrive at the CPU with a VLAN tag (EtherType 0x8100
> at offset 12 instead of 802.3 length <= 1500), userspace STP daemons
> (such as ustpd) and the kernel bridge drop or ignore them, breaking
> STP convergence.

[Severity: Medium]

Can this failure happen on the current tree? Both mt7530_setup() and
mt7531_setup_common() already set:

	ds->untag_vlan_aware_bridge_pvid = true;

That line came from commit 4cb3cd670b2a ("net: dsa: mt7530: untag
VLAN-aware bridge PVID"). It was part of the same series as 3ac85bcfd404
and handles this same hardware behaviour.

With the flag set, every received frame goes through the software untag
path, including trapped link-local frames:

dsa_switch_rcv()
  dsa_software_vlan_untag()
    dsa_software_untag_vlan_aware_bridge()

The PVID tag is then cleared here:

net/dsa/tag.h:dsa_software_untag_vlan_aware_bridge() {
    ...
	if (vid == pvid && skb->vlan_proto == htons(proto))
		__vlan_hwaccel_clear_tag(skb);
}

The kernel-doc for that helper says it exists because "Software like
STP/PTP may not" handle tagged packets.

The VID that the switch inserts is the bridge port PVID.
mt7530_port_vlan_add() stores vlan->vid in priv->ports[port].pvid, and
mt7530_port_set_vlan_aware() programs G0_PORT_VID(priv->ports[port].pvid).
An untagged BPDU stacked with the PVID tag should therefore reach
br_handle_frame(), and packet sockets on the user port, untagged.

Which condition stops this untagging for trapped frames? If the breakage
was seen on a backport that has 3ac85bcfd404 but not 4cb3cd670b2a, or with
an 802.1ad bridge, could the commit message say so? As written, it isn't
clear whether the STP breakage exists on mainline at all, and that affects
the Fixes: tag and stable backports.

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 3e61eb3c2b1e2..20bb07fac0f7b 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1309,39 +1309,39 @@ static void
>  mt753x_trap_frames(struct mt7530_priv *priv)
>  {

[ ... ]

>  	mt7530_rmw(priv, MT753X_BPC,
>  		   PAE_BPDU_FR | PAE_EG_TAG_MASK | PAE_PORT_FW_MASK |
>  			   BPDU_EG_TAG_MASK | BPDU_PORT_FW_MASK,
> -		   PAE_BPDU_FR | PAE_EG_TAG(MT7530_VLAN_EG_DISABLED) |
> +		   PAE_BPDU_FR | PAE_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
>  			   PAE_PORT_FW(TO_CPU_FW_CPU_ONLY) |
> -			   BPDU_EG_TAG(MT7530_VLAN_EG_DISABLED) |
> +			   BPDU_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
>  			   TO_CPU_FW_CPU_ONLY);

[Severity: Medium]

Does this bring back the problem that 3ac85bcfd404 fixed? That problem
was VLAN-tagged link-local frames (STP, LLDP, PTP, as exercised by the
local_termination selftest) arriving on VLAN-aware user ports.

Commit e8bf353577f3 ("net: dsa: mt7530: fix link-local frames that ingress
vlan filtering ports") says the per-frame EG_TAG in BPC, RGAC1 and RGAC2
takes priority over the VLAN table. It describes CONSISTENT like this:

    CONSISTENT means frames egress exactly as they ingress. [...]
    Although link-local frames are not supposed to be transmitted
    VLAN-tagged, if they are done so, when egressing through a CPU
    port, the special tag field will be broken.

Its captures show that stack mode keeps a separate 802.1Q header after
the special tag. Tagged mode instead merges the TCI into the special tag,
with no 0x8100 TPID:

    stacked: 0001 0000 8100 0001
    tagged:  0001 0001 0026 4242

For these frames, CONSISTENT now overrides the stack rule that
mt7530_hw_vlan_add() sets for the CPU port:

	val = MT7530_VLAN_EGRESS_STACK;

mt7530_port_set_vlan_aware() configures VLAN-aware user ports with
VLAN_ATTR(MT7530_VLAN_USER), so the switch parses their ingress tags. A
tagged BPDU, LLDP or PTP frame received on such a port would then leave
the CPU port in the merged tagged format.

mtk_tag_rcv() always strips MTK_HDR_LEN bytes as a separate special tag:

	skb_pull_rcsum(skb, MTK_HDR_LEN);

	dsa_strip_etype_header(skb, MTK_HDR_LEN);

Wouldn't that discard the VLAN TCI and deliver the frame to the base user
netdev instead of the VLAN upper?

The commit message says "VLAN-tagged frames preserve their original VLAN
tag". Is there a capture or datasheet reference showing that CONSISTENT
gives the stacked format at the CPU port, unlike what e8bf353577f3
observed?

The next patch in the series, "net: dsa: mt7530: trap frames to conduit in
MFC on EN7581 and AN7583", does not touch mt753x_trap_frames(). If this is
a problem, it is still there at the end of the series.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923055103.949540-1-gongtao0607%40gmail.com

[PATCH net v2 2/2] net: dsa: mt7530: trap frames to conduit in MFC on EN7581 and AN7583

[Tao Gong]

On MT7531 and MT7988 switches, trapped frames forwarding is directed to
CPU ports specified in the MT7531_CFC (0x04) CPU_PMAP bitmap. When support
for EN7581 was introduced, it was assumed to follow the MT7531 model.

However, on Airoha EN7581 and AN7583 switch IP, setting MT7531_CFC alone
is not sufficient for trapped control frames (such as STP BPDUs) to exit
towards the CPU port. The switch forwarding hardware also requires the
MT7530_CPU_EN bit and MT7530_CPU_PORT field to be enabled in MT753X_MFC
(0x10). Without this, trapped frames are dropped by the switch forwarding
logic and never reach the CPU conduit.

Include ID_EN7581 and ID_AN7583 in mt753x_conduit_state_change() to set
MT7530_CPU_EN and MT7530_CPU_PORT in MT753X_MFC when the conduit becomes
operational, matching the handling for MT7530, MT7621, and EN7528.

Fixes: 2b0229f67932 ("net: dsa: mt7530: Add EN7581 support")
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
Signed-off-by: Tao Gong <gongtao0607@gmail.com>
---
 drivers/net/dsa/mt7530.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 20bb07fac0f7..92b5e949ffab 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -3338,7 +3338,8 @@ mt753x_conduit_state_change(struct dsa_switch *ds,
 	 * interface is up.
 	 */
 	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
-	    priv->id != ID_EN7528)
+	    priv->id != ID_EN7528 &&
+	    priv->id != ID_EN7581 && priv->id != ID_AN7583)
 		return;
 
 	mask = BIT(cpu_dp->index);
-- 
2.55.0

Re: [PATCH net v2 2/2] net: dsa: mt7530: trap frames to conduit in MFC on EN7581 and AN7583

[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 patch fixes the same missing MFC CPU_EN/CPU_PORT programming
  on two switch variants, but the Fixes: tag names only the EN7581…
- [Low] After this patch, the comments no longer match the code for
  EN7581/AN7583.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but port
  mirroring on AN7583 writes the wrong bit.

--

net: dsa: mt7530: trap frames to conduit in MFC on EN7581 and AN7583

On EN7581 and AN7583, setting the MT7531_CFC CPU_PMAP bitmap by itself
does not get trapped control frames to the CPU port. This patch adds
ID_EN7581 and ID_AN7583 to mt753x_conduit_state_change(). MT7530_CPU_EN
and MT7530_CPU_PORT in MT753X_MFC are then programmed when the conduit
becomes operational.

> Fixes: 2b0229f67932 ("net: dsa: mt7530: Add EN7581 support")

[Severity: Low]
Should there also be a Fixes tag for the AN7583 commit?

2b0229f67932 did not add ID_AN7583. It was added by d76556db10bf41
("net: dsa: mt7530: Add AN7583 support"), and that commit added AN7583
with the same missing MFC programming.

Stable trees that have 2b0229f67932 but not d76556db10bf41 have no
ID_AN7583. The AN7583 half of the check in mt753x_conduit_state_change()
can't be applied there as written.

Perhaps something like:

Fixes: d76556db10bf ("net: dsa: mt7530: Add AN7583 support")

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 20bb07fac0f7b..92b5e949ffab7 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -3338,7 +3338,8 @@ mt753x_conduit_state_change(struct dsa_switch *ds,
>  	 * interface is up.
>  	 */
>  	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
> -	    priv->id != ID_EN7528)
> +	    priv->id != ID_EN7528 &&
> +	    priv->id != ID_EN7581 && priv->id != ID_AN7583)
>  		return;

[Severity: Low]
This isn't a bug, but should the comments be updated now that this check
also covers EN7581 and AN7583?

The comment above this check still mentions only MT7530:

	/* Set the CPU port to trap frames to for MT7530. Trapped frames will be
	 * forwarded to the numerically smallest CPU port whose conduit
	 * interface is up.
	 */

The comment in mt753x_cpu_port_enable() says the CFC CPU_PMAP bitmap
chooses the trap destination. Its check also includes EN7581 and AN7583:

	/* Add the CPU port to the CPU port bitmap for MT7531 and the switch on
	 * the MT7988 SoC. Trapped frames will be forwarded to the CPU port that
	 * is affine to the inbound user port.
	 */
	if (priv->id == ID_MT7531 || priv->id == ID_MT7988 ||
	    priv->id == ID_EN7581 || priv->id == ID_AN7583)
		mt7530_set(priv, MT7531_CFC, MT7531_CPU_PMAP(BIT(port)));

The two comments now describe different forwarding rules for the same
chips. Neither one says that EN7581 and AN7583 need both registers, or
which register chooses the destination.

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. Looking
at the AN7583 handling, MT753X_MIRROR_EN() in mt7530.h seems to be missing
ID_AN7583:

drivers/net/dsa/mt7530.h:
#define MT753X_MIRROR_EN(id)		((id == ID_MT7531 || \
					  id == ID_MT7988 || \
					  id == ID_EN7581) ? \
					 MT7531_MIRROR_EN : MT7530_MIRROR_EN)

MT753X_MIRROR_REG() and the MT753X_MIRROR_PORT_*() macros do include
ID_AN7583. On AN7583 the register is therefore MT7531_CFC, but the enable
bit is MT7530_MIRROR_EN (BIT(3)). That bit is inside MT7531_CPU_PMAP_MASK
(GENMASK(7, 0)).

Does mt753x_port_mirror_add() then add port 3 to the CPU port bitmap for
trapped frames, instead of enabling mirroring?

	val = mt7530_read(priv, MT753X_MIRROR_REG(priv->id));
	...
	val |= MT753X_MIRROR_EN(priv->id);
	...
	mt7530_write(priv, MT753X_MIRROR_REG(priv->id), val);

MT7531_MIRROR_EN (BIT(19)) is never set, so mirroring would not work on
AN7583. The -EEXIST check in mt753x_port_mirror_add() also tests the wrong
bit.

When the last mirror is removed, mt753x_port_mirror_del() does this:

	val &= ~MT753X_MIRROR_EN(priv->id);

That clears CPU_PMAP bit 3 in MT7531_CFC, not the mirror enable bit.

This seems to go back to d76556db10bf41 ("net: dsa: mt7530: Add AN7583
support"). Adding ID_AN7583 to MT753X_MIRROR_EN() would fix it.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923055103.949540-1-gongtao0607%40gmail.com