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

Re: [PATCH net-next v3 10/12] dpaa2-switch: offload port objects on an upper bond device

From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: 2026-06-08 11:45:05
Also in: lkml

On Fri, Jun 05, 2026 at 07:10:56PM -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 port objects on an upper bond device

This patch adds support for offloading port objects, VLANs and MDBs,
added on upper bond devices. It introduces switchdev_handle_*() replication
helpers for port objects and a refcount mechanism for handling MDBs in bond
devices.
quoted
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index fadebf6a758d1..9aba5f348abc0 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -2215,14 +2231,17 @@ static int dpaa2_switch_port_mdb_del(struct net_device *netdev,
 				     const struct switchdev_obj_port_mdb *mdb)
 {
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
+	struct dpaa2_switch_lag *lag;
 	int err;
 
 	if (!dpaa2_switch_port_lookup_address(netdev, 0, mdb->addr))
 		return -ENOENT;
[Severity: High]
Does returning an error here abort the switchdev_handle_port_obj_del()
replication loop for the remaining bond slaves?

This patch uses switchdev_handle_port_obj_del() to replicate object
deletions across all slave devices. If a driver callback returns an error
other than -EOPNOTSUPP on one of the slaves, the replication loop aborts,
skipping all remaining slaves.

For example, dpaa2_switch_port_mdb_del() returns -ENOENT if the MAC is not
found, and dpaa2_switch_port_vlans_del() propagates hardware deletion errors.

Will this leave the remaining slave ports with stale VLANs and MDBs fully
active in software and hardware, causing silent data leaks and inconsistent
state?
I don't see how I could fix this at driver level without a change in
switchdev_handle_port_obj_del() or the callback prototype for del_cb.
[ ... ]
quoted
@@ -3128,37 +3154,23 @@ static int dpaa2_switch_port_event(struct notifier_block *nb,
 	}
 }
 
-static int dpaa2_switch_port_obj_event(unsigned long event,
[ ... ]
quoted
 static int dpaa2_switch_port_blocking_event(struct notifier_block *nb,
 					    unsigned long event, void *ptr)
 {
 	struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+	int err;
 
 	switch (event) {
 	case SWITCHDEV_PORT_OBJ_ADD:
+		err = switchdev_handle_port_obj_add(dev, ptr,
+						    dpaa2_switch_port_dev_check,
+						    dpaa2_switch_port_obj_add);
+		return notifier_from_errno(err);
[Severity: High]
Can this code leak memory and hardware MDBs when a slave leaves a bond?

When switchdev_handle_port_obj_add() replicates MDB additions across all
bond slaves, it increments the a->refcount in lag->fdbs for each slave. 

If a slave leaves the bond (e.g., via ip link set eth1 nomaster), shouldn't
dpaa2_switch_port_bond_leave() be updated to decrement the slave's share
of the MDB refcounts?

Without this, when the bond is later removed from the bridge,
SWITCHDEV_PORT_OBJ_DEL replicates only over the remaining slaves. The MDB
refcounts will not reach zero because the decrements from the departed
slaves are permanently lost. This would leak struct dpaa2_mac_addr memory
and leave the hardware MDB permanently programmed on the LAG.
No, the concern is not valid. Please look at the
nbp_switchdev_unsync_objs() function called from
switchdev_bridge_port_unoffload() -> call_switchdev_blocking_notifiers()
-> br_switchdev_blocking_event() -> br_switchdev_port_unoffload().
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help