Commit 256f8d72 authored by David S. Miller's avatar David S. Miller

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: default avatarDavid S. Miller <davem@davemloft.net>
parents b4bffa4c 7f297314
...@@ -2708,17 +2708,17 @@ static void sja1105_port_deferred_xmit(struct kthread_work *work) ...@@ -2708,17 +2708,17 @@ static void sja1105_port_deferred_xmit(struct kthread_work *work)
static int sja1105_connect_tag_protocol(struct dsa_switch *ds, static int sja1105_connect_tag_protocol(struct dsa_switch *ds,
enum dsa_tag_protocol proto) enum dsa_tag_protocol proto)
{ {
struct sja1105_private *priv = ds->priv;
struct sja1105_tagger_data *tagger_data; struct sja1105_tagger_data *tagger_data;
switch (proto) { if (proto != priv->info->tag_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:
return -EPROTONOSUPPORT; 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, /* The MAXAGE setting belongs to the L2 Forwarding Parameters table,
......
...@@ -70,7 +70,8 @@ struct sja1105_skb_cb { ...@@ -70,7 +70,8 @@ struct sja1105_skb_cb {
static inline struct sja1105_tagger_data * static inline struct sja1105_tagger_data *
sja1105_tagger_data(struct dsa_switch *ds) 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; return ds->tagger_data;
} }
......
...@@ -82,15 +82,14 @@ enum dsa_tag_protocol { ...@@ -82,15 +82,14 @@ enum dsa_tag_protocol {
}; };
struct dsa_switch; struct dsa_switch;
struct dsa_switch_tree;
struct dsa_device_ops { struct dsa_device_ops {
struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev); struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
struct sk_buff *(*rcv)(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, void (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
int *offset); int *offset);
int (*connect)(struct dsa_switch_tree *dst); int (*connect)(struct dsa_switch *ds);
void (*disconnect)(struct dsa_switch_tree *dst); void (*disconnect)(struct dsa_switch *ds);
unsigned int needed_headroom; unsigned int needed_headroom;
unsigned int needed_tailroom; unsigned int needed_tailroom;
const char *name; const char *name;
......
...@@ -248,12 +248,8 @@ static struct dsa_switch_tree *dsa_tree_alloc(int index) ...@@ -248,12 +248,8 @@ static struct dsa_switch_tree *dsa_tree_alloc(int index)
static void dsa_tree_free(struct dsa_switch_tree *dst) static void dsa_tree_free(struct dsa_switch_tree *dst)
{ {
if (dst->tag_ops) { if (dst->tag_ops)
if (dst->tag_ops->disconnect)
dst->tag_ops->disconnect(dst);
dsa_tag_driver_put(dst->tag_ops); dsa_tag_driver_put(dst->tag_ops);
}
list_del(&dst->list); list_del(&dst->list);
kfree(dst); kfree(dst);
} }
...@@ -841,17 +837,29 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds) ...@@ -841,17 +837,29 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds)
} }
connect: connect:
if (tag_ops->connect) {
err = tag_ops->connect(ds);
if (err)
return err;
}
if (ds->ops->connect_tag_protocol) { if (ds->ops->connect_tag_protocol) {
err = ds->ops->connect_tag_protocol(ds, tag_ops->proto); err = ds->ops->connect_tag_protocol(ds, tag_ops->proto);
if (err) { if (err) {
dev_err(ds->dev, dev_err(ds->dev,
"Unable to connect to tag protocol \"%s\": %pe\n", "Unable to connect to tag protocol \"%s\": %pe\n",
tag_ops->name, ERR_PTR(err)); tag_ops->name, ERR_PTR(err));
return err; goto disconnect;
} }
} }
return 0; return 0;
disconnect:
if (tag_ops->disconnect)
tag_ops->disconnect(ds);
return err;
} }
static int dsa_switch_setup(struct dsa_switch *ds) 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, ...@@ -1160,13 +1168,6 @@ static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst,
dst->tag_ops = tag_ops; 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 /* Notify the switches from this tree about the connection
* to the new tagger * to the new tagger
*/ */
...@@ -1176,16 +1177,14 @@ static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst, ...@@ -1176,16 +1177,14 @@ static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst,
goto out_disconnect; goto out_disconnect;
/* Notify the old tagger about the disconnection from this tree */ /* Notify the old tagger about the disconnection from this tree */
if (old_tag_ops->disconnect) info.tag_ops = old_tag_ops;
old_tag_ops->disconnect(dst); dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
return 0; return 0;
out_disconnect: out_disconnect:
/* Revert the new tagger's connection to this tree */ info.tag_ops = tag_ops;
if (tag_ops->disconnect) dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
tag_ops->disconnect(dst);
out_revert:
dst->tag_ops = old_tag_ops; dst->tag_ops = old_tag_ops;
return err; return err;
...@@ -1318,7 +1317,6 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master, ...@@ -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; struct dsa_switch_tree *dst = ds->dst;
const struct dsa_device_ops *tag_ops; const struct dsa_device_ops *tag_ops;
enum dsa_tag_protocol default_proto; enum dsa_tag_protocol default_proto;
int err;
/* Find out which protocol the switch would prefer. */ /* Find out which protocol the switch would prefer. */
default_proto = dsa_get_tag_protocol(dp, master); 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, ...@@ -1366,12 +1364,6 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
*/ */
dsa_tag_driver_put(tag_ops); dsa_tag_driver_put(tag_ops);
} else { } else {
if (tag_ops->connect) {
err = tag_ops->connect(dst);
if (err)
return err;
}
dst->tag_ops = tag_ops; dst->tag_ops = tag_ops;
} }
......
...@@ -38,6 +38,7 @@ enum { ...@@ -38,6 +38,7 @@ enum {
DSA_NOTIFIER_MTU, DSA_NOTIFIER_MTU,
DSA_NOTIFIER_TAG_PROTO, DSA_NOTIFIER_TAG_PROTO,
DSA_NOTIFIER_TAG_PROTO_CONNECT, DSA_NOTIFIER_TAG_PROTO_CONNECT,
DSA_NOTIFIER_TAG_PROTO_DISCONNECT,
DSA_NOTIFIER_MRP_ADD, DSA_NOTIFIER_MRP_ADD,
DSA_NOTIFIER_MRP_DEL, DSA_NOTIFIER_MRP_DEL,
DSA_NOTIFIER_MRP_ADD_RING_ROLE, DSA_NOTIFIER_MRP_ADD_RING_ROLE,
......
...@@ -647,15 +647,58 @@ static int dsa_switch_change_tag_proto(struct dsa_switch *ds, ...@@ -647,15 +647,58 @@ static int dsa_switch_change_tag_proto(struct dsa_switch *ds,
return 0; return 0;
} }
static int dsa_switch_connect_tag_proto(struct dsa_switch *ds, /* We use the same cross-chip notifiers to inform both the tagger side, as well
struct dsa_notifier_tag_proto_info *info) * 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; 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) if (!ds->ops->connect_tag_protocol)
return -EOPNOTSUPP; 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, static int dsa_switch_mrp_add(struct dsa_switch *ds,
...@@ -780,6 +823,9 @@ static int dsa_switch_event(struct notifier_block *nb, ...@@ -780,6 +823,9 @@ static int dsa_switch_event(struct notifier_block *nb,
case DSA_NOTIFIER_TAG_PROTO_CONNECT: case DSA_NOTIFIER_TAG_PROTO_CONNECT:
err = dsa_switch_connect_tag_proto(ds, info); err = dsa_switch_connect_tag_proto(ds, info);
break; break;
case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
err = dsa_switch_disconnect_tag_proto(ds, info);
break;
case DSA_NOTIFIER_MRP_ADD: case DSA_NOTIFIER_MRP_ADD:
err = dsa_switch_mrp_add(ds, info); err = dsa_switch_mrp_add(ds, info);
break; break;
......
...@@ -81,55 +81,34 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb, ...@@ -81,55 +81,34 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb,
return 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 ocelot_8021q_tagger_private *priv = ds->tagger_data;
struct dsa_port *dp;
list_for_each_entry(dp, &dst->ports, list) {
priv = dp->ds->tagger_data;
if (!priv)
continue;
if (priv->xmit_worker) kthread_destroy_worker(priv->xmit_worker);
kthread_destroy_worker(priv->xmit_worker); kfree(priv);
ds->tagger_data = NULL;
kfree(priv);
dp->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 ocelot_8021q_tagger_private *priv;
struct dsa_port *dp;
int err; int err;
list_for_each_entry(dp, &dst->ports, list) { priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (dp->ds->tagger_data) if (!priv)
continue; return -ENOMEM;
priv = kzalloc(sizeof(*priv), GFP_KERNEL); priv->xmit_worker = kthread_create_worker(0, "felix_xmit");
if (!priv) { if (IS_ERR(priv->xmit_worker)) {
err = -ENOMEM; err = PTR_ERR(priv->xmit_worker);
goto out; kfree(priv);
} return err;
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;
} }
return 0; ds->tagger_data = priv;
out: return 0;
ocelot_disconnect(dst);
return err;
} }
static const struct dsa_device_ops ocelot_8021q_netdev_ops = { static const struct dsa_device_ops ocelot_8021q_netdev_ops = {
......
...@@ -741,65 +741,44 @@ static void sja1110_flow_dissect(const struct sk_buff *skb, __be16 *proto, ...@@ -741,65 +741,44 @@ static void sja1110_flow_dissect(const struct sk_buff *skb, __be16 *proto,
*proto = ((__be16 *)skb->data)[(VLAN_HLEN / 2) - 1]; *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 sja1105_tagger_private *priv = ds->tagger_data;
struct dsa_port *dp;
list_for_each_entry(dp, &dst->ports, list) {
priv = dp->ds->tagger_data;
if (!priv)
continue;
if (priv->xmit_worker) kthread_destroy_worker(priv->xmit_worker);
kthread_destroy_worker(priv->xmit_worker); kfree(priv);
ds->tagger_data = NULL;
kfree(priv);
dp->ds->priv = 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_data *tagger_data;
struct sja1105_tagger_private *priv; struct sja1105_tagger_private *priv;
struct kthread_worker *xmit_worker; struct kthread_worker *xmit_worker;
struct dsa_port *dp;
int err; int err;
list_for_each_entry(dp, &dst->ports, list) { priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (dp->ds->tagger_data) if (!priv)
continue; return -ENOMEM;
priv = kzalloc(sizeof(*priv), GFP_KERNEL); spin_lock_init(&priv->meta_lock);
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;
}
priv->xmit_worker = xmit_worker; xmit_worker = kthread_create_worker(0, "dsa%d:%d_xmit",
/* Export functions for switch driver use */ ds->dst->index, ds->index);
tagger_data = &priv->data; if (IS_ERR(xmit_worker)) {
tagger_data->rxtstamp_get_state = sja1105_rxtstamp_get_state; err = PTR_ERR(xmit_worker);
tagger_data->rxtstamp_set_state = sja1105_rxtstamp_set_state; kfree(priv);
dp->ds->tagger_data = 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: return 0;
sja1105_disconnect(dst);
return err;
} }
static const struct dsa_device_ops sja1105_netdev_ops = { static const struct dsa_device_ops sja1105_netdev_ops = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment