Re: [PATCH net-next v3 09/12] dpaa2-switch: offload FDBs added on an upper bond device

[Ioana Ciornei]

On Fri, Jun 05, 2026 at 07:10:54PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> dpaa2-switch: offload FDBs added on an upper bond device
> 
> This patch adds support for offloading FDB entries added on upper bond
> devices.
> 
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > index fb754dae80697..fadebf6a758d1 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> [ ... ]
> > @@ -616,6 +640,91 @@ static int dpaa2_switch_port_fdb_del(struct ethsw_port_priv *port_priv,
> >  		return dpaa2_switch_port_fdb_del_mc(port_priv, addr);
> >  }
> >  
> > +static struct dpaa2_mac_addr *
> > +dpaa2_switch_mac_addr_find(struct list_head *addr_list,
> > +			   const unsigned char *addr, u16 vid)
> > +{
> > +	struct dpaa2_mac_addr *a;
> > +
> > +	list_for_each_entry(a, addr_list, list)
> > +		if (ether_addr_equal(a->addr, addr) && a->vid == vid)
> > +			return a;
> > +
> > +	return NULL;
> > +}
> > +
> > +static int dpaa2_switch_lag_fdb_add(struct dpaa2_switch_lag *lag,
> > +				    const unsigned char *addr, u16 vid)
> > +{
> > +	struct ethsw_port_priv *port_priv;
> > +	struct dpaa2_mac_addr *a;
> > +	int err = 0;
> > +
> > +	mutex_lock(&lag->fdb_lock);
> > +
> > +	a = dpaa2_switch_mac_addr_find(&lag->fdbs, addr, vid);
> > +	if (a) {
> > +		refcount_inc(&a->refcount);
> 
> [Severity: High]
> If a port leaves the LAG, the fan-out for the FDB delete event will only
> reach the remaining ports. Could this result in the refcount decrementing
> fewer times than it incremented, preventing it from reaching zero and
> permanently leaking the struct dpaa2_mac_addr?

No, this is not a valid concern. When a port leaves a LAG which is also
under a bridge port, the driver will first call
switchdev_bridge_port_unoffload() which will eventually generate the
necessary deletions of FDB entries.

> > @@ -3903,8 +4073,10 @@ static void dpaa2_switch_remove(struct fsl_mc_device *sw_dev)
> >  	for (i = 0; i < DPAA2_SWITCH_RX_NUM_FQS; i++)
> >  		netif_napi_del(&ethsw->fq[i].napi);
> >  
> > -	for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> >  		dpaa2_switch_remove_port(ethsw, i);
> > +		mutex_destroy(&ethsw->lags[i].fdb_lock);
> > +	}
> 
> [Severity: High]
> Are the dynamically allocated struct dpaa2_mac_addr entries in the lag->fdbs
> list freed during driver teardown? It appears ethsw->lags is freed shortly
> after this without iterating through the fdbs lists, which might cause a
> memory leak of all active LAG FDB entries. Could this also happen if all
> ports leave a LAG and it becomes unused, inheriting stale entries upon reuse?

Not a valid concern. The deletion happens when the ports leave the LAG,
there is no need to also walk through the entries.