Commit c64b9c05 authored by Vladimir Oltean's avatar Vladimir Oltean Committed by David S. Miller

net: dsa: tag_8021q: add proper cross-chip notifier support

The big problem which mandates cross-chip notifiers for tag_8021q is
this:

                                             |
    sw0p0     sw0p1     sw0p2     sw0p3     sw0p4
 [  user ] [  user ] [  user ] [  dsa  ] [  cpu  ]
                                   |
                                   +---------+
                                             |
    sw1p0     sw1p1     sw1p2     sw1p3     sw1p4
 [  user ] [  user ] [  user ] [  dsa  ] [  dsa  ]
                                   |
                                   +---------+
                                             |
    sw2p0     sw2p1     sw2p2     sw2p3     sw2p4
 [  user ] [  user ] [  user ] [  dsa  ] [  dsa  ]

When the user runs:

ip link add br0 type bridge
ip link set sw0p0 master br0
ip link set sw2p0 master br0

It doesn't work.

This is because dsa_8021q_crosschip_bridge_join() assumes that "ds" and
"other_ds" are at most 1 hop away from each other, so it is sufficient
to add the RX VLAN of {ds, port} into {other_ds, other_port} and vice
versa and presto, the cross-chip link works. When there is another
switch in the middle, such as in this case switch 1 with its DSA links
sw1p3 and sw1p4, somebody needs to tell it about these VLANs too.

Which is exactly why the problem is quadratic: when a port joins a
bridge, for each port in the tree that's already in that same bridge we
notify a tag_8021q VLAN addition of that port's RX VLAN to the entire
tree. It is a very complicated web of VLANs.

It must be mentioned that currently we install tag_8021q VLANs on too
many ports (DSA links - to be precise, on all of them). For example,
when sw2p0 joins br0, and assuming sw1p0 was part of br0 too, we add the
RX VLAN of sw2p0 on the DSA links of switch 0 too, even though there
isn't any port of switch 0 that is a member of br0 (at least yet).
In theory we could notify only the switches which sit in between the
port joining the bridge and the port reacting to that bridge_join event.
But in practice that is impossible, because of the way 'link' properties
are described in the device tree. The DSA bindings require DT writers to
list out not only the real/physical DSA links, but in fact the entire
routing table, like for example switch 0 above will have:

	sw0p3: port@3 {
		link = <&sw1p4 &sw2p4>;
	};

This was done because:

/* TODO: ideally DSA ports would have a single dp->link_dp member,
 * and no dst->rtable nor this struct dsa_link would be needed,
 * but this would require some more complex tree walking,
 * so keep it stupid at the moment and list them all.
 */

but it is a perfect example of a situation where too much information is
actively detrimential, because we are now in the position where we
cannot distinguish a real DSA link from one that is put there to avoid
the 'complex tree walking'. And because DT is ABI, there is not much we
can change.

And because we do not know which DSA links are real and which ones
aren't, we can't really know if DSA switch A is in the data path between
switches B and C, in the general case.

So this is why tag_8021q RX VLANs are added on all DSA links, and
probably why it will never change.

On the other hand, at least the number of additions/deletions is well
balanced, and this means that once we implement reference counting at
the cross-chip notifier level a la fdb/mdb, there is absolutely zero
need for a struct dsa_8021q_crosschip_link, it's all self-managing.

In fact, with the tag_8021q notifiers emitted from the bridge join
notifiers, it becomes so generic that sja1105 does not need to do
anything anymore, we can just delete its implementation of the
.crosschip_bridge_{join,leave} methods.

Among other things we can simply delete is the home-grown implementation
of sja1105_notify_crosschip_switches(). The reason why that is wrong is
because it is not quadratic - it only covers remote switches to which we
have a cross-chip bridging link and that does not cover in-between
switches. This deletion is part of the same patch because sja1105 used
to poke deep inside the guts of the tag_8021q context in order to do
that. Because the cross-chip links went away, so needs the sja1105 code.

Last but not least, dsa_8021q_setup_port() is simplified (and also
renamed). Because our TAG_8021Q_VLAN_ADD notifier is designed to react
on the CPU port too, the four dsa_8021q_vid_apply() calls:
- 1 for RX VLAN on user port
- 1 for the user port's RX VLAN on the CPU port
- 1 for TX VLAN on user port
- 1 for the user port's TX VLAN on the CPU port

now get squashed into only 2 notifier calls via
dsa_port_tag_8021q_vlan_add.

And because the notifiers to add and to delete a tag_8021q VLAN are
distinct, now we finally break up the port setup and teardown into
separate functions instead of relying on a "bool enabled" flag which
tells us what to do. Arguably it should have been this way from the
get go.
Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent e19cc13c
......@@ -1990,61 +1990,6 @@ static int sja1105_pvid_apply(struct sja1105_private *priv, int port, u16 pvid)
&mac[port], true);
}
static int sja1105_crosschip_bridge_join(struct dsa_switch *ds,
int tree_index, int sw_index,
int other_port, struct net_device *br)
{
struct dsa_switch *other_ds = dsa_switch_find(tree_index, sw_index);
int port, rc;
if (other_ds->ops != &sja1105_switch_ops)
return 0;
for (port = 0; port < ds->num_ports; port++) {
if (!dsa_is_user_port(ds, port))
continue;
if (dsa_to_port(ds, port)->bridge_dev != br)
continue;
rc = dsa_8021q_crosschip_bridge_join(ds, port, other_ds,
other_port);
if (rc)
return rc;
rc = dsa_8021q_crosschip_bridge_join(other_ds, other_port,
ds, port);
if (rc)
return rc;
}
return 0;
}
static void sja1105_crosschip_bridge_leave(struct dsa_switch *ds,
int tree_index, int sw_index,
int other_port,
struct net_device *br)
{
struct dsa_switch *other_ds = dsa_switch_find(tree_index, sw_index);
int port;
if (other_ds->ops != &sja1105_switch_ops)
return;
for (port = 0; port < ds->num_ports; port++) {
if (!dsa_is_user_port(ds, port))
continue;
if (dsa_to_port(ds, port)->bridge_dev != br)
continue;
dsa_8021q_crosschip_bridge_leave(ds, port, other_ds,
other_port);
dsa_8021q_crosschip_bridge_leave(other_ds, other_port,
ds, port);
}
}
static enum dsa_tag_protocol
sja1105_get_tag_protocol(struct dsa_switch *ds, int port,
enum dsa_tag_protocol mp)
......@@ -2135,11 +2080,6 @@ static int sja1105_commit_vlans(struct sja1105_private *priv,
return 0;
}
struct sja1105_crosschip_switch {
struct list_head list;
struct dsa_8021q_context *other_ctx;
};
static int sja1105_commit_pvid(struct sja1105_private *priv)
{
struct sja1105_bridge_vlan *v;
......@@ -2205,59 +2145,7 @@ sja1105_build_dsa_8021q_vlans(struct sja1105_private *priv,
return 0;
}
static int sja1105_build_vlan_table(struct sja1105_private *priv, bool notify);
static int sja1105_notify_crosschip_switches(struct sja1105_private *priv)
{
struct dsa_8021q_context *ctx = priv->ds->tag_8021q_ctx;
struct sja1105_crosschip_switch *s, *pos;
struct list_head crosschip_switches;
struct dsa_8021q_crosschip_link *c;
int rc = 0;
INIT_LIST_HEAD(&crosschip_switches);
list_for_each_entry(c, &ctx->crosschip_links, list) {
bool already_added = false;
list_for_each_entry(s, &crosschip_switches, list) {
if (s->other_ctx == c->other_ctx) {
already_added = true;
break;
}
}
if (already_added)
continue;
s = kzalloc(sizeof(*s), GFP_KERNEL);
if (!s) {
dev_err(priv->ds->dev, "Failed to allocate memory\n");
rc = -ENOMEM;
goto out;
}
s->other_ctx = c->other_ctx;
list_add(&s->list, &crosschip_switches);
}
list_for_each_entry(s, &crosschip_switches, list) {
struct sja1105_private *other_priv = s->other_ctx->ds->priv;
rc = sja1105_build_vlan_table(other_priv, false);
if (rc)
goto out;
}
out:
list_for_each_entry_safe(s, pos, &crosschip_switches, list) {
list_del(&s->list);
kfree(s);
}
return rc;
}
static int sja1105_build_vlan_table(struct sja1105_private *priv, bool notify)
static int sja1105_build_vlan_table(struct sja1105_private *priv)
{
struct sja1105_vlan_lookup_entry *new_vlan;
struct sja1105_table *table;
......@@ -2296,12 +2184,6 @@ static int sja1105_build_vlan_table(struct sja1105_private *priv, bool notify)
if (rc)
goto out;
if (notify) {
rc = sja1105_notify_crosschip_switches(priv);
if (rc)
goto out;
}
out:
kfree(new_vlan);
......@@ -2389,7 +2271,7 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
l2_lookup_params = table->entries;
l2_lookup_params->shared_learn = !priv->vlan_aware;
rc = sja1105_build_vlan_table(priv, false);
rc = sja1105_build_vlan_table(priv);
if (rc)
return rc;
......@@ -2485,7 +2367,7 @@ static int sja1105_vlan_add(struct dsa_switch *ds, int port,
if (!vlan_table_changed)
return 0;
return sja1105_build_vlan_table(priv, true);
return sja1105_build_vlan_table(priv);
}
static int sja1105_vlan_del(struct dsa_switch *ds, int port,
......@@ -2502,7 +2384,7 @@ static int sja1105_vlan_del(struct dsa_switch *ds, int port,
if (!vlan_table_changed)
return 0;
return sja1105_build_vlan_table(priv, true);
return sja1105_build_vlan_table(priv);
}
static int sja1105_dsa_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid,
......@@ -2515,7 +2397,7 @@ static int sja1105_dsa_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid,
if (rc <= 0)
return rc;
return sja1105_build_vlan_table(priv, true);
return sja1105_build_vlan_table(priv);
}
static int sja1105_dsa_8021q_vlan_del(struct dsa_switch *ds, int port, u16 vid)
......@@ -2527,7 +2409,7 @@ static int sja1105_dsa_8021q_vlan_del(struct dsa_switch *ds, int port, u16 vid)
if (!rc)
return 0;
return sja1105_build_vlan_table(priv, true);
return sja1105_build_vlan_table(priv);
}
/* The programming model for the SJA1105 switch is "all-at-once" via static
......@@ -3132,8 +3014,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
.cls_flower_add = sja1105_cls_flower_add,
.cls_flower_del = sja1105_cls_flower_del,
.cls_flower_stats = sja1105_cls_flower_stats,
.crosschip_bridge_join = sja1105_crosschip_bridge_join,
.crosschip_bridge_leave = sja1105_crosschip_bridge_leave,
.devlink_info_get = sja1105_devlink_info_get,
.tag_8021q_vlan_add = sja1105_dsa_8021q_vlan_add,
.tag_8021q_vlan_del = sja1105_dsa_8021q_vlan_del,
......
......@@ -11,19 +11,17 @@
struct dsa_switch;
struct sk_buff;
struct net_device;
struct dsa_8021q_context;
struct dsa_8021q_crosschip_link {
struct dsa_tag_8021q_vlan {
struct list_head list;
int port;
struct dsa_8021q_context *other_ctx;
int other_port;
u16 vid;
refcount_t refcount;
};
struct dsa_8021q_context {
struct dsa_switch *ds;
struct list_head crosschip_links;
struct list_head vlans;
/* EtherType of RX VID, used for filtering on master interface */
__be16 proto;
};
......@@ -32,14 +30,6 @@ int dsa_tag_8021q_register(struct dsa_switch *ds, __be16 proto);
void dsa_tag_8021q_unregister(struct dsa_switch *ds);
int dsa_8021q_crosschip_bridge_join(struct dsa_switch *ds, int port,
struct dsa_switch *other_ds,
int other_port);
int dsa_8021q_crosschip_bridge_leave(struct dsa_switch *ds, int port,
struct dsa_switch *other_ds,
int other_port);
struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev,
u16 tpid, u16 tci);
......
......@@ -39,6 +39,8 @@ enum {
DSA_NOTIFIER_MRP_DEL,
DSA_NOTIFIER_MRP_ADD_RING_ROLE,
DSA_NOTIFIER_MRP_DEL_RING_ROLE,
DSA_NOTIFIER_TAG_8021Q_VLAN_ADD,
DSA_NOTIFIER_TAG_8021Q_VLAN_DEL,
};
/* DSA_NOTIFIER_AGEING_TIME */
......@@ -113,6 +115,14 @@ struct dsa_notifier_mrp_ring_role_info {
int port;
};
/* DSA_NOTIFIER_TAG_8021Q_VLAN_* */
struct dsa_notifier_tag_8021q_vlan_info {
int tree_index;
int sw_index;
int port;
u16 vid;
};
struct dsa_switchdev_event_work {
struct dsa_switch *ds;
int port;
......@@ -253,6 +263,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);
extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
......@@ -391,6 +403,10 @@ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
struct dsa_notifier_bridge_info *info);
int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
struct dsa_notifier_bridge_info *info);
int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds,
struct dsa_notifier_tag_8021q_vlan_info *info);
int dsa_switch_tag_8021q_vlan_del(struct dsa_switch *ds,
struct dsa_notifier_tag_8021q_vlan_info *info);
extern struct list_head dsa_tree_list;
......
......@@ -1217,3 +1217,31 @@ void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr)
if (err)
pr_err("DSA: failed to notify DSA_NOTIFIER_HSR_LEAVE\n");
}
int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid)
{
struct dsa_notifier_tag_8021q_vlan_info info = {
.tree_index = dp->ds->dst->index,
.sw_index = dp->ds->index,
.port = dp->index,
.vid = vid,
};
return dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info);
}
void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid)
{
struct dsa_notifier_tag_8021q_vlan_info info = {
.tree_index = dp->ds->dst->index,
.sw_index = dp->ds->index,
.port = dp->index,
.vid = vid,
};
int err;
err = dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
if (err)
pr_err("DSA: failed to notify tag_8021q VLAN deletion: %pe\n",
ERR_PTR(err));
}
......@@ -734,6 +734,12 @@ static int dsa_switch_event(struct notifier_block *nb,
case DSA_NOTIFIER_MRP_DEL_RING_ROLE:
err = dsa_switch_mrp_del_ring_role(ds, info);
break;
case DSA_NOTIFIER_TAG_8021Q_VLAN_ADD:
err = dsa_switch_tag_8021q_vlan_add(ds, info);
break;
case DSA_NOTIFIER_TAG_8021Q_VLAN_DEL:
err = dsa_switch_tag_8021q_vlan_del(ds, info);
break;
default:
err = -EOPNOTSUPP;
break;
......
This diff is collapsed.
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