netdev - Re: [PATCH net-next 2/2] net: dsa: tag_8021q: don't broadcast during setup/teardown

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: <c08d8fac-708f-64e8-f002-0c98a0fb1888@gmail.com>
Date:   Thu, 12 Aug 2021 01:38:19 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Subject: Re: [PATCH net-next 2/2] net: dsa: tag_8021q: don't broadcast during
 setup/teardown



On 8/11/2021 6:46 AM, Vladimir Oltean wrote:
> Currently, on my board with multiple sja1105 switches in disjoint trees
> described in commit f66a6a69f97a ("net: dsa: permit cross-chip bridging
> between all trees in the system"), rebooting the board triggers the
> following benign warnings:
> 
> [   12.345566] sja1105 spi2.0: port 0 failed to notify tag_8021q VLAN 1088 deletion: -ENOENT
> [   12.353804] sja1105 spi2.0: port 0 failed to notify tag_8021q VLAN 2112 deletion: -ENOENT
> [   12.362019] sja1105 spi2.0: port 1 failed to notify tag_8021q VLAN 1089 deletion: -ENOENT
> [   12.370246] sja1105 spi2.0: port 1 failed to notify tag_8021q VLAN 2113 deletion: -ENOENT
> [   12.378466] sja1105 spi2.0: port 2 failed to notify tag_8021q VLAN 1090 deletion: -ENOENT
> [   12.386683] sja1105 spi2.0: port 2 failed to notify tag_8021q VLAN 2114 deletion: -ENOENT
> 
> Basically switch 1 calls dsa_tag_8021q_unregister, and switch 1's TX and
> RX VLANs cannot be found on switch 2's CPU port.
> 
> But why would switch 2 even attempt to delete switch 1's TX and RX
> tag_8021q VLANs from its CPU port? Well, because we use dsa_broadcast,
> and it is supposed that it had added those VLANs in the first place
> (because in dsa_port_tag_8021q_vlan_match, all CPU ports match
> regardless of their tree index or switch index).
> 
> The two trees probe asynchronously, and when switch 1 probed, it called
> dsa_broadcast which did not notify the tree of switch 2, because that
> didn't probe yet. But during unbind, switch 2's tree _is_ probed, so it
> _is_ notified of the deletion.
> 
> Before jumping to introduce a synchronization mechanism between the
> probing across disjoint switch trees, let's take a step back and see
> whether we _need_ to do that in the first place.
> 
> The RX and TX VLANs of switch 1 would be needed on switch 2's CPU port
> only if switch 1 and 2 were part of a cross-chip bridge. And
> dsa_tag_8021q_bridge_join takes care precisely of that (but if probing
> was synchronous, the bridge_join would just end up bumping the VLANs'
> refcount, because they are already installed by the setup path).
> 
> Since by the time the ports are bridged, all DSA trees are already set
> up, and we don't need the tag_8021q VLANs of one switch installed on the
> other switches during probe time, the answer is that we don't need to
> fix the synchronization issue.
> 
> So make the setup and teardown code paths call dsa_port_notify, which
> notifies only the local tree, and the bridge code paths call
> dsa_broadcast, which let the other trees know as well.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
-- 
Florian