Re: [PATCH net-next 05/13] dpaa2-switch: add dpaa2_switch_port_to_bridge_port() helpe

[Ioana Ciornei]

On Wed, May 06, 2026 at 06:15:32PM +0300, Ioana Ciornei wrote:
> With the addition of offloading support for upper bond devices we have
> to let the switchdev framework know if a specific bridge port is
> offloaded or not, even if that port is bond device.
> 
> For this to happen, create the dpaa2_switch_port_to_bridge_port function
> which will determine the bridge port corresponding to a particulat DPAA2
> switch interface and use it in the switchdev_bridge_port_offload call.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@xxxxxxx>
> ---
>  .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 26 ++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> index 9d3a0aef1568..aaa22dc15038 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> @@ -2076,6 +2076,18 @@ static int dpaa2_switch_port_attr_set_event(struct net_device *netdev,
>  	return notifier_from_errno(err);
>  }
>  
> +static struct net_device *
> +dpaa2_switch_port_to_bridge_port(struct ethsw_port_priv *port_priv)
> +{
> +	if (!port_priv->fdb->bridge_dev)
> +		return NULL;
> +
> +	if (port_priv->lag)
> +		return port_priv->lag->bond_dev;
> +
> +	return port_priv->netdev;
> +}
> +
>  static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
>  					 struct net_device *upper_dev,
>  					 struct netlink_ext_ack *extack)
> @@ -2083,6 +2095,7 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
>  	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
>  	struct dpaa2_switch_fdb *old_fdb = port_priv->fdb;
>  	struct ethsw_core *ethsw = port_priv->ethsw_data;
> +	struct net_device *brport_dev;
>  	bool learn_ena;
>  	int err;
>  
> @@ -2094,7 +2107,8 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
>  	dpaa2_switch_port_set_fdb(port_priv, upper_dev, true);
>  
>  	/* Inherit the initial bridge port learning state */
> -	learn_ena = br_port_flag_is_set(netdev, BR_LEARNING);
> +	brport_dev = dpaa2_switch_port_to_bridge_port(port_priv);
> +	learn_ena = br_port_flag_is_set(brport_dev, BR_LEARNING);
>  	err = dpaa2_switch_port_set_learning(port_priv, learn_ena);
>  	port_priv->learn_ena = learn_ena;

Sashiko notes:

	Does this code ignore the return value of
	dpaa2_switch_port_set_learning()?  If the firmware command
	fails, the driver will silently ignore it.

	This could leave the port's hardware state desynchronized from
	the kernel's tracking state (port_priv->learn_ena), which could
	lead to forwarding anomalies.

True, but the code was there before this patch set. I am leaning towards
not fixing it in this set since we are already at 13 patches.

>  
> @@ -2108,7 +2122,8 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
>  	if (err)
>  		goto err_egress_flood;
>  
> -	err = switchdev_bridge_port_offload(netdev, netdev, NULL,
> +	brport_dev = dpaa2_switch_port_to_bridge_port(port_priv);
> +	err = switchdev_bridge_port_offload(brport_dev, netdev, NULL,
>  					    NULL, NULL, false, extack);
>  	if (err)
>  		goto err_switchdev_offload;

Sashiko notes:

	Are we missing a rollback of the port state in the
	err_switchdev_offload error path?

The same response as above.

> @@ -2143,7 +2158,12 @@ static int dpaa2_switch_port_restore_rxvlan(struct net_device *vdev, int vid, vo
>  
>  static void dpaa2_switch_port_pre_bridge_leave(struct net_device *netdev)
>  {
> -	switchdev_bridge_port_unoffload(netdev, NULL, NULL, NULL);
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	struct net_device *brport_dev;
> +
> +	brport_dev = dpaa2_switch_port_to_bridge_port(port_priv);
> +
> +	switchdev_bridge_port_unoffload(brport_dev, NULL, NULL, NULL);

Sashiko notes:

	Is there a missing switchdev unoffload when a port leaves a bond
	device that is part of a bridge?

	When a port joins a bond that is part of a bridge,
	dpaa2_switch_port_bond_join() calls
	dpaa2_switch_port_bridge_join(), incrementing the bridge's
	offload_count for the bond device.

	When the port is later removed from the bond,
	NETDEV_PRECHANGEUPPER is triggered with !info->linking where
	upper_dev is the LAG. However,
	dpaa2_switch_port_prechangeupper() only handles the
	info->linking case, explicitly ignoring the !info->linking case.

	Thus, dpaa2_switch_port_pre_bridge_leave() is skipped.
	Subsequently, dpaa2_switch_port_bond_leave() resets the port to
	standalone mode in hardware but never calls
	switchdev_bridge_port_unoffload() or
	dpaa2_switch_port_bridge_leave().

	Can this completely bypass the software teardown path,
	permanently leaking the offload_count reference for the bridge
	port and corrupting the switchdev offload state?

Yes, I forgot to also call dpaa2_switch_port_bridge_leave() when leaving
a bond which is a bridge port. Will fix it in the commit managing the
changeupper events, not in this patch.