net: dsa: all DSA masters must be down when changing the tagging protocol

The fact that the tagging protocol is set and queried from the
/sys/class/net/<dsa-master>/dsa/tagging file is a bit of a quirk from
the single CPU port days which isn't aging very well now that DSA can
have more than a single CPU port. This is because the tagging protocol
is a switch property, yet in the presence of multiple CPU ports it can
be queried and set from multiple sysfs files, all of which are handled
by the same implementation.

The current logic ensures that the net device whose sysfs file we're
changing the tagging protocol through must be down. That net device is
the DSA master, and this is fine for single DSA master / CPU port setups.

But exactly because the tagging protocol is per switch [ tree, in fact ]
and not per DSA master, this isn't fine any longer with multiple CPU
ports, and we must iterate through the tree and find all DSA masters,
and make sure that all of them are down.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 055a6d1d43727413ebfaf483f0f82d49c876caf9..c7bdf7106d2382edd00672f27097d74cf93cea1d 100644 (file)
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1252,7 +1252,6 @@ out_disconnect:
  * they would have formed disjoint trees (different "dsa,member" values).
  */
 int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
-                             struct net_device *master,
                              const struct dsa_device_ops *tag_ops,
                              const struct dsa_device_ops *old_tag_ops)
 {
@@ -1268,14 +1267,11 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
         * attempts to change the tagging protocol. If we ever lift the IFF_UP
         * restriction, there needs to be another mutex which serializes this.
         */
-       if (master->flags & IFF_UP)
-               goto out_unlock;
-
        list_for_each_entry(dp, &dst->ports, list) {
-               if (!dsa_port_is_user(dp))
-                       continue;
+               if (dsa_port_is_cpu(dp) && (dp->master->flags & IFF_UP))
+                       goto out_unlock;
 
-               if (dp->slave->flags & IFF_UP)
+               if (dsa_port_is_user(dp) && (dp->slave->flags & IFF_UP))
                        goto out_unlock;
        }
 

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 8924366467e094b55c323c80311d8a228e20689c..614fbba8fe396e2f799b00f4ee1d50336982c506 100644 (file)
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -545,7 +545,6 @@ struct dsa_lag *dsa_tree_lag_find(struct dsa_switch_tree *dst,
 int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v);
 int dsa_broadcast(unsigned long e, void *v);
 int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
-                             struct net_device *master,
                              const struct dsa_device_ops *tag_ops,
                              const struct dsa_device_ops *old_tag_ops);
 void dsa_tree_master_admin_state_change(struct dsa_switch_tree *dst,

diff --git a/net/dsa/master.c b/net/dsa/master.c
index 9e5dabc751d933c4e8332b535d170606806d271d..fb810edc8281148d9e9b384fdc997e2c4356c4cc 100644 (file)
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -307,7 +307,7 @@ static ssize_t tagging_store(struct device *d, struct device_attribute *attr,
                 */
                goto out;
 
-       err = dsa_tree_change_tag_proto(cpu_dp->ds->dst, dev, new_tag_ops,
+       err = dsa_tree_change_tag_proto(cpu_dp->ds->dst, new_tag_ops,
                                        old_tag_ops);
        if (err) {
                /* On failure the old tagger is restored, so we don't need the