Merge branch 'dsa-fixups'

Vladimir Oltean says:

====================
DSA tagger-owned storage fixups

It seems that the DSA tagger-owned storage changes were insufficiently
tested and do not work in all cases. Specifically, the NXP Bluebox 3
(arch/arm64/boot/dts/freescale/fsl-lx2160a-bluebox3.dts) got broken by
these changes, because
(a) I forgot that DSA_TAG_PROTO_SJA1110 exists and differs from
    DSA_TAG_PROTO_SJA1105
(b) the Bluebox 3 uses a DSA switch tree with 2 switches, and the
    tagger-owned storage patches don't cover that use case well, it
    seems

Therefore, I'm sorry to say that there needs to be an API fixup: tagging
protocol drivers will from now on connect to individual switches from a
tree, rather than to the tree as a whole. This is more robust against
various ordering constraints in the DSA probe and teardown paths, and is
also symmetrical with the connection API exposed to the switch drivers
themselves, which is also per switch.

With these changes, the Bluebox 3 also works fine.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>

drivers/net/dsa/sja1105/sja1105_main.c

@@ -2708,17 +2708,17 @@ static void sja1105_port_deferred_xmit(struct kthread_work *work)
 static int sja1105_connect_tag_protocol(struct dsa_switch *ds,
 					enum dsa_tag_protocol proto)
 {
+	struct sja1105_private *priv = ds->priv;
 	struct sja1105_tagger_data *tagger_data;
 
-	switch (proto) {
-	case DSA_TAG_PROTO_SJA1105:
-		tagger_data = sja1105_tagger_data(ds);
-		tagger_data->xmit_work_fn = sja1105_port_deferred_xmit;
-		tagger_data->meta_tstamp_handler = sja1110_process_meta_tstamp;
-		return 0;
-	default:
+	if (proto != priv->info->tag_proto)
 		return -EPROTONOSUPPORT;
-	}
+
+	tagger_data = sja1105_tagger_data(ds);
+	tagger_data->xmit_work_fn = sja1105_port_deferred_xmit;
+	tagger_data->meta_tstamp_handler = sja1110_process_meta_tstamp;
+
+	return 0;
 }
 
 /* The MAXAGE setting belongs to the L2 Forwarding Parameters table,

include/linux/dsa/sja1105.h

@@ -70,7 +70,8 @@ struct sja1105_skb_cb {
 static inline struct sja1105_tagger_data *
 sja1105_tagger_data(struct dsa_switch *ds)
 {
-	BUG_ON(ds->dst->tag_ops->proto != DSA_TAG_PROTO_SJA1105);
+	BUG_ON(ds->dst->tag_ops->proto != DSA_TAG_PROTO_SJA1105 &&
+	       ds->dst->tag_ops->proto != DSA_TAG_PROTO_SJA1110);
 
 	return ds->tagger_data;
 }

include/net/dsa.h

@@ -82,15 +82,14 @@ enum dsa_tag_protocol {
 };
 
 struct dsa_switch;
-struct dsa_switch_tree;
 
 struct dsa_device_ops {
 	struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
 	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev);
 	void (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
 			     int *offset);
-	int (*connect)(struct dsa_switch_tree *dst);
-	void (*disconnect)(struct dsa_switch_tree *dst);
+	int (*connect)(struct dsa_switch *ds);
+	void (*disconnect)(struct dsa_switch *ds);
 	unsigned int needed_headroom;
 	unsigned int needed_tailroom;
 	const char *name;

net/dsa/dsa2.c

@@ -248,12 +248,8 @@ static struct dsa_switch_tree *dsa_tree_alloc(int index)
 
 static void dsa_tree_free(struct dsa_switch_tree *dst)
 {
-	if (dst->tag_ops) {
-		if (dst->tag_ops->disconnect)
-			dst->tag_ops->disconnect(dst);
-
+	if (dst->tag_ops)
 		dsa_tag_driver_put(dst->tag_ops);
-	}
 	list_del(&dst->list);
 	kfree(dst);
 }
@@ -841,17 +837,29 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds)
 	}
 
 connect:
+	if (tag_ops->connect) {
+		err = tag_ops->connect(ds);
+		if (err)
+			return err;
+	}
+
 	if (ds->ops->connect_tag_protocol) {
 		err = ds->ops->connect_tag_protocol(ds, tag_ops->proto);
 		if (err) {
 			dev_err(ds->dev,
 				"Unable to connect to tag protocol \"%s\": %pe\n",
 				tag_ops->name, ERR_PTR(err));
-			return err;
+			goto disconnect;
 		}
 	}
 
 	return 0;
+
+disconnect:
+	if (tag_ops->disconnect)
+		tag_ops->disconnect(ds);
+
+	return err;
 }
 
 static int dsa_switch_setup(struct dsa_switch *ds)
@@ -1160,13 +1168,6 @@ static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst,
 
 	dst->tag_ops = tag_ops;
 
-	/* Notify the new tagger about the connection to this tree */
-	if (tag_ops->connect) {
-		err = tag_ops->connect(dst);
-		if (err)
-			goto out_revert;
-	}
-
 	/* Notify the switches from this tree about the connection
 	 * to the new tagger
 	 */
@@ -1176,16 +1177,14 @@ static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst,
 		goto out_disconnect;
 
 	/* Notify the old tagger about the disconnection from this tree */
-	if (old_tag_ops->disconnect)
-		old_tag_ops->disconnect(dst);
+	info.tag_ops = old_tag_ops;
+	dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
 
 	return 0;
 
 out_disconnect:
-	/* Revert the new tagger's connection to this tree */
-	if (tag_ops->disconnect)
-		tag_ops->disconnect(dst);
-out_revert:
+	info.tag_ops = tag_ops;
+	dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
 	dst->tag_ops = old_tag_ops;
 
 	return err;
@@ -1318,7 +1317,6 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
 	struct dsa_switch_tree *dst = ds->dst;
 	const struct dsa_device_ops *tag_ops;
 	enum dsa_tag_protocol default_proto;
-	int err;
 
 	/* Find out which protocol the switch would prefer. */
 	default_proto = dsa_get_tag_protocol(dp, master);
@@ -1366,12 +1364,6 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
 		 */
 		dsa_tag_driver_put(tag_ops);
 	} else {
-		if (tag_ops->connect) {
-			err = tag_ops->connect(dst);
-			if (err)
-				return err;
-		}
-
 		dst->tag_ops = tag_ops;
 	}
 

net/dsa/dsa_priv.h

@@ -38,6 +38,7 @@ enum {
 	DSA_NOTIFIER_MTU,
 	DSA_NOTIFIER_TAG_PROTO,
 	DSA_NOTIFIER_TAG_PROTO_CONNECT,
+	DSA_NOTIFIER_TAG_PROTO_DISCONNECT,
 	DSA_NOTIFIER_MRP_ADD,
 	DSA_NOTIFIER_MRP_DEL,
 	DSA_NOTIFIER_MRP_ADD_RING_ROLE,

net/dsa/switch.c

@@ -647,15 +647,58 @@ static int dsa_switch_change_tag_proto(struct dsa_switch *ds,
 	return 0;
 }
 
-static int dsa_switch_connect_tag_proto(struct dsa_switch *ds,
-					struct dsa_notifier_tag_proto_info *info)
+/* We use the same cross-chip notifiers to inform both the tagger side, as well
+ * as the switch side, of connection and disconnection events.
+ * Since ds->tagger_data is owned by the tagger, it isn't a hard error if the
+ * switch side doesn't support connecting to this tagger, and therefore, the
+ * fact that we don't disconnect the tagger side doesn't constitute a memory
+ * leak: the tagger will still operate with persistent per-switch memory, just
+ * with the switch side unconnected to it. What does constitute a hard error is
+ * when the switch side supports connecting but fails.
+ */
+static int
+dsa_switch_connect_tag_proto(struct dsa_switch *ds,
+			     struct dsa_notifier_tag_proto_info *info)
 {
 	const struct dsa_device_ops *tag_ops = info->tag_ops;
+	int err;
+
+	/* Notify the new tagger about the connection to this switch */
+	if (tag_ops->connect) {
+		err = tag_ops->connect(ds);
+		if (err)
+			return err;
+	}
 
 	if (!ds->ops->connect_tag_protocol)
 		return -EOPNOTSUPP;
 
-	return ds->ops->connect_tag_protocol(ds, tag_ops->proto);
+	/* Notify the switch about the connection to the new tagger */
+	err = ds->ops->connect_tag_protocol(ds, tag_ops->proto);
+	if (err) {
+		/* Revert the new tagger's connection to this tree */
+		if (tag_ops->disconnect)
+			tag_ops->disconnect(ds);
+		return err;
+	}
+
+	return 0;
+}
+
+static int
+dsa_switch_disconnect_tag_proto(struct dsa_switch *ds,
+				struct dsa_notifier_tag_proto_info *info)
+{
+	const struct dsa_device_ops *tag_ops = info->tag_ops;
+
+	/* Notify the tagger about the disconnection from this switch */
+	if (tag_ops->disconnect && ds->tagger_data)
+		tag_ops->disconnect(ds);
+
+	/* No need to notify the switch, since it shouldn't have any
+	 * resources to tear down
+	 */
+	return 0;
 }
 
 static int dsa_switch_mrp_add(struct dsa_switch *ds,
@@ -780,6 +823,9 @@ static int dsa_switch_event(struct notifier_block *nb,
 	case DSA_NOTIFIER_TAG_PROTO_CONNECT:
 		err = dsa_switch_connect_tag_proto(ds, info);
 		break;
+	case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
+		err = dsa_switch_disconnect_tag_proto(ds, info);
+		break;
 	case DSA_NOTIFIER_MRP_ADD:
 		err = dsa_switch_mrp_add(ds, info);
 		break;

net/dsa/tag_ocelot_8021q.c

@@ -81,55 +81,34 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb,
 	return skb;
 }
 
-static void ocelot_disconnect(struct dsa_switch_tree *dst)
+static void ocelot_disconnect(struct dsa_switch *ds)
 {
-	struct ocelot_8021q_tagger_private *priv;
-	struct dsa_port *dp;
-
-	list_for_each_entry(dp, &dst->ports, list) {
-		priv = dp->ds->tagger_data;
-
-		if (!priv)
-			continue;
+	struct ocelot_8021q_tagger_private *priv = ds->tagger_data;
 
-		if (priv->xmit_worker)
-			kthread_destroy_worker(priv->xmit_worker);
-
-		kfree(priv);
-		dp->ds->tagger_data = NULL;
-	}
+	kthread_destroy_worker(priv->xmit_worker);
+	kfree(priv);
+	ds->tagger_data = NULL;
 }
 
-static int ocelot_connect(struct dsa_switch_tree *dst)
+static int ocelot_connect(struct dsa_switch *ds)
 {
 	struct ocelot_8021q_tagger_private *priv;
-	struct dsa_port *dp;
 	int err;
 
-	list_for_each_entry(dp, &dst->ports, list) {
-		if (dp->ds->tagger_data)
-			continue;
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
 
-		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-		if (!priv) {
-			err = -ENOMEM;
-			goto out;
-		}
-
-		priv->xmit_worker = kthread_create_worker(0, "felix_xmit");
-		if (IS_ERR(priv->xmit_worker)) {
-			err = PTR_ERR(priv->xmit_worker);
-			goto out;
-		}
-
-		dp->ds->tagger_data = priv;
+	priv->xmit_worker = kthread_create_worker(0, "felix_xmit");
+	if (IS_ERR(priv->xmit_worker)) {
+		err = PTR_ERR(priv->xmit_worker);
+		kfree(priv);
+		return err;
 	}
 
-	return 0;
+	ds->tagger_data = priv;
 
-out:
-	ocelot_disconnect(dst);
-	return err;
+	return 0;
 }
 
 static const struct dsa_device_ops ocelot_8021q_netdev_ops = {

net/dsa/tag_sja1105.c

@@ -741,65 +741,44 @@ static void sja1110_flow_dissect(const struct sk_buff *skb, __be16 *proto,
 	*proto = ((__be16 *)skb->data)[(VLAN_HLEN / 2) - 1];
 }
 
-static void sja1105_disconnect(struct dsa_switch_tree *dst)
+static void sja1105_disconnect(struct dsa_switch *ds)
 {
-	struct sja1105_tagger_private *priv;
-	struct dsa_port *dp;
-
-	list_for_each_entry(dp, &dst->ports, list) {
-		priv = dp->ds->tagger_data;
-
-		if (!priv)
-			continue;
+	struct sja1105_tagger_private *priv = ds->tagger_data;
 
-		if (priv->xmit_worker)
-			kthread_destroy_worker(priv->xmit_worker);
-
-		kfree(priv);
-		dp->ds->priv = NULL;
-	}
+	kthread_destroy_worker(priv->xmit_worker);
+	kfree(priv);
+	ds->tagger_data = NULL;
 }
 
-static int sja1105_connect(struct dsa_switch_tree *dst)
+static int sja1105_connect(struct dsa_switch *ds)
 {
 	struct sja1105_tagger_data *tagger_data;
 	struct sja1105_tagger_private *priv;
 	struct kthread_worker *xmit_worker;
-	struct dsa_port *dp;
 	int err;
 
-	list_for_each_entry(dp, &dst->ports, list) {
-		if (dp->ds->tagger_data)
-			continue;
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
 
-		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-		if (!priv) {
-			err = -ENOMEM;
-			goto out;
-		}
-
-		spin_lock_init(&priv->meta_lock);
-
-		xmit_worker = kthread_create_worker(0, "dsa%d:%d_xmit",
-						    dst->index, dp->ds->index);
-		if (IS_ERR(xmit_worker)) {
-			err = PTR_ERR(xmit_worker);
-			goto out;
-		}
+	spin_lock_init(&priv->meta_lock);
 
-		priv->xmit_worker = xmit_worker;
-		/* Export functions for switch driver use */
-		tagger_data = &priv->data;
-		tagger_data->rxtstamp_get_state = sja1105_rxtstamp_get_state;
-		tagger_data->rxtstamp_set_state = sja1105_rxtstamp_set_state;
-		dp->ds->tagger_data = priv;
+	xmit_worker = kthread_create_worker(0, "dsa%d:%d_xmit",
+					    ds->dst->index, ds->index);
+	if (IS_ERR(xmit_worker)) {
+		err = PTR_ERR(xmit_worker);
+		kfree(priv);
+		return err;
 	}
 
-	return 0;
+	priv->xmit_worker = xmit_worker;
+	/* Export functions for switch driver use */
+	tagger_data = &priv->data;
+	tagger_data->rxtstamp_get_state = sja1105_rxtstamp_get_state;
+	tagger_data->rxtstamp_set_state = sja1105_rxtstamp_set_state;
+	ds->tagger_data = priv;
 
-out:
-	sja1105_disconnect(dst);
-	return err;
+	return 0;
 }
 
 static const struct dsa_device_ops sja1105_netdev_ops = {