| 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
| ||
|
Message-Id: <20210811134606.2777146-3-vladimir.oltean@nxp.com>
Date: Wed, 11 Aug 2021 16:46:06 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
"David S. Miller" <davem@...emloft.net>
Cc: Florian Fainelli <f.fainelli@...il.com>,
Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Vladimir Oltean <olteanv@...il.com>
Subject: [PATCH net-next 2/2] net: dsa: tag_8021q: don't broadcast during setup/teardown
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>
---
net/dsa/dsa2.c | 3 +++
net/dsa/dsa_priv.h | 4 ++--
net/dsa/port.c | 14 ++++++++++----
net/dsa/tag_8021q.c | 21 +++++++++++----------
4 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8150e16aaa55..dcd67801eca4 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -49,6 +49,9 @@ int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v)
* Can be used to notify the switching fabric of events such as cross-chip
* bridging between disjoint trees (such as islands of tagger-compatible
* switches bridged by an incompatible middle switch).
+ *
+ * WARNING: this function is not reliable during probe time, because probing
+ * between trees is asynchronous and not all DSA trees might have probed.
*/
int dsa_broadcast(unsigned long e, void *v)
{
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9575cabd3ec3..d9cc1ffcca10 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -261,8 +261,8 @@ int dsa_port_link_register_of(struct dsa_port *dp);
void dsa_port_link_unregister_of(struct dsa_port *dp);
int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr);
void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr);
-int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid);
-void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid);
+int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast);
+void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast);
extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ee1c6bfcb386..979042a64d1a 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1314,7 +1314,7 @@ void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr)
dp->index, ERR_PTR(err));
}
-int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid)
+int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast)
{
struct dsa_notifier_tag_8021q_vlan_info info = {
.tree_index = dp->ds->dst->index,
@@ -1323,10 +1323,13 @@ int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid)
.vid = vid,
};
- return dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info);
+ if (broadcast)
+ return dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info);
+
+ return dsa_port_notify(dp, DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info);
}
-void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid)
+void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast)
{
struct dsa_notifier_tag_8021q_vlan_info info = {
.tree_index = dp->ds->dst->index,
@@ -1336,7 +1339,10 @@ void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid)
};
int err;
- err = dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
+ if (broadcast)
+ err = dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
+ else
+ err = dsa_port_notify(dp, DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
if (err)
dev_err(dp->ds->dev,
"port %d failed to notify tag_8021q VLAN %d deletion: %pe\n",
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 654697ebb6f3..e6d5f3b4fd89 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -362,12 +362,12 @@ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
continue;
/* Install the RX VID of the targeted port in our VLAN table */
- err = dsa_port_tag_8021q_vlan_add(dp, targeted_rx_vid);
+ err = dsa_port_tag_8021q_vlan_add(dp, targeted_rx_vid, false);
if (err)
return err;
/* Install our RX VID into the targeted port's VLAN table */
- err = dsa_port_tag_8021q_vlan_add(targeted_dp, rx_vid);
+ err = dsa_port_tag_8021q_vlan_add(targeted_dp, rx_vid, false);
if (err)
return err;
}
@@ -398,10 +398,10 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
continue;
/* Remove the RX VID of the targeted port from our VLAN table */
- dsa_port_tag_8021q_vlan_del(dp, targeted_rx_vid);
+ dsa_port_tag_8021q_vlan_del(dp, targeted_rx_vid, true);
/* Remove our RX VID from the targeted port's VLAN table */
- dsa_port_tag_8021q_vlan_del(targeted_dp, rx_vid);
+ dsa_port_tag_8021q_vlan_del(targeted_dp, rx_vid, true);
}
return 0;
@@ -413,7 +413,8 @@ int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
{
u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
- return dsa_port_tag_8021q_vlan_add(dsa_to_port(ds, port), tx_vid);
+ return dsa_port_tag_8021q_vlan_add(dsa_to_port(ds, port), tx_vid,
+ true);
}
EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
@@ -423,7 +424,7 @@ void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
{
u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
- dsa_port_tag_8021q_vlan_del(dsa_to_port(ds, port), tx_vid);
+ dsa_port_tag_8021q_vlan_del(dsa_to_port(ds, port), tx_vid, true);
}
EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_unoffload);
@@ -450,7 +451,7 @@ static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)
* L2 forwarding rules still take precedence when there are no VLAN
* restrictions, so there are no concerns about leaking traffic.
*/
- err = dsa_port_tag_8021q_vlan_add(dp, rx_vid);
+ err = dsa_port_tag_8021q_vlan_add(dp, rx_vid, true);
if (err) {
dev_err(ds->dev,
"Failed to apply RX VID %d to port %d: %pe\n",
@@ -462,7 +463,7 @@ static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)
vlan_vid_add(master, ctx->proto, rx_vid);
/* Finally apply the TX VID on this port and on the CPU port */
- err = dsa_port_tag_8021q_vlan_add(dp, tx_vid);
+ err = dsa_port_tag_8021q_vlan_add(dp, tx_vid, true);
if (err) {
dev_err(ds->dev,
"Failed to apply TX VID %d on port %d: %pe\n",
@@ -489,11 +490,11 @@ static void dsa_tag_8021q_port_teardown(struct dsa_switch *ds, int port)
master = dp->cpu_dp->master;
- dsa_port_tag_8021q_vlan_del(dp, rx_vid);
+ dsa_port_tag_8021q_vlan_del(dp, rx_vid, false);
vlan_vid_del(master, ctx->proto, rx_vid);
- dsa_port_tag_8021q_vlan_del(dp, tx_vid);
+ dsa_port_tag_8021q_vlan_del(dp, tx_vid, false);
}
static int dsa_tag_8021q_setup(struct dsa_switch *ds)
--
2.25.1
Powered by blists - more mailing lists