Re: [PATCH net-next 08/13] dpaa2-switch: consolidate unicast and multicast management

[Ioana Ciornei]

On Wed, May 06, 2026 at 06:15:35PM +0300, Ioana Ciornei wrote:
> This patch consolidates the unicast and multicast management by creating
> two new functions - dpaa2_switch_port_fdb_[add|del]() - which can be
> used for either uc or mc addresses. Having this common entrypoint for
> both types of addresses will help us in the next patches to streamline
> the same addresses but on LAG ports.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@xxxxxxx>
> ---
>  .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 39 +++++++++++++------
>  1 file changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> index a057fd85bd27..bced37335a9e 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c

(...)

> @@ -2725,12 +2747,8 @@ static void dpaa2_switch_event_work(struct work_struct *work)
>  
>  	switch (switchdev_work->event) {
>  	case SWITCHDEV_FDB_ADD_TO_DEVICE:
> -		if (is_unicast_ether_addr(fdb_info->addr))
> -			err = dpaa2_switch_port_fdb_add_uc(netdev_priv(dev),
> -							   fdb_info->addr);
> -		else
> -			err = dpaa2_switch_port_fdb_add_mc(netdev_priv(dev),
> -							   fdb_info->addr);
> +		err = dpaa2_switch_port_fdb_add(netdev_priv(dev),
> +						fdb_info->addr);

Sashiko notes:

	Does this code leave an orphaned unicast FDB entry in hardware
	if dev_mc_add() fails just below this diff context?

	When dpaa2_switch_port_fdb_add() is called, it dynamically
	routes the addition to either the unicast or multicast hardware
	FDB.

	However, the rollback path immediately below this hunk still
	hardcodes the multicast deletion:

	err = dev_mc_add(netdev, mdb->addr);
	if (err) {
		netdev_err(netdev, "dev_mc_add err %d\n", err);
		dpaa2_switch_port_fdb_del_mc(port_priv, mdb->addr);
	}

	If userspace passes a unicast MAC address for an L2 MDB entry
	and dev_mc_add() fails, will the rollback attempt to remove the
	MAC from the multicast FDB instead of the unicast FDB where it
	was just added?

	A later commit introduces LAG FDB abstractions, and this missed
	error path continues to hardcode the deletion, which might also
	bypass the LAG refcounting teardown.

	Should the rollback use the newly introduced
	dpaa2_switch_port_fdb_del() to ensure symmetrical cleanup?

I cannot see how the "If userspace passes a unicast MAC address for an
L2 MDB entry" statement could be true.

But yes, the rollback can use the newly introduced helper just for
symmetry.