[PATCH net v3 0/2] net: dsa: b53: fix 8021q uppers on standalone ports

[PATCH net v3 0/2] net: dsa: b53: fix 8021q uppers on standalone ports

[Semih Baskan]

Since v5.15, a standalone port on a bcm5301x b53 switch cannot receive
its own tagged traffic: the switch VID lookup is always active, an 8021q
upper's VID never reaches the VLAN table, and a tagged frame with a
missing VID is forwarded only toward the IMP0 management port, which the
in-tree bcm5301x topology leaves disabled, so it never reaches the CPU.
The common victim is a VLAN-tagged PPPoE WAN, where the PADI goes out
and the tagged PADO never reaches the CPU.

My first attempt disabled the VLAN table while not filtering:

  https://lore.kernel.org/all/20260805072641.402-1-strst.gs@gmail.com/

Jonas pointed out that this moves the ARL to shared VLAN learning and
desynchronizes the hardware table from the bridge fdb, and I withdrew
it. I then measured the alternatives on an RT-N18U (BCM53011 rev 5),
with the outbound direction of the same link as a positive control on
every run:

  - With the table enabled, no ingress VID check setting delivers the
    frame: VC4_NO_ING_VID_CHK, VC4_ING_VID_VIO_FWD and
    VC4_ING_VID_VIO_TO_IMP all give 0, and the miss control bit
    VC5_DROP_VTABLE_MISS already sits in its non-drop state, whose only
    delivery target is the disabled IMP0. The frame does not die at
    ingress admission, it dies on the miss path behind it.
  - With the table disabled, a static fdb entry with VID 100 is lost
    from the hardware ARL no matter how the driver drives the ARL
    registers: keeping ARLTBL_IVL_SVL_SELECT at IVL does not preserve
    it, and neither does additionally keeping the VID learning bits in
    VLAN_CTRL0 set.
  - The VID to PVID rewrite bit (CHANGE_1Q_VID) does deliver such a
    frame, but only by rewriting the VID to the PVID, which destroys the
    VID the upper is keyed on.

So on this hardware, delivering the frame and keeping VID-keyed ARL
entries are mutually exclusive unless the VID is in the table. This
series therefore programs the table, narrowed to what is actually
needed: a standalone port only needs the VIDs its 8021q uppers use,
which is one table write per upper instead of entries for all 4096
VIDs.

I looked for a fix inside b53 first. The one existing way to have the
upper VIDs delivered to the driver, ds->needs_standalone_vlan_filtering,
does not work here. It was measured insufficient, because f089652b6b16
makes .port_vlan_add skip the hardware write while not filtering, and
its other effect is one b53 cannot take: with vlan_filtering_is_global,
the forced vlan_filtering=1 in dsa_port_reset_vlan_filtering() would
flip the whole switch into VLAN filtering when any port leaves a
VLAN-unaware bridge. hellcreek relies on exactly those semantics, so
patch 1 adds a narrower opt-in that only delivers the VIDs and leaves
vlan_filtering alone, and patch 2 uses it in b53 and programs entries
that carry standalone members, masked so bridge VLANs stay without
effect while not filtering.

Tested on the RT-N18U: the standalone upper receives 7 of 7 probe frames
with vlan_filtering staying 0, the static fdb entry with a VID now
survives a vlan_filtering toggle since the table enable and the ARL mode
are never touched, uppers keep working across bridge join and leave and
across a vlan_filtering toggle including on ports that were bridged
while the toggle happened, deleting an upper or bridging its port
verifiably stops delivery of that VID to the CPU, and the PPPoE session
establishes. 802.1ad uppers keep working as software VLANs, since this
switch does not parse 0x88a8, and stacked QinQ over an offloaded upper
works too.

Changes in v3:
 - patch 2: BCM5325 and BCM5365 are left out of the opt-in, which now
   gets set after chip detection. Both forward a VLAN table miss, so
   their standalone uppers already work, and their tables hold only 16
   and 256 entries, so v2 made b53_vlan_prepare() refuse an upper whose
   VID lies beyond the table. The message no longer claims such uppers
   fail loudly.
 - patch 1: unchanged.

Changes in v2:
 - patch 1's commit message rewritten after Vladimir Oltean's review.
 - the delivery failure is scoped to bcm5301x in both messages; Jonas
   Gorski observed that other family members still deliver unknown
   VIDs, and the programmed entries are correct there as well.
 - the cover's description of the miss path corrected per the v1
   thread register discussion, and the measured alternatives extended
   with the CHANGE_1Q_VID result.
 - patch 1: the conduit change path no longer skips ports that sit
   under a bridge; with the permanent feature bit their uppers are
   offloaded too, so their CPU port VLANs must move with the conduit.
   Not reachable on b53, which has no .port_change_conduit.
 - patch 2: code unchanged.

v2: https://lore.kernel.org/all/20260826171526.391-1-strst.gs@gmail.com/
v1: https://lore.kernel.org/all/20260806073119.387-1-strst.gs@gmail.com/

Semih Baskan (2):
  net: dsa: let drivers offload 8021q uppers on standalone ports
  net: dsa: b53: offload 8021q uppers on standalone ports

 drivers/net/dsa/b53/b53_common.c | 119 ++++++++++++++++++++++++++-----
 include/net/dsa.h                |   3 +
 net/dsa/port.c                   |  22 ++++--
 net/dsa/user.c                   |   4 +-
 4 files changed, 122 insertions(+), 26 deletions(-)

-- 
2.53.0.windows.1

[PATCH net v3 1/2] net: dsa: let drivers offload 8021q uppers on standalone ports

[Semih Baskan]

Before v5.15, DSA delivered the VIDs of 8021q uppers to switch
drivers unconditionally: user ports advertised
NETIF_F_HW_VLAN_CTAG_FILTER, the 8021q layer reported upper VIDs to
.ndo_vlan_rx_add_vid, and .port_vlan_add programmed them whether or
not a bridge had enabled VLAN filtering. Commit 06cfb2df7eb0 ("net:
dsa: don't advertise 'rx-vlan-filter' when not needed") stopped the
delivery for standalone ports and commit f089652b6b16 ("net: dsa: b53:
do not program vlans when vlan filtering is off") stopped the
programming, on the model that a standalone port is VLAN-unaware and
any 8021q upper is a software VLAN.

That model does not fit hardware whose VID lookup cannot be turned off.
b53 keeps its lookup enabled at all times, because disabling it moves
the ARL to shared VLAN learning: the hash that selects the ARL slot then
treats every VID as 0, entries keyed by a real VID become unreachable,
and the hardware table drifts away from the bridge fdb. With the lookup
active, a tagged frame whose VID is absent from the table is discarded
before it reaches the CPU, measured on bcm5301x. Such a port is never
VLAN-unaware, whatever the bridge asked for. Commit 06cfb2df7eb0 ("net:
dsa: don't advertise 'rx-vlan-filter' when not needed") lists the
reasons a driver may keep it on, and this is its first case, standalone
ports that would otherwise drop VLAN-tagged traffic, except that here
the VLAN awareness is held on by the silicon itself rather than by a
VLAN-aware bridge elsewhere on the switch.

The existing opt-in, ds->needs_standalone_vlan_filtering, is not a
fit. It exists for hellcreek, whose traffic separation depends on
per-port VLANs, so standalone operation there needs the
vlan_filtering state itself forced on:
dsa_port_reset_vlan_filtering() forces vlan_filtering=1 when a port
leaves a VLAN-unaware bridge, and with vlan_filtering_is_global that
lands the whole switch in the state hellcreek wants. On b53 the same
flip is a user-visible mode change for every port on the switch:
bridge VLANs that were committed while inactive become enforced, and
the unknown-VID ingress drop modes turn on chip-wide.
b53 needs the VIDs, not the state.

Add ds->needs_standalone_vlan_offload for that narrower need. It
advertises NETIF_F_HW_VLAN_CTAG_FILTER on user ports permanently, so
upper VIDs reach .port_vlan_add again, and it leaves the
vlan_filtering state alone. This restores the pre-v5.15 delivery
pipeline for drivers that opt in and changes nothing for drivers
that do not.

A permanent feature bit also means dsa_user_manage_vlan_filtering()
must not run on vlan_filtering toggles of such a switch. The
ds->ops->port_vlan_filtering call is unchanged and the driver still
sees every toggle; what is skipped only toggles the feature bit and
replays or clears the VID list, and both halves are wrong when the
bit never goes away. The replay re-adds VIDs that were never cleared,
so vlan_vid_add() refcounts every upper VID twice. The clear strips
the feature bit and the VIDs from a port that happens to be bridged
at toggle time, and its uppers then stay dead even after it leaves
the bridge, because nothing re-offloads them once the feature bit is
gone. Both effects were measured on bcm5301x hardware. The conduit
change path keeps its explicit teardown and restore of the 8021q
upper VLANs, and now runs it for every port of such a switch,
bridged or not, because with the permanent feature bit every port
with uppers has VLANs on the CPU port.

Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
Cc: stable@vger.kernel.org
Signed-off-by: Semih Baskan <strst.gs@gmail.com>
---
 include/net/dsa.h |  3 +++
 net/dsa/port.c    | 22 +++++++++++++++-------
 net/dsa/user.c    |  4 +++-
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7507d632e7c6..67a01fc5f81e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -405,6 +405,9 @@ struct dsa_switch {
 	/* Keep VLAN filtering enabled on ports not offloading any upper */
 	u32			needs_standalone_vlan_filtering:1;
 
+	/* Offload 8021q uppers of standalone ports even when not filtering */
+	u32			needs_standalone_vlan_offload:1;
+
 	/* Pass .port_vlan_add and .port_vlan_del to drivers even for bridges
 	 * that have vlan_filtering=0. All drivers should ideally set this (and
 	 * then the option would get removed), but it is unknown whether this
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 1f5536c0dffc..e61abc7c74f7 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -831,6 +831,9 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 			if (!user)
 				continue;
 
+			if (ds->needs_standalone_vlan_offload)
+				continue;
+
 			err = dsa_user_manage_vlan_filtering(user,
 							     vlan_filtering);
 			if (err)
@@ -839,10 +842,12 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 	} else {
 		dp->vlan_filtering = vlan_filtering;
 
-		err = dsa_user_manage_vlan_filtering(dp->user,
-						     vlan_filtering);
-		if (err)
-			goto restore;
+		if (!ds->needs_standalone_vlan_offload) {
+			err = dsa_user_manage_vlan_filtering(dp->user,
+							     vlan_filtering);
+			if (err)
+				goto restore;
+		}
 	}
 
 	return 0;
@@ -1445,10 +1450,13 @@ int dsa_port_change_conduit(struct dsa_port *dp, struct net_device *conduit,
 
 	/* The port might still be VLAN filtering even if it's no longer
 	 * under a bridge, either due to ds->vlan_filtering_is_global or
-	 * ds->needs_standalone_vlan_filtering. In turn this means VLANs
-	 * on the CPU port.
+	 * ds->needs_standalone_vlan_filtering, and every port of a
+	 * ds->needs_standalone_vlan_offload switch keeps its 8021q upper
+	 * VLANs whether bridged or not. In turn this means VLANs on the
+	 * CPU port.
 	 */
-	vlan_filtering = dsa_port_is_vlan_filtering(dp);
+	vlan_filtering = dsa_port_is_vlan_filtering(dp) ||
+			 ds->needs_standalone_vlan_offload;
 	if (vlan_filtering) {
 		err = dsa_user_manage_vlan_filtering(dev, false);
 		if (err) {
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 041f9060c8ef..fda6ba4fdd13 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1946,6 +1946,7 @@ static int dsa_user_clear_vlan(struct net_device *vdev, int vid, void *arg)
  *
  * - If standalone (this includes software bridge, software LAG):
  *     - if ds->needs_standalone_vlan_filtering = true, OR if
+ *       ds->needs_standalone_vlan_offload = true, OR if
  *       (ds->vlan_filtering_is_global = true AND there are bridges spanning
  *       this switch chip which have vlan_filtering=1)
  *         - the 8021q upper VLANs
@@ -2717,7 +2718,8 @@ void dsa_user_setup_tagger(struct net_device *user)
 	user->hw_features |= NETIF_F_HW_TC;
 	if (user->needed_tailroom)
 		user->features &= ~(NETIF_F_SG | NETIF_F_FRAGLIST);
-	if (ds->needs_standalone_vlan_filtering)
+	if (ds->needs_standalone_vlan_filtering ||
+	    ds->needs_standalone_vlan_offload)
 		user->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
 
 	user->lltx = true;
-- 
2.53.0.windows.1

Re: [PATCH net v3 1/2] net: dsa: let drivers offload 8021q uppers on standalone ports

[Vladimir Oltean]

On Mon, Aug 31, 2026 at 11:52:16AM +0300, Semih Baskan wrote:
> Before v5.15, DSA delivered the VIDs of 8021q uppers to switch
> drivers unconditionally: user ports advertised
> NETIF_F_HW_VLAN_CTAG_FILTER, the 8021q layer reported upper VIDs to
> .ndo_vlan_rx_add_vid, and .port_vlan_add programmed them whether or
> not a bridge had enabled VLAN filtering. Commit 06cfb2df7eb0 ("net:
> dsa: don't advertise 'rx-vlan-filter' when not needed") stopped the
> delivery for standalone ports and commit f089652b6b16 ("net: dsa: b53:
> do not program vlans when vlan filtering is off") stopped the
> programming, on the model that a standalone port is VLAN-unaware and
> any 8021q upper is a software VLAN.
> 
> That model does not fit hardware whose VID lookup cannot be turned off.
> b53 keeps its lookup enabled at all times, because disabling it moves
> the ARL to shared VLAN learning: the hash that selects the ARL slot then
> treats every VID as 0, entries keyed by a real VID become unreachable,
> and the hardware table drifts away from the bridge fdb. With the lookup
> active, a tagged frame whose VID is absent from the table is discarded
> before it reaches the CPU, measured on bcm5301x. Such a port is never
> VLAN-unaware, whatever the bridge asked for. Commit 06cfb2df7eb0 ("net:
> dsa: don't advertise 'rx-vlan-filter' when not needed") lists the
> reasons a driver may keep it on, and this is its first case, standalone
> ports that would otherwise drop VLAN-tagged traffic, except that here
> the VLAN awareness is held on by the silicon itself rather than by a
> VLAN-aware bridge elsewhere on the switch.
> 
> The existing opt-in, ds->needs_standalone_vlan_filtering, is not a
> fit. It exists for hellcreek, whose traffic separation depends on
> per-port VLANs, so standalone operation there needs the
> vlan_filtering state itself forced on:
> dsa_port_reset_vlan_filtering() forces vlan_filtering=1 when a port
> leaves a VLAN-unaware bridge, and with vlan_filtering_is_global that
> lands the whole switch in the state hellcreek wants. On b53 the same
> flip is a user-visible mode change for every port on the switch:
> bridge VLANs that were committed while inactive become enforced, and
> the unknown-VID ingress drop modes turn on chip-wide.
> b53 needs the VIDs, not the state.
> 
> Add ds->needs_standalone_vlan_offload for that narrower need. It
> advertises NETIF_F_HW_VLAN_CTAG_FILTER on user ports permanently, so
> upper VIDs reach .port_vlan_add again, and it leaves the
> vlan_filtering state alone. This restores the pre-v5.15 delivery
> pipeline for drivers that opt in and changes nothing for drivers
> that do not.
> 
> A permanent feature bit also means dsa_user_manage_vlan_filtering()
> must not run on vlan_filtering toggles of such a switch. The
> ds->ops->port_vlan_filtering call is unchanged and the driver still
> sees every toggle; what is skipped only toggles the feature bit and
> replays or clears the VID list, and both halves are wrong when the
> bit never goes away. The replay re-adds VIDs that were never cleared,
> so vlan_vid_add() refcounts every upper VID twice. The clear strips
> the feature bit and the VIDs from a port that happens to be bridged
> at toggle time, and its uppers then stay dead even after it leaves
> the bridge, because nothing re-offloads them once the feature bit is
> gone. Both effects were measured on bcm5301x hardware. The conduit
> change path keeps its explicit teardown and restore of the 8021q
> upper VLANs, and now runs it for every port of such a switch,
> bridged or not, because with the permanent feature bit every port
> with uppers has VLANs on the CPU port.
> 
> Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
> Cc: stable@vger.kernel.org
> Signed-off-by: Semih Baskan <strst.gs@gmail.com>
> ---

I'm sorry I wasn't clear enough the first time when this patch was
proposed.

Nacked-by: Vladimir Oltean <vladimir.oltean@nxp.com>

If you cannot get VLAN-unaware mode to work on this hardware (though
that would still be preferable), then the only acceptable DSA core
change is to always require NETIF_F_HW_VLAN_CTAG_FILTER on user ports
(what this patch does), *as well as* refuse offloading VLAN-unaware
bridges. Otherwise it is just split-brain logic, where the core
limitation leads to restrictions being applied inconsistently.

Sorry, but you can't talk away the need to also handle VLAN-unaware
bridging when you touch the DSA core.

Re: [PATCH net v3 1/2] net: dsa: let drivers offload 8021q uppers on standalone ports

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · Low: 3

- [Medium] Making NETIF_F_HW_VLAN_CTAG_FILTER permanent
  (net/dsa/user.c:2721-2723) and skipping…
- [Medium] The patch introduces an invariant -
  NETIF_F_HW_VLAN_CTAG_FILTER is a permanent property of user ports on a…
- [Low] DISCARDED (baseline false): claim that the new
  ds->needs_standalone_vlan_offload opt-in is dead code and that b53…
- [Low] Comment vs implementation mismatch about the scope of the new
  flag.
- [Low] Documentation/networking/dsa/dsa.rst lists the optional struct
  dsa_switch flags a driver may set ('their behavior when set is…

--

net: dsa: let drivers offload 8021q uppers on standalone ports

Adds ds->needs_standalone_vlan_offload so a driver can have the VIDs of
8021q uppers delivered to .port_vlan_add on standalone ports without
forcing the vlan_filtering state on. The feature bit
NETIF_F_HW_VLAN_CTAG_FILTER becomes permanent for user ports of such a
switch, and dsa_port_vlan_filtering() therefore skips
dsa_user_manage_vlan_filtering(), while dsa_port_change_conduit() keeps
its explicit teardown and restore.

A few questions below.

> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 7507d632e7c6..67a01fc5f81e 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -405,6 +405,9 @@ struct dsa_switch {
>  	/* Keep VLAN filtering enabled on ports not offloading any upper */
>  	u32			needs_standalone_vlan_filtering:1;
>  
> +	/* Offload 8021q uppers of standalone ports even when not filtering */
> +	u32			needs_standalone_vlan_offload:1;
> +

[Severity: Low]
Documentation/networking/dsa/dsa.rst has a list of the optional
dsa_switch flags a driver may set, introduced by "In addition, the
following flags in the dsa_switch structure may optionally be configured
to obtain driver-specific behavior from the DSA core."

That list currently names ds->vlan_filtering_is_global,
ds->needs_standalone_vlan_filtering, ds->configure_vlan_while_not_filtering,
ds->untag_bridge_pvid, ds->assisted_learning_on_cpu_port,
ds->mtu_enforcement_ingress and ds->fdb_isolation.

Should ds->needs_standalone_vlan_offload be added there too, given the
changelog spends several paragraphs on the choice between it and
ds->needs_standalone_vlan_filtering?

>  	/* Pass .port_vlan_add and .port_vlan_del to drivers even for bridges
>  	 * that have vlan_filtering=0. All drivers should ideally set this (and
>  	 * then the option would get removed), but it is unknown whether this
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 1f5536c0dffc..e61abc7c74f7 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -831,6 +831,9 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
>  			if (!user)
>  				continue;
>  
> +			if (ds->needs_standalone_vlan_offload)
> +				continue;
> +
>  			err = dsa_user_manage_vlan_filtering(user,
>  							     vlan_filtering);
>  			if (err)
> @@ -839,10 +842,12 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
>  	} else {
>  		dp->vlan_filtering = vlan_filtering;
>  
> -		err = dsa_user_manage_vlan_filtering(dp->user,
> -						     vlan_filtering);
> -		if (err)
> -			goto restore;
> +		if (!ds->needs_standalone_vlan_offload) {
> +			err = dsa_user_manage_vlan_filtering(dp->user,
> +							     vlan_filtering);
> +			if (err)
> +				goto restore;
> +		}
>  	}

[Severity: Medium]
With the feature bit permanent and this skip in place, the 8021q upper
VIDs of such a switch stay offloaded while the port is enslaved to a
VLAN-unaware bridge.  In exactly that state, are both duplicate-VID
guards bypassed?

dsa_user_vlan_add() only checks for colliding uppers when the bridge is
VLAN-aware:

	if (br_vlan_enabled(dsa_port_bridge_dev_get(dp))) {
		rcu_read_lock();
		err = dsa_user_vlan_check_for_8021q_uppers(dev, vlan);
		rcu_read_unlock();

and the symmetric check in dsa_user_check_8021q_upper() bails out early:

	if (!br || !br_vlan_enabled(br))
		return NOTIFY_DONE;

Meanwhile the bridge VLAN is still committed to hardware, because
ds->configure_vlan_while_not_filtering defaults to true so
dsa_port_skip_vlan_configuration() returns false.

dsa_port_vlan_add()/dsa_port_vlan_del() do no refcounting for user
ports, so can this sequence revoke a VID that the upper still owns?

	ip link add link swp0 name swp0.100 type vlan id 100
	bridge vlan add dev swp0 vid 100
	bridge vlan del dev swp0 vid 100

That is two .port_vlan_add calls and one .port_vlan_del.  On b53 at the
end of the series, the delete clears dev->vlans[100].members & BIT(port),
and b53_standalone_vlan_resync() only iterates VIDs with

	if (!(vl->members & BIT(port)))
		continue;

so when the port later leaves the bridge, VID 100 is not restored and
the always-on VID lookup keeps dropping the upper's tagged traffic until
the upper is destroyed and re-created.  Should the collision guards or
the VID accounting be extended for the new flag?

>  
>  	return 0;
> @@ -1445,10 +1450,13 @@ int dsa_port_change_conduit(struct dsa_port *dp, struct net_device *conduit,
>  
>  	/* The port might still be VLAN filtering even if it's no longer
>  	 * under a bridge, either due to ds->vlan_filtering_is_global or
> -	 * ds->needs_standalone_vlan_filtering. In turn this means VLANs
> -	 * on the CPU port.
> +	 * ds->needs_standalone_vlan_filtering, and every port of a
> +	 * ds->needs_standalone_vlan_offload switch keeps its 8021q upper
> +	 * VLANs whether bridged or not. In turn this means VLANs on the
> +	 * CPU port.
>  	 */
> -	vlan_filtering = dsa_port_is_vlan_filtering(dp);
> +	vlan_filtering = dsa_port_is_vlan_filtering(dp) ||
> +			 ds->needs_standalone_vlan_offload;
>  	if (vlan_filtering) {
>  		err = dsa_user_manage_vlan_filtering(dev, false);
>  		if (err) {

[Severity: Medium]
This patch establishes NETIF_F_HW_VLAN_CTAG_FILTER as a permanent
property of user ports on such a switch, set once in
dsa_user_setup_tagger().  But dsa_port_change_conduit() still tears the
bit down and rebuilds it through dsa_user_manage_vlan_filtering(), and
with the new condition it now does so for every port of such a switch.

Can the error paths leave that invariant broken for good?

The teardown side of dsa_user_manage_vlan_filtering() returns on the
first failure with no rollback of the VIDs it already removed:

	} else {
		err = vlan_for_each(user, dsa_user_clear_vlan, user);
		if (err)
			return err;

		user->features &= ~NETIF_F_HW_VLAN_CTAG_FILTER;
	}

and dsa_port_change_conduit() then jumps to rewind_old_bridge, which
only rejoins the bridge.

The restore side strips the bit on failure:

		err = vlan_for_each(user, dsa_user_restore_vlan, user);
		if (err) {
			vlan_for_each(user, dsa_user_clear_vlan, user);
			user->features &= ~NETIF_F_HW_VLAN_CTAG_FILTER;
			return err;
		}

and the retry in the rewind_old_addrs path of dsa_port_change_conduit()
only logs "failed to restore standalone VLANs".

Once the bit is lost, what re-arms it?  dsa_port_vlan_filtering() now
skips dsa_user_manage_vlan_filtering() unconditionally for such a
switch, dsa_port_assign_conduit() does not call
dsa_user_setup_tagger(), and dsa_port_reset_vlan_filtering() does
nothing when leaving a VLAN-unaware bridge with
needs_standalone_vlan_filtering=0.

That looks like the failure mode the changelog describes as "its uppers
then stay dead even after it leaves the bridge, because nothing
re-offloads them once the feature bit is gone".  Would gating the
clearing of the bit on the new flag, so no error path can drop it, be a
better fit for the intended invariant?

> diff --git a/net/dsa/user.c b/net/dsa/user.c
> index 041f9060c8ef..fda6ba4fdd13 100644
> --- a/net/dsa/user.c
> +++ b/net/dsa/user.c
> @@ -1946,6 +1946,7 @@ static int dsa_user_clear_vlan(struct net_device *vdev, int vid, void *arg)
>   *
>   * - If standalone (this includes software bridge, software LAG):
>   *     - if ds->needs_standalone_vlan_filtering = true, OR if
> + *       ds->needs_standalone_vlan_offload = true, OR if
>   *       (ds->vlan_filtering_is_global = true AND there are bridges spanning
>   *       this switch chip which have vlan_filtering=1)
>   *         - the 8021q upper VLANs

[Severity: Low]
Only the "If standalone" bullet of this summary comment is amended, yet
the next bullet still says that a port "under a vlan_filtering=0 bridge
which it offload" offloads only "the bridge VLANs".  The comment on the
new field in include/net/dsa.h says the same thing in shorter form,
"Offload 8021q uppers of standalone ports even when not filtering".

Is the implementation actually standalone-scoped?
dsa_user_setup_tagger() sets NETIF_F_HW_VLAN_CTAG_FILTER for every user
port of such a switch at port setup, and dsa_port_vlan_filtering() never
clears it again, so a port enslaved to a VLAN-unaware bridge keeps
offloading its 8021q upper VLANs.

The new comment in dsa_port_change_conduit() states the opposite of the
two comments above:

	 * ds->needs_standalone_vlan_filtering, and every port of a
	 * ds->needs_standalone_vlan_offload switch keeps its 8021q upper
	 * VLANs whether bridged or not.

Could the vlan_filtering=0 bridge bullet and the include/net/dsa.h field
comment be updated to match?

> @@ -2717,7 +2718,8 @@ void dsa_user_setup_tagger(struct net_device *user)
>  	user->hw_features |= NETIF_F_HW_TC;
>  	if (user->needed_tailroom)
>  		user->features &= ~(NETIF_F_SG | NETIF_F_FRAGLIST);
> -	if (ds->needs_standalone_vlan_filtering)
> +	if (ds->needs_standalone_vlan_filtering ||
> +	    ds->needs_standalone_vlan_offload)
>  		user->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>  
>  	user->lltx = true;

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

[PATCH net v3 2/2] net: dsa: b53: offload 8021q uppers on standalone ports

[Semih Baskan]

b53 keeps the hardware VID lookup enabled at all times:
b53_switch_alloc() sets dev->vlan_enabled and nothing ever clears it. On
bcm5301x switches, a tagged frame whose VID is absent from the VLAN
table is forwarded only toward the IMP0 management port, which the
in-tree topology leaves disabled, so it never reaches the CPU. Jonas
Gorski reports that other family members still deliver such frames,
older BCM5325/5365 by forwarding them and BCM63268/BCM53115 to the CPU
only, and the entries this patch programs are correct on BCM63268 and
BCM53115 as well. Disabling the lookup is not an option either, because
that moves the ARL to shared VLAN learning, where ARL operations force
VID 0 and the hardware table drifts away from the bridge fdb.

Commit 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when
not needed") stopped advertising NETIF_F_HW_VLAN_CTAG_FILTER on ports
that do not offload a VLAN-aware bridge, so creating an 8021q upper on
a standalone port no longer reaches .ndo_vlan_rx_add_vid and the VID
is never offloaded. Commit f089652b6b16 ("net: dsa: b53: do not
program vlans when vlan filtering is off") then made .port_vlan_add
skip the hardware write while dev->vlan_filtering is false, which it
is for a standalone port. Together they leave standalone ports unable
to receive their own tagged traffic.

This breaks a common configuration, a VLAN-tagged WAN for a PPPoE ISP.
The PADI leaves the port correctly tagged, the concentrator answers,
and the switch discards the tagged PADO, so the session never
establishes. The breakage reached users when OpenWrt 23.05 shipped
v5.15 and is still reproducible.

Take the new needs_standalone_vlan_offload opt-in so DSA reports upper
VIDs again, and program VLAN entries that carry a standalone port even
while not filtering. Only the standalone members and the CPU port are
written to such an entry. A VID used by both an 8021q upper and a
bridge VLAN therefore does not gain the bridged ports as members, so
bridge VLANs keep having no effect while filtering is off, which is
what Documentation/networking/switchdev.rst requires and what that
commit implements. The PVID register writes stay gated on
vlan_filtering for the same reason.

BCM5325 and BCM5365 stay out of the opt-in, which is why the flag is
set after chip detection rather than in b53_switch_alloc(). Both
forward a table miss, so a standalone port there already receives its
uppers' traffic with no entry. Their tables also hold only 16 and 256
VLANs, and with the feature bit back on b53_vlan_prepare() would refuse
any upper whose VID lies beyond the table, an interface that works
today.

b53_configure_vlan() used to restore entries only while filtering, so
restore the standalone ones there as well, otherwise the next
b53_apply_config() wipes them. Bridge join and leave rewrite the
entries of the moved port, because its standalone state is part of the
masking decision: joining removes the port from its uppers' entries,
and leaving adds it back, including uppers that were created while the
port was still bridged. When the last standalone member leaves a VID,
the entry is written back empty, so deleting an upper or bridging its
port returns the hardware to the state it had before the upper
existed.

A tagged upper on BCM7278 port 7 now fails at creation:
b53_vlan_prepare() rejects tagged VLANs on that port, which cannot
receive tagged frames. Previously the ndo was never called, so the
upper was silently created broken.

Measured on an Asus RT-N18U (BCM53011 rev 5) against a peer device. A
probe over an 8021q upper on the standalone WAN port received 0 frames
before and 7 of 7 after, with the outbound direction as a positive
control and vlan_filtering staying 0 throughout. A static fdb entry
with VID 100 survived a vlan_filtering 1->0 toggle in hardware, since
dev->vlan_enabled is never touched and the ARL keeps using independent
VLAN learning. The PPPoE session establishes.

Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
Cc: stable@vger.kernel.org
Signed-off-by: Semih Baskan <strst.gs@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 119 ++++++++++++++++++++++++++-----
 1 file changed, 101 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 0880310c9ce3..b57c0bdacb27 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -898,10 +898,52 @@ static bool b53_vlan_port_may_join_untagged(struct dsa_switch *ds, int port)
 	return dp->bridge == NULL;
 }
 
+static bool b53_vlan_hw_entry(struct dsa_switch *ds, const struct b53_vlan *vl,
+			      struct b53_vlan *hw)
+{
+	struct b53_device *dev = ds->priv;
+	bool standalone = false;
+	struct dsa_port *dp;
+	unsigned int port;
+
+	*hw = *vl;
+
+	if (dev->vlan_filtering)
+		return true;
+
+	hw->members = 0;
+	hw->untag = 0;
+
+	b53_for_each_port(dev, port) {
+		if (!(vl->members & BIT(port)))
+			continue;
+
+		dp = dsa_to_port(ds, port);
+
+		if (!dsa_port_is_cpu(dp)) {
+			if (dp->bridge)
+				continue;
+
+			standalone = true;
+		}
+
+		hw->members |= BIT(port);
+		hw->untag |= vl->untag & BIT(port);
+	}
+
+	if (!standalone) {
+		hw->members = 0;
+		hw->untag = 0;
+	}
+
+	return standalone;
+}
+
 int b53_configure_vlan(struct dsa_switch *ds)
 {
 	struct b53_device *dev = ds->priv;
 	struct b53_vlan vl = { 0 };
+	struct b53_vlan hw;
 	struct b53_vlan *v;
 	int i, def_vid;
 	u16 vid;
@@ -937,20 +979,23 @@ int b53_configure_vlan(struct dsa_switch *ds)
 	}
 	b53_set_vlan_entry(dev, def_vid, &vl);
 
-	if (dev->vlan_filtering) {
-		/* Upon initial call we have not set-up any VLANs, but upon
-		 * system resume, we need to restore all VLAN entries.
-		 */
-		for (vid = def_vid + 1; vid < dev->num_vlans; vid++) {
-			v = &dev->vlans[vid];
+	/* Upon initial call we have not set-up any VLANs, but upon
+	 * system resume, we need to restore all VLAN entries.
+	 */
+	for (vid = def_vid + 1; vid < dev->num_vlans; vid++) {
+		v = &dev->vlans[vid];
 
-			if (!v->members)
-				continue;
+		if (!v->members)
+			continue;
 
-			b53_set_vlan_entry(dev, vid, v);
-			b53_fast_age_vlan(dev, vid);
-		}
+		if (!b53_vlan_hw_entry(ds, v, &hw))
+			continue;
+
+		b53_set_vlan_entry(dev, vid, &hw);
+		b53_fast_age_vlan(dev, vid);
+	}
 
+	if (dev->vlan_filtering) {
 		b53_for_each_port(dev, i) {
 			if (!dsa_is_cpu_port(ds, i))
 				b53_write16(dev, B53_VLAN_PAGE,
@@ -1720,6 +1765,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
 	struct b53_device *dev = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
+	struct b53_vlan hw;
 	struct b53_vlan *vl;
 	u16 old_pvid, new_pvid;
 	int err;
@@ -1751,13 +1797,14 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
 	else
 		vl->untag &= ~BIT(port);
 
-	if (!dev->vlan_filtering)
+	if (!b53_vlan_hw_entry(ds, vl, &hw))
 		return 0;
 
-	b53_set_vlan_entry(dev, vlan->vid, vl);
+	b53_set_vlan_entry(dev, vlan->vid, &hw);
 	b53_fast_age_vlan(dev, vlan->vid);
 
-	if (!dsa_is_cpu_port(ds, port) && new_pvid != old_pvid) {
+	if (dev->vlan_filtering &&
+	    !dsa_is_cpu_port(ds, port) && new_pvid != old_pvid) {
 		b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port),
 			    new_pvid);
 		b53_fast_age_vlan(dev, old_pvid);
@@ -1772,7 +1819,9 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
 {
 	struct b53_device *dev = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+	struct b53_vlan hw;
 	struct b53_vlan *vl;
+	bool needs_hw;
 	u16 pvid;
 
 	if (vlan->vid == 0)
@@ -1782,6 +1831,8 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
 
 	vl = &dev->vlans[vlan->vid];
 
+	needs_hw = b53_vlan_hw_entry(ds, vl, &hw);
+
 	vl->members &= ~BIT(port);
 
 	if (pvid == vlan->vid)
@@ -1791,14 +1842,18 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
 	if (untagged && !b53_vlan_port_needs_forced_tagged(ds, port))
 		vl->untag &= ~(BIT(port));
 
-	if (!dev->vlan_filtering)
+	if (!needs_hw)
 		return 0;
 
-	b53_set_vlan_entry(dev, vlan->vid, vl);
+	b53_vlan_hw_entry(ds, vl, &hw);
+	b53_set_vlan_entry(dev, vlan->vid, &hw);
 	b53_fast_age_vlan(dev, vlan->vid);
 
-	b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port), pvid);
-	b53_fast_age_vlan(dev, pvid);
+	if (dev->vlan_filtering) {
+		b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port),
+			    pvid);
+		b53_fast_age_vlan(dev, pvid);
+	}
 
 	return 0;
 }
@@ -2261,6 +2316,28 @@ int b53_mdb_del(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL(b53_mdb_del);
 
+static void b53_standalone_vlan_resync(struct dsa_switch *ds, int port)
+{
+	struct b53_device *dev = ds->priv;
+	struct b53_vlan hw;
+	struct b53_vlan *vl;
+	u16 vid;
+
+	if (dev->vlan_filtering)
+		return;
+
+	for (vid = b53_default_pvid(dev) + 1; vid < dev->num_vlans; vid++) {
+		vl = &dev->vlans[vid];
+
+		if (!(vl->members & BIT(port)))
+			continue;
+
+		b53_vlan_hw_entry(ds, vl, &hw);
+		b53_set_vlan_entry(dev, vid, &hw);
+		b53_fast_age_vlan(dev, vid);
+	}
+}
+
 int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
 		bool *tx_fwd_offload, struct netlink_ext_ack *extack)
 {
@@ -2324,6 +2401,8 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
 	b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), pvlan);
 	dev->ports[port].vlan_ctl_mask = pvlan;
 
+	b53_standalone_vlan_resync(ds, port);
+
 	return 0;
 }
 EXPORT_SYMBOL(b53_br_join);
@@ -2376,6 +2455,8 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
 		vl->members |= BIT(port);
 		b53_set_vlan_entry(dev, pvid, vl);
 	}
+
+	b53_standalone_vlan_resync(ds, port);
 }
 EXPORT_SYMBOL(b53_br_leave);
 
@@ -3172,6 +3253,8 @@ static int b53_switch_init(struct b53_device *dev)
 	if (!dev->vlans)
 		return -ENOMEM;
 
+	dev->ds->needs_standalone_vlan_offload = !is5325(dev) && !is5365(dev);
+
 	dev->reset_gpio = b53_switch_get_reset_gpio(dev);
 
 	if (PTR_ERR(dev->reset_gpio) == -EPROBE_DEFER)
-- 
2.53.0.windows.1

Re: [PATCH net v3 2/2] net: dsa: b53: offload 8021q uppers on standalone ports

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 5 · Low: 0

- [Medium] b53_vlan_hw_entry() (drivers/net/dsa/b53/b53_common.c:901)
  classifies every non-CPU member port whose dp->bridge is NULL as a…
- [Medium] b53_standalone_vlan_resync() at the end of b53_br_leave()
  (drivers/net/dsa/b53/b53_common.c:2458) derives the leaving port's…
- [Medium] The new opt-in `dev->ds->needs_standalone_vlan_offload =
  !is5325(dev) && !is5365(dev);` (drivers/net/dsa/b53/b53_common.c:3256)…
- [Medium] When a port that already has a working 8021q upper joins a
  VLAN-unaware offloaded bridge, b53_br_join() ->…
- [Medium] b53 records both a bridge VLAN and an 8021q upper VLAN for the
  same port/VID in the single bit dev->vlans[vid].members & BIT(port),…

--

net: dsa: b53: offload 8021q uppers on standalone ports

The patch takes the new needs_standalone_vlan_offload opt-in in b53 and
programs VLAN table entries for VIDs that carry a standalone port even while
dev->vlan_filtering is false, masking the entry down to the standalone
members plus the CPU port.  Bridge join/leave and b53_configure_vlan() now
rewrite those entries, and the opt-in is skipped for BCM5325/BCM5365.

No Link: tags present in the commit header.

> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 0880310c9ce3..b57c0bdacb27 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -898,10 +898,52 @@ static bool b53_vlan_port_may_join_untagged(struct dsa_switch *ds, int port)
>  	return dp->bridge == NULL;
>  }
>  
> +static bool b53_vlan_hw_entry(struct dsa_switch *ds, const struct b53_vlan *vl,
> +			      struct b53_vlan *hw)
> +{
> +	struct b53_device *dev = ds->priv;
> +	bool standalone = false;
> +	struct dsa_port *dp;
> +	unsigned int port;
> +
> +	*hw = *vl;
> +
> +	if (dev->vlan_filtering)
> +		return true;
> +
> +	hw->members = 0;
> +	hw->untag = 0;
> +
> +	b53_for_each_port(dev, port) {
> +		if (!(vl->members & BIT(port)))
> +			continue;
> +
> +		dp = dsa_to_port(ds, port);
> +
> +		if (!dsa_port_is_cpu(dp)) {
> +			if (dp->bridge)
> +				continue;
> +
> +			standalone = true;
> +		}

[Severity: Medium]
Should a DSA link port be able to set standalone here?  A port of type
DSA_PORT_TYPE_DSA never has dp->bridge set, so it passes the
!dsa_port_is_cpu(dp) test and sets standalone = true.

The DSA core does issue VLAN objects for those ports, so their bits do end
up in dev->vlans[vid].members:

net/dsa/switch.c:
    /* Port VLANs match on the targeted port and on all DSA ports */
    static bool dsa_port_vlan_match(struct dsa_port *dp,
                                    struct dsa_notifier_vlan_info *info)
    {
            return dsa_port_is_dsa(dp) || dp == info->dp;
    }

On a cascaded b53 tree, does that mean b53_vlan_hw_entry() returns true for
a VID whose only user-port members are bridge ports, and b53_vlan_add() /
b53_vlan_del() / b53_configure_vlan() then write an entry containing only
the DSA links and the CPU port?  Since b53 keeps VC0_VLAN_EN|VC0_VID_CHK_EN
set at all times, the programmed member mask governs egress for that VID,
so the bridged ports would be removed from that VID's forwarding domain
while filtering is off.

There is also no corrective path afterwards: b53_standalone_vlan_resync()
is only reached from b53_br_join()/b53_br_leave(), which a DSA link never
traverses, so the entry stays as written.

Would including DSA link ports as members without letting them set
standalone match the intent stated in the commit message ("Only the
standalone members and the CPU port are written to such an entry")?

> +
> +		hw->members |= BIT(port);
> +		hw->untag |= vl->untag & BIT(port);
> +	}
> +
> +	if (!standalone) {
> +		hw->members = 0;
> +		hw->untag = 0;
> +	}
> +
> +	return standalone;
> +}
> +

[ ... ]

> @@ -1772,7 +1819,9 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
>  {
>  	struct b53_device *dev = ds->priv;
>  	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> +	struct b53_vlan hw;
>  	struct b53_vlan *vl;
> +	bool needs_hw;
>  	u16 pvid;
>  
>  	if (vlan->vid == 0)
> @@ -1782,6 +1831,8 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
>  
>  	vl = &dev->vlans[vlan->vid];
>  
> +	needs_hw = b53_vlan_hw_entry(ds, vl, &hw);
> +
>  	vl->members &= ~BIT(port);

[Severity: Medium]
Can a bridge VLAN deletion here destroy the record of an 8021q upper on the
same port and VID?  dev->vlans[vid].members has one bit per port with no
per-owner accounting, and the core does not refcount VLANs on user ports:

net/dsa/switch.c:dsa_port_do_vlan_add() {
        /* No need to bother with refcounting for user ports. */
        if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp))) {
                err = ds->ops->port_vlan_add(ds, port, vlan, extack);

The colliding pair is only rejected while bridge VLAN filtering is on:

net/dsa/user.c:dsa_user_vlan_add() {
        if (br_vlan_enabled(dsa_port_bridge_dev_get(dp))) {
                rcu_read_lock();
                err = dsa_user_vlan_check_for_8021q_uppers(dev, vlan);

so with vlan_filtering 0 the sequence

  ip link add link ethX name ethX.100 type vlan id 100
  ip link set dev ethX master br0        (br0 with vlan_filtering 0)
  bridge vlan add dev ethX vid 100
  bridge vlan del dev ethX vid 100

leaves vl->members without BIT(port) although ethX.100 still exists.  After
that, does anything rewrite the entry again?  b53_standalone_vlan_resync()
skips the VID because of

        if (!(vl->members & BIT(port)))
                continue;

and b53_configure_vlan()'s restore loop keys off v->members as well, so
tagged reception on ethX.100 would stay dead until the upper is re-created.

>  
>  	if (pvid == vlan->vid)
> @@ -1791,14 +1842,18 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
>  	if (untagged && !b53_vlan_port_needs_forced_tagged(ds, port))
>  		vl->untag &= ~(BIT(port));
>  
> -	if (!dev->vlan_filtering)
> +	if (!needs_hw)
>  		return 0;
>  
> -	b53_set_vlan_entry(dev, vlan->vid, vl);
> +	b53_vlan_hw_entry(ds, vl, &hw);
> +	b53_set_vlan_entry(dev, vlan->vid, &hw);
>  	b53_fast_age_vlan(dev, vlan->vid);
>  
> -	b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port), pvid);
> -	b53_fast_age_vlan(dev, pvid);
> +	if (dev->vlan_filtering) {
> +		b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port),
> +			    pvid);
> +		b53_fast_age_vlan(dev, pvid);
> +	}
>  
>  	return 0;
>  }

[ ... ]

> @@ -2261,6 +2316,28 @@ int b53_mdb_del(struct dsa_switch *ds, int port,
>  }
>  EXPORT_SYMBOL(b53_mdb_del);
>  
> +static void b53_standalone_vlan_resync(struct dsa_switch *ds, int port)
> +{
> +	struct b53_device *dev = ds->priv;
> +	struct b53_vlan hw;
> +	struct b53_vlan *vl;
> +	u16 vid;
> +
> +	if (dev->vlan_filtering)
> +		return;
> +
> +	for (vid = b53_default_pvid(dev) + 1; vid < dev->num_vlans; vid++) {
> +		vl = &dev->vlans[vid];
> +
> +		if (!(vl->members & BIT(port)))
> +			continue;
> +
> +		b53_vlan_hw_entry(ds, vl, &hw);
> +		b53_set_vlan_entry(dev, vid, &hw);
> +		b53_fast_age_vlan(dev, vid);
> +	}
> +}
> +
>  int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>  		bool *tx_fwd_offload, struct netlink_ext_ack *extack)
>  {
> @@ -2324,6 +2401,8 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>  	b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), pvlan);
>  	dev->ports[port].vlan_ctl_mask = pvlan;
>  
> +	b53_standalone_vlan_resync(ds, port);
> +

[Severity: Medium]
When a port that already has a working 8021q upper joins a VLAN-unaware
bridge, this call recomputes that upper's VID with dp->bridge now set, so
b53_vlan_hw_entry() hits

        if (dp->bridge)
                continue;

standalone stays false, and the entry is written back empty.  On bcm5301x,
where a table miss is only forwarded toward the disabled IMP0 port, does
that silently stop the upper from receiving tagged frames?

Nothing reports the loss either: b53_vlan_add() returns 0 when
b53_vlan_hw_entry() finds no standalone member, so DSA continues to track
the VID as offloaded.

Documentation/networking/switchdev.rst says:

    When there is a VLAN device (e.g: sw0p1.100) configured on top of a
    switchdev network device which is a bridge port member, the behavior of
    the software network stack must be preserved, or the configuration must
    be refused if that is not possible.

and the DSA-side comment added by this series states:

    net/dsa/port.c:
         * ds->needs_standalone_vlan_offload switch keeps its 8021q upper
         * VLANs whether bridged or not.

Should this case be refused with an extack instead of dropping the offload
silently?  The commit message's argument that "bridge VLANs keep having no
effect while filtering is off" covers bridge VLANs, but the VID here comes
from an 8021q upper.

>  	return 0;
>  }
>  EXPORT_SYMBOL(b53_br_join);
> @@ -2376,6 +2455,8 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>  		vl->members |= BIT(port);
>  		b53_set_vlan_entry(dev, pvid, vl);
>  	}
> +
> +	b53_standalone_vlan_resync(ds, port);
>  }
>  EXPORT_SYMBOL(b53_br_leave);

[Severity: Medium]
Is dp->bridge always already cleared when this runs?  On the normal leave
path dsa_port_bridge_leave() calls dsa_port_bridge_destroy() before
broadcasting DSA_NOTIFIER_BRIDGE_LEAVE, but the join rollback does the
opposite order:

net/dsa/port.c:dsa_port_bridge_join() {
        ...
out_rollback_unbridge:
        dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
out_rollback:
        dsa_port_bridge_destroy(dp, br);
        return err;
}

So when switchdev_bridge_port_offload() or dsa_port_switchdev_sync_attrs()
fails, b53_br_leave() -> b53_standalone_vlan_resync() runs with dp->bridge
still set, b53_vlan_hw_entry() skips the port, and the entry is written
empty even though the port becomes standalone again a moment later.

Does anything restore it afterwards?  b53_vlan_add()/b53_vlan_del() are not
called because the upper still exists, and b53_configure_vlan() only runs
from b53_apply_config() on setup, resume, or a vlan_filtering change, so
tagged reception on the port's uppers would stay off after a failed bridge
join.

>  
> @@ -3172,6 +3253,8 @@ static int b53_switch_init(struct b53_device *dev)
>  	if (!dev->vlans)
>  		return -ENOMEM;
>  
> +	dev->ds->needs_standalone_vlan_offload = !is5325(dev) && !is5365(dev);

[Severity: Medium]
Does this condition exclude BCM5365 on kernels built without
CONFIG_BCM47XX?

drivers/net/dsa/b53/b53_priv.h:
    static inline int is5365(struct b53_device *dev)
    {
    #ifdef CONFIG_BCM47XX
            return dev->chip_id == BCM5365_DEVICE_ID;
    #else
            return 0;
    #endif
    }

while b53_switch_detect() sets the chip id with no config guard:

        } else {
                dev->chip_id = BCM5365_DEVICE_ID;
        }

and both b53_mdio.c and b53_spi.c carry a "brcm,bcm5365" match.  On such a
build the flag would be set for a chip with .vlans = 256, so
NETIF_F_HW_VLAN_CTAG_FILTER gets advertised and every upper VID reaches
b53_vlan_prepare():

        if (vlan->vid >= dev->num_vlans)
                return -ERANGE;

Would ip link add link ethX name ethX.300 type vlan id 300 then fail where
it previously created a working software VLAN?  That is the case the commit
message says is preserved:

    BCM5325 and BCM5365 stay out of the opt-in, ... b53_vlan_prepare() would
    refuse any upper whose VID lies beyond the table, an interface that works
    today.

Would testing dev->chip_id (or dev->num_vlans) directly rather than the
CONFIG_BCM47XX-conditional helper keep that policy in every build?

>  
>  	dev->reset_gpio = b53_switch_get_reset_gpio(dev);
>  
>  	if (PTR_ERR(dev->reset_gpio) == -EPROBE_DEFER)

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

Re: [PATCH net v3 0/2] net: dsa: b53: fix 8021q uppers on standalone ports

[Jonas Gorski]

Hi,

On Mon, Aug 31, 2026 at 10:52 AM Semih Baskan <strst.gs@gmail.com> wrote:
>
> Since v5.15, a standalone port on a bcm5301x b53 switch cannot receive
> its own tagged traffic: the switch VID lookup is always active, an 8021q
> upper's VID never reaches the VLAN table, and a tagged frame with a
> missing VID is forwarded only toward the IMP0 management port, which the
> in-tree bcm5301x topology leaves disabled, so it never reaches the CPU.
> The common victim is a VLAN-tagged PPPoE WAN, where the PADI goes out
> and the tagged PADO never reaches the CPU.

I really don't agree with the framing here. The subject essentially
claims that uppers don't work in general, but that is not true at all.
They don't work for devices with a *broken* device tree configuration.

So this is a work-around for broken configuration.

Also this is an incomplete fix, because it still won't work 100%, and
there is no way to fix this from within the driver, the device tree
needs to be fixed. E.g. with the (assumed) disabled EAP mode on
stand-alone ports, you will have the issue that the switch will drop
any frames with destination MACs that match a learned entry on a
bridged port.

I really think we should just fix up the in-tree device trees and then
complain loudly about the broken configuration on probe in case there
are out of tree device trees with the broken configuration. I mean it
was broken since v5.15 and wasn't noticed until recently, so it
doesn't seem affect many users.

Best regards,
Jonas

Re: [PATCH net v3 0/2] net: dsa: b53: fix 8021q uppers on standalone ports

[Semih Baskan]

Hi Jonas,

On Mon, Aug 31, 2026 at 12:34 PM Jonas Gorski <jonas.gorski@gmail.com> wrote:
> I really think we should just fix up the in-tree device trees and then
> complain loudly about the broken configuration on probe in case there
> are out of tree device trees with the broken configuration.

I agree, fixing this up in the device tree is the more robust way.
In the meantime OpenWrt can carry these patches as a band aid for
the affected boards.

Best regards,
Semih

Re: [PATCH net v3 0/2] net: dsa: b53: fix 8021q uppers on standalone ports

[Florian Fainelli]

On 8/31/2026 3:17 AM, Semih Baskan wrote:
> Hi Jonas,
> 
> On Mon, Aug 31, 2026 at 12:34 PM Jonas Gorski <jonas.gorski@gmail.com> wrote:
>> I really think we should just fix up the in-tree device trees and then
>> complain loudly about the broken configuration on probe in case there
>> are out of tree device trees with the broken configuration.
> 
> I agree, fixing this up in the device tree is the more robust way.
> In the meantime OpenWrt can carry these patches as a band aid for
> the affected boards.

But you control exact how the DTS gets distributed in OpenWrt, why not 
just fix all of the DTSes there and have the users confirm that using 
port 8 as the IMP works?

Don't get me wrong, I am very appreciative that you spent a decent 
amount of time trying to come up with some sort of solution.

Thanks
-- 
Florian

Re: [PATCH net v3 0/2] net: dsa: b53: fix 8021q uppers on standalone ports

[Semih Baskan]

Hi Florian,

On Tue, Sep 1, 2026 at 3:24 AM Florian Fainelli
<florian.fainelli@broadcom.com> wrote:
> But you control exact how the DTS gets distributed in OpenWrt, why not
> just fix all of the DTSes there and have the users confirm that using
> port 8 as the IMP works?
>
> Don't get me wrong, I am very appreciative that you spent a decent
> amount of time trying to come up with some sort of solution.

Thank you. Yes, that is what I am planning to do. Port 8 as the IMP
is already confirmed on my board, tagged RX and PPPoE come back with
no driver changes. Next week I will start with my own board in
OpenWrt and hope owners of the other bcm53xx devices can confirm
theirs.

Best regards,
Semih