Re: [PATCH RFC net 0/2] net: dsa: fix handling brentry vlans with flags

[Vladimir Oltean <olteanv@gmail.com>]

On Sat, Apr 12, 2025 at 02:24:26PM +0200, Jonas Gorski wrote:
> While trying to figure out the hardware behavior of a DSA supported
> switch chip and printing various internal vlan state changes, I noticed
> that some flows never triggered adding the cpu port to vlans, preventing
> it from receiving any of the VLANs traffic.
> 
> E.g. the following sequence would cause the cpu port not being member of
> the vlan, despite the bridge vlan output looking correct:
> 
> $ ip link add swbridge type bridge vlan_filtering 1 vlan_default_pvid 1
> $ ip link set lan1 master swbridge

At this step, dsa_port_bridge_join() -> switchdev_bridge_port_offload()
-> ... -> br_switchdev_port_offload() -> nbp_switchdev_sync_objs() ->
br_switchdev_vlan_replay() -> br_switchdev_vlan_replay_group(br_vlan_group(br))
-> br_switchdev_vlan_replay_one() should have notified DSA, with changed=false.
It should be processed by dsa_user_host_vlan_add() -> dsa_port_host_vlan_add().

You make it sound like that doesn't happen.

I notice you didn't mention which "DSA supported chip" you are using.
By any chance, does its driver set ds->configure_vlan_while_not_filtering = false?
That would be my prime suspect, making dsa_port_skip_vlan_configuration() ignore
the code path above, because the bridge port is not yet VLAN filtering.
It becomes VLAN filtering only a bit later in dsa_port_bridge_join(),
with the dsa_port_switchdev_sync_attrs() -> dsa_port_vlan_filtering(br_vlan_enabled(br))
call.

If that is the case, the only thing that is slightly confusing to me is
why you haven't seen the "skipping configuration of VLAN" extack message.
But anyway, if the theory above is true, you should instead be looking
at adding proper VLAN support to said driver, and drop this set instead,
because VLAN replay isn't working properly.

> $ bridge vlan add dev lan1 vid 1 pvid untagged
> $ bridge vlan add dev swbridge vid 1 pvid untagged self
> 
> Adding more printk debugging, I traced it br_vlan_add_existing() setting
> changed to true (since the vlan "gained" the pvid untagged flags), and
> then the dsa code ignoring the vlan notification, since it is a vlan for
> the cpu port that is updated.

Yes, this part and everything that follows should be correct.