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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOiHx=mYJm_mrwrDCuQ4ZEviPKLgWDqNcaSXPzaXHeGYWBJCaA@mail.gmail.com>
Date: Sat, 12 Apr 2025 15:50:00 +0200
From: Jonas Gorski <jonas.gorski@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Nikolay Aleksandrov <razor@...ckwall.org>, Ido Schimmel <idosch@...dia.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Andrew Lunn <andrew@...n.ch>, Vladimir Oltean <vladimir.oltean@....com>, bridge@...ts.linux.dev, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net 0/2] net: dsa: fix handling brentry vlans with flags

On Sat, Apr 12, 2025 at 3:34 PM Vladimir Oltean <olteanv@...il.com> wrote:
>
> 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.

Yes, because I messed up writing down what I did. That should have
been vlan_default_pvid 0 so there are no VLANs configured by default.
>
> 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.

It's b53, and on first glance it does not set it. But as I just
noticed, I messed up the example.

So adding the lan1 vid is supposed to be the very first vlan that is configured.

>
> > $ 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.

Sorry for the mistake. I did it correctly while testing though!

Best regards,
Jonas