Re: [PATCH net-next] net/mlx5e: Rename state_destroy to rx_mode_active in flow steering

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] net/mlx5e: spurious WARN_ON_ONCE in mlx5e_del_vlan_rules() on error paths
--

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

net/mlx5e: Rename state_destroy to rx_mode_active in flow steering

This commit renames the state_destroy flag to rx_mode_active to better
clarify its purpose in the flow steering code. It controls the enablement
of RX mode handling and acts as a safeguard during device teardown.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
> index 12492c4a5d410..c19238bc821fb 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c

[ ... ]

> @@ -560,7 +560,7 @@ static void mlx5e_del_vlan_rules(struct mlx5e_flow_steering *fs)
>  	for_each_set_bit(i, fs->vlan->active_svlans, VLAN_N_VID)
>  		mlx5e_fs_del_vlan_rule(fs, MLX5E_VLAN_RULE_TYPE_MATCH_STAG_VID, i);
>  
> -	WARN_ON_ONCE(fs->state_destroy);
> +	WARN_ON_ONCE(fs->rx_mode_active);

[Severity: Low]
Does this trigger a spurious warning on error paths?

If mlx5e_attach_netdev() begins execution, it sets rx_mode_active to true.
If a subsequent function like profile->init_rx() fails (for example, if
mlx5e_ptp_alloc_rx_fs() fails), the error rollback path calls
mlx5e_destroy_vlan_table() which calls mlx5e_del_vlan_rules(). Since
rx_mode_active is still true, the WARN_ON_ONCE() is hit during the rollback.

[ ... ]

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index f90b5a34e0972..478b5956044bd 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -6557,8 +6557,9 @@ int mlx5e_attach_netdev(struct mlx5e_priv *priv)
>  
>  	clear_bit(MLX5E_STATE_DESTROYING, &priv->state);
>  	if (priv->fs)
> -		mlx5e_fs_set_state_destroy(priv->fs,
> -					   !test_bit(MLX5E_STATE_DESTROYING, &priv->state));
> +		mlx5e_fs_set_rx_mode_active(priv->fs,
> +					    !test_bit(MLX5E_STATE_DESTROYING,
> +						      &priv->state));

[Severity: Low]
Because rx_mode_active is set to true early in device attachment here before
calling init_rx(), could it remain true when handling a failure and lead to
the spurious warning mentioned above?

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