Re: [PATCH net-next 1/2] net/mlx5e: Select DCBX HOST/AUTO from capability flags in setdcbx

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: mlx5e: stale DCBX capabilities reported due to skipped dcbx->cap update
--

commit d9bd56bb438776afa3f0fbae89fef22fddf584b9
Author: Alexei Lazar <alazar@nvidia.com>

net/mlx5e: Select DCBX HOST/AUTO from capability flags in setdcbx

This commit switches the firmware HOST/AUTO modes based on the
DCB_CAP_DCBX_HOST bit rather than using mode 0. It also attempts to
simplify the code by folding two different execution paths into one.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> index 741f75b5bfec5..4eef929f5ae9b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> @@ -453,32 +434,31 @@ static u8 mlx5e_dcbnl_getdcbx(struct net_device *dev)
>  
>  static u8 mlx5e_dcbnl_setdcbx(struct net_device *dev, u8 mode)
>  {
> +	bool new_is_host = mode & DCB_CAP_DCBX_HOST;
>  	struct mlx5e_priv *priv = netdev_priv(dev);
>  	struct mlx5e_dcbx *dcbx = &priv->dcbx;
> +	enum mlx5_dcbx_oper_mode target_mode;
>  
>  	if (mode & DCB_CAP_DCBX_LLD_MANAGED)
>  		return 1;
>  
> -	if ((!mode) && MLX5_CAP_GEN(priv->mdev, dcbx)) {
> [ ... ]
> +	if (!MLX5_CAP_GEN(priv->mdev, dcbx))
> +		return !new_is_host;

[Severity: Medium]
When the hardware lacks DCBX capability and new_is_host is true, this
early return evaluates to 0 (success). Does this inadvertently skip the
dcbx->cap = mode assignment at the end of the function?

If a privileged user updates the capabilities, it appears they will be
silently dropped, causing mlx5e_dcbnl_getdcbx() to report stale values.

>  
> +	target_mode = new_is_host ? MLX5E_DCBX_PARAM_VER_OPER_HOST :
> +				    MLX5E_DCBX_PARAM_VER_OPER_AUTO;
> +	if (dcbx->mode == target_mode)
> +		return 0;

[Severity: Medium]
If the hardware is already in the target operational mode, this also returns
early and skips the dcbx->cap = mode assignment. Could this cause valid
capability bit changes (such as switching between CEE and IEEE) to be lost?

In the previous implementation, these paths fell through and correctly
updated the software capability state.

>  
> -	if (!(mode & DCB_CAP_DCBX_HOST))
> +	if (mlx5e_dcbnl_set_dcbx_mode(priv, target_mode))
>  		return 1;
>  
> -	if (mlx5e_dcbnl_switch_to_host_mode(netdev_priv(dev)))
> -		return 1;
> +	if (!new_is_host)
> +		mode = DCB_CAP_DCBX_VER_CEE |
> +		       DCB_CAP_DCBX_VER_IEEE |
> +		       DCB_CAP_DCBX_LLD_MANAGED;
>  
> +	dcbx->mode = target_mode;
>  	dcbx->cap = mode;
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011112900.802150-1-tariqt@nvidia.com?part=1