Re: [PATCH net-next 10/15] net/mlx5: LAG, disable both regular and SD LAG on lag_disable_change

[Shay Drori <shayd@nvidia.com>]

On 04/06/2026 14:44, Tariq Toukan wrote:
> From: Shay Drory <shayd@nvidia.com>
> 
> Extend mlx5_lag_disable_change() to properly disable both regular LAG
> and SD LAG when requested. Each LAG type uses its own devcom component
> for locking.
> 
> Use mlx5_sd_get_devcom() helper to retrieve the SD devcom component,
> needed for proper locking when disabling SD LAG.
> 
> Signed-off-by: Shay Drory <shayd@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
>   .../net/ethernet/mellanox/mlx5/core/lag/lag.c | 25 +++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> index e23c1e81b98f..b660253ffc6d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> @@ -2494,13 +2494,18 @@ EXPORT_SYMBOL(mlx5_lag_is_shared_fdb);
>   
>   void mlx5_lag_disable_change(struct mlx5_core_dev *dev)
>   {
> +	struct mlx5_devcom_comp_dev *sd_devcom = mlx5_sd_get_devcom(dev);
> +	struct mlx5_core_dev *primary;
>   	struct mlx5_lag *ldev;
> +	struct lag_func *pf;
> +	int i;
>   
>   	ldev = mlx5_lag_dev(dev);
>   	if (!ldev)
>   		return;
>   
> -	mlx5_devcom_comp_lock(dev->priv.hca_devcom_comp);
> +	primary = mlx5_sd_get_primary(dev) ?: dev;
> +	mlx5_devcom_comp_lock(primary->priv.hca_devcom_comp);

sashiko.dev says:
Could this lockless retrieval of the primary device pointer lead to a
use-after-free if the primary device is concurrently unbound?
For secondary devices, mlx5_sd_get_primary() reads sd->primary_dev without
acquiring a reference count or holding the sd->devcom lock. In a hot-unplug
scenario where the primary PF is being unbound:
CPU1 (Unbind Primary)
mlx5_sd_cleanup()
     mlx5_devcom_comp_lock(sd->devcom);
     peer_sd->primary_dev = NULL; // cleared for all secondaries
     mlx5_devcom_comp_unlock(sd->devcom);
     // proceeds to free the primary mlx5_core_dev
CPU2 (eswitch mode change on secondary PF)
mlx5_lag_disable_change()
     primary = mlx5_sd_get_primary(dev); // reads pointer before CPU1 
clears it
If CPU2 reads the pointer before CPU1 clears it, wouldn't CPU2 then
dereference a freed primary device when calling
mlx5_devcom_comp_lock(primary->priv.hca_devcom_comp) shortly after?
Should the SD devcom lock be acquired before fetching the primary device
to guarantee its valid lifecycle?

Added the lock in V2

>   	mutex_lock(&ldev->lock);
>   
>   	ldev->mode_changes_in_progress++;
> @@ -2512,7 +2517,23 @@ void mlx5_lag_disable_change(struct mlx5_core_dev *dev)
>   	}
>   
>   	mutex_unlock(&ldev->lock);
> -	mlx5_devcom_comp_unlock(dev->priv.hca_devcom_comp);
> +	mlx5_devcom_comp_unlock(primary->priv.hca_devcom_comp);
> +
> +	if (!sd_devcom)
> +		return;
> +
> +	/* Teardown SD shared FDB for this device's group if active */
> +	mlx5_devcom_comp_lock(sd_devcom);
> +	mutex_lock(&ldev->lock);
> +	mlx5_lag_for_each(i, 0, ldev, MLX5_LAG_FILTER_ALL) {
> +		pf = mlx5_lag_pf(ldev, i);
> +		if (pf->dev == dev && pf->sd_fdb_active) {
> +			mlx5_lag_shared_fdb_destroy(ldev, pf->group_id);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&ldev->lock);
> +	mlx5_devcom_comp_unlock(sd_devcom);
>   }
>   
>   void mlx5_lag_enable_change(struct mlx5_core_dev *dev)