This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
i40e: move ATR sample rate from ring to PF level
This commit moves the ATR sample rate configuration from the per-ring level
to the PF level. It initializes the sample rate globally during software init
and updates the fast path to reference the PF structure.
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index a04683004a567..4d4d4b07a0f3a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
[ ... ]
> @@ -3454,12 +3454,7 @@ static int i40e_configure_tx_ring(struct i40e_ring
> *ring)
> ring->xsk_pool = i40e_xsk_pool(ring);
>
> /* some ATR related tx ring init */
> - if (test_bit(I40E_FLAG_FD_ATR_ENA, vsi->back->flags)) {
> - ring->atr_sample_rate = I40E_DEFAULT_ATR_SAMPLE_RATE;
> - ring->atr_count = 0;
> - } else {
> - ring->atr_sample_rate = 0;
> - }
> + ring->atr_count = 0;
[Severity: High]
Does removing this per-ring initialization remove the protection against
dynamic ethtool flag toggles?
Previously, if the ring was configured while Flow Director ATR was disabled,
the ring's sample rate was cached to 0 in i40e_configure_tx_ring(). This
prevented descriptor injection even if the PF flag was later toggled on
without a ring reset.
[ ... ]
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index 894f2d06d39d0..b36e0fc9b31d2 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
[ ... ]
> @@ -2879,7 +2879,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct
> sk_buff *skb,
> return;
>
> /* if sampling is disabled do nothing */
> - if (!tx_ring->atr_sample_rate)
> + if (!pf->atr_sample_rate)
> return;
[Severity: High]
Can this global check lead to a hardware Malicious Driver Detection (MDD)
event if ethtool flags are modified dynamically?
If an administrator performs the following sequence:
1. Disables flow-director-atr via ethtool.
2. Disables ntuple (which resets the ring and sets tx_ctx.fd_ena = 0).
3. Re-enables flow-director-atr.
The final step does not trigger a ring reset, so fd_ena remains 0 in the
hardware queue context.
However, I40E_FLAG_FD_ATR_ENA is now true, and pf->atr_sample_rate is
globally set to a non-zero value.
Will i40e_atr() now proceed and inject FDIR descriptors into a TX queue
that is not configured for FDIR?
If so, does this cause the hardware to trigger an MDD event and hang the
TX queue?
