Re: [PATCH net-next 04/13] dpaa2-switch: extend dpaa2_switch_port_set_fdb() to cover bond scenarios

[Ioana Ciornei]

On Wed, May 06, 2026 at 06:15:31PM +0300, Ioana Ciornei wrote:
> The dpaa2_switch_port_set_fdb() function is responsible with determining
> what FDB should be used by a port as a consequence of changing its upper
> device. This patch extends the function to also cover the circumstances
> in which a DPAA2 switch port offloads a bond device.
> 
> This will allow us, for example, to setup the same FDB on all DPAA2
> switch ports which are under the same bridge, even though not directly
> but rather through an upper bond device which is bridged. How the
> function does this is by first determining a DPAA2 port is already under
> the same bridge and if so, choosing its FDB. To cover the entire
> hierarchy in depth, we add an extra walk through all the lowers of a
> bridged bond device.
> 
> When leaving an upper device, the DPAA2 switch port must find a new FDB
> to use. If before it just searched for an unused FDB to go along with
> its new standalone status, now it first checks if the port is still part
> of a LAG and then uses the FDB of any port that already left the same
> bridge.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@xxxxxxx>
> ---
>  .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 102 +++++++++++++-----
>  1 file changed, 78 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> index f1b4c24b8a47..9d3a0aef1568 100644

(...)

> -		if (!fdb) {
> -			port_priv->fdb->bridge_dev = NULL;
> -			return;
> +		/* Try to get hold of an unused FDB to use */
> +		if (!fdb)
> +			fdb = dpaa2_switch_fdb_get_unused(port_priv->ethsw_data);

Sashiko notes:

	Does this sequence introduce a resource leak of FDB entries?
	When a bridged bond leaves the bridge, the first port acquires
	an unused FDB.  Subsequent ports in the bond bypass the unused
	pool and directly copy the first port's FDB, leaving surplus
	FDBs in the pool.

That is not the correct understanding at all. The FDBs are there as a
means to correctly setup the forwarding domains in a switch, there is no
such thing as "surplus FDBs in the pool"

> +
> +		if (fdb) {
> +			port_priv->fdb = fdb;
> +			port_priv->fdb->in_use = true;
>  		}
>  
> -		port_priv->fdb = fdb;
> -		port_priv->fdb->in_use = true;
> -		port_priv->fdb->bridge_dev = NULL;
> +		if (netif_is_bridge_master(upper_dev))
> +			port_priv->fdb->bridge_dev = NULL;
> +
> +		/* In case all FDBs are already in use, we must be the last
> +		 * port that becomes standalone. We can just keep the FDB that
> +		 * we already have. Nothing more to do in this case.
> +		 */
>  		return;

Sashiko notes:

	When the final port in the bridge (e.g., a standalone port)
	subsequently leaves, it will find these surplus FDBs and
	successfully acquire one. The original bridge FDB is then
	completely abandoned, but since its in_use flag is never
	explicitly set to false in this path, the FDB is leaked.

	If a bond and a standalone port repeatedly join and leave a
	bridge, could this exhaust the hardware FDB pool? Once
	exhausted, it seems subsequent standalone ports would fail to
	acquire exclusive FDBs and might silently share existing FDBs
	with other networks.

Not correct at all. The FDBs cannot be "leaked". There is an array
holding them per ethsw_data.

Also, it's ok not to have all FDBs used at a moment in time. In this
exemple above, after an offloaded bond device and a dpaa2 switch port
leave a bridge, it's expected for all the switch ports under the same
bond to use the same FDB, thus leaving unused FDBs.