diff mbox series

[RFC,net-next,09/20] net: dsa: tag_8021q: replace the SVL bridging with VLAN-unaware IVL bridging

Message ID 20210818120150.892647-10-vladimir.oltean@nxp.com
State New
Headers show
Series DSA FDB isolation | expand

Commit Message

Vladimir Oltean Aug. 18, 2021, 12:01 p.m. UTC
For VLAN-unaware bridging, tag_8021q uses something perhaps a bit too
tied with the sja1105 switch: each port uses the same pvid which is also
used for standalone operation (a unique one from which the source port
and device ID can be retrieved when packets from that port are forwarded
to the CPU). Since each port has a unique pvid when performing
autonomous forwarding, the switch must be configured for Shared VLAN
Learning (SVL) such that the VLAN ID itself is ignored when performing
FDB lookups. Without SVL, packets would always be flooded.

First of all, to make tag_8021q more palatable to switches which might
not support Shared VLAN Learning, let's just use a common VLAN for all
ports that are under a bridge.

Secondly, using Shared VLAN Learning means that FDB isolation can never
be enforced. But now, when all ports under the same VLAN-unaware bridge
share the same VLAN ID, it can.

The disadvantage is that the CPU port can no longer perform precise
source port identification for these packets. But at least we have a
mechanism which has proven to be adequate for that situation: imprecise
RX, which is what we use for VLAN-aware bridging.

The VLAN ID that VLAN-unaware bridges will use with tag_8021q is the
same one as we were previously using for imprecise TX (bridge TX
forwarding offload). It is already allocated, it is just a matter of
using it.

Note that because now all ports under the same bridge share the same
VLAN, the complexity of performing a tag_8021q bridge join decreases
dramatically. We no longer have to install the RX VLAN of a newly
joining port into the port membership of the existing bridge ports.
The newly joining port just becomes a member of the VLAN corresponding
to that bridge, and the other ports are already members of it. So
forwarding works properly.

This means that we can unhook dsa_tag_8021q_bridge_{join,leave} from the
cross-chip notifier level dsa_switch_bridge_{join,leave}. We can put
these calls directly into the sja1105 driver.

With this new mode of operation, a port controlled by tag_8021q can have
two pvids whereas before it could only have one. The pvid for standalone
operation is different from the pvid used for VLAN-unaware bridging.
This is done, again, so that FDB isolation can be enforced.
Let tag_8021q manage this by deleting the standalone pvid when a port
joins a bridge, and restoring it when it leaves it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c |  32 +++++-
 include/linux/dsa/8021q.h              |  14 ++-
 net/dsa/dsa_priv.h                     |   4 -
 net/dsa/switch.c                       |   4 +-
 net/dsa/tag_8021q.c                    | 137 +++++++++----------------
 5 files changed, 85 insertions(+), 106 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 12a92deb5e5b..1d05a6234a6d 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1987,13 +1987,24 @@  static void sja1105_bridge_stp_state_set(struct dsa_switch *ds, int port,
 static int sja1105_bridge_join(struct dsa_switch *ds, int port,
 			       struct net_device *br, int bridge_num)
 {
-	return sja1105_bridge_member(ds, port, br, true);
+	int rc;
+
+	rc = dsa_tag_8021q_bridge_join(ds, port, br, bridge_num);
+	if (rc)
+		return rc;
+
+	rc = sja1105_bridge_member(ds, port, br, true);
+	if (rc)
+		dsa_tag_8021q_bridge_leave(ds, port, br, bridge_num);
+
+	return rc;
 }
 
 static void sja1105_bridge_leave(struct dsa_switch *ds, int port,
 				 struct net_device *br, int bridge_num)
 {
 	sja1105_bridge_member(ds, port, br, false);
+	dsa_tag_8021q_bridge_leave(ds, port, br, bridge_num);
 }
 
 #define BYTES_PER_KBIT (1000LL / 8)
@@ -2915,6 +2926,21 @@  static int sja1105_port_bridge_flags(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int sja1105_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
+					 struct net_device *br,
+					 int bridge_num)
+{
+	/* Nothing to do, tag_8021q took care of everything */
+	return 0;
+}
+
+static void sja1105_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
+					    struct net_device *br,
+					    int bridge_num)
+{
+	/* Nothing to do, tag_8021q took care of everything */
+}
+
 static void sja1105_teardown_ports(struct sja1105_private *priv)
 {
 	struct dsa_switch *ds = priv->ds;
@@ -3144,8 +3170,8 @@  const struct dsa_switch_ops sja1105_switch_ops = {
 	.tag_8021q_vlan_add	= sja1105_dsa_8021q_vlan_add,
 	.tag_8021q_vlan_del	= sja1105_dsa_8021q_vlan_del,
 	.port_prechangeupper	= sja1105_prechangeupper,
-	.port_bridge_tx_fwd_offload = dsa_tag_8021q_bridge_tx_fwd_offload,
-	.port_bridge_tx_fwd_unoffload = dsa_tag_8021q_bridge_tx_fwd_unoffload,
+	.port_bridge_tx_fwd_offload = sja1105_bridge_tx_fwd_offload,
+	.port_bridge_tx_fwd_unoffload = sja1105_bridge_tx_fwd_unoffload,
 };
 EXPORT_SYMBOL_GPL(sja1105_switch_ops);
 
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index c7fa4a3498fe..29b9c0e195ae 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -30,19 +30,17 @@  int dsa_tag_8021q_register(struct dsa_switch *ds, __be16 proto);
 
 void dsa_tag_8021q_unregister(struct dsa_switch *ds);
 
+int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, int port,
+			      struct net_device *br, int bridge_num);
+
+void dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, int port,
+				struct net_device *br, int bridge_num);
+
 struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev,
 			       u16 tpid, u16 tci);
 
 void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
 
-int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
-					struct net_device *br,
-					int bridge_num);
-
-void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
-					   struct net_device *br,
-					   int bridge_num);
-
 u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num);
 
 u16 dsa_8021q_tx_vid(struct dsa_switch *ds, int port);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 55b908f588ac..417fac51c6e8 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -533,10 +533,6 @@  int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
 void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num);
 
 /* 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);
 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,
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 44d40a267632..26dbd70ebb5e 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -110,7 +110,7 @@  static int dsa_switch_bridge_join(struct dsa_switch *ds,
 			return err;
 	}
 
-	return dsa_tag_8021q_bridge_join(ds, info);
+	return 0;
 }
 
 static int dsa_switch_bridge_leave(struct dsa_switch *ds,
@@ -162,7 +162,7 @@  static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 			return err;
 	}
 
-	return dsa_tag_8021q_bridge_leave(ds, info);
+	return 0;
 }
 
 /* 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 f8f7b7c34e7d..ae94c684961d 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -109,6 +109,15 @@  int dsa_8021q_rx_source_port(u16 vid)
 }
 EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
 
+/* Returns the decoded VBID from the RX VID. */
+static int dsa_tag_8021q_rx_vbid(u16 vid)
+{
+	u16 vbid_hi = (vid & DSA_8021Q_VBID_HI_MASK) >> DSA_8021Q_VBID_HI_SHIFT;
+	u16 vbid_lo = (vid & DSA_8021Q_VBID_LO_MASK) >> DSA_8021Q_VBID_LO_SHIFT;
+
+	return (vbid_hi << 2) | vbid_lo;
+}
+
 bool vid_is_dsa_8021q_rxvlan(u16 vid)
 {
 	return (vid & DSA_8021Q_DIR_MASK) == DSA_8021Q_DIR_RX;
@@ -239,11 +248,17 @@  int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds,
 			if (dsa_is_user_port(ds, port))
 				flags |= BRIDGE_VLAN_INFO_UNTAGGED;
 
+			/* Standalone VLANs are PVIDs */
 			if (vid_is_dsa_8021q_rxvlan(info->vid) &&
 			    dsa_8021q_rx_switch_id(info->vid) == ds->index &&
 			    dsa_8021q_rx_source_port(info->vid) == port)
 				flags |= BRIDGE_VLAN_INFO_PVID;
 
+			/* And bridging VLANs are PVIDs too on user ports */
+			if (dsa_tag_8021q_rx_vbid(info->vid) &&
+			    dsa_is_user_port(ds, port))
+				flags |= BRIDGE_VLAN_INFO_PVID;
+
 			err = dsa_switch_do_tag_8021q_vlan_add(ds, port,
 							       info->vid,
 							       flags);
@@ -322,111 +337,55 @@  int dsa_switch_tag_8021q_vlan_del(struct dsa_switch *ds,
  * +-+-----+-+-----+-+-----+-+-----+-+    +-+-----+-+-----+-+-----+-+-----+-+
  *   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)
+int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, int port,
+			      struct net_device *br, int bridge_num)
 {
 	struct dsa_port *dp = dsa_to_port(ds, port);
+	u16 standalone_vid, bridge_vid;
+	int err;
 
-	/* 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;
-	struct dsa_port *targeted_dp;
-	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_dp = dsa_to_port(targeted_ds, info->port);
-	targeted_rx_vid = dsa_8021q_rx_vid(targeted_ds, info->port);
-
-	for (port = 0; port < ds->num_ports; port++) {
-		struct dsa_port *dp = dsa_to_port(ds, port);
-		u16 rx_vid = dsa_8021q_rx_vid(ds, port);
-
-		if (!dsa_tag_8021q_bridge_match(ds, port, info))
-			continue;
+	/* Delete the standalone VLAN of the port and replace it with a
+	 * bridging VLAN
+	 */
+	standalone_vid = dsa_8021q_rx_vid(ds, port);
+	bridge_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
 
-		/* Install the RX VID of the targeted port in our VLAN table */
-		err = dsa_port_tag_8021q_vlan_add(dp, targeted_rx_vid, true);
-		if (err)
-			return err;
+	dsa_port_tag_8021q_vlan_del(dp, standalone_vid, false);
 
-		/* Install our RX VID into the targeted port's VLAN table */
-		err = dsa_port_tag_8021q_vlan_add(targeted_dp, rx_vid, true);
-		if (err)
-			return err;
+	err = dsa_port_tag_8021q_vlan_add(dp, bridge_vid, true);
+	if (err) {
+		dsa_port_tag_8021q_vlan_add(dp, standalone_vid, false);
+		return err;
 	}
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_join);
 
-int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
-			       struct dsa_notifier_bridge_info *info)
+void dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, int port,
+				struct net_device *br, int bridge_num)
 {
-	struct dsa_switch *targeted_ds;
-	struct dsa_port *targeted_dp;
-	u16 targeted_rx_vid;
-	int port;
-
-	if (!ds->tag_8021q_ctx)
-		return 0;
-
-	targeted_ds = dsa_switch_find(info->tree_index, info->sw_index);
-	targeted_dp = dsa_to_port(targeted_ds, info->port);
-	targeted_rx_vid = dsa_8021q_rx_vid(targeted_ds, info->port);
-
-	for (port = 0; port < ds->num_ports; port++) {
-		struct dsa_port *dp = dsa_to_port(ds, port);
-		u16 rx_vid = dsa_8021q_rx_vid(ds, port);
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	u16 standalone_vid, bridge_vid;
+	int err;
 
-		if (!dsa_tag_8021q_bridge_match(ds, port, info))
-			continue;
+	/* Delete the bridging VLAN of the port and replace it with a
+	 * standalone VLAN
+	 */
+	standalone_vid = dsa_8021q_rx_vid(ds, port);
+	bridge_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
 
-		/* Remove the RX VID of the targeted port from our VLAN table */
-		dsa_port_tag_8021q_vlan_del(dp, targeted_rx_vid, true);
+	dsa_port_tag_8021q_vlan_del(dp, bridge_vid, true);
 
-		/* Remove our RX VID from the targeted port's VLAN table */
-		dsa_port_tag_8021q_vlan_del(targeted_dp, rx_vid, true);
+	err = dsa_port_tag_8021q_vlan_add(dp, standalone_vid, false);
+	if (err) {
+		dev_err(ds->dev,
+			"Failed to delete tag_8021q standalone VLAN %d from port %d: %pe\n",
+			standalone_vid, port, ERR_PTR(err));
+		dsa_port_tag_8021q_vlan_add(dp, bridge_vid, true);
 	}
-
-	return 0;
-}
-
-int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
-					struct net_device *br,
-					int bridge_num)
-{
-	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
-
-	return dsa_port_tag_8021q_vlan_add(dsa_to_port(ds, port), tx_vid,
-					   true);
-}
-EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
-
-void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
-					   struct net_device *br,
-					   int bridge_num)
-{
-	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
-
-	dsa_port_tag_8021q_vlan_del(dsa_to_port(ds, port), tx_vid, true);
 }
-EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_unoffload);
+EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_leave);
 
 /* Set up a port's tag_8021q RX and TX VLAN for standalone mode operation */
 static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)