All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API
@ 2021-10-26 16:26 Vladimir Oltean
  2021-10-26 16:26 ` [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based Vladimir Oltean
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-10-26 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang, Alvin Šipraga

This change set replaces struct net_device *dp->bridge_dev with a
struct dsa_bridge *dp->bridge that contains some extra information about
that bridge, like a unique number kept by DSA.

Up until now we computed that number only with the bridge TX forwarding
offload feature, but it will be needed for other features too, like for
isolation of FDB entries belonging to different bridges. Hardware
implementations vary, but one common pattern seems to be the presence of
a FID field which can be associated with that bridge number kept by DSA.
The idea was outlined here:
https://patchwork.kernel.org/project/netdevbpf/patch/20210818120150.892647-16-vladimir.oltean@nxp.com/
(the difference being that with this new proposal, drivers would not
need to call dsa_bridge_num_find, instead the bridge_num would be part
of the struct dsa_bridge :: num passed as argument).

No functional change intended.

Vladimir Oltean (6):
  net: dsa: make dp->bridge_num one-based
  net: dsa: assign a bridge number even without TX forwarding offload
  net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers
  net: dsa: rename dsa_port_offloads_bridge to
    dsa_port_offloads_bridge_dev
  net: dsa: keep the bridge_dev and bridge_num as part of the same
    structure
  net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload

 drivers/net/dsa/b53/b53_common.c       |   9 +-
 drivers/net/dsa/b53/b53_priv.h         |   5 +-
 drivers/net/dsa/dsa_loop.c             |   9 +-
 drivers/net/dsa/hirschmann/hellcreek.c |   5 +-
 drivers/net/dsa/lan9303-core.c         |   7 +-
 drivers/net/dsa/lantiq_gswip.c         |  25 +++--
 drivers/net/dsa/microchip/ksz_common.c |   5 +-
 drivers/net/dsa/microchip/ksz_common.h |   4 +-
 drivers/net/dsa/mt7530.c               |  18 +--
 drivers/net/dsa/mv88e6xxx/chip.c       | 145 ++++++++++++-------------
 drivers/net/dsa/ocelot/felix.c         |   8 +-
 drivers/net/dsa/qca8k.c                |  13 ++-
 drivers/net/dsa/rtl8366rb.c            |   9 +-
 drivers/net/dsa/sja1105/sja1105_main.c |  40 +++++--
 drivers/net/dsa/xrs700x/xrs700x.c      |  10 +-
 include/linux/dsa/8021q.h              |   9 +-
 include/net/dsa.h                      | 102 +++++++++++++----
 net/dsa/dsa2.c                         |  57 ++++++----
 net/dsa/dsa_priv.h                     |  59 ++--------
 net/dsa/port.c                         | 123 +++++++++++----------
 net/dsa/slave.c                        |  34 +++---
 net/dsa/switch.c                       |  20 ++--
 net/dsa/tag_8021q.c                    |  20 ++--
 net/dsa/tag_dsa.c                      |   5 +-
 net/dsa/tag_ocelot.c                   |   2 +-
 net/dsa/tag_sja1105.c                  |  11 +-
 26 files changed, 414 insertions(+), 340 deletions(-)

-- 
2.25.1


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

* [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based
  2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
@ 2021-10-26 16:26 ` Vladimir Oltean
  2021-11-11 12:24   ` Alvin Šipraga
  2021-10-26 16:26 ` [RFC PATCH net-next 2/6] net: dsa: assign a bridge number even without TX forwarding offload Vladimir Oltean
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2021-10-26 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang, Alvin Šipraga

I have seen too many bugs already due to the fact that we must encode an
invalid dp->bridge_num as a negative value, because the natural tendency
is to check that invalid value using (!dp->bridge_num). Latest example
can be seen in commit 1bec0f05062c ("net: dsa: fix bridge_num not
getting cleared after ports leaving the bridge").

Convert the existing users to assume that dp->bridge_num == 0 is the
encoding for invalid, and valid bridge numbers start from 1.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 12 ++++++------
 include/linux/dsa/8021q.h        |  6 +++---
 include/net/dsa.h                |  6 +++---
 net/dsa/dsa2.c                   | 24 ++++++++++++------------
 net/dsa/dsa_priv.h               |  5 +++--
 net/dsa/port.c                   | 11 ++++++-----
 net/dsa/tag_8021q.c              | 12 +++++++-----
 net/dsa/tag_dsa.c                |  2 +-
 8 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 14c678a9e41b..0d0ccb7f8ccd 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1247,10 +1247,10 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
 	/* dev is a virtual bridge */
 	} else {
 		list_for_each_entry(dp, &dst->ports, list) {
-			if (dp->bridge_num < 0)
+			if (!dp->bridge_num)
 				continue;
 
-			if (dp->bridge_num + 1 + dst->last_switch != dev)
+			if (dp->bridge_num + dst->last_switch != dev)
 				continue;
 
 			br = dp->bridge_dev;
@@ -2524,9 +2524,9 @@ static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
  * physical switches, so start from beyond that range.
  */
 static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
-					       int bridge_num)
+					       unsigned int bridge_num)
 {
-	u8 dev = bridge_num + ds->dst->last_switch + 1;
+	u8 dev = bridge_num + ds->dst->last_switch;
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
 
@@ -2539,14 +2539,14 @@ static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
 
 static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
 					   struct net_device *br,
-					   int bridge_num)
+					   unsigned int bridge_num)
 {
 	return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
 }
 
 static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
 					      struct net_device *br,
-					      int bridge_num)
+					      unsigned int bridge_num)
 {
 	int err;
 
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index 254b165f2b44..0af4371fbebb 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -38,13 +38,13 @@ void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
 
 int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
 					struct net_device *br,
-					int bridge_num);
+					unsigned int bridge_num);
 
 void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
 					   struct net_device *br,
-					   int bridge_num);
+					   unsigned int bridge_num);
 
-u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num);
+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/include/net/dsa.h b/include/net/dsa.h
index badd214f7470..56a90f05df49 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -257,7 +257,7 @@ struct dsa_port {
 	bool			learning;
 	u8			stp_state;
 	struct net_device	*bridge_dev;
-	int			bridge_num;
+	unsigned int		bridge_num;
 	struct devlink_port	devlink_port;
 	bool			devlink_port_setup;
 	struct phylink		*pl;
@@ -752,11 +752,11 @@ struct dsa_switch_ops {
 	/* Called right after .port_bridge_join() */
 	int	(*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
 					      struct net_device *bridge,
-					      int bridge_num);
+					      unsigned int bridge_num);
 	/* Called right before .port_bridge_leave() */
 	void	(*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
 						struct net_device *bridge,
-						int bridge_num);
+						unsigned int bridge_num);
 	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
 				      u8 state);
 	void	(*port_fast_age)(struct dsa_switch *ds, int port);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 826957b6442b..9606e56710a5 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -141,23 +141,23 @@ static int dsa_bridge_num_find(const struct net_device *bridge_dev)
 	 */
 	list_for_each_entry(dst, &dsa_tree_list, list)
 		list_for_each_entry(dp, &dst->ports, list)
-			if (dp->bridge_dev == bridge_dev &&
-			    dp->bridge_num != -1)
+			if (dp->bridge_dev == bridge_dev && dp->bridge_num)
 				return dp->bridge_num;
 
-	return -1;
+	return 0;
 }
 
-int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
+unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
 {
-	int bridge_num = dsa_bridge_num_find(bridge_dev);
+	unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
 
-	if (bridge_num < 0) {
+	if (!bridge_num) {
 		/* First port that offloads TX forwarding for this bridge */
-		bridge_num = find_first_zero_bit(&dsa_fwd_offloading_bridges,
-						 DSA_MAX_NUM_OFFLOADING_BRIDGES);
+		bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
+						DSA_MAX_NUM_OFFLOADING_BRIDGES,
+						1);
 		if (bridge_num >= max)
-			return -1;
+			return 0;
 
 		set_bit(bridge_num, &dsa_fwd_offloading_bridges);
 	}
@@ -165,12 +165,13 @@ int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
 	return bridge_num;
 }
 
-void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num)
+void dsa_bridge_num_put(const struct net_device *bridge_dev,
+			unsigned 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 (dsa_bridge_num_find(bridge_dev) < 0)
+	if (!dsa_bridge_num_find(bridge_dev))
 		clear_bit(bridge_num, &dsa_fwd_offloading_bridges);
 }
 
@@ -1184,7 +1185,6 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
 
 	dp->ds = ds;
 	dp->index = index;
-	dp->bridge_num = -1;
 
 	INIT_LIST_HEAD(&dp->list);
 	list_add_tail(&dp->list, &dst->ports);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index a5c9bc7b66c6..2a64c41813bf 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -546,8 +546,9 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
 			      struct net_device *master,
 			      const struct dsa_device_ops *tag_ops,
 			      const struct dsa_device_ops *old_tag_ops);
-int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
-void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num);
+unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
+void dsa_bridge_num_put(const struct net_device *bridge_dev,
+			unsigned int bridge_num);
 
 /* tag_8021q.c */
 int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index c0e630f7f0bd..1772817a1214 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -273,14 +273,14 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
 static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
 					     struct net_device *bridge_dev)
 {
-	int bridge_num = dp->bridge_num;
+	unsigned int bridge_num = dp->bridge_num;
 	struct dsa_switch *ds = dp->ds;
 
 	/* No bridge TX forwarding offload => do nothing */
-	if (!ds->ops->port_bridge_tx_fwd_unoffload || dp->bridge_num == -1)
+	if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num)
 		return;
 
-	dp->bridge_num = -1;
+	dp->bridge_num = 0;
 
 	dsa_bridge_num_put(bridge_dev, bridge_num);
 
@@ -295,14 +295,15 @@ static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
 					   struct net_device *bridge_dev)
 {
 	struct dsa_switch *ds = dp->ds;
-	int bridge_num, err;
+	unsigned int bridge_num;
+	int err;
 
 	if (!ds->ops->port_bridge_tx_fwd_offload)
 		return false;
 
 	bridge_num = dsa_bridge_num_get(bridge_dev,
 					ds->num_fwd_offloading_bridges);
-	if (bridge_num < 0)
+	if (!bridge_num)
 		return false;
 
 	dp->bridge_num = bridge_num;
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 72cac2c0af7b..df59f16436a5 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -67,10 +67,12 @@
 #define DSA_8021Q_PORT(x)		(((x) << DSA_8021Q_PORT_SHIFT) & \
 						 DSA_8021Q_PORT_MASK)
 
-u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num)
+u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num)
 {
-	/* The VBID value of 0 is reserved for precise TX */
-	return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num + 1);
+	/* 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);
 }
 EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid);
 
@@ -409,7 +411,7 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
 
 int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
 					struct net_device *br,
-					int bridge_num)
+					unsigned int bridge_num)
 {
 	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
 
@@ -420,7 +422,7 @@ EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
 
 void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
 					   struct net_device *br,
-					   int bridge_num)
+					   unsigned int bridge_num)
 {
 	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
 
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index b3da4b2ea11c..a7d70ae7cc97 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -140,7 +140,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
 		 * packets on behalf of a virtual switch device with an index
 		 * past the physical switches.
 		 */
-		tag_dev = dst->last_switch + 1 + dp->bridge_num;
+		tag_dev = dst->last_switch + dp->bridge_num;
 		tag_port = 0;
 	} else {
 		cmd = DSA_CMD_FROM_CPU;
-- 
2.25.1


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

* [RFC PATCH net-next 2/6] net: dsa: assign a bridge number even without TX forwarding offload
  2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
  2021-10-26 16:26 ` [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based Vladimir Oltean
@ 2021-10-26 16:26 ` Vladimir Oltean
  2021-11-11 12:27   ` Alvin Šipraga
  2021-10-26 16:26 ` [RFC PATCH net-next 3/6] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers Vladimir Oltean
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2021-10-26 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang, Alvin Šipraga

The service where DSA assigns a unique bridge number for each forwarding
domain is useful even for drivers which do not implement the TX
forwarding offload feature.

For example, drivers might use the dp->bridge_num for FDB isolation.

So rename ds->num_fwd_offloading_bridges to ds->max_num_bridges, and
calculate a unique bridge_num for all drivers that set this value.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c       |  4 +-
 drivers/net/dsa/sja1105/sja1105_main.c |  2 +-
 include/net/dsa.h                      | 10 ++--
 net/dsa/port.c                         | 81 +++++++++++++++++---------
 4 files changed, 63 insertions(+), 34 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 0d0ccb7f8ccd..ae09cbc2f42f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3183,8 +3183,8 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	 * time.
 	 */
 	if (mv88e6xxx_has_pvt(chip))
-		ds->num_fwd_offloading_bridges = MV88E6XXX_MAX_PVT_SWITCHES -
-						 ds->dst->last_switch - 1;
+		ds->max_num_bridges = MV88E6XXX_MAX_PVT_SWITCHES -
+				      ds->dst->last_switch - 1;
 
 	mv88e6xxx_reg_lock(chip);
 
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index c343effe2e96..355b56cf94d8 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3139,7 +3139,7 @@ static int sja1105_setup(struct dsa_switch *ds)
 	ds->vlan_filtering_is_global = true;
 	ds->untag_bridge_pvid = true;
 	/* tag_8021q has 3 bits for the VBID, and the value 0 is reserved */
-	ds->num_fwd_offloading_bridges = 7;
+	ds->max_num_bridges = 7;
 
 	/* Advertise the 8 egress queues */
 	ds->num_tx_queues = SJA1105_NUM_TC;
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 56a90f05df49..970bc89a0062 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -413,12 +413,12 @@ struct dsa_switch {
 	 */
 	unsigned int		num_lag_ids;
 
-	/* Drivers that support bridge forwarding offload should set this to
-	 * the maximum number of bridges spanning the same switch tree (or all
-	 * trees, in the case of cross-tree bridging support) that can be
-	 * offloaded.
+	/* Drivers that support bridge forwarding offload or FDB isolation
+	 * should set this to the maximum number of bridges spanning the same
+	 * switch tree (or all trees, in the case of cross-tree bridging
+	 * support) that can be offloaded.
 	 */
-	unsigned int		num_fwd_offloading_bridges;
+	unsigned int		max_num_bridges;
 
 	size_t num_ports;
 };
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 1772817a1214..b7867746af15 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -271,19 +271,15 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
 }
 
 static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
-					     struct net_device *bridge_dev)
+					     struct net_device *bridge_dev,
+					     unsigned int bridge_num)
 {
-	unsigned int bridge_num = dp->bridge_num;
 	struct dsa_switch *ds = dp->ds;
 
 	/* No bridge TX forwarding offload => do nothing */
-	if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num)
+	if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge_num)
 		return;
 
-	dp->bridge_num = 0;
-
-	dsa_bridge_num_put(bridge_dev, bridge_num);
-
 	/* Notify the chips only once the offload has been deactivated, so
 	 * that they can update their configuration accordingly.
 	 */
@@ -292,31 +288,60 @@ static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
 }
 
 static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
-					   struct net_device *bridge_dev)
+					   struct net_device *bridge_dev,
+					   unsigned int bridge_num)
 {
 	struct dsa_switch *ds = dp->ds;
-	unsigned int bridge_num;
 	int err;
 
-	if (!ds->ops->port_bridge_tx_fwd_offload)
-		return false;
-
-	bridge_num = dsa_bridge_num_get(bridge_dev,
-					ds->num_fwd_offloading_bridges);
-	if (!bridge_num)
+	/* FDB isolation is required for TX forwarding offload */
+	if (!ds->ops->port_bridge_tx_fwd_offload || !bridge_num)
 		return false;
 
-	dp->bridge_num = bridge_num;
-
 	/* Notify the driver */
 	err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge_dev,
 						  bridge_num);
-	if (err) {
-		dsa_port_bridge_tx_fwd_unoffload(dp, bridge_dev);
-		return false;
+
+	return err ? false : true;
+}
+
+static int dsa_port_bridge_create(struct dsa_port *dp,
+				  struct net_device *br,
+				  struct netlink_ext_ack *extack)
+{
+	struct dsa_switch *ds = dp->ds;
+	unsigned int bridge_num;
+
+	dp->bridge_dev = br;
+
+	if (!ds->max_num_bridges)
+		return 0;
+
+	bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges);
+	if (!bridge_num) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Range of offloadable bridges exceeded");
+		return -EOPNOTSUPP;
 	}
 
-	return true;
+	dp->bridge_num = bridge_num;
+
+	return 0;
+}
+
+static void dsa_port_bridge_destroy(struct dsa_port *dp,
+				    const struct net_device *br)
+{
+	struct dsa_switch *ds = dp->ds;
+
+	dp->bridge_dev = NULL;
+
+	if (ds->max_num_bridges) {
+		int bridge_num = dp->bridge_num;
+
+		dp->bridge_num = 0;
+		dsa_bridge_num_put(br, bridge_num);
+	}
 }
 
 int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
@@ -336,7 +361,9 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 	/* Here the interface is already bridged. Reflect the current
 	 * configuration so that drivers can program their chips accordingly.
 	 */
-	dp->bridge_dev = br;
+	err = dsa_port_bridge_create(dp, br, extack);
+	if (err)
+		return err;
 
 	brport_dev = dsa_port_to_bridge_port(dp);
 
@@ -344,7 +371,8 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 	if (err)
 		goto out_rollback;
 
-	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br);
+	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
+							dp->bridge_num);
 
 	err = switchdev_bridge_port_offload(brport_dev, dev, dp,
 					    &dsa_slave_switchdev_notifier,
@@ -366,7 +394,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 out_rollback_unbridge:
 	dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
 out_rollback:
-	dp->bridge_dev = NULL;
+	dsa_port_bridge_destroy(dp, br);
 	return err;
 }
 
@@ -393,14 +421,15 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 		.port = dp->index,
 		.br = br,
 	};
+	int bridge_num = dp->bridge_num;
 	int err;
 
 	/* Here the port is already unbridged. Reflect the current configuration
 	 * so that drivers can program their chips accordingly.
 	 */
-	dp->bridge_dev = NULL;
+	dsa_port_bridge_destroy(dp, br);
 
-	dsa_port_bridge_tx_fwd_unoffload(dp, br);
+	dsa_port_bridge_tx_fwd_unoffload(dp, br, bridge_num);
 
 	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
 	if (err)
-- 
2.25.1


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

* [RFC PATCH net-next 3/6] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers
  2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
  2021-10-26 16:26 ` [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based Vladimir Oltean
  2021-10-26 16:26 ` [RFC PATCH net-next 2/6] net: dsa: assign a bridge number even without TX forwarding offload Vladimir Oltean
@ 2021-10-26 16:26 ` Vladimir Oltean
  2021-10-26 16:26 ` [RFC PATCH net-next 4/6] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev Vladimir Oltean
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-10-26 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang, Alvin Šipraga

The location of the bridge device pointer and number is going to change.
It is not going to be kept individually per port, but in a common
structure allocated dynamically and which will have lockdep validation.

Create helpers to access these elements so that we have a migration path
to the new organization.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c       |  4 +-
 drivers/net/dsa/lan9303-core.c         |  2 +-
 drivers/net/dsa/lantiq_gswip.c         | 10 ++---
 drivers/net/dsa/mt7530.c               | 14 ++++---
 drivers/net/dsa/mv88e6xxx/chip.c       | 56 ++++++++++++++------------
 drivers/net/dsa/qca8k.c                |  4 +-
 drivers/net/dsa/rtl8366rb.c            |  4 +-
 drivers/net/dsa/sja1105/sja1105_main.c | 10 +++--
 drivers/net/dsa/xrs700x/xrs700x.c      |  4 +-
 include/net/dsa.h                      | 15 +++++++
 net/dsa/dsa_priv.h                     |  4 +-
 net/dsa/port.c                         | 35 ++++++++--------
 net/dsa/slave.c                        | 14 ++++---
 net/dsa/switch.c                       |  6 +--
 net/dsa/tag_8021q.c                    |  2 +-
 net/dsa/tag_dsa.c                      |  5 ++-
 net/dsa/tag_ocelot.c                   |  2 +-
 net/dsa/tag_sja1105.c                  | 11 +++--
 18 files changed, 116 insertions(+), 86 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index af4761968733..d5e78f51f42d 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1887,7 +1887,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
 	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
 
 	b53_for_each_port(dev, i) {
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
 			continue;
 
 		/* Add this local port to the remote port VLAN control
@@ -1923,7 +1923,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
 
 	b53_for_each_port(dev, i) {
 		/* Don't touch the remaining ports */
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
 			continue;
 
 		b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), &reg);
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 89f920289ae2..1c2bdcde6979 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1108,7 +1108,7 @@ static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
 	struct lan9303 *chip = ds->priv;
 
 	dev_dbg(chip->dev, "%s(port %d)\n", __func__, port);
-	if (dsa_to_port(ds, 1)->bridge_dev == dsa_to_port(ds, 2)->bridge_dev) {
+	if (dsa_port_bridge_same(dsa_to_port(ds, 1), dsa_to_port(ds, 2))) {
 		lan9303_bridge_ports(chip);
 		chip->is_bridged = true;  /* unleash stp_state_set() */
 	}
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 7056d98d8177..c60b1eef3545 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -759,7 +759,7 @@ static int gswip_port_vlan_filtering(struct dsa_switch *ds, int port,
 				     bool vlan_filtering,
 				     struct netlink_ext_ack *extack)
 {
-	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
+	struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
 	struct gswip_priv *priv = ds->priv;
 
 	/* Do not allow changing the VLAN filtering options while in bridge */
@@ -1183,8 +1183,8 @@ static int gswip_port_vlan_prepare(struct dsa_switch *ds, int port,
 				   const struct switchdev_obj_port_vlan *vlan,
 				   struct netlink_ext_ack *extack)
 {
+	struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
 	struct gswip_priv *priv = ds->priv;
-	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
 	unsigned int max_ports = priv->hw_info->max_ports;
 	int pos = max_ports;
 	int i, idx = -1;
@@ -1229,8 +1229,8 @@ static int gswip_port_vlan_add(struct dsa_switch *ds, int port,
 			       const struct switchdev_obj_port_vlan *vlan,
 			       struct netlink_ext_ack *extack)
 {
+	struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
 	struct gswip_priv *priv = ds->priv;
-	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 	int err;
@@ -1254,8 +1254,8 @@ static int gswip_port_vlan_add(struct dsa_switch *ds, int port,
 static int gswip_port_vlan_del(struct dsa_switch *ds, int port,
 			       const struct switchdev_obj_port_vlan *vlan)
 {
+	struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
 	struct gswip_priv *priv = ds->priv;
-	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 
 	/* We have to receive all packets on the CPU port and should not
@@ -1340,8 +1340,8 @@ static void gswip_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
 static int gswip_port_fdb(struct dsa_switch *ds, int port,
 			  const unsigned char *addr, u16 vid, bool add)
 {
+	struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
 	struct gswip_priv *priv = ds->priv;
-	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
 	struct gswip_pce_table_entry mac_bridge = {0,};
 	unsigned int cpu_port = priv->hw_info->cpu_port;
 	int fid = -1;
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 9890672a206d..79bde263b06f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1195,12 +1195,14 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
 	mutex_lock(&priv->reg_mutex);
 
 	for (i = 0; i < MT7530_NUM_PORTS; i++) {
+		struct dsa_port *other_dp = dsa_to_port(ds, port);
+
 		/* Add this port to the port matrix of the other ports in the
 		 * same bridge. If the port is disabled, port matrix is kept
 		 * and not being setup until the port becomes enabled.
 		 */
-		if (dsa_is_user_port(ds, i) && i != port) {
-			if (dsa_to_port(ds, i)->bridge_dev != bridge)
+		if (dsa_port_is_user(other_dp) && i != port) {
+			if (dsa_port_bridge_dev_get(other_dp) != bridge)
 				continue;
 			if (priv->ports[i].enable)
 				mt7530_set(priv, MT7530_PCR_P(i),
@@ -1236,7 +1238,7 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
 	/* This is called after .port_bridge_leave when leaving a VLAN-aware
 	 * bridge. Don't set standalone ports to fallback mode.
 	 */
-	if (dsa_to_port(ds, port)->bridge_dev)
+	if (dsa_port_bridge_dev_get(dsa_to_port(ds, port)))
 		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
 			   MT7530_PORT_FALLBACK_MODE);
 
@@ -1307,12 +1309,14 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 	mutex_lock(&priv->reg_mutex);
 
 	for (i = 0; i < MT7530_NUM_PORTS; i++) {
+		struct dsa_port *other_dp = dsa_to_port(ds, i);
+
 		/* Remove this port from the port matrix of the other ports
 		 * in the same bridge. If the port is disabled, port matrix
 		 * is kept and not being setup until the port becomes enabled.
 		 */
-		if (dsa_is_user_port(ds, i) && i != port) {
-			if (dsa_to_port(ds, i)->bridge_dev != bridge)
+		if (dsa_port_is_user(other_dp) && i != port) {
+			if (dsa_port_bridge_dev_get(other_dp) != bridge)
 				continue;
 			if (priv->ports[i].enable)
 				mt7530_clear(priv, MT7530_PCR_P(i),
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index ae09cbc2f42f..b76ce4423ea8 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1225,8 +1225,7 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
 {
 	struct dsa_switch *ds = chip->ds;
 	struct dsa_switch_tree *dst = ds->dst;
-	struct net_device *br;
-	struct dsa_port *dp;
+	struct dsa_port *dp, *other_dp;
 	bool found = false;
 	u16 pvlan;
 
@@ -1235,11 +1234,9 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
 		list_for_each_entry(dp, &dst->ports, list) {
 			if (dp->ds->index == dev && dp->index == port) {
 				/* dp might be a DSA link or a user port, so it
-				 * might or might not have a bridge_dev
-				 * pointer. Use the "found" variable for both
-				 * cases.
+				 * might or might not have a bridge.
+				 * Use the "found" variable for both cases.
 				 */
-				br = dp->bridge_dev;
 				found = true;
 				break;
 			}
@@ -1247,13 +1244,14 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
 	/* dev is a virtual bridge */
 	} else {
 		list_for_each_entry(dp, &dst->ports, list) {
-			if (!dp->bridge_num)
+			unsigned int bridge_num = dsa_port_bridge_num_get(dp);
+
+			if (!bridge_num)
 				continue;
 
-			if (dp->bridge_num + dst->last_switch != dev)
+			if (bridge_num + dst->last_switch != dev)
 				continue;
 
-			br = dp->bridge_dev;
 			found = true;
 			break;
 		}
@@ -1272,12 +1270,12 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
 	/* Frames from user ports can egress any local DSA links and CPU ports,
 	 * as well as any local member of their bridge group.
 	 */
-	list_for_each_entry(dp, &dst->ports, list)
-		if (dp->ds == ds &&
-		    (dp->type == DSA_PORT_TYPE_CPU ||
-		     dp->type == DSA_PORT_TYPE_DSA ||
-		     (br && dp->bridge_dev == br)))
-			pvlan |= BIT(dp->index);
+	list_for_each_entry(other_dp, &dst->ports, list)
+		if (other_dp->ds == ds &&
+		    (other_dp->type == DSA_PORT_TYPE_CPU ||
+		     other_dp->type == DSA_PORT_TYPE_DSA ||
+		     dsa_port_bridge_same(dp, other_dp)))
+			pvlan |= BIT(other_dp->index);
 
 	return pvlan;
 }
@@ -1644,8 +1642,10 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
 static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 					u16 vid)
 {
+	struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
 	struct mv88e6xxx_chip *chip = ds->priv;
 	struct mv88e6xxx_vtu_entry vlan;
+	struct net_device *other_br;
 	int i, err;
 
 	/* DSA and CPU ports have to be members of multiple vlans */
@@ -1659,27 +1659,30 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 	if (!vlan.valid)
 		return 0;
 
-	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
-		if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
+	list_for_each_entry(other_dp, &ds->dst->ports, list) {
+		if (other_dp->ds != ds)
 			continue;
 
-		if (!dsa_to_port(ds, i)->slave)
+		if (dsa_port_is_dsa(other_dp) || dsa_port_is_cpu(other_dp))
+			continue;
+
+		if (!other_dp->slave)
 			continue;
 
 		if (vlan.member[i] ==
 		    MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
 			continue;
 
-		if (dsa_to_port(ds, i)->bridge_dev ==
-		    dsa_to_port(ds, port)->bridge_dev)
+		other_br = dsa_port_bridge_dev_get(other_dp);
+
+		if (dsa_port_bridge_same(dp, other_dp))
 			break; /* same bridge, check next VLAN */
 
-		if (!dsa_to_port(ds, i)->bridge_dev)
+		if (!other_br)
 			continue;
 
 		dev_err(ds->dev, "p%d: hw VLAN %d already used by port %d in %s\n",
-			port, vlan.vid, i,
-			netdev_name(dsa_to_port(ds, i)->bridge_dev));
+			port, vlan.vid, i, netdev_name(other_br));
 		return -EOPNOTSUPP;
 	}
 
@@ -1689,13 +1692,14 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 static int mv88e6xxx_port_commit_pvid(struct mv88e6xxx_chip *chip, int port)
 {
 	struct dsa_port *dp = dsa_to_port(chip->ds, port);
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	struct mv88e6xxx_port *p = &chip->ports[port];
 	u16 pvid = MV88E6XXX_VID_STANDALONE;
 	bool drop_untagged = false;
 	int err;
 
-	if (dp->bridge_dev) {
-		if (br_vlan_enabled(dp->bridge_dev)) {
+	if (br) {
+		if (br_vlan_enabled(br)) {
 			pvid = p->bridge_pvid.vid;
 			drop_untagged = !p->bridge_pvid.valid;
 		} else {
@@ -2421,7 +2425,7 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
 	int err;
 
 	list_for_each_entry(dp, &dst->ports, list) {
-		if (dp->bridge_dev == br) {
+		if (dsa_port_bridge_dev_get(dp) == br) {
 			if (dp->ds == ds) {
 				/* This is a local bridge group member,
 				 * remap its Port VLAN Map.
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index ea7f12778922..efe20db287a5 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1741,7 +1741,7 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
 		if (dsa_is_cpu_port(ds, i))
 			continue;
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
 			continue;
 		/* Add this port to the portvlan mask of the other ports
 		 * in the bridge
@@ -1773,7 +1773,7 @@ qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
 		if (dsa_is_cpu_port(ds, i))
 			continue;
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
 			continue;
 		/* Remove this port to the portvlan mask of the other ports
 		 * in the bridge
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index 03deacd83e61..b6f277a04989 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -1198,7 +1198,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
 		if (i == port)
 			continue;
 		/* Not on this bridge */
-		if (dsa_to_port(ds, i)->bridge_dev != bridge)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
 			continue;
 		/* Join this port to each other port on the bridge */
 		ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
@@ -1230,7 +1230,7 @@ rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
 		if (i == port)
 			continue;
 		/* Not on this bridge */
-		if (dsa_to_port(ds, i)->bridge_dev != bridge)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
 			continue;
 		/* Remove this port from any other port on the bridge */
 		ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 355b56cf94d8..5e03eda4c16f 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -118,13 +118,14 @@ static int sja1105_pvid_apply(struct sja1105_private *priv, int port, u16 pvid)
 static int sja1105_commit_pvid(struct dsa_switch *ds, int port)
 {
 	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	struct sja1105_private *priv = ds->priv;
 	struct sja1105_vlan_lookup_entry *vlan;
 	bool drop_untagged = false;
 	int match, rc;
 	u16 pvid;
 
-	if (dp->bridge_dev && br_vlan_enabled(dp->bridge_dev))
+	if (br && br_vlan_enabled(br))
 		pvid = priv->bridge_pvid[port];
 	else
 		pvid = priv->tag_8021q_pvid[port];
@@ -2004,7 +2005,7 @@ static int sja1105_bridge_member(struct dsa_switch *ds, int port,
 		 */
 		if (i == port)
 			continue;
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
 			continue;
 		sja1105_port_allow_traffic(l2_fwd, i, port, member);
 		sja1105_port_allow_traffic(l2_fwd, port, i, member);
@@ -2587,8 +2588,9 @@ static int sja1105_prechangeupper(struct dsa_switch *ds, int port,
 
 	if (netif_is_bridge_master(upper)) {
 		list_for_each_entry(dp, &dst->ports, list) {
-			if (dp->bridge_dev && dp->bridge_dev != upper &&
-			    br_vlan_enabled(dp->bridge_dev)) {
+			struct net_device *br = dsa_port_bridge_dev_get(dp);
+
+			if (br && br != upper && br_vlan_enabled(br)) {
 				NL_SET_ERR_MSG_MOD(extack,
 						   "Only one VLAN-aware bridge is supported");
 				return -EBUSY;
diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index 910fcb3b252b..7c2b6c32242d 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -513,14 +513,14 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
 
 		cpu_mask |= BIT(i);
 
-		if (dsa_to_port(ds, i)->bridge_dev == bridge)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) == bridge)
 			continue;
 
 		mask |= BIT(i);
 	}
 
 	for (i = 0; i < ds->num_ports; i++) {
-		if (dsa_to_port(ds, i)->bridge_dev != bridge)
+		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
 			continue;
 
 		/* 1 = Disable forwarding to the port */
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 970bc89a0062..81fd1821a0aa 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -599,6 +599,21 @@ struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
 	return dp->slave;
 }
 
+static inline struct net_device *dsa_port_bridge_dev_get(struct dsa_port *dp)
+{
+	return dp->bridge_dev;
+}
+
+static inline unsigned int dsa_port_bridge_num_get(struct dsa_port *dp)
+{
+	return dp->bridge_num;
+}
+
+static inline bool dsa_port_bridge_same(struct dsa_port *a, struct dsa_port *b)
+{
+	return dsa_port_bridge_dev_get(a) == dsa_port_bridge_dev_get(b);
+}
+
 typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
 			      bool is_static, void *data);
 struct dsa_switch_ops {
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2a64c41813bf..62b719929ef4 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -278,7 +278,7 @@ static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
 	/* DSA ports connected to a bridge, and event was emitted
 	 * for the bridge.
 	 */
-	return dp->bridge_dev == bridge_dev;
+	return dsa_port_bridge_dev_get(dp) == bridge_dev;
 }
 
 /* Returns true if any port of this tree offloads the given net_device */
@@ -345,7 +345,7 @@ dsa_slave_to_master(const struct net_device *dev)
 static inline struct sk_buff *dsa_untag_bridge_pvid(struct sk_buff *skb)
 {
 	struct dsa_port *dp = dsa_slave_to_port(skb->dev);
-	struct net_device *br = dp->bridge_dev;
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	struct net_device *dev = skb->dev;
 	struct net_device *upper_dev;
 	u16 vid, pvid, proto;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index b7867746af15..aaa978ee165e 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -221,7 +221,7 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
 					 struct netlink_ext_ack *extack)
 {
 	struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
-	struct net_device *br = dp->bridge_dev;
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	int err;
 
 	err = dsa_port_inherit_brport_flags(dp, extack);
@@ -372,7 +372,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 		goto out_rollback;
 
 	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
-							dp->bridge_num);
+							dsa_port_bridge_num_get(dp));
 
 	err = switchdev_bridge_port_offload(brport_dev, dev, dp,
 					    &dsa_slave_switchdev_notifier,
@@ -415,13 +415,13 @@ void dsa_port_pre_bridge_leave(struct dsa_port *dp, struct net_device *br)
 
 void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 {
+	unsigned int bridge_num = dsa_port_bridge_num_get(dp);
 	struct dsa_notifier_bridge_info info = {
 		.tree_index = dp->ds->dst->index,
 		.sw_index = dp->ds->index,
 		.port = dp->index,
 		.br = br,
 	};
-	int bridge_num = dp->bridge_num;
 	int err;
 
 	/* Here the port is already unbridged. Reflect the current configuration
@@ -507,12 +507,15 @@ int dsa_port_lag_join(struct dsa_port *dp, struct net_device *lag,
 
 void dsa_port_pre_lag_leave(struct dsa_port *dp, struct net_device *lag)
 {
-	if (dp->bridge_dev)
-		dsa_port_pre_bridge_leave(dp, dp->bridge_dev);
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
+
+	if (br)
+		dsa_port_pre_bridge_leave(dp, br);
 }
 
 void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag)
 {
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	struct dsa_notifier_lag_info info = {
 		.sw_index = dp->ds->index,
 		.port = dp->index,
@@ -526,8 +529,8 @@ void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag)
 	/* Port might have been part of a LAG that in turn was
 	 * attached to a bridge.
 	 */
-	if (dp->bridge_dev)
-		dsa_port_bridge_leave(dp, dp->bridge_dev);
+	if (br)
+		dsa_port_bridge_leave(dp, br);
 
 	dp->lag_tx_enabled = false;
 	dp->lag_dev = NULL;
@@ -556,8 +559,8 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 	 * as long as we have 8021q uppers.
 	 */
 	if (vlan_filtering && dsa_port_is_user(dp)) {
+		struct net_device *br = dsa_port_bridge_dev_get(dp);
 		struct net_device *upper_dev, *slave = dp->slave;
-		struct net_device *br = dp->bridge_dev;
 		struct list_head *iter;
 
 		netdev_for_each_upper_dev_rcu(slave, upper_dev, iter) {
@@ -591,17 +594,15 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 	 * different setting than what is being requested.
 	 */
 	dsa_switch_for_each_port(other_dp, ds) {
-		struct net_device *other_bridge;
+		struct net_device *other_br = dsa_port_bridge_dev_get(other_dp);
 
-		other_bridge = other_dp->bridge_dev;
-		if (!other_bridge)
-			continue;
 		/* If it's the same bridge, it also has same
 		 * vlan_filtering setting => no need to check
 		 */
-		if (other_bridge == dp->bridge_dev)
+		if (!other_br || other_br == dsa_port_bridge_dev_get(dp))
 			continue;
-		if (br_vlan_enabled(other_bridge) != vlan_filtering) {
+
+		if (br_vlan_enabled(other_br) != vlan_filtering) {
 			NL_SET_ERR_MSG_MOD(extack,
 					   "VLAN filtering is a global setting");
 			return false;
@@ -685,13 +686,13 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
  */
 bool dsa_port_skip_vlan_configuration(struct dsa_port *dp)
 {
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	struct dsa_switch *ds = dp->ds;
 
-	if (!dp->bridge_dev)
+	if (!br)
 		return false;
 
-	return (!ds->configure_vlan_while_not_filtering &&
-		!br_vlan_enabled(dp->bridge_dev));
+	return !ds->configure_vlan_while_not_filtering && !br_vlan_enabled(br);
 }
 
 int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index dbda0e0fbffa..ad873bb2f676 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -363,7 +363,7 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 	/* Deny adding a bridge VLAN when there is already an 802.1Q upper with
 	 * the same VID.
 	 */
-	if (br_vlan_enabled(dp->bridge_dev)) {
+	if (br_vlan_enabled(dsa_port_bridge_dev_get(dp))) {
 		rcu_read_lock();
 		err = dsa_slave_vlan_check_for_8021q_uppers(dev, &vlan);
 		rcu_read_unlock();
@@ -1580,7 +1580,8 @@ static void dsa_bridge_mtu_normalization(struct dsa_port *dp)
 			if (other_dp->type != DSA_PORT_TYPE_USER)
 				continue;
 
-			if (other_dp->bridge_dev != dp->bridge_dev)
+			if (dsa_port_bridge_dev_get(other_dp) !=
+			    dsa_port_bridge_dev_get(dp))
 				continue;
 
 			if (!other_dp->ds->mtu_enforcement_ingress)
@@ -2229,7 +2230,7 @@ dsa_prevent_bridging_8021q_upper(struct net_device *dev,
 				 struct netdev_notifier_changeupper_info *info)
 {
 	struct netlink_ext_ack *ext_ack;
-	struct net_device *slave;
+	struct net_device *slave, *br;
 	struct dsa_port *dp;
 
 	ext_ack = netdev_notifier_info_to_extack(&info->info);
@@ -2242,11 +2243,12 @@ dsa_prevent_bridging_8021q_upper(struct net_device *dev,
 		return NOTIFY_DONE;
 
 	dp = dsa_slave_to_port(slave);
-	if (!dp->bridge_dev)
+	br = dsa_port_bridge_dev_get(dp);
+	if (!br)
 		return NOTIFY_DONE;
 
 	/* Deny enslaving a VLAN device into a VLAN-aware bridge */
-	if (br_vlan_enabled(dp->bridge_dev) &&
+	if (br_vlan_enabled(br) &&
 	    netif_is_bridge_master(info->upper_dev) && info->linking) {
 		NL_SET_ERR_MSG_MOD(ext_ack,
 				   "Cannot enslave VLAN device into VLAN aware bridge");
@@ -2261,7 +2263,7 @@ dsa_slave_check_8021q_upper(struct net_device *dev,
 			    struct netdev_notifier_changeupper_info *info)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
-	struct net_device *br = dp->bridge_dev;
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	struct bridge_vlan_info br_info;
 	struct netlink_ext_ack *extack;
 	int err = NOTIFY_DONE;
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index bb155a16d454..7993192fe769 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -151,11 +151,9 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 	 */
 	if (change_vlan_filtering && ds->vlan_filtering_is_global) {
 		dsa_switch_for_each_port(dp, ds) {
-			struct net_device *bridge_dev;
+			struct net_device *br = dsa_port_bridge_dev_get(dp);
 
-			bridge_dev = dp->bridge_dev;
-
-			if (bridge_dev && br_vlan_enabled(bridge_dev)) {
+			if (br && br_vlan_enabled(br)) {
 				change_vlan_filtering = false;
 				break;
 			}
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index df59f16436a5..e9d5e566973c 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -337,7 +337,7 @@ dsa_port_tag_8021q_bridge_match(struct dsa_port *dp,
 		return false;
 
 	if (dsa_port_is_user(dp))
-		return dp->bridge_dev == info->br;
+		return dsa_port_bridge_dev_get(dp) == info->br;
 
 	return false;
 }
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index a7d70ae7cc97..8abf39dcac64 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -132,6 +132,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
 	u8 *dsa_header;
 
 	if (skb->offload_fwd_mark) {
+		unsigned int bridge_num = dsa_port_bridge_num_get(dp);
 		struct dsa_switch_tree *dst = dp->ds->dst;
 
 		cmd = DSA_CMD_FORWARD;
@@ -140,7 +141,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
 		 * packets on behalf of a virtual switch device with an index
 		 * past the physical switches.
 		 */
-		tag_dev = dst->last_switch + dp->bridge_num;
+		tag_dev = dst->last_switch + bridge_num;
 		tag_port = 0;
 	} else {
 		cmd = DSA_CMD_FROM_CPU;
@@ -165,7 +166,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
 			dsa_header[2] &= ~0x10;
 		}
 	} else {
-		struct net_device *br = dp->bridge_dev;
+		struct net_device *br = dsa_port_bridge_dev_get(dp);
 		u16 vid;
 
 		vid = br ? MV88E6XXX_VID_BRIDGED : MV88E6XXX_VID_STANDALONE;
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index cd60b94fc175..462bfa1a17da 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -12,7 +12,7 @@
 static void ocelot_xmit_get_vlan_info(struct sk_buff *skb, struct dsa_port *dp,
 				      u64 *vlan_tci, u64 *tag_type)
 {
-	struct net_device *br = READ_ONCE(dp->bridge_dev);
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	struct vlan_ethhdr *hdr;
 	u16 proto, tci;
 
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 262c8833a910..6c293c2a3008 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -159,14 +159,16 @@ static u16 sja1105_xmit_tpid(struct dsa_port *dp)
 	 * need to find it.
 	 */
 	dsa_switch_for_each_port(other_dp, ds) {
-		if (!other_dp->bridge_dev)
+		struct net_device *br = dsa_port_bridge_dev_get(other_dp);
+
+		if (!br)
 			continue;
 
 		/* Error is returned only if CONFIG_BRIDGE_VLAN_FILTERING,
 		 * which seems pointless to handle, as our port cannot become
 		 * VLAN-aware in that case.
 		 */
-		br_vlan_get_proto(other_dp->bridge_dev, &proto);
+		br_vlan_get_proto(br, &proto);
 
 		return proto;
 	}
@@ -180,7 +182,8 @@ static struct sk_buff *sja1105_imprecise_xmit(struct sk_buff *skb,
 					      struct net_device *netdev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(netdev);
-	struct net_device *br = dp->bridge_dev;
+	unsigned int bridge_num = dsa_port_bridge_num_get(dp);
+	struct net_device *br = dsa_port_bridge_dev_get(dp);
 	u16 tx_vid;
 
 	/* If the port is under a VLAN-aware bridge, just slide the
@@ -196,7 +199,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(dp->bridge_num);
+	tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
 
 	return dsa_8021q_xmit(skb, netdev, sja1105_xmit_tpid(dp), tx_vid);
 }
-- 
2.25.1


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

* [RFC PATCH net-next 4/6] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev
  2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
                   ` (2 preceding siblings ...)
  2021-10-26 16:26 ` [RFC PATCH net-next 3/6] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers Vladimir Oltean
@ 2021-10-26 16:26 ` Vladimir Oltean
  2021-11-11 12:33   ` Alvin Šipraga
  2021-10-26 16:26 ` [RFC PATCH net-next 5/6] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2021-10-26 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang, Alvin Šipraga

Currently the majority of dsa_port_bridge_dev_get() calls in drivers is
just to check whether a port is under the bridge device provided as
argument by the DSA API.

We'd like to change that DSA API so that a more complex structure is
provided as argument. To keep things more generic, and considering that
the new complex structure will be provided by value and not by
reference, direct comparisons between dp->bridge and the provided bridge
will be broken. The generic way to do the checking would simply be to
do something like dsa_port_offloads_bridge(dp, &bridge).

But there's a problem, we already have a function named that way, which
actually takes a bridge_dev net_device as argument. Rename it so that we
can use dsa_port_offloads_bridge for something else.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa_priv.h | 12 +++++++-----
 net/dsa/slave.c    | 18 +++++++++---------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 62b719929ef4..daba10adbd22 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -272,8 +272,9 @@ static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
 	return dsa_port_to_bridge_port(dp) == dev;
 }
 
-static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
-					    const struct net_device *bridge_dev)
+static inline bool
+dsa_port_offloads_bridge_dev(struct dsa_port *dp,
+			     const struct net_device *bridge_dev)
 {
 	/* DSA ports connected to a bridge, and event was emitted
 	 * for the bridge.
@@ -295,13 +296,14 @@ static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
 }
 
 /* Returns true if any port of this tree offloads the given bridge */
-static inline bool dsa_tree_offloads_bridge(struct dsa_switch_tree *dst,
-					    const struct net_device *bridge_dev)
+static inline bool
+dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
+			     const struct net_device *bridge_dev)
 {
 	struct dsa_port *dp;
 
 	list_for_each_entry(dp, &dst->ports, list)
-		if (dsa_port_offloads_bridge(dp, bridge_dev))
+		if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
 			return true;
 
 	return false;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ad873bb2f676..98fd5e972d28 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -289,14 +289,14 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
 		ret = dsa_port_set_state(dp, attr->u.stp_state, true);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
-		if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
+		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
 			return -EOPNOTSUPP;
 
 		ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering,
 					      extack);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
-		if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
+		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
 			return -EOPNOTSUPP;
 
 		ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
@@ -409,7 +409,7 @@ static int dsa_slave_port_obj_add(struct net_device *dev, const void *ctx,
 		err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_HOST_MDB:
-		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
 		err = dsa_port_host_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
@@ -421,13 +421,13 @@ static int dsa_slave_port_obj_add(struct net_device *dev, const void *ctx,
 		err = dsa_slave_vlan_add(dev, obj, extack);
 		break;
 	case SWITCHDEV_OBJ_ID_MRP:
-		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
 		err = dsa_port_mrp_add(dp, SWITCHDEV_OBJ_MRP(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
-		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
 		err = dsa_port_mrp_add_ring_role(dp,
@@ -483,7 +483,7 @@ static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
 		err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_HOST_MDB:
-		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
 		err = dsa_port_host_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
@@ -495,13 +495,13 @@ static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
 		err = dsa_slave_vlan_del(dev, obj);
 		break;
 	case SWITCHDEV_OBJ_ID_MRP:
-		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
 		err = dsa_port_mrp_del(dp, SWITCHDEV_OBJ_MRP(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
-		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
+		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
 		err = dsa_port_mrp_del_ring_role(dp,
@@ -2460,7 +2460,7 @@ static bool dsa_foreign_dev_check(const struct net_device *dev,
 	struct dsa_switch_tree *dst = dp->ds->dst;
 
 	if (netif_is_bridge_master(foreign_dev))
-		return !dsa_tree_offloads_bridge(dst, foreign_dev);
+		return !dsa_tree_offloads_bridge_dev(dst, foreign_dev);
 
 	if (netif_is_bridge_port(foreign_dev))
 		return !dsa_tree_offloads_bridge_port(dst, foreign_dev);
-- 
2.25.1


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

* [RFC PATCH net-next 5/6] net: dsa: keep the bridge_dev and bridge_num as part of the same structure
  2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
                   ` (3 preceding siblings ...)
  2021-10-26 16:26 ` [RFC PATCH net-next 4/6] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev Vladimir Oltean
@ 2021-10-26 16:26 ` Vladimir Oltean
  2021-11-11 13:17   ` Alvin Šipraga
  2021-10-26 16:26 ` [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
  2021-11-02 10:07 ` [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
  6 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2021-10-26 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang, Alvin Šipraga

The main desire behind this is to provide coherent bridge information to
the fast path without locking.

For example, right now we set dp->bridge_dev and dp->bridge_num from
separate code paths, it is theoretically possible for a packet
transmission to read these two port properties consecutively and find a
bridge number which does not correspond with the bridge device.

Another desire is to start passing more complex bridge information to
dsa_switch_ops functions. For example, with FDB isolation, it is
expected that drivers will need to be passed the bridge which requested
an FDB/MDB entry to be offloaded, and along with that bridge_dev, the
associated bridge_num should be passed too, in case the driver might
want to implement an isolation scheme based on that number.

We already pass the {bridge_dev, bridge_num} pair to the TX forwarding
offload switch API, however we'd like to remove that and squash it into
the basic bridge join/leave API. So that means we need to pass this
pair to the bridge join/leave API.

During dsa_port_bridge_leave, first we unset dp->bridge_dev, then we
call the driver's .port_bridge_leave with what used to be our
dp->bridge_dev, but provided as an argument.

When bridge_dev and bridge_num get folded into a single structure, we
need to preserve this behavior in dsa_port_bridge_leave: we need a copy
of what used to be in dp->bridge.

Switch drivers check bridge membership by comparing dp->bridge_dev with
the provided bridge_dev, but now, if we provide the struct dsa_bridge as
a pointer, they cannot keep comparing dp->bridge to the provided
pointer, since this only points to an on-stack copy. To make this
obvious and prevent driver writers from forgetting and doing stupid
things, in this new API, the struct dsa_bridge is provided as a full
structure (not very large, contains an int and a pointer) instead of a
pointer. An explicit comparison function needs to be used to determine
bridge membership: dsa_port_offloads_bridge().

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c       |  8 +--
 drivers/net/dsa/b53/b53_priv.h         |  4 +-
 drivers/net/dsa/dsa_loop.c             |  8 +--
 drivers/net/dsa/hirschmann/hellcreek.c |  4 +-
 drivers/net/dsa/lan9303-core.c         |  4 +-
 drivers/net/dsa/lantiq_gswip.c         | 14 +++--
 drivers/net/dsa/microchip/ksz_common.c |  4 +-
 drivers/net/dsa/microchip/ksz_common.h |  4 +-
 drivers/net/dsa/mt7530.c               |  8 +--
 drivers/net/dsa/mv88e6xxx/chip.c       | 26 ++++-----
 drivers/net/dsa/ocelot/felix.c         |  8 +--
 drivers/net/dsa/qca8k.c                | 12 ++--
 drivers/net/dsa/rtl8366rb.c            |  8 +--
 drivers/net/dsa/sja1105/sja1105_main.c | 12 ++--
 drivers/net/dsa/xrs700x/xrs700x.c      | 10 ++--
 include/linux/dsa/8021q.h              |  7 +--
 include/net/dsa.h                      | 77 +++++++++++++++++++++-----
 net/dsa/dsa2.c                         | 35 ++++++++----
 net/dsa/dsa_priv.h                     | 53 ++----------------
 net/dsa/port.c                         | 69 ++++++++++++-----------
 net/dsa/slave.c                        |  2 +-
 net/dsa/switch.c                       | 13 +++--
 net/dsa/tag_8021q.c                    | 12 ++--
 23 files changed, 215 insertions(+), 187 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index d5e78f51f42d..4e41b1a63108 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1860,7 +1860,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 net_device *br)
+int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
 {
 	struct b53_device *dev = ds->priv;
 	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
@@ -1887,7 +1887,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
 	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
 
 	b53_for_each_port(dev, i) {
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 
 		/* Add this local port to the remote port VLAN control
@@ -1911,7 +1911,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
 }
 EXPORT_SYMBOL(b53_br_join);
 
-void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
+void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
 {
 	struct b53_device *dev = ds->priv;
 	struct b53_vlan *vl = &dev->vlans[0];
@@ -1923,7 +1923,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
 
 	b53_for_each_port(dev, i) {
 		/* Don't touch the remaining ports */
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 
 		b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), &reg);
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 579da74ada64..ee17f8b516ca 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -324,8 +324,8 @@ void b53_get_strings(struct dsa_switch *ds, int port, u32 stringset,
 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 net_device *bridge);
-void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *bridge);
+int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
+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);
 int b53_br_flags_pre(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index e638e3eea911..70db3a9aa355 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -167,19 +167,19 @@ 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 net_device *bridge)
+				     struct dsa_bridge bridge)
 {
 	dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
-		__func__, port, bridge->name);
+		__func__, port, bridge.dev->name);
 
 	return 0;
 }
 
 static void dsa_loop_port_bridge_leave(struct dsa_switch *ds, int port,
-				       struct net_device *bridge)
+				       struct dsa_bridge bridge)
 {
 	dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
-		__func__, port, bridge->name);
+		__func__, port, bridge.dev->name);
 }
 
 static void dsa_loop_port_stp_state_set(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 4e0b53d94b52..17c2d13670a9 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -674,7 +674,7 @@ static int hellcreek_bridge_flags(struct dsa_switch *ds, int port,
 }
 
 static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
-				      struct net_device *br)
+				      struct dsa_bridge bridge)
 {
 	struct hellcreek *hellcreek = ds->priv;
 
@@ -691,7 +691,7 @@ static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
 }
 
 static void hellcreek_port_bridge_leave(struct dsa_switch *ds, int port,
-					struct net_device *br)
+					struct dsa_bridge bridge)
 {
 	struct hellcreek *hellcreek = ds->priv;
 
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 1c2bdcde6979..29d909484275 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1103,7 +1103,7 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port)
 }
 
 static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
-				    struct net_device *br)
+				    struct dsa_bridge bridge)
 {
 	struct lan9303 *chip = ds->priv;
 
@@ -1117,7 +1117,7 @@ static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
 }
 
 static void lan9303_port_bridge_leave(struct dsa_switch *ds, int port,
-				      struct net_device *br)
+				      struct dsa_bridge bridge)
 {
 	struct lan9303 *chip = ds->priv;
 
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index c60b1eef3545..6b9b9c712e58 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1146,16 +1146,17 @@ static int gswip_vlan_remove(struct gswip_priv *priv,
 }
 
 static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
-				  struct net_device *bridge)
+				  struct dsa_bridge bridge)
 {
+	struct net_device *br = bridge.dev;
 	struct gswip_priv *priv = ds->priv;
 	int err;
 
 	/* When the bridge uses VLAN filtering we have to configure VLAN
 	 * specific bridges. No bridge is configured here.
 	 */
-	if (!br_vlan_enabled(bridge)) {
-		err = gswip_vlan_add_unaware(priv, bridge, port);
+	if (!br_vlan_enabled(br)) {
+		err = gswip_vlan_add_unaware(priv, br, port);
 		if (err)
 			return err;
 		priv->port_vlan_filter &= ~BIT(port);
@@ -1166,8 +1167,9 @@ static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
 }
 
 static void gswip_port_bridge_leave(struct dsa_switch *ds, int port,
-				    struct net_device *bridge)
+				    struct dsa_bridge bridge)
 {
+	struct net_device *br = bridge.dev;
 	struct gswip_priv *priv = ds->priv;
 
 	gswip_add_single_port_br(priv, port, true);
@@ -1175,8 +1177,8 @@ static void gswip_port_bridge_leave(struct dsa_switch *ds, int port,
 	/* When the bridge uses VLAN filtering we have to configure VLAN
 	 * specific bridges. No bridge is configured here.
 	 */
-	if (!br_vlan_enabled(bridge))
-		gswip_vlan_remove(priv, bridge, port, 0, true, false);
+	if (!br_vlan_enabled(br))
+		gswip_vlan_remove(priv, br, port, 0, true, false);
 }
 
 static int gswip_port_vlan_prepare(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 7c2968a639eb..23784d14a5da 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -173,7 +173,7 @@ void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
 EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
 
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
-			 struct net_device *br)
+			 struct dsa_bridge bridge)
 {
 	struct ksz_device *dev = ds->priv;
 
@@ -190,7 +190,7 @@ int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 EXPORT_SYMBOL_GPL(ksz_port_bridge_join);
 
 void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
-			   struct net_device *br)
+			   struct dsa_bridge bridge)
 {
 	struct ksz_device *dev = ds->priv;
 
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 1597c63988b4..df953c2bc56e 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -159,9 +159,9 @@ 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 net_device *br);
+			 struct dsa_bridge bridge);
 void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
-			   struct net_device *br);
+			   struct dsa_bridge bridge);
 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);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 79bde263b06f..85cc5aca7f96 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1186,7 +1186,7 @@ mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
 
 static int
 mt7530_port_bridge_join(struct dsa_switch *ds, int port,
-			struct net_device *bridge)
+			struct dsa_bridge bridge)
 {
 	struct mt7530_priv *priv = ds->priv;
 	u32 port_bitmap = BIT(MT7530_CPU_PORT);
@@ -1202,7 +1202,7 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
 		 * and not being setup until the port becomes enabled.
 		 */
 		if (dsa_port_is_user(other_dp) && i != port) {
-			if (dsa_port_bridge_dev_get(other_dp) != bridge)
+			if (!dsa_port_offloads_bridge(other_dp, &bridge))
 				continue;
 			if (priv->ports[i].enable)
 				mt7530_set(priv, MT7530_PCR_P(i),
@@ -1301,7 +1301,7 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
 
 static void
 mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
-			 struct net_device *bridge)
+			 struct dsa_bridge bridge)
 {
 	struct mt7530_priv *priv = ds->priv;
 	int i;
@@ -1316,7 +1316,7 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 		 * is kept and not being setup until the port becomes enabled.
 		 */
 		if (dsa_port_is_user(other_dp) && i != port) {
-			if (dsa_port_bridge_dev_get(other_dp) != bridge)
+			if (!dsa_port_offloads_bridge(other_dp, &bridge))
 				continue;
 			if (priv->ports[i].enable)
 				mt7530_clear(priv, MT7530_PCR_P(i),
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b76ce4423ea8..73613a667f6c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2417,7 +2417,7 @@ static int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
 }
 
 static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
-				struct net_device *br)
+				struct dsa_bridge bridge)
 {
 	struct dsa_switch *ds = chip->ds;
 	struct dsa_switch_tree *dst = ds->dst;
@@ -2425,7 +2425,7 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
 	int err;
 
 	list_for_each_entry(dp, &dst->ports, list) {
-		if (dsa_port_bridge_dev_get(dp) == br) {
+		if (dsa_port_offloads_bridge(dp, &bridge)) {
 			if (dp->ds == ds) {
 				/* This is a local bridge group member,
 				 * remap its Port VLAN Map.
@@ -2449,14 +2449,14 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
 }
 
 static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
-				      struct net_device *br)
+				      struct dsa_bridge bridge)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
 
 	mv88e6xxx_reg_lock(chip);
 
-	err = mv88e6xxx_bridge_map(chip, br);
+	err = mv88e6xxx_bridge_map(chip, bridge);
 	if (err)
 		goto unlock;
 
@@ -2471,14 +2471,14 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 }
 
 static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
-					struct net_device *br)
+					struct dsa_bridge bridge)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
 
 	mv88e6xxx_reg_lock(chip);
 
-	if (mv88e6xxx_bridge_map(chip, br) ||
+	if (mv88e6xxx_bridge_map(chip, bridge) ||
 	    mv88e6xxx_port_vlan_map(chip, port))
 		dev_err(ds->dev, "failed to remap in-chip Port VLAN\n");
 
@@ -2493,7 +2493,7 @@ 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 net_device *br)
+					   int port, struct dsa_bridge bridge)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
@@ -2510,7 +2510,7 @@ static int mv88e6xxx_crosschip_bridge_join(struct dsa_switch *ds,
 
 static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
 					     int tree_index, int sw_index,
-					     int port, struct net_device *br)
+					     int port, struct dsa_bridge bridge)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 
@@ -2542,19 +2542,17 @@ static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
 }
 
 static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
-					   struct net_device *br,
-					   unsigned int bridge_num)
+					   struct dsa_bridge bridge)
 {
-	return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
+	return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
 }
 
 static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
-					      struct net_device *br,
-					      unsigned int bridge_num)
+					      struct dsa_bridge bridge)
 {
 	int err;
 
-	err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
+	err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
 	if (err) {
 		dev_err(ds->dev, "failed to remap cross-chip Port VLAN: %pe\n",
 			ERR_PTR(err));
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 83808e7dbdda..8cf82fa91a6b 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -698,21 +698,21 @@ static int felix_bridge_flags(struct dsa_switch *ds, int port,
 }
 
 static int felix_bridge_join(struct dsa_switch *ds, int port,
-			     struct net_device *br)
+			     struct dsa_bridge bridge)
 {
 	struct ocelot *ocelot = ds->priv;
 
-	ocelot_port_bridge_join(ocelot, port, br);
+	ocelot_port_bridge_join(ocelot, port, bridge.dev);
 
 	return 0;
 }
 
 static void felix_bridge_leave(struct dsa_switch *ds, int port,
-			       struct net_device *br)
+			       struct dsa_bridge bridge)
 {
 	struct ocelot *ocelot = ds->priv;
 
-	ocelot_port_bridge_leave(ocelot, port, br);
+	ocelot_port_bridge_leave(ocelot, port, bridge.dev);
 }
 
 static int felix_lag_join(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index efe20db287a5..3ae10f22ada9 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1728,8 +1728,8 @@ qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
 		  QCA8K_PORT_LOOKUP_STATE_MASK, stp_state);
 }
 
-static int
-qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
+static int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
+				  struct dsa_bridge bridge)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
 	int port_mask, cpu_port;
@@ -1741,7 +1741,7 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
 		if (dsa_is_cpu_port(ds, i))
 			continue;
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 		/* Add this port to the portvlan mask of the other ports
 		 * in the bridge
@@ -1762,8 +1762,8 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
 	return ret;
 }
 
-static void
-qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
+static void qca8k_port_bridge_leave(struct dsa_switch *ds, int port,
+				    struct dsa_bridge bridge)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
 	int cpu_port, i;
@@ -1773,7 +1773,7 @@ qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
 		if (dsa_is_cpu_port(ds, i))
 			continue;
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 		/* Remove this port to the portvlan mask of the other ports
 		 * in the bridge
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index b6f277a04989..fac2333a3f5e 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -1186,7 +1186,7 @@ rtl8366rb_port_disable(struct dsa_switch *ds, int port)
 
 static int
 rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
-			   struct net_device *bridge)
+			   struct dsa_bridge bridge)
 {
 	struct realtek_smi *smi = ds->priv;
 	unsigned int port_bitmap = 0;
@@ -1198,7 +1198,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
 		if (i == port)
 			continue;
 		/* Not on this bridge */
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 		/* Join this port to each other port on the bridge */
 		ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
@@ -1218,7 +1218,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
 
 static void
 rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
-			    struct net_device *bridge)
+			    struct dsa_bridge bridge)
 {
 	struct realtek_smi *smi = ds->priv;
 	unsigned int port_bitmap = 0;
@@ -1230,7 +1230,7 @@ rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
 		if (i == port)
 			continue;
 		/* Not on this bridge */
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 		/* Remove this port from any other port on the bridge */
 		ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 5e03eda4c16f..24584fe2e760 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1980,7 +1980,7 @@ static int sja1105_manage_flood_domains(struct sja1105_private *priv)
 }
 
 static int sja1105_bridge_member(struct dsa_switch *ds, int port,
-				 struct net_device *br, bool member)
+				 struct dsa_bridge bridge, bool member)
 {
 	struct sja1105_l2_forwarding_entry *l2_fwd;
 	struct sja1105_private *priv = ds->priv;
@@ -2005,7 +2005,7 @@ static int sja1105_bridge_member(struct dsa_switch *ds, int port,
 		 */
 		if (i == port)
 			continue;
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 		sja1105_port_allow_traffic(l2_fwd, i, port, member);
 		sja1105_port_allow_traffic(l2_fwd, port, i, member);
@@ -2074,15 +2074,15 @@ static void sja1105_bridge_stp_state_set(struct dsa_switch *ds, int port,
 }
 
 static int sja1105_bridge_join(struct dsa_switch *ds, int port,
-			       struct net_device *br)
+			       struct dsa_bridge bridge)
 {
-	return sja1105_bridge_member(ds, port, br, true);
+	return sja1105_bridge_member(ds, port, bridge, true);
 }
 
 static void sja1105_bridge_leave(struct dsa_switch *ds, int port,
-				 struct net_device *br)
+				 struct dsa_bridge bridge)
 {
-	sja1105_bridge_member(ds, port, br, false);
+	sja1105_bridge_member(ds, port, bridge, false);
 }
 
 #define BYTES_PER_KBIT (1000LL / 8)
diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index 7c2b6c32242d..ebb55dfd9c4e 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -501,7 +501,7 @@ static void xrs700x_mac_link_up(struct dsa_switch *ds, int port,
 }
 
 static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
-				 struct net_device *bridge, bool join)
+				 struct dsa_bridge bridge, bool join)
 {
 	unsigned int i, cpu_mask = 0, mask = 0;
 	struct xrs700x *priv = ds->priv;
@@ -513,14 +513,14 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
 
 		cpu_mask |= BIT(i);
 
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) == bridge)
+		if (dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 
 		mask |= BIT(i);
 	}
 
 	for (i = 0; i < ds->num_ports; i++) {
-		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
 			continue;
 
 		/* 1 = Disable forwarding to the port */
@@ -540,13 +540,13 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
 }
 
 static int xrs700x_bridge_join(struct dsa_switch *ds, int port,
-			       struct net_device *bridge)
+			       struct dsa_bridge bridge)
 {
 	return xrs700x_bridge_common(ds, port, bridge, true);
 }
 
 static void xrs700x_bridge_leave(struct dsa_switch *ds, int port,
-				 struct net_device *bridge)
+				 struct dsa_bridge bridge)
 {
 	xrs700x_bridge_common(ds, port, bridge, false);
 }
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index 0af4371fbebb..939a1beaddf7 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -7,6 +7,7 @@
 
 #include <linux/refcount.h>
 #include <linux/types.h>
+#include <net/dsa.h>
 
 struct dsa_switch;
 struct dsa_port;
@@ -37,12 +38,10 @@ struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev,
 void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
 
 int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
-					struct net_device *br,
-					unsigned int bridge_num);
+					struct dsa_bridge bridge);
 
 void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
-					   struct net_device *br,
-					   unsigned int bridge_num);
+					   struct dsa_bridge bridge);
 
 u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 81fd1821a0aa..5e83c64bc96a 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -219,6 +219,11 @@ struct dsa_mall_tc_entry {
 	};
 };
 
+struct dsa_bridge {
+	struct net_device *dev;
+	unsigned int num;
+	refcount_t refcount;
+};
 
 struct dsa_port {
 	/* A CPU port is physically connected to a master device.
@@ -256,8 +261,7 @@ struct dsa_port {
 	/* Managed by DSA on user ports and by drivers on CPU and DSA ports */
 	bool			learning;
 	u8			stp_state;
-	struct net_device	*bridge_dev;
-	unsigned int		bridge_num;
+	struct dsa_bridge	*bridge;
 	struct devlink_port	devlink_port;
 	bool			devlink_port_setup;
 	struct phylink		*pl;
@@ -588,7 +592,7 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp)
 static inline
 struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
 {
-	if (!dp->bridge_dev)
+	if (!dp->bridge)
 		return NULL;
 
 	if (dp->lag_dev)
@@ -601,12 +605,12 @@ struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
 
 static inline struct net_device *dsa_port_bridge_dev_get(struct dsa_port *dp)
 {
-	return dp->bridge_dev;
+	return dp->bridge ? dp->bridge->dev : NULL;
 }
 
 static inline unsigned int dsa_port_bridge_num_get(struct dsa_port *dp)
 {
-	return dp->bridge_num;
+	return dp->bridge ? dp->bridge->num : 0;
 }
 
 static inline bool dsa_port_bridge_same(struct dsa_port *a, struct dsa_port *b)
@@ -614,6 +618,55 @@ static inline bool dsa_port_bridge_same(struct dsa_port *a, struct dsa_port *b)
 	return dsa_port_bridge_dev_get(a) == dsa_port_bridge_dev_get(b);
 }
 
+static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
+						 const struct net_device *dev)
+{
+	return dsa_port_to_bridge_port(dp) == dev;
+}
+
+static inline bool
+dsa_port_offloads_bridge_dev(struct dsa_port *dp,
+			     const struct net_device *bridge_dev)
+{
+	/* DSA ports connected to a bridge, and event was emitted
+	 * for the bridge.
+	 */
+	return dsa_port_bridge_dev_get(dp) == bridge_dev;
+}
+
+static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
+					    const struct dsa_bridge *bridge)
+{
+	return dsa_port_bridge_dev_get(dp) == bridge->dev;
+}
+
+/* Returns true if any port of this tree offloads the given net_device */
+static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
+						 const struct net_device *dev)
+{
+	struct dsa_port *dp;
+
+	list_for_each_entry(dp, &dst->ports, list)
+		if (dsa_port_offloads_bridge_port(dp, dev))
+			return true;
+
+	return false;
+}
+
+/* Returns true if any port of this tree offloads the given bridge */
+static inline bool
+dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
+			     const struct net_device *bridge_dev)
+{
+	struct dsa_port *dp;
+
+	list_for_each_entry(dp, &dst->ports, list)
+		if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
+			return true;
+
+	return false;
+}
+
 typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
 			      bool is_static, void *data);
 struct dsa_switch_ops {
@@ -761,17 +814,15 @@ 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 net_device *bridge);
+				    struct dsa_bridge bridge);
 	void	(*port_bridge_leave)(struct dsa_switch *ds, int port,
-				     struct net_device *bridge);
+				     struct dsa_bridge bridge);
 	/* Called right after .port_bridge_join() */
 	int	(*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
-					      struct net_device *bridge,
-					      unsigned int bridge_num);
+					      struct dsa_bridge bridge);
 	/* Called right before .port_bridge_leave() */
 	void	(*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
-						struct net_device *bridge,
-						unsigned int bridge_num);
+						struct dsa_bridge bridge);
 	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
 				      u8 state);
 	void	(*port_fast_age)(struct dsa_switch *ds, int port);
@@ -843,10 +894,10 @@ struct dsa_switch_ops {
 	 */
 	int	(*crosschip_bridge_join)(struct dsa_switch *ds, int tree_index,
 					 int sw_index, int port,
-					 struct net_device *br);
+					 struct dsa_bridge bridge);
 	void	(*crosschip_bridge_leave)(struct dsa_switch *ds, int tree_index,
 					  int sw_index, int port,
-					  struct net_device *br);
+					  struct dsa_bridge bridge);
 	int	(*crosschip_lag_change)(struct dsa_switch *ds, int sw_index,
 					int port);
 	int	(*crosschip_lag_join)(struct dsa_switch *ds, int sw_index,
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 9606e56710a5..7c014aeea559 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -129,20 +129,29 @@ void dsa_lag_unmap(struct dsa_switch_tree *dst, struct net_device *lag)
 	}
 }
 
+struct dsa_bridge *dsa_tree_bridge_find(struct dsa_switch_tree *dst,
+					const struct net_device *br)
+{
+	struct dsa_port *dp;
+
+	list_for_each_entry(dp, &dst->ports, list)
+		if (dsa_port_bridge_dev_get(dp) == br)
+			return dp->bridge;
+
+	return NULL;
+}
+
 static int dsa_bridge_num_find(const struct net_device *bridge_dev)
 {
 	struct dsa_switch_tree *dst;
-	struct dsa_port *dp;
 
-	/* When preparing the offload for a port, it will have a valid
-	 * dp->bridge_dev pointer but a not yet valid dp->bridge_num.
-	 * However there might be other ports having the same dp->bridge_dev
-	 * and a valid dp->bridge_num, so just ignore this port.
-	 */
-	list_for_each_entry(dst, &dsa_tree_list, list)
-		list_for_each_entry(dp, &dst->ports, list)
-			if (dp->bridge_dev == bridge_dev && dp->bridge_num)
-				return dp->bridge_num;
+	list_for_each_entry(dst, &dsa_tree_list, list) {
+		struct dsa_bridge *bridge;
+
+		bridge = dsa_tree_bridge_find(dst, bridge_dev);
+		if (bridge)
+			return bridge->num;
+	}
 
 	return 0;
 }
@@ -151,6 +160,12 @@ unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
 {
 	unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
 
+	/* Switches without FDB isolation support don't get unique
+	 * bridge numbering
+	 */
+	if (!max)
+		return 0;
+
 	if (!bridge_num) {
 		/* First port that offloads TX forwarding for this bridge */
 		bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index daba10adbd22..489712ead08f 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -52,7 +52,7 @@ struct dsa_notifier_ageing_time_info {
 
 /* DSA_NOTIFIER_BRIDGE_* */
 struct dsa_notifier_bridge_info {
-	struct net_device *br;
+	struct dsa_bridge bridge;
 	int tree_index;
 	int sw_index;
 	int port;
@@ -266,49 +266,6 @@ int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast);
 void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast);
 extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
 
-static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
-						 const struct net_device *dev)
-{
-	return dsa_port_to_bridge_port(dp) == dev;
-}
-
-static inline bool
-dsa_port_offloads_bridge_dev(struct dsa_port *dp,
-			     const struct net_device *bridge_dev)
-{
-	/* DSA ports connected to a bridge, and event was emitted
-	 * for the bridge.
-	 */
-	return dsa_port_bridge_dev_get(dp) == bridge_dev;
-}
-
-/* Returns true if any port of this tree offloads the given net_device */
-static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
-						 const struct net_device *dev)
-{
-	struct dsa_port *dp;
-
-	list_for_each_entry(dp, &dst->ports, list)
-		if (dsa_port_offloads_bridge_port(dp, dev))
-			return true;
-
-	return false;
-}
-
-/* Returns true if any port of this tree offloads the given bridge */
-static inline bool
-dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
-			     const struct net_device *bridge_dev)
-{
-	struct dsa_port *dp;
-
-	list_for_each_entry(dp, &dst->ports, list)
-		if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
-			return true;
-
-	return false;
-}
-
 /* slave.c */
 extern const struct dsa_device_ops notag_netdev_ops;
 extern struct notifier_block dsa_slave_switchdev_notifier;
@@ -417,7 +374,7 @@ dsa_find_designated_bridge_port_by_vid(struct net_device *master, u16 vid)
 		if (dp->type != DSA_PORT_TYPE_USER)
 			continue;
 
-		if (!dp->bridge_dev)
+		if (!dp->bridge)
 			continue;
 
 		if (dp->stp_state != BR_STATE_LEARNING &&
@@ -446,7 +403,7 @@ dsa_find_designated_bridge_port_by_vid(struct net_device *master, u16 vid)
 /* If the ingress port offloads the bridge, we mark the frame as autonomously
  * forwarded by hardware, so the software bridge doesn't forward in twice, back
  * to us, because we already did. However, if we're in fallback mode and we do
- * software bridging, we are not offloading it, therefore the dp->bridge_dev
+ * software bridging, we are not offloading it, therefore the dp->bridge
  * pointer is not populated, and flooding needs to be done by software (we are
  * effectively operating in standalone ports mode).
  */
@@ -454,7 +411,7 @@ static inline void dsa_default_offload_fwd_mark(struct sk_buff *skb)
 {
 	struct dsa_port *dp = dsa_slave_to_port(skb->dev);
 
-	skb->offload_fwd_mark = !!(dp->bridge_dev);
+	skb->offload_fwd_mark = !!(dp->bridge);
 }
 
 /* Helper for removing DSA header tags from packets in the RX path.
@@ -551,6 +508,8 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
 unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
 void dsa_bridge_num_put(const struct net_device *bridge_dev,
 			unsigned int bridge_num);
+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,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index aaa978ee165e..b2e66994f72c 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -130,7 +130,7 @@ int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy)
 			return err;
 	}
 
-	if (!dp->bridge_dev)
+	if (!dp->bridge)
 		dsa_port_set_state_now(dp, BR_STATE_FORWARDING, false);
 
 	if (dp->pl)
@@ -158,7 +158,7 @@ void dsa_port_disable_rt(struct dsa_port *dp)
 	if (dp->pl)
 		phylink_stop(dp->pl);
 
-	if (!dp->bridge_dev)
+	if (!dp->bridge)
 		dsa_port_set_state_now(dp, BR_STATE_DISABLED, false);
 
 	if (ds->ops->port_disable)
@@ -271,36 +271,32 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
 }
 
 static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
-					     struct net_device *bridge_dev,
-					     unsigned int bridge_num)
+					     struct dsa_bridge bridge)
 {
 	struct dsa_switch *ds = dp->ds;
 
 	/* No bridge TX forwarding offload => do nothing */
-	if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge_num)
+	if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge.num)
 		return;
 
 	/* Notify the chips only once the offload has been deactivated, so
 	 * that they can update their configuration accordingly.
 	 */
-	ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge_dev,
-					      bridge_num);
+	ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge);
 }
 
 static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
-					   struct net_device *bridge_dev,
-					   unsigned int bridge_num)
+					   struct dsa_bridge bridge)
 {
 	struct dsa_switch *ds = dp->ds;
 	int err;
 
 	/* FDB isolation is required for TX forwarding offload */
-	if (!ds->ops->port_bridge_tx_fwd_offload || !bridge_num)
+	if (!ds->ops->port_bridge_tx_fwd_offload || !bridge.num)
 		return false;
 
 	/* Notify the driver */
-	err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge_dev,
-						  bridge_num);
+	err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge);
 
 	return err ? false : true;
 }
@@ -310,21 +306,31 @@ static int dsa_port_bridge_create(struct dsa_port *dp,
 				  struct netlink_ext_ack *extack)
 {
 	struct dsa_switch *ds = dp->ds;
-	unsigned int bridge_num;
+	struct dsa_bridge *bridge;
 
-	dp->bridge_dev = br;
-
-	if (!ds->max_num_bridges)
+	bridge = dsa_tree_bridge_find(ds->dst, br);
+	if (bridge) {
+		refcount_inc(&bridge->refcount);
+		dp->bridge = bridge;
 		return 0;
+	}
+
+	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
+	if (!bridge)
+		return -ENOMEM;
+
+	refcount_set(&bridge->refcount, 1);
+
+	bridge->dev = br;
 
-	bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges);
-	if (!bridge_num) {
+	bridge->num = dsa_bridge_num_get(br, ds->max_num_bridges);
+	if (ds->max_num_bridges && !bridge->num) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Range of offloadable bridges exceeded");
 		return -EOPNOTSUPP;
 	}
 
-	dp->bridge_num = bridge_num;
+	dp->bridge = bridge;
 
 	return 0;
 }
@@ -332,16 +338,17 @@ static int dsa_port_bridge_create(struct dsa_port *dp,
 static void dsa_port_bridge_destroy(struct dsa_port *dp,
 				    const struct net_device *br)
 {
-	struct dsa_switch *ds = dp->ds;
+	struct dsa_bridge *bridge = dp->bridge;
+
+	dp->bridge = NULL;
 
-	dp->bridge_dev = NULL;
+	if (!refcount_dec_and_test(&bridge->refcount))
+		return;
 
-	if (ds->max_num_bridges) {
-		int bridge_num = dp->bridge_num;
+	if (bridge->num)
+		dsa_bridge_num_put(br, bridge->num);
 
-		dp->bridge_num = 0;
-		dsa_bridge_num_put(br, bridge_num);
-	}
+	kfree(bridge);
 }
 
 int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
@@ -351,7 +358,6 @@ 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,
-		.br = br,
 	};
 	struct net_device *dev = dp->slave;
 	struct net_device *brport_dev;
@@ -367,12 +373,12 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 
 	brport_dev = dsa_port_to_bridge_port(dp);
 
+	info.bridge = *dp->bridge;
 	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_JOIN, &info);
 	if (err)
 		goto out_rollback;
 
-	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
-							dsa_port_bridge_num_get(dp));
+	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, info.bridge);
 
 	err = switchdev_bridge_port_offload(brport_dev, dev, dp,
 					    &dsa_slave_switchdev_notifier,
@@ -415,12 +421,11 @@ void dsa_port_pre_bridge_leave(struct dsa_port *dp, struct net_device *br)
 
 void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 {
-	unsigned int bridge_num = dsa_port_bridge_num_get(dp);
 	struct dsa_notifier_bridge_info info = {
 		.tree_index = dp->ds->dst->index,
 		.sw_index = dp->ds->index,
 		.port = dp->index,
-		.br = br,
+		.bridge = *dp->bridge,
 	};
 	int err;
 
@@ -429,7 +434,7 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 	 */
 	dsa_port_bridge_destroy(dp, br);
 
-	dsa_port_bridge_tx_fwd_unoffload(dp, br, bridge_num);
+	dsa_port_bridge_tx_fwd_unoffload(dp, info.bridge);
 
 	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
 	if (err)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 98fd5e972d28..a2918232bad6 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1564,7 +1564,7 @@ static void dsa_bridge_mtu_normalization(struct dsa_port *dp)
 	if (!dp->ds->mtu_enforcement_ingress)
 		return;
 
-	if (!dp->bridge_dev)
+	if (!dp->bridge)
 		return;
 
 	INIT_LIST_HEAD(&hw_port_list);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 7993192fe769..cd0630dd5417 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -95,7 +95,7 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
 		if (!ds->ops->port_bridge_join)
 			return -EOPNOTSUPP;
 
-		err = ds->ops->port_bridge_join(ds, info->port, info->br);
+		err = ds->ops->port_bridge_join(ds, info->port, info->bridge);
 		if (err)
 			return err;
 	}
@@ -104,7 +104,7 @@ 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->br);
+						     info->port, info->bridge);
 		if (err)
 			return err;
 	}
@@ -124,19 +124,20 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 
 	if (dst->index == info->tree_index && ds->index == info->sw_index &&
 	    ds->ops->port_bridge_leave)
-		ds->ops->port_bridge_leave(ds, info->port, info->br);
+		ds->ops->port_bridge_leave(ds, info->port, info->bridge);
 
 	if ((dst->index != info->tree_index || ds->index != info->sw_index) &&
 	    ds->ops->crosschip_bridge_leave)
 		ds->ops->crosschip_bridge_leave(ds, info->tree_index,
 						info->sw_index, info->port,
-						info->br);
+						info->bridge);
 
-	if (ds->needs_standalone_vlan_filtering && !br_vlan_enabled(info->br)) {
+	if (ds->needs_standalone_vlan_filtering &&
+	    !br_vlan_enabled(info->bridge.dev)) {
 		change_vlan_filtering = true;
 		vlan_filtering = true;
 	} else if (!ds->needs_standalone_vlan_filtering &&
-		   br_vlan_enabled(info->br)) {
+		   br_vlan_enabled(info->bridge.dev)) {
 		change_vlan_filtering = true;
 		vlan_filtering = false;
 	}
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index e9d5e566973c..27712a81c967 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -337,7 +337,7 @@ dsa_port_tag_8021q_bridge_match(struct dsa_port *dp,
 		return false;
 
 	if (dsa_port_is_user(dp))
-		return dsa_port_bridge_dev_get(dp) == info->br;
+		return dsa_port_offloads_bridge(dp, &info->bridge);
 
 	return false;
 }
@@ -410,10 +410,9 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
 }
 
 int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
-					struct net_device *br,
-					unsigned int bridge_num)
+					struct dsa_bridge bridge)
 {
-	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
 
 	return dsa_port_tag_8021q_vlan_add(dsa_to_port(ds, port), tx_vid,
 					   true);
@@ -421,10 +420,9 @@ int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
 EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
 
 void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
-					   struct net_device *br,
-					   unsigned int bridge_num)
+					   struct dsa_bridge bridge)
 {
-	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
 
 	dsa_port_tag_8021q_vlan_del(dsa_to_port(ds, port), tx_vid, true);
 }
-- 
2.25.1


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

* [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload
  2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
                   ` (4 preceding siblings ...)
  2021-10-26 16:26 ` [RFC PATCH net-next 5/6] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
@ 2021-10-26 16:26 ` Vladimir Oltean
  2021-11-11 13:23   ` Alvin Šipraga
  2021-11-19 15:38   ` Tobias Waldekranz
  2021-11-02 10:07 ` [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
  6 siblings, 2 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-10-26 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang, Alvin Šipraga

We don't really need new switch API for these, and with new switches
which intend to add support for this feature, it will become cumbersome
to maintain.

Simply pass a boolean argument to ->port_bridge_join which the driver
must set to true if it implements the TX forwarding offload feature.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c       |  3 +-
 drivers/net/dsa/b53/b53_priv.h         |  3 +-
 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 |  2 +-
 drivers/net/dsa/mt7530.c               |  2 +-
 drivers/net/dsa/mv88e6xxx/chip.c       | 71 ++++++++++++--------------
 drivers/net/dsa/ocelot/felix.c         |  2 +-
 drivers/net/dsa/qca8k.c                |  3 +-
 drivers/net/dsa/rtl8366rb.c            |  3 +-
 drivers/net/dsa/sja1105/sja1105_main.c | 22 ++++++--
 drivers/net/dsa/xrs700x/xrs700x.c      |  2 +-
 include/net/dsa.h                      | 10 ++--
 net/dsa/dsa_priv.h                     |  1 +
 net/dsa/port.c                         | 39 ++------------
 net/dsa/switch.c                       |  3 +-
 19 files changed, 81 insertions(+), 100 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 4e41b1a63108..3867f3d4545f 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1860,7 +1860,8 @@ 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)
+int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
+		bool *tx_fwd_offload)
 {
 	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 ee17f8b516ca..b41dc8ac2ca8 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -324,7 +324,8 @@ void b53_get_strings(struct dsa_switch *ds, int port, u32 stringset,
 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);
+int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
+		bool *tx_fwd_offload);
 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 70db3a9aa355..33daaf10c488 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -167,7 +167,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)
+				     struct dsa_bridge bridge,
+				     bool *tx_fwd_offload)
 {
 	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 17c2d13670a9..53ab5f8ae92e 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -674,7 +674,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)
+				      struct dsa_bridge bridge,
+				      bool *tx_fwd_offload)
 {
 	struct hellcreek *hellcreek = ds->priv;
 
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 29d909484275..d55784d19fa4 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1103,7 +1103,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)
+				    struct dsa_bridge bridge,
+				    bool *tx_fwd_offload)
 {
 	struct lan9303 *chip = ds->priv;
 
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 6b9b9c712e58..6b5bef7f2a59 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1146,7 +1146,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)
+				  struct dsa_bridge bridge,
+				  bool *tx_fwd_offload)
 {
 	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 23784d14a5da..1d13aa56e7d1 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -173,7 +173,8 @@ void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
 EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
 
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
-			 struct dsa_bridge bridge)
+			 struct dsa_bridge bridge,
+			 bool *tx_fwd_offload)
 {
 	struct ksz_device *dev = ds->priv;
 
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index df953c2bc56e..b444df218c2d 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -159,7 +159,7 @@ 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);
+			 struct dsa_bridge bridge, bool *tx_fwd_offload);
 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 85cc5aca7f96..dcd1a3932cfb 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1186,7 +1186,7 @@ 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)
+			struct dsa_bridge bridge, bool *tx_fwd_offload)
 {
 	struct mt7530_priv *priv = ds->priv;
 	u32 port_bitmap = BIT(MT7530_CPU_PORT);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 73613a667f6c..0aac71dece29 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2448,8 +2448,27 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
+/* Treat the software bridge as a virtual single-port switch behind the
+ * CPU and map in the PVT. First dst->last_switch elements are taken by
+ * physical switches, so start from beyond that range.
+ */
+static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
+					       unsigned int bridge_num)
+{
+	u8 dev = bridge_num + ds->dst->last_switch;
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	mv88e6xxx_reg_lock(chip);
+	err = mv88e6xxx_pvt_map(chip, dev, 0);
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
 static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
-				      struct dsa_bridge bridge)
+				      struct dsa_bridge bridge,
+				      bool *tx_fwd_offload)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
@@ -2464,6 +2483,14 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 	if (err)
 		goto unlock;
 
+	if (mv88e6xxx_has_pvt(chip)) {
+		err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
+		if (err)
+			goto unlock;
+
+		*tx_fwd_offload = true;
+	}
+
 unlock:
 	mv88e6xxx_reg_unlock(chip);
 
@@ -2478,6 +2505,10 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
 
 	mv88e6xxx_reg_lock(chip);
 
+	if (bridge.tx_fwd_offload &&
+	    mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num))
+		dev_err(ds->dev, "failed to remap cross-chip Port VLAN\n");
+
 	if (mv88e6xxx_bridge_map(chip, bridge) ||
 	    mv88e6xxx_port_vlan_map(chip, port))
 		dev_err(ds->dev, "failed to remap in-chip Port VLAN\n");
@@ -2523,42 +2554,6 @@ static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
 	mv88e6xxx_reg_unlock(chip);
 }
 
-/* Treat the software bridge as a virtual single-port switch behind the
- * CPU and map in the PVT. First dst->last_switch elements are taken by
- * physical switches, so start from beyond that range.
- */
-static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
-					       unsigned int bridge_num)
-{
-	u8 dev = bridge_num + ds->dst->last_switch;
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int err;
-
-	mv88e6xxx_reg_lock(chip);
-	err = mv88e6xxx_pvt_map(chip, dev, 0);
-	mv88e6xxx_reg_unlock(chip);
-
-	return err;
-}
-
-static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
-					   struct dsa_bridge bridge)
-{
-	return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
-}
-
-static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
-					      struct dsa_bridge bridge)
-{
-	int err;
-
-	err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
-	if (err) {
-		dev_err(ds->dev, "failed to remap cross-chip Port VLAN: %pe\n",
-			ERR_PTR(err));
-	}
-}
-
 static int mv88e6xxx_software_reset(struct mv88e6xxx_chip *chip)
 {
 	if (chip->info->ops->reset)
@@ -6278,8 +6273,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.crosschip_lag_change	= mv88e6xxx_crosschip_lag_change,
 	.crosschip_lag_join	= mv88e6xxx_crosschip_lag_join,
 	.crosschip_lag_leave	= mv88e6xxx_crosschip_lag_leave,
-	.port_bridge_tx_fwd_offload = mv88e6xxx_bridge_tx_fwd_offload,
-	.port_bridge_tx_fwd_unoffload = mv88e6xxx_bridge_tx_fwd_unoffload,
 };
 
 static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 8cf82fa91a6b..001213891b91 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -698,7 +698,7 @@ 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)
+			     struct dsa_bridge bridge, bool *tx_fwd_offload)
 {
 	struct ocelot *ocelot = ds->priv;
 
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 3ae10f22ada9..390dad77f062 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1729,7 +1729,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)
+				  struct dsa_bridge bridge,
+				  bool *tx_fwd_offload)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
 	int port_mask, cpu_port;
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index fac2333a3f5e..ecc19bd5115f 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -1186,7 +1186,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)
+			   struct dsa_bridge bridge,
+			   bool *tx_fwd_offload)
 {
 	struct realtek_smi *smi = 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 24584fe2e760..cefde41ce8d6 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2074,14 +2074,30 @@ 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)
+			       struct dsa_bridge bridge,
+			       bool *tx_fwd_offload)
 {
-	return sja1105_bridge_member(ds, port, bridge, true);
+	int rc;
+
+	rc = sja1105_bridge_member(ds, port, bridge, true);
+	if (rc)
+		return rc;
+
+	rc = dsa_tag_8021q_bridge_tx_fwd_offload(ds, port, bridge);
+	if (rc) {
+		sja1105_bridge_member(ds, port, bridge, false);
+		return rc;
+	}
+
+	*tx_fwd_offload = true;
+
+	return 0;
 }
 
 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);
 	sja1105_bridge_member(ds, port, bridge, false);
 }
 
@@ -3230,8 +3246,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
 	.tag_8021q_vlan_add	= sja1105_dsa_8021q_vlan_add,
 	.tag_8021q_vlan_del	= sja1105_dsa_8021q_vlan_del,
 	.port_prechangeupper	= sja1105_prechangeupper,
-	.port_bridge_tx_fwd_offload = dsa_tag_8021q_bridge_tx_fwd_offload,
-	.port_bridge_tx_fwd_unoffload = dsa_tag_8021q_bridge_tx_fwd_unoffload,
 };
 
 static const struct of_device_id sja1105_dt_ids[];
diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index ebb55dfd9c4e..35fa19ddaf19 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -540,7 +540,7 @@ 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)
+			       struct dsa_bridge bridge, bool *tx_fwd_offload)
 {
 	return xrs700x_bridge_common(ds, port, bridge, true);
 }
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 5e83c64bc96a..94dc8b5ee9c8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -222,6 +222,7 @@ struct dsa_mall_tc_entry {
 struct dsa_bridge {
 	struct net_device *dev;
 	unsigned int num;
+	bool tx_fwd_offload;
 	refcount_t refcount;
 };
 
@@ -814,15 +815,10 @@ 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);
+				    struct dsa_bridge bridge,
+				    bool *tx_fwd_offload);
 	void	(*port_bridge_leave)(struct dsa_switch *ds, int port,
 				     struct dsa_bridge bridge);
-	/* Called right after .port_bridge_join() */
-	int	(*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
-					      struct dsa_bridge bridge);
-	/* Called right before .port_bridge_leave() */
-	void	(*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
-						struct dsa_bridge bridge);
 	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
 				      u8 state);
 	void	(*port_fast_age)(struct dsa_switch *ds, int port);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 489712ead08f..5abe6864967a 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -56,6 +56,7 @@ struct dsa_notifier_bridge_info {
 	int tree_index;
 	int sw_index;
 	int port;
+	bool tx_fwd_offload;
 };
 
 /* DSA_NOTIFIER_FDB_* */
diff --git a/net/dsa/port.c b/net/dsa/port.c
index b2e66994f72c..b1113788bf9b 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -270,37 +270,6 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
 	 */
 }
 
-static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
-					     struct dsa_bridge bridge)
-{
-	struct dsa_switch *ds = dp->ds;
-
-	/* No bridge TX forwarding offload => do nothing */
-	if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge.num)
-		return;
-
-	/* Notify the chips only once the offload has been deactivated, so
-	 * that they can update their configuration accordingly.
-	 */
-	ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge);
-}
-
-static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
-					   struct dsa_bridge bridge)
-{
-	struct dsa_switch *ds = dp->ds;
-	int err;
-
-	/* FDB isolation is required for TX forwarding offload */
-	if (!ds->ops->port_bridge_tx_fwd_offload || !bridge.num)
-		return false;
-
-	/* Notify the driver */
-	err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge);
-
-	return err ? false : true;
-}
-
 static int dsa_port_bridge_create(struct dsa_port *dp,
 				  struct net_device *br,
 				  struct netlink_ext_ack *extack)
@@ -361,7 +330,6 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 	};
 	struct net_device *dev = dp->slave;
 	struct net_device *brport_dev;
-	bool tx_fwd_offload;
 	int err;
 
 	/* Here the interface is already bridged. Reflect the current
@@ -378,12 +346,13 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 	if (err)
 		goto out_rollback;
 
-	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, info.bridge);
+	/* Drivers which support bridge TX forwarding should set this */
+	dp->bridge->tx_fwd_offload = info.tx_fwd_offload;
 
 	err = switchdev_bridge_port_offload(brport_dev, dev, dp,
 					    &dsa_slave_switchdev_notifier,
 					    &dsa_slave_switchdev_blocking_notifier,
-					    tx_fwd_offload, extack);
+					    dp->bridge->tx_fwd_offload, extack);
 	if (err)
 		goto out_rollback_unbridge;
 
@@ -434,8 +403,6 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 	 */
 	dsa_port_bridge_destroy(dp, br);
 
-	dsa_port_bridge_tx_fwd_unoffload(dp, info.bridge);
-
 	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
 	if (err)
 		dev_err(dp->ds->dev,
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index cd0630dd5417..9c92edd96961 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -95,7 +95,8 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
 		if (!ds->ops->port_bridge_join)
 			return -EOPNOTSUPP;
 
-		err = ds->ops->port_bridge_join(ds, info->port, info->bridge);
+		err = ds->ops->port_bridge_join(ds, info->port, info->bridge,
+						&info->tx_fwd_offload);
 		if (err)
 			return err;
 	}
-- 
2.25.1


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

* Re: [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API
  2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
                   ` (5 preceding siblings ...)
  2021-10-26 16:26 ` [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
@ 2021-11-02 10:07 ` Vladimir Oltean
  2021-11-04 11:39   ` Tobias Waldekranz
  6 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2021-11-02 10:07 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang, Alvin Šipraga

On Tue, Oct 26, 2021 at 07:26:19PM +0300, Vladimir Oltean wrote:
> This change set replaces struct net_device *dp->bridge_dev with a
> struct dsa_bridge *dp->bridge that contains some extra information about
> that bridge, like a unique number kept by DSA.
> 
> Up until now we computed that number only with the bridge TX forwarding
> offload feature, but it will be needed for other features too, like for
> isolation of FDB entries belonging to different bridges. Hardware
> implementations vary, but one common pattern seems to be the presence of
> a FID field which can be associated with that bridge number kept by DSA.
> The idea was outlined here:
> https://patchwork.kernel.org/project/netdevbpf/patch/20210818120150.892647-16-vladimir.oltean@nxp.com/
> (the difference being that with this new proposal, drivers would not
> need to call dsa_bridge_num_find, instead the bridge_num would be part
> of the struct dsa_bridge :: num passed as argument).
> 
> No functional change intended.
> 
> Vladimir Oltean (6):
>   net: dsa: make dp->bridge_num one-based
>   net: dsa: assign a bridge number even without TX forwarding offload
>   net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers
>   net: dsa: rename dsa_port_offloads_bridge to
>     dsa_port_offloads_bridge_dev
>   net: dsa: keep the bridge_dev and bridge_num as part of the same
>     structure
>   net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload
> 
>  drivers/net/dsa/b53/b53_common.c       |   9 +-
>  drivers/net/dsa/b53/b53_priv.h         |   5 +-
>  drivers/net/dsa/dsa_loop.c             |   9 +-
>  drivers/net/dsa/hirschmann/hellcreek.c |   5 +-
>  drivers/net/dsa/lan9303-core.c         |   7 +-
>  drivers/net/dsa/lantiq_gswip.c         |  25 +++--
>  drivers/net/dsa/microchip/ksz_common.c |   5 +-
>  drivers/net/dsa/microchip/ksz_common.h |   4 +-
>  drivers/net/dsa/mt7530.c               |  18 +--
>  drivers/net/dsa/mv88e6xxx/chip.c       | 145 ++++++++++++-------------
>  drivers/net/dsa/ocelot/felix.c         |   8 +-
>  drivers/net/dsa/qca8k.c                |  13 ++-
>  drivers/net/dsa/rtl8366rb.c            |   9 +-
>  drivers/net/dsa/sja1105/sja1105_main.c |  40 +++++--
>  drivers/net/dsa/xrs700x/xrs700x.c      |  10 +-
>  include/linux/dsa/8021q.h              |   9 +-
>  include/net/dsa.h                      | 102 +++++++++++++----
>  net/dsa/dsa2.c                         |  57 ++++++----
>  net/dsa/dsa_priv.h                     |  59 ++--------
>  net/dsa/port.c                         | 123 +++++++++++----------
>  net/dsa/slave.c                        |  34 +++---
>  net/dsa/switch.c                       |  20 ++--
>  net/dsa/tag_8021q.c                    |  20 ++--
>  net/dsa/tag_dsa.c                      |   5 +-
>  net/dsa/tag_ocelot.c                   |   2 +-
>  net/dsa/tag_sja1105.c                  |  11 +-
>  26 files changed, 414 insertions(+), 340 deletions(-)
> 
> -- 
> 2.25.1
>

Any feedback?

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

* Re: [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API
  2021-11-02 10:07 ` [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
@ 2021-11-04 11:39   ` Tobias Waldekranz
  0 siblings, 0 replies; 17+ messages in thread
From: Tobias Waldekranz @ 2021-11-04 11:39 UTC (permalink / raw)
  To: Vladimir Oltean, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, DENG Qingfang,
	Alvin Šipraga

On Tue, Nov 02, 2021 at 10:07, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> On Tue, Oct 26, 2021 at 07:26:19PM +0300, Vladimir Oltean wrote:
>> This change set replaces struct net_device *dp->bridge_dev with a
>> struct dsa_bridge *dp->bridge that contains some extra information about
>> that bridge, like a unique number kept by DSA.
>> 
>> Up until now we computed that number only with the bridge TX forwarding
>> offload feature, but it will be needed for other features too, like for
>> isolation of FDB entries belonging to different bridges. Hardware
>> implementations vary, but one common pattern seems to be the presence of
>> a FID field which can be associated with that bridge number kept by DSA.
>> The idea was outlined here:
>> https://patchwork.kernel.org/project/netdevbpf/patch/20210818120150.892647-16-vladimir.oltean@nxp.com/
>> (the difference being that with this new proposal, drivers would not
>> need to call dsa_bridge_num_find, instead the bridge_num would be part
>> of the struct dsa_bridge :: num passed as argument).
>> 
>> No functional change intended.
>
> Any feedback?

Sorry Vladimir, I've been on holiday. I will try to review this
ASAP. Based on a quick look, I like it.

I actually had some cross-chip fixes w.r.t. forward offloading queued
up, and this series should make that cleaner as well.

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

* Re: [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based
  2021-10-26 16:26 ` [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based Vladimir Oltean
@ 2021-11-11 12:24   ` Alvin Šipraga
  2021-11-11 12:45     ` Vladimir Oltean
  0 siblings, 1 reply; 17+ messages in thread
From: Alvin Šipraga @ 2021-11-11 12:24 UTC (permalink / raw)
  To: Vladimir Oltean, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang

On 10/26/21 18:26, Vladimir Oltean wrote:
> I have seen too many bugs already due to the fact that we must encode an
> invalid dp->bridge_num as a negative value, because the natural tendency
> is to check that invalid value using (!dp->bridge_num). Latest example
> can be seen in commit 1bec0f05062c ("net: dsa: fix bridge_num not
> getting cleared after ports leaving the bridge").
> 
> Convert the existing users to assume that dp->bridge_num == 0 is the
> encoding for invalid, and valid bridge numbers start from 1.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Small remark inline.

>   drivers/net/dsa/mv88e6xxx/chip.c | 12 ++++++------
>   include/linux/dsa/8021q.h        |  6 +++---
>   include/net/dsa.h                |  6 +++---
>   net/dsa/dsa2.c                   | 24 ++++++++++++------------
>   net/dsa/dsa_priv.h               |  5 +++--
>   net/dsa/port.c                   | 11 ++++++-----
>   net/dsa/tag_8021q.c              | 12 +++++++-----
>   net/dsa/tag_dsa.c                |  2 +-
>   8 files changed, 41 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 14c678a9e41b..0d0ccb7f8ccd 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1247,10 +1247,10 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
>   	/* dev is a virtual bridge */
>   	} else {
>   		list_for_each_entry(dp, &dst->ports, list) {
> -			if (dp->bridge_num < 0)
> +			if (!dp->bridge_num)
>   				continue;
>   
> -			if (dp->bridge_num + 1 + dst->last_switch != dev)
> +			if (dp->bridge_num + dst->last_switch != dev)
>   				continue;
>   
>   			br = dp->bridge_dev;
> @@ -2524,9 +2524,9 @@ static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
>    * physical switches, so start from beyond that range.
>    */
>   static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
> -					       int bridge_num)
> +					       unsigned int bridge_num)
>   {
> -	u8 dev = bridge_num + ds->dst->last_switch + 1;
> +	u8 dev = bridge_num + ds->dst->last_switch;
>   	struct mv88e6xxx_chip *chip = ds->priv;
>   	int err;
>   
> @@ -2539,14 +2539,14 @@ static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
>   
>   static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
>   					   struct net_device *br,
> -					   int bridge_num)
> +					   unsigned int bridge_num)
>   {
>   	return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
>   }
>   
>   static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
>   					      struct net_device *br,
> -					      int bridge_num)
> +					      unsigned int bridge_num)
>   {
>   	int err;
>   
> diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
> index 254b165f2b44..0af4371fbebb 100644
> --- a/include/linux/dsa/8021q.h
> +++ b/include/linux/dsa/8021q.h
> @@ -38,13 +38,13 @@ void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
>   
>   int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
>   					struct net_device *br,
> -					int bridge_num);
> +					unsigned int bridge_num);
>   
>   void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
>   					   struct net_device *br,
> -					   int bridge_num);
> +					   unsigned int bridge_num);
>   
> -u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num);
> +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/include/net/dsa.h b/include/net/dsa.h
> index badd214f7470..56a90f05df49 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -257,7 +257,7 @@ struct dsa_port {
>   	bool			learning;
>   	u8			stp_state;
>   	struct net_device	*bridge_dev;
> -	int			bridge_num;
> +	unsigned int		bridge_num;
>   	struct devlink_port	devlink_port;
>   	bool			devlink_port_setup;
>   	struct phylink		*pl;
> @@ -752,11 +752,11 @@ struct dsa_switch_ops {
>   	/* Called right after .port_bridge_join() */
>   	int	(*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
>   					      struct net_device *bridge,
> -					      int bridge_num);
> +					      unsigned int bridge_num);
>   	/* Called right before .port_bridge_leave() */
>   	void	(*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
>   						struct net_device *bridge,
> -						int bridge_num);
> +						unsigned int bridge_num);
>   	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
>   				      u8 state);
>   	void	(*port_fast_age)(struct dsa_switch *ds, int port);
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 826957b6442b..9606e56710a5 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -141,23 +141,23 @@ static int dsa_bridge_num_find(const struct net_device *bridge_dev)
>   	 */
>   	list_for_each_entry(dst, &dsa_tree_list, list)
>   		list_for_each_entry(dp, &dst->ports, list)
> -			if (dp->bridge_dev == bridge_dev &&
> -			    dp->bridge_num != -1)
> +			if (dp->bridge_dev == bridge_dev && dp->bridge_num)
>   				return dp->bridge_num;
>   
> -	return -1;
> +	return 0;
>   }
>   
> -int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
> +unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
>   {
> -	int bridge_num = dsa_bridge_num_find(bridge_dev);
> +	unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
>   
> -	if (bridge_num < 0) {
> +	if (!bridge_num) {
>   		/* First port that offloads TX forwarding for this bridge */

Perhaps you want to update this comment in patch 2/6, since bridge_num 
is no longer just about TX forwarding offload.

> -		bridge_num = find_first_zero_bit(&dsa_fwd_offloading_bridges,
> -						 DSA_MAX_NUM_OFFLOADING_BRIDGES);
> +		bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
> +						DSA_MAX_NUM_OFFLOADING_BRIDGES,
> +						1);
>   		if (bridge_num >= max)
> -			return -1;
> +			return 0;
>   
>   		set_bit(bridge_num, &dsa_fwd_offloading_bridges);
>   	}
> @@ -165,12 +165,13 @@ int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
>   	return bridge_num;
>   }
>   
> -void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num)
> +void dsa_bridge_num_put(const struct net_device *bridge_dev,
> +			unsigned 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 (dsa_bridge_num_find(bridge_dev) < 0)
> +	if (!dsa_bridge_num_find(bridge_dev))
>   		clear_bit(bridge_num, &dsa_fwd_offloading_bridges);
>   }
>   
> @@ -1184,7 +1185,6 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
>   
>   	dp->ds = ds;
>   	dp->index = index;
> -	dp->bridge_num = -1;
>   
>   	INIT_LIST_HEAD(&dp->list);
>   	list_add_tail(&dp->list, &dst->ports);
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index a5c9bc7b66c6..2a64c41813bf 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -546,8 +546,9 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
>   			      struct net_device *master,
>   			      const struct dsa_device_ops *tag_ops,
>   			      const struct dsa_device_ops *old_tag_ops);
> -int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
> -void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num);
> +unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
> +void dsa_bridge_num_put(const struct net_device *bridge_dev,
> +			unsigned int bridge_num);
>   
>   /* tag_8021q.c */
>   int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index c0e630f7f0bd..1772817a1214 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -273,14 +273,14 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
>   static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
>   					     struct net_device *bridge_dev)
>   {
> -	int bridge_num = dp->bridge_num;
> +	unsigned int bridge_num = dp->bridge_num;
>   	struct dsa_switch *ds = dp->ds;
>   
>   	/* No bridge TX forwarding offload => do nothing */
> -	if (!ds->ops->port_bridge_tx_fwd_unoffload || dp->bridge_num == -1)
> +	if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num)
>   		return;
>   
> -	dp->bridge_num = -1;
> +	dp->bridge_num = 0;
>   
>   	dsa_bridge_num_put(bridge_dev, bridge_num);
>   
> @@ -295,14 +295,15 @@ static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
>   					   struct net_device *bridge_dev)
>   {
>   	struct dsa_switch *ds = dp->ds;
> -	int bridge_num, err;
> +	unsigned int bridge_num;
> +	int err;
>   
>   	if (!ds->ops->port_bridge_tx_fwd_offload)
>   		return false;
>   
>   	bridge_num = dsa_bridge_num_get(bridge_dev,
>   					ds->num_fwd_offloading_bridges);
> -	if (bridge_num < 0)
> +	if (!bridge_num)
>   		return false;
>   
>   	dp->bridge_num = bridge_num;
> diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
> index 72cac2c0af7b..df59f16436a5 100644
> --- a/net/dsa/tag_8021q.c
> +++ b/net/dsa/tag_8021q.c
> @@ -67,10 +67,12 @@
>   #define DSA_8021Q_PORT(x)		(((x) << DSA_8021Q_PORT_SHIFT) & \
>   						 DSA_8021Q_PORT_MASK)
>   
> -u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num)
> +u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num)
>   {
> -	/* The VBID value of 0 is reserved for precise TX */
> -	return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num + 1);
> +	/* 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);
>   }
>   EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid);
>   
> @@ -409,7 +411,7 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
>   
>   int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
>   					struct net_device *br,
> -					int bridge_num)
> +					unsigned int bridge_num)
>   {
>   	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
>   
> @@ -420,7 +422,7 @@ EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
>   
>   void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
>   					   struct net_device *br,
> -					   int bridge_num)
> +					   unsigned int bridge_num)
>   {
>   	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
>   
> diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
> index b3da4b2ea11c..a7d70ae7cc97 100644
> --- a/net/dsa/tag_dsa.c
> +++ b/net/dsa/tag_dsa.c
> @@ -140,7 +140,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
>   		 * packets on behalf of a virtual switch device with an index
>   		 * past the physical switches.
>   		 */
> -		tag_dev = dst->last_switch + 1 + dp->bridge_num;
> +		tag_dev = dst->last_switch + dp->bridge_num;
>   		tag_port = 0;
>   	} else {
>   		cmd = DSA_CMD_FROM_CPU;
> 


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

* Re: [RFC PATCH net-next 2/6] net: dsa: assign a bridge number even without TX forwarding offload
  2021-10-26 16:26 ` [RFC PATCH net-next 2/6] net: dsa: assign a bridge number even without TX forwarding offload Vladimir Oltean
@ 2021-11-11 12:27   ` Alvin Šipraga
  0 siblings, 0 replies; 17+ messages in thread
From: Alvin Šipraga @ 2021-11-11 12:27 UTC (permalink / raw)
  To: Vladimir Oltean, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang

On 10/26/21 18:26, Vladimir Oltean wrote:
> The service where DSA assigns a unique bridge number for each forwarding
> domain is useful even for drivers which do not implement the TX
> forwarding offload feature.
> 
> For example, drivers might use the dp->bridge_num for FDB isolation.
> 
> So rename ds->num_fwd_offloading_bridges to ds->max_num_bridges, and
> calculate a unique bridge_num for all drivers that set this value.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Thanks for this, it makes FDB isolation much more straightforward to handle.

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

>   drivers/net/dsa/mv88e6xxx/chip.c       |  4 +-
>   drivers/net/dsa/sja1105/sja1105_main.c |  2 +-
>   include/net/dsa.h                      | 10 ++--
>   net/dsa/port.c                         | 81 +++++++++++++++++---------
>   4 files changed, 63 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 0d0ccb7f8ccd..ae09cbc2f42f 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -3183,8 +3183,8 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
>   	 * time.
>   	 */
>   	if (mv88e6xxx_has_pvt(chip))
> -		ds->num_fwd_offloading_bridges = MV88E6XXX_MAX_PVT_SWITCHES -
> -						 ds->dst->last_switch - 1;
> +		ds->max_num_bridges = MV88E6XXX_MAX_PVT_SWITCHES -
> +				      ds->dst->last_switch - 1;
>   
>   	mv88e6xxx_reg_lock(chip);
>   
> diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> index c343effe2e96..355b56cf94d8 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -3139,7 +3139,7 @@ static int sja1105_setup(struct dsa_switch *ds)
>   	ds->vlan_filtering_is_global = true;
>   	ds->untag_bridge_pvid = true;
>   	/* tag_8021q has 3 bits for the VBID, and the value 0 is reserved */
> -	ds->num_fwd_offloading_bridges = 7;
> +	ds->max_num_bridges = 7;
>   
>   	/* Advertise the 8 egress queues */
>   	ds->num_tx_queues = SJA1105_NUM_TC;
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 56a90f05df49..970bc89a0062 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -413,12 +413,12 @@ struct dsa_switch {
>   	 */
>   	unsigned int		num_lag_ids;
>   
> -	/* Drivers that support bridge forwarding offload should set this to
> -	 * the maximum number of bridges spanning the same switch tree (or all
> -	 * trees, in the case of cross-tree bridging support) that can be
> -	 * offloaded.
> +	/* Drivers that support bridge forwarding offload or FDB isolation
> +	 * should set this to the maximum number of bridges spanning the same
> +	 * switch tree (or all trees, in the case of cross-tree bridging
> +	 * support) that can be offloaded.
>   	 */
> -	unsigned int		num_fwd_offloading_bridges;
> +	unsigned int		max_num_bridges;
>   
>   	size_t num_ports;
>   };
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 1772817a1214..b7867746af15 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -271,19 +271,15 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
>   }
>   
>   static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
> -					     struct net_device *bridge_dev)
> +					     struct net_device *bridge_dev,
> +					     unsigned int bridge_num)
>   {
> -	unsigned int bridge_num = dp->bridge_num;
>   	struct dsa_switch *ds = dp->ds;
>   
>   	/* No bridge TX forwarding offload => do nothing */
> -	if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num)
> +	if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge_num)
>   		return;
>   
> -	dp->bridge_num = 0;
> -
> -	dsa_bridge_num_put(bridge_dev, bridge_num);
> -
>   	/* Notify the chips only once the offload has been deactivated, so
>   	 * that they can update their configuration accordingly.
>   	 */
> @@ -292,31 +288,60 @@ static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
>   }
>   
>   static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
> -					   struct net_device *bridge_dev)
> +					   struct net_device *bridge_dev,
> +					   unsigned int bridge_num)
>   {
>   	struct dsa_switch *ds = dp->ds;
> -	unsigned int bridge_num;
>   	int err;
>   
> -	if (!ds->ops->port_bridge_tx_fwd_offload)
> -		return false;
> -
> -	bridge_num = dsa_bridge_num_get(bridge_dev,
> -					ds->num_fwd_offloading_bridges);
> -	if (!bridge_num)
> +	/* FDB isolation is required for TX forwarding offload */
> +	if (!ds->ops->port_bridge_tx_fwd_offload || !bridge_num)
>   		return false;
>   
> -	dp->bridge_num = bridge_num;
> -
>   	/* Notify the driver */
>   	err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge_dev,
>   						  bridge_num);
> -	if (err) {
> -		dsa_port_bridge_tx_fwd_unoffload(dp, bridge_dev);
> -		return false;
> +
> +	return err ? false : true;
> +}
> +
> +static int dsa_port_bridge_create(struct dsa_port *dp,
> +				  struct net_device *br,
> +				  struct netlink_ext_ack *extack)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +	unsigned int bridge_num;
> +
> +	dp->bridge_dev = br;
> +
> +	if (!ds->max_num_bridges)
> +		return 0;
> +
> +	bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges);
> +	if (!bridge_num) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Range of offloadable bridges exceeded");
> +		return -EOPNOTSUPP;
>   	}
>   
> -	return true;
> +	dp->bridge_num = bridge_num;
> +
> +	return 0;
> +}
> +
> +static void dsa_port_bridge_destroy(struct dsa_port *dp,
> +				    const struct net_device *br)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +
> +	dp->bridge_dev = NULL;
> +
> +	if (ds->max_num_bridges) {
> +		int bridge_num = dp->bridge_num;
> +
> +		dp->bridge_num = 0;
> +		dsa_bridge_num_put(br, bridge_num);
> +	}
>   }
>   
>   int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
> @@ -336,7 +361,9 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
>   	/* Here the interface is already bridged. Reflect the current
>   	 * configuration so that drivers can program their chips accordingly.
>   	 */
> -	dp->bridge_dev = br;
> +	err = dsa_port_bridge_create(dp, br, extack);
> +	if (err)
> +		return err;
>   
>   	brport_dev = dsa_port_to_bridge_port(dp);
>   
> @@ -344,7 +371,8 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
>   	if (err)
>   		goto out_rollback;
>   
> -	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br);
> +	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
> +							dp->bridge_num);
>   
>   	err = switchdev_bridge_port_offload(brport_dev, dev, dp,
>   					    &dsa_slave_switchdev_notifier,
> @@ -366,7 +394,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
>   out_rollback_unbridge:
>   	dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
>   out_rollback:
> -	dp->bridge_dev = NULL;
> +	dsa_port_bridge_destroy(dp, br);
>   	return err;
>   }
>   
> @@ -393,14 +421,15 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
>   		.port = dp->index,
>   		.br = br,
>   	};
> +	int bridge_num = dp->bridge_num;
>   	int err;
>   
>   	/* Here the port is already unbridged. Reflect the current configuration
>   	 * so that drivers can program their chips accordingly.
>   	 */
> -	dp->bridge_dev = NULL;
> +	dsa_port_bridge_destroy(dp, br);
>   
> -	dsa_port_bridge_tx_fwd_unoffload(dp, br);
> +	dsa_port_bridge_tx_fwd_unoffload(dp, br, bridge_num);
>   
>   	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
>   	if (err)
> 


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

* Re: [RFC PATCH net-next 4/6] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev
  2021-10-26 16:26 ` [RFC PATCH net-next 4/6] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev Vladimir Oltean
@ 2021-11-11 12:33   ` Alvin Šipraga
  0 siblings, 0 replies; 17+ messages in thread
From: Alvin Šipraga @ 2021-11-11 12:33 UTC (permalink / raw)
  To: Vladimir Oltean, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang

On 10/26/21 18:26, Vladimir Oltean wrote:
> Currently the majority of dsa_port_bridge_dev_get() calls in drivers is
> just to check whether a port is under the bridge device provided as
> argument by the DSA API.
> 
> We'd like to change that DSA API so that a more complex structure is
> provided as argument. To keep things more generic, and considering that
> the new complex structure will be provided by value and not by
> reference, direct comparisons between dp->bridge and the provided bridge
> will be broken. The generic way to do the checking would simply be to
> do something like dsa_port_offloads_bridge(dp, &bridge).
> 
> But there's a problem, we already have a function named that way, which
> actually takes a bridge_dev net_device as argument. Rename it so that we
> can use dsa_port_offloads_bridge for something else.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

>   net/dsa/dsa_priv.h | 12 +++++++-----
>   net/dsa/slave.c    | 18 +++++++++---------
>   2 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 62b719929ef4..daba10adbd22 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -272,8 +272,9 @@ static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
>   	return dsa_port_to_bridge_port(dp) == dev;
>   }
>   
> -static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
> -					    const struct net_device *bridge_dev)
> +static inline bool
> +dsa_port_offloads_bridge_dev(struct dsa_port *dp,
> +			     const struct net_device *bridge_dev)
>   {
>   	/* DSA ports connected to a bridge, and event was emitted
>   	 * for the bridge.
> @@ -295,13 +296,14 @@ static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
>   }
>   
>   /* Returns true if any port of this tree offloads the given bridge */
> -static inline bool dsa_tree_offloads_bridge(struct dsa_switch_tree *dst,
> -					    const struct net_device *bridge_dev)
> +static inline bool
> +dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
> +			     const struct net_device *bridge_dev)
>   {
>   	struct dsa_port *dp;
>   
>   	list_for_each_entry(dp, &dst->ports, list)
> -		if (dsa_port_offloads_bridge(dp, bridge_dev))
> +		if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
>   			return true;
>   
>   	return false;
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index ad873bb2f676..98fd5e972d28 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -289,14 +289,14 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
>   		ret = dsa_port_set_state(dp, attr->u.stp_state, true);
>   		break;
>   	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
> -		if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
> +		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
>   			return -EOPNOTSUPP;
>   
>   		ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering,
>   					      extack);
>   		break;
>   	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
> -		if (!dsa_port_offloads_bridge(dp, attr->orig_dev))
> +		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
>   			return -EOPNOTSUPP;
>   
>   		ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
> @@ -409,7 +409,7 @@ static int dsa_slave_port_obj_add(struct net_device *dev, const void *ctx,
>   		err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
>   		break;
>   	case SWITCHDEV_OBJ_ID_HOST_MDB:
> -		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
> +		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
>   			return -EOPNOTSUPP;
>   
>   		err = dsa_port_host_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
> @@ -421,13 +421,13 @@ static int dsa_slave_port_obj_add(struct net_device *dev, const void *ctx,
>   		err = dsa_slave_vlan_add(dev, obj, extack);
>   		break;
>   	case SWITCHDEV_OBJ_ID_MRP:
> -		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
> +		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
>   			return -EOPNOTSUPP;
>   
>   		err = dsa_port_mrp_add(dp, SWITCHDEV_OBJ_MRP(obj));
>   		break;
>   	case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
> -		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
> +		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
>   			return -EOPNOTSUPP;
>   
>   		err = dsa_port_mrp_add_ring_role(dp,
> @@ -483,7 +483,7 @@ static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
>   		err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
>   		break;
>   	case SWITCHDEV_OBJ_ID_HOST_MDB:
> -		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
> +		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
>   			return -EOPNOTSUPP;
>   
>   		err = dsa_port_host_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
> @@ -495,13 +495,13 @@ static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
>   		err = dsa_slave_vlan_del(dev, obj);
>   		break;
>   	case SWITCHDEV_OBJ_ID_MRP:
> -		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
> +		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
>   			return -EOPNOTSUPP;
>   
>   		err = dsa_port_mrp_del(dp, SWITCHDEV_OBJ_MRP(obj));
>   		break;
>   	case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
> -		if (!dsa_port_offloads_bridge(dp, obj->orig_dev))
> +		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
>   			return -EOPNOTSUPP;
>   
>   		err = dsa_port_mrp_del_ring_role(dp,
> @@ -2460,7 +2460,7 @@ static bool dsa_foreign_dev_check(const struct net_device *dev,
>   	struct dsa_switch_tree *dst = dp->ds->dst;
>   
>   	if (netif_is_bridge_master(foreign_dev))
> -		return !dsa_tree_offloads_bridge(dst, foreign_dev);
> +		return !dsa_tree_offloads_bridge_dev(dst, foreign_dev);
>   
>   	if (netif_is_bridge_port(foreign_dev))
>   		return !dsa_tree_offloads_bridge_port(dst, foreign_dev);
> 


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

* Re: [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based
  2021-11-11 12:24   ` Alvin Šipraga
@ 2021-11-11 12:45     ` Vladimir Oltean
  2021-11-11 13:18       ` Alvin Šipraga
  0 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2021-11-11 12:45 UTC (permalink / raw)
  To: Alvin Šipraga
  Cc: netdev, Florian Fainelli, Andrew Lunn, Vivien Didelot,
	Tobias Waldekranz, DENG Qingfang

On Thu, Nov 11, 2021 at 12:24:47PM +0000, Alvin Šipraga wrote:
> On 10/26/21 18:26, Vladimir Oltean wrote:
> > I have seen too many bugs already due to the fact that we must encode an
> > invalid dp->bridge_num as a negative value, because the natural tendency
> > is to check that invalid value using (!dp->bridge_num). Latest example
> > can be seen in commit 1bec0f05062c ("net: dsa: fix bridge_num not
> > getting cleared after ports leaving the bridge").
> > 
> > Convert the existing users to assume that dp->bridge_num == 0 is the
> > encoding for invalid, and valid bridge numbers start from 1.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> 
> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Thanks for the review.

> Small remark inline.
> 
> > -int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
> > +unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
> >   {
> > -	int bridge_num = dsa_bridge_num_find(bridge_dev);
> > +	unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
> >   
> > -	if (bridge_num < 0) {
> > +	if (!bridge_num) {
> >   		/* First port that offloads TX forwarding for this bridge */
> 
> Perhaps you want to update this comment in patch 2/6, since bridge_num 
> is no longer just about TX forwarding offload.
> 
> > -		bridge_num = find_first_zero_bit(&dsa_fwd_offloading_bridges,
> > -						 DSA_MAX_NUM_OFFLOADING_BRIDGES);
> > +		bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
> > +						DSA_MAX_NUM_OFFLOADING_BRIDGES,
> > +						1);

I will update this comment in patch 2 to say "First port that requests
FDB isolation or TX forwarding offload for this bridge". Sounds ok?

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

* Re: [RFC PATCH net-next 5/6] net: dsa: keep the bridge_dev and bridge_num as part of the same structure
  2021-10-26 16:26 ` [RFC PATCH net-next 5/6] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
@ 2021-11-11 13:17   ` Alvin Šipraga
  0 siblings, 0 replies; 17+ messages in thread
From: Alvin Šipraga @ 2021-11-11 13:17 UTC (permalink / raw)
  To: Vladimir Oltean, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang

On 10/26/21 18:26, Vladimir Oltean wrote:
> The main desire behind this is to provide coherent bridge information to
> the fast path without locking.
> 
> For example, right now we set dp->bridge_dev and dp->bridge_num from
> separate code paths, it is theoretically possible for a packet
> transmission to read these two port properties consecutively and find a
> bridge number which does not correspond with the bridge device.
> 
> Another desire is to start passing more complex bridge information to
> dsa_switch_ops functions. For example, with FDB isolation, it is
> expected that drivers will need to be passed the bridge which requested
> an FDB/MDB entry to be offloaded, and along with that bridge_dev, the
> associated bridge_num should be passed too, in case the driver might
> want to implement an isolation scheme based on that number.
> 
> We already pass the {bridge_dev, bridge_num} pair to the TX forwarding
> offload switch API, however we'd like to remove that and squash it into
> the basic bridge join/leave API. So that means we need to pass this
> pair to the bridge join/leave API.
> 
> During dsa_port_bridge_leave, first we unset dp->bridge_dev, then we
> call the driver's .port_bridge_leave with what used to be our
> dp->bridge_dev, but provided as an argument.
> 
> When bridge_dev and bridge_num get folded into a single structure, we
> need to preserve this behavior in dsa_port_bridge_leave: we need a copy
> of what used to be in dp->bridge.
> 
> Switch drivers check bridge membership by comparing dp->bridge_dev with
> the provided bridge_dev, but now, if we provide the struct dsa_bridge as
> a pointer, they cannot keep comparing dp->bridge to the provided
> pointer, since this only points to an on-stack copy. To make this
> obvious and prevent driver writers from forgetting and doing stupid
> things, in this new API, the struct dsa_bridge is provided as a full
> structure (not very large, contains an int and a pointer) instead of a
> pointer. An explicit comparison function needs to be used to determine
> bridge membership: dsa_port_offloads_bridge().
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Just one suggestion down below, but this looks fine either way.

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

>   drivers/net/dsa/b53/b53_common.c       |  8 +--
>   drivers/net/dsa/b53/b53_priv.h         |  4 +-
>   drivers/net/dsa/dsa_loop.c             |  8 +--
>   drivers/net/dsa/hirschmann/hellcreek.c |  4 +-
>   drivers/net/dsa/lan9303-core.c         |  4 +-
>   drivers/net/dsa/lantiq_gswip.c         | 14 +++--
>   drivers/net/dsa/microchip/ksz_common.c |  4 +-
>   drivers/net/dsa/microchip/ksz_common.h |  4 +-
>   drivers/net/dsa/mt7530.c               |  8 +--
>   drivers/net/dsa/mv88e6xxx/chip.c       | 26 ++++-----
>   drivers/net/dsa/ocelot/felix.c         |  8 +--
>   drivers/net/dsa/qca8k.c                | 12 ++--
>   drivers/net/dsa/rtl8366rb.c            |  8 +--
>   drivers/net/dsa/sja1105/sja1105_main.c | 12 ++--
>   drivers/net/dsa/xrs700x/xrs700x.c      | 10 ++--
>   include/linux/dsa/8021q.h              |  7 +--
>   include/net/dsa.h                      | 77 +++++++++++++++++++++-----
>   net/dsa/dsa2.c                         | 35 ++++++++----
>   net/dsa/dsa_priv.h                     | 53 ++----------------
>   net/dsa/port.c                         | 69 ++++++++++++-----------
>   net/dsa/slave.c                        |  2 +-
>   net/dsa/switch.c                       | 13 +++--
>   net/dsa/tag_8021q.c                    | 12 ++--
>   23 files changed, 215 insertions(+), 187 deletions(-)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index d5e78f51f42d..4e41b1a63108 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -1860,7 +1860,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 net_device *br)
> +int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>   {
>   	struct b53_device *dev = ds->priv;
>   	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
> @@ -1887,7 +1887,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
>   	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
>   
>   	b53_for_each_port(dev, i) {
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   
>   		/* Add this local port to the remote port VLAN control
> @@ -1911,7 +1911,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
>   }
>   EXPORT_SYMBOL(b53_br_join);
>   
> -void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
> +void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>   {
>   	struct b53_device *dev = ds->priv;
>   	struct b53_vlan *vl = &dev->vlans[0];
> @@ -1923,7 +1923,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
>   
>   	b53_for_each_port(dev, i) {
>   		/* Don't touch the remaining ports */
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   
>   		b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), &reg);
> diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
> index 579da74ada64..ee17f8b516ca 100644
> --- a/drivers/net/dsa/b53/b53_priv.h
> +++ b/drivers/net/dsa/b53/b53_priv.h
> @@ -324,8 +324,8 @@ void b53_get_strings(struct dsa_switch *ds, int port, u32 stringset,
>   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 net_device *bridge);
> -void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *bridge);
> +int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge);
> +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);
>   int b53_br_flags_pre(struct dsa_switch *ds, int port,
> diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
> index e638e3eea911..70db3a9aa355 100644
> --- a/drivers/net/dsa/dsa_loop.c
> +++ b/drivers/net/dsa/dsa_loop.c
> @@ -167,19 +167,19 @@ 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 net_device *bridge)
> +				     struct dsa_bridge bridge)
>   {
>   	dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
> -		__func__, port, bridge->name);
> +		__func__, port, bridge.dev->name);
>   
>   	return 0;
>   }
>   
>   static void dsa_loop_port_bridge_leave(struct dsa_switch *ds, int port,
> -				       struct net_device *bridge)
> +				       struct dsa_bridge bridge)
>   {
>   	dev_dbg(ds->dev, "%s: port: %d, bridge: %s\n",
> -		__func__, port, bridge->name);
> +		__func__, port, bridge.dev->name);
>   }
>   
>   static void dsa_loop_port_stp_state_set(struct dsa_switch *ds, int port,
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index 4e0b53d94b52..17c2d13670a9 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -674,7 +674,7 @@ static int hellcreek_bridge_flags(struct dsa_switch *ds, int port,
>   }
>   
>   static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
> -				      struct net_device *br)
> +				      struct dsa_bridge bridge)
>   {
>   	struct hellcreek *hellcreek = ds->priv;
>   
> @@ -691,7 +691,7 @@ static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
>   }
>   
>   static void hellcreek_port_bridge_leave(struct dsa_switch *ds, int port,
> -					struct net_device *br)
> +					struct dsa_bridge bridge)
>   {
>   	struct hellcreek *hellcreek = ds->priv;
>   
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index 1c2bdcde6979..29d909484275 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -1103,7 +1103,7 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port)
>   }
>   
>   static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
> -				    struct net_device *br)
> +				    struct dsa_bridge bridge)
>   {
>   	struct lan9303 *chip = ds->priv;
>   
> @@ -1117,7 +1117,7 @@ static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
>   }
>   
>   static void lan9303_port_bridge_leave(struct dsa_switch *ds, int port,
> -				      struct net_device *br)
> +				      struct dsa_bridge bridge)
>   {
>   	struct lan9303 *chip = ds->priv;
>   
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index c60b1eef3545..6b9b9c712e58 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -1146,16 +1146,17 @@ static int gswip_vlan_remove(struct gswip_priv *priv,
>   }
>   
>   static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
> -				  struct net_device *bridge)
> +				  struct dsa_bridge bridge)
>   {
> +	struct net_device *br = bridge.dev;
>   	struct gswip_priv *priv = ds->priv;
>   	int err;
>   
>   	/* When the bridge uses VLAN filtering we have to configure VLAN
>   	 * specific bridges. No bridge is configured here.
>   	 */
> -	if (!br_vlan_enabled(bridge)) {
> -		err = gswip_vlan_add_unaware(priv, bridge, port);
> +	if (!br_vlan_enabled(br)) {
> +		err = gswip_vlan_add_unaware(priv, br, port);
>   		if (err)
>   			return err;
>   		priv->port_vlan_filter &= ~BIT(port);
> @@ -1166,8 +1167,9 @@ static int gswip_port_bridge_join(struct dsa_switch *ds, int port,
>   }
>   
>   static void gswip_port_bridge_leave(struct dsa_switch *ds, int port,
> -				    struct net_device *bridge)
> +				    struct dsa_bridge bridge)
>   {
> +	struct net_device *br = bridge.dev;
>   	struct gswip_priv *priv = ds->priv;
>   
>   	gswip_add_single_port_br(priv, port, true);
> @@ -1175,8 +1177,8 @@ static void gswip_port_bridge_leave(struct dsa_switch *ds, int port,
>   	/* When the bridge uses VLAN filtering we have to configure VLAN
>   	 * specific bridges. No bridge is configured here.
>   	 */
> -	if (!br_vlan_enabled(bridge))
> -		gswip_vlan_remove(priv, bridge, port, 0, true, false);
> +	if (!br_vlan_enabled(br))
> +		gswip_vlan_remove(priv, br, port, 0, true, false);
>   }
>   
>   static int gswip_port_vlan_prepare(struct dsa_switch *ds, int port,
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 7c2968a639eb..23784d14a5da 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -173,7 +173,7 @@ void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
>   EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
>   
>   int ksz_port_bridge_join(struct dsa_switch *ds, int port,
> -			 struct net_device *br)
> +			 struct dsa_bridge bridge)
>   {
>   	struct ksz_device *dev = ds->priv;
>   
> @@ -190,7 +190,7 @@ int ksz_port_bridge_join(struct dsa_switch *ds, int port,
>   EXPORT_SYMBOL_GPL(ksz_port_bridge_join);
>   
>   void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
> -			   struct net_device *br)
> +			   struct dsa_bridge bridge)
>   {
>   	struct ksz_device *dev = ds->priv;
>   
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index 1597c63988b4..df953c2bc56e 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -159,9 +159,9 @@ 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 net_device *br);
> +			 struct dsa_bridge bridge);
>   void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
> -			   struct net_device *br);
> +			   struct dsa_bridge bridge);
>   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);
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 79bde263b06f..85cc5aca7f96 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1186,7 +1186,7 @@ mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
>   
>   static int
>   mt7530_port_bridge_join(struct dsa_switch *ds, int port,
> -			struct net_device *bridge)
> +			struct dsa_bridge bridge)
>   {
>   	struct mt7530_priv *priv = ds->priv;
>   	u32 port_bitmap = BIT(MT7530_CPU_PORT);
> @@ -1202,7 +1202,7 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
>   		 * and not being setup until the port becomes enabled.
>   		 */
>   		if (dsa_port_is_user(other_dp) && i != port) {
> -			if (dsa_port_bridge_dev_get(other_dp) != bridge)
> +			if (!dsa_port_offloads_bridge(other_dp, &bridge))
>   				continue;
>   			if (priv->ports[i].enable)
>   				mt7530_set(priv, MT7530_PCR_P(i),
> @@ -1301,7 +1301,7 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
>   
>   static void
>   mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
> -			 struct net_device *bridge)
> +			 struct dsa_bridge bridge)
>   {
>   	struct mt7530_priv *priv = ds->priv;
>   	int i;
> @@ -1316,7 +1316,7 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
>   		 * is kept and not being setup until the port becomes enabled.
>   		 */
>   		if (dsa_port_is_user(other_dp) && i != port) {
> -			if (dsa_port_bridge_dev_get(other_dp) != bridge)
> +			if (!dsa_port_offloads_bridge(other_dp, &bridge))
>   				continue;
>   			if (priv->ports[i].enable)
>   				mt7530_clear(priv, MT7530_PCR_P(i),
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index b76ce4423ea8..73613a667f6c 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2417,7 +2417,7 @@ static int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
>   }
>   
>   static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
> -				struct net_device *br)
> +				struct dsa_bridge bridge)
>   {
>   	struct dsa_switch *ds = chip->ds;
>   	struct dsa_switch_tree *dst = ds->dst;
> @@ -2425,7 +2425,7 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
>   	int err;
>   
>   	list_for_each_entry(dp, &dst->ports, list) {
> -		if (dsa_port_bridge_dev_get(dp) == br) {
> +		if (dsa_port_offloads_bridge(dp, &bridge)) {
>   			if (dp->ds == ds) {
>   				/* This is a local bridge group member,
>   				 * remap its Port VLAN Map.
> @@ -2449,14 +2449,14 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
>   }
>   
>   static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
> -				      struct net_device *br)
> +				      struct dsa_bridge bridge)
>   {
>   	struct mv88e6xxx_chip *chip = ds->priv;
>   	int err;
>   
>   	mv88e6xxx_reg_lock(chip);
>   
> -	err = mv88e6xxx_bridge_map(chip, br);
> +	err = mv88e6xxx_bridge_map(chip, bridge);
>   	if (err)
>   		goto unlock;
>   
> @@ -2471,14 +2471,14 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
>   }
>   
>   static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
> -					struct net_device *br)
> +					struct dsa_bridge bridge)
>   {
>   	struct mv88e6xxx_chip *chip = ds->priv;
>   	int err;
>   
>   	mv88e6xxx_reg_lock(chip);
>   
> -	if (mv88e6xxx_bridge_map(chip, br) ||
> +	if (mv88e6xxx_bridge_map(chip, bridge) ||
>   	    mv88e6xxx_port_vlan_map(chip, port))
>   		dev_err(ds->dev, "failed to remap in-chip Port VLAN\n");
>   
> @@ -2493,7 +2493,7 @@ 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 net_device *br)
> +					   int port, struct dsa_bridge bridge)
>   {
>   	struct mv88e6xxx_chip *chip = ds->priv;
>   	int err;
> @@ -2510,7 +2510,7 @@ static int mv88e6xxx_crosschip_bridge_join(struct dsa_switch *ds,
>   
>   static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
>   					     int tree_index, int sw_index,
> -					     int port, struct net_device *br)
> +					     int port, struct dsa_bridge bridge)
>   {
>   	struct mv88e6xxx_chip *chip = ds->priv;
>   
> @@ -2542,19 +2542,17 @@ static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
>   }
>   
>   static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
> -					   struct net_device *br,
> -					   unsigned int bridge_num)
> +					   struct dsa_bridge bridge)
>   {
> -	return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
> +	return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
>   }
>   
>   static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
> -					      struct net_device *br,
> -					      unsigned int bridge_num)
> +					      struct dsa_bridge bridge)
>   {
>   	int err;
>   
> -	err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
> +	err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
>   	if (err) {
>   		dev_err(ds->dev, "failed to remap cross-chip Port VLAN: %pe\n",
>   			ERR_PTR(err));
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index 83808e7dbdda..8cf82fa91a6b 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -698,21 +698,21 @@ static int felix_bridge_flags(struct dsa_switch *ds, int port,
>   }
>   
>   static int felix_bridge_join(struct dsa_switch *ds, int port,
> -			     struct net_device *br)
> +			     struct dsa_bridge bridge)
>   {
>   	struct ocelot *ocelot = ds->priv;
>   
> -	ocelot_port_bridge_join(ocelot, port, br);
> +	ocelot_port_bridge_join(ocelot, port, bridge.dev);
>   
>   	return 0;
>   }
>   
>   static void felix_bridge_leave(struct dsa_switch *ds, int port,
> -			       struct net_device *br)
> +			       struct dsa_bridge bridge)
>   {
>   	struct ocelot *ocelot = ds->priv;
>   
> -	ocelot_port_bridge_leave(ocelot, port, br);
> +	ocelot_port_bridge_leave(ocelot, port, bridge.dev);
>   }
>   
>   static int felix_lag_join(struct dsa_switch *ds, int port,
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index efe20db287a5..3ae10f22ada9 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -1728,8 +1728,8 @@ qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
>   		  QCA8K_PORT_LOOKUP_STATE_MASK, stp_state);
>   }
>   
> -static int
> -qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
> +static int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
> +				  struct dsa_bridge bridge)
>   {
>   	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
>   	int port_mask, cpu_port;
> @@ -1741,7 +1741,7 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
>   	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
>   		if (dsa_is_cpu_port(ds, i))
>   			continue;
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   		/* Add this port to the portvlan mask of the other ports
>   		 * in the bridge
> @@ -1762,8 +1762,8 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
>   	return ret;
>   }
>   
> -static void
> -qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
> +static void qca8k_port_bridge_leave(struct dsa_switch *ds, int port,
> +				    struct dsa_bridge bridge)
>   {
>   	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
>   	int cpu_port, i;
> @@ -1773,7 +1773,7 @@ qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
>   	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
>   		if (dsa_is_cpu_port(ds, i))
>   			continue;
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   		/* Remove this port to the portvlan mask of the other ports
>   		 * in the bridge
> diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
> index b6f277a04989..fac2333a3f5e 100644
> --- a/drivers/net/dsa/rtl8366rb.c
> +++ b/drivers/net/dsa/rtl8366rb.c
> @@ -1186,7 +1186,7 @@ rtl8366rb_port_disable(struct dsa_switch *ds, int port)
>   
>   static int
>   rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
> -			   struct net_device *bridge)
> +			   struct dsa_bridge bridge)
>   {
>   	struct realtek_smi *smi = ds->priv;
>   	unsigned int port_bitmap = 0;
> @@ -1198,7 +1198,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
>   		if (i == port)
>   			continue;
>   		/* Not on this bridge */
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   		/* Join this port to each other port on the bridge */
>   		ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
> @@ -1218,7 +1218,7 @@ rtl8366rb_port_bridge_join(struct dsa_switch *ds, int port,
>   
>   static void
>   rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
> -			    struct net_device *bridge)
> +			    struct dsa_bridge bridge)
>   {
>   	struct realtek_smi *smi = ds->priv;
>   	unsigned int port_bitmap = 0;
> @@ -1230,7 +1230,7 @@ rtl8366rb_port_bridge_leave(struct dsa_switch *ds, int port,
>   		if (i == port)
>   			continue;
>   		/* Not on this bridge */
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   		/* Remove this port from any other port on the bridge */
>   		ret = regmap_update_bits(smi->map, RTL8366RB_PORT_ISO(i),
> diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> index 5e03eda4c16f..24584fe2e760 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -1980,7 +1980,7 @@ static int sja1105_manage_flood_domains(struct sja1105_private *priv)
>   }
>   
>   static int sja1105_bridge_member(struct dsa_switch *ds, int port,
> -				 struct net_device *br, bool member)
> +				 struct dsa_bridge bridge, bool member)
>   {
>   	struct sja1105_l2_forwarding_entry *l2_fwd;
>   	struct sja1105_private *priv = ds->priv;
> @@ -2005,7 +2005,7 @@ static int sja1105_bridge_member(struct dsa_switch *ds, int port,
>   		 */
>   		if (i == port)
>   			continue;
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != br)
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   		sja1105_port_allow_traffic(l2_fwd, i, port, member);
>   		sja1105_port_allow_traffic(l2_fwd, port, i, member);
> @@ -2074,15 +2074,15 @@ static void sja1105_bridge_stp_state_set(struct dsa_switch *ds, int port,
>   }
>   
>   static int sja1105_bridge_join(struct dsa_switch *ds, int port,
> -			       struct net_device *br)
> +			       struct dsa_bridge bridge)
>   {
> -	return sja1105_bridge_member(ds, port, br, true);
> +	return sja1105_bridge_member(ds, port, bridge, true);
>   }
>   
>   static void sja1105_bridge_leave(struct dsa_switch *ds, int port,
> -				 struct net_device *br)
> +				 struct dsa_bridge bridge)
>   {
> -	sja1105_bridge_member(ds, port, br, false);
> +	sja1105_bridge_member(ds, port, bridge, false);
>   }
>   
>   #define BYTES_PER_KBIT (1000LL / 8)
> diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
> index 7c2b6c32242d..ebb55dfd9c4e 100644
> --- a/drivers/net/dsa/xrs700x/xrs700x.c
> +++ b/drivers/net/dsa/xrs700x/xrs700x.c
> @@ -501,7 +501,7 @@ static void xrs700x_mac_link_up(struct dsa_switch *ds, int port,
>   }
>   
>   static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
> -				 struct net_device *bridge, bool join)
> +				 struct dsa_bridge bridge, bool join)
>   {
>   	unsigned int i, cpu_mask = 0, mask = 0;
>   	struct xrs700x *priv = ds->priv;
> @@ -513,14 +513,14 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
>   
>   		cpu_mask |= BIT(i);
>   
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) == bridge)
> +		if (dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   
>   		mask |= BIT(i);
>   	}
>   
>   	for (i = 0; i < ds->num_ports; i++) {
> -		if (dsa_port_bridge_dev_get(dsa_to_port(ds, i)) != bridge)
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
>   			continue;
>   
>   		/* 1 = Disable forwarding to the port */
> @@ -540,13 +540,13 @@ static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
>   }
>   
>   static int xrs700x_bridge_join(struct dsa_switch *ds, int port,
> -			       struct net_device *bridge)
> +			       struct dsa_bridge bridge)
>   {
>   	return xrs700x_bridge_common(ds, port, bridge, true);
>   }
>   
>   static void xrs700x_bridge_leave(struct dsa_switch *ds, int port,
> -				 struct net_device *bridge)
> +				 struct dsa_bridge bridge)
>   {
>   	xrs700x_bridge_common(ds, port, bridge, false);
>   }
> diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
> index 0af4371fbebb..939a1beaddf7 100644
> --- a/include/linux/dsa/8021q.h
> +++ b/include/linux/dsa/8021q.h
> @@ -7,6 +7,7 @@
>   
>   #include <linux/refcount.h>
>   #include <linux/types.h>
> +#include <net/dsa.h>
>   
>   struct dsa_switch;
>   struct dsa_port;
> @@ -37,12 +38,10 @@ struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev,
>   void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
>   
>   int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
> -					struct net_device *br,
> -					unsigned int bridge_num);
> +					struct dsa_bridge bridge);
>   
>   void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
> -					   struct net_device *br,
> -					   unsigned int bridge_num);
> +					   struct dsa_bridge bridge);
>   
>   u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
>   
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 81fd1821a0aa..5e83c64bc96a 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -219,6 +219,11 @@ struct dsa_mall_tc_entry {
>   	};
>   };
>   
> +struct dsa_bridge {
> +	struct net_device *dev;
> +	unsigned int num;
> +	refcount_t refcount;
> +};
>   
>   struct dsa_port {
>   	/* A CPU port is physically connected to a master device.
> @@ -256,8 +261,7 @@ struct dsa_port {
>   	/* Managed by DSA on user ports and by drivers on CPU and DSA ports */
>   	bool			learning;
>   	u8			stp_state;
> -	struct net_device	*bridge_dev;
> -	unsigned int		bridge_num;
> +	struct dsa_bridge	*bridge;
>   	struct devlink_port	devlink_port;
>   	bool			devlink_port_setup;
>   	struct phylink		*pl;
> @@ -588,7 +592,7 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp)
>   static inline
>   struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
>   {
> -	if (!dp->bridge_dev)
> +	if (!dp->bridge)
>   		return NULL;
>   
>   	if (dp->lag_dev)
> @@ -601,12 +605,12 @@ struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
>   
>   static inline struct net_device *dsa_port_bridge_dev_get(struct dsa_port *dp)
>   {
> -	return dp->bridge_dev;
> +	return dp->bridge ? dp->bridge->dev : NULL;
>   }
>   
>   static inline unsigned int dsa_port_bridge_num_get(struct dsa_port *dp)
>   {
> -	return dp->bridge_num;
> +	return dp->bridge ? dp->bridge->num : 0;
>   }
>   
>   static inline bool dsa_port_bridge_same(struct dsa_port *a, struct dsa_port *b)
> @@ -614,6 +618,55 @@ static inline bool dsa_port_bridge_same(struct dsa_port *a, struct dsa_port *b)
>   	return dsa_port_bridge_dev_get(a) == dsa_port_bridge_dev_get(b);
>   }
>   
> +static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
> +						 const struct net_device *dev)
> +{
> +	return dsa_port_to_bridge_port(dp) == dev;
> +}
> +
> +static inline bool
> +dsa_port_offloads_bridge_dev(struct dsa_port *dp,
> +			     const struct net_device *bridge_dev)
> +{
> +	/* DSA ports connected to a bridge, and event was emitted
> +	 * for the bridge.
> +	 */
> +	return dsa_port_bridge_dev_get(dp) == bridge_dev;
> +}
> +
> +static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
> +					    const struct dsa_bridge *bridge)
> +{
> +	return dsa_port_bridge_dev_get(dp) == bridge->dev;
> +}
> +
> +/* Returns true if any port of this tree offloads the given net_device */
> +static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
> +						 const struct net_device *dev)
> +{
> +	struct dsa_port *dp;
> +
> +	list_for_each_entry(dp, &dst->ports, list)
> +		if (dsa_port_offloads_bridge_port(dp, dev))
> +			return true;
> +
> +	return false;
> +}
> +
> +/* Returns true if any port of this tree offloads the given bridge */
> +static inline bool
> +dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
> +			     const struct net_device *bridge_dev)
> +{
> +	struct dsa_port *dp;
> +
> +	list_for_each_entry(dp, &dst->ports, list)
> +		if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
> +			return true;
> +
> +	return false;
> +}
> +
>   typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
>   			      bool is_static, void *data);
>   struct dsa_switch_ops {
> @@ -761,17 +814,15 @@ 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 net_device *bridge);
> +				    struct dsa_bridge bridge);
>   	void	(*port_bridge_leave)(struct dsa_switch *ds, int port,
> -				     struct net_device *bridge);
> +				     struct dsa_bridge bridge);
>   	/* Called right after .port_bridge_join() */
>   	int	(*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
> -					      struct net_device *bridge,
> -					      unsigned int bridge_num);
> +					      struct dsa_bridge bridge);
>   	/* Called right before .port_bridge_leave() */
>   	void	(*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
> -						struct net_device *bridge,
> -						unsigned int bridge_num);
> +						struct dsa_bridge bridge);
>   	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
>   				      u8 state);
>   	void	(*port_fast_age)(struct dsa_switch *ds, int port);
> @@ -843,10 +894,10 @@ struct dsa_switch_ops {
>   	 */
>   	int	(*crosschip_bridge_join)(struct dsa_switch *ds, int tree_index,
>   					 int sw_index, int port,
> -					 struct net_device *br);
> +					 struct dsa_bridge bridge);
>   	void	(*crosschip_bridge_leave)(struct dsa_switch *ds, int tree_index,
>   					  int sw_index, int port,
> -					  struct net_device *br);
> +					  struct dsa_bridge bridge);
>   	int	(*crosschip_lag_change)(struct dsa_switch *ds, int sw_index,
>   					int port);
>   	int	(*crosschip_lag_join)(struct dsa_switch *ds, int sw_index,
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 9606e56710a5..7c014aeea559 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -129,20 +129,29 @@ void dsa_lag_unmap(struct dsa_switch_tree *dst, struct net_device *lag)
>   	}
>   }
>   
> +struct dsa_bridge *dsa_tree_bridge_find(struct dsa_switch_tree *dst,
> +					const struct net_device *br)
> +{
> +	struct dsa_port *dp;
> +
> +	list_for_each_entry(dp, &dst->ports, list)
> +		if (dsa_port_bridge_dev_get(dp) == br)
> +			return dp->bridge;
> +
> +	return NULL;
> +}
> +
>   static int dsa_bridge_num_find(const struct net_device *bridge_dev)
>   {
>   	struct dsa_switch_tree *dst;
> -	struct dsa_port *dp;
>   
> -	/* When preparing the offload for a port, it will have a valid
> -	 * dp->bridge_dev pointer but a not yet valid dp->bridge_num.
> -	 * However there might be other ports having the same dp->bridge_dev
> -	 * and a valid dp->bridge_num, so just ignore this port.
> -	 */
> -	list_for_each_entry(dst, &dsa_tree_list, list)
> -		list_for_each_entry(dp, &dst->ports, list)
> -			if (dp->bridge_dev == bridge_dev && dp->bridge_num)
> -				return dp->bridge_num;
> +	list_for_each_entry(dst, &dsa_tree_list, list) {
> +		struct dsa_bridge *bridge;
> +
> +		bridge = dsa_tree_bridge_find(dst, bridge_dev);
> +		if (bridge)
> +			return bridge->num;
> +	}
>   
>   	return 0;
>   }
> @@ -151,6 +160,12 @@ unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
>   {
>   	unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
>   
> +	/* Switches without FDB isolation support don't get unique
> +	 * bridge numbering
> +	 */
> +	if (!max)
> +		return 0;
> +
>   	if (!bridge_num) {
>   		/* First port that offloads TX forwarding for this bridge */
>   		bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index daba10adbd22..489712ead08f 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -52,7 +52,7 @@ struct dsa_notifier_ageing_time_info {
>   
>   /* DSA_NOTIFIER_BRIDGE_* */
>   struct dsa_notifier_bridge_info {
> -	struct net_device *br;
> +	struct dsa_bridge bridge;
>   	int tree_index;
>   	int sw_index;
>   	int port;
> @@ -266,49 +266,6 @@ int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast);
>   void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast);
>   extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
>   
> -static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
> -						 const struct net_device *dev)
> -{
> -	return dsa_port_to_bridge_port(dp) == dev;
> -}
> -
> -static inline bool
> -dsa_port_offloads_bridge_dev(struct dsa_port *dp,
> -			     const struct net_device *bridge_dev)
> -{
> -	/* DSA ports connected to a bridge, and event was emitted
> -	 * for the bridge.
> -	 */
> -	return dsa_port_bridge_dev_get(dp) == bridge_dev;
> -}
> -
> -/* Returns true if any port of this tree offloads the given net_device */
> -static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst,
> -						 const struct net_device *dev)
> -{
> -	struct dsa_port *dp;
> -
> -	list_for_each_entry(dp, &dst->ports, list)
> -		if (dsa_port_offloads_bridge_port(dp, dev))
> -			return true;
> -
> -	return false;
> -}
> -
> -/* Returns true if any port of this tree offloads the given bridge */
> -static inline bool
> -dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
> -			     const struct net_device *bridge_dev)
> -{
> -	struct dsa_port *dp;
> -
> -	list_for_each_entry(dp, &dst->ports, list)
> -		if (dsa_port_offloads_bridge_dev(dp, bridge_dev))
> -			return true;
> -
> -	return false;
> -}
> -
>   /* slave.c */
>   extern const struct dsa_device_ops notag_netdev_ops;
>   extern struct notifier_block dsa_slave_switchdev_notifier;
> @@ -417,7 +374,7 @@ dsa_find_designated_bridge_port_by_vid(struct net_device *master, u16 vid)
>   		if (dp->type != DSA_PORT_TYPE_USER)
>   			continue;
>   
> -		if (!dp->bridge_dev)
> +		if (!dp->bridge)
>   			continue;
>   
>   		if (dp->stp_state != BR_STATE_LEARNING &&
> @@ -446,7 +403,7 @@ dsa_find_designated_bridge_port_by_vid(struct net_device *master, u16 vid)
>   /* If the ingress port offloads the bridge, we mark the frame as autonomously
>    * forwarded by hardware, so the software bridge doesn't forward in twice, back
>    * to us, because we already did. However, if we're in fallback mode and we do
> - * software bridging, we are not offloading it, therefore the dp->bridge_dev
> + * software bridging, we are not offloading it, therefore the dp->bridge
>    * pointer is not populated, and flooding needs to be done by software (we are
>    * effectively operating in standalone ports mode).
>    */
> @@ -454,7 +411,7 @@ static inline void dsa_default_offload_fwd_mark(struct sk_buff *skb)
>   {
>   	struct dsa_port *dp = dsa_slave_to_port(skb->dev);
>   
> -	skb->offload_fwd_mark = !!(dp->bridge_dev);
> +	skb->offload_fwd_mark = !!(dp->bridge);
>   }
>   
>   /* Helper for removing DSA header tags from packets in the RX path.
> @@ -551,6 +508,8 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
>   unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
>   void dsa_bridge_num_put(const struct net_device *bridge_dev,
>   			unsigned int bridge_num);
> +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,
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index aaa978ee165e..b2e66994f72c 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -130,7 +130,7 @@ int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy)
>   			return err;
>   	}
>   
> -	if (!dp->bridge_dev)
> +	if (!dp->bridge)
>   		dsa_port_set_state_now(dp, BR_STATE_FORWARDING, false);
>   
>   	if (dp->pl)
> @@ -158,7 +158,7 @@ void dsa_port_disable_rt(struct dsa_port *dp)
>   	if (dp->pl)
>   		phylink_stop(dp->pl);
>   
> -	if (!dp->bridge_dev)
> +	if (!dp->bridge)
>   		dsa_port_set_state_now(dp, BR_STATE_DISABLED, false);
>   
>   	if (ds->ops->port_disable)
> @@ -271,36 +271,32 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
>   }
>   
>   static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
> -					     struct net_device *bridge_dev,
> -					     unsigned int bridge_num)
> +					     struct dsa_bridge bridge)
>   {
>   	struct dsa_switch *ds = dp->ds;
>   
>   	/* No bridge TX forwarding offload => do nothing */
> -	if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge_num)
> +	if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge.num)
>   		return;
>   
>   	/* Notify the chips only once the offload has been deactivated, so
>   	 * that they can update their configuration accordingly.
>   	 */
> -	ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge_dev,
> -					      bridge_num);
> +	ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge);
>   }
>   
>   static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
> -					   struct net_device *bridge_dev,
> -					   unsigned int bridge_num)
> +					   struct dsa_bridge bridge)
>   {
>   	struct dsa_switch *ds = dp->ds;
>   	int err;
>   
>   	/* FDB isolation is required for TX forwarding offload */
> -	if (!ds->ops->port_bridge_tx_fwd_offload || !bridge_num)
> +	if (!ds->ops->port_bridge_tx_fwd_offload || !bridge.num)
>   		return false;
>   
>   	/* Notify the driver */
> -	err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge_dev,
> -						  bridge_num);
> +	err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge);
>   
>   	return err ? false : true;
>   }
> @@ -310,21 +306,31 @@ static int dsa_port_bridge_create(struct dsa_port *dp,
>   				  struct netlink_ext_ack *extack)
>   {
>   	struct dsa_switch *ds = dp->ds;
> -	unsigned int bridge_num;
> +	struct dsa_bridge *bridge;
>   
> -	dp->bridge_dev = br;
> -
> -	if (!ds->max_num_bridges)
> +	bridge = dsa_tree_bridge_find(ds->dst, br);
> +	if (bridge) {
> +		refcount_inc(&bridge->refcount);
> +		dp->bridge = bridge;
>   		return 0;
> +	}
> +
> +	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
> +	if (!bridge)
> +		return -ENOMEM;
> +
> +	refcount_set(&bridge->refcount, 1);
> +
> +	bridge->dev = br;
>   
> -	bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges);
> -	if (!bridge_num) {
> +	bridge->num = dsa_bridge_num_get(br, ds->max_num_bridges);
> +	if (ds->max_num_bridges && !bridge->num) {
>   		NL_SET_ERR_MSG_MOD(extack,
>   				   "Range of offloadable bridges exceeded");
>   		return -EOPNOTSUPP;
>   	}
>   
> -	dp->bridge_num = bridge_num;
> +	dp->bridge = bridge;
>   
>   	return 0;
>   }
> @@ -332,16 +338,17 @@ static int dsa_port_bridge_create(struct dsa_port *dp,
>   static void dsa_port_bridge_destroy(struct dsa_port *dp,
>   				    const struct net_device *br)
>   {
> -	struct dsa_switch *ds = dp->ds;
> +	struct dsa_bridge *bridge = dp->bridge;
> +
> +	dp->bridge = NULL;
>   
> -	dp->bridge_dev = NULL;
> +	if (!refcount_dec_and_test(&bridge->refcount))
> +		return;
>   
> -	if (ds->max_num_bridges) {
> -		int bridge_num = dp->bridge_num;
> +	if (bridge->num)
> +		dsa_bridge_num_put(br, bridge->num);

Hm, since you're refcounting dp->bridge now, can't you simplify 
dsa_bridge_num_put() to just clear the bit in dsa_fwd_offloading_bridges 
without calling dsa_bridge_num_find() (which will always return 0 now, 
from what I can tell)? Or just squash the bridge number getting/putting 
logic into dsa_port_bridge_{create,destroy}.

>   
> -		dp->bridge_num = 0;
> -		dsa_bridge_num_put(br, bridge_num);
> -	}
> +	kfree(bridge);
>   }
>   
>   int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
> @@ -351,7 +358,6 @@ 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,
> -		.br = br,
>   	};
>   	struct net_device *dev = dp->slave;
>   	struct net_device *brport_dev;
> @@ -367,12 +373,12 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
>   
>   	brport_dev = dsa_port_to_bridge_port(dp);
>   
> +	info.bridge = *dp->bridge;
>   	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_JOIN, &info);
>   	if (err)
>   		goto out_rollback;
>   
> -	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
> -							dsa_port_bridge_num_get(dp));
> +	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, info.bridge);
>   
>   	err = switchdev_bridge_port_offload(brport_dev, dev, dp,
>   					    &dsa_slave_switchdev_notifier,
> @@ -415,12 +421,11 @@ void dsa_port_pre_bridge_leave(struct dsa_port *dp, struct net_device *br)
>   
>   void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
>   {
> -	unsigned int bridge_num = dsa_port_bridge_num_get(dp);
>   	struct dsa_notifier_bridge_info info = {
>   		.tree_index = dp->ds->dst->index,
>   		.sw_index = dp->ds->index,
>   		.port = dp->index,
> -		.br = br,
> +		.bridge = *dp->bridge,
>   	};
>   	int err;
>   
> @@ -429,7 +434,7 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
>   	 */
>   	dsa_port_bridge_destroy(dp, br);
>   
> -	dsa_port_bridge_tx_fwd_unoffload(dp, br, bridge_num);
> +	dsa_port_bridge_tx_fwd_unoffload(dp, info.bridge);
>   
>   	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
>   	if (err)
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 98fd5e972d28..a2918232bad6 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1564,7 +1564,7 @@ static void dsa_bridge_mtu_normalization(struct dsa_port *dp)
>   	if (!dp->ds->mtu_enforcement_ingress)
>   		return;
>   
> -	if (!dp->bridge_dev)
> +	if (!dp->bridge)
>   		return;
>   
>   	INIT_LIST_HEAD(&hw_port_list);
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index 7993192fe769..cd0630dd5417 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -95,7 +95,7 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
>   		if (!ds->ops->port_bridge_join)
>   			return -EOPNOTSUPP;
>   
> -		err = ds->ops->port_bridge_join(ds, info->port, info->br);
> +		err = ds->ops->port_bridge_join(ds, info->port, info->bridge);
>   		if (err)
>   			return err;
>   	}
> @@ -104,7 +104,7 @@ 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->br);
> +						     info->port, info->bridge);
>   		if (err)
>   			return err;
>   	}
> @@ -124,19 +124,20 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
>   
>   	if (dst->index == info->tree_index && ds->index == info->sw_index &&
>   	    ds->ops->port_bridge_leave)
> -		ds->ops->port_bridge_leave(ds, info->port, info->br);
> +		ds->ops->port_bridge_leave(ds, info->port, info->bridge);
>   
>   	if ((dst->index != info->tree_index || ds->index != info->sw_index) &&
>   	    ds->ops->crosschip_bridge_leave)
>   		ds->ops->crosschip_bridge_leave(ds, info->tree_index,
>   						info->sw_index, info->port,
> -						info->br);
> +						info->bridge);
>   
> -	if (ds->needs_standalone_vlan_filtering && !br_vlan_enabled(info->br)) {
> +	if (ds->needs_standalone_vlan_filtering &&
> +	    !br_vlan_enabled(info->bridge.dev)) {
>   		change_vlan_filtering = true;
>   		vlan_filtering = true;
>   	} else if (!ds->needs_standalone_vlan_filtering &&
> -		   br_vlan_enabled(info->br)) {
> +		   br_vlan_enabled(info->bridge.dev)) {
>   		change_vlan_filtering = true;
>   		vlan_filtering = false;
>   	}
> diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
> index e9d5e566973c..27712a81c967 100644
> --- a/net/dsa/tag_8021q.c
> +++ b/net/dsa/tag_8021q.c
> @@ -337,7 +337,7 @@ dsa_port_tag_8021q_bridge_match(struct dsa_port *dp,
>   		return false;
>   
>   	if (dsa_port_is_user(dp))
> -		return dsa_port_bridge_dev_get(dp) == info->br;
> +		return dsa_port_offloads_bridge(dp, &info->bridge);
>   
>   	return false;
>   }
> @@ -410,10 +410,9 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
>   }
>   
>   int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
> -					struct net_device *br,
> -					unsigned int bridge_num)
> +					struct dsa_bridge bridge)
>   {
> -	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
> +	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
>   
>   	return dsa_port_tag_8021q_vlan_add(dsa_to_port(ds, port), tx_vid,
>   					   true);
> @@ -421,10 +420,9 @@ int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
>   EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
>   
>   void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
> -					   struct net_device *br,
> -					   unsigned int bridge_num)
> +					   struct dsa_bridge bridge)
>   {
> -	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
> +	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge.num);
>   
>   	dsa_port_tag_8021q_vlan_del(dsa_to_port(ds, port), tx_vid, true);
>   }
> 


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

* Re: [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based
  2021-11-11 12:45     ` Vladimir Oltean
@ 2021-11-11 13:18       ` Alvin Šipraga
  0 siblings, 0 replies; 17+ messages in thread
From: Alvin Šipraga @ 2021-11-11 13:18 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Florian Fainelli, Andrew Lunn, Vivien Didelot,
	Tobias Waldekranz, DENG Qingfang

On 11/11/21 13:45, Vladimir Oltean wrote:
> On Thu, Nov 11, 2021 at 12:24:47PM +0000, Alvin Šipraga wrote:
>> On 10/26/21 18:26, Vladimir Oltean wrote:
>>> I have seen too many bugs already due to the fact that we must encode an
>>> invalid dp->bridge_num as a negative value, because the natural tendency
>>> is to check that invalid value using (!dp->bridge_num). Latest example
>>> can be seen in commit 1bec0f05062c ("net: dsa: fix bridge_num not
>>> getting cleared after ports leaving the bridge").
>>>
>>> Convert the existing users to assume that dp->bridge_num == 0 is the
>>> encoding for invalid, and valid bridge numbers start from 1.
>>>
>>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>>> ---
>>
>> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> Thanks for the review.
> 
>> Small remark inline.
>>
>>> -int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
>>> +unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
>>>    {
>>> -	int bridge_num = dsa_bridge_num_find(bridge_dev);
>>> +	unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
>>>    
>>> -	if (bridge_num < 0) {
>>> +	if (!bridge_num) {
>>>    		/* First port that offloads TX forwarding for this bridge */
>>
>> Perhaps you want to update this comment in patch 2/6, since bridge_num
>> is no longer just about TX forwarding offload.
>>
>>> -		bridge_num = find_first_zero_bit(&dsa_fwd_offloading_bridges,
>>> -						 DSA_MAX_NUM_OFFLOADING_BRIDGES);
>>> +		bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
>>> +						DSA_MAX_NUM_OFFLOADING_BRIDGES,
>>> +						1);
> 
> I will update this comment in patch 2 to say "First port that requests
> FDB isolation or TX forwarding offload for this bridge". Sounds ok?
> 

Yes sounds good - that's also what I had in mind.

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

* Re: [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload
  2021-10-26 16:26 ` [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
@ 2021-11-11 13:23   ` Alvin Šipraga
  2021-11-19 15:38   ` Tobias Waldekranz
  1 sibling, 0 replies; 17+ messages in thread
From: Alvin Šipraga @ 2021-11-11 13:23 UTC (permalink / raw)
  To: Vladimir Oltean, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Tobias Waldekranz,
	DENG Qingfang

On 10/26/21 18:26, Vladimir Oltean wrote:
> We don't really need new switch API for these, and with new switches
> which intend to add support for this feature, it will become cumbersome
> to maintain.
> 
> Simply pass a boolean argument to ->port_bridge_join which the driver
> must set to true if it implements the TX forwarding offload feature.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

>   drivers/net/dsa/b53/b53_common.c       |  3 +-
>   drivers/net/dsa/b53/b53_priv.h         |  3 +-
>   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 |  2 +-
>   drivers/net/dsa/mt7530.c               |  2 +-
>   drivers/net/dsa/mv88e6xxx/chip.c       | 71 ++++++++++++--------------
>   drivers/net/dsa/ocelot/felix.c         |  2 +-
>   drivers/net/dsa/qca8k.c                |  3 +-
>   drivers/net/dsa/rtl8366rb.c            |  3 +-
>   drivers/net/dsa/sja1105/sja1105_main.c | 22 ++++++--
>   drivers/net/dsa/xrs700x/xrs700x.c      |  2 +-
>   include/net/dsa.h                      | 10 ++--
>   net/dsa/dsa_priv.h                     |  1 +
>   net/dsa/port.c                         | 39 ++------------
>   net/dsa/switch.c                       |  3 +-
>   19 files changed, 81 insertions(+), 100 deletions(-)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 4e41b1a63108..3867f3d4545f 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -1860,7 +1860,8 @@ 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)
> +int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
> +		bool *tx_fwd_offload)
>   {
>   	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 ee17f8b516ca..b41dc8ac2ca8 100644
> --- a/drivers/net/dsa/b53/b53_priv.h
> +++ b/drivers/net/dsa/b53/b53_priv.h
> @@ -324,7 +324,8 @@ void b53_get_strings(struct dsa_switch *ds, int port, u32 stringset,
>   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);
> +int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
> +		bool *tx_fwd_offload);
>   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 70db3a9aa355..33daaf10c488 100644
> --- a/drivers/net/dsa/dsa_loop.c
> +++ b/drivers/net/dsa/dsa_loop.c
> @@ -167,7 +167,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)
> +				     struct dsa_bridge bridge,
> +				     bool *tx_fwd_offload)
>   {
>   	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 17c2d13670a9..53ab5f8ae92e 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -674,7 +674,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)
> +				      struct dsa_bridge bridge,
> +				      bool *tx_fwd_offload)
>   {
>   	struct hellcreek *hellcreek = ds->priv;
>   
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index 29d909484275..d55784d19fa4 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -1103,7 +1103,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)
> +				    struct dsa_bridge bridge,
> +				    bool *tx_fwd_offload)
>   {
>   	struct lan9303 *chip = ds->priv;
>   
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index 6b9b9c712e58..6b5bef7f2a59 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -1146,7 +1146,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)
> +				  struct dsa_bridge bridge,
> +				  bool *tx_fwd_offload)
>   {
>   	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 23784d14a5da..1d13aa56e7d1 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -173,7 +173,8 @@ void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
>   EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
>   
>   int ksz_port_bridge_join(struct dsa_switch *ds, int port,
> -			 struct dsa_bridge bridge)
> +			 struct dsa_bridge bridge,
> +			 bool *tx_fwd_offload)
>   {
>   	struct ksz_device *dev = ds->priv;
>   
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index df953c2bc56e..b444df218c2d 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -159,7 +159,7 @@ 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);
> +			 struct dsa_bridge bridge, bool *tx_fwd_offload);
>   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 85cc5aca7f96..dcd1a3932cfb 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1186,7 +1186,7 @@ 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)
> +			struct dsa_bridge bridge, bool *tx_fwd_offload)
>   {
>   	struct mt7530_priv *priv = ds->priv;
>   	u32 port_bitmap = BIT(MT7530_CPU_PORT);
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 73613a667f6c..0aac71dece29 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2448,8 +2448,27 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
>   	return 0;
>   }
>   
> +/* Treat the software bridge as a virtual single-port switch behind the
> + * CPU and map in the PVT. First dst->last_switch elements are taken by
> + * physical switches, so start from beyond that range.
> + */
> +static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
> +					       unsigned int bridge_num)
> +{
> +	u8 dev = bridge_num + ds->dst->last_switch;
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	int err;
> +
> +	mv88e6xxx_reg_lock(chip);
> +	err = mv88e6xxx_pvt_map(chip, dev, 0);
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	return err;
> +}
> +
>   static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
> -				      struct dsa_bridge bridge)
> +				      struct dsa_bridge bridge,
> +				      bool *tx_fwd_offload)
>   {
>   	struct mv88e6xxx_chip *chip = ds->priv;
>   	int err;
> @@ -2464,6 +2483,14 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
>   	if (err)
>   		goto unlock;
>   
> +	if (mv88e6xxx_has_pvt(chip)) {
> +		err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
> +		if (err)
> +			goto unlock;
> +
> +		*tx_fwd_offload = true;
> +	}
> +
>   unlock:
>   	mv88e6xxx_reg_unlock(chip);
>   
> @@ -2478,6 +2505,10 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
>   
>   	mv88e6xxx_reg_lock(chip);
>   
> +	if (bridge.tx_fwd_offload &&
> +	    mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num))
> +		dev_err(ds->dev, "failed to remap cross-chip Port VLAN\n");
> +
>   	if (mv88e6xxx_bridge_map(chip, bridge) ||
>   	    mv88e6xxx_port_vlan_map(chip, port))
>   		dev_err(ds->dev, "failed to remap in-chip Port VLAN\n");
> @@ -2523,42 +2554,6 @@ static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
>   	mv88e6xxx_reg_unlock(chip);
>   }
>   
> -/* Treat the software bridge as a virtual single-port switch behind the
> - * CPU and map in the PVT. First dst->last_switch elements are taken by
> - * physical switches, so start from beyond that range.
> - */
> -static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
> -					       unsigned int bridge_num)
> -{
> -	u8 dev = bridge_num + ds->dst->last_switch;
> -	struct mv88e6xxx_chip *chip = ds->priv;
> -	int err;
> -
> -	mv88e6xxx_reg_lock(chip);
> -	err = mv88e6xxx_pvt_map(chip, dev, 0);
> -	mv88e6xxx_reg_unlock(chip);
> -
> -	return err;
> -}
> -
> -static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
> -					   struct dsa_bridge bridge)
> -{
> -	return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
> -}
> -
> -static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
> -					      struct dsa_bridge bridge)
> -{
> -	int err;
> -
> -	err = mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge.num);
> -	if (err) {
> -		dev_err(ds->dev, "failed to remap cross-chip Port VLAN: %pe\n",
> -			ERR_PTR(err));
> -	}
> -}
> -
>   static int mv88e6xxx_software_reset(struct mv88e6xxx_chip *chip)
>   {
>   	if (chip->info->ops->reset)
> @@ -6278,8 +6273,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
>   	.crosschip_lag_change	= mv88e6xxx_crosschip_lag_change,
>   	.crosschip_lag_join	= mv88e6xxx_crosschip_lag_join,
>   	.crosschip_lag_leave	= mv88e6xxx_crosschip_lag_leave,
> -	.port_bridge_tx_fwd_offload = mv88e6xxx_bridge_tx_fwd_offload,
> -	.port_bridge_tx_fwd_unoffload = mv88e6xxx_bridge_tx_fwd_unoffload,
>   };
>   
>   static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index 8cf82fa91a6b..001213891b91 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -698,7 +698,7 @@ 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)
> +			     struct dsa_bridge bridge, bool *tx_fwd_offload)
>   {
>   	struct ocelot *ocelot = ds->priv;
>   
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 3ae10f22ada9..390dad77f062 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -1729,7 +1729,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)
> +				  struct dsa_bridge bridge,
> +				  bool *tx_fwd_offload)
>   {
>   	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
>   	int port_mask, cpu_port;
> diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
> index fac2333a3f5e..ecc19bd5115f 100644
> --- a/drivers/net/dsa/rtl8366rb.c
> +++ b/drivers/net/dsa/rtl8366rb.c
> @@ -1186,7 +1186,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)
> +			   struct dsa_bridge bridge,
> +			   bool *tx_fwd_offload)
>   {
>   	struct realtek_smi *smi = 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 24584fe2e760..cefde41ce8d6 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -2074,14 +2074,30 @@ 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)
> +			       struct dsa_bridge bridge,
> +			       bool *tx_fwd_offload)
>   {
> -	return sja1105_bridge_member(ds, port, bridge, true);
> +	int rc;
> +
> +	rc = sja1105_bridge_member(ds, port, bridge, true);
> +	if (rc)
> +		return rc;
> +
> +	rc = dsa_tag_8021q_bridge_tx_fwd_offload(ds, port, bridge);
> +	if (rc) {
> +		sja1105_bridge_member(ds, port, bridge, false);
> +		return rc;
> +	}
> +
> +	*tx_fwd_offload = true;
> +
> +	return 0;
>   }
>   
>   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);
>   	sja1105_bridge_member(ds, port, bridge, false);
>   }
>   
> @@ -3230,8 +3246,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
>   	.tag_8021q_vlan_add	= sja1105_dsa_8021q_vlan_add,
>   	.tag_8021q_vlan_del	= sja1105_dsa_8021q_vlan_del,
>   	.port_prechangeupper	= sja1105_prechangeupper,
> -	.port_bridge_tx_fwd_offload = dsa_tag_8021q_bridge_tx_fwd_offload,
> -	.port_bridge_tx_fwd_unoffload = dsa_tag_8021q_bridge_tx_fwd_unoffload,
>   };
>   
>   static const struct of_device_id sja1105_dt_ids[];
> diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
> index ebb55dfd9c4e..35fa19ddaf19 100644
> --- a/drivers/net/dsa/xrs700x/xrs700x.c
> +++ b/drivers/net/dsa/xrs700x/xrs700x.c
> @@ -540,7 +540,7 @@ 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)
> +			       struct dsa_bridge bridge, bool *tx_fwd_offload)
>   {
>   	return xrs700x_bridge_common(ds, port, bridge, true);
>   }
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 5e83c64bc96a..94dc8b5ee9c8 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -222,6 +222,7 @@ struct dsa_mall_tc_entry {
>   struct dsa_bridge {
>   	struct net_device *dev;
>   	unsigned int num;
> +	bool tx_fwd_offload;
>   	refcount_t refcount;
>   };
>   
> @@ -814,15 +815,10 @@ 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);
> +				    struct dsa_bridge bridge,
> +				    bool *tx_fwd_offload);
>   	void	(*port_bridge_leave)(struct dsa_switch *ds, int port,
>   				     struct dsa_bridge bridge);
> -	/* Called right after .port_bridge_join() */
> -	int	(*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
> -					      struct dsa_bridge bridge);
> -	/* Called right before .port_bridge_leave() */
> -	void	(*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
> -						struct dsa_bridge bridge);
>   	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
>   				      u8 state);
>   	void	(*port_fast_age)(struct dsa_switch *ds, int port);
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 489712ead08f..5abe6864967a 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -56,6 +56,7 @@ struct dsa_notifier_bridge_info {
>   	int tree_index;
>   	int sw_index;
>   	int port;
> +	bool tx_fwd_offload;
>   };
>   
>   /* DSA_NOTIFIER_FDB_* */
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index b2e66994f72c..b1113788bf9b 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -270,37 +270,6 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
>   	 */
>   }
>   
> -static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
> -					     struct dsa_bridge bridge)
> -{
> -	struct dsa_switch *ds = dp->ds;
> -
> -	/* No bridge TX forwarding offload => do nothing */
> -	if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge.num)
> -		return;
> -
> -	/* Notify the chips only once the offload has been deactivated, so
> -	 * that they can update their configuration accordingly.
> -	 */
> -	ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge);
> -}
> -
> -static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
> -					   struct dsa_bridge bridge)
> -{
> -	struct dsa_switch *ds = dp->ds;
> -	int err;
> -
> -	/* FDB isolation is required for TX forwarding offload */
> -	if (!ds->ops->port_bridge_tx_fwd_offload || !bridge.num)
> -		return false;
> -
> -	/* Notify the driver */
> -	err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge);
> -
> -	return err ? false : true;
> -}
> -
>   static int dsa_port_bridge_create(struct dsa_port *dp,
>   				  struct net_device *br,
>   				  struct netlink_ext_ack *extack)
> @@ -361,7 +330,6 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
>   	};
>   	struct net_device *dev = dp->slave;
>   	struct net_device *brport_dev;
> -	bool tx_fwd_offload;
>   	int err;
>   
>   	/* Here the interface is already bridged. Reflect the current
> @@ -378,12 +346,13 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
>   	if (err)
>   		goto out_rollback;
>   
> -	tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, info.bridge);
> +	/* Drivers which support bridge TX forwarding should set this */
> +	dp->bridge->tx_fwd_offload = info.tx_fwd_offload;
>   
>   	err = switchdev_bridge_port_offload(brport_dev, dev, dp,
>   					    &dsa_slave_switchdev_notifier,
>   					    &dsa_slave_switchdev_blocking_notifier,
> -					    tx_fwd_offload, extack);
> +					    dp->bridge->tx_fwd_offload, extack);
>   	if (err)
>   		goto out_rollback_unbridge;
>   
> @@ -434,8 +403,6 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
>   	 */
>   	dsa_port_bridge_destroy(dp, br);
>   
> -	dsa_port_bridge_tx_fwd_unoffload(dp, info.bridge);
> -
>   	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
>   	if (err)
>   		dev_err(dp->ds->dev,
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index cd0630dd5417..9c92edd96961 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -95,7 +95,8 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
>   		if (!ds->ops->port_bridge_join)
>   			return -EOPNOTSUPP;
>   
> -		err = ds->ops->port_bridge_join(ds, info->port, info->bridge);
> +		err = ds->ops->port_bridge_join(ds, info->port, info->bridge,
> +						&info->tx_fwd_offload);
>   		if (err)
>   			return err;
>   	}
> 


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

* Re: [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload
  2021-10-26 16:26 ` [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
  2021-11-11 13:23   ` Alvin Šipraga
@ 2021-11-19 15:38   ` Tobias Waldekranz
  1 sibling, 0 replies; 17+ messages in thread
From: Tobias Waldekranz @ 2021-11-19 15:38 UTC (permalink / raw)
  To: Vladimir Oltean, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, DENG Qingfang,
	Alvin Šipraga

On Tue, Oct 26, 2021 at 19:26, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> We don't really need new switch API for these, and with new switches
> which intend to add support for this feature, it will become cumbersome
> to maintain.
>
> Simply pass a boolean argument to ->port_bridge_join which the driver
> must set to true if it implements the TX forwarding offload feature.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/b53/b53_common.c       |  3 +-
>  drivers/net/dsa/b53/b53_priv.h         |  3 +-
>  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 |  2 +-
>  drivers/net/dsa/mt7530.c               |  2 +-
>  drivers/net/dsa/mv88e6xxx/chip.c       | 71 ++++++++++++--------------
>  drivers/net/dsa/ocelot/felix.c         |  2 +-
>  drivers/net/dsa/qca8k.c                |  3 +-
>  drivers/net/dsa/rtl8366rb.c            |  3 +-
>  drivers/net/dsa/sja1105/sja1105_main.c | 22 ++++++--
>  drivers/net/dsa/xrs700x/xrs700x.c      |  2 +-
>  include/net/dsa.h                      | 10 ++--
>  net/dsa/dsa_priv.h                     |  1 +
>  net/dsa/port.c                         | 39 ++------------
>  net/dsa/switch.c                       |  3 +-
>  19 files changed, 81 insertions(+), 100 deletions(-)
>
...
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 73613a667f6c..0aac71dece29 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2448,8 +2448,27 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
>  	return 0;
>  }
>  
> +/* Treat the software bridge as a virtual single-port switch behind the
> + * CPU and map in the PVT. First dst->last_switch elements are taken by
> + * physical switches, so start from beyond that range.
> + */
> +static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
> +					       unsigned int bridge_num)
> +{
> +	u8 dev = bridge_num + ds->dst->last_switch;
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	int err;
> +
> +	mv88e6xxx_reg_lock(chip);

This deadlocks every time. The caller already holds this lock from both
call sites.

> +	err = mv88e6xxx_pvt_map(chip, dev, 0);
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	return err;
> +}
> +
...

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

end of thread, other threads:[~2021-11-19 15:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
2021-10-26 16:26 ` [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based Vladimir Oltean
2021-11-11 12:24   ` Alvin Šipraga
2021-11-11 12:45     ` Vladimir Oltean
2021-11-11 13:18       ` Alvin Šipraga
2021-10-26 16:26 ` [RFC PATCH net-next 2/6] net: dsa: assign a bridge number even without TX forwarding offload Vladimir Oltean
2021-11-11 12:27   ` Alvin Šipraga
2021-10-26 16:26 ` [RFC PATCH net-next 3/6] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers Vladimir Oltean
2021-10-26 16:26 ` [RFC PATCH net-next 4/6] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev Vladimir Oltean
2021-11-11 12:33   ` Alvin Šipraga
2021-10-26 16:26 ` [RFC PATCH net-next 5/6] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
2021-11-11 13:17   ` Alvin Šipraga
2021-10-26 16:26 ` [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
2021-11-11 13:23   ` Alvin Šipraga
2021-11-19 15:38   ` Tobias Waldekranz
2021-11-02 10:07 ` [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
2021-11-04 11:39   ` Tobias Waldekranz

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.