Re: [PATCH net 2/3] net/mlx5e: Sync CQE moderation params with pflags in set_pflag_cqe_based_moder

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] net/mlx5e: stale hardware CQ moderation when toggling via priv-flag
- [Medium] net/mlx5e: unhandled toggle_dim failure leaves DIM disabled and channels partially configured
--

commit 7d36de6d06985b57ee8f31a9e94e4235c92121ae
Author: Yael Chemla <ychemla@nvidia.com>
Subject: net/mlx5e: Sync CQE moderation params with pflags in set_pflag_cqe_based_moder

This commit synchronizes the CQE-based moderation parameters with priv-flags 
when they are toggled via set_pflag_cqe_based_moder(). It updates the channel 
parameters so that tools like ethtool -c report the correct state, and resets 
the moderation usecs to the defaults for the new mode.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index 3ed59ced0407a..c2bee69c26af7 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -2193,12 +2193,20 @@ static int set_pflag_cqe_based_moder(struct net_device *netdev, bool enable,
>  		mlx5e_channels_rx_toggle_dim(&priv->channels);

[Severity: Medium]
The return value of mlx5e_channels_rx_toggle_dim() is unconditionally ignored
here.

If re-enabling DIM fails (for instance, if mlx5e_dim_rx_change() fails due
to memory allocation errors in mlx5e_dim_enable()), does this leave the
current channel's DIM completely disabled while skipping the remaining
channels? This could result in a broken state where the priv-flags appear
successfully updated but DIM is silently disabled on some channels.

>  		MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_RX_CQE_BASED_MODER,
>  				cq_period_mode);
> +		new_params.rx_moder_use_cqe_mode = enable;
> +		mlx5e_reset_rx_moderation(&new_params.rx_cq_moderation,
> +					  cq_period_mode,
> +					  new_params.rx_dim_enabled);

[Severity: Medium]
Does updating the software state cache here fail to push the updates to the
hardware when DIM is disabled?

When DIM is disabled (e.g., adaptive RX off), mlx5e_channels_rx_toggle_dim()
skips the channels:

mlx5e_channels_rx_toggle_dim() {
    ...
    if (chs->c[i]->rq.dim) {
        ...
    }
}

Since the channels are skipped, no asynchronous hardware update is scheduled
by the DIM work thread. Later, mlx5e_safe_switch_params() is called with
reset=false:

mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, false);

Passing reset=false skips reopening the channels. Because no hardware
modify command is issued, could this leave the actual hardware operating
with stale CQE moderation parameters while the software ethtool -c
configuration reports the new state?

>  	} else {
>  		mlx5e_reset_tx_channels_moderation(&priv->channels, cq_period_mode,
>  						   false, true);
>  		mlx5e_channels_tx_toggle_dim(&priv->channels);

[Severity: Medium]
The return value of mlx5e_channels_tx_toggle_dim() is unconditionally
ignored here as well.

Similar to the RX side, if this fails, could it leave DIM disabled on some
channels and skip applying the new moderation mode to subsequent channels
without reporting an error?

>  		MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_TX_CQE_BASED_MODER,
>  				cq_period_mode);
> +		new_params.tx_moder_use_cqe_mode = enable;
> +		mlx5e_reset_tx_moderation(&new_params.tx_cq_moderation,
> +					  cq_period_mode,
> +					  new_params.tx_dim_enabled);
>  	}
>  
>  	/* Update pflags of existing channels without resetting them */

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