Re: [PATCH iwl-next v5 1/2] i40e: move ATR sample rate from ring to PF level

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] i40e: reset_needed bitmask overwritten instead of ORed in i40e_set_priv_flags
--

commit a5576ab3ed2619cb9e4c77239e0fd5bb5851d0df
Author: Mohammad Heib <mheib@redhat.com>

i40e: move ATR sample rate from ring to PF level

This commit moves the ATR sample rate from the per-ring struct to the
PF level to apply it uniformly across all TX rings. It also triggers a
PF reset when re-enabling ATR to ensure fd_ena is properly re-evaluated
in the TX queue hardware context.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -5300,6 +5300,16 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
>  	    test_bit(I40E_FLAG_SOURCE_PRUNING_DIS, changed_flags))
>  		reset_needed = BIT(__I40E_PF_RESET_REQUESTED);
>  
> +	/* Re-enabling ATR requires a reset to reprogram fd_ena in the
> +	 * TX queue HW context. Skip when FD_SB (ntuple) is already
> +	 * enabled, since i40e_configure_tx_ring() sets fd_ena when
> +	 * either FD flag is active.
> +	 */
> +	if (test_bit(I40E_FLAG_FD_ATR_ENA, changed_flags) &&
> +	    test_bit(I40E_FLAG_FD_ATR_ENA, new_flags) &&
> +	    !test_bit(I40E_FLAG_FD_SB_ENA, new_flags))
> +		reset_needed = BIT(__I40E_PF_RESET_REQUESTED);

[Severity: Medium]
Does this direct assignment introduce a regression by inadvertently
overwriting previously accumulated reset requests?

Earlier in i40e_set_priv_flags(), we have:

        if (test_bit(I40E_FLAG_FW_LLDP_DIS, changed_flags))
                reset_needed = I40E_PF_RESET_AND_REBUILD_FLAG;

If multiple flags are modified in a single ethtool command (for example,
changing I40E_FLAG_FW_LLDP_DIS alongside enabling flow-director-atr),
wouldn't using '=' here instead of '|=' silently drop the earlier
I40E_PF_RESET_AND_REBUILD_FLAG requirement?

> +
>  	/* Before we finalize any flag changes, we need to perform some
>  	 * checks to ensure that the changes are supported and safe.
>  	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008131824.248508-1-mheib@redhat.com?part=1