Thread (25 messages) 25 messages, 2 authors, 2026-06-09

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(&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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help