Re: [PATCH net-next v3 09/12] dpaa2-switch: offload FDBs added on an upper bond device
From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: 2026-06-08 10:35:13
Also in:
lkml
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.quoted
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[ ... ]quoted
@@ -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.
quoted
@@ -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(ðsw->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(ðsw->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.