Re: [PATCH net-next 03/13] dpaa2-switch: change dpaa2_switch_port_set_fdb() function prototype

[Ioana Ciornei]

On Wed, May 06, 2026 at 06:15:30PM +0300, Ioana Ciornei wrote:
> 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. This will enable
> us to pass the upper device that we are joining/leaving in all possible
> cases. This will get used in the next patches to determine what kind of
> device the upper is: a bridge or a bond.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@xxxxxxx>
> ---
>  .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 33 +++++++++----------
>  1 file changed, 15 insertions(+), 18 deletions(-)

(...)

> @@ -83,13 +82,13 @@ static u16 dpaa2_switch_port_set_fdb(struct ethsw_port_priv *port_priv,
>  
>  		if (!fdb) {
>  			port_priv->fdb->bridge_dev = NULL;
> -			return 0;
> +			return;
>  		}
>  
>  		port_priv->fdb = fdb;
>  		port_priv->fdb->in_use = true;
>  		port_priv->fdb->bridge_dev = NULL;
> -		return 0;
> +		return;


Shahiko notes:
	This problem wasn't introduced by this patch, but does this leak
	the previous FDB tracking structure?  When a port leaves a
	bridge or bond (when linking is false), it assigns the newly
	allocated fdb to port_priv->fdb and sets in_use to true.
	However, the in_use flag of the old FDB is never cleared.  If
	this port was the last user of the shared FDB, does that FDB
	remain permanently leaked? Over time, repeated topology changes
	could exhaust all FDBs. Once exhausted,
	dpaa2_switch_fdb_get_unused() would return NULL, which seems
	like it would cause the leaving port to keep its shared FDB,
	potentially leaking traffic between standalone ports and active
	bridges.

That is not true. In case port_priv is the last user of the shared FDB,
aka the last port that leaves the last bridge as noted in the code
comment, then dpaa2_switch_fdb_get_unused() will return NULL indicating
this condition. In this case, port_priv will continue to use the current
FDB which is the correct action since it's certain that this is the only
user of the FDB.


(...)

> @@ -2381,7 +2378,7 @@ static int dpaa2_switch_port_bond_join(struct net_device *netdev,
>  	u8 lag_id;
>  
>  	/* Setup the egress flood policy (broadcast, unknown unicast) */
> -	dpaa2_switch_port_set_fdb(port_priv, bond_dev);
> +	dpaa2_switch_port_set_fdb(port_priv, bond_dev, true);

Sashiko notes:
	This problem wasn't introduced by this patch, but if a port
	joins a bond that is already enslaved to a bridge, does this
	split the bond across different FDBs?

	If dpaa2_switch_port_bond_join() is called, it first executes
	dpaa2_switch_port_set_fdb(..., bond_dev, true), adopting the
	bond's shared FDB.

	Immediately after, it calls dpaa2_switch_port_bridge_join(),
	which executes dpaa2_switch_port_set_fdb(..., bridge_dev, true).

	If the bridge contains another standalone DPAA2 port, the logic
	will match that port and execute: if (other_port_priv) {
	port_priv->fdb->in_use = false; port_priv->fdb->bridge_dev =
	NULL; port_priv->fdb = other_port_priv->fdb; }

	Because port_priv->fdb is the bond's shared FDB, doesn't this
	incorrectly mark the bond's active FDB as unused while other
	bond slaves are still actively using it?


There is no such problem as the one described above.

If a port joins a bond that is already under a bridge, then the joining
port will use the FDB shared by all the ports under that same bridge.

Also, any other bond slaves already under the bridged bond will be
already using the same FDB.

Ioana