[PATCH net-next 0/5] dpaa2-switch: FDB management refactoring

[PATCH net-next 0/5] dpaa2-switch: FDB management refactoring

[Ioana Ciornei]

The FDB management done by the dpaa2_switch_port_set_fdb() function is
hard to follow even by trained eyes. This series tries to make it easier
to read and understand it by factoring out some code blocks into helper
functions and unifying the join and leave paths in terms of FDB
management.

Ioana Ciornei (5):
  dpaa2-switch: change dpaa2_switch_port_set_fdb() function prototype
  dpaa2-switch: factor out the FDB in-use check into a helper
  dpaa2-switch: move FDB selection for join path into a helper
  dpaa2-switch: move FDB selection for leave path into a helper
  dpaa2-switch: unify the FDB update logic in
    dpaa2_switch_port_set_fdb()

 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 128 ++++++++++--------
 1 file changed, 70 insertions(+), 58 deletions(-)

-- 
2.25.1

[PATCH net-next 1/5] dpaa2-switch: change dpaa2_switch_port_set_fdb() function prototype

[Ioana Ciornei]

Since there dpaa2_switch_port_set_fdb() never fails and its return value
was never checked, change its prototype to return void.

Also, instead of determining if the DPAA2 port is joining or leaving an
upper based on the value of the 'bridge_dev' parameter, add the
'linking' parameter to explicitly specify the action.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 29 +++++++++----------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index a0bf5b50aae5..6ac747b4264c 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -51,8 +51,9 @@ dpaa2_switch_filter_block_get_unused(struct ethsw_core *ethsw)
 	return NULL;
 }
 
-static u16 dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
-				     struct net_device *bridge_dev)
+static void dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
+				      struct net_device *upper_dev,
+				      bool linking)
 {
 	struct ethsw_core *ethsw = port_priv->ethsw_data;
 	struct ethsw_port_priv *other_port_priv = NULL;
@@ -62,10 +63,8 @@ static u16 dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 	struct list_head *iter;
 	int i;
 
-	/* If we leave a bridge (bridge_dev is NULL), find an unused
-	 * FDB and use that.
-	 */
-	if (!bridge_dev) {
+	/* If we leave a bridge, find an unused FDB and use that. */
+	if (!linking) {
 		/* First verify if this is the last port to leave this bridge */
 		for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
 			if (!ethsw->ports[i] || ethsw->ports[i] == port_priv)
@@ -79,7 +78,7 @@ static u16 dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 		/* If this is the last user of the FDB, just keep using it. */
 		if (last_fdb_user) {
 			port_priv->fdb->bridge_dev = NULL;
-			return 0;
+			return;
 		}
 
 		/* Since we are not the last port which leaves a bridge,
@@ -90,12 +89,12 @@ static u16 dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 		 */
 		fdb = dpaa2_switch_fdb_get_unused(port_priv->ethsw_data);
 		if (WARN_ON(!fdb))
-			return 0;
+			return;
 
 		port_priv->fdb = fdb;
 		port_priv->fdb->in_use = true;
 		port_priv->fdb->bridge_dev = NULL;
-		return 0;
+		return;
 	}
 
 	/* The below call to netdev_for_each_lower_dev() demands the RTNL lock
@@ -107,7 +106,7 @@ static u16 dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 	/* If part of a bridge, use the FDB of the first dpaa2 switch interface
 	 * to be present in that bridge
 	 */
-	netdev_for_each_lower_dev(bridge_dev, other_dev, iter) {
+	netdev_for_each_lower_dev(upper_dev, other_dev, iter) {
 		if (!dpaa2_switch_port_dev_check(other_dev))
 			continue;
 
@@ -133,9 +132,7 @@ static u16 dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 	}
 
 	/* Keep track of the new upper bridge device */
-	port_priv->fdb->bridge_dev = bridge_dev;
-
-	return 0;
+	port_priv->fdb->bridge_dev = upper_dev;
 }
 
 static void dpaa2_switch_fdb_get_flood_cfg(struct ethsw_core *ethsw, u16 fdb_id,
@@ -2051,7 +2048,7 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
 	if (err)
 		return err;
 
-	dpaa2_switch_port_set_fdb(port_priv, upper_dev);
+	dpaa2_switch_port_set_fdb(port_priv, upper_dev, true);
 
 	/* Inherit the initial bridge port learning state */
 	learn_ena = br_port_flag_is_set(netdev, BR_LEARNING);
@@ -2077,7 +2074,7 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
 
 err_switchdev_offload:
 err_egress_flood:
-	dpaa2_switch_port_set_fdb(port_priv, NULL);
+	dpaa2_switch_port_set_fdb(port_priv, upper_dev, false);
 	return err;
 }
 
@@ -2124,7 +2121,7 @@ static int dpaa2_switch_port_bridge_leave(struct net_device *netdev)
 	if (err)
 		netdev_err(netdev, "Unable to clear RX VLANs from old FDB table, err (%d)\n", err);
 
-	dpaa2_switch_port_set_fdb(port_priv, NULL);
+	dpaa2_switch_port_set_fdb(port_priv, port_priv->fdb->bridge_dev, false);
 
 	/* Restore all RX VLANs into the new FDB table that we just joined */
 	err = vlan_for_each(netdev, dpaa2_switch_port_restore_rxvlan, netdev);
-- 
2.25.1

[PATCH net-next 2/5] dpaa2-switch: factor out the FDB in-use check into a helper

[Ioana Ciornei]

The dpaa2_switch_port_set_fdb() function is hard to follow and
open-coding the in-use check into it makes it even harder to read.
Factor out that code block into a new helper -
dpaa2_switch_fdb_in_use_by_others().

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 31 +++++++++++--------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 6ac747b4264c..3052a1c63c2d 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -51,6 +51,22 @@ dpaa2_switch_filter_block_get_unused(struct ethsw_core *ethsw)
 	return NULL;
 }
 
+static bool dpaa2_switch_fdb_in_use_by_others(struct ethsw_core *ethsw,
+					      struct dpaa2_switch_fdb *fdb,
+					      struct ethsw_port_priv *except)
+{
+	int i;
+
+	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
+		if (!ethsw->ports[i] || ethsw->ports[i] == except)
+			continue;
+		if (ethsw->ports[i]->fdb == fdb)
+			return true;
+	}
+
+	return false;
+}
+
 static void dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 				      struct net_device *upper_dev,
 				      bool linking)
@@ -59,24 +75,13 @@ static void dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 	struct ethsw_port_priv *other_port_priv = NULL;
 	struct dpaa2_switch_fdb *fdb;
 	struct net_device *other_dev;
-	bool last_fdb_user = true;
 	struct list_head *iter;
-	int i;
 
 	/* If we leave a bridge, find an unused FDB and use that. */
 	if (!linking) {
-		/* First verify if this is the last port to leave this bridge */
-		for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
-			if (!ethsw->ports[i] || ethsw->ports[i] == port_priv)
-				continue;
-			if (ethsw->ports[i]->fdb == port_priv->fdb) {
-				last_fdb_user = false;
-				break;
-			}
-		}
-
 		/* If this is the last user of the FDB, just keep using it. */
-		if (last_fdb_user) {
+		if (!dpaa2_switch_fdb_in_use_by_others(ethsw, port_priv->fdb,
+						       port_priv)) {
 			port_priv->fdb->bridge_dev = NULL;
 			return;
 		}
-- 
2.25.1

[PATCH net-next 3/5] dpaa2-switch: move FDB selection for join path into a helper

[Ioana Ciornei]

The dpaa2_switch_port_set_fdb() function handles the setup of the FDB
for both changeupper cases: join and leave. Move the code block which
handles the join path into a new helper - dpaa2_switch_fdb_for_join() -
with the hope that the entire function will become easier to read and
extend with other use cases in the future.

This new helper just determines and returns what FDB should be used for
a specific port, the cleanup of the old FDB and the actual setup in the
per port structure remains in the dpaa2_switch_port_set_fdb() function.

No changes in the actual behavior are intended.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 60 +++++++++++--------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 3052a1c63c2d..158d0f510eae 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -67,15 +67,43 @@ static bool dpaa2_switch_fdb_in_use_by_others(struct ethsw_core *ethsw,
 	return false;
 }
 
+static struct dpaa2_switch_fdb *
+dpaa2_switch_fdb_for_join(struct ethsw_port_priv *port_priv,
+			  struct net_device *upper_dev)
+{
+	struct ethsw_port_priv *other_port_priv;
+	struct net_device *other_dev;
+	struct list_head *iter;
+
+	/* The below call to netdev_for_each_lower_dev() demands the RTNL lock
+	 * being held. Assert on it so that it's easier to catch new code
+	 * paths that reach this point without the RTNL lock.
+	 */
+	ASSERT_RTNL();
+
+	/* If part of a bridge, use the FDB of the first dpaa2 switch interface
+	 * to be present in that bridge
+	 */
+	netdev_for_each_lower_dev(upper_dev, other_dev, iter) {
+		if (!dpaa2_switch_port_dev_check(other_dev))
+			continue;
+
+		if (other_dev == port_priv->netdev)
+			continue;
+
+		other_port_priv = netdev_priv(other_dev);
+		return other_port_priv->fdb;
+	}
+
+	return port_priv->fdb;
+}
+
 static void dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 				      struct net_device *upper_dev,
 				      bool linking)
 {
 	struct ethsw_core *ethsw = port_priv->ethsw_data;
-	struct ethsw_port_priv *other_port_priv = NULL;
-	struct dpaa2_switch_fdb *fdb;
-	struct net_device *other_dev;
-	struct list_head *iter;
+	struct dpaa2_switch_fdb *new_fdb, *fdb;
 
 	/* If we leave a bridge, find an unused FDB and use that. */
 	if (!linking) {
@@ -102,30 +130,12 @@ static void dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 		return;
 	}
 
-	/* The below call to netdev_for_each_lower_dev() demands the RTNL lock
-	 * being held. Assert on it so that it's easier to catch new code
-	 * paths that reach this point without the RTNL lock.
-	 */
-	ASSERT_RTNL();
-
-	/* If part of a bridge, use the FDB of the first dpaa2 switch interface
-	 * to be present in that bridge
-	 */
-	netdev_for_each_lower_dev(upper_dev, other_dev, iter) {
-		if (!dpaa2_switch_port_dev_check(other_dev))
-			continue;
-
-		if (other_dev == port_priv->netdev)
-			continue;
-
-		other_port_priv = netdev_priv(other_dev);
-		break;
-	}
+	new_fdb = dpaa2_switch_fdb_for_join(port_priv, upper_dev);
 
 	/* The current port is about to change its FDB to the one used by the
 	 * first port that joined the bridge.
 	 */
-	if (other_port_priv) {
+	if (port_priv->fdb != new_fdb) {
 		/* The previous FDB is about to become unused, since the
 		 * interface is no longer standalone.
 		 */
@@ -133,7 +143,7 @@ static void dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 		port_priv->fdb->bridge_dev = NULL;
 
 		/* Get a reference to the new FDB */
-		port_priv->fdb = other_port_priv->fdb;
+		port_priv->fdb = new_fdb;
 	}
 
 	/* Keep track of the new upper bridge device */
-- 
2.25.1

[PATCH net-next 4/5] dpaa2-switch: move FDB selection for leave path into a helper

[Ioana Ciornei]

Move the FDB selection for when a port leaves bridge into a new helper -
dpaa2_switch_fdb_for_leave(). This will hopefully make the
dpaa2_switch_port_set_fdb() function easier to read and follow. The new
helper only determines the FDB to be used, any updates into the private
port structure still gets done in the set_fdb() function.

No changes in the actual behavior are intended.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 48 ++++++++++++-------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 158d0f510eae..09604c84a614 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -98,35 +98,47 @@ dpaa2_switch_fdb_for_join(struct ethsw_port_priv *port_priv,
 	return port_priv->fdb;
 }
 
+static struct dpaa2_switch_fdb *
+dpaa2_switch_fdb_for_leave(struct ethsw_port_priv *port_priv)
+{
+	struct ethsw_core *ethsw = port_priv->ethsw_data;
+
+	/* If this is the last user of the FDB, just keep using it. */
+	if (!dpaa2_switch_fdb_in_use_by_others(ethsw, port_priv->fdb,
+					       port_priv)) {
+		return port_priv->fdb;
+	}
+
+	/* Since we are not the last port which leaves a bridge,
+	 * acquire a new FDB and use it.
+	 */
+	return dpaa2_switch_fdb_get_unused(ethsw);
+}
+
 static void dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 				      struct net_device *upper_dev,
 				      bool linking)
 {
-	struct ethsw_core *ethsw = port_priv->ethsw_data;
-	struct dpaa2_switch_fdb *new_fdb, *fdb;
+	struct dpaa2_switch_fdb *new_fdb;
 
 	/* If we leave a bridge, find an unused FDB and use that. */
 	if (!linking) {
-		/* If this is the last user of the FDB, just keep using it. */
-		if (!dpaa2_switch_fdb_in_use_by_others(ethsw, port_priv->fdb,
-						       port_priv)) {
-			port_priv->fdb->bridge_dev = NULL;
-			return;
-		}
-
-		/* Since we are not the last port which leaves a bridge,
-		 * acquire a new FDB and use it. The number of FDBs is sized to
-		 * accommodate all switch ports as standalone, each with its
-		 * private FDB, which means that dpaa2_switch_fdb_get_unused()
-		 * must succeed here. WARN if not.
+		/* The number of FDBs is sized to accommodate all switch ports
+		 * as standalone, each with its private FDB, which means that
+		 * dpaa2_switch_fdb_get_unused() must succeed here. WARN if
+		 * not.
 		 */
-		fdb = dpaa2_switch_fdb_get_unused(port_priv->ethsw_data);
-		if (WARN_ON(!fdb))
+		new_fdb = dpaa2_switch_fdb_for_leave(port_priv);
+		if (WARN_ON(!new_fdb))
 			return;
 
-		port_priv->fdb = fdb;
-		port_priv->fdb->in_use = true;
+		if (port_priv->fdb != new_fdb) {
+			port_priv->fdb = new_fdb;
+			port_priv->fdb->in_use = true;
+		}
+
 		port_priv->fdb->bridge_dev = NULL;
+
 		return;
 	}
 
-- 
2.25.1

[PATCH net-next 5/5] dpaa2-switch: unify the FDB update logic in dpaa2_switch_port_set_fdb()

[Ioana Ciornei]

For both the join and leave paths, the logic goes through the following
steps: determines which FDB should be used on a port after the current
changeupper change, populate the private port structures with the new
FDB and, if necessary, make as not used the old FDB.
Instead of having two distinct paths inside the
dpaa2_switch_port_set_fdb() for linking=true and linking=false, unify
them. This will hopefully help in making this function easier to read.

No behavior changes are expected.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 50 +++++++------------
 1 file changed, 19 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 09604c84a614..45f276c2c3ec 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -119,47 +119,35 @@ static void dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
 				      struct net_device *upper_dev,
 				      bool linking)
 {
+	struct dpaa2_switch_fdb *old_fdb = port_priv->fdb;
+	struct ethsw_core *ethsw = port_priv->ethsw_data;
 	struct dpaa2_switch_fdb *new_fdb;
 
-	/* If we leave a bridge, find an unused FDB and use that. */
-	if (!linking) {
-		/* The number of FDBs is sized to accommodate all switch ports
-		 * as standalone, each with its private FDB, which means that
-		 * dpaa2_switch_fdb_get_unused() must succeed here. WARN if
-		 * not.
-		 */
-		new_fdb = dpaa2_switch_fdb_for_leave(port_priv);
-		if (WARN_ON(!new_fdb))
-			return;
-
-		if (port_priv->fdb != new_fdb) {
-			port_priv->fdb = new_fdb;
-			port_priv->fdb->in_use = true;
-		}
-
-		port_priv->fdb->bridge_dev = NULL;
-
+	new_fdb = linking ? dpaa2_switch_fdb_for_join(port_priv, upper_dev) :
+			    dpaa2_switch_fdb_for_leave(port_priv);
+	/* The number of FDBs is sized to accommodate all switch ports as
+	 * standalone, each with its private FDB, which means that FDB must be
+	 * valid here even on the leave path. WARN if not.
+	 */
+	if (WARN_ON(!new_fdb))
 		return;
-	}
 
-	new_fdb = dpaa2_switch_fdb_for_join(port_priv, upper_dev);
-
-	/* The current port is about to change its FDB to the one used by the
-	 * first port that joined the bridge.
-	 */
-	if (port_priv->fdb != new_fdb) {
-		/* The previous FDB is about to become unused, since the
-		 * interface is no longer standalone.
+	if (old_fdb != new_fdb) {
+		/* The previous FDB is about to become unused, release
+		 * it if we are the last user.
 		 */
-		port_priv->fdb->in_use = false;
-		port_priv->fdb->bridge_dev = NULL;
+		if (!dpaa2_switch_fdb_in_use_by_others(ethsw, port_priv->fdb,
+						       port_priv)) {
+			old_fdb->in_use = false;
+			old_fdb->bridge_dev = NULL;
+		}
 
-		/* Get a reference to the new FDB */
+		new_fdb->in_use = true;
 		port_priv->fdb = new_fdb;
 	}
 
 	/* Keep track of the new upper bridge device */
-	port_priv->fdb->bridge_dev = upper_dev;
+	port_priv->fdb->bridge_dev = linking ? upper_dev : NULL;
 }
 
 static void dpaa2_switch_fdb_get_flood_cfg(struct ethsw_core *ethsw, u16 fdb_id,
-- 
2.25.1

Re: [PATCH net-next 0/5] dpaa2-switch: FDB management refactoring

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 10 Jun 2026 18:09:07 +0300 you wrote:
> The FDB management done by the dpaa2_switch_port_set_fdb() function is
> hard to follow even by trained eyes. This series tries to make it easier
> to read and understand it by factoring out some code blocks into helper
> functions and unifying the join and leave paths in terms of FDB
> management.
> 
> Ioana Ciornei (5):
>   dpaa2-switch: change dpaa2_switch_port_set_fdb() function prototype
>   dpaa2-switch: factor out the FDB in-use check into a helper
>   dpaa2-switch: move FDB selection for join path into a helper
>   dpaa2-switch: move FDB selection for leave path into a helper
>   dpaa2-switch: unify the FDB update logic in
>     dpaa2_switch_port_set_fdb()
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] dpaa2-switch: change dpaa2_switch_port_set_fdb() function prototype
    https://git.kernel.org/netdev/net-next/c/646077cc6b20
  - [net-next,2/5] dpaa2-switch: factor out the FDB in-use check into a helper
    https://git.kernel.org/netdev/net-next/c/5617bf8538ec
  - [net-next,3/5] dpaa2-switch: move FDB selection for join path into a helper
    https://git.kernel.org/netdev/net-next/c/e31f457ac7da
  - [net-next,4/5] dpaa2-switch: move FDB selection for leave path into a helper
    https://git.kernel.org/netdev/net-next/c/2230a2e62251
  - [net-next,5/5] dpaa2-switch: unify the FDB update logic in dpaa2_switch_port_set_fdb()
    https://git.kernel.org/netdev/net-next/c/7aae797a003e

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