net: dsa: tag_8021q: manage RX VLANs dynamically at bridge join/leave time

There has been at least one wasted opportunity for tag_8021q to be used
by a driver:

https://patchwork.ozlabs.org/project/netdev/patch/20200710113611.3398-3-kurt@linutronix.de/#2484272

because of a design decision: the declared purpose of tag_8021q is to
offer source port/switch identification for a tagging driver for packets
coming from a switch with no hardware DSA tagging support. It is not
intended to provide VLAN-based port isolation, because its first user,
sja1105, had another mechanism for bridging domain isolation, the L2
Forwarding Table. So even if 2 ports are in the same VLAN but they are
separated via the L2 Forwarding Table, they will not communicate with
one another. The L2 Forwarding Table is managed by the
sja1105_bridge_join() and sja1105_bridge_leave() methods.

As a consequence, today tag_8021q does not bother too much with hooking
into .port_bridge_join() and .port_bridge_leave() because that would
introduce yet another degree of freedom, it just iterates statically
through all ports of a switch and adds the RX VLAN of one port to all
the others. In this way, whenever .port_bridge_join() is called,
bridging will magically work because the RX VLANs are already installed
everywhere they need to be.

This is not to say that the reason for the change in this patch is to
satisfy the hellcreek and similar use cases, that is merely a nice side
effect. Instead it is to make sja1105 cross-chip links work properly
over a DSA link.

For context, sja1105 today supports a degenerate form of cross-chip
bridging, where the switches are interconnected through their CPU ports
("disjoint trees" topology). There is some code which has been
generalized into dsa_8021q_crosschip_link_{add,del}, but it is not
enough, and frankly it is impossible to build upon that.
Real multi-switch DSA trees, like daisy chains or H trees, which have
actual DSA links, do not work.

The problem is that sja1105 is unlike mv88e6xxx, and does not have a PVT
for cross-chip bridging, which is a table by which the local switch can
select the forwarding domain for packets from a certain ingress switch
ID and source port. The sja1105 switches cannot parse their own DSA
tags, because, well, they don't really have support for DSA tags, it's
all VLANs.

So to make something like cross-chip bridging between sw0p0 and sw1p0 to
work over the sw0p3/sw1p3 DSA link to work with sja1105 in the topology
below:

                         |                                  |
    sw0p0     sw0p1     sw0p2     sw0p3          sw1p3     sw1p2     sw1p1     sw1p0
 [  user ] [  user ] [  cpu  ] [  dsa  ] ---- [  dsa  ] [  cpu  ] [  user ] [  user ]

we need to ask ourselves 2 questions:

(1) how should the L2 Forwarding Table be managed?
(2) how should the VLAN Lookup Table be managed?

i.e. what should prevent packets from going to unwanted ports?

Since as mentioned, there is no PVT, the L2 Forwarding Table only
contains forwarding rules for local ports. So we can say "all user ports
are allowed to forward to all CPU ports and all DSA links".

If we allow forwarding to DSA links unconditionally, this means we must
prevent forwarding using the VLAN Lookup Table. This is in fact
asymmetric with what we do for tag_8021q on ports local to the same
switch, and it matters because now that we are making tag_8021q a core
DSA feature, we need to hook into .crosschip_bridge_join() to add/remove
the tag_8021q VLANs. So for symmetry it makes sense to manage the VLANs
for local forwarding in the same way as cross-chip forwarding.

Note that there is a very precise reason why tag_8021q hooks into
dsa_switch_bridge_join() which acts at the cross-chip notifier level,
and not at a higher level such as dsa_port_bridge_join(). We need to
install the RX VLAN of the newly joining port into the VLAN table of all
the existing ports across the tree that are part of the same bridge, and
the notifier already does the iteration through the switches for us.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

3 files changed, 126 insertions, 38 deletions

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index f201c33980bf..28c4d1107b6d 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -386,6 +386,12 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
 			      const struct dsa_device_ops *tag_ops,
 			      const struct dsa_device_ops *old_tag_ops);
 
+/* tag_8021q.c */
+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);
+
 extern struct list_head dsa_tree_list;
 
 #endif
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 5ece05dfd8f2..38560de99b80 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -90,18 +90,25 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
 				  struct dsa_notifier_bridge_info *info)
 {
 	struct dsa_switch_tree *dst = ds->dst;
+	int err;
 
 	if (dst->index == info->tree_index && ds->index == info->sw_index &&
-	    ds->ops->port_bridge_join)
-		return ds->ops->port_bridge_join(ds, info->port, info->br);
+	    ds->ops->port_bridge_join) {
+		err = ds->ops->port_bridge_join(ds, info->port, info->br);
+		if (err)
+			return err;
+	}
 
 	if ((dst->index != info->tree_index || ds->index != info->sw_index) &&
-	    ds->ops->crosschip_bridge_join)
-		return ds->ops->crosschip_bridge_join(ds, info->tree_index,
-						      info->sw_index,
-						      info->port, info->br);
+	    ds->ops->crosschip_bridge_join) {
+		err = ds->ops->crosschip_bridge_join(ds, info->tree_index,
+						     info->sw_index,
+						     info->port, info->br);
+		if (err)
+			return err;
+	}
 
-	return 0;
+	return dsa_tag_8021q_bridge_join(ds, info);
 }
 
 static int dsa_switch_bridge_leave(struct dsa_switch *ds,
@@ -151,7 +158,8 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 		if (err && err != EOPNOTSUPP)
 			return err;
 	}
-	return 0;
+
+	return dsa_tag_8021q_bridge_leave(ds, info);
 }
 
 /* Matches for all upstream-facing ports (the CPU port and all upstream-facing
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 9785c8497039..0946169033a5 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -137,12 +137,6 @@ static int dsa_8021q_vid_apply(struct dsa_switch *ds, int port, u16 vid,
  *    force all switched traffic to pass through the CPU. So we must also make
  *    the other front-panel ports members of this VID we're adding, albeit
  *    we're not making it their PVID (they'll still have their own).
- *    By the way - just because we're installing the same VID in multiple
- *    switch ports doesn't mean that they'll start to talk to one another, even
- *    while not bridged: the final forwarding decision is still an AND between
- *    the L2 forwarding information (which is limiting forwarding in this case)
- *    and the VLAN-based restrictions (of which there are none in this case,
- *    since all ports are members).
  *  - On TX (ingress from CPU and towards network) we are faced with a problem.
  *    If we were to tag traffic (from within DSA) with the port's pvid, all
  *    would be well, assuming the switch ports were standalone. Frames would
@@ -156,9 +150,10 @@ static int dsa_8021q_vid_apply(struct dsa_switch *ds, int port, u16 vid,
  *    a member of the VID we're tagging the traffic with - the desired one.
  *
  * So at the end, each front-panel port will have one RX VID (also the PVID),
- * the RX VID of all other front-panel ports, and one TX VID. Whereas the CPU
- * port will have the RX and TX VIDs of all front-panel ports, and on top of
- * that, is also tagged-input and tagged-output (VLAN trunk).
+ * the RX VID of all other front-panel ports that are in the same bridge, and
+ * one TX VID. Whereas the CPU port will have the RX and TX VIDs of all
+ * front-panel ports, and on top of that, is also tagged-input and
+ * tagged-output (VLAN trunk).
  *
  *               CPU port                               CPU port
  * +-------------+-----+-------------+    +-------------+-----+-------------+
@@ -176,6 +171,98 @@ static int dsa_8021q_vid_apply(struct dsa_switch *ds, int port, u16 vid,
  * +-+-----+-+-----+-+-----+-+-----+-+    +-+-----+-+-----+-+-----+-+-----+-+
  *   swp0    swp1    swp2    swp3           swp0    swp1    swp2    swp3
  */
+static bool dsa_tag_8021q_bridge_match(struct dsa_switch *ds, int port,
+				       struct dsa_notifier_bridge_info *info)
+{
+	struct dsa_port *dp = dsa_to_port(ds, port);
+
+	/* Don't match on self */
+	if (ds->dst->index == info->tree_index &&
+	    ds->index == info->sw_index &&
+	    port == info->port)
+		return false;
+
+	if (dsa_port_is_user(dp))
+		return dp->bridge_dev == info->br;
+
+	return false;
+}
+
+int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
+			      struct dsa_notifier_bridge_info *info)
+{
+	struct dsa_switch *targeted_ds;
+	u16 targeted_rx_vid;
+	int err, port;
+
+	if (!ds->tag_8021q_ctx)
+		return 0;
+
+	targeted_ds = dsa_switch_find(info->tree_index, info->sw_index);
+	targeted_rx_vid = dsa_8021q_rx_vid(targeted_ds, info->port);
+
+	for (port = 0; port < ds->num_ports; port++) {
+		u16 rx_vid = dsa_8021q_rx_vid(ds, port);
+
+		if (!dsa_tag_8021q_bridge_match(ds, port, info))
+			continue;
+
+		/* Install the RX VID of the targeted port in our VLAN table */
+		err = dsa_8021q_vid_apply(ds, port, targeted_rx_vid,
+					  BRIDGE_VLAN_INFO_UNTAGGED, true);
+		if (err)
+			return err;
+
+		/* Install our RX VID into the targeted port's VLAN table */
+		err = dsa_8021q_vid_apply(targeted_ds, info->port, rx_vid,
+					  BRIDGE_VLAN_INFO_UNTAGGED, true);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
+			       struct dsa_notifier_bridge_info *info)
+{
+	struct dsa_switch *targeted_ds;
+	u16 targeted_rx_vid;
+	int err, port;
+
+	if (!ds->tag_8021q_ctx)
+		return 0;
+
+	targeted_ds = dsa_switch_find(info->tree_index, info->sw_index);
+	targeted_rx_vid = dsa_8021q_rx_vid(targeted_ds, info->port);
+
+	for (port = 0; port < ds->num_ports; port++) {
+		u16 rx_vid = dsa_8021q_rx_vid(ds, port);
+
+		if (!dsa_tag_8021q_bridge_match(ds, port, info))
+			continue;
+
+		/* Remove the RX VID of the targeted port from our VLAN table */
+		err = dsa_8021q_vid_apply(ds, port, targeted_rx_vid,
+					  BRIDGE_VLAN_INFO_UNTAGGED, false);
+		if (err)
+			dev_err(ds->dev,
+				"port %d failed to delete tag_8021q VLAN: %pe\n",
+				port, ERR_PTR(err));
+
+		/* Remove our RX VID from the targeted port's VLAN table */
+		err = dsa_8021q_vid_apply(targeted_ds, info->port, rx_vid,
+					  BRIDGE_VLAN_INFO_UNTAGGED, false);
+		if (err)
+			dev_err(targeted_ds->dev,
+				"port %d failed to delete tag_8021q VLAN: %pe\n",
+				info->port, ERR_PTR(err));
+	}
+
+	return 0;
+}
+
+/* Set up a port's tag_8021q RX and TX VLAN for standalone mode operation */
 static int dsa_8021q_setup_port(struct dsa_switch *ds, int port, bool enabled)
 {
 	struct dsa_8021q_context *ctx = ds->tag_8021q_ctx;
@@ -183,7 +270,7 @@ static int dsa_8021q_setup_port(struct dsa_switch *ds, int port, bool enabled)
 	u16 rx_vid = dsa_8021q_rx_vid(ds, port);
 	u16 tx_vid = dsa_8021q_tx_vid(ds, port);
 	struct net_device *master;
-	int i, err;
+	int err;
 
 	/* The CPU port is implicitly configured by
 	 * configuring the front-panel ports
@@ -198,26 +285,13 @@ static int dsa_8021q_setup_port(struct dsa_switch *ds, int port, bool enabled)
 	 * L2 forwarding rules still take precedence when there are no VLAN
 	 * restrictions, so there are no concerns about leaking traffic.
 	 */
-	for (i = 0; i < ds->num_ports; i++) {
-		u16 flags;
-
-		if (i == upstream)
-			continue;
-		else if (i == port)
-			/* The RX VID is pvid on this port */
-			flags = BRIDGE_VLAN_INFO_UNTAGGED |
-				BRIDGE_VLAN_INFO_PVID;
-		else
-			/* The RX VID is a regular VLAN on all others */
-			flags = BRIDGE_VLAN_INFO_UNTAGGED;
-
-		err = dsa_8021q_vid_apply(ds, i, rx_vid, flags, enabled);
-		if (err) {
-			dev_err(ds->dev,
-				"Failed to apply RX VID %d to port %d: %pe\n",
-				rx_vid, port, ERR_PTR(err));
-			return err;
-		}
+	err = dsa_8021q_vid_apply(ds, port, rx_vid, BRIDGE_VLAN_INFO_UNTAGGED |
+				  BRIDGE_VLAN_INFO_PVID, enabled);
+	if (err) {
+		dev_err(ds->dev,
+			"Failed to apply RX VID %d to port %d: %pe\n",
+			rx_vid, port, ERR_PTR(err));
+		return err;
 	}
 
 	/* CPU port needs to see this port's RX VID