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

[Vladimir Oltean <vladimir.oltean@nxp.com>]

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.