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().