All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 net-next 00/10] DSA FDB isolation
@ 2022-02-25  9:22 Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 01/10] net: dsa: tag_8021q: replace the SVL bridging with VLAN-unaware IVL bridging Vladimir Oltean
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

There are use cases which need FDB isolation between standalone ports
and bridged ports, as well as isolation between ports of different
bridges. Most of these use cases are a result of the fact that packets
can now be partially forwarded by the software bridge, so one port might
need to send a packet to the CPU but its FDB lookup will see that it can
forward it directly to a bridge port where that packet was autonomously
learned. So the source port will attempt to shortcircuit the CPU and
forward autonomously, which it can't due to the forwarding isolation we
have in place. So we will have packet drops instead of proper operation.

Additionally, before DSA can implement IFF_UNICAST_FLT for standalone
ports, we must have control over which database we install FDB entries
corresponding to port MAC addresses in. We don't want to hinder the
operation of the bridging layer.

DSA does not have a driver API that encourages FDB isolation, so this
needs to be created. The basis for this is a new struct dsa_db which
annotates each FDB and MDB entry with the database it belongs to.

The sja1105 and felix drivers are modified to observe the dsa_db
argument, and therefore, enforce the FDB isolation.

Compared to the previous RFC patch series from August:
https://patchwork.kernel.org/project/netdevbpf/cover/20210818120150.892647-1-vladimir.oltean@nxp.com/

what is different is that I stopped trying to make SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE
blocking, instead I'm making use of the fact that DSA waits for switchdev FDB work
items to finish before a port leaves the bridge. This is possible since:
https://patchwork.kernel.org/project/netdevbpf/patch/20211024171757.3753288-7-vladimir.oltean@nxp.com/

Additionally, v2 is also rebased over the DSA LAG FDB work.

Vladimir Oltean (10):
  net: dsa: tag_8021q: replace the SVL bridging with VLAN-unaware IVL
    bridging
  net: dsa: tag_8021q: add support for imprecise RX based on the VBID
  docs: net: dsa: sja1105: document limitations of tc-flower rule VLAN
    awareness
  net: dsa: felix: delete workarounds present due to SVL tag_8021q
    bridging
  net: dsa: tag_8021q: merge RX and TX VLANs
  net: dsa: tag_8021q: rename dsa_8021q_bridge_tx_fwd_offload_vid
  net: dsa: request drivers to perform FDB isolation
  net: dsa: pass extack to .port_bridge_join driver methods
  net: dsa: sja1105: enforce FDB isolation
  net: mscc: ocelot: enforce FDB isolation when VLAN-unaware

 Documentation/networking/dsa/sja1105.rst |  27 ++
 drivers/net/dsa/b53/b53_common.c         |  14 +-
 drivers/net/dsa/b53/b53_priv.h           |  14 +-
 drivers/net/dsa/dsa_loop.c               |   3 +-
 drivers/net/dsa/hirschmann/hellcreek.c   |   9 +-
 drivers/net/dsa/lan9303-core.c           |  16 +-
 drivers/net/dsa/lantiq_gswip.c           |   9 +-
 drivers/net/dsa/microchip/ksz9477.c      |  12 +-
 drivers/net/dsa/microchip/ksz_common.c   |   9 +-
 drivers/net/dsa/microchip/ksz_common.h   |   9 +-
 drivers/net/dsa/mt7530.c                 |  15 +-
 drivers/net/dsa/mv88e6xxx/chip.c         |  18 +-
 drivers/net/dsa/ocelot/felix.c           | 221 +++++++++-------
 drivers/net/dsa/qca8k.c                  |  15 +-
 drivers/net/dsa/realtek/rtl8366rb.c      |   3 +-
 drivers/net/dsa/sja1105/sja1105_main.c   |  94 ++++---
 drivers/net/dsa/sja1105/sja1105_vl.c     |  16 +-
 drivers/net/dsa/xrs700x/xrs700x.c        |   3 +-
 drivers/net/ethernet/mscc/ocelot.c       | 200 ++++++++++++--
 drivers/net/ethernet/mscc/ocelot.h       |   5 +-
 drivers/net/ethernet/mscc/ocelot_mrp.c   |   8 +-
 drivers/net/ethernet/mscc/ocelot_net.c   |  66 ++++-
 include/linux/dsa/8021q.h                |  26 +-
 include/net/dsa.h                        |  48 +++-
 include/soc/mscc/ocelot.h                |  31 ++-
 net/dsa/dsa_priv.h                       |   8 +-
 net/dsa/port.c                           |  76 +++++-
 net/dsa/switch.c                         | 109 +++++---
 net/dsa/tag_8021q.c                      | 319 +++++++++--------------
 net/dsa/tag_ocelot_8021q.c               |   4 +-
 net/dsa/tag_sja1105.c                    |  28 +-
 31 files changed, 925 insertions(+), 510 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 01/10] net: dsa: tag_8021q: replace the SVL bridging with VLAN-unaware IVL bridging
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 02/10] net: dsa: tag_8021q: add support for imprecise RX based on the VBID Vladimir Oltean
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

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, since FDB
lookup in the source port's VLAN would never find any entry.

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 the same bridge.

Secondly, using Shared VLAN Learning means that FDB isolation can never
be enforced. But if 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 (dsa_find_designated_bridge_port_by_vid), which is what we use for
termination on VLAN-aware bridges.

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 from when
they joined the bridge themselves. 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 |   4 +-
 drivers/net/dsa/sja1105/sja1105_vl.c   |  15 ++-
 include/linux/dsa/8021q.h              |  12 +--
 net/dsa/dsa_priv.h                     |   4 -
 net/dsa/switch.c                       |   4 +-
 net/dsa/tag_8021q.c                    | 132 +++++++++----------------
 6 files changed, 70 insertions(+), 101 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index b513713be610..1e43a7ef0f2e 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2083,7 +2083,7 @@ static int sja1105_bridge_join(struct dsa_switch *ds, int port,
 	if (rc)
 		return rc;
 
-	rc = dsa_tag_8021q_bridge_tx_fwd_offload(ds, port, bridge);
+	rc = dsa_tag_8021q_bridge_join(ds, port, bridge);
 	if (rc) {
 		sja1105_bridge_member(ds, port, bridge, false);
 		return rc;
@@ -2097,7 +2097,7 @@ static int sja1105_bridge_join(struct dsa_switch *ds, int port,
 static void sja1105_bridge_leave(struct dsa_switch *ds, int port,
 				 struct dsa_bridge bridge)
 {
-	dsa_tag_8021q_bridge_tx_fwd_unoffload(ds, port, bridge);
+	dsa_tag_8021q_bridge_leave(ds, port, bridge);
 	sja1105_bridge_member(ds, port, bridge, false);
 }
 
diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index f5dca6a9b0f9..14e6dd7fb103 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -296,6 +296,19 @@ static bool sja1105_vl_key_lower(struct sja1105_vl_lookup_entry *a,
 	return false;
 }
 
+/* FIXME: this should change when the bridge upper of the port changes. */
+static u16 sja1105_port_get_tag_8021q_vid(struct dsa_port *dp)
+{
+	unsigned long bridge_num;
+
+	if (!dp->bridge)
+		return dsa_tag_8021q_rx_vid(dp);
+
+	bridge_num = dsa_port_bridge_num_get(dp);
+
+	return dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+}
+
 static int sja1105_init_virtual_links(struct sja1105_private *priv,
 				      struct netlink_ext_ack *extack)
 {
@@ -395,7 +408,7 @@ static int sja1105_init_virtual_links(struct sja1105_private *priv,
 				vl_lookup[k].vlanprior = rule->key.vl.pcp;
 			} else {
 				struct dsa_port *dp = dsa_to_port(priv->ds, port);
-				u16 vid = dsa_tag_8021q_rx_vid(dp);
+				u16 vid = sja1105_port_get_tag_8021q_vid(dp);
 
 				vl_lookup[k].vlanid = vid;
 				vl_lookup[k].vlanprior = 0;
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index 939a1beaddf7..f47f227baa27 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -32,17 +32,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 dsa_bridge bridge);
+
+void dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, int port,
+				struct dsa_bridge bridge);
+
 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 dsa_bridge bridge);
-
-void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
-					   struct dsa_bridge bridge);
-
 u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
 
 u16 dsa_tag_8021q_tx_vid(const struct dsa_port *dp);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 1ba93afdc874..7a1c98581f53 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -522,10 +522,6 @@ struct dsa_bridge *dsa_tree_bridge_find(struct dsa_switch_tree *dst,
 					const struct net_device *br);
 
 /* 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 0c2961cbc105..eb38beb10147 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_sync_vlan_filtering(struct dsa_switch *ds,
@@ -186,7 +186,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 114f663332d0..c6555003f5df 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -110,6 +110,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;
@@ -244,11 +253,17 @@ int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds,
 			if (dsa_port_is_user(dp))
 				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) == dp->index)
 				flags |= BRIDGE_VLAN_INFO_PVID;
 
+			/* And bridging VLANs are PVIDs too on user ports */
+			if (dsa_tag_8021q_rx_vbid(info->vid) &&
+			    dsa_port_is_user(dp))
+				flags |= BRIDGE_VLAN_INFO_PVID;
+
 			err = dsa_port_do_tag_8021q_vlan_add(dp, info->vid,
 							     flags);
 			if (err)
@@ -326,107 +341,52 @@ int dsa_switch_tag_8021q_vlan_del(struct dsa_switch *ds,
  * +-+-----+-+-----+-+-----+-+-----+-+    +-+-----+-+-----+-+-----+-+-----+-+
  *   swp0    swp1    swp2    swp3           swp0    swp1    swp2    swp3
  */
-static bool
-dsa_port_tag_8021q_bridge_match(struct dsa_port *dp,
-				struct dsa_notifier_bridge_info *info)
+int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, int port,
+			      struct dsa_bridge bridge)
 {
-	/* Don't match on self */
-	if (dp->ds->dst->index == info->tree_index &&
-	    dp->ds->index == info->sw_index &&
-	    dp->index == info->port)
-		return false;
-
-	if (dsa_port_is_user(dp))
-		return dsa_port_offloads_bridge(dp, &info->bridge);
-
-	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;
-	struct dsa_port *dp;
-	u16 targeted_rx_vid;
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	u16 standalone_vid, bridge_vid;
 	int err;
 
-	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_tag_8021q_rx_vid(targeted_dp);
-
-	dsa_switch_for_each_port(dp, ds) {
-		u16 rx_vid = dsa_tag_8021q_rx_vid(dp);
-
-		if (!dsa_port_tag_8021q_bridge_match(dp, info))
-			continue;
+	/* Delete the standalone VLAN of the port and replace it with a
+	 * bridging VLAN
+	 */
+	standalone_vid = dsa_tag_8021q_rx_vid(dp);
+	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;
+	err = dsa_port_tag_8021q_vlan_add(dp, bridge_vid, true);
+	if (err)
+		return err;
 
-		/* 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;
-	}
+	dsa_port_tag_8021q_vlan_del(dp, standalone_vid, false);
 
 	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 dsa_bridge bridge)
 {
-	struct dsa_switch *targeted_ds;
-	struct dsa_port *targeted_dp;
-	struct dsa_port *dp;
-	u16 targeted_rx_vid;
-
-	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_tag_8021q_rx_vid(targeted_dp);
-
-	dsa_switch_for_each_port(dp, ds) {
-		u16 rx_vid = dsa_tag_8021q_rx_vid(dp);
-
-		if (!dsa_port_tag_8021q_bridge_match(dp, info))
-			continue;
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	u16 standalone_vid, bridge_vid;
+	int err;
 
-		/* Remove the RX VID of the targeted port from our VLAN table */
-		dsa_port_tag_8021q_vlan_del(dp, targeted_rx_vid, true);
+	/* Delete the bridging VLAN of the port and replace it with a
+	 * standalone VLAN
+	 */
+	standalone_vid = dsa_tag_8021q_rx_vid(dp);
+	bridge_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
 
-		/* 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));
 	}
 
-	return 0;
-}
-
-int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
-					struct dsa_bridge bridge)
-{
-	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 dsa_bridge bridge)
-{
-	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);
+	dsa_port_tag_8021q_vlan_del(dp, bridge_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)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 02/10] net: dsa: tag_8021q: add support for imprecise RX based on the VBID
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 01/10] net: dsa: tag_8021q: replace the SVL bridging with VLAN-unaware IVL bridging Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 03/10] docs: net: dsa: sja1105: document limitations of tc-flower rule VLAN awareness Vladimir Oltean
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

The sja1105 switch can't populate the PORT field of the tag_8021q header
when sending a frame to the CPU with a non-zero VBID.

Similar to dsa_find_designated_bridge_port_by_vid() which performs
imprecise RX for VLAN-aware bridges, let's introduce a helper in
tag_8021q for performing imprecise RX based on the VLAN that it has
allocated for a VLAN-unaware bridge.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/linux/dsa/8021q.h  |  6 +++++-
 net/dsa/tag_8021q.c        | 38 ++++++++++++++++++++++++++++++++++++--
 net/dsa/tag_ocelot_8021q.c |  2 +-
 net/dsa/tag_sja1105.c      | 22 +++++++++++++---------
 4 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index f47f227baa27..92f5243b841e 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -41,7 +41,11 @@ void dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, int port,
 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);
+void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id,
+		   int *vbid);
+
+struct net_device *dsa_tag_8021q_find_port_by_vbid(struct net_device *master,
+						   int vbid);
 
 u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
 
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index c6555003f5df..1cf245a6f18e 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -32,7 +32,7 @@
  * VBID - { VID[9], VID[5:4] }:
  *	Virtual bridge ID. If between 1 and 7, packet targets the broadcast
  *	domain of a bridge. If transmitted as zero, packet targets a single
- *	port. Field only valid on transmit, must be ignored on receive.
+ *	port.
  *
  * PORT - VID[3:0]:
  *	Index of switch port. Must be between 0 and 15.
@@ -533,7 +533,37 @@ struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev,
 }
 EXPORT_SYMBOL_GPL(dsa_8021q_xmit);
 
-void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id)
+struct net_device *dsa_tag_8021q_find_port_by_vbid(struct net_device *master,
+						   int vbid)
+{
+	struct dsa_port *cpu_dp = master->dsa_ptr;
+	struct dsa_switch_tree *dst = cpu_dp->dst;
+	struct dsa_port *dp;
+
+	if (WARN_ON(!vbid))
+		return NULL;
+
+	dsa_tree_for_each_user_port(dp, dst) {
+		if (!dp->bridge)
+			continue;
+
+		if (dp->stp_state != BR_STATE_LEARNING &&
+		    dp->stp_state != BR_STATE_FORWARDING)
+			continue;
+
+		if (dp->cpu_dp != cpu_dp)
+			continue;
+
+		if (dsa_port_bridge_num_get(dp) == vbid)
+			return dp->slave;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(dsa_tag_8021q_find_port_by_vbid);
+
+void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id,
+		   int *vbid)
 {
 	u16 vid, tci;
 
@@ -550,6 +580,10 @@ void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id)
 
 	*source_port = dsa_8021q_rx_source_port(vid);
 	*switch_id = dsa_8021q_rx_switch_id(vid);
+
+	if (vbid)
+		*vbid = dsa_tag_8021q_rx_vbid(vid);
+
 	skb->priority = (tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
 }
 EXPORT_SYMBOL_GPL(dsa_8021q_rcv);
diff --git a/net/dsa/tag_ocelot_8021q.c b/net/dsa/tag_ocelot_8021q.c
index bd6f1d0e5372..1144a87ad0db 100644
--- a/net/dsa/tag_ocelot_8021q.c
+++ b/net/dsa/tag_ocelot_8021q.c
@@ -77,7 +77,7 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb,
 {
 	int src_port, switch_id;
 
-	dsa_8021q_rcv(skb, &src_port, &switch_id);
+	dsa_8021q_rcv(skb, &src_port, &switch_id, NULL);
 
 	skb->dev = dsa_master_find_slave(netdev, switch_id, src_port);
 	if (!skb->dev)
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 72d5e0ef8dcf..9c5c00980b06 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -509,7 +509,7 @@ static bool sja1110_skb_has_inband_control_extension(const struct sk_buff *skb)
  * packet.
  */
 static void sja1105_vlan_rcv(struct sk_buff *skb, int *source_port,
-			     int *switch_id, u16 *vid)
+			     int *switch_id, int *vbid, u16 *vid)
 {
 	struct vlan_ethhdr *hdr = (struct vlan_ethhdr *)skb_mac_header(skb);
 	u16 vlan_tci;
@@ -519,8 +519,8 @@ static void sja1105_vlan_rcv(struct sk_buff *skb, int *source_port,
 	else
 		vlan_tci = ntohs(hdr->h_vlan_TCI);
 
-	if (vid_is_dsa_8021q_rxvlan(vlan_tci & VLAN_VID_MASK))
-		return dsa_8021q_rcv(skb, source_port, switch_id);
+	if (vid_is_dsa_8021q(vlan_tci & VLAN_VID_MASK))
+		return dsa_8021q_rcv(skb, source_port, switch_id, vbid);
 
 	/* Try our best with imprecise RX */
 	*vid = vlan_tci & VLAN_VID_MASK;
@@ -529,7 +529,7 @@ static void sja1105_vlan_rcv(struct sk_buff *skb, int *source_port,
 static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
 				   struct net_device *netdev)
 {
-	int source_port = -1, switch_id = -1;
+	int source_port = -1, switch_id = -1, vbid = -1;
 	struct sja1105_meta meta = {0};
 	struct ethhdr *hdr;
 	bool is_link_local;
@@ -542,7 +542,7 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
 
 	if (sja1105_skb_has_tag_8021q(skb)) {
 		/* Normal traffic path. */
-		sja1105_vlan_rcv(skb, &source_port, &switch_id, &vid);
+		sja1105_vlan_rcv(skb, &source_port, &switch_id, &vbid, &vid);
 	} else if (is_link_local) {
 		/* Management traffic path. Switch embeds the switch ID and
 		 * port ID into bytes of the destination MAC, courtesy of
@@ -561,7 +561,9 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
 		return NULL;
 	}
 
-	if (source_port == -1 || switch_id == -1)
+	if (vbid >= 1)
+		skb->dev = dsa_tag_8021q_find_port_by_vbid(netdev, vbid);
+	else if (source_port == -1 || switch_id == -1)
 		skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);
 	else
 		skb->dev = dsa_master_find_slave(netdev, switch_id, source_port);
@@ -686,7 +688,7 @@ static struct sk_buff *sja1110_rcv_inband_control_extension(struct sk_buff *skb,
 static struct sk_buff *sja1110_rcv(struct sk_buff *skb,
 				   struct net_device *netdev)
 {
-	int source_port = -1, switch_id = -1;
+	int source_port = -1, switch_id = -1, vbid = -1;
 	bool host_only = false;
 	u16 vid = 0;
 
@@ -700,9 +702,11 @@ static struct sk_buff *sja1110_rcv(struct sk_buff *skb,
 
 	/* Packets with in-band control extensions might still have RX VLANs */
 	if (likely(sja1105_skb_has_tag_8021q(skb)))
-		sja1105_vlan_rcv(skb, &source_port, &switch_id, &vid);
+		sja1105_vlan_rcv(skb, &source_port, &switch_id, &vbid, &vid);
 
-	if (source_port == -1 || switch_id == -1)
+	if (vbid >= 1)
+		skb->dev = dsa_tag_8021q_find_port_by_vbid(netdev, vbid);
+	else if (source_port == -1 || switch_id == -1)
 		skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);
 	else
 		skb->dev = dsa_master_find_slave(netdev, switch_id, source_port);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 03/10] docs: net: dsa: sja1105: document limitations of tc-flower rule VLAN awareness
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 01/10] net: dsa: tag_8021q: replace the SVL bridging with VLAN-unaware IVL bridging Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 02/10] net: dsa: tag_8021q: add support for imprecise RX based on the VBID Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 04/10] net: dsa: felix: delete workarounds present due to SVL tag_8021q bridging Vladimir Oltean
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

After change "net: dsa: tag_8021q: replace the SVL bridging with
VLAN-unaware IVL bridging", tag_8021q enforces two different pvids on a
port, depending on whether it is standalone or in a VLAN-unaware bridge.

Up until now, there was a single pvid, represented by
dsa_tag_8021q_rx_vid(), and that was used as the VLAN for VLAN-unaware
virtual link rules, regardless of whether the port was bridged or
standalone.

To keep VLAN-unaware virtual links working, we need to follow whether
the port is in a bridge or not, and update the VLAN ID from those rules.

In fact we can't fully do that. Depending on whether the switch is
VLAN-aware or not, we can accept Virtual Link rules with just the MAC
DA, or with a MAC DA and a VID. So we already deny changes to the VLAN
awareness of the switch. But the VLAN awareness may also change as a
result of joining or leaving a bridge.

One might say we could just allow the following: a port may leave a
VLAN-unaware bridge while it has VLAN-unaware VL (tc-flower) rules, and
the driver will update those with the new tag_8021q pvid for standalone
mode, but the driver won't accept joining a bridge at all while VL rules
were installed in standalone mode. This is sort of a compromise made
because leaving a bridge is an operation that cannot be vetoed.
But this sort of setup change is not fully supported, either: as
mentioned, VLAN filtering changes can also be triggered by leaving a
bridge, therefore, the existing veto we have in place for turning VLAN
filtering off with VLAN-aware VL rules active still isn't fully
effective.

I really don't know how to deal with this in a way that produces
predictable behavior for user space. Since at the moment, keeping this
feature fully functional on constellation changes (not changing the
tag_8021q port pvid when joining a bridge) is blocking progress for the
DSA FDB isolation, I'd rather document it as a (potentially temporary)
limitation and go on without it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/sja1105.rst | 27 ++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/networking/dsa/sja1105.rst b/Documentation/networking/dsa/sja1105.rst
index 29b1bae0cf00..e0219c1452ab 100644
--- a/Documentation/networking/dsa/sja1105.rst
+++ b/Documentation/networking/dsa/sja1105.rst
@@ -293,6 +293,33 @@ of dropped frames, which is a sum of frames dropped due to timing violations,
 lack of destination ports and MTU enforcement checks). Byte-level counters are
 not available.
 
+Limitations
+===========
+
+The SJA1105 switch family always performs VLAN processing. When configured as
+VLAN-unaware, frames carry a different VLAN tag internally, depending on
+whether the port is standalone or under a VLAN-unaware bridge.
+
+The virtual link keys are always fixed at {MAC DA, VLAN ID, VLAN PCP}, but the
+driver asks for the VLAN ID and VLAN PCP when the port is under a VLAN-aware
+bridge. Otherwise, it fills in the VLAN ID and PCP automatically, based on
+whether the port is standalone or in a VLAN-unaware bridge, and accepts only
+"VLAN-unaware" tc-flower keys (MAC DA).
+
+The existing tc-flower keys that are offloaded using virtual links are no
+longer operational after one of the following happens:
+
+- port was standalone and joins a bridge (VLAN-aware or VLAN-unaware)
+- port is part of a bridge whose VLAN awareness state changes
+- port was part of a bridge and becomes standalone
+- port was standalone, but another port joins a VLAN-aware bridge and this
+  changes the global VLAN awareness state of the bridge
+
+The driver cannot veto all these operations, and it cannot update/remove the
+existing tc-flower filters either. So for proper operation, the tc-flower
+filters should be installed only after the forwarding configuration of the port
+has been made, and removed by user space before making any changes to it.
+
 Device Tree bindings and board design
 =====================================
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 04/10] net: dsa: felix: delete workarounds present due to SVL tag_8021q bridging
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
                   ` (2 preceding siblings ...)
  2022-02-25  9:22 ` [PATCH v2 net-next 03/10] docs: net: dsa: sja1105: document limitations of tc-flower rule VLAN awareness Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 05/10] net: dsa: tag_8021q: merge RX and TX VLANs Vladimir Oltean
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

The felix driver, which also has a tagging protocol implementation based
on tag_8021q, does not care about adding the RX VLAN that is pvid on one
port on the other ports that are in the same bridge with it. It simply
doesn't need that, because in its implementation, the RX VLAN that is
pvid of a port is only used to install a TCAM rule that pushes that VLAN
ID towards the CPU port.

Now that tag_8021q no longer performs Shared VLAN Learning based
forwarding, the RX VLANs are actually segregated into two types:
standalone VLANs and VLAN-unaware bridging VLANs. Since you actually
have to call dsa_tag_8021q_bridge_join() to get a bridging VLAN from
tag_8021q, and felix does not do that because it doesn't need it, it
means that it only gets standalone port VLANs from tag_8021q. Which is
perfect because this means it can drop its workarounds that avoid the
VLANs it does not need.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 9959407fede8..39ff5d201262 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -33,13 +33,6 @@ static int felix_tag_8021q_rxvlan_add(struct felix *felix, int port, u16 vid,
 	struct dsa_switch *ds = felix->ds;
 	int key_length, upstream, err;
 
-	/* We don't need to install the rxvlan into the other ports' filtering
-	 * tables, because we're just pushing the rxvlan when sending towards
-	 * the CPU
-	 */
-	if (!pvid)
-		return 0;
-
 	key_length = ocelot->vcap[VCAP_ES0].keys[VCAP_ES0_IGR_PORT].length;
 	upstream = dsa_upstream_port(ds, port);
 
@@ -170,16 +163,8 @@ static int felix_tag_8021q_rxvlan_del(struct felix *felix, int port, u16 vid)
 
 	outer_tagging_rule = ocelot_vcap_block_find_filter_by_id(block_vcap_es0,
 								 port, false);
-	/* In rxvlan_add, we had the "if (!pvid) return 0" logic to avoid
-	 * installing outer tagging ES0 rules where they weren't needed.
-	 * But in rxvlan_del, the API doesn't give us the "flags" anymore,
-	 * so that forces us to be slightly sloppy here, and just assume that
-	 * if we didn't find an outer_tagging_rule it means that there was
-	 * none in the first place, i.e. rxvlan_del is called on a non-pvid
-	 * port. This is most probably true though.
-	 */
 	if (!outer_tagging_rule)
-		return 0;
+		return -ENOENT;
 
 	return ocelot_vcap_filter_del(ocelot, outer_tagging_rule);
 }
@@ -201,7 +186,7 @@ static int felix_tag_8021q_txvlan_del(struct felix *felix, int port, u16 vid)
 	untagging_rule = ocelot_vcap_block_find_filter_by_id(block_vcap_is1,
 							     port, false);
 	if (!untagging_rule)
-		return 0;
+		return -ENOENT;
 
 	err = ocelot_vcap_filter_del(ocelot, untagging_rule);
 	if (err)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 05/10] net: dsa: tag_8021q: merge RX and TX VLANs
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
                   ` (3 preceding siblings ...)
  2022-02-25  9:22 ` [PATCH v2 net-next 04/10] net: dsa: felix: delete workarounds present due to SVL tag_8021q bridging Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 06/10] net: dsa: tag_8021q: rename dsa_8021q_bridge_tx_fwd_offload_vid Vladimir Oltean
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

In the old Shared VLAN Learning mode of operation that tag_8021q
previously used for forwarding, we needed to have distinct concepts for
an RX and a TX VLAN.

An RX VLAN could be installed on all ports that were members of a given
bridge, so that autonomous forwarding could still work, while a TX VLAN
was dedicated for precise packet steering, so it just contained the CPU
port and one egress port.

Now that tag_8021q uses Independent VLAN Learning and imprecise RX/TX
all over, those lines have been blurred and we no longer have the need
to do precise TX towards a port that is in a bridge. As for standalone
ports, it is fine to use the same VLAN ID for both RX and TX.

This patch changes the tag_8021q format by shifting the VLAN range it
reserves, and halving it. Previously, our DIR bits were encoding the
VLAN direction (RX/TX) and were set to either 1 or 2. This meant that
tag_8021q reserved 2K VLANs, or 50% of the available range.

Change the DIR bits to a hardcoded value of 3 now, which makes tag_8021q
reserve only 1K VLANs, and a different range now (the last 1K). This is
done so that we leave the old format in place in case we need to return
to it.

In terms of code, the vid_is_dsa_8021q_rxvlan and vid_is_dsa_8021q_txvlan
functions go away. Any vid_is_dsa_8021q is both a TX and an RX VLAN, and
they are no longer distinct. For example, felix which did different
things for different VLAN types, now needs to handle the RX and the TX
logic for the same VLAN.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c         | 120 ++++++++++--------
 drivers/net/dsa/sja1105/sja1105_main.c |   2 +-
 drivers/net/dsa/sja1105/sja1105_vl.c   |   3 +-
 include/linux/dsa/8021q.h              |   8 +-
 net/dsa/tag_8021q.c                    | 169 +++++++------------------
 net/dsa/tag_ocelot_8021q.c             |   2 +-
 net/dsa/tag_sja1105.c                  |   4 +-
 7 files changed, 115 insertions(+), 193 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 39ff5d201262..04f5da33b944 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -25,8 +25,10 @@
 #include <net/dsa.h>
 #include "felix.h"
 
-static int felix_tag_8021q_rxvlan_add(struct felix *felix, int port, u16 vid,
-				      bool pvid, bool untagged)
+/* Set up VCAP ES0 rules for pushing a tag_8021q VLAN towards the CPU such that
+ * the tagger can perform RX source port identification.
+ */
+static int felix_tag_8021q_vlan_add_rx(struct felix *felix, int port, u16 vid)
 {
 	struct ocelot_vcap_filter *outer_tagging_rule;
 	struct ocelot *ocelot = &felix->ocelot;
@@ -64,21 +66,32 @@ static int felix_tag_8021q_rxvlan_add(struct felix *felix, int port, u16 vid,
 	return err;
 }
 
-static int felix_tag_8021q_txvlan_add(struct felix *felix, int port, u16 vid,
-				      bool pvid, bool untagged)
+static int felix_tag_8021q_vlan_del_rx(struct felix *felix, int port, u16 vid)
+{
+	struct ocelot_vcap_filter *outer_tagging_rule;
+	struct ocelot_vcap_block *block_vcap_es0;
+	struct ocelot *ocelot = &felix->ocelot;
+
+	block_vcap_es0 = &ocelot->block[VCAP_ES0];
+
+	outer_tagging_rule = ocelot_vcap_block_find_filter_by_id(block_vcap_es0,
+								 port, false);
+	if (!outer_tagging_rule)
+		return -ENOENT;
+
+	return ocelot_vcap_filter_del(ocelot, outer_tagging_rule);
+}
+
+/* Set up VCAP IS1 rules for stripping the tag_8021q VLAN on TX and VCAP IS2
+ * rules for steering those tagged packets towards the correct destination port
+ */
+static int felix_tag_8021q_vlan_add_tx(struct felix *felix, int port, u16 vid)
 {
 	struct ocelot_vcap_filter *untagging_rule, *redirect_rule;
 	struct ocelot *ocelot = &felix->ocelot;
 	struct dsa_switch *ds = felix->ds;
 	int upstream, err;
 
-	/* tag_8021q.c assumes we are implementing this via port VLAN
-	 * membership, which we aren't. So we don't need to add any VCAP filter
-	 * for the CPU port.
-	 */
-	if (ocelot->ports[port]->is_dsa_8021q_cpu)
-		return 0;
-
 	untagging_rule = kzalloc(sizeof(struct ocelot_vcap_filter), GFP_KERNEL);
 	if (!untagging_rule)
 		return -ENOMEM;
@@ -135,41 +148,7 @@ static int felix_tag_8021q_txvlan_add(struct felix *felix, int port, u16 vid,
 	return 0;
 }
 
-static int felix_tag_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid,
-				    u16 flags)
-{
-	bool untagged = flags & BRIDGE_VLAN_INFO_UNTAGGED;
-	bool pvid = flags & BRIDGE_VLAN_INFO_PVID;
-	struct ocelot *ocelot = ds->priv;
-
-	if (vid_is_dsa_8021q_rxvlan(vid))
-		return felix_tag_8021q_rxvlan_add(ocelot_to_felix(ocelot),
-						  port, vid, pvid, untagged);
-
-	if (vid_is_dsa_8021q_txvlan(vid))
-		return felix_tag_8021q_txvlan_add(ocelot_to_felix(ocelot),
-						  port, vid, pvid, untagged);
-
-	return 0;
-}
-
-static int felix_tag_8021q_rxvlan_del(struct felix *felix, int port, u16 vid)
-{
-	struct ocelot_vcap_filter *outer_tagging_rule;
-	struct ocelot_vcap_block *block_vcap_es0;
-	struct ocelot *ocelot = &felix->ocelot;
-
-	block_vcap_es0 = &ocelot->block[VCAP_ES0];
-
-	outer_tagging_rule = ocelot_vcap_block_find_filter_by_id(block_vcap_es0,
-								 port, false);
-	if (!outer_tagging_rule)
-		return -ENOENT;
-
-	return ocelot_vcap_filter_del(ocelot, outer_tagging_rule);
-}
-
-static int felix_tag_8021q_txvlan_del(struct felix *felix, int port, u16 vid)
+static int felix_tag_8021q_vlan_del_tx(struct felix *felix, int port, u16 vid)
 {
 	struct ocelot_vcap_filter *untagging_rule, *redirect_rule;
 	struct ocelot_vcap_block *block_vcap_is1;
@@ -177,9 +156,6 @@ static int felix_tag_8021q_txvlan_del(struct felix *felix, int port, u16 vid)
 	struct ocelot *ocelot = &felix->ocelot;
 	int err;
 
-	if (ocelot->ports[port]->is_dsa_8021q_cpu)
-		return 0;
-
 	block_vcap_is1 = &ocelot->block[VCAP_IS1];
 	block_vcap_is2 = &ocelot->block[VCAP_IS2];
 
@@ -195,22 +171,54 @@ static int felix_tag_8021q_txvlan_del(struct felix *felix, int port, u16 vid)
 	redirect_rule = ocelot_vcap_block_find_filter_by_id(block_vcap_is2,
 							    port, false);
 	if (!redirect_rule)
-		return 0;
+		return -ENOENT;
 
 	return ocelot_vcap_filter_del(ocelot, redirect_rule);
 }
 
+static int felix_tag_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid,
+				    u16 flags)
+{
+	struct ocelot *ocelot = ds->priv;
+	int err;
+
+	/* tag_8021q.c assumes we are implementing this via port VLAN
+	 * membership, which we aren't. So we don't need to add any VCAP filter
+	 * for the CPU port.
+	 */
+	if (!dsa_is_user_port(ds, port))
+		return 0;
+
+	err = felix_tag_8021q_vlan_add_rx(ocelot_to_felix(ocelot), port, vid);
+	if (err)
+		return err;
+
+	err = felix_tag_8021q_vlan_add_tx(ocelot_to_felix(ocelot), port, vid);
+	if (err) {
+		felix_tag_8021q_vlan_del_rx(ocelot_to_felix(ocelot), port, vid);
+		return err;
+	}
+
+	return 0;
+}
+
 static int felix_tag_8021q_vlan_del(struct dsa_switch *ds, int port, u16 vid)
 {
 	struct ocelot *ocelot = ds->priv;
+	int err;
+
+	if (!dsa_is_user_port(ds, port))
+		return 0;
 
-	if (vid_is_dsa_8021q_rxvlan(vid))
-		return felix_tag_8021q_rxvlan_del(ocelot_to_felix(ocelot),
-						  port, vid);
+	err = felix_tag_8021q_vlan_del_rx(ocelot_to_felix(ocelot), port, vid);
+	if (err)
+		return err;
 
-	if (vid_is_dsa_8021q_txvlan(vid))
-		return felix_tag_8021q_txvlan_del(ocelot_to_felix(ocelot),
-						  port, vid);
+	err = felix_tag_8021q_vlan_del_tx(ocelot_to_felix(ocelot), port, vid);
+	if (err) {
+		felix_tag_8021q_vlan_add_rx(ocelot_to_felix(ocelot), port, vid);
+		return err;
+	}
 
 	return 0;
 }
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 1e43a7ef0f2e..dd89b077aae6 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2525,7 +2525,7 @@ static int sja1105_bridge_vlan_add(struct dsa_switch *ds, int port,
 	 */
 	if (vid_is_dsa_8021q(vlan->vid)) {
 		NL_SET_ERR_MSG_MOD(extack,
-				   "Range 1024-3071 reserved for dsa_8021q operation");
+				   "Range 3072-4095 reserved for dsa_8021q operation");
 		return -EBUSY;
 	}
 
diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index 14e6dd7fb103..1eef60207c6b 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -302,7 +302,7 @@ static u16 sja1105_port_get_tag_8021q_vid(struct dsa_port *dp)
 	unsigned long bridge_num;
 
 	if (!dp->bridge)
-		return dsa_tag_8021q_rx_vid(dp);
+		return dsa_tag_8021q_standalone_vid(dp);
 
 	bridge_num = dsa_port_bridge_num_get(dp);
 
@@ -407,6 +407,7 @@ static int sja1105_init_virtual_links(struct sja1105_private *priv,
 				vl_lookup[k].vlanid = rule->key.vl.vid;
 				vl_lookup[k].vlanprior = rule->key.vl.pcp;
 			} else {
+				/* FIXME */
 				struct dsa_port *dp = dsa_to_port(priv->ds, port);
 				u16 vid = sja1105_port_get_tag_8021q_vid(dp);
 
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index 92f5243b841e..b4e2862633f6 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -49,18 +49,12 @@ struct net_device *dsa_tag_8021q_find_port_by_vbid(struct net_device *master,
 
 u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
 
-u16 dsa_tag_8021q_tx_vid(const struct dsa_port *dp);
-
-u16 dsa_tag_8021q_rx_vid(const struct dsa_port *dp);
+u16 dsa_tag_8021q_standalone_vid(const struct dsa_port *dp);
 
 int dsa_8021q_rx_switch_id(u16 vid);
 
 int dsa_8021q_rx_source_port(u16 vid);
 
-bool vid_is_dsa_8021q_rxvlan(u16 vid);
-
-bool vid_is_dsa_8021q_txvlan(u16 vid);
-
 bool vid_is_dsa_8021q(u16 vid);
 
 #endif /* _NET_DSA_8021Q_H */
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 1cf245a6f18e..eac43f5b4e07 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -16,15 +16,11 @@
  *
  * | 11  | 10  |  9  |  8  |  7  |  6  |  5  |  4  |  3  |  2  |  1  |  0  |
  * +-----------+-----+-----------------+-----------+-----------------------+
- * |    DIR    | VBID|    SWITCH_ID    |   VBID    |          PORT         |
+ * |    RSV    | VBID|    SWITCH_ID    |   VBID    |          PORT         |
  * +-----------+-----+-----------------+-----------+-----------------------+
  *
- * DIR - VID[11:10]:
- *	Direction flags.
- *	* 1 (0b01) for RX VLAN,
- *	* 2 (0b10) for TX VLAN.
- *	These values make the special VIDs of 0, 1 and 4095 to be left
- *	unused by this coding scheme.
+ * RSV - VID[11:10]:
+ *	Reserved. Must be set to 3 (0b11).
  *
  * SWITCH_ID - VID[8:6]:
  *	Index of switch within DSA tree. Must be between 0 and 7.
@@ -38,12 +34,11 @@
  *	Index of switch port. Must be between 0 and 15.
  */
 
-#define DSA_8021Q_DIR_SHIFT		10
-#define DSA_8021Q_DIR_MASK		GENMASK(11, 10)
-#define DSA_8021Q_DIR(x)		(((x) << DSA_8021Q_DIR_SHIFT) & \
-						 DSA_8021Q_DIR_MASK)
-#define DSA_8021Q_DIR_RX		DSA_8021Q_DIR(1)
-#define DSA_8021Q_DIR_TX		DSA_8021Q_DIR(2)
+#define DSA_8021Q_RSV_VAL		3
+#define DSA_8021Q_RSV_SHIFT		10
+#define DSA_8021Q_RSV_MASK		GENMASK(11, 10)
+#define DSA_8021Q_RSV			((DSA_8021Q_RSV_VAL << DSA_8021Q_RSV_SHIFT) & \
+							       DSA_8021Q_RSV_MASK)
 
 #define DSA_8021Q_SWITCH_ID_SHIFT	6
 #define DSA_8021Q_SWITCH_ID_MASK	GENMASK(8, 6)
@@ -72,29 +67,19 @@ u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num)
 	/* The VBID value of 0 is reserved for precise TX, but it is also
 	 * reserved/invalid for the bridge_num, so all is well.
 	 */
-	return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num);
+	return DSA_8021Q_RSV | DSA_8021Q_VBID(bridge_num);
 }
 EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid);
 
-/* Returns the VID to be inserted into the frame from xmit for switch steering
- * instructions on egress. Encodes switch ID and port ID.
- */
-u16 dsa_tag_8021q_tx_vid(const struct dsa_port *dp)
-{
-	return DSA_8021Q_DIR_TX | DSA_8021Q_SWITCH_ID(dp->ds->index) |
-	       DSA_8021Q_PORT(dp->index);
-}
-EXPORT_SYMBOL_GPL(dsa_tag_8021q_tx_vid);
-
 /* Returns the VID that will be installed as pvid for this switch port, sent as
  * tagged egress towards the CPU port and decoded by the rcv function.
  */
-u16 dsa_tag_8021q_rx_vid(const struct dsa_port *dp)
+u16 dsa_tag_8021q_standalone_vid(const struct dsa_port *dp)
 {
-	return DSA_8021Q_DIR_RX | DSA_8021Q_SWITCH_ID(dp->ds->index) |
+	return DSA_8021Q_RSV | DSA_8021Q_SWITCH_ID(dp->ds->index) |
 	       DSA_8021Q_PORT(dp->index);
 }
-EXPORT_SYMBOL_GPL(dsa_tag_8021q_rx_vid);
+EXPORT_SYMBOL_GPL(dsa_tag_8021q_standalone_vid);
 
 /* Returns the decoded switch ID from the RX VID. */
 int dsa_8021q_rx_switch_id(u16 vid)
@@ -119,21 +104,11 @@ static int dsa_tag_8021q_rx_vbid(u16 vid)
 	return (vbid_hi << 2) | vbid_lo;
 }
 
-bool vid_is_dsa_8021q_rxvlan(u16 vid)
-{
-	return (vid & DSA_8021Q_DIR_MASK) == DSA_8021Q_DIR_RX;
-}
-EXPORT_SYMBOL_GPL(vid_is_dsa_8021q_rxvlan);
-
-bool vid_is_dsa_8021q_txvlan(u16 vid)
-{
-	return (vid & DSA_8021Q_DIR_MASK) == DSA_8021Q_DIR_TX;
-}
-EXPORT_SYMBOL_GPL(vid_is_dsa_8021q_txvlan);
-
 bool vid_is_dsa_8021q(u16 vid)
 {
-	return vid_is_dsa_8021q_rxvlan(vid) || vid_is_dsa_8021q_txvlan(vid);
+	u16 rsv = (vid & DSA_8021Q_RSV_MASK) >> DSA_8021Q_RSV_SHIFT;
+
+	return rsv == DSA_8021Q_RSV_VAL;
 }
 EXPORT_SYMBOL_GPL(vid_is_dsa_8021q);
 
@@ -251,18 +226,8 @@ int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds,
 			u16 flags = 0;
 
 			if (dsa_port_is_user(dp))
-				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) == dp->index)
-				flags |= BRIDGE_VLAN_INFO_PVID;
-
-			/* And bridging VLANs are PVIDs too on user ports */
-			if (dsa_tag_8021q_rx_vbid(info->vid) &&
-			    dsa_port_is_user(dp))
-				flags |= BRIDGE_VLAN_INFO_PVID;
+				flags |= BRIDGE_VLAN_INFO_UNTAGGED |
+					 BRIDGE_VLAN_INFO_PVID;
 
 			err = dsa_port_do_tag_8021q_vlan_add(dp, info->vid,
 							     flags);
@@ -294,52 +259,24 @@ int dsa_switch_tag_8021q_vlan_del(struct dsa_switch *ds,
 	return 0;
 }
 
-/* RX VLAN tagging (left) and TX VLAN tagging (right) setup shown for a single
- * front-panel switch port (here swp0).
+/* There are 2 ways of offloading tag_8021q VLANs.
  *
- * Port identification through VLAN (802.1Q) tags has different requirements
- * for it to work effectively:
- *  - On RX (ingress from network): each front-panel port must have a pvid
- *    that uniquely identifies it, and the egress of this pvid must be tagged
- *    towards the CPU port, so that software can recover the source port based
- *    on the VID in the frame. But this would only work for standalone ports;
- *    if bridged, this VLAN setup would break autonomous forwarding and would
- *    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).
- *  - 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
- *    have no choice but to be directed towards the correct front-panel port.
- *    But because we also want the RX VLAN to not break bridging, then
- *    inevitably that means that we have to give them a choice (of what
- *    front-panel port to go out on), and therefore we cannot steer traffic
- *    based on the RX VID. So what we do is simply install one more VID on the
- *    front-panel and CPU ports, and profit off of the fact that steering will
- *    work just by virtue of the fact that there is only one other port that's
- *    a member of the VID we're tagging the traffic with - the desired one.
+ * One is to use a hardware TCAM to push the port's standalone VLAN into the
+ * frame when forwarding it to the CPU, as an egress modification rule on the
+ * CPU port. This is preferable because it has no side effects for the
+ * autonomous forwarding path, and accomplishes tag_8021q's primary goal of
+ * identifying the source port of each packet based on VLAN ID.
  *
- * 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 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).
+ * The other is to commit the tag_8021q VLAN as a PVID to the VLAN table, and
+ * to configure the port as VLAN-unaware. This is less preferable because
+ * unique source port identification can only be done for standalone ports;
+ * under a VLAN-unaware bridge, all ports share the same tag_8021q VLAN as
+ * PVID, and under a VLAN-aware bridge, packets received by software will not
+ * have tag_8021q VLANs appended, just bridge VLANs.
  *
- *               CPU port                               CPU port
- * +-------------+-----+-------------+    +-------------+-----+-------------+
- * |  RX VID     |     |             |    |  TX VID     |     |             |
- * |  of swp0    |     |             |    |  of swp0    |     |             |
- * |             +-----+             |    |             +-----+             |
- * |                ^ T              |    |                | Tagged         |
- * |                |                |    |                | ingress        |
- * |    +-------+---+---+-------+    |    |    +-----------+                |
- * |    |       |       |       |    |    |    | Untagged                   |
- * |    |     U v     U v     U v    |    |    v egress                     |
- * | +-----+ +-----+ +-----+ +-----+ |    | +-----+ +-----+ +-----+ +-----+ |
- * | |     | |     | |     | |     | |    | |     | |     | |     | |     | |
- * | |PVID | |     | |     | |     | |    | |     | |     | |     | |     | |
- * +-+-----+-+-----+-+-----+-+-----+-+    +-+-----+-+-----+-+-----+-+-----+-+
- *   swp0    swp1    swp2    swp3           swp0    swp1    swp2    swp3
+ * For tag_8021q implementations of the second type, this method is used to
+ * replace the standalone tag_8021q VLAN of a port with the tag_8021q VLAN to
+ * be used for VLAN-unaware bridging.
  */
 int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, int port,
 			      struct dsa_bridge bridge)
@@ -351,7 +288,7 @@ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, int port,
 	/* Delete the standalone VLAN of the port and replace it with a
 	 * bridging VLAN
 	 */
-	standalone_vid = dsa_tag_8021q_rx_vid(dp);
+	standalone_vid = dsa_tag_8021q_standalone_vid(dp);
 	bridge_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
 
 	err = dsa_port_tag_8021q_vlan_add(dp, bridge_vid, true);
@@ -374,7 +311,7 @@ void dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, int port,
 	/* Delete the bridging VLAN of the port and replace it with a
 	 * standalone VLAN
 	 */
-	standalone_vid = dsa_tag_8021q_rx_vid(dp);
+	standalone_vid = dsa_tag_8021q_standalone_vid(dp);
 	bridge_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
 
 	err = dsa_port_tag_8021q_vlan_add(dp, standalone_vid, false);
@@ -388,13 +325,12 @@ void dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_leave);
 
-/* Set up a port's tag_8021q RX and TX VLAN for standalone mode operation */
+/* Set up a port's standalone tag_8021q VLAN */
 static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)
 {
 	struct dsa_8021q_context *ctx = ds->tag_8021q_ctx;
 	struct dsa_port *dp = dsa_to_port(ds, port);
-	u16 rx_vid = dsa_tag_8021q_rx_vid(dp);
-	u16 tx_vid = dsa_tag_8021q_tx_vid(dp);
+	u16 vid = dsa_tag_8021q_standalone_vid(dp);
 	struct net_device *master;
 	int err;
 
@@ -406,30 +342,16 @@ static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)
 
 	master = dp->cpu_dp->master;
 
-	/* Add this user port's RX VID to the membership list of all others
-	 * (including itself). This is so that bridging will not be hindered.
-	 * L2 forwarding rules still take precedence when there are no VLAN
-	 * restrictions, so there are no concerns about leaking traffic.
-	 */
-	err = dsa_port_tag_8021q_vlan_add(dp, rx_vid, false);
+	err = dsa_port_tag_8021q_vlan_add(dp, vid, false);
 	if (err) {
 		dev_err(ds->dev,
-			"Failed to apply RX VID %d to port %d: %pe\n",
-			rx_vid, port, ERR_PTR(err));
+			"Failed to apply standalone VID %d to port %d: %pe\n",
+			vid, port, ERR_PTR(err));
 		return err;
 	}
 
-	/* Add @rx_vid to the master's RX filter. */
-	vlan_vid_add(master, ctx->proto, rx_vid);
-
-	/* Finally apply the TX VID on this port and on the CPU port */
-	err = dsa_port_tag_8021q_vlan_add(dp, tx_vid, false);
-	if (err) {
-		dev_err(ds->dev,
-			"Failed to apply TX VID %d on port %d: %pe\n",
-			tx_vid, port, ERR_PTR(err));
-		return err;
-	}
+	/* Add the VLAN to the master's RX filter. */
+	vlan_vid_add(master, ctx->proto, vid);
 
 	return err;
 }
@@ -438,8 +360,7 @@ static void dsa_tag_8021q_port_teardown(struct dsa_switch *ds, int port)
 {
 	struct dsa_8021q_context *ctx = ds->tag_8021q_ctx;
 	struct dsa_port *dp = dsa_to_port(ds, port);
-	u16 rx_vid = dsa_tag_8021q_rx_vid(dp);
-	u16 tx_vid = dsa_tag_8021q_tx_vid(dp);
+	u16 vid = dsa_tag_8021q_standalone_vid(dp);
 	struct net_device *master;
 
 	/* The CPU port is implicitly configured by
@@ -450,11 +371,9 @@ static void dsa_tag_8021q_port_teardown(struct dsa_switch *ds, int port)
 
 	master = dp->cpu_dp->master;
 
-	dsa_port_tag_8021q_vlan_del(dp, rx_vid, false);
-
-	vlan_vid_del(master, ctx->proto, rx_vid);
+	dsa_port_tag_8021q_vlan_del(dp, vid, false);
 
-	dsa_port_tag_8021q_vlan_del(dp, tx_vid, false);
+	vlan_vid_del(master, ctx->proto, vid);
 }
 
 static int dsa_tag_8021q_setup(struct dsa_switch *ds)
diff --git a/net/dsa/tag_ocelot_8021q.c b/net/dsa/tag_ocelot_8021q.c
index 1144a87ad0db..37ccf00404ea 100644
--- a/net/dsa/tag_ocelot_8021q.c
+++ b/net/dsa/tag_ocelot_8021q.c
@@ -62,7 +62,7 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 	struct dsa_port *dp = dsa_slave_to_port(netdev);
 	u16 queue_mapping = skb_get_queue_mapping(skb);
 	u8 pcp = netdev_txq_to_tc(netdev, queue_mapping);
-	u16 tx_vid = dsa_tag_8021q_tx_vid(dp);
+	u16 tx_vid = dsa_tag_8021q_standalone_vid(dp);
 	struct ethhdr *hdr = eth_hdr(skb);
 
 	if (ocelot_ptp_rew_op(skb) || is_link_local_ether_addr(hdr->h_dest))
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 9c5c00980b06..f3832ac54098 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -267,7 +267,7 @@ static struct sk_buff *sja1105_xmit(struct sk_buff *skb,
 	struct dsa_port *dp = dsa_slave_to_port(netdev);
 	u16 queue_mapping = skb_get_queue_mapping(skb);
 	u8 pcp = netdev_txq_to_tc(netdev, queue_mapping);
-	u16 tx_vid = dsa_tag_8021q_tx_vid(dp);
+	u16 tx_vid = dsa_tag_8021q_standalone_vid(dp);
 
 	if (skb->offload_fwd_mark)
 		return sja1105_imprecise_xmit(skb, netdev);
@@ -295,7 +295,7 @@ static struct sk_buff *sja1110_xmit(struct sk_buff *skb,
 	struct dsa_port *dp = dsa_slave_to_port(netdev);
 	u16 queue_mapping = skb_get_queue_mapping(skb);
 	u8 pcp = netdev_txq_to_tc(netdev, queue_mapping);
-	u16 tx_vid = dsa_tag_8021q_tx_vid(dp);
+	u16 tx_vid = dsa_tag_8021q_standalone_vid(dp);
 	__be32 *tx_trailer;
 	__be16 *tx_header;
 	int trailer_pos;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 06/10] net: dsa: tag_8021q: rename dsa_8021q_bridge_tx_fwd_offload_vid
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
                   ` (4 preceding siblings ...)
  2022-02-25  9:22 ` [PATCH v2 net-next 05/10] net: dsa: tag_8021q: merge RX and TX VLANs Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation Vladimir Oltean
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

The dsa_8021q_bridge_tx_fwd_offload_vid is no longer used just for
bridge TX forwarding offload, it is the private VLAN reserved for
VLAN-unaware bridging in a way that is compatible with FDB isolation.

So just rename it dsa_tag_8021q_bridge_vid.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_vl.c | 2 +-
 include/linux/dsa/8021q.h            | 2 +-
 net/dsa/tag_8021q.c                  | 8 ++++----
 net/dsa/tag_sja1105.c                | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index 1eef60207c6b..b7e95d60a6e4 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -306,7 +306,7 @@ static u16 sja1105_port_get_tag_8021q_vid(struct dsa_port *dp)
 
 	bridge_num = dsa_port_bridge_num_get(dp);
 
-	return dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+	return dsa_tag_8021q_bridge_vid(bridge_num);
 }
 
 static int sja1105_init_virtual_links(struct sja1105_private *priv,
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index b4e2862633f6..3ed117e299ec 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -47,7 +47,7 @@ void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id,
 struct net_device *dsa_tag_8021q_find_port_by_vbid(struct net_device *master,
 						   int vbid);
 
-u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
+u16 dsa_tag_8021q_bridge_vid(unsigned int bridge_num);
 
 u16 dsa_tag_8021q_standalone_vid(const struct dsa_port *dp);
 
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index eac43f5b4e07..a786569203f0 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -62,14 +62,14 @@
 #define DSA_8021Q_PORT(x)		(((x) << DSA_8021Q_PORT_SHIFT) & \
 						 DSA_8021Q_PORT_MASK)
 
-u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num)
+u16 dsa_tag_8021q_bridge_vid(unsigned int bridge_num)
 {
 	/* The VBID value of 0 is reserved for precise TX, but it is also
 	 * reserved/invalid for the bridge_num, so all is well.
 	 */
 	return DSA_8021Q_RSV | DSA_8021Q_VBID(bridge_num);
 }
-EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid);
+EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_vid);
 
 /* Returns the VID that will be installed as pvid for this switch port, sent as
  * tagged egress towards the CPU port and decoded by the rcv function.
@@ -289,7 +289,7 @@ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, int port,
 	 * bridging VLAN
 	 */
 	standalone_vid = dsa_tag_8021q_standalone_vid(dp);
-	bridge_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
+	bridge_vid = dsa_tag_8021q_bridge_vid(bridge.num);
 
 	err = dsa_port_tag_8021q_vlan_add(dp, bridge_vid, true);
 	if (err)
@@ -312,7 +312,7 @@ void dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, int port,
 	 * standalone VLAN
 	 */
 	standalone_vid = dsa_tag_8021q_standalone_vid(dp);
-	bridge_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
+	bridge_vid = dsa_tag_8021q_bridge_vid(bridge.num);
 
 	err = dsa_port_tag_8021q_vlan_add(dp, standalone_vid, false);
 	if (err) {
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index f3832ac54098..83e4136516b0 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -226,7 +226,7 @@ static struct sk_buff *sja1105_imprecise_xmit(struct sk_buff *skb,
 	 * TX VLAN that targets the bridge's entire broadcast domain,
 	 * instead of just the specific port.
 	 */
-	tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+	tx_vid = dsa_tag_8021q_bridge_vid(bridge_num);
 
 	return dsa_8021q_xmit(skb, netdev, sja1105_xmit_tpid(dp), tx_vid);
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
                   ` (5 preceding siblings ...)
  2022-02-25  9:22 ` [PATCH v2 net-next 06/10] net: dsa: tag_8021q: rename dsa_8021q_bridge_tx_fwd_offload_vid Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-04-26 15:01   ` Hans Schultz
  2022-02-25  9:22 ` [PATCH v2 net-next 08/10] net: dsa: pass extack to .port_bridge_join driver methods Vladimir Oltean
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

For DSA, to encourage drivers to perform FDB isolation simply means to
track which bridge does each FDB and MDB entry belong to. It then
becomes the driver responsibility to use something that makes the FDB
entry from one bridge not match the FDB lookup of ports from other
bridges.

The top-level functions where the bridge is determined are:
- dsa_port_fdb_{add,del}
- dsa_port_host_fdb_{add,del}
- dsa_port_mdb_{add,del}
- dsa_port_host_mdb_{add,del}

aka the pre-crosschip-notifier functions.

Changing the API to pass a reference to a bridge is not superfluous, and
looking at the passed bridge argument is not the same as having the
driver look at dsa_to_port(ds, port)->bridge from the ->port_fdb_add()
method.

DSA installs FDB and MDB entries on shared (CPU and DSA) ports as well,
and those do not have any dp->bridge information to retrieve, because
they are not in any bridge - they are merely the pipes that serve the
user ports that are in one or multiple bridges.

The struct dsa_bridge associated with each FDB/MDB entry is encapsulated
in a larger "struct dsa_db" database. Although only databases associated
to bridges are notified for now, this API will be the starting point for
implementing IFF_UNICAST_FLT in DSA. There, the idea is to install FDB
entries on the CPU port which belong to the corresponding user port's
port database. These are supposed to match only when the port is
standalone.

It is better to introduce the API in its expected final form than to
introduce it for bridges first, then to have to change drivers which may
have made one or more assumptions.

Drivers can use the provided bridge.num, but they can also use a
different numbering scheme that is more convenient.

DSA must perform refcounting on the CPU and DSA ports by also taking
into account the bridge number. So if two bridges request the same local
address, DSA must notify the driver twice, once for each bridge.

In fact, if the driver supports FDB isolation, DSA must perform
refcounting per bridge, but if the driver doesn't, DSA must refcount
host addresses across all bridges, otherwise it would be telling the
driver to delete an FDB entry for a bridge and the driver would delete
it for all bridges. So introduce a bool fdb_isolation in drivers which
would make all bridge databases passed to the cross-chip notifier have
the same number (0). This makes dsa_mac_addr_find() -> dsa_db_equal()
say that all bridge databases are the same database - which is
essentially the legacy behavior.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c       | 12 ++--
 drivers/net/dsa/b53/b53_priv.h         | 12 ++--
 drivers/net/dsa/hirschmann/hellcreek.c |  6 +-
 drivers/net/dsa/lan9303-core.c         | 13 ++--
 drivers/net/dsa/lantiq_gswip.c         |  6 +-
 drivers/net/dsa/microchip/ksz9477.c    | 12 ++--
 drivers/net/dsa/microchip/ksz_common.c |  6 +-
 drivers/net/dsa/microchip/ksz_common.h |  6 +-
 drivers/net/dsa/mt7530.c               | 12 ++--
 drivers/net/dsa/mv88e6xxx/chip.c       | 12 ++--
 drivers/net/dsa/ocelot/felix.c         | 18 +++--
 drivers/net/dsa/qca8k.c                | 12 ++--
 drivers/net/dsa/sja1105/sja1105_main.c | 26 +++++--
 include/net/dsa.h                      | 42 +++++++++--
 net/dsa/dsa_priv.h                     |  3 +
 net/dsa/port.c                         | 75 ++++++++++++++++++-
 net/dsa/switch.c                       | 99 +++++++++++++++++---------
 17 files changed, 280 insertions(+), 92 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 83bf30349c26..a8cc6e182c45 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1708,7 +1708,8 @@ static int b53_arl_op(struct b53_device *dev, int op, int port,
 }
 
 int b53_fdb_add(struct dsa_switch *ds, int port,
-		const unsigned char *addr, u16 vid)
+		const unsigned char *addr, u16 vid,
+		struct dsa_db db)
 {
 	struct b53_device *priv = ds->priv;
 	int ret;
@@ -1728,7 +1729,8 @@ int b53_fdb_add(struct dsa_switch *ds, int port,
 EXPORT_SYMBOL(b53_fdb_add);
 
 int b53_fdb_del(struct dsa_switch *ds, int port,
-		const unsigned char *addr, u16 vid)
+		const unsigned char *addr, u16 vid,
+		struct dsa_db db)
 {
 	struct b53_device *priv = ds->priv;
 	int ret;
@@ -1829,7 +1831,8 @@ int b53_fdb_dump(struct dsa_switch *ds, int port,
 EXPORT_SYMBOL(b53_fdb_dump);
 
 int b53_mdb_add(struct dsa_switch *ds, int port,
-		const struct switchdev_obj_port_mdb *mdb)
+		const struct switchdev_obj_port_mdb *mdb,
+		struct dsa_db db)
 {
 	struct b53_device *priv = ds->priv;
 	int ret;
@@ -1849,7 +1852,8 @@ int b53_mdb_add(struct dsa_switch *ds, int port,
 EXPORT_SYMBOL(b53_mdb_add);
 
 int b53_mdb_del(struct dsa_switch *ds, int port,
-		const struct switchdev_obj_port_mdb *mdb)
+		const struct switchdev_obj_port_mdb *mdb,
+		struct dsa_db db)
 {
 	struct b53_device *priv = ds->priv;
 	int ret;
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index a6b339fcb17e..d3091f0ad3e6 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -359,15 +359,19 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
 int b53_vlan_del(struct dsa_switch *ds, int port,
 		 const struct switchdev_obj_port_vlan *vlan);
 int b53_fdb_add(struct dsa_switch *ds, int port,
-		const unsigned char *addr, u16 vid);
+		const unsigned char *addr, u16 vid,
+		struct dsa_db db);
 int b53_fdb_del(struct dsa_switch *ds, int port,
-		const unsigned char *addr, u16 vid);
+		const unsigned char *addr, u16 vid,
+		struct dsa_db db);
 int b53_fdb_dump(struct dsa_switch *ds, int port,
 		 dsa_fdb_dump_cb_t *cb, void *data);
 int b53_mdb_add(struct dsa_switch *ds, int port,
-		const struct switchdev_obj_port_mdb *mdb);
+		const struct switchdev_obj_port_mdb *mdb,
+		struct dsa_db db);
 int b53_mdb_del(struct dsa_switch *ds, int port,
-		const struct switchdev_obj_port_mdb *mdb);
+		const struct switchdev_obj_port_mdb *mdb,
+		struct dsa_db db);
 int b53_mirror_add(struct dsa_switch *ds, int port,
 		   struct dsa_mall_mirror_tc_entry *mirror, bool ingress);
 enum dsa_tag_protocol b53_get_tag_protocol(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 726f267cb228..cb89be9de43a 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -827,7 +827,8 @@ static int hellcreek_fdb_get(struct hellcreek *hellcreek,
 }
 
 static int hellcreek_fdb_add(struct dsa_switch *ds, int port,
-			     const unsigned char *addr, u16 vid)
+			     const unsigned char *addr, u16 vid,
+			     struct dsa_db db)
 {
 	struct hellcreek_fdb_entry entry = { 0 };
 	struct hellcreek *hellcreek = ds->priv;
@@ -872,7 +873,8 @@ static int hellcreek_fdb_add(struct dsa_switch *ds, int port,
 }
 
 static int hellcreek_fdb_del(struct dsa_switch *ds, int port,
-			     const unsigned char *addr, u16 vid)
+			     const unsigned char *addr, u16 vid,
+			     struct dsa_db db)
 {
 	struct hellcreek_fdb_entry entry = { 0 };
 	struct hellcreek *hellcreek = ds->priv;
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 3969d89fa4db..a21184e7fcb6 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1188,7 +1188,8 @@ static void lan9303_port_fast_age(struct dsa_switch *ds, int port)
 }
 
 static int lan9303_port_fdb_add(struct dsa_switch *ds, int port,
-				const unsigned char *addr, u16 vid)
+				const unsigned char *addr, u16 vid,
+				struct dsa_db db)
 {
 	struct lan9303 *chip = ds->priv;
 
@@ -1200,8 +1201,8 @@ static int lan9303_port_fdb_add(struct dsa_switch *ds, int port,
 }
 
 static int lan9303_port_fdb_del(struct dsa_switch *ds, int port,
-				const unsigned char *addr, u16 vid)
-
+				const unsigned char *addr, u16 vid,
+				struct dsa_db db)
 {
 	struct lan9303 *chip = ds->priv;
 
@@ -1245,7 +1246,8 @@ static int lan9303_port_mdb_prepare(struct dsa_switch *ds, int port,
 }
 
 static int lan9303_port_mdb_add(struct dsa_switch *ds, int port,
-				const struct switchdev_obj_port_mdb *mdb)
+				const struct switchdev_obj_port_mdb *mdb,
+				struct dsa_db db)
 {
 	struct lan9303 *chip = ds->priv;
 	int err;
@@ -1260,7 +1262,8 @@ static int lan9303_port_mdb_add(struct dsa_switch *ds, int port,
 }
 
 static int lan9303_port_mdb_del(struct dsa_switch *ds, int port,
-				const struct switchdev_obj_port_mdb *mdb)
+				const struct switchdev_obj_port_mdb *mdb,
+				struct dsa_db db)
 {
 	struct lan9303 *chip = ds->priv;
 
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 8a7a8093a156..3dfb532b7784 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1389,13 +1389,15 @@ static int gswip_port_fdb(struct dsa_switch *ds, int port,
 }
 
 static int gswip_port_fdb_add(struct dsa_switch *ds, int port,
-			      const unsigned char *addr, u16 vid)
+			      const unsigned char *addr, u16 vid,
+			      struct dsa_db db)
 {
 	return gswip_port_fdb(ds, port, addr, vid, true);
 }
 
 static int gswip_port_fdb_del(struct dsa_switch *ds, int port,
-			      const unsigned char *addr, u16 vid)
+			      const unsigned char *addr, u16 vid,
+			      struct dsa_db db)
 {
 	return gswip_port_fdb(ds, port, addr, vid, false);
 }
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 18ffc8ded7ee..94ad6d9504f4 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -640,7 +640,8 @@ static int ksz9477_port_vlan_del(struct dsa_switch *ds, int port,
 }
 
 static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
-				const unsigned char *addr, u16 vid)
+				const unsigned char *addr, u16 vid,
+				struct dsa_db db)
 {
 	struct ksz_device *dev = ds->priv;
 	u32 alu_table[4];
@@ -697,7 +698,8 @@ static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
 }
 
 static int ksz9477_port_fdb_del(struct dsa_switch *ds, int port,
-				const unsigned char *addr, u16 vid)
+				const unsigned char *addr, u16 vid,
+				struct dsa_db db)
 {
 	struct ksz_device *dev = ds->priv;
 	u32 alu_table[4];
@@ -839,7 +841,8 @@ static int ksz9477_port_fdb_dump(struct dsa_switch *ds, int port,
 }
 
 static int ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
-				const struct switchdev_obj_port_mdb *mdb)
+				const struct switchdev_obj_port_mdb *mdb,
+				struct dsa_db db)
 {
 	struct ksz_device *dev = ds->priv;
 	u32 static_table[4];
@@ -914,7 +917,8 @@ static int ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 }
 
 static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
-				const struct switchdev_obj_port_mdb *mdb)
+				const struct switchdev_obj_port_mdb *mdb,
+				struct dsa_db db)
 {
 	struct ksz_device *dev = ds->priv;
 	u32 static_table[4];
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 94e618b8352b..104458ec9cbc 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -276,7 +276,8 @@ int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
 EXPORT_SYMBOL_GPL(ksz_port_fdb_dump);
 
 int ksz_port_mdb_add(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_mdb *mdb)
+		     const struct switchdev_obj_port_mdb *mdb,
+		     struct dsa_db db)
 {
 	struct ksz_device *dev = ds->priv;
 	struct alu_struct alu;
@@ -321,7 +322,8 @@ int ksz_port_mdb_add(struct dsa_switch *ds, int port,
 EXPORT_SYMBOL_GPL(ksz_port_mdb_add);
 
 int ksz_port_mdb_del(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_mdb *mdb)
+		     const struct switchdev_obj_port_mdb *mdb,
+		     struct dsa_db db)
 {
 	struct ksz_device *dev = ds->priv;
 	struct alu_struct alu;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index c6fa487fb006..66933445a447 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -166,9 +166,11 @@ void ksz_port_fast_age(struct dsa_switch *ds, int port);
 int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
 		      void *data);
 int ksz_port_mdb_add(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_mdb *mdb);
+		     const struct switchdev_obj_port_mdb *mdb,
+		     struct dsa_db db);
 int ksz_port_mdb_del(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_mdb *mdb);
+		     const struct switchdev_obj_port_mdb *mdb,
+		     struct dsa_db db);
 int ksz_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy);
 
 /* Common register access functions */
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index f74f25f479ed..abe63ec05066 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1349,7 +1349,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 
 static int
 mt7530_port_fdb_add(struct dsa_switch *ds, int port,
-		    const unsigned char *addr, u16 vid)
+		    const unsigned char *addr, u16 vid,
+		    struct dsa_db db)
 {
 	struct mt7530_priv *priv = ds->priv;
 	int ret;
@@ -1365,7 +1366,8 @@ mt7530_port_fdb_add(struct dsa_switch *ds, int port,
 
 static int
 mt7530_port_fdb_del(struct dsa_switch *ds, int port,
-		    const unsigned char *addr, u16 vid)
+		    const unsigned char *addr, u16 vid,
+		    struct dsa_db db)
 {
 	struct mt7530_priv *priv = ds->priv;
 	int ret;
@@ -1416,7 +1418,8 @@ mt7530_port_fdb_dump(struct dsa_switch *ds, int port,
 
 static int
 mt7530_port_mdb_add(struct dsa_switch *ds, int port,
-		    const struct switchdev_obj_port_mdb *mdb)
+		    const struct switchdev_obj_port_mdb *mdb,
+		    struct dsa_db db)
 {
 	struct mt7530_priv *priv = ds->priv;
 	const u8 *addr = mdb->addr;
@@ -1442,7 +1445,8 @@ mt7530_port_mdb_add(struct dsa_switch *ds, int port,
 
 static int
 mt7530_port_mdb_del(struct dsa_switch *ds, int port,
-		    const struct switchdev_obj_port_mdb *mdb)
+		    const struct switchdev_obj_port_mdb *mdb,
+		    struct dsa_db db)
 {
 	struct mt7530_priv *priv = ds->priv;
 	const u8 *addr = mdb->addr;
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 1b9a20bf1bd6..d79c65bb227e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2456,7 +2456,8 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 }
 
 static int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
-				  const unsigned char *addr, u16 vid)
+				  const unsigned char *addr, u16 vid,
+				  struct dsa_db db)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
@@ -2470,7 +2471,8 @@ static int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
 }
 
 static int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
-				  const unsigned char *addr, u16 vid)
+				  const unsigned char *addr, u16 vid,
+				  struct dsa_db db)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
@@ -6002,7 +6004,8 @@ static int mv88e6xxx_change_tag_protocol(struct dsa_switch *ds, int port,
 }
 
 static int mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
-				  const struct switchdev_obj_port_mdb *mdb)
+				  const struct switchdev_obj_port_mdb *mdb,
+				  struct dsa_db db)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
@@ -6016,7 +6019,8 @@ static int mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
 }
 
 static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
-				  const struct switchdev_obj_port_mdb *mdb)
+				  const struct switchdev_obj_port_mdb *mdb,
+				  struct dsa_db db)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 04f5da33b944..d92feee97c63 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -592,7 +592,8 @@ static int felix_fdb_dump(struct dsa_switch *ds, int port,
 }
 
 static int felix_fdb_add(struct dsa_switch *ds, int port,
-			 const unsigned char *addr, u16 vid)
+			 const unsigned char *addr, u16 vid,
+			 struct dsa_db db)
 {
 	struct ocelot *ocelot = ds->priv;
 
@@ -600,7 +601,8 @@ static int felix_fdb_add(struct dsa_switch *ds, int port,
 }
 
 static int felix_fdb_del(struct dsa_switch *ds, int port,
-			 const unsigned char *addr, u16 vid)
+			 const unsigned char *addr, u16 vid,
+			 struct dsa_db db)
 {
 	struct ocelot *ocelot = ds->priv;
 
@@ -608,7 +610,8 @@ static int felix_fdb_del(struct dsa_switch *ds, int port,
 }
 
 static int felix_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag lag,
-			     const unsigned char *addr, u16 vid)
+			     const unsigned char *addr, u16 vid,
+			     struct dsa_db db)
 {
 	struct ocelot *ocelot = ds->priv;
 
@@ -616,7 +619,8 @@ static int felix_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag lag,
 }
 
 static int felix_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag lag,
-			     const unsigned char *addr, u16 vid)
+			     const unsigned char *addr, u16 vid,
+			     struct dsa_db db)
 {
 	struct ocelot *ocelot = ds->priv;
 
@@ -624,7 +628,8 @@ static int felix_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag lag,
 }
 
 static int felix_mdb_add(struct dsa_switch *ds, int port,
-			 const struct switchdev_obj_port_mdb *mdb)
+			 const struct switchdev_obj_port_mdb *mdb,
+			 struct dsa_db db)
 {
 	struct ocelot *ocelot = ds->priv;
 
@@ -632,7 +637,8 @@ static int felix_mdb_add(struct dsa_switch *ds, int port,
 }
 
 static int felix_mdb_del(struct dsa_switch *ds, int port,
-			 const struct switchdev_obj_port_mdb *mdb)
+			 const struct switchdev_obj_port_mdb *mdb,
+			 struct dsa_db db)
 {
 	struct ocelot *ocelot = ds->priv;
 
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 6844106975a9..7189fd8120d7 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -2397,7 +2397,8 @@ qca8k_port_fdb_insert(struct qca8k_priv *priv, const u8 *addr,
 
 static int
 qca8k_port_fdb_add(struct dsa_switch *ds, int port,
-		   const unsigned char *addr, u16 vid)
+		   const unsigned char *addr, u16 vid,
+		   struct dsa_db db)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
 	u16 port_mask = BIT(port);
@@ -2407,7 +2408,8 @@ qca8k_port_fdb_add(struct dsa_switch *ds, int port,
 
 static int
 qca8k_port_fdb_del(struct dsa_switch *ds, int port,
-		   const unsigned char *addr, u16 vid)
+		   const unsigned char *addr, u16 vid,
+		   struct dsa_db db)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
 	u16 port_mask = BIT(port);
@@ -2444,7 +2446,8 @@ qca8k_port_fdb_dump(struct dsa_switch *ds, int port,
 
 static int
 qca8k_port_mdb_add(struct dsa_switch *ds, int port,
-		   const struct switchdev_obj_port_mdb *mdb)
+		   const struct switchdev_obj_port_mdb *mdb,
+		   struct dsa_db db)
 {
 	struct qca8k_priv *priv = ds->priv;
 	const u8 *addr = mdb->addr;
@@ -2455,7 +2458,8 @@ qca8k_port_mdb_add(struct dsa_switch *ds, int port,
 
 static int
 qca8k_port_mdb_del(struct dsa_switch *ds, int port,
-		   const struct switchdev_obj_port_mdb *mdb)
+		   const struct switchdev_obj_port_mdb *mdb,
+		   struct dsa_db db)
 {
 	struct qca8k_priv *priv = ds->priv;
 	const u8 *addr = mdb->addr;
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index dd89b077aae6..91b0e636d194 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1819,7 +1819,8 @@ int sja1105pqrs_fdb_del(struct dsa_switch *ds, int port,
 }
 
 static int sja1105_fdb_add(struct dsa_switch *ds, int port,
-			   const unsigned char *addr, u16 vid)
+			   const unsigned char *addr, u16 vid,
+			   struct dsa_db db)
 {
 	struct sja1105_private *priv = ds->priv;
 
@@ -1827,7 +1828,8 @@ static int sja1105_fdb_add(struct dsa_switch *ds, int port,
 }
 
 static int sja1105_fdb_del(struct dsa_switch *ds, int port,
-			   const unsigned char *addr, u16 vid)
+			   const unsigned char *addr, u16 vid,
+			   struct dsa_db db)
 {
 	struct sja1105_private *priv = ds->priv;
 
@@ -1885,7 +1887,15 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
 
 static void sja1105_fast_age(struct dsa_switch *ds, int port)
 {
+	struct dsa_port *dp = dsa_to_port(ds, port);
 	struct sja1105_private *priv = ds->priv;
+	struct dsa_db db = {
+		.type = DSA_DB_BRIDGE,
+		.bridge = {
+			.dev = dsa_port_bridge_dev_get(dp),
+			.num = dsa_port_bridge_num_get(dp),
+		},
+	};
 	int i;
 
 	for (i = 0; i < SJA1105_MAX_L2_LOOKUP_COUNT; i++) {
@@ -1913,7 +1923,7 @@ static void sja1105_fast_age(struct dsa_switch *ds, int port)
 
 		u64_to_ether_addr(l2_lookup.macaddr, macaddr);
 
-		rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid);
+		rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
 		if (rc) {
 			dev_err(ds->dev,
 				"Failed to delete FDB entry %pM vid %lld: %pe\n",
@@ -1924,15 +1934,17 @@ static void sja1105_fast_age(struct dsa_switch *ds, int port)
 }
 
 static int sja1105_mdb_add(struct dsa_switch *ds, int port,
-			   const struct switchdev_obj_port_mdb *mdb)
+			   const struct switchdev_obj_port_mdb *mdb,
+			   struct dsa_db db)
 {
-	return sja1105_fdb_add(ds, port, mdb->addr, mdb->vid);
+	return sja1105_fdb_add(ds, port, mdb->addr, mdb->vid, db);
 }
 
 static int sja1105_mdb_del(struct dsa_switch *ds, int port,
-			   const struct switchdev_obj_port_mdb *mdb)
+			   const struct switchdev_obj_port_mdb *mdb,
+			   struct dsa_db db)
 {
-	return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid);
+	return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid, db);
 }
 
 /* Common function for unicast and broadcast flood configuration.
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 01faba89c987..87c5f18eb381 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -341,11 +341,28 @@ struct dsa_link {
 	struct list_head list;
 };
 
+enum dsa_db_type {
+	DSA_DB_PORT,
+	DSA_DB_LAG,
+	DSA_DB_BRIDGE,
+};
+
+struct dsa_db {
+	enum dsa_db_type type;
+
+	union {
+		const struct dsa_port *dp;
+		struct dsa_lag lag;
+		struct dsa_bridge bridge;
+	};
+};
+
 struct dsa_mac_addr {
 	unsigned char addr[ETH_ALEN];
 	u16 vid;
 	refcount_t refcount;
 	struct list_head list;
+	struct dsa_db db;
 };
 
 struct dsa_vlan {
@@ -409,6 +426,13 @@ struct dsa_switch {
 	 */
 	u32			mtu_enforcement_ingress:1;
 
+	/* Drivers that isolate the FDBs of multiple bridges must set this
+	 * to true to receive the bridge as an argument in .port_fdb_{add,del}
+	 * and .port_mdb_{add,del}. Otherwise, the bridge.num will always be
+	 * passed as zero.
+	 */
+	u32			fdb_isolation:1;
+
 	/* Listener for switch fabric events */
 	struct notifier_block	nb;
 
@@ -941,23 +965,29 @@ struct dsa_switch_ops {
 	 * Forwarding database
 	 */
 	int	(*port_fdb_add)(struct dsa_switch *ds, int port,
-				const unsigned char *addr, u16 vid);
+				const unsigned char *addr, u16 vid,
+				struct dsa_db db);
 	int	(*port_fdb_del)(struct dsa_switch *ds, int port,
-				const unsigned char *addr, u16 vid);
+				const unsigned char *addr, u16 vid,
+				struct dsa_db db);
 	int	(*port_fdb_dump)(struct dsa_switch *ds, int port,
 				 dsa_fdb_dump_cb_t *cb, void *data);
 	int	(*lag_fdb_add)(struct dsa_switch *ds, struct dsa_lag lag,
-			       const unsigned char *addr, u16 vid);
+			       const unsigned char *addr, u16 vid,
+			       struct dsa_db db);
 	int	(*lag_fdb_del)(struct dsa_switch *ds, struct dsa_lag lag,
-			       const unsigned char *addr, u16 vid);
+			       const unsigned char *addr, u16 vid,
+			       struct dsa_db db);
 
 	/*
 	 * Multicast database
 	 */
 	int	(*port_mdb_add)(struct dsa_switch *ds, int port,
-				const struct switchdev_obj_port_mdb *mdb);
+				const struct switchdev_obj_port_mdb *mdb,
+				struct dsa_db db);
 	int	(*port_mdb_del)(struct dsa_switch *ds, int port,
-				const struct switchdev_obj_port_mdb *mdb);
+				const struct switchdev_obj_port_mdb *mdb,
+				struct dsa_db db);
 	/*
 	 * RXNFC
 	 */
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7a1c98581f53..27575fc3883e 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -67,6 +67,7 @@ struct dsa_notifier_fdb_info {
 	int port;
 	const unsigned char *addr;
 	u16 vid;
+	struct dsa_db db;
 };
 
 /* DSA_NOTIFIER_LAG_FDB_* */
@@ -74,6 +75,7 @@ struct dsa_notifier_lag_fdb_info {
 	struct dsa_lag *lag;
 	const unsigned char *addr;
 	u16 vid;
+	struct dsa_db db;
 };
 
 /* DSA_NOTIFIER_MDB_* */
@@ -81,6 +83,7 @@ struct dsa_notifier_mdb_info {
 	const struct switchdev_obj_port_mdb *mdb;
 	int sw_index;
 	int port;
+	struct dsa_db db;
 };
 
 /* DSA_NOTIFIER_LAG_* */
diff --git a/net/dsa/port.c b/net/dsa/port.c
index adab159c8c21..7af44a28f032 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -798,8 +798,19 @@ int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		.port = dp->index,
 		.addr = addr,
 		.vid = vid,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 
+	/* Refcounting takes bridge.num as a key, and should be global for all
+	 * bridges in the absence of FDB isolation, and per bridge otherwise.
+	 * Force the bridge.num to zero here in the absence of FDB isolation.
+	 */
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_FDB_ADD, &info);
 }
 
@@ -811,9 +822,15 @@ int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 		.port = dp->index,
 		.addr = addr,
 		.vid = vid,
-
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_FDB_DEL, &info);
 }
 
@@ -825,6 +842,10 @@ int dsa_port_host_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		.port = dp->index,
 		.addr = addr,
 		.vid = vid,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 	struct dsa_port *cpu_dp = dp->cpu_dp;
 	int err;
@@ -839,6 +860,9 @@ int dsa_port_host_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 			return err;
 	}
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_FDB_ADD, &info);
 }
 
@@ -850,6 +874,10 @@ int dsa_port_host_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 		.port = dp->index,
 		.addr = addr,
 		.vid = vid,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 	struct dsa_port *cpu_dp = dp->cpu_dp;
 	int err;
@@ -860,6 +888,9 @@ int dsa_port_host_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 			return err;
 	}
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_FDB_DEL, &info);
 }
 
@@ -870,8 +901,15 @@ int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		.lag = dp->lag,
 		.addr = addr,
 		.vid = vid,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_ADD, &info);
 }
 
@@ -882,8 +920,15 @@ int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 		.lag = dp->lag,
 		.addr = addr,
 		.vid = vid,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_DEL, &info);
 }
 
@@ -905,8 +950,15 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
 		.sw_index = dp->ds->index,
 		.port = dp->index,
 		.mdb = mdb,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_ADD, &info);
 }
 
@@ -917,8 +969,15 @@ int dsa_port_mdb_del(const struct dsa_port *dp,
 		.sw_index = dp->ds->index,
 		.port = dp->index,
 		.mdb = mdb,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_DEL, &info);
 }
 
@@ -929,6 +988,10 @@ int dsa_port_host_mdb_add(const struct dsa_port *dp,
 		.sw_index = dp->ds->index,
 		.port = dp->index,
 		.mdb = mdb,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 	struct dsa_port *cpu_dp = dp->cpu_dp;
 	int err;
@@ -937,6 +1000,9 @@ int dsa_port_host_mdb_add(const struct dsa_port *dp,
 	if (err)
 		return err;
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_ADD, &info);
 }
 
@@ -947,6 +1013,10 @@ int dsa_port_host_mdb_del(const struct dsa_port *dp,
 		.sw_index = dp->ds->index,
 		.port = dp->index,
 		.mdb = mdb,
+		.db = {
+			.type = DSA_DB_BRIDGE,
+			.bridge = *dp->bridge,
+		},
 	};
 	struct dsa_port *cpu_dp = dp->cpu_dp;
 	int err;
@@ -955,6 +1025,9 @@ int dsa_port_host_mdb_del(const struct dsa_port *dp,
 	if (err)
 		return err;
 
+	if (!dp->ds->fdb_isolation)
+		info.db.bridge.num = 0;
+
 	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_DEL, &info);
 }
 
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index eb38beb10147..1d3c161e3131 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -210,21 +210,41 @@ static bool dsa_port_host_address_match(struct dsa_port *dp,
 	return false;
 }
 
+static bool dsa_db_equal(const struct dsa_db *a, const struct dsa_db *b)
+{
+	if (a->type != b->type)
+		return false;
+
+	switch (a->type) {
+	case DSA_DB_PORT:
+		return a->dp == b->dp;
+	case DSA_DB_LAG:
+		return a->lag.dev == b->lag.dev;
+	case DSA_DB_BRIDGE:
+		return a->bridge.num == b->bridge.num;
+	default:
+		WARN_ON(1);
+		return false;
+	}
+}
+
 static struct dsa_mac_addr *dsa_mac_addr_find(struct list_head *addr_list,
-					      const unsigned char *addr,
-					      u16 vid)
+					      const unsigned char *addr, u16 vid,
+					      struct dsa_db db)
 {
 	struct dsa_mac_addr *a;
 
 	list_for_each_entry(a, addr_list, list)
-		if (ether_addr_equal(a->addr, addr) && a->vid == vid)
+		if (ether_addr_equal(a->addr, addr) && a->vid == vid &&
+		    dsa_db_equal(&a->db, &db))
 			return a;
 
 	return NULL;
 }
 
 static int dsa_port_do_mdb_add(struct dsa_port *dp,
-			       const struct switchdev_obj_port_mdb *mdb)
+			       const struct switchdev_obj_port_mdb *mdb,
+			       struct dsa_db db)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_mac_addr *a;
@@ -233,11 +253,11 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp,
 
 	/* No need to bother with refcounting for user ports */
 	if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)))
-		return ds->ops->port_mdb_add(ds, port, mdb);
+		return ds->ops->port_mdb_add(ds, port, mdb, db);
 
 	mutex_lock(&dp->addr_lists_lock);
 
-	a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid);
+	a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid, db);
 	if (a) {
 		refcount_inc(&a->refcount);
 		goto out;
@@ -249,7 +269,7 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp,
 		goto out;
 	}
 
-	err = ds->ops->port_mdb_add(ds, port, mdb);
+	err = ds->ops->port_mdb_add(ds, port, mdb, db);
 	if (err) {
 		kfree(a);
 		goto out;
@@ -257,6 +277,7 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp,
 
 	ether_addr_copy(a->addr, mdb->addr);
 	a->vid = mdb->vid;
+	a->db = db;
 	refcount_set(&a->refcount, 1);
 	list_add_tail(&a->list, &dp->mdbs);
 
@@ -267,7 +288,8 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp,
 }
 
 static int dsa_port_do_mdb_del(struct dsa_port *dp,
-			       const struct switchdev_obj_port_mdb *mdb)
+			       const struct switchdev_obj_port_mdb *mdb,
+			       struct dsa_db db)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_mac_addr *a;
@@ -276,11 +298,11 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp,
 
 	/* No need to bother with refcounting for user ports */
 	if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)))
-		return ds->ops->port_mdb_del(ds, port, mdb);
+		return ds->ops->port_mdb_del(ds, port, mdb, db);
 
 	mutex_lock(&dp->addr_lists_lock);
 
-	a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid);
+	a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid, db);
 	if (!a) {
 		err = -ENOENT;
 		goto out;
@@ -289,7 +311,7 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp,
 	if (!refcount_dec_and_test(&a->refcount))
 		goto out;
 
-	err = ds->ops->port_mdb_del(ds, port, mdb);
+	err = ds->ops->port_mdb_del(ds, port, mdb, db);
 	if (err) {
 		refcount_set(&a->refcount, 1);
 		goto out;
@@ -305,7 +327,7 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp,
 }
 
 static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
-			       u16 vid)
+			       u16 vid, struct dsa_db db)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_mac_addr *a;
@@ -314,11 +336,11 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 
 	/* No need to bother with refcounting for user ports */
 	if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)))
-		return ds->ops->port_fdb_add(ds, port, addr, vid);
+		return ds->ops->port_fdb_add(ds, port, addr, vid, db);
 
 	mutex_lock(&dp->addr_lists_lock);
 
-	a = dsa_mac_addr_find(&dp->fdbs, addr, vid);
+	a = dsa_mac_addr_find(&dp->fdbs, addr, vid, db);
 	if (a) {
 		refcount_inc(&a->refcount);
 		goto out;
@@ -330,7 +352,7 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		goto out;
 	}
 
-	err = ds->ops->port_fdb_add(ds, port, addr, vid);
+	err = ds->ops->port_fdb_add(ds, port, addr, vid, db);
 	if (err) {
 		kfree(a);
 		goto out;
@@ -338,6 +360,7 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 
 	ether_addr_copy(a->addr, addr);
 	a->vid = vid;
+	a->db = db;
 	refcount_set(&a->refcount, 1);
 	list_add_tail(&a->list, &dp->fdbs);
 
@@ -348,7 +371,7 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 }
 
 static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
-			       u16 vid)
+			       u16 vid, struct dsa_db db)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_mac_addr *a;
@@ -357,11 +380,11 @@ static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 
 	/* No need to bother with refcounting for user ports */
 	if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)))
-		return ds->ops->port_fdb_del(ds, port, addr, vid);
+		return ds->ops->port_fdb_del(ds, port, addr, vid, db);
 
 	mutex_lock(&dp->addr_lists_lock);
 
-	a = dsa_mac_addr_find(&dp->fdbs, addr, vid);
+	a = dsa_mac_addr_find(&dp->fdbs, addr, vid, db);
 	if (!a) {
 		err = -ENOENT;
 		goto out;
@@ -370,7 +393,7 @@ static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 	if (!refcount_dec_and_test(&a->refcount))
 		goto out;
 
-	err = ds->ops->port_fdb_del(ds, port, addr, vid);
+	err = ds->ops->port_fdb_del(ds, port, addr, vid, db);
 	if (err) {
 		refcount_set(&a->refcount, 1);
 		goto out;
@@ -386,14 +409,15 @@ static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 }
 
 static int dsa_switch_do_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag *lag,
-				     const unsigned char *addr, u16 vid)
+				     const unsigned char *addr, u16 vid,
+				     struct dsa_db db)
 {
 	struct dsa_mac_addr *a;
 	int err = 0;
 
 	mutex_lock(&lag->fdb_lock);
 
-	a = dsa_mac_addr_find(&lag->fdbs, addr, vid);
+	a = dsa_mac_addr_find(&lag->fdbs, addr, vid, db);
 	if (a) {
 		refcount_inc(&a->refcount);
 		goto out;
@@ -405,7 +429,7 @@ static int dsa_switch_do_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag *lag,
 		goto out;
 	}
 
-	err = ds->ops->lag_fdb_add(ds, *lag, addr, vid);
+	err = ds->ops->lag_fdb_add(ds, *lag, addr, vid, db);
 	if (err) {
 		kfree(a);
 		goto out;
@@ -423,14 +447,15 @@ static int dsa_switch_do_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag *lag,
 }
 
 static int dsa_switch_do_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag *lag,
-				     const unsigned char *addr, u16 vid)
+				     const unsigned char *addr, u16 vid,
+				     struct dsa_db db)
 {
 	struct dsa_mac_addr *a;
 	int err = 0;
 
 	mutex_lock(&lag->fdb_lock);
 
-	a = dsa_mac_addr_find(&lag->fdbs, addr, vid);
+	a = dsa_mac_addr_find(&lag->fdbs, addr, vid, db);
 	if (!a) {
 		err = -ENOENT;
 		goto out;
@@ -439,7 +464,7 @@ static int dsa_switch_do_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag *lag,
 	if (!refcount_dec_and_test(&a->refcount))
 		goto out;
 
-	err = ds->ops->lag_fdb_del(ds, *lag, addr, vid);
+	err = ds->ops->lag_fdb_del(ds, *lag, addr, vid, db);
 	if (err) {
 		refcount_set(&a->refcount, 1);
 		goto out;
@@ -466,7 +491,8 @@ static int dsa_switch_host_fdb_add(struct dsa_switch *ds,
 	dsa_switch_for_each_port(dp, ds) {
 		if (dsa_port_host_address_match(dp, info->sw_index,
 						info->port)) {
-			err = dsa_port_do_fdb_add(dp, info->addr, info->vid);
+			err = dsa_port_do_fdb_add(dp, info->addr, info->vid,
+						  info->db);
 			if (err)
 				break;
 		}
@@ -487,7 +513,8 @@ static int dsa_switch_host_fdb_del(struct dsa_switch *ds,
 	dsa_switch_for_each_port(dp, ds) {
 		if (dsa_port_host_address_match(dp, info->sw_index,
 						info->port)) {
-			err = dsa_port_do_fdb_del(dp, info->addr, info->vid);
+			err = dsa_port_do_fdb_del(dp, info->addr, info->vid,
+						  info->db);
 			if (err)
 				break;
 		}
@@ -505,7 +532,7 @@ static int dsa_switch_fdb_add(struct dsa_switch *ds,
 	if (!ds->ops->port_fdb_add)
 		return -EOPNOTSUPP;
 
-	return dsa_port_do_fdb_add(dp, info->addr, info->vid);
+	return dsa_port_do_fdb_add(dp, info->addr, info->vid, info->db);
 }
 
 static int dsa_switch_fdb_del(struct dsa_switch *ds,
@@ -517,7 +544,7 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds,
 	if (!ds->ops->port_fdb_del)
 		return -EOPNOTSUPP;
 
-	return dsa_port_do_fdb_del(dp, info->addr, info->vid);
+	return dsa_port_do_fdb_del(dp, info->addr, info->vid, info->db);
 }
 
 static int dsa_switch_lag_fdb_add(struct dsa_switch *ds,
@@ -532,7 +559,8 @@ static int dsa_switch_lag_fdb_add(struct dsa_switch *ds,
 	dsa_switch_for_each_port(dp, ds)
 		if (dsa_port_offloads_lag(dp, info->lag))
 			return dsa_switch_do_lag_fdb_add(ds, info->lag,
-							 info->addr, info->vid);
+							 info->addr, info->vid,
+							 info->db);
 
 	return 0;
 }
@@ -549,7 +577,8 @@ static int dsa_switch_lag_fdb_del(struct dsa_switch *ds,
 	dsa_switch_for_each_port(dp, ds)
 		if (dsa_port_offloads_lag(dp, info->lag))
 			return dsa_switch_do_lag_fdb_del(ds, info->lag,
-							 info->addr, info->vid);
+							 info->addr, info->vid,
+							 info->db);
 
 	return 0;
 }
@@ -604,7 +633,7 @@ static int dsa_switch_mdb_add(struct dsa_switch *ds,
 	if (!ds->ops->port_mdb_add)
 		return -EOPNOTSUPP;
 
-	return dsa_port_do_mdb_add(dp, info->mdb);
+	return dsa_port_do_mdb_add(dp, info->mdb, info->db);
 }
 
 static int dsa_switch_mdb_del(struct dsa_switch *ds,
@@ -616,7 +645,7 @@ static int dsa_switch_mdb_del(struct dsa_switch *ds,
 	if (!ds->ops->port_mdb_del)
 		return -EOPNOTSUPP;
 
-	return dsa_port_do_mdb_del(dp, info->mdb);
+	return dsa_port_do_mdb_del(dp, info->mdb, info->db);
 }
 
 static int dsa_switch_host_mdb_add(struct dsa_switch *ds,
@@ -631,7 +660,7 @@ static int dsa_switch_host_mdb_add(struct dsa_switch *ds,
 	dsa_switch_for_each_port(dp, ds) {
 		if (dsa_port_host_address_match(dp, info->sw_index,
 						info->port)) {
-			err = dsa_port_do_mdb_add(dp, info->mdb);
+			err = dsa_port_do_mdb_add(dp, info->mdb, info->db);
 			if (err)
 				break;
 		}
@@ -652,7 +681,7 @@ static int dsa_switch_host_mdb_del(struct dsa_switch *ds,
 	dsa_switch_for_each_port(dp, ds) {
 		if (dsa_port_host_address_match(dp, info->sw_index,
 						info->port)) {
-			err = dsa_port_do_mdb_del(dp, info->mdb);
+			err = dsa_port_do_mdb_del(dp, info->mdb, info->db);
 			if (err)
 				break;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 08/10] net: dsa: pass extack to .port_bridge_join driver methods
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
                   ` (6 preceding siblings ...)
  2022-02-25  9:22 ` [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 09/10] net: dsa: sja1105: enforce FDB isolation Vladimir Oltean
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

As FDB isolation cannot be enforced between VLAN-aware bridges in lack
of hardware assistance like extra FID bits, it seems plausible that many
DSA switches cannot do it. Therefore, they need to reject configurations
with multiple VLAN-aware bridges from the two code paths that can
transition towards that state:

- joining a VLAN-aware bridge
- toggling VLAN awareness on an existing bridge

The .port_vlan_filtering method already propagates the netlink extack to
the driver, let's propagate it from .port_bridge_join too, to make sure
that the driver can use the same function for both.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c       | 2 +-
 drivers/net/dsa/b53/b53_priv.h         | 2 +-
 drivers/net/dsa/dsa_loop.c             | 3 ++-
 drivers/net/dsa/hirschmann/hellcreek.c | 3 ++-
 drivers/net/dsa/lan9303-core.c         | 3 ++-
 drivers/net/dsa/lantiq_gswip.c         | 3 ++-
 drivers/net/dsa/microchip/ksz_common.c | 3 ++-
 drivers/net/dsa/microchip/ksz_common.h | 3 ++-
 drivers/net/dsa/mt7530.c               | 3 ++-
 drivers/net/dsa/mv88e6xxx/chip.c       | 6 ++++--
 drivers/net/dsa/ocelot/felix.c         | 3 ++-
 drivers/net/dsa/qca8k.c                | 3 ++-
 drivers/net/dsa/realtek/rtl8366rb.c    | 3 ++-
 drivers/net/dsa/sja1105/sja1105_main.c | 3 ++-
 drivers/net/dsa/xrs700x/xrs700x.c      | 3 ++-
 include/net/dsa.h                      | 6 ++++--
 net/dsa/dsa_priv.h                     | 1 +
 net/dsa/port.c                         | 1 +
 net/dsa/switch.c                       | 6 ++++--
 19 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a8cc6e182c45..122e63762979 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1869,7 +1869,7 @@ int b53_mdb_del(struct dsa_switch *ds, int port,
 EXPORT_SYMBOL(b53_mdb_del);
 
 int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
-		bool *tx_fwd_offload)
+		bool *tx_fwd_offload, struct netlink_ext_ack *extack)
 {
 	struct b53_device *dev = ds->priv;
 	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index d3091f0ad3e6..86e7eb7924e7 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -324,7 +324,7 @@ void b53_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data);
 int b53_get_sset_count(struct dsa_switch *ds, int port, int sset);
 void b53_get_ethtool_phy_stats(struct dsa_switch *ds, int port, uint64_t *data);
 int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
-		bool *tx_fwd_offload);
+		bool *tx_fwd_offload, struct netlink_ext_ack *extack);
 void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
 void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
 void b53_br_fast_age(struct dsa_switch *ds, int port);
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index 33daaf10c488..263e41191c29 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -168,7 +168,8 @@ static int dsa_loop_phy_write(struct dsa_switch *ds, int port,
 
 static int dsa_loop_port_bridge_join(struct dsa_switch *ds, int port,
 				     struct dsa_bridge bridge,
-				     bool *tx_fwd_offload)
+				     bool *tx_fwd_offload,
+				     struct netlink_ext_ack *extack)
 {
 	dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
 		__func__, port, bridge.dev->name);
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index cb89be9de43a..ac1f3b3a7040 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -675,7 +675,8 @@ static int hellcreek_bridge_flags(struct dsa_switch *ds, int port,
 
 static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
 				      struct dsa_bridge bridge,
-				      bool *tx_fwd_offload)
+				      bool *tx_fwd_offload,
+				      struct netlink_ext_ack *extack)
 {
 	struct hellcreek *hellcreek = ds->priv;
 
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index a21184e7fcb6..e03ff1f267bb 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1111,7 +1111,8 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port)
 
 static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
 				    struct dsa_bridge bridge,
-				    bool *tx_fwd_offload)
+				    bool *tx_fwd_offload,
+				    struct netlink_ext_ack *extack)
 {
 	struct lan9303 *chip = ds->priv;
 
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 3dfb532b7784..a8bd233f3cb9 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1152,7 +1152,8 @@ static int gswip_vlan_remove(struct gswip_priv *priv,
 
 static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
 				  struct dsa_bridge bridge,
-				  bool *tx_fwd_offload)
+				  bool *tx_fwd_offload,
+				  struct netlink_ext_ack *extack)
 {
 	struct net_device *br = bridge.dev;
 	struct gswip_priv *priv = ds->priv;
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 104458ec9cbc..8014b18d9391 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -217,7 +217,8 @@ EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
 
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 			 struct dsa_bridge bridge,
-			 bool *tx_fwd_offload)
+			 bool *tx_fwd_offload,
+			 struct netlink_ext_ack *extack)
 {
 	/* port_stp_state_set() will be called after to put the port in
 	 * appropriate state so there is no need to do anything.
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 66933445a447..4ff0a159ce3c 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -159,7 +159,8 @@ void ksz_mac_link_down(struct dsa_switch *ds, int port, unsigned int mode,
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
 void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf);
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
-			 struct dsa_bridge bridge, bool *tx_fwd_offload);
+			 struct dsa_bridge bridge, bool *tx_fwd_offload,
+			 struct netlink_ext_ack *extack);
 void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
 			   struct dsa_bridge bridge);
 void ksz_port_fast_age(struct dsa_switch *ds, int port);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index abe63ec05066..66b00c19ebe0 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1186,7 +1186,8 @@ mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
 
 static int
 mt7530_port_bridge_join(struct dsa_switch *ds, int port,
-			struct dsa_bridge bridge, bool *tx_fwd_offload)
+			struct dsa_bridge bridge, bool *tx_fwd_offload,
+			struct netlink_ext_ack *extack)
 {
 	struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
 	u32 port_bitmap = BIT(MT7530_CPU_PORT);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d79c65bb227e..84b90fc36c58 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2618,7 +2618,8 @@ static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
 
 static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 				      struct dsa_bridge bridge,
-				      bool *tx_fwd_offload)
+				      bool *tx_fwd_offload,
+				      struct netlink_ext_ack *extack)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
@@ -2684,7 +2685,8 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
 
 static int mv88e6xxx_crosschip_bridge_join(struct dsa_switch *ds,
 					   int tree_index, int sw_index,
-					   int port, struct dsa_bridge bridge)
+					   int port, struct dsa_bridge bridge,
+					   struct netlink_ext_ack *extack)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index d92feee97c63..0b1bcbd230ee 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -674,7 +674,8 @@ static int felix_bridge_flags(struct dsa_switch *ds, int port,
 }
 
 static int felix_bridge_join(struct dsa_switch *ds, int port,
-			     struct dsa_bridge bridge, bool *tx_fwd_offload)
+			     struct dsa_bridge bridge, bool *tx_fwd_offload,
+			     struct netlink_ext_ack *extack)
 {
 	struct ocelot *ocelot = ds->priv;
 
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 7189fd8120d7..4585332aa7a8 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -2246,7 +2246,8 @@ qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
 
 static int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
 				  struct dsa_bridge bridge,
-				  bool *tx_fwd_offload)
+				  bool *tx_fwd_offload,
+				  struct netlink_ext_ack *extack)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
 	int port_mask, cpu_port;
diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
index fb6565e68401..1a3406b9e64c 100644
--- a/drivers/net/dsa/realtek/rtl8366rb.c
+++ b/drivers/net/dsa/realtek/rtl8366rb.c
@@ -1189,7 +1189,8 @@ rtl8366rb_port_disable(struct dsa_switch *ds, int port)
 static int
 rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
 			   struct dsa_bridge bridge,
-			   bool *tx_fwd_offload)
+			   bool *tx_fwd_offload,
+			   struct netlink_ext_ack *extack)
 {
 	struct realtek_priv *priv = ds->priv;
 	unsigned int port_bitmap = 0;
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 91b0e636d194..13bb05241d53 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2087,7 +2087,8 @@ 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 dsa_bridge bridge,
-			       bool *tx_fwd_offload)
+			       bool *tx_fwd_offload,
+			       struct netlink_ext_ack *extack)
 {
 	int rc;
 
diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index bc06fe6bac6b..3887ed33c5fe 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -534,7 +534,8 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
 }
 
 static int xrs700x_bridge_join(struct dsa_switch *ds, int port,
-			       struct dsa_bridge bridge, bool *tx_fwd_offload)
+			       struct dsa_bridge bridge, bool *tx_fwd_offload,
+			       struct netlink_ext_ack *extack)
 {
 	return xrs700x_bridge_common(ds, port, bridge, true);
 }
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 87c5f18eb381..cfedcfb86350 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -937,7 +937,8 @@ struct dsa_switch_ops {
 	int	(*set_ageing_time)(struct dsa_switch *ds, unsigned int msecs);
 	int	(*port_bridge_join)(struct dsa_switch *ds, int port,
 				    struct dsa_bridge bridge,
-				    bool *tx_fwd_offload);
+				    bool *tx_fwd_offload,
+				    struct netlink_ext_ack *extack);
 	void	(*port_bridge_leave)(struct dsa_switch *ds, int port,
 				     struct dsa_bridge bridge);
 	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
@@ -1021,7 +1022,8 @@ struct dsa_switch_ops {
 	 */
 	int	(*crosschip_bridge_join)(struct dsa_switch *ds, int tree_index,
 					 int sw_index, int port,
-					 struct dsa_bridge bridge);
+					 struct dsa_bridge bridge,
+					 struct netlink_ext_ack *extack);
 	void	(*crosschip_bridge_leave)(struct dsa_switch *ds, int tree_index,
 					  int sw_index, int port,
 					  struct dsa_bridge bridge);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 27575fc3883e..07c0ad52395a 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -59,6 +59,7 @@ struct dsa_notifier_bridge_info {
 	int sw_index;
 	int port;
 	bool tx_fwd_offload;
+	struct netlink_ext_ack *extack;
 };
 
 /* DSA_NOTIFIER_FDB_* */
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 7af44a28f032..d9da425a17fb 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -328,6 +328,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 		.tree_index = dp->ds->dst->index,
 		.sw_index = dp->ds->index,
 		.port = dp->index,
+		.extack = extack,
 	};
 	struct net_device *dev = dp->slave;
 	struct net_device *brport_dev;
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 1d3c161e3131..327d66bf7b47 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -96,7 +96,8 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
 			return -EOPNOTSUPP;
 
 		err = ds->ops->port_bridge_join(ds, info->port, info->bridge,
-						&info->tx_fwd_offload);
+						&info->tx_fwd_offload,
+						info->extack);
 		if (err)
 			return err;
 	}
@@ -105,7 +106,8 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
 	    ds->ops->crosschip_bridge_join) {
 		err = ds->ops->crosschip_bridge_join(ds, info->tree_index,
 						     info->sw_index,
-						     info->port, info->bridge);
+						     info->port, info->bridge,
+						     info->extack);
 		if (err)
 			return err;
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 09/10] net: dsa: sja1105: enforce FDB isolation
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
                   ` (7 preceding siblings ...)
  2022-02-25  9:22 ` [PATCH v2 net-next 08/10] net: dsa: pass extack to .port_bridge_join driver methods Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-25  9:22 ` [PATCH v2 net-next 10/10] net: mscc: ocelot: enforce FDB isolation when VLAN-unaware Vladimir Oltean
  2022-02-27 11:10 ` [PATCH v2 net-next 00/10] DSA FDB isolation patchwork-bot+netdevbpf
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

For sja1105, to enforce FDB isolation simply means to turn on
Independent VLAN Learning unconditionally, and to remap VLAN-unaware FDB
and MDB entries towards the private VLAN allocated by tag_8021q for each
bridge.

Standalone ports each have their own standalone tag_8021q VLAN. No
learning happens in that VLAN due to:
- learning being disabled on standalone user ports
- learning being disabled on the CPU port (we use
  assisted_learning_on_cpu_port which only installs bridge FDBs)

VLAN-aware ports learn FDB entries with the bridge VLANs.

VLAN-unaware bridge ports learn with the tag_8021q VLAN for bridging.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 59 +++++++++++++-------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 13bb05241d53..1d7d31a1d27b 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -393,10 +393,8 @@ static int sja1105_init_l2_lookup_params(struct sja1105_private *priv)
 		.start_dynspc = 0,
 		/* 2^8 + 2^5 + 2^3 + 2^2 + 2^1 + 1 in Koopman notation */
 		.poly = 0x97,
-		/* This selects between Independent VLAN Learning (IVL) and
-		 * Shared VLAN Learning (SVL)
-		 */
-		.shared_learn = true,
+		/* Always use Independent VLAN Learning (IVL) */
+		.shared_learn = false,
 		/* Don't discard management traffic based on ENFPORT -
 		 * we don't perform SMAC port enforcement anyway, so
 		 * what we are setting here doesn't matter.
@@ -1824,6 +1822,19 @@ static int sja1105_fdb_add(struct dsa_switch *ds, int port,
 {
 	struct sja1105_private *priv = ds->priv;
 
+	if (!vid) {
+		switch (db.type) {
+		case DSA_DB_PORT:
+			vid = dsa_tag_8021q_standalone_vid(db.dp);
+			break;
+		case DSA_DB_BRIDGE:
+			vid = dsa_tag_8021q_bridge_vid(db.bridge.num);
+			break;
+		default:
+			return -EOPNOTSUPP;
+		}
+	}
+
 	return priv->info->fdb_add_cmd(ds, port, addr, vid);
 }
 
@@ -1833,13 +1844,25 @@ static int sja1105_fdb_del(struct dsa_switch *ds, int port,
 {
 	struct sja1105_private *priv = ds->priv;
 
+	if (!vid) {
+		switch (db.type) {
+		case DSA_DB_PORT:
+			vid = dsa_tag_8021q_standalone_vid(db.dp);
+			break;
+		case DSA_DB_BRIDGE:
+			vid = dsa_tag_8021q_bridge_vid(db.bridge.num);
+			break;
+		default:
+			return -EOPNOTSUPP;
+		}
+	}
+
 	return priv->info->fdb_del_cmd(ds, port, addr, vid);
 }
 
 static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
 			    dsa_fdb_dump_cb_t *cb, void *data)
 {
-	struct dsa_port *dp = dsa_to_port(ds, port);
 	struct sja1105_private *priv = ds->priv;
 	struct device *dev = ds->dev;
 	int i;
@@ -1876,7 +1899,7 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
 		u64_to_ether_addr(l2_lookup.macaddr, macaddr);
 
 		/* We need to hide the dsa_8021q VLANs from the user. */
-		if (!dsa_port_is_vlan_filtering(dp))
+		if (vid_is_dsa_8021q(l2_lookup.vlanid))
 			l2_lookup.vlanid = 0;
 		rc = cb(macaddr, l2_lookup.vlanid, l2_lookup.lockeds, data);
 		if (rc)
@@ -2370,7 +2393,6 @@ sja1105_get_tag_protocol(struct dsa_switch *ds, int port,
 int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
 			   struct netlink_ext_ack *extack)
 {
-	struct sja1105_l2_lookup_params_entry *l2_lookup_params;
 	struct sja1105_general_params_entry *general_params;
 	struct sja1105_private *priv = ds->priv;
 	struct sja1105_table *table;
@@ -2408,28 +2430,6 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
 	general_params->incl_srcpt1 = enabled;
 	general_params->incl_srcpt0 = enabled;
 
-	/* VLAN filtering => independent VLAN learning.
-	 * No VLAN filtering (or best effort) => shared VLAN learning.
-	 *
-	 * In shared VLAN learning mode, untagged traffic still gets
-	 * pvid-tagged, and the FDB table gets populated with entries
-	 * containing the "real" (pvid or from VLAN tag) VLAN ID.
-	 * However the switch performs a masked L2 lookup in the FDB,
-	 * effectively only looking up a frame's DMAC (and not VID) for the
-	 * forwarding decision.
-	 *
-	 * This is extremely convenient for us, because in modes with
-	 * vlan_filtering=0, dsa_8021q actually installs unique pvid's into
-	 * each front panel port. This is good for identification but breaks
-	 * learning badly - the VID of the learnt FDB entry is unique, aka
-	 * no frames coming from any other port are going to have it. So
-	 * for forwarding purposes, this is as though learning was broken
-	 * (all frames get flooded).
-	 */
-	table = &priv->static_config.tables[BLK_IDX_L2_LOOKUP_PARAMS];
-	l2_lookup_params = table->entries;
-	l2_lookup_params->shared_learn = !enabled;
-
 	for (port = 0; port < ds->num_ports; port++) {
 		if (dsa_is_unused_port(ds, port))
 			continue;
@@ -3115,6 +3115,7 @@ static int sja1105_setup(struct dsa_switch *ds)
 	 */
 	ds->vlan_filtering_is_global = true;
 	ds->untag_bridge_pvid = true;
+	ds->fdb_isolation = true;
 	/* tag_8021q has 3 bits for the VBID, and the value 0 is reserved */
 	ds->max_num_bridges = 7;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 net-next 10/10] net: mscc: ocelot: enforce FDB isolation when VLAN-unaware
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
                   ` (8 preceding siblings ...)
  2022-02-25  9:22 ` [PATCH v2 net-next 09/10] net: dsa: sja1105: enforce FDB isolation Vladimir Oltean
@ 2022-02-25  9:22 ` Vladimir Oltean
  2022-02-27 11:10 ` [PATCH v2 net-next 00/10] DSA FDB isolation patchwork-bot+netdevbpf
  10 siblings, 0 replies; 19+ messages in thread
From: Vladimir Oltean @ 2022-02-25  9:22 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

Currently ocelot uses a pvid of 0 for standalone ports and ports under a
VLAN-unaware bridge, and the pvid of the bridge for ports under a
VLAN-aware bridge. Standalone ports do not perform learning, but packets
received on them are still subject to FDB lookups. So if the MAC DA that
a standalone port receives has been also learned on a VLAN-unaware
bridge port, ocelot will attempt to forward to that port, even though it
can't, so it will drop packets.

So there is a desire to avoid that, and isolate the FDBs of different
bridges from one another, and from standalone ports.

The ocelot switch library has two distinct entry points: the felix DSA
driver and the ocelot switchdev driver.

We need to code up a minimal bridge_num allocation in the ocelot
switchdev driver too, this is copied from DSA with the exception that
ocelot does not care about DSA trees, cross-chip bridging etc. So it
only looks at its own ports that are already in the same bridge.

The ocelot switchdev driver uses the bridge_num it has allocated itself,
while the felix driver uses the bridge_num allocated by DSA. They are
both stored inside ocelot_port->bridge_num by the common function
ocelot_port_bridge_join() which receives the bridge_num passed by value.

Once we have a bridge_num, we can only use it to enforce isolation
between VLAN-unaware bridges. As far as I can see, ocelot does not have
anything like a FID that further makes VLAN 100 from a port be different
to VLAN 100 from another port with regard to FDB lookup. So we simply
deny multiple VLAN-aware bridges.

For VLAN-unaware bridges, we crop the 4000-4095 VLAN region and we
allocate a VLAN for each bridge_num. This will be used as the pvid of
each port that is under that VLAN-unaware bridge, for as long as that
bridge is VLAN-unaware.

VID 0 remains only for standalone ports. It is okay if all standalone
ports use the same VID 0, since they perform no address learning, the
FDB will contain no entry in VLAN 0, so the packets will always be
flooded to the only possible destination, the CPU port.

The CPU port module doesn't need to be member of the VLANs to receive
packets, but if we use the DSA tag_8021q protocol, those packets are
part of the data plane as far as ocelot is concerned, so there it needs
to. Just ensure that the DSA tag_8021q CPU port is a member of all
reserved VLANs when it is created, and is removed when it is deleted.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c         |  63 ++++++--
 drivers/net/ethernet/mscc/ocelot.c     | 200 ++++++++++++++++++++++---
 drivers/net/ethernet/mscc/ocelot.h     |   5 +-
 drivers/net/ethernet/mscc/ocelot_mrp.c |   8 +-
 drivers/net/ethernet/mscc/ocelot_net.c |  66 ++++++--
 include/soc/mscc/ocelot.h              |  31 ++--
 6 files changed, 317 insertions(+), 56 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 0b1bcbd230ee..3f2bf73b274f 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -234,7 +234,7 @@ static void felix_8021q_cpu_port_init(struct ocelot *ocelot, int port)
 {
 	mutex_lock(&ocelot->fwd_domain_lock);
 
-	ocelot->ports[port]->is_dsa_8021q_cpu = true;
+	ocelot_port_set_dsa_8021q_cpu(ocelot, port);
 	ocelot->npi = -1;
 
 	/* Overwrite PGID_CPU with the non-tagging port */
@@ -250,6 +250,7 @@ static void felix_8021q_cpu_port_deinit(struct ocelot *ocelot, int port)
 	mutex_lock(&ocelot->fwd_domain_lock);
 
 	ocelot->ports[port]->is_dsa_8021q_cpu = false;
+	ocelot_port_unset_dsa_8021q_cpu(ocelot, port);
 
 	/* Restore PGID_CPU */
 	ocelot_write_rix(ocelot, BIT(ocelot->num_phys_ports), ANA_PGID_PGID,
@@ -591,58 +592,99 @@ static int felix_fdb_dump(struct dsa_switch *ds, int port,
 	return ocelot_fdb_dump(ocelot, port, cb, data);
 }
 
+/* Translate the DSA database API into the ocelot switch library API,
+ * which uses VID 0 for all ports that aren't part of a bridge,
+ * and expects the bridge_dev to be NULL in that case.
+ */
+static struct net_device *felix_classify_db(struct dsa_db db)
+{
+	switch (db.type) {
+	case DSA_DB_PORT:
+	case DSA_DB_LAG:
+		return NULL;
+	case DSA_DB_BRIDGE:
+		return db.bridge.dev;
+	default:
+		return ERR_PTR(-EOPNOTSUPP);
+	}
+}
+
 static int felix_fdb_add(struct dsa_switch *ds, int port,
 			 const unsigned char *addr, u16 vid,
 			 struct dsa_db db)
 {
+	struct net_device *bridge_dev = felix_classify_db(db);
 	struct ocelot *ocelot = ds->priv;
 
-	return ocelot_fdb_add(ocelot, port, addr, vid);
+	if (IS_ERR(bridge_dev))
+		return PTR_ERR(bridge_dev);
+
+	return ocelot_fdb_add(ocelot, port, addr, vid, bridge_dev);
 }
 
 static int felix_fdb_del(struct dsa_switch *ds, int port,
 			 const unsigned char *addr, u16 vid,
 			 struct dsa_db db)
 {
+	struct net_device *bridge_dev = felix_classify_db(db);
 	struct ocelot *ocelot = ds->priv;
 
-	return ocelot_fdb_del(ocelot, port, addr, vid);
+	if (IS_ERR(bridge_dev))
+		return PTR_ERR(bridge_dev);
+
+	return ocelot_fdb_del(ocelot, port, addr, vid, bridge_dev);
 }
 
 static int felix_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag lag,
 			     const unsigned char *addr, u16 vid,
 			     struct dsa_db db)
 {
+	struct net_device *bridge_dev = felix_classify_db(db);
 	struct ocelot *ocelot = ds->priv;
 
-	return ocelot_lag_fdb_add(ocelot, lag.dev, addr, vid);
+	if (IS_ERR(bridge_dev))
+		return PTR_ERR(bridge_dev);
+
+	return ocelot_lag_fdb_add(ocelot, lag.dev, addr, vid, bridge_dev);
 }
 
 static int felix_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag lag,
 			     const unsigned char *addr, u16 vid,
 			     struct dsa_db db)
 {
+	struct net_device *bridge_dev = felix_classify_db(db);
 	struct ocelot *ocelot = ds->priv;
 
-	return ocelot_lag_fdb_del(ocelot, lag.dev, addr, vid);
+	if (IS_ERR(bridge_dev))
+		return PTR_ERR(bridge_dev);
+
+	return ocelot_lag_fdb_del(ocelot, lag.dev, addr, vid, bridge_dev);
 }
 
 static int felix_mdb_add(struct dsa_switch *ds, int port,
 			 const struct switchdev_obj_port_mdb *mdb,
 			 struct dsa_db db)
 {
+	struct net_device *bridge_dev = felix_classify_db(db);
 	struct ocelot *ocelot = ds->priv;
 
-	return ocelot_port_mdb_add(ocelot, port, mdb);
+	if (IS_ERR(bridge_dev))
+		return PTR_ERR(bridge_dev);
+
+	return ocelot_port_mdb_add(ocelot, port, mdb, bridge_dev);
 }
 
 static int felix_mdb_del(struct dsa_switch *ds, int port,
 			 const struct switchdev_obj_port_mdb *mdb,
 			 struct dsa_db db)
 {
+	struct net_device *bridge_dev = felix_classify_db(db);
 	struct ocelot *ocelot = ds->priv;
 
-	return ocelot_port_mdb_del(ocelot, port, mdb);
+	if (IS_ERR(bridge_dev))
+		return PTR_ERR(bridge_dev);
+
+	return ocelot_port_mdb_del(ocelot, port, mdb, bridge_dev);
 }
 
 static void felix_bridge_stp_state_set(struct dsa_switch *ds, int port,
@@ -679,9 +721,8 @@ static int felix_bridge_join(struct dsa_switch *ds, int port,
 {
 	struct ocelot *ocelot = ds->priv;
 
-	ocelot_port_bridge_join(ocelot, port, bridge.dev);
-
-	return 0;
+	return ocelot_port_bridge_join(ocelot, port, bridge.dev, bridge.num,
+				       extack);
 }
 
 static void felix_bridge_leave(struct dsa_switch *ds, int port,
@@ -1191,6 +1232,8 @@ static int felix_setup(struct dsa_switch *ds)
 
 	ds->mtu_enforcement_ingress = true;
 	ds->assisted_learning_on_cpu_port = true;
+	ds->fdb_isolation = true;
+	ds->max_num_bridges = ds->num_ports;
 
 	return 0;
 
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 0e8fa0a4fc69..0af321f6fb54 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -13,6 +13,7 @@
 
 #define TABLE_UPDATE_SLEEP_US 10
 #define TABLE_UPDATE_TIMEOUT_US 100000
+#define OCELOT_RSV_VLAN_RANGE_START 4000
 
 struct ocelot_mact_entry {
 	u8 mac[ETH_ALEN];
@@ -221,6 +222,35 @@ static void ocelot_vcap_enable(struct ocelot *ocelot, int port)
 		       REW_PORT_CFG, port);
 }
 
+static int ocelot_single_vlan_aware_bridge(struct ocelot *ocelot,
+					   struct netlink_ext_ack *extack)
+{
+	struct net_device *bridge = NULL;
+	int port;
+
+	for (port = 0; port < ocelot->num_phys_ports; port++) {
+		struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+		if (!ocelot_port || !ocelot_port->bridge ||
+		    !br_vlan_enabled(ocelot_port->bridge))
+			continue;
+
+		if (!bridge) {
+			bridge = ocelot_port->bridge;
+			continue;
+		}
+
+		if (bridge == ocelot_port->bridge)
+			continue;
+
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Only one VLAN-aware bridge is supported");
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
 static inline u32 ocelot_vlant_read_vlanaccess(struct ocelot *ocelot)
 {
 	return ocelot_read(ocelot, ANA_TABLES_VLANACCESS);
@@ -347,12 +377,45 @@ static void ocelot_port_manage_port_tag(struct ocelot *ocelot, int port)
 	}
 }
 
+int ocelot_bridge_num_find(struct ocelot *ocelot,
+			   const struct net_device *bridge)
+{
+	int port;
+
+	for (port = 0; port < ocelot->num_phys_ports; port++) {
+		struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+		if (ocelot_port && ocelot_port->bridge == bridge)
+			return ocelot_port->bridge_num;
+	}
+
+	return -1;
+}
+EXPORT_SYMBOL_GPL(ocelot_bridge_num_find);
+
+static u16 ocelot_vlan_unaware_pvid(struct ocelot *ocelot,
+				    const struct net_device *bridge)
+{
+	int bridge_num;
+
+	/* Standalone ports use VID 0 */
+	if (!bridge)
+		return 0;
+
+	bridge_num = ocelot_bridge_num_find(ocelot, bridge);
+	if (WARN_ON(bridge_num < 0))
+		return 0;
+
+	/* VLAN-unaware bridges use a reserved VID going from 4095 downwards */
+	return VLAN_N_VID - bridge_num - 1;
+}
+
 /* Default vlan to clasify for untagged frames (may be zero) */
 static void ocelot_port_set_pvid(struct ocelot *ocelot, int port,
 				 const struct ocelot_bridge_vlan *pvid_vlan)
 {
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
-	u16 pvid = OCELOT_VLAN_UNAWARE_PVID;
+	u16 pvid = ocelot_vlan_unaware_pvid(ocelot, ocelot_port->bridge);
 	u32 val = 0;
 
 	ocelot_port->pvid_vlan = pvid_vlan;
@@ -466,12 +529,29 @@ static int ocelot_vlan_member_del(struct ocelot *ocelot, int port, u16 vid)
 	return 0;
 }
 
+static int ocelot_add_vlan_unaware_pvid(struct ocelot *ocelot, int port,
+					const struct net_device *bridge)
+{
+	u16 vid = ocelot_vlan_unaware_pvid(ocelot, bridge);
+
+	return ocelot_vlan_member_add(ocelot, port, vid, true);
+}
+
+static int ocelot_del_vlan_unaware_pvid(struct ocelot *ocelot, int port,
+					const struct net_device *bridge)
+{
+	u16 vid = ocelot_vlan_unaware_pvid(ocelot, bridge);
+
+	return ocelot_vlan_member_del(ocelot, port, vid);
+}
+
 int ocelot_port_vlan_filtering(struct ocelot *ocelot, int port,
 			       bool vlan_aware, struct netlink_ext_ack *extack)
 {
 	struct ocelot_vcap_block *block = &ocelot->block[VCAP_IS1];
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
 	struct ocelot_vcap_filter *filter;
+	int err;
 	u32 val;
 
 	list_for_each_entry(filter, &block->rules, list) {
@@ -483,6 +563,19 @@ int ocelot_port_vlan_filtering(struct ocelot *ocelot, int port,
 		}
 	}
 
+	err = ocelot_single_vlan_aware_bridge(ocelot, extack);
+	if (err)
+		return err;
+
+	if (vlan_aware)
+		err = ocelot_del_vlan_unaware_pvid(ocelot, port,
+						   ocelot_port->bridge);
+	else
+		err = ocelot_add_vlan_unaware_pvid(ocelot, port,
+						   ocelot_port->bridge);
+	if (err)
+		return err;
+
 	ocelot_port->vlan_aware = vlan_aware;
 
 	if (vlan_aware)
@@ -521,6 +614,12 @@ int ocelot_vlan_prepare(struct ocelot *ocelot, int port, u16 vid, bool pvid,
 		}
 	}
 
+	if (vid > OCELOT_RSV_VLAN_RANGE_START) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "VLAN range 4000-4095 reserved for VLAN-unaware bridging");
+		return -EBUSY;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL(ocelot_vlan_prepare);
@@ -584,11 +683,11 @@ static void ocelot_vlan_init(struct ocelot *ocelot)
 	for (vid = 1; vid < VLAN_N_VID; vid++)
 		ocelot_vlant_set_mask(ocelot, vid, 0);
 
-	/* Because VLAN filtering is enabled, we need VID 0 to get untagged
-	 * traffic.  It is added automatically if 8021q module is loaded, but
-	 * we can't rely on it since module may be not loaded.
+	/* We need VID 0 to get traffic on standalone ports.
+	 * It is added automatically if the 8021q module is loaded, but we
+	 * can't rely on that since it might not be.
 	 */
-	ocelot_vlant_set_mask(ocelot, OCELOT_VLAN_UNAWARE_PVID, all_ports);
+	ocelot_vlant_set_mask(ocelot, OCELOT_STANDALONE_PVID, all_ports);
 
 	/* Set vlan ingress filter mask to all ports but the CPU port by
 	 * default.
@@ -1237,21 +1336,27 @@ void ocelot_drain_cpu_queue(struct ocelot *ocelot, int grp)
 }
 EXPORT_SYMBOL(ocelot_drain_cpu_queue);
 
-int ocelot_fdb_add(struct ocelot *ocelot, int port,
-		   const unsigned char *addr, u16 vid)
+int ocelot_fdb_add(struct ocelot *ocelot, int port, const unsigned char *addr,
+		   u16 vid, const struct net_device *bridge)
 {
 	int pgid = port;
 
 	if (port == ocelot->npi)
 		pgid = PGID_CPU;
 
+	if (!vid)
+		vid = ocelot_vlan_unaware_pvid(ocelot, bridge);
+
 	return ocelot_mact_learn(ocelot, pgid, addr, vid, ENTRYTYPE_LOCKED);
 }
 EXPORT_SYMBOL(ocelot_fdb_add);
 
-int ocelot_fdb_del(struct ocelot *ocelot, int port,
-		   const unsigned char *addr, u16 vid)
+int ocelot_fdb_del(struct ocelot *ocelot, int port, const unsigned char *addr,
+		   u16 vid, const struct net_device *bridge)
 {
+	if (!vid)
+		vid = ocelot_vlan_unaware_pvid(ocelot, bridge);
+
 	return ocelot_mact_forget(ocelot, addr, vid);
 }
 EXPORT_SYMBOL(ocelot_fdb_del);
@@ -1413,6 +1518,12 @@ int ocelot_fdb_dump(struct ocelot *ocelot, int port,
 
 			is_static = (entry.type == ENTRYTYPE_LOCKED);
 
+			/* Hide the reserved VLANs used for
+			 * VLAN-unaware bridging.
+			 */
+			if (entry.vid > OCELOT_RSV_VLAN_RANGE_START)
+				entry.vid = 0;
+
 			err = cb(entry.mac, entry.vid, is_static, data);
 			if (err)
 				break;
@@ -2054,6 +2165,28 @@ void ocelot_apply_bridge_fwd_mask(struct ocelot *ocelot, bool joining)
 }
 EXPORT_SYMBOL(ocelot_apply_bridge_fwd_mask);
 
+void ocelot_port_set_dsa_8021q_cpu(struct ocelot *ocelot, int port)
+{
+	u16 vid;
+
+	ocelot->ports[port]->is_dsa_8021q_cpu = true;
+
+	for (vid = OCELOT_RSV_VLAN_RANGE_START; vid < VLAN_N_VID; vid++)
+		ocelot_vlan_member_add(ocelot, port, vid, true);
+}
+EXPORT_SYMBOL_GPL(ocelot_port_set_dsa_8021q_cpu);
+
+void ocelot_port_unset_dsa_8021q_cpu(struct ocelot *ocelot, int port)
+{
+	u16 vid;
+
+	ocelot->ports[port]->is_dsa_8021q_cpu = false;
+
+	for (vid = OCELOT_RSV_VLAN_RANGE_START; vid < VLAN_N_VID; vid++)
+		ocelot_vlan_member_del(ocelot, port, vid);
+}
+EXPORT_SYMBOL_GPL(ocelot_port_unset_dsa_8021q_cpu);
+
 void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
 {
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
@@ -2198,7 +2331,8 @@ static void ocelot_encode_ports_to_mdb(unsigned char *addr,
 }
 
 int ocelot_port_mdb_add(struct ocelot *ocelot, int port,
-			const struct switchdev_obj_port_mdb *mdb)
+			const struct switchdev_obj_port_mdb *mdb,
+			const struct net_device *bridge)
 {
 	unsigned char addr[ETH_ALEN];
 	struct ocelot_multicast *mc;
@@ -2208,6 +2342,9 @@ int ocelot_port_mdb_add(struct ocelot *ocelot, int port,
 	if (port == ocelot->npi)
 		port = ocelot->num_phys_ports;
 
+	if (!vid)
+		vid = ocelot_vlan_unaware_pvid(ocelot, bridge);
+
 	mc = ocelot_multicast_get(ocelot, mdb->addr, vid);
 	if (!mc) {
 		/* New entry */
@@ -2254,7 +2391,8 @@ int ocelot_port_mdb_add(struct ocelot *ocelot, int port,
 EXPORT_SYMBOL(ocelot_port_mdb_add);
 
 int ocelot_port_mdb_del(struct ocelot *ocelot, int port,
-			const struct switchdev_obj_port_mdb *mdb)
+			const struct switchdev_obj_port_mdb *mdb,
+			const struct net_device *bridge)
 {
 	unsigned char addr[ETH_ALEN];
 	struct ocelot_multicast *mc;
@@ -2264,6 +2402,9 @@ int ocelot_port_mdb_del(struct ocelot *ocelot, int port,
 	if (port == ocelot->npi)
 		port = ocelot->num_phys_ports;
 
+	if (!vid)
+		vid = ocelot_vlan_unaware_pvid(ocelot, bridge);
+
 	mc = ocelot_multicast_get(ocelot, mdb->addr, vid);
 	if (!mc)
 		return -ENOENT;
@@ -2297,18 +2438,30 @@ int ocelot_port_mdb_del(struct ocelot *ocelot, int port,
 }
 EXPORT_SYMBOL(ocelot_port_mdb_del);
 
-void ocelot_port_bridge_join(struct ocelot *ocelot, int port,
-			     struct net_device *bridge)
+int ocelot_port_bridge_join(struct ocelot *ocelot, int port,
+			    struct net_device *bridge, int bridge_num,
+			    struct netlink_ext_ack *extack)
 {
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
+	int err;
+
+	err = ocelot_single_vlan_aware_bridge(ocelot, extack);
+	if (err)
+		return err;
 
 	mutex_lock(&ocelot->fwd_domain_lock);
 
 	ocelot_port->bridge = bridge;
+	ocelot_port->bridge_num = bridge_num;
 
 	ocelot_apply_bridge_fwd_mask(ocelot, true);
 
 	mutex_unlock(&ocelot->fwd_domain_lock);
+
+	if (br_vlan_enabled(bridge))
+		return 0;
+
+	return ocelot_add_vlan_unaware_pvid(ocelot, port, bridge);
 }
 EXPORT_SYMBOL(ocelot_port_bridge_join);
 
@@ -2319,7 +2472,11 @@ void ocelot_port_bridge_leave(struct ocelot *ocelot, int port,
 
 	mutex_lock(&ocelot->fwd_domain_lock);
 
+	if (!br_vlan_enabled(bridge))
+		ocelot_del_vlan_unaware_pvid(ocelot, port, bridge);
+
 	ocelot_port->bridge = NULL;
+	ocelot_port->bridge_num = -1;
 
 	ocelot_port_set_pvid(ocelot, port, NULL);
 	ocelot_port_manage_port_tag(ocelot, port);
@@ -2544,7 +2701,8 @@ void ocelot_port_lag_change(struct ocelot *ocelot, int port, bool lag_tx_active)
 EXPORT_SYMBOL(ocelot_port_lag_change);
 
 int ocelot_lag_fdb_add(struct ocelot *ocelot, struct net_device *bond,
-		       const unsigned char *addr, u16 vid)
+		       const unsigned char *addr, u16 vid,
+		       const struct net_device *bridge)
 {
 	struct ocelot_lag_fdb *fdb;
 	int lag, err;
@@ -2553,11 +2711,15 @@ int ocelot_lag_fdb_add(struct ocelot *ocelot, struct net_device *bond,
 	if (!fdb)
 		return -ENOMEM;
 
+	mutex_lock(&ocelot->fwd_domain_lock);
+
+	if (!vid)
+		vid = ocelot_vlan_unaware_pvid(ocelot, bridge);
+
 	ether_addr_copy(fdb->addr, addr);
 	fdb->vid = vid;
 	fdb->bond = bond;
 
-	mutex_lock(&ocelot->fwd_domain_lock);
 	lag = ocelot_bond_get_id(ocelot, bond);
 
 	err = ocelot_mact_learn(ocelot, lag, addr, vid, ENTRYTYPE_LOCKED);
@@ -2575,12 +2737,16 @@ int ocelot_lag_fdb_add(struct ocelot *ocelot, struct net_device *bond,
 EXPORT_SYMBOL_GPL(ocelot_lag_fdb_add);
 
 int ocelot_lag_fdb_del(struct ocelot *ocelot, struct net_device *bond,
-		       const unsigned char *addr, u16 vid)
+		       const unsigned char *addr, u16 vid,
+		       const struct net_device *bridge)
 {
 	struct ocelot_lag_fdb *fdb, *tmp;
 
 	mutex_lock(&ocelot->fwd_domain_lock);
 
+	if (!vid)
+		vid = ocelot_vlan_unaware_pvid(ocelot, bridge);
+
 	list_for_each_entry_safe(fdb, tmp, &ocelot->lag_fdbs, list) {
 		if (!ether_addr_equal(fdb->addr, addr) || fdb->vid != vid ||
 		    fdb->bond != bond)
@@ -2832,7 +2998,7 @@ static void ocelot_cpu_port_init(struct ocelot *ocelot)
 
 	/* Configure the CPU port to be VLAN aware */
 	ocelot_write_gix(ocelot,
-			 ANA_PORT_VLAN_CFG_VLAN_VID(OCELOT_VLAN_UNAWARE_PVID) |
+			 ANA_PORT_VLAN_CFG_VLAN_VID(OCELOT_STANDALONE_PVID) |
 			 ANA_PORT_VLAN_CFG_VLAN_AWARE_ENA |
 			 ANA_PORT_VLAN_CFG_VLAN_POP_CNT(1),
 			 ANA_PORT_VLAN_CFG, cpu);
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index 5277c4b53af4..f8dc0d75eb5d 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -26,7 +26,7 @@
 #include "ocelot_rew.h"
 #include "ocelot_qs.h"
 
-#define OCELOT_VLAN_UNAWARE_PVID 0
+#define OCELOT_STANDALONE_PVID 0
 #define OCELOT_BUFFER_CELL_SZ 60
 
 #define OCELOT_STATS_CHECK_DELAY (2 * HZ)
@@ -81,6 +81,9 @@ struct ocelot_multicast {
 	struct ocelot_pgid *pgid;
 };
 
+int ocelot_bridge_num_find(struct ocelot *ocelot,
+			   const struct net_device *bridge);
+
 int ocelot_port_fdb_do_dump(const unsigned char *addr, u16 vid,
 			    bool is_static, void *data);
 int ocelot_mact_learn(struct ocelot *ocelot, int port,
diff --git a/drivers/net/ethernet/mscc/ocelot_mrp.c b/drivers/net/ethernet/mscc/ocelot_mrp.c
index 142e897ea2af..3ccec488a304 100644
--- a/drivers/net/ethernet/mscc/ocelot_mrp.c
+++ b/drivers/net/ethernet/mscc/ocelot_mrp.c
@@ -107,16 +107,16 @@ static void ocelot_mrp_save_mac(struct ocelot *ocelot,
 				struct ocelot_port *port)
 {
 	ocelot_mact_learn(ocelot, PGID_BLACKHOLE, mrp_test_dmac,
-			  OCELOT_VLAN_UNAWARE_PVID, ENTRYTYPE_LOCKED);
+			  OCELOT_STANDALONE_PVID, ENTRYTYPE_LOCKED);
 	ocelot_mact_learn(ocelot, PGID_BLACKHOLE, mrp_control_dmac,
-			  OCELOT_VLAN_UNAWARE_PVID, ENTRYTYPE_LOCKED);
+			  OCELOT_STANDALONE_PVID, ENTRYTYPE_LOCKED);
 }
 
 static void ocelot_mrp_del_mac(struct ocelot *ocelot,
 			       struct ocelot_port *port)
 {
-	ocelot_mact_forget(ocelot, mrp_test_dmac, OCELOT_VLAN_UNAWARE_PVID);
-	ocelot_mact_forget(ocelot, mrp_control_dmac, OCELOT_VLAN_UNAWARE_PVID);
+	ocelot_mact_forget(ocelot, mrp_test_dmac, OCELOT_STANDALONE_PVID);
+	ocelot_mact_forget(ocelot, mrp_control_dmac, OCELOT_STANDALONE_PVID);
 }
 
 int ocelot_mrp_add(struct ocelot *ocelot, int port,
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index e271b6225b72..cfe767d077f8 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -419,7 +419,7 @@ static int ocelot_vlan_vid_del(struct net_device *dev, u16 vid)
 	 * with VLAN filtering feature. We need to keep it to receive
 	 * untagged traffic.
 	 */
-	if (vid == OCELOT_VLAN_UNAWARE_PVID)
+	if (vid == OCELOT_STANDALONE_PVID)
 		return 0;
 
 	ret = ocelot_vlan_del(ocelot, port, vid);
@@ -559,7 +559,7 @@ static int ocelot_mc_unsync(struct net_device *dev, const unsigned char *addr)
 	struct ocelot_mact_work_ctx w;
 
 	ether_addr_copy(w.forget.addr, addr);
-	w.forget.vid = OCELOT_VLAN_UNAWARE_PVID;
+	w.forget.vid = OCELOT_STANDALONE_PVID;
 	w.type = OCELOT_MACT_FORGET;
 
 	return ocelot_enqueue_mact_action(ocelot, &w);
@@ -573,7 +573,7 @@ static int ocelot_mc_sync(struct net_device *dev, const unsigned char *addr)
 	struct ocelot_mact_work_ctx w;
 
 	ether_addr_copy(w.learn.addr, addr);
-	w.learn.vid = OCELOT_VLAN_UNAWARE_PVID;
+	w.learn.vid = OCELOT_STANDALONE_PVID;
 	w.learn.pgid = PGID_CPU;
 	w.learn.entry_type = ENTRYTYPE_LOCKED;
 	w.type = OCELOT_MACT_LEARN;
@@ -608,9 +608,9 @@ static int ocelot_port_set_mac_address(struct net_device *dev, void *p)
 
 	/* Learn the new net device MAC address in the mac table. */
 	ocelot_mact_learn(ocelot, PGID_CPU, addr->sa_data,
-			  OCELOT_VLAN_UNAWARE_PVID, ENTRYTYPE_LOCKED);
+			  OCELOT_STANDALONE_PVID, ENTRYTYPE_LOCKED);
 	/* Then forget the previous one. */
-	ocelot_mact_forget(ocelot, dev->dev_addr, OCELOT_VLAN_UNAWARE_PVID);
+	ocelot_mact_forget(ocelot, dev->dev_addr, OCELOT_STANDALONE_PVID);
 
 	eth_hw_addr_set(dev, addr->sa_data);
 	return 0;
@@ -662,10 +662,11 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 			       struct netlink_ext_ack *extack)
 {
 	struct ocelot_port_private *priv = netdev_priv(dev);
-	struct ocelot *ocelot = priv->port.ocelot;
+	struct ocelot_port *ocelot_port = &priv->port;
+	struct ocelot *ocelot = ocelot_port->ocelot;
 	int port = priv->chip_port;
 
-	return ocelot_fdb_add(ocelot, port, addr, vid);
+	return ocelot_fdb_add(ocelot, port, addr, vid, ocelot_port->bridge);
 }
 
 static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
@@ -673,10 +674,11 @@ static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
 			       const unsigned char *addr, u16 vid)
 {
 	struct ocelot_port_private *priv = netdev_priv(dev);
-	struct ocelot *ocelot = priv->port.ocelot;
+	struct ocelot_port *ocelot_port = &priv->port;
+	struct ocelot *ocelot = ocelot_port->ocelot;
 	int port = priv->chip_port;
 
-	return ocelot_fdb_del(ocelot, port, addr, vid);
+	return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
 }
 
 static int ocelot_port_fdb_dump(struct sk_buff *skb,
@@ -988,7 +990,7 @@ static int ocelot_port_obj_add_mdb(struct net_device *dev,
 	struct ocelot *ocelot = ocelot_port->ocelot;
 	int port = priv->chip_port;
 
-	return ocelot_port_mdb_add(ocelot, port, mdb);
+	return ocelot_port_mdb_add(ocelot, port, mdb, ocelot_port->bridge);
 }
 
 static int ocelot_port_obj_del_mdb(struct net_device *dev,
@@ -999,7 +1001,7 @@ static int ocelot_port_obj_del_mdb(struct net_device *dev,
 	struct ocelot *ocelot = ocelot_port->ocelot;
 	int port = priv->chip_port;
 
-	return ocelot_port_mdb_del(ocelot, port, mdb);
+	return ocelot_port_mdb_del(ocelot, port, mdb, ocelot_port->bridge);
 }
 
 static int ocelot_port_obj_mrp_add(struct net_device *dev,
@@ -1173,6 +1175,33 @@ static int ocelot_switchdev_unsync(struct ocelot *ocelot, int port)
 	return 0;
 }
 
+static int ocelot_bridge_num_get(struct ocelot *ocelot,
+				 const struct net_device *bridge_dev)
+{
+	int bridge_num = ocelot_bridge_num_find(ocelot, bridge_dev);
+
+	if (bridge_num < 0) {
+		/* First port that offloads this bridge */
+		bridge_num = find_first_zero_bit(&ocelot->bridges,
+						 ocelot->num_phys_ports);
+
+		set_bit(bridge_num, &ocelot->bridges);
+	}
+
+	return bridge_num;
+}
+
+static void ocelot_bridge_num_put(struct ocelot *ocelot,
+				  const struct net_device *bridge_dev,
+				  int bridge_num)
+{
+	/* Check if the bridge is still in use, otherwise it is time
+	 * to clean it up so we can reuse this bridge_num later.
+	 */
+	if (!ocelot_bridge_num_find(ocelot, bridge_dev))
+		clear_bit(bridge_num, &ocelot->bridges);
+}
+
 static int ocelot_netdevice_bridge_join(struct net_device *dev,
 					struct net_device *brport_dev,
 					struct net_device *bridge,
@@ -1182,9 +1211,14 @@ static int ocelot_netdevice_bridge_join(struct net_device *dev,
 	struct ocelot_port *ocelot_port = &priv->port;
 	struct ocelot *ocelot = ocelot_port->ocelot;
 	int port = priv->chip_port;
-	int err;
+	int bridge_num, err;
 
-	ocelot_port_bridge_join(ocelot, port, bridge);
+	bridge_num = ocelot_bridge_num_get(ocelot, bridge);
+
+	err = ocelot_port_bridge_join(ocelot, port, bridge, bridge_num,
+				      extack);
+	if (err)
+		goto err_join;
 
 	err = switchdev_bridge_port_offload(brport_dev, dev, priv,
 					    &ocelot_switchdev_nb,
@@ -1205,6 +1239,8 @@ static int ocelot_netdevice_bridge_join(struct net_device *dev,
 					&ocelot_switchdev_blocking_nb);
 err_switchdev_offload:
 	ocelot_port_bridge_leave(ocelot, port, bridge);
+err_join:
+	ocelot_bridge_num_put(ocelot, bridge, bridge_num);
 	return err;
 }
 
@@ -1225,6 +1261,7 @@ static int ocelot_netdevice_bridge_leave(struct net_device *dev,
 	struct ocelot_port_private *priv = netdev_priv(dev);
 	struct ocelot_port *ocelot_port = &priv->port;
 	struct ocelot *ocelot = ocelot_port->ocelot;
+	int bridge_num = ocelot_port->bridge_num;
 	int port = priv->chip_port;
 	int err;
 
@@ -1233,6 +1270,7 @@ static int ocelot_netdevice_bridge_leave(struct net_device *dev,
 		return err;
 
 	ocelot_port_bridge_leave(ocelot, port, bridge);
+	ocelot_bridge_num_put(ocelot, bridge, bridge_num);
 
 	return 0;
 }
@@ -1700,7 +1738,7 @@ int ocelot_probe_port(struct ocelot *ocelot, int port, struct regmap *target,
 		eth_hw_addr_gen(dev, ocelot->base_mac, port);
 
 	ocelot_mact_learn(ocelot, PGID_CPU, dev->dev_addr,
-			  OCELOT_VLAN_UNAWARE_PVID, ENTRYTYPE_LOCKED);
+			  OCELOT_STANDALONE_PVID, ENTRYTYPE_LOCKED);
 
 	ocelot_init_port(ocelot, port);
 
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index dd4fc34d2992..ee3c59639d70 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -668,6 +668,7 @@ struct ocelot_port {
 	u16				mrp_ring_id;
 
 	struct net_device		*bridge;
+	int				bridge_num;
 	u8				stp_state;
 
 	int				speed;
@@ -713,6 +714,8 @@ struct ocelot {
 	enum ocelot_tag_prefix		npi_inj_prefix;
 	enum ocelot_tag_prefix		npi_xtr_prefix;
 
+	unsigned long			bridges;
+
 	struct list_head		multicast;
 	struct list_head		pgids;
 
@@ -846,6 +849,9 @@ void ocelot_deinit(struct ocelot *ocelot);
 void ocelot_init_port(struct ocelot *ocelot, int port);
 void ocelot_deinit_port(struct ocelot *ocelot, int port);
 
+void ocelot_port_set_dsa_8021q_cpu(struct ocelot *ocelot, int port);
+void ocelot_port_unset_dsa_8021q_cpu(struct ocelot *ocelot, int port);
+
 /* DSA callbacks */
 void ocelot_get_strings(struct ocelot *ocelot, int port, u32 sset, u8 *data);
 void ocelot_get_ethtool_stats(struct ocelot *ocelot, int port, u64 *data);
@@ -863,21 +869,24 @@ int ocelot_port_pre_bridge_flags(struct ocelot *ocelot, int port,
 				 struct switchdev_brport_flags val);
 void ocelot_port_bridge_flags(struct ocelot *ocelot, int port,
 			      struct switchdev_brport_flags val);
-void ocelot_port_bridge_join(struct ocelot *ocelot, int port,
-			     struct net_device *bridge);
+int ocelot_port_bridge_join(struct ocelot *ocelot, int port,
+			    struct net_device *bridge, int bridge_num,
+			    struct netlink_ext_ack *extack);
 void ocelot_port_bridge_leave(struct ocelot *ocelot, int port,
 			      struct net_device *bridge);
 int ocelot_mact_flush(struct ocelot *ocelot, int port);
 int ocelot_fdb_dump(struct ocelot *ocelot, int port,
 		    dsa_fdb_dump_cb_t *cb, void *data);
-int ocelot_fdb_add(struct ocelot *ocelot, int port,
-		   const unsigned char *addr, u16 vid);
-int ocelot_fdb_del(struct ocelot *ocelot, int port,
-		   const unsigned char *addr, u16 vid);
+int ocelot_fdb_add(struct ocelot *ocelot, int port, const unsigned char *addr,
+		   u16 vid, const struct net_device *bridge);
+int ocelot_fdb_del(struct ocelot *ocelot, int port, const unsigned char *addr,
+		   u16 vid, const struct net_device *bridge);
 int ocelot_lag_fdb_add(struct ocelot *ocelot, struct net_device *bond,
-		       const unsigned char *addr, u16 vid);
+		       const unsigned char *addr, u16 vid,
+		       const struct net_device *bridge);
 int ocelot_lag_fdb_del(struct ocelot *ocelot, struct net_device *bond,
-		       const unsigned char *addr, u16 vid);
+		       const unsigned char *addr, u16 vid,
+		       const struct net_device *bridge);
 int ocelot_vlan_prepare(struct ocelot *ocelot, int port, u16 vid, bool pvid,
 			bool untagged, struct netlink_ext_ack *extack);
 int ocelot_vlan_add(struct ocelot *ocelot, int port, u16 vid, bool pvid,
@@ -901,9 +910,11 @@ int ocelot_cls_flower_destroy(struct ocelot *ocelot, int port,
 int ocelot_cls_flower_stats(struct ocelot *ocelot, int port,
 			    struct flow_cls_offload *f, bool ingress);
 int ocelot_port_mdb_add(struct ocelot *ocelot, int port,
-			const struct switchdev_obj_port_mdb *mdb);
+			const struct switchdev_obj_port_mdb *mdb,
+			const struct net_device *bridge);
 int ocelot_port_mdb_del(struct ocelot *ocelot, int port,
-			const struct switchdev_obj_port_mdb *mdb);
+			const struct switchdev_obj_port_mdb *mdb,
+			const struct net_device *bridge);
 int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 			 struct net_device *bond,
 			 struct netdev_lag_upper_info *info);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 net-next 00/10] DSA FDB isolation
  2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
                   ` (9 preceding siblings ...)
  2022-02-25  9:22 ` [PATCH v2 net-next 10/10] net: mscc: ocelot: enforce FDB isolation when VLAN-unaware Vladimir Oltean
@ 2022-02-27 11:10 ` patchwork-bot+netdevbpf
  10 siblings, 0 replies; 19+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-27 11:10 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, kuba, davem, f.fainelli, andrew, vivien.didelot, olteanv,
	kurt, hauke, woojung.huh, UNGLinuxDriver, sean.wang, Landen.Chao,
	dqfext, claudiu.manoil, alexandre.belloni, linus.walleij, alsi,
	george.mccollister

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 25 Feb 2022 11:22:15 +0200 you wrote:
> There are use cases which need FDB isolation between standalone ports
> and bridged ports, as well as isolation between ports of different
> bridges. Most of these use cases are a result of the fact that packets
> can now be partially forwarded by the software bridge, so one port might
> need to send a packet to the CPU but its FDB lookup will see that it can
> forward it directly to a bridge port where that packet was autonomously
> learned. So the source port will attempt to shortcircuit the CPU and
> forward autonomously, which it can't due to the forwarding isolation we
> have in place. So we will have packet drops instead of proper operation.
> 
> [...]

Here is the summary with links:
  - [v2,net-next,01/10] net: dsa: tag_8021q: replace the SVL bridging with VLAN-unaware IVL bridging
    https://git.kernel.org/netdev/net-next/c/91495f21fcec
  - [v2,net-next,02/10] net: dsa: tag_8021q: add support for imprecise RX based on the VBID
    https://git.kernel.org/netdev/net-next/c/d7f9787a763f
  - [v2,net-next,03/10] docs: net: dsa: sja1105: document limitations of tc-flower rule VLAN awareness
    https://git.kernel.org/netdev/net-next/c/d27656d02d85
  - [v2,net-next,04/10] net: dsa: felix: delete workarounds present due to SVL tag_8021q bridging
    https://git.kernel.org/netdev/net-next/c/08f44db3abe6
  - [v2,net-next,05/10] net: dsa: tag_8021q: merge RX and TX VLANs
    https://git.kernel.org/netdev/net-next/c/04b67e18ce5b
  - [v2,net-next,06/10] net: dsa: tag_8021q: rename dsa_8021q_bridge_tx_fwd_offload_vid
    https://git.kernel.org/netdev/net-next/c/b6362bdf750b
  - [v2,net-next,07/10] net: dsa: request drivers to perform FDB isolation
    https://git.kernel.org/netdev/net-next/c/c26933639b54
  - [v2,net-next,08/10] net: dsa: pass extack to .port_bridge_join driver methods
    https://git.kernel.org/netdev/net-next/c/06b9cce42634
  - [v2,net-next,09/10] net: dsa: sja1105: enforce FDB isolation
    https://git.kernel.org/netdev/net-next/c/219827ef92f8
  - [v2,net-next,10/10] net: mscc: ocelot: enforce FDB isolation when VLAN-unaware
    https://git.kernel.org/netdev/net-next/c/54c319846086

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation
  2022-02-25  9:22 ` [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation Vladimir Oltean
@ 2022-04-26 15:01   ` Hans Schultz
  2022-04-26 16:14     ` Andrew Lunn
  0 siblings, 1 reply; 19+ messages in thread
From: Hans Schultz @ 2022-04-26 15:01 UTC (permalink / raw)
  To: Vladimir Oltean, netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

On fre, feb 25, 2022 at 11:22, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> For DSA, to encourage drivers to perform FDB isolation simply means to
> track which bridge does each FDB and MDB entry belong to. It then
> becomes the driver responsibility to use something that makes the FDB
> entry from one bridge not match the FDB lookup of ports from other
> bridges.
>
> The top-level functions where the bridge is determined are:
> - dsa_port_fdb_{add,del}
> - dsa_port_host_fdb_{add,del}
> - dsa_port_mdb_{add,del}
> - dsa_port_host_mdb_{add,del}
>
> aka the pre-crosschip-notifier functions.
>
> Changing the API to pass a reference to a bridge is not superfluous, and
> looking at the passed bridge argument is not the same as having the
> driver look at dsa_to_port(ds, port)->bridge from the ->port_fdb_add()
> method.
>
> DSA installs FDB and MDB entries on shared (CPU and DSA) ports as well,
> and those do not have any dp->bridge information to retrieve, because
> they are not in any bridge - they are merely the pipes that serve the
> user ports that are in one or multiple bridges.
>
> The struct dsa_bridge associated with each FDB/MDB entry is encapsulated
> in a larger "struct dsa_db" database. Although only databases associated
> to bridges are notified for now, this API will be the starting point for
> implementing IFF_UNICAST_FLT in DSA. There, the idea is to install FDB
> entries on the CPU port which belong to the corresponding user port's
> port database. These are supposed to match only when the port is
> standalone.
>
> It is better to introduce the API in its expected final form than to
> introduce it for bridges first, then to have to change drivers which may
> have made one or more assumptions.
>
> Drivers can use the provided bridge.num, but they can also use a
> different numbering scheme that is more convenient.
>
> DSA must perform refcounting on the CPU and DSA ports by also taking
> into account the bridge number. So if two bridges request the same local
> address, DSA must notify the driver twice, once for each bridge.
>
> In fact, if the driver supports FDB isolation, DSA must perform
> refcounting per bridge, but if the driver doesn't, DSA must refcount
> host addresses across all bridges, otherwise it would be telling the
> driver to delete an FDB entry for a bridge and the driver would delete
> it for all bridges. So introduce a bool fdb_isolation in drivers which
> would make all bridge databases passed to the cross-chip notifier have
> the same number (0). This makes dsa_mac_addr_find() -> dsa_db_equal()
> say that all bridge databases are the same database - which is
> essentially the legacy behavior.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/b53/b53_common.c       | 12 ++--
>  drivers/net/dsa/b53/b53_priv.h         | 12 ++--
>  drivers/net/dsa/hirschmann/hellcreek.c |  6 +-
>  drivers/net/dsa/lan9303-core.c         | 13 ++--
>  drivers/net/dsa/lantiq_gswip.c         |  6 +-
>  drivers/net/dsa/microchip/ksz9477.c    | 12 ++--
>  drivers/net/dsa/microchip/ksz_common.c |  6 +-
>  drivers/net/dsa/microchip/ksz_common.h |  6 +-
>  drivers/net/dsa/mt7530.c               | 12 ++--
>  drivers/net/dsa/mv88e6xxx/chip.c       | 12 ++--
>  drivers/net/dsa/ocelot/felix.c         | 18 +++--
>  drivers/net/dsa/qca8k.c                | 12 ++--
>  drivers/net/dsa/sja1105/sja1105_main.c | 26 +++++--
>  include/net/dsa.h                      | 42 +++++++++--
>  net/dsa/dsa_priv.h                     |  3 +
>  net/dsa/port.c                         | 75 ++++++++++++++++++-
>  net/dsa/switch.c                       | 99 +++++++++++++++++---------
>  17 files changed, 280 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 83bf30349c26..a8cc6e182c45 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -1708,7 +1708,8 @@ static int b53_arl_op(struct b53_device *dev, int op, int port,
>  }
>  
>  int b53_fdb_add(struct dsa_switch *ds, int port,
> -		const unsigned char *addr, u16 vid)
> +		const unsigned char *addr, u16 vid,
> +		struct dsa_db db)
>  {
>  	struct b53_device *priv = ds->priv;
>  	int ret;
> @@ -1728,7 +1729,8 @@ int b53_fdb_add(struct dsa_switch *ds, int port,
>  EXPORT_SYMBOL(b53_fdb_add);
>  
>  int b53_fdb_del(struct dsa_switch *ds, int port,
> -		const unsigned char *addr, u16 vid)
> +		const unsigned char *addr, u16 vid,
> +		struct dsa_db db)
>  {
>  	struct b53_device *priv = ds->priv;
>  	int ret;
> @@ -1829,7 +1831,8 @@ int b53_fdb_dump(struct dsa_switch *ds, int port,
>  EXPORT_SYMBOL(b53_fdb_dump);
>  
>  int b53_mdb_add(struct dsa_switch *ds, int port,
> -		const struct switchdev_obj_port_mdb *mdb)
> +		const struct switchdev_obj_port_mdb *mdb,
> +		struct dsa_db db)
>  {
>  	struct b53_device *priv = ds->priv;
>  	int ret;
> @@ -1849,7 +1852,8 @@ int b53_mdb_add(struct dsa_switch *ds, int port,
>  EXPORT_SYMBOL(b53_mdb_add);
>  
>  int b53_mdb_del(struct dsa_switch *ds, int port,
> -		const struct switchdev_obj_port_mdb *mdb)
> +		const struct switchdev_obj_port_mdb *mdb,
> +		struct dsa_db db)
>  {
>  	struct b53_device *priv = ds->priv;
>  	int ret;
> diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
> index a6b339fcb17e..d3091f0ad3e6 100644
> --- a/drivers/net/dsa/b53/b53_priv.h
> +++ b/drivers/net/dsa/b53/b53_priv.h
> @@ -359,15 +359,19 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
>  int b53_vlan_del(struct dsa_switch *ds, int port,
>  		 const struct switchdev_obj_port_vlan *vlan);
>  int b53_fdb_add(struct dsa_switch *ds, int port,
> -		const unsigned char *addr, u16 vid);
> +		const unsigned char *addr, u16 vid,
> +		struct dsa_db db);
>  int b53_fdb_del(struct dsa_switch *ds, int port,
> -		const unsigned char *addr, u16 vid);
> +		const unsigned char *addr, u16 vid,
> +		struct dsa_db db);
>  int b53_fdb_dump(struct dsa_switch *ds, int port,
>  		 dsa_fdb_dump_cb_t *cb, void *data);
>  int b53_mdb_add(struct dsa_switch *ds, int port,
> -		const struct switchdev_obj_port_mdb *mdb);
> +		const struct switchdev_obj_port_mdb *mdb,
> +		struct dsa_db db);
>  int b53_mdb_del(struct dsa_switch *ds, int port,
> -		const struct switchdev_obj_port_mdb *mdb);
> +		const struct switchdev_obj_port_mdb *mdb,
> +		struct dsa_db db);
>  int b53_mirror_add(struct dsa_switch *ds, int port,
>  		   struct dsa_mall_mirror_tc_entry *mirror, bool ingress);
>  enum dsa_tag_protocol b53_get_tag_protocol(struct dsa_switch *ds, int port,
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index 726f267cb228..cb89be9de43a 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -827,7 +827,8 @@ static int hellcreek_fdb_get(struct hellcreek *hellcreek,
>  }
>  
>  static int hellcreek_fdb_add(struct dsa_switch *ds, int port,
> -			     const unsigned char *addr, u16 vid)
> +			     const unsigned char *addr, u16 vid,
> +			     struct dsa_db db)
>  {
>  	struct hellcreek_fdb_entry entry = { 0 };
>  	struct hellcreek *hellcreek = ds->priv;
> @@ -872,7 +873,8 @@ static int hellcreek_fdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int hellcreek_fdb_del(struct dsa_switch *ds, int port,
> -			     const unsigned char *addr, u16 vid)
> +			     const unsigned char *addr, u16 vid,
> +			     struct dsa_db db)
>  {
>  	struct hellcreek_fdb_entry entry = { 0 };
>  	struct hellcreek *hellcreek = ds->priv;
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index 3969d89fa4db..a21184e7fcb6 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -1188,7 +1188,8 @@ static void lan9303_port_fast_age(struct dsa_switch *ds, int port)
>  }
>  
>  static int lan9303_port_fdb_add(struct dsa_switch *ds, int port,
> -				const unsigned char *addr, u16 vid)
> +				const unsigned char *addr, u16 vid,
> +				struct dsa_db db)
>  {
>  	struct lan9303 *chip = ds->priv;
>  
> @@ -1200,8 +1201,8 @@ static int lan9303_port_fdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int lan9303_port_fdb_del(struct dsa_switch *ds, int port,
> -				const unsigned char *addr, u16 vid)
> -
> +				const unsigned char *addr, u16 vid,
> +				struct dsa_db db)
>  {
>  	struct lan9303 *chip = ds->priv;
>  
> @@ -1245,7 +1246,8 @@ static int lan9303_port_mdb_prepare(struct dsa_switch *ds, int port,
>  }
>  
>  static int lan9303_port_mdb_add(struct dsa_switch *ds, int port,
> -				const struct switchdev_obj_port_mdb *mdb)
> +				const struct switchdev_obj_port_mdb *mdb,
> +				struct dsa_db db)
>  {
>  	struct lan9303 *chip = ds->priv;
>  	int err;
> @@ -1260,7 +1262,8 @@ static int lan9303_port_mdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int lan9303_port_mdb_del(struct dsa_switch *ds, int port,
> -				const struct switchdev_obj_port_mdb *mdb)
> +				const struct switchdev_obj_port_mdb *mdb,
> +				struct dsa_db db)
>  {
>  	struct lan9303 *chip = ds->priv;
>  
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index 8a7a8093a156..3dfb532b7784 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -1389,13 +1389,15 @@ static int gswip_port_fdb(struct dsa_switch *ds, int port,
>  }
>  
>  static int gswip_port_fdb_add(struct dsa_switch *ds, int port,
> -			      const unsigned char *addr, u16 vid)
> +			      const unsigned char *addr, u16 vid,
> +			      struct dsa_db db)
>  {
>  	return gswip_port_fdb(ds, port, addr, vid, true);
>  }
>  
>  static int gswip_port_fdb_del(struct dsa_switch *ds, int port,
> -			      const unsigned char *addr, u16 vid)
> +			      const unsigned char *addr, u16 vid,
> +			      struct dsa_db db)
>  {
>  	return gswip_port_fdb(ds, port, addr, vid, false);
>  }
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index 18ffc8ded7ee..94ad6d9504f4 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -640,7 +640,8 @@ static int ksz9477_port_vlan_del(struct dsa_switch *ds, int port,
>  }
>  
>  static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
> -				const unsigned char *addr, u16 vid)
> +				const unsigned char *addr, u16 vid,
> +				struct dsa_db db)
>  {
>  	struct ksz_device *dev = ds->priv;
>  	u32 alu_table[4];
> @@ -697,7 +698,8 @@ static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int ksz9477_port_fdb_del(struct dsa_switch *ds, int port,
> -				const unsigned char *addr, u16 vid)
> +				const unsigned char *addr, u16 vid,
> +				struct dsa_db db)
>  {
>  	struct ksz_device *dev = ds->priv;
>  	u32 alu_table[4];
> @@ -839,7 +841,8 @@ static int ksz9477_port_fdb_dump(struct dsa_switch *ds, int port,
>  }
>  
>  static int ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
> -				const struct switchdev_obj_port_mdb *mdb)
> +				const struct switchdev_obj_port_mdb *mdb,
> +				struct dsa_db db)
>  {
>  	struct ksz_device *dev = ds->priv;
>  	u32 static_table[4];
> @@ -914,7 +917,8 @@ static int ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
> -				const struct switchdev_obj_port_mdb *mdb)
> +				const struct switchdev_obj_port_mdb *mdb,
> +				struct dsa_db db)
>  {
>  	struct ksz_device *dev = ds->priv;
>  	u32 static_table[4];
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 94e618b8352b..104458ec9cbc 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -276,7 +276,8 @@ int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
>  EXPORT_SYMBOL_GPL(ksz_port_fdb_dump);
>  
>  int ksz_port_mdb_add(struct dsa_switch *ds, int port,
> -		     const struct switchdev_obj_port_mdb *mdb)
> +		     const struct switchdev_obj_port_mdb *mdb,
> +		     struct dsa_db db)
>  {
>  	struct ksz_device *dev = ds->priv;
>  	struct alu_struct alu;
> @@ -321,7 +322,8 @@ int ksz_port_mdb_add(struct dsa_switch *ds, int port,
>  EXPORT_SYMBOL_GPL(ksz_port_mdb_add);
>  
>  int ksz_port_mdb_del(struct dsa_switch *ds, int port,
> -		     const struct switchdev_obj_port_mdb *mdb)
> +		     const struct switchdev_obj_port_mdb *mdb,
> +		     struct dsa_db db)
>  {
>  	struct ksz_device *dev = ds->priv;
>  	struct alu_struct alu;
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index c6fa487fb006..66933445a447 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -166,9 +166,11 @@ void ksz_port_fast_age(struct dsa_switch *ds, int port);
>  int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
>  		      void *data);
>  int ksz_port_mdb_add(struct dsa_switch *ds, int port,
> -		     const struct switchdev_obj_port_mdb *mdb);
> +		     const struct switchdev_obj_port_mdb *mdb,
> +		     struct dsa_db db);
>  int ksz_port_mdb_del(struct dsa_switch *ds, int port,
> -		     const struct switchdev_obj_port_mdb *mdb);
> +		     const struct switchdev_obj_port_mdb *mdb,
> +		     struct dsa_db db);
>  int ksz_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy);
>  
>  /* Common register access functions */
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index f74f25f479ed..abe63ec05066 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1349,7 +1349,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
>  
>  static int
>  mt7530_port_fdb_add(struct dsa_switch *ds, int port,
> -		    const unsigned char *addr, u16 vid)
> +		    const unsigned char *addr, u16 vid,
> +		    struct dsa_db db)
>  {
>  	struct mt7530_priv *priv = ds->priv;
>  	int ret;
> @@ -1365,7 +1366,8 @@ mt7530_port_fdb_add(struct dsa_switch *ds, int port,
>  
>  static int
>  mt7530_port_fdb_del(struct dsa_switch *ds, int port,
> -		    const unsigned char *addr, u16 vid)
> +		    const unsigned char *addr, u16 vid,
> +		    struct dsa_db db)
>  {
>  	struct mt7530_priv *priv = ds->priv;
>  	int ret;
> @@ -1416,7 +1418,8 @@ mt7530_port_fdb_dump(struct dsa_switch *ds, int port,
>  
>  static int
>  mt7530_port_mdb_add(struct dsa_switch *ds, int port,
> -		    const struct switchdev_obj_port_mdb *mdb)
> +		    const struct switchdev_obj_port_mdb *mdb,
> +		    struct dsa_db db)
>  {
>  	struct mt7530_priv *priv = ds->priv;
>  	const u8 *addr = mdb->addr;
> @@ -1442,7 +1445,8 @@ mt7530_port_mdb_add(struct dsa_switch *ds, int port,
>  
>  static int
>  mt7530_port_mdb_del(struct dsa_switch *ds, int port,
> -		    const struct switchdev_obj_port_mdb *mdb)
> +		    const struct switchdev_obj_port_mdb *mdb,
> +		    struct dsa_db db)
>  {
>  	struct mt7530_priv *priv = ds->priv;
>  	const u8 *addr = mdb->addr;
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 1b9a20bf1bd6..d79c65bb227e 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2456,7 +2456,8 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
>  }
>  
>  static int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
> -				  const unsigned char *addr, u16 vid)
> +				  const unsigned char *addr, u16 vid,
> +				  struct dsa_db db)
>  {
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  	int err;
> @@ -2470,7 +2471,8 @@ static int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
> -				  const unsigned char *addr, u16 vid)
> +				  const unsigned char *addr, u16 vid,
> +				  struct dsa_db db)
>  {
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  	int err;
> @@ -6002,7 +6004,8 @@ static int mv88e6xxx_change_tag_protocol(struct dsa_switch *ds, int port,
>  }
>  
>  static int mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
> -				  const struct switchdev_obj_port_mdb *mdb)
> +				  const struct switchdev_obj_port_mdb *mdb,
> +				  struct dsa_db db)
>  {
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  	int err;
> @@ -6016,7 +6019,8 @@ static int mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
> -				  const struct switchdev_obj_port_mdb *mdb)
> +				  const struct switchdev_obj_port_mdb *mdb,
> +				  struct dsa_db db)
>  {
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  	int err;
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index 04f5da33b944..d92feee97c63 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -592,7 +592,8 @@ static int felix_fdb_dump(struct dsa_switch *ds, int port,
>  }
>  
>  static int felix_fdb_add(struct dsa_switch *ds, int port,
> -			 const unsigned char *addr, u16 vid)
> +			 const unsigned char *addr, u16 vid,
> +			 struct dsa_db db)
>  {
>  	struct ocelot *ocelot = ds->priv;
>  
> @@ -600,7 +601,8 @@ static int felix_fdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int felix_fdb_del(struct dsa_switch *ds, int port,
> -			 const unsigned char *addr, u16 vid)
> +			 const unsigned char *addr, u16 vid,
> +			 struct dsa_db db)
>  {
>  	struct ocelot *ocelot = ds->priv;
>  
> @@ -608,7 +610,8 @@ static int felix_fdb_del(struct dsa_switch *ds, int port,
>  }
>  
>  static int felix_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag lag,
> -			     const unsigned char *addr, u16 vid)
> +			     const unsigned char *addr, u16 vid,
> +			     struct dsa_db db)
>  {
>  	struct ocelot *ocelot = ds->priv;
>  
> @@ -616,7 +619,8 @@ static int felix_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag lag,
>  }
>  
>  static int felix_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag lag,
> -			     const unsigned char *addr, u16 vid)
> +			     const unsigned char *addr, u16 vid,
> +			     struct dsa_db db)
>  {
>  	struct ocelot *ocelot = ds->priv;
>  
> @@ -624,7 +628,8 @@ static int felix_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag lag,
>  }
>  
>  static int felix_mdb_add(struct dsa_switch *ds, int port,
> -			 const struct switchdev_obj_port_mdb *mdb)
> +			 const struct switchdev_obj_port_mdb *mdb,
> +			 struct dsa_db db)
>  {
>  	struct ocelot *ocelot = ds->priv;
>  
> @@ -632,7 +637,8 @@ static int felix_mdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int felix_mdb_del(struct dsa_switch *ds, int port,
> -			 const struct switchdev_obj_port_mdb *mdb)
> +			 const struct switchdev_obj_port_mdb *mdb,
> +			 struct dsa_db db)
>  {
>  	struct ocelot *ocelot = ds->priv;
>  
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 6844106975a9..7189fd8120d7 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -2397,7 +2397,8 @@ qca8k_port_fdb_insert(struct qca8k_priv *priv, const u8 *addr,
>  
>  static int
>  qca8k_port_fdb_add(struct dsa_switch *ds, int port,
> -		   const unsigned char *addr, u16 vid)
> +		   const unsigned char *addr, u16 vid,
> +		   struct dsa_db db)
>  {
>  	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
>  	u16 port_mask = BIT(port);
> @@ -2407,7 +2408,8 @@ qca8k_port_fdb_add(struct dsa_switch *ds, int port,
>  
>  static int
>  qca8k_port_fdb_del(struct dsa_switch *ds, int port,
> -		   const unsigned char *addr, u16 vid)
> +		   const unsigned char *addr, u16 vid,
> +		   struct dsa_db db)
>  {
>  	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
>  	u16 port_mask = BIT(port);
> @@ -2444,7 +2446,8 @@ qca8k_port_fdb_dump(struct dsa_switch *ds, int port,
>  
>  static int
>  qca8k_port_mdb_add(struct dsa_switch *ds, int port,
> -		   const struct switchdev_obj_port_mdb *mdb)
> +		   const struct switchdev_obj_port_mdb *mdb,
> +		   struct dsa_db db)
>  {
>  	struct qca8k_priv *priv = ds->priv;
>  	const u8 *addr = mdb->addr;
> @@ -2455,7 +2458,8 @@ qca8k_port_mdb_add(struct dsa_switch *ds, int port,
>  
>  static int
>  qca8k_port_mdb_del(struct dsa_switch *ds, int port,
> -		   const struct switchdev_obj_port_mdb *mdb)
> +		   const struct switchdev_obj_port_mdb *mdb,
> +		   struct dsa_db db)
>  {
>  	struct qca8k_priv *priv = ds->priv;
>  	const u8 *addr = mdb->addr;
> diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> index dd89b077aae6..91b0e636d194 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -1819,7 +1819,8 @@ int sja1105pqrs_fdb_del(struct dsa_switch *ds, int port,
>  }
>  
>  static int sja1105_fdb_add(struct dsa_switch *ds, int port,
> -			   const unsigned char *addr, u16 vid)
> +			   const unsigned char *addr, u16 vid,
> +			   struct dsa_db db)
>  {
>  	struct sja1105_private *priv = ds->priv;
>  
> @@ -1827,7 +1828,8 @@ static int sja1105_fdb_add(struct dsa_switch *ds, int port,
>  }
>  
>  static int sja1105_fdb_del(struct dsa_switch *ds, int port,
> -			   const unsigned char *addr, u16 vid)
> +			   const unsigned char *addr, u16 vid,
> +			   struct dsa_db db)
>  {
>  	struct sja1105_private *priv = ds->priv;
>  
> @@ -1885,7 +1887,15 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
>  
>  static void sja1105_fast_age(struct dsa_switch *ds, int port)
>  {
> +	struct dsa_port *dp = dsa_to_port(ds, port);
>  	struct sja1105_private *priv = ds->priv;
> +	struct dsa_db db = {
> +		.type = DSA_DB_BRIDGE,
> +		.bridge = {
> +			.dev = dsa_port_bridge_dev_get(dp),
> +			.num = dsa_port_bridge_num_get(dp),
> +		},
> +	};
>  	int i;
>  
>  	for (i = 0; i < SJA1105_MAX_L2_LOOKUP_COUNT; i++) {
> @@ -1913,7 +1923,7 @@ static void sja1105_fast_age(struct dsa_switch *ds, int port)
>  
>  		u64_to_ether_addr(l2_lookup.macaddr, macaddr);
>  
> -		rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid);
> +		rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
>  		if (rc) {
>  			dev_err(ds->dev,
>  				"Failed to delete FDB entry %pM vid %lld: %pe\n",
> @@ -1924,15 +1934,17 @@ static void sja1105_fast_age(struct dsa_switch *ds, int port)
>  }
>  
>  static int sja1105_mdb_add(struct dsa_switch *ds, int port,
> -			   const struct switchdev_obj_port_mdb *mdb)
> +			   const struct switchdev_obj_port_mdb *mdb,
> +			   struct dsa_db db)
>  {
> -	return sja1105_fdb_add(ds, port, mdb->addr, mdb->vid);
> +	return sja1105_fdb_add(ds, port, mdb->addr, mdb->vid, db);
>  }
>  
>  static int sja1105_mdb_del(struct dsa_switch *ds, int port,
> -			   const struct switchdev_obj_port_mdb *mdb)
> +			   const struct switchdev_obj_port_mdb *mdb,
> +			   struct dsa_db db)
>  {
> -	return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid);
> +	return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid, db);
>  }
>  
>  /* Common function for unicast and broadcast flood configuration.
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 01faba89c987..87c5f18eb381 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -341,11 +341,28 @@ struct dsa_link {
>  	struct list_head list;
>  };
>  
> +enum dsa_db_type {
> +	DSA_DB_PORT,
> +	DSA_DB_LAG,
> +	DSA_DB_BRIDGE,
> +};
> +
> +struct dsa_db {
> +	enum dsa_db_type type;
> +
> +	union {
> +		const struct dsa_port *dp;
> +		struct dsa_lag lag;
> +		struct dsa_bridge bridge;
> +	};
> +};
> +
>  struct dsa_mac_addr {
>  	unsigned char addr[ETH_ALEN];
>  	u16 vid;
>  	refcount_t refcount;
>  	struct list_head list;
> +	struct dsa_db db;
>  };
>  
>  struct dsa_vlan {
> @@ -409,6 +426,13 @@ struct dsa_switch {
>  	 */
>  	u32			mtu_enforcement_ingress:1;
>  
> +	/* Drivers that isolate the FDBs of multiple bridges must set this
> +	 * to true to receive the bridge as an argument in .port_fdb_{add,del}
> +	 * and .port_mdb_{add,del}. Otherwise, the bridge.num will always be
> +	 * passed as zero.
> +	 */
> +	u32			fdb_isolation:1;
> +
>  	/* Listener for switch fabric events */
>  	struct notifier_block	nb;
>  
> @@ -941,23 +965,29 @@ struct dsa_switch_ops {
>  	 * Forwarding database
>  	 */
>  	int	(*port_fdb_add)(struct dsa_switch *ds, int port,
> -				const unsigned char *addr, u16 vid);
> +				const unsigned char *addr, u16 vid,
> +				struct dsa_db db);

Hi! Wouldn't it be better to have a struct that has all the functions
parameters in one instead of adding further parameters to these
functions?

I am asking because I am also needing to add a parameter to
port_fdb_add(), and it would be more future oriented to have a single
function parameter as a struct, so that it is easier to add parameters
to these functions without havíng to change the prototype of the
function every time.

>  	int	(*port_fdb_del)(struct dsa_switch *ds, int port,
> -				const unsigned char *addr, u16 vid);
> +				const unsigned char *addr, u16 vid,
> +				struct dsa_db db);
>  	int	(*port_fdb_dump)(struct dsa_switch *ds, int port,
>  				 dsa_fdb_dump_cb_t *cb, void *data);
>  	int	(*lag_fdb_add)(struct dsa_switch *ds, struct dsa_lag lag,
> -			       const unsigned char *addr, u16 vid);
> +			       const unsigned char *addr, u16 vid,
> +			       struct dsa_db db);
>  	int	(*lag_fdb_del)(struct dsa_switch *ds, struct dsa_lag lag,
> -			       const unsigned char *addr, u16 vid);
> +			       const unsigned char *addr, u16 vid,
> +			       struct dsa_db db);
>  
>  	/*
>  	 * Multicast database
>  	 */
>  	int	(*port_mdb_add)(struct dsa_switch *ds, int port,
> -				const struct switchdev_obj_port_mdb *mdb);
> +				const struct switchdev_obj_port_mdb *mdb,
> +				struct dsa_db db);
>  	int	(*port_mdb_del)(struct dsa_switch *ds, int port,
> -				const struct switchdev_obj_port_mdb *mdb);
> +				const struct switchdev_obj_port_mdb *mdb,
> +				struct dsa_db db);
>  	/*
>  	 * RXNFC
>  	 */
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 7a1c98581f53..27575fc3883e 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -67,6 +67,7 @@ struct dsa_notifier_fdb_info {
>  	int port;
>  	const unsigned char *addr;
>  	u16 vid;
> +	struct dsa_db db;
>  };
>  
>  /* DSA_NOTIFIER_LAG_FDB_* */
> @@ -74,6 +75,7 @@ struct dsa_notifier_lag_fdb_info {
>  	struct dsa_lag *lag;
>  	const unsigned char *addr;
>  	u16 vid;
> +	struct dsa_db db;
>  };
>  
>  /* DSA_NOTIFIER_MDB_* */
> @@ -81,6 +83,7 @@ struct dsa_notifier_mdb_info {
>  	const struct switchdev_obj_port_mdb *mdb;
>  	int sw_index;
>  	int port;
> +	struct dsa_db db;
>  };
>  
>  /* DSA_NOTIFIER_LAG_* */
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index adab159c8c21..7af44a28f032 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -798,8 +798,19 @@ int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>  		.port = dp->index,
>  		.addr = addr,
>  		.vid = vid,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  
> +	/* Refcounting takes bridge.num as a key, and should be global for all
> +	 * bridges in the absence of FDB isolation, and per bridge otherwise.
> +	 * Force the bridge.num to zero here in the absence of FDB isolation.
> +	 */
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_FDB_ADD, &info);
>  }
>  
> @@ -811,9 +822,15 @@ int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
>  		.port = dp->index,
>  		.addr = addr,
>  		.vid = vid,
> -
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_FDB_DEL, &info);
>  }
>  
> @@ -825,6 +842,10 @@ int dsa_port_host_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>  		.port = dp->index,
>  		.addr = addr,
>  		.vid = vid,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  	struct dsa_port *cpu_dp = dp->cpu_dp;
>  	int err;
> @@ -839,6 +860,9 @@ int dsa_port_host_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>  			return err;
>  	}
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_FDB_ADD, &info);
>  }
>  
> @@ -850,6 +874,10 @@ int dsa_port_host_fdb_del(struct dsa_port *dp, const unsigned char *addr,
>  		.port = dp->index,
>  		.addr = addr,
>  		.vid = vid,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  	struct dsa_port *cpu_dp = dp->cpu_dp;
>  	int err;
> @@ -860,6 +888,9 @@ int dsa_port_host_fdb_del(struct dsa_port *dp, const unsigned char *addr,
>  			return err;
>  	}
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_FDB_DEL, &info);
>  }
>  
> @@ -870,8 +901,15 @@ int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>  		.lag = dp->lag,
>  		.addr = addr,
>  		.vid = vid,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_ADD, &info);
>  }
>  
> @@ -882,8 +920,15 @@ int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
>  		.lag = dp->lag,
>  		.addr = addr,
>  		.vid = vid,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_DEL, &info);
>  }
>  
> @@ -905,8 +950,15 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
>  		.sw_index = dp->ds->index,
>  		.port = dp->index,
>  		.mdb = mdb,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_ADD, &info);
>  }
>  
> @@ -917,8 +969,15 @@ int dsa_port_mdb_del(const struct dsa_port *dp,
>  		.sw_index = dp->ds->index,
>  		.port = dp->index,
>  		.mdb = mdb,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_DEL, &info);
>  }
>  
> @@ -929,6 +988,10 @@ int dsa_port_host_mdb_add(const struct dsa_port *dp,
>  		.sw_index = dp->ds->index,
>  		.port = dp->index,
>  		.mdb = mdb,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  	struct dsa_port *cpu_dp = dp->cpu_dp;
>  	int err;
> @@ -937,6 +1000,9 @@ int dsa_port_host_mdb_add(const struct dsa_port *dp,
>  	if (err)
>  		return err;
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_ADD, &info);
>  }
>  
> @@ -947,6 +1013,10 @@ int dsa_port_host_mdb_del(const struct dsa_port *dp,
>  		.sw_index = dp->ds->index,
>  		.port = dp->index,
>  		.mdb = mdb,
> +		.db = {
> +			.type = DSA_DB_BRIDGE,
> +			.bridge = *dp->bridge,
> +		},
>  	};
>  	struct dsa_port *cpu_dp = dp->cpu_dp;
>  	int err;
> @@ -955,6 +1025,9 @@ int dsa_port_host_mdb_del(const struct dsa_port *dp,
>  	if (err)
>  		return err;
>  
> +	if (!dp->ds->fdb_isolation)
> +		info.db.bridge.num = 0;
> +
>  	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_DEL, &info);
>  }
>  
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index eb38beb10147..1d3c161e3131 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -210,21 +210,41 @@ static bool dsa_port_host_address_match(struct dsa_port *dp,
>  	return false;
>  }
>  
> +static bool dsa_db_equal(const struct dsa_db *a, const struct dsa_db *b)
> +{
> +	if (a->type != b->type)
> +		return false;
> +
> +	switch (a->type) {
> +	case DSA_DB_PORT:
> +		return a->dp == b->dp;
> +	case DSA_DB_LAG:
> +		return a->lag.dev == b->lag.dev;
> +	case DSA_DB_BRIDGE:
> +		return a->bridge.num == b->bridge.num;
> +	default:
> +		WARN_ON(1);
> +		return false;
> +	}
> +}
> +
>  static struct dsa_mac_addr *dsa_mac_addr_find(struct list_head *addr_list,
> -					      const unsigned char *addr,
> -					      u16 vid)
> +					      const unsigned char *addr, u16 vid,
> +					      struct dsa_db db)
>  {
>  	struct dsa_mac_addr *a;
>  
>  	list_for_each_entry(a, addr_list, list)
> -		if (ether_addr_equal(a->addr, addr) && a->vid == vid)
> +		if (ether_addr_equal(a->addr, addr) && a->vid == vid &&
> +		    dsa_db_equal(&a->db, &db))
>  			return a;
>  
>  	return NULL;
>  }
>  
>  static int dsa_port_do_mdb_add(struct dsa_port *dp,
> -			       const struct switchdev_obj_port_mdb *mdb)
> +			       const struct switchdev_obj_port_mdb *mdb,
> +			       struct dsa_db db)
>  {
>  	struct dsa_switch *ds = dp->ds;
>  	struct dsa_mac_addr *a;
> @@ -233,11 +253,11 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp,
>  
>  	/* No need to bother with refcounting for user ports */
>  	if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)))
> -		return ds->ops->port_mdb_add(ds, port, mdb);
> +		return ds->ops->port_mdb_add(ds, port, mdb, db);
>  
>  	mutex_lock(&dp->addr_lists_lock);
>  
> -	a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid);
> +	a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid, db);
>  	if (a) {
>  		refcount_inc(&a->refcount);
>  		goto out;
> @@ -249,7 +269,7 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp,
>  		goto out;
>  	}
>  
> -	err = ds->ops->port_mdb_add(ds, port, mdb);
> +	err = ds->ops->port_mdb_add(ds, port, mdb, db);
>  	if (err) {
>  		kfree(a);
>  		goto out;
> @@ -257,6 +277,7 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp,
>  
>  	ether_addr_copy(a->addr, mdb->addr);
>  	a->vid = mdb->vid;
> +	a->db = db;
>  	refcount_set(&a->refcount, 1);
>  	list_add_tail(&a->list, &dp->mdbs);
>  
> @@ -267,7 +288,8 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp,
>  }
>  
>  static int dsa_port_do_mdb_del(struct dsa_port *dp,
> -			       const struct switchdev_obj_port_mdb *mdb)
> +			       const struct switchdev_obj_port_mdb *mdb,
> +			       struct dsa_db db)
>  {
>  	struct dsa_switch *ds = dp->ds;
>  	struct dsa_mac_addr *a;
> @@ -276,11 +298,11 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp,
>  
>  	/* No need to bother with refcounting for user ports */
>  	if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)))
> -		return ds->ops->port_mdb_del(ds, port, mdb);
> +		return ds->ops->port_mdb_del(ds, port, mdb, db);
>  
>  	mutex_lock(&dp->addr_lists_lock);
>  
> -	a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid);
> +	a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid, db);
>  	if (!a) {
>  		err = -ENOENT;
>  		goto out;
> @@ -289,7 +311,7 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp,
>  	if (!refcount_dec_and_test(&a->refcount))
>  		goto out;
>  
> -	err = ds->ops->port_mdb_del(ds, port, mdb);
> +	err = ds->ops->port_mdb_del(ds, port, mdb, db);
>  	if (err) {
>  		refcount_set(&a->refcount, 1);
>  		goto out;
> @@ -305,7 +327,7 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp,
>  }
>  
>  static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
> -			       u16 vid)
> +			       u16 vid, struct dsa_db db)
>  {
>  	struct dsa_switch *ds = dp->ds;
>  	struct dsa_mac_addr *a;
> @@ -314,11 +336,11 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>  
>  	/* No need to bother with refcounting for user ports */
>  	if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)))
> -		return ds->ops->port_fdb_add(ds, port, addr, vid);
> +		return ds->ops->port_fdb_add(ds, port, addr, vid, db);
>  
>  	mutex_lock(&dp->addr_lists_lock);
>  
> -	a = dsa_mac_addr_find(&dp->fdbs, addr, vid);
> +	a = dsa_mac_addr_find(&dp->fdbs, addr, vid, db);
>  	if (a) {
>  		refcount_inc(&a->refcount);
>  		goto out;
> @@ -330,7 +352,7 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>  		goto out;
>  	}
>  
> -	err = ds->ops->port_fdb_add(ds, port, addr, vid);
> +	err = ds->ops->port_fdb_add(ds, port, addr, vid, db);
>  	if (err) {
>  		kfree(a);
>  		goto out;
> @@ -338,6 +360,7 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>  
>  	ether_addr_copy(a->addr, addr);
>  	a->vid = vid;
> +	a->db = db;
>  	refcount_set(&a->refcount, 1);
>  	list_add_tail(&a->list, &dp->fdbs);
>  
> @@ -348,7 +371,7 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>  }
>  
>  static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
> -			       u16 vid)
> +			       u16 vid, struct dsa_db db)
>  {
>  	struct dsa_switch *ds = dp->ds;
>  	struct dsa_mac_addr *a;
> @@ -357,11 +380,11 @@ static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
>  
>  	/* No need to bother with refcounting for user ports */
>  	if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)))
> -		return ds->ops->port_fdb_del(ds, port, addr, vid);
> +		return ds->ops->port_fdb_del(ds, port, addr, vid, db);
>  
>  	mutex_lock(&dp->addr_lists_lock);
>  
> -	a = dsa_mac_addr_find(&dp->fdbs, addr, vid);
> +	a = dsa_mac_addr_find(&dp->fdbs, addr, vid, db);
>  	if (!a) {
>  		err = -ENOENT;
>  		goto out;
> @@ -370,7 +393,7 @@ static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
>  	if (!refcount_dec_and_test(&a->refcount))
>  		goto out;
>  
> -	err = ds->ops->port_fdb_del(ds, port, addr, vid);
> +	err = ds->ops->port_fdb_del(ds, port, addr, vid, db);
>  	if (err) {
>  		refcount_set(&a->refcount, 1);
>  		goto out;
> @@ -386,14 +409,15 @@ static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
>  }
>  
>  static int dsa_switch_do_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag *lag,
> -				     const unsigned char *addr, u16 vid)
> +				     const unsigned char *addr, u16 vid,
> +				     struct dsa_db db)
>  {
>  	struct dsa_mac_addr *a;
>  	int err = 0;
>  
>  	mutex_lock(&lag->fdb_lock);
>  
> -	a = dsa_mac_addr_find(&lag->fdbs, addr, vid);
> +	a = dsa_mac_addr_find(&lag->fdbs, addr, vid, db);
>  	if (a) {
>  		refcount_inc(&a->refcount);
>  		goto out;
> @@ -405,7 +429,7 @@ static int dsa_switch_do_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag *lag,
>  		goto out;
>  	}
>  
> -	err = ds->ops->lag_fdb_add(ds, *lag, addr, vid);
> +	err = ds->ops->lag_fdb_add(ds, *lag, addr, vid, db);
>  	if (err) {
>  		kfree(a);
>  		goto out;
> @@ -423,14 +447,15 @@ static int dsa_switch_do_lag_fdb_add(struct dsa_switch *ds, struct dsa_lag *lag,
>  }
>  
>  static int dsa_switch_do_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag *lag,
> -				     const unsigned char *addr, u16 vid)
> +				     const unsigned char *addr, u16 vid,
> +				     struct dsa_db db)
>  {
>  	struct dsa_mac_addr *a;
>  	int err = 0;
>  
>  	mutex_lock(&lag->fdb_lock);
>  
> -	a = dsa_mac_addr_find(&lag->fdbs, addr, vid);
> +	a = dsa_mac_addr_find(&lag->fdbs, addr, vid, db);
>  	if (!a) {
>  		err = -ENOENT;
>  		goto out;
> @@ -439,7 +464,7 @@ static int dsa_switch_do_lag_fdb_del(struct dsa_switch *ds, struct dsa_lag *lag,
>  	if (!refcount_dec_and_test(&a->refcount))
>  		goto out;
>  
> -	err = ds->ops->lag_fdb_del(ds, *lag, addr, vid);
> +	err = ds->ops->lag_fdb_del(ds, *lag, addr, vid, db);
>  	if (err) {
>  		refcount_set(&a->refcount, 1);
>  		goto out;
> @@ -466,7 +491,8 @@ static int dsa_switch_host_fdb_add(struct dsa_switch *ds,
>  	dsa_switch_for_each_port(dp, ds) {
>  		if (dsa_port_host_address_match(dp, info->sw_index,
>  						info->port)) {
> -			err = dsa_port_do_fdb_add(dp, info->addr, info->vid);
> +			err = dsa_port_do_fdb_add(dp, info->addr, info->vid,
> +						  info->db);
>  			if (err)
>  				break;
>  		}
> @@ -487,7 +513,8 @@ static int dsa_switch_host_fdb_del(struct dsa_switch *ds,
>  	dsa_switch_for_each_port(dp, ds) {
>  		if (dsa_port_host_address_match(dp, info->sw_index,
>  						info->port)) {
> -			err = dsa_port_do_fdb_del(dp, info->addr, info->vid);
> +			err = dsa_port_do_fdb_del(dp, info->addr, info->vid,
> +						  info->db);
>  			if (err)
>  				break;
>  		}
> @@ -505,7 +532,7 @@ static int dsa_switch_fdb_add(struct dsa_switch *ds,
>  	if (!ds->ops->port_fdb_add)
>  		return -EOPNOTSUPP;
>  
> -	return dsa_port_do_fdb_add(dp, info->addr, info->vid);
> +	return dsa_port_do_fdb_add(dp, info->addr, info->vid, info->db);
>  }
>  
>  static int dsa_switch_fdb_del(struct dsa_switch *ds,
> @@ -517,7 +544,7 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds,
>  	if (!ds->ops->port_fdb_del)
>  		return -EOPNOTSUPP;
>  
> -	return dsa_port_do_fdb_del(dp, info->addr, info->vid);
> +	return dsa_port_do_fdb_del(dp, info->addr, info->vid, info->db);
>  }
>  
>  static int dsa_switch_lag_fdb_add(struct dsa_switch *ds,
> @@ -532,7 +559,8 @@ static int dsa_switch_lag_fdb_add(struct dsa_switch *ds,
>  	dsa_switch_for_each_port(dp, ds)
>  		if (dsa_port_offloads_lag(dp, info->lag))
>  			return dsa_switch_do_lag_fdb_add(ds, info->lag,
> -							 info->addr, info->vid);
> +							 info->addr, info->vid,
> +							 info->db);
>  
>  	return 0;
>  }
> @@ -549,7 +577,8 @@ static int dsa_switch_lag_fdb_del(struct dsa_switch *ds,
>  	dsa_switch_for_each_port(dp, ds)
>  		if (dsa_port_offloads_lag(dp, info->lag))
>  			return dsa_switch_do_lag_fdb_del(ds, info->lag,
> -							 info->addr, info->vid);
> +							 info->addr, info->vid,
> +							 info->db);
>  
>  	return 0;
>  }
> @@ -604,7 +633,7 @@ static int dsa_switch_mdb_add(struct dsa_switch *ds,
>  	if (!ds->ops->port_mdb_add)
>  		return -EOPNOTSUPP;
>  
> -	return dsa_port_do_mdb_add(dp, info->mdb);
> +	return dsa_port_do_mdb_add(dp, info->mdb, info->db);
>  }
>  
>  static int dsa_switch_mdb_del(struct dsa_switch *ds,
> @@ -616,7 +645,7 @@ static int dsa_switch_mdb_del(struct dsa_switch *ds,
>  	if (!ds->ops->port_mdb_del)
>  		return -EOPNOTSUPP;
>  
> -	return dsa_port_do_mdb_del(dp, info->mdb);
> +	return dsa_port_do_mdb_del(dp, info->mdb, info->db);
>  }
>  
>  static int dsa_switch_host_mdb_add(struct dsa_switch *ds,
> @@ -631,7 +660,7 @@ static int dsa_switch_host_mdb_add(struct dsa_switch *ds,
>  	dsa_switch_for_each_port(dp, ds) {
>  		if (dsa_port_host_address_match(dp, info->sw_index,
>  						info->port)) {
> -			err = dsa_port_do_mdb_add(dp, info->mdb);
> +			err = dsa_port_do_mdb_add(dp, info->mdb, info->db);
>  			if (err)
>  				break;
>  		}
> @@ -652,7 +681,7 @@ static int dsa_switch_host_mdb_del(struct dsa_switch *ds,
>  	dsa_switch_for_each_port(dp, ds) {
>  		if (dsa_port_host_address_match(dp, info->sw_index,
>  						info->port)) {
> -			err = dsa_port_do_mdb_del(dp, info->mdb);
> +			err = dsa_port_do_mdb_del(dp, info->mdb, info->db);
>  			if (err)
>  				break;
>  		}
> -- 
> 2.25.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation
  2022-04-26 15:01   ` Hans Schultz
@ 2022-04-26 16:14     ` Andrew Lunn
  2022-04-26 23:17       ` Vladimir Oltean
  2022-04-27  6:45       ` Hans Schultz
  0 siblings, 2 replies; 19+ messages in thread
From: Andrew Lunn @ 2022-04-26 16:14 UTC (permalink / raw)
  To: Hans Schultz
  Cc: Vladimir Oltean, netdev, Jakub Kicinski, David S. Miller,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

> > @@ -941,23 +965,29 @@ struct dsa_switch_ops {
> >  	 * Forwarding database
> >  	 */
> >  	int	(*port_fdb_add)(struct dsa_switch *ds, int port,
> > -				const unsigned char *addr, u16 vid);
> > +				const unsigned char *addr, u16 vid,
> > +				struct dsa_db db);
> 
> Hi! Wouldn't it be better to have a struct that has all the functions
> parameters in one instead of adding further parameters to these
> functions?
> 
> I am asking because I am also needing to add a parameter to
> port_fdb_add(), and it would be more future oriented to have a single
> function parameter as a struct, so that it is easier to add parameters
> to these functions without havíng to change the prototype of the
> function every time.

Hi Hans

Please trim the text to only what is relevant when replying. It is
easy to miss comments when having to Page Down, Page Down, Page down,
to find comments.

   Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation
  2022-04-26 16:14     ` Andrew Lunn
@ 2022-04-26 23:17       ` Vladimir Oltean
  2022-04-27  8:38         ` Hans Schultz
  2022-04-27  6:45       ` Hans Schultz
  1 sibling, 1 reply; 19+ messages in thread
From: Vladimir Oltean @ 2022-04-26 23:17 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Hans Schultz, netdev, Jakub Kicinski, David S. Miller,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

On Tue, Apr 26, 2022 at 06:14:23PM +0200, Andrew Lunn wrote:
> > > @@ -941,23 +965,29 @@ struct dsa_switch_ops {
> > >  	 * Forwarding database
> > >  	 */
> > >  	int	(*port_fdb_add)(struct dsa_switch *ds, int port,
> > > -				const unsigned char *addr, u16 vid);
> > > +				const unsigned char *addr, u16 vid,
> > > +				struct dsa_db db);
> > 
> > Hi! Wouldn't it be better to have a struct that has all the functions
> > parameters in one instead of adding further parameters to these
> > functions?
> > 
> > I am asking because I am also needing to add a parameter to
> > port_fdb_add(), and it would be more future oriented to have a single
> > function parameter as a struct, so that it is easier to add parameters
> > to these functions without havíng to change the prototype of the
> > function every time.
> 
> Hi Hans
> 
> Please trim the text to only what is relevant when replying. It is
> easy to miss comments when having to Page Down, Page Down, Page down,
> to find comments.
> 
>    Andrew

Agreed, had to scroll too much.

Hans, what extra argument do you want to add to port_fdb_add?
A static/dynamic, I suppose, similar to what exists in port_fdb_dump?

But we surely wouldn't pass _all_ parameters of port_fdb_add through
some giant struct args_of_port_fdb_add, would we?  Not ds, port, db,
just what is naturally grouped together as an FDB entry: addr, vid,
maybe your new static/dynamic thing.

If we group the addr and vid in port_fdb_add into a structure that
represents an FDB entry, what do we do about those in port_fdb_del?
Group those as well, maybe for consistency?

Same question for port_fdb_dump and its dsa_fdb_dump_cb_t: would you
change it for uniformity, or would you keep it the way it is to reduce
the churn? I mean it's a perfectly viable candidate for argument
grouping, but your stated goal _is_ to reduce churn.

But if we add the static/dynamic boolean to this structure, does it make
sense on deletion? And if it doesn't, why have we changed the prototype
of port_fdb_del to include it?

Restated: do we want to treat the "static/dynamic" info as a property of
an FDB entry (i.e. a member of the structure), or as the way in which a
certain FDB entry can be added to hardware (case in which it is relevant
only to _add and to _dump)?  After all, an FDB entry for {addr, vid}
learned statically, and another FDB entry for the same {addr, vid} but
learned dynamically, are fundamentally the same object.

And if we don't go with a big struct args_of_port_fdb_add (which would
be silly if we did), who guarantees that the argument list of port_fdb_add
won't grow in the future anyway? Like in the example I just gave above,
where "static/dynamic" doesn't fully appear to group naturally with
"addr" and "vid", and would probably still be a separate boolean,
rendering the whole point moot.

And even grouping only those last 2 together is maybe debatable due to
practical reasons - where do we declare this small structure? We have a
struct switchdev_notifier_fdb_info with some more stuff that we
deliberately do not want to expose, and {addr, vid} are all that remain.

Although maybe there are benefits to having a small {addr, vid} structure
defined somewhere publicly, too, and referenced consistently by driver
code. Like for example to avoid bad patterns from proliferating.
Currently we have "const unsigned char *addr, u16 vid", so on a 64 bit
machine, this is a pointer plus an unsigned short, 10 bytes, padded up
by the compiler, maybe to 16. But the Ethernet addresses are 6 bytes,
those are shorter than the pointer itself, so on a 64-bit machine,
having "unsigned char addr[ETH_ALEN], u16 vid" makes a lot more space,
saves some real memory.

Anyway, I'm side tracking. If you want to make changes, propose a
patch, but come up with a more real argument than "reducing churn"
(while effectively producing churn).

To give you a counter example, phylink_mac_config() used to have the
opposite problem, the arguments were all neatly packed into a struct
phylink_link_state, but as the kerneldocs from include/linux/phylink.h
put it, not all members of that structure were guaranteed to contain
valid values. So there were bugs due to people not realizing this, and
consequently, phylink_mac_link_up() took the opposite approach, of
explicitly passing all known parameters of the resolved link state as
individual arguments. Now there are 10 arguments to that function, but
somehow at least this appears to have worked better, in the sense that
there isn't an explanatory note saying what's valid and what isn't.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation
  2022-04-26 16:14     ` Andrew Lunn
  2022-04-26 23:17       ` Vladimir Oltean
@ 2022-04-27  6:45       ` Hans Schultz
  1 sibling, 0 replies; 19+ messages in thread
From: Hans Schultz @ 2022-04-27  6:45 UTC (permalink / raw)
  To: Andrew Lunn, Hans Schultz
  Cc: Vladimir Oltean, netdev, Jakub Kicinski, David S. Miller,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

On tis, apr 26, 2022 at 18:14, Andrew Lunn <andrew@lunn.ch> wrote:
>> > @@ -941,23 +965,29 @@ struct dsa_switch_ops {
>> >  	 * Forwarding database
>> >  	 */
>> >  	int	(*port_fdb_add)(struct dsa_switch *ds, int port,
>> > -				const unsigned char *addr, u16 vid);
>> > +				const unsigned char *addr, u16 vid,
>> > +				struct dsa_db db);
>> 
>> Hi! Wouldn't it be better to have a struct that has all the functions
>> parameters in one instead of adding further parameters to these
>> functions?
>> 
>> I am asking because I am also needing to add a parameter to
>> port_fdb_add(), and it would be more future oriented to have a single
>> function parameter as a struct, so that it is easier to add parameters
>> to these functions without havíng to change the prototype of the
>> function every time.
>
> Hi Hans
>
> Please trim the text to only what is relevant when replying. It is
> easy to miss comments when having to Page Down, Page Down, Page down,
> to find comments.
>
>    Andrew

Hi Andrew,

ahh yes, my client collapses those lines, but thanks for letting me
know. I will trim going forward.

Hans

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation
  2022-04-26 23:17       ` Vladimir Oltean
@ 2022-04-27  8:38         ` Hans Schultz
  2022-04-27 10:32           ` Vladimir Oltean
  0 siblings, 1 reply; 19+ messages in thread
From: Hans Schultz @ 2022-04-27  8:38 UTC (permalink / raw)
  To: Vladimir Oltean, Andrew Lunn
  Cc: Hans Schultz, netdev, Jakub Kicinski, David S. Miller,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

On tis, apr 26, 2022 at 23:17, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> On Tue, Apr 26, 2022 at 06:14:23PM +0200, Andrew Lunn wrote:
>> > > @@ -941,23 +965,29 @@ struct dsa_switch_ops {
>> > >  	 * Forwarding database
>> > >  	 */
>> > >  	int	(*port_fdb_add)(struct dsa_switch *ds, int port,
>> > > -				const unsigned char *addr, u16 vid);
>> > > +				const unsigned char *addr, u16 vid,
>> > > +				struct dsa_db db);
>> > 
>> > Hi! Wouldn't it be better to have a struct that has all the functions
>> > parameters in one instead of adding further parameters to these
>> > functions?
>> > 
>> > I am asking because I am also needing to add a parameter to
>> > port_fdb_add(), and it would be more future oriented to have a single
>> > function parameter as a struct, so that it is easier to add parameters
>> > to these functions without havíng to change the prototype of the
>> > function every time.
>> 
>> Hi Hans
>> 
>> Please trim the text to only what is relevant when replying. It is
>> easy to miss comments when having to Page Down, Page Down, Page down,
>> to find comments.
>> 
>>    Andrew
>
> Agreed, had to scroll too much.
>
> Hans, what extra argument do you want to add to port_fdb_add?
> A static/dynamic, I suppose, similar to what exists in port_fdb_dump?

Yes, a static/dynamic bool, or could be called a flag field.

>
> But we surely wouldn't pass _all_ parameters of port_fdb_add through
> some giant struct args_of_port_fdb_add, would we?  Not ds, port, db,
> just what is naturally grouped together as an FDB entry: addr, vid,
> maybe your new static/dynamic thing.
>
> If we group the addr and vid in port_fdb_add into a structure that
> represents an FDB entry, what do we do about those in port_fdb_del?
> Group those as well, maybe for consistency?

I think the 'old' interface that several other functions use should have
one struct... e.g. port, addr and vid. But somehow it would be good to
have something more dynamic. There could be two layer of structs, but
generally i think that for these op functions now in relation to fdb
should only have structs as parameters in a logical way that is
expandable and thus future oriented.

Something else to consider is what do switchcore drivers that don't use
'include/net/dsa.h' do and why?

>
> Same question for port_fdb_dump and its dsa_fdb_dump_cb_t: would you
> change it for uniformity, or would you keep it the way it is to reduce
> the churn? I mean it's a perfectly viable candidate for argument
> grouping, but your stated goal _is_ to reduce churn.

I think port_fdb_dump() is maybe the last one that I would change, but
all those functions where you have added the struct dsa_db would be 
candidates.

>
> But if we add the static/dynamic boolean to this structure, does it make
> sense on deletion? And if it doesn't, why have we changed the prototype
> of port_fdb_del to include it?
>

True a static/dynamic boolean doesn't make much sense on deletion, only
if it came in because it is part of a generic struct.

> Restated: do we want to treat the "static/dynamic" info as a property of
> an FDB entry (i.e. a member of the structure), or as the way in which a
> certain FDB entry can be added to hardware (case in which it is relevant
> only to _add and to _dump)?  After all, an FDB entry for {addr, vid}
> learned statically, and another FDB entry for the same {addr, vid} but
> learned dynamically, are fundamentally the same object.
>

I cannot answer for the workings of all switchcores, but for my sake I
use a debug tool to show the age of a dynamic entry in the ATU, so I
don't think that it has much relevance outside of that.

> And if we don't go with a big struct args_of_port_fdb_add (which would
> be silly if we did), who guarantees that the argument list of port_fdb_add
> won't grow in the future anyway? Like in the example I just gave above,
> where "static/dynamic" doesn't fully appear to group naturally with
> "addr" and "vid", and would probably still be a separate boolean,
> rendering the whole point moot.
>
> And even grouping only those last 2 together is maybe debatable due to
> practical reasons - where do we declare this small structure? We have a
> struct switchdev_notifier_fdb_info with some more stuff that we
> deliberately do not want to expose, and {addr, vid} are all that remain.
>
> Although maybe there are benefits to having a small {addr, vid} structure
> defined somewhere publicly, too, and referenced consistently by driver
> code. Like for example to avoid bad patterns from proliferating.
> Currently we have "const unsigned char *addr, u16 vid", so on a 64 bit
> machine, this is a pointer plus an unsigned short, 10 bytes, padded up
> by the compiler, maybe to 16. But the Ethernet addresses are 6 bytes,
> those are shorter than the pointer itself, so on a 64-bit machine,
> having "unsigned char addr[ETH_ALEN], u16 vid" makes a lot more space,
> saves some real memory.

I see that there is definitions for 64bit mac addresses out there, which
might also be needed to be supported at some point?

>
> Anyway, I'm side tracking. If you want to make changes, propose a
> patch, but come up with a more real argument than "reducing churn"
> (while effectively producing churn).

Unfortunately I don't have the time to make such a patch suggestion for
some time to come as I also have other patch sets coming up, and I
should study a bit what your patch set with the dsa_db is about also, so
maybe I must just add the bool to port_fdb_add() for now.

>
> To give you a counter example, phylink_mac_config() used to have the
> opposite problem, the arguments were all neatly packed into a struct
> phylink_link_state, but as the kerneldocs from include/linux/phylink.h
> put it, not all members of that structure were guaranteed to contain
> valid values. So there were bugs due to people not realizing this, and
> consequently, phylink_mac_link_up() took the opposite approach, of
> explicitly passing all known parameters of the resolved link state as
> individual arguments. Now there are 10 arguments to that function, but
> somehow at least this appears to have worked better, in the sense that
> there isn't an explanatory note saying what's valid and what isn't.

Yes, I can see the danger of it, but something like phylink is also
different as it is more hardware related, which has a slower development
cycle than feature/protocol stuff.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation
  2022-04-27  8:38         ` Hans Schultz
@ 2022-04-27 10:32           ` Vladimir Oltean
  2022-04-27 11:06             ` Hans Schultz
  0 siblings, 1 reply; 19+ messages in thread
From: Vladimir Oltean @ 2022-04-27 10:32 UTC (permalink / raw)
  To: Hans Schultz
  Cc: Andrew Lunn, netdev, Jakub Kicinski, David S. Miller,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

On Wed, Apr 27, 2022 at 10:38:18AM +0200, Hans Schultz wrote:
> > But we surely wouldn't pass _all_ parameters of port_fdb_add through
> > some giant struct args_of_port_fdb_add, would we?  Not ds, port, db,
> > just what is naturally grouped together as an FDB entry: addr, vid,
> > maybe your new static/dynamic thing.
> >
> > If we group the addr and vid in port_fdb_add into a structure that
> > represents an FDB entry, what do we do about those in port_fdb_del?
> > Group those as well, maybe for consistency?
> 
> I think the 'old' interface that several other functions use should have
> one struct... e.g. port, addr and vid. But somehow it would be good to
> have something more dynamic. There could be two layer of structs, but
> generally i think that for these op functions now in relation to fdb
> should only have structs as parameters in a logical way that is
> expandable and thus future oriented.

As a disadvantage to your proposal, such a change would not only modify
the port_fdb_add prototype of these functions once again, but it would
also modify the way in which they *access* their arguments (instead of
accessing "addr" they now need to access "fdb->addr" for example).

You can argue that this would be the change to end all changes, but what
if we then need to add more unrelated arguments to port_fdb_add, like an
"extack". You still need to modify the prototypes for all drivers again.
I think it's a non-goal, you may disagree.

> Something else to consider is what do switchcore drivers that don't use
> 'include/net/dsa.h' do and why?

They tend to copy-paste bad coding patterns from each other. The
SWITCHDEV_FDB_ADD_TO_DEVICE and SWITCHDEV_FDB_DEL_TO_DEVICE handlers are
quite a big mess, but that's a story for another time.

Otherwise, generally speaking, they have access from the atomic notifier
to the struct switchdev_notifier_fdb_info *fdb_info, then they allocate
a work item on a private work queue, copy the stuff from this notifier
object that they find relevant, and use that private structure from the
deferred context.

DSA does basically the same thing, except for the fact that the hardware
access is abstracted behind an indirect call to port_fdb_add().

> > Restated: do we want to treat the "static/dynamic" info as a property of
> > an FDB entry (i.e. a member of the structure), or as the way in which a
> > certain FDB entry can be added to hardware (case in which it is relevant
> > only to _add and to _dump)?  After all, an FDB entry for {addr, vid}
> > learned statically, and another FDB entry for the same {addr, vid} but
> > learned dynamically, are fundamentally the same object.
> 
> I cannot answer for the workings of all switchcores, but for my sake I
> use a debug tool to show the age of a dynamic entry in the ATU, so I
> don't think that it has much relevance outside of that.
> 
> > And if we don't go with a big struct args_of_port_fdb_add (which would
> > be silly if we did), who guarantees that the argument list of port_fdb_add
> > won't grow in the future anyway? Like in the example I just gave above,
> > where "static/dynamic" doesn't fully appear to group naturally with
> > "addr" and "vid", and would probably still be a separate boolean,
> > rendering the whole point moot.
> >
> > And even grouping only those last 2 together is maybe debatable due to
> > practical reasons - where do we declare this small structure? We have a
> > struct switchdev_notifier_fdb_info with some more stuff that we
> > deliberately do not want to expose, and {addr, vid} are all that remain.
> >
> > Although maybe there are benefits to having a small {addr, vid} structure
> > defined somewhere publicly, too, and referenced consistently by driver
> > code. Like for example to avoid bad patterns from proliferating.
> > Currently we have "const unsigned char *addr, u16 vid", so on a 64 bit
> > machine, this is a pointer plus an unsigned short, 10 bytes, padded up
> > by the compiler, maybe to 16. But the Ethernet addresses are 6 bytes,
> > those are shorter than the pointer itself, so on a 64-bit machine,
> > having "unsigned char addr[ETH_ALEN], u16 vid" makes a lot more space,
> > saves some real memory.
> 
> I see that there is definitions for 64bit mac addresses out there, which
> might also be needed to be supported at some point?

I don't know about 64-bit MAC addresses, do you have more information?

> > Anyway, I'm side tracking. If you want to make changes, propose a
> > patch, but come up with a more real argument than "reducing churn"
> > (while effectively producing churn).
> 
> Unfortunately I don't have the time to make such a patch suggestion for
> some time to come as I also have other patch sets coming up, and I
> should study a bit what your patch set with the dsa_db is about also, so
> maybe I must just add the bool to port_fdb_add() for now.

Yeah, I should update the DSA documentation with clear info about that,
it's just the commit messages for now.

> > To give you a counter example, phylink_mac_config() used to have the
> > opposite problem, the arguments were all neatly packed into a struct
> > phylink_link_state, but as the kerneldocs from include/linux/phylink.h
> > put it, not all members of that structure were guaranteed to contain
> > valid values. So there were bugs due to people not realizing this, and
> > consequently, phylink_mac_link_up() took the opposite approach, of
> > explicitly passing all known parameters of the resolved link state as
> > individual arguments. Now there are 10 arguments to that function, but
> > somehow at least this appears to have worked better, in the sense that
> > there isn't an explanatory note saying what's valid and what isn't.
> 
> Yes, I can see the danger of it, but something like phylink is also
> different as it is more hardware related, which has a slower development
> cycle than feature/protocol stuff.

My advice is just add what you need to add. The prototype changes for
port_fdb_add took place in:

2022-02-25 c26933639b54 ("net: dsa: request drivers to perform FDB isolation")
2017-08-06 1b6dd556c304 ("net: dsa: Remove prepare phase for FDB")
2017-08-06 6c2c1dcb185f ("net: dsa: Change DSA slave FDB API to be switchdev independent")
2016-04-06 8497aa618dd6 ("net: dsa: make the FDB add function return void")
2015-10-08 1f36faf26943 ("net: dsa: push prepare phase in port_fdb_add")
2015-08-10 2a778e1b5899 ("net: dsa: change FDB routines prototypes")
2015-08-06 55045ddded0f ("net: dsa: add support for switchdev FDB objects")
2015-03-26 339d82626d22 ("net: dsa: Add basic framework to support ndo_fdb functions")

As you can see, those aren't a lot of changes. I'm happy to see new
development in this area, but there was a long period of stability,
which is likely to continue.

Also, if you study the phylink code, you'll notice that it does't have
"a slower development cycle", quite the contrary, it is even aggressively
converting drivers to make use of new API, and marking unconverted drivers
as deprecated/legacy.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation
  2022-04-27 10:32           ` Vladimir Oltean
@ 2022-04-27 11:06             ` Hans Schultz
  0 siblings, 0 replies; 19+ messages in thread
From: Hans Schultz @ 2022-04-27 11:06 UTC (permalink / raw)
  To: Vladimir Oltean, Hans Schultz
  Cc: Andrew Lunn, netdev, Jakub Kicinski, David S. Miller,
	Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh, UNGLinuxDriver,
	Sean Wang, Landen Chao, DENG Qingfang, Claudiu Manoil,
	Alexandre Belloni, Linus Walleij, Alvin Šipraga,
	George McCollister

On ons, apr 27, 2022 at 10:32, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> On Wed, Apr 27, 2022 at 10:38:18AM +0200, Hans Schultz wrote:
>> 
>> I see that there is definitions for 64bit mac addresses out there, which
>> might also be needed to be supported at some point?
>
> I don't know about 64-bit MAC addresses, do you have more information?
>

I have only seen that there is a section about it in rfc7043:
https://www.rfc-editor.org/rfc/rfc7043

and some in rfc7042 also.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-04-27 11:06 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25  9:22 [PATCH v2 net-next 00/10] DSA FDB isolation Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 01/10] net: dsa: tag_8021q: replace the SVL bridging with VLAN-unaware IVL bridging Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 02/10] net: dsa: tag_8021q: add support for imprecise RX based on the VBID Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 03/10] docs: net: dsa: sja1105: document limitations of tc-flower rule VLAN awareness Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 04/10] net: dsa: felix: delete workarounds present due to SVL tag_8021q bridging Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 05/10] net: dsa: tag_8021q: merge RX and TX VLANs Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 06/10] net: dsa: tag_8021q: rename dsa_8021q_bridge_tx_fwd_offload_vid Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 07/10] net: dsa: request drivers to perform FDB isolation Vladimir Oltean
2022-04-26 15:01   ` Hans Schultz
2022-04-26 16:14     ` Andrew Lunn
2022-04-26 23:17       ` Vladimir Oltean
2022-04-27  8:38         ` Hans Schultz
2022-04-27 10:32           ` Vladimir Oltean
2022-04-27 11:06             ` Hans Schultz
2022-04-27  6:45       ` Hans Schultz
2022-02-25  9:22 ` [PATCH v2 net-next 08/10] net: dsa: pass extack to .port_bridge_join driver methods Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 09/10] net: dsa: sja1105: enforce FDB isolation Vladimir Oltean
2022-02-25  9:22 ` [PATCH v2 net-next 10/10] net: mscc: ocelot: enforce FDB isolation when VLAN-unaware Vladimir Oltean
2022-02-27 11:10 ` [PATCH v2 net-next 00/10] DSA FDB isolation patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.