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@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8150e16aaa55e8bacd76e4033d3e6c2f52d0ef5e..dcd67801eca48cb97393968637babd8bc3003fab 100644 (file)
--- 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 9ea637832ea9f4f499b703d5aea36e85e1877e94..b7a269e0513f9e8d69b7c25ed28caaaba4a2f710 100644 (file)
--- 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 ee1c6bfcb3868e66e0cf03a03442bddd8dac75e1..979042a64d1ad2cd72522d367a90337520a7dfa5 100644 (file)
--- 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 654697ebb6f3904dce4464407533c52a264b52db..e6d5f3b4fd89a2a0b3c27d49fc00618763c1c857 100644 (file)
--- 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)