[PATCH iwl-next v6 0/2] i40e: add runtime-tunable ATR sample rate

[PATCH iwl-next v6 0/2] i40e: add runtime-tunable ATR sample rate

[mheib]

From: Mohammad Heib <mheib@redhat.com>

This series makes the Flow Director ATR sample rate configurable at
runtime through a devlink parameter.

On systems with many queues and high-rate TCP workloads, the fixed
default sampling interval can cause frequent Flow Director
reprogramming and TCP packet reordering. The optimal interval depends
on the workload and system configuration.

Patch 1 moves atr_sample_rate from per-ring to PF level, since it is
a global policy. It also triggers a PF reset when re-enabling ATR via
ethtool private flags so that fd_ena is reprogrammed in the TX queue
HW context.

Patch 2 exposes atr_sample_rate as a devlink runtime parameter so
administrators can tune it without rebuilding the driver.

Testing with iperf3 -t 60 showed ~7k TCP reordering events at the
default sample rate (20). Setting atr_sample_rate to 10000 reduced
reordering to ~769 (~90% reduction).

Changes since v5:
- Use '|=' instead of '=' when setting reset_needed for the ATR
  re-enable path, to avoid overwriting unrelated reset requests
  (Sashiko)

Changes since v4:
- Folded the PF reset patch into patch 1 for bisect cleanliness
  (was patch 3/3 in v4)
- Skip PF reset when FD_SB (ntuple) already holds fd_ena high,
  avoiding unnecessary traffic disruption
- Fix LLDP EEXIST path to not drop unrelated reset requests
  (reset_needed &= ~REBUILD_FLAG instead of = 0)
- Flush existing ATR entries when setting atr_sample_rate to 0 via
  devlink, so stale filters do not persist on devices without HW
  ATR eviction
- Documentation: clarify that the sampling interval is per Tx queue
  and counts TCP packets only; fix formatting

Mohammad Heib (2):
  i40e: move ATR sample rate from ring to PF level
  i40e: add devlink parameter for Flow Director ATR sample rate

 Documentation/networking/devlink/i40e.rst     | 21 +++++++++
 drivers/net/ethernet/intel/i40e/i40e.h        |  1 +
 .../net/ethernet/intel/i40e/i40e_devlink.c    | 46 +++++++++++++++++++
 .../net/ethernet/intel/i40e/i40e_ethtool.c    | 12 ++++-
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  9 ++--
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  6 ++-
 drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  3 +-
 7 files changed, 87 insertions(+), 11 deletions(-)

-- 
2.56.0

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

[mheib]

From: Mohammad Heib <mheib@redhat.com>

The ATR sample rate is currently stored per-ring and initialized when each
TX ring is configured. Since the sample rate is a global policy that
applies uniformly across all rings, it makes more sense to store it at
the PF level.

Move atr_sample_rate from struct i40e_ring to struct i40e_pf and initialize
it once during i40e_sw_init(). Update i40e_atr() to reference the PF-level
field. Change atr_count from u8 to u32 to match the sample rate type.

This also changes a user-visible behavior: previously, re-enabling ATR via
"ethtool --set-priv-flags ethX flow-director-atr on" did not resume
sampling until an ifdown/ifup or PF reset, because the per-ring sample
rate was latched at ring configuration time. With the PF-level rate always
initialized, the flag takes effect immediately.

Trigger a PF reset when re-enabling ATR so that fd_ena is re-evaluated
in the TX queue HW context.

Signed-off-by: Mohammad Heib <mheib@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h         |  1 +
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 +++++++++++-
 drivers/net/ethernet/intel/i40e/i40e_main.c    |  9 +++------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c    |  6 ++++--
 drivers/net/ethernet/intel/i40e/i40e_txrx.h    |  3 +--
 5 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index a9d79bbd3ea7..528ef329a647 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -486,6 +486,7 @@ struct i40e_pf {
 	u16 rss_size_max;          /* HW defined max RSS queues */
 	u16 fdir_pf_filter_count;  /* num of guaranteed filters for this PF */
 	u16 num_alloc_vsi;         /* num VSIs this driver supports */
+	u32 atr_sample_rate;
 	bool wol_en;
 
 	struct hlist_head fdir_filter_list;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 0ae50d7f3465..bc9b4a2bc23b 100644
--- 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);
+
 	/* Before we finalize any flag changes, we need to perform some
 	 * checks to ensure that the changes are supported and safe.
 	 */
@@ -5431,7 +5441,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 				case LIBIE_AQ_RC_EEXIST:
 					dev_warn(&pf->pdev->dev,
 						 "FW LLDP agent is already running\n");
-					reset_needed = 0;
+					reset_needed &= ~I40E_PF_RESET_AND_REBUILD_FLAG;
 					break;
 				case LIBIE_AQ_RC_EPERM:
 					dev_warn(&pf->pdev->dev,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 774bfc595aa9..98d2fcdcee68 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3459,12 +3459,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;
 
 	/* configure XPS */
 	i40e_config_xps_tx_ring(ring);
@@ -12750,6 +12745,8 @@ static int i40e_sw_init(struct i40e_pf *pf)
 		}
 	}
 
+	pf->atr_sample_rate = I40E_DEFAULT_ATR_SAMPLE_RATE;
+
 	if ((pf->hw.func_caps.fd_filters_guaranteed > 0) ||
 	    (pf->hw.func_caps.fd_filters_best_effort > 0)) {
 		set_bit(I40E_FLAG_FD_ATR_ENA, pf->flags);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 2032465c9b7c..acb47a9fd86f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2887,6 +2887,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	struct tcphdr *th;
 	unsigned int hlen;
 	u32 flex_ptype, dtype_cmd;
+	u32 sample_rate;
 	int l4_proto;
 	u16 i;
 
@@ -2898,7 +2899,8 @@ 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)
+	sample_rate = READ_ONCE(pf->atr_sample_rate);
+	if (!sample_rate)
 		return;
 
 	/* Currently only IPv4/IPv6 with TCP is supported */
@@ -2950,7 +2952,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	if (!th->fin &&
 	    !th->syn &&
 	    !th->rst &&
-	    (tx_ring->atr_count < tx_ring->atr_sample_rate))
+	    (tx_ring->atr_count < sample_rate))
 		return;
 
 	tx_ring->atr_count = 0;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index bb741ff3e5f2..be587f804e7a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -372,8 +372,7 @@ struct i40e_ring {
 	u16 next_to_clean;
 	u16 xdp_tx_active;
 
-	u8 atr_sample_rate;
-	u8 atr_count;
+	u32 atr_count;
 
 	bool ring_active;		/* is ring online or not */
 	bool arm_wb;		/* do something to arm write back */
-- 
2.56.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] i40e: combined reset flags downgrade reset level to weaker PF_RESET
--

commit 6e65935fd380dc5da870cee78bd93449fc30b74c
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 struct, initializing it once during software initialization. It also
triggers a PF reset when re-enabling ATR so that the TX queue HW context
is appropriately reprogrammed.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 0ae50d7f34651..bc9b4a2bc23b2 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
[ ... ]
>  	if (test_bit(I40E_FLAG_FW_LLDP_DIS, changed_flags))
>  		reset_needed = I40E_PF_RESET_AND_REBUILD_FLAG;
>  
>  	if (test_bit(I40E_FLAG_VEB_STATS_ENA, changed_flags) ||
>  	    test_bit(I40E_FLAG_LEGACY_RX_ENA, changed_flags) ||
>  	    test_bit(I40E_FLAG_SOURCE_PRUNING_DIS, changed_flags))
>  		reset_needed = BIT(__I40E_PF_RESET_REQUESTED);

[Severity: Medium]
This problem wasn't introduced by this patch, but does this unconditionally
overwrite reset_needed? If I40E_FLAG_FW_LLDP_DIS is also in changed_flags,
it looks like the I40E_PF_RESET_AND_REBUILD_FLAG will be silently dropped.

>  
> +	/* 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]
If changed_flags contains multiple flags (for instance FW_LLDP_DIS and
FD_ATR_ENA simultaneously via ethtool), this bitwise OR operation can cause
reset_needed to contain both I40E_PF_RESET_AND_REBUILD_FLAG and
BIT(__I40E_PF_RESET_REQUESTED).

When both flags are set, does this cause the weaker reset to be executed?
Looking at i40e_do_reset() in i40e_main.c, there is a cascade that checks
the weaker flag before I40E_PF_RESET_AND_REBUILD_FLAG:

    } else if (reset_flags & I40E_PF_RESET_FLAG) {
        ...
        dev_dbg(&pf->pdev->dev, "PFR requested\n");
        i40e_handle_reset_warning(pf, lock_acquired);

    } else if (reset_flags & I40E_PF_RESET_AND_REBUILD_FLAG) {

This means the requested rebuild from I40E_PF_RESET_AND_REBUILD_FLAG would
be skipped.

Also, looking at i40e_handle_reset_warning(), it appears to synchronously call
i40e_reset_and_rebuild() with reinit set to false, instead of deferring it via
__I40E_RESET_INTR_RECEIVED:

static void i40e_handle_reset_warning(struct i40e_pf *pf, bool lock_acquired)
{
        i40e_prep_for_reset(pf);
        i40e_reset_and_rebuild(pf, false, lock_acquired);
}

Can this lead to the VSI reinitialization being incorrectly skipped when
both flags are present?

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

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

[Mohammad Heib]

On Mon, Oct 12, 2026 at 01:50:32PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] i40e: combined reset flags downgrade reset level to weaker PF_RESET
> --
> 
> commit 6e65935fd380dc5da870cee78bd93449fc30b74c
> 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 struct, initializing it once during software initialization. It also
> triggers a PF reset when re-enabling ATR so that the TX queue HW context
> is appropriately reprogrammed.
> 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > index 0ae50d7f34651..bc9b4a2bc23b2 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> [ ... ]
> >  	if (test_bit(I40E_FLAG_FW_LLDP_DIS, changed_flags))
> >  		reset_needed = I40E_PF_RESET_AND_REBUILD_FLAG;
> >  
> >  	if (test_bit(I40E_FLAG_VEB_STATS_ENA, changed_flags) ||
> >  	    test_bit(I40E_FLAG_LEGACY_RX_ENA, changed_flags) ||
> >  	    test_bit(I40E_FLAG_SOURCE_PRUNING_DIS, changed_flags))
> >  		reset_needed = BIT(__I40E_PF_RESET_REQUESTED);
> 
> [Severity: Medium]
> This problem wasn't introduced by this patch, but does this unconditionally
> overwrite reset_needed? If I40E_FLAG_FW_LLDP_DIS is also in changed_flags,
> it looks like the I40E_PF_RESET_AND_REBUILD_FLAG will be silently dropped.
> 
> >  

Right, as you noted this is pre-existing — the plain '=' assignments
above already clobber earlier reset requests regardless of this patch.

> > +	/* 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]
> If changed_flags contains multiple flags (for instance FW_LLDP_DIS and
> FD_ATR_ENA simultaneously via ethtool), this bitwise OR operation can cause
> reset_needed to contain both I40E_PF_RESET_AND_REBUILD_FLAG and
> BIT(__I40E_PF_RESET_REQUESTED).
> 
> When both flags are set, does this cause the weaker reset to be executed?
> Looking at i40e_do_reset() in i40e_main.c, there is a cascade that checks
> the weaker flag before I40E_PF_RESET_AND_REBUILD_FLAG:
> 
>     } else if (reset_flags & I40E_PF_RESET_FLAG) {
>         ...
>         dev_dbg(&pf->pdev->dev, "PFR requested\n");
>         i40e_handle_reset_warning(pf, lock_acquired);
> 
>     } else if (reset_flags & I40E_PF_RESET_AND_REBUILD_FLAG) {
> 
> This means the requested rebuild from I40E_PF_RESET_AND_REBUILD_FLAG would
> be skipped.
> 
> Also, looking at i40e_handle_reset_warning(), it appears to synchronously call
> i40e_reset_and_rebuild() with reinit set to false, instead of deferring it via
> __I40E_RESET_INTR_RECEIVED:
> 
> static void i40e_handle_reset_warning(struct i40e_pf *pf, bool lock_acquired)
> {
>         i40e_prep_for_reset(pf);
>         i40e_reset_and_rebuild(pf, false, lock_acquired);
> }
> 
> Can this lead to the VSI reinitialization being incorrectly skipped when
> both flags are present?
> 
Good observation, but in practice this cannot happen with the current
code: the '=' assignment for the LLDP path already overwrites
reset_needed before our '|=' runs, so the REBUILD flag is never
present when we OR in the PF reset bit.

That said, I agree the reset_needed handling and the priority ordering
in i40e_do_reset() could use cleanup. I'll look into addressing both
in a separate patch.
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261011134952.54887-1-mheib@redhat.com?part=1

[PATCH iwl-next v6 2/2] i40e: add devlink parameter for Flow Director ATR sample rate

[mheib]

From: Mohammad Heib <mheib@redhat.com>

The i40e driver uses Flow Director ATR to periodically update flow
steering information for active TCP flows. The update frequency is
currently controlled by I40E_DEFAULT_ATR_SAMPLE_RATE and is fixed at
driver build time.

On systems with a large number of queues and high-rate TCP workloads,
the default sampling interval can result in frequent Flow Director
reprogramming for long-lived flows.

The amount of TCP packet reordering observed on some systems is
sensitive to the ATR sampling interval. Increasing the interval reduces
Flow Director programming activity and can significantly reduce the
associated reordering.

Since the optimal sampling interval depends on the workload and system
configuration, a single fixed value is not suitable for all deployments.

Add a devlink parameter to allow administrators to tune the ATR sample
rate at runtime without rebuilding the driver or disabling ATR
functionality entirely.

Signed-off-by: Mohammad Heib <mheib@redhat.com>
---
 Documentation/networking/devlink/i40e.rst     | 21 +++++++++
 .../net/ethernet/intel/i40e/i40e_devlink.c    | 46 +++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/Documentation/networking/devlink/i40e.rst b/Documentation/networking/devlink/i40e.rst
index 51c887f0dc83..d280c17fe987 100644
--- a/Documentation/networking/devlink/i40e.rst
+++ b/Documentation/networking/devlink/i40e.rst
@@ -40,6 +40,27 @@ Parameters
 
         The default value is ``0`` (internal calculation is used).
 
+.. list-table:: Driver specific parameters implemented
+    :widths: 5 5 90
+
+    * - Name
+      - Mode
+      - Notes
+    * - ``atr_sample_rate``
+      - runtime
+      - Controls how frequently Flow Director ATR updates flow steering
+        information for active TCP flows.
+
+        ATR programs Flow Director entries based on sampled TCP packets
+        per Tx queue. The sampling interval is the number of TCP packets
+        transmitted on each queue between ATR updates.
+
+        Lower values increase Flow Director programming activity, while
+        higher values reduce the update frequency.
+
+        Setting to ``0`` disables ATR sampling (no filters will be programmed).
+
+        The default value is ``20``.
 
 Info versions
 =============
diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.c b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
index 229179ccc131..8af2487776f2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_devlink.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
@@ -33,12 +33,58 @@ static int i40e_max_mac_per_vf_get(struct devlink *devlink,
 	return 0;
 }
 
+static int i40e_atr_sample_rate_set(struct devlink *devlink,
+				    u32 id,
+				    struct devlink_param_gset_ctx *ctx,
+				    struct netlink_ext_ack *extack)
+{
+	struct i40e_pf *pf = devlink_priv(devlink);
+	u32 sample_rate = ctx->val.vu32;
+
+	WRITE_ONCE(pf->atr_sample_rate, sample_rate);
+
+	/* Setting rate to 0 disables sampling but does not remove
+	 * existing ATR entries. Flush them so stale filters do not
+	 * persist on devices without HW ATR eviction.
+	 */
+	if (!sample_rate) {
+		set_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state);
+		set_bit(__I40E_FD_FLUSH_REQUESTED, pf->state);
+	}
+
+	return 0;
+}
+
+static int i40e_atr_sample_rate_get(struct devlink *devlink,
+				    u32 id,
+				    struct devlink_param_gset_ctx *ctx,
+				    struct netlink_ext_ack *extack)
+{
+	struct i40e_pf *pf = devlink_priv(devlink);
+
+	ctx->val.vu32 = READ_ONCE(pf->atr_sample_rate);
+
+	return 0;
+}
+
+enum i40e_dl_param_id {
+	I40E_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+	I40E_DEVLINK_PARAM_ID_ATR_SAMPLE_RATE,
+};
+
 static const struct devlink_param i40e_dl_params[] = {
 	DEVLINK_PARAM_GENERIC(MAX_MAC_PER_VF,
 			      BIT(DEVLINK_PARAM_CMODE_RUNTIME),
 			      i40e_max_mac_per_vf_get,
 			      i40e_max_mac_per_vf_set,
 			      NULL),
+	DEVLINK_PARAM_DRIVER(I40E_DEVLINK_PARAM_ID_ATR_SAMPLE_RATE,
+			     "atr_sample_rate",
+			     DEVLINK_PARAM_TYPE_U32,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     i40e_atr_sample_rate_get,
+			     i40e_atr_sample_rate_set,
+			     NULL),
 };
 
 static void i40e_info_get_dsn(struct i40e_pf *pf, char *buf, size_t len)
-- 
2.56.0

Re: [PATCH iwl-next v6 2/2] i40e: add devlink parameter for Flow Director ATR sample rate

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] i40e: missing state check in devlink atr_sample_rate callback causes continuous flushes
--

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

i40e: add devlink parameter for Flow Director ATR sample rate

This commit introduces a new devlink parameter, atr_sample_rate, to allow
administrators to tune the update frequency of Flow Director ATR dynamically.
It provides set and get callbacks to apply changes without rebuilding the
driver or disabling ATR functionality entirely.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.c b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
> index 229179ccc1319..8af2487776f28 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_devlink.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
[ ... ]
> @@ -33,12 +33,58 @@ static int i40e_max_mac_per_vf_get(struct devlink *devlink,
>  	return 0;
>  }
>  
> +static int i40e_atr_sample_rate_set(struct devlink *devlink,
> +				    u32 id,
> +				    struct devlink_param_gset_ctx *ctx,
> +				    struct netlink_ext_ack *extack)
> +{
> +	struct i40e_pf *pf = devlink_priv(devlink);
> +	u32 sample_rate = ctx->val.vu32;
> +
> +	WRITE_ONCE(pf->atr_sample_rate, sample_rate);
> +
> +	/* Setting rate to 0 disables sampling but does not remove
> +	 * existing ATR entries. Flush them so stale filters do not
> +	 * persist on devices without HW ATR eviction.
> +	 */
> +	if (!sample_rate) {
> +		set_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state);
> +		set_bit(__I40E_FD_FLUSH_REQUESTED, pf->state);
> +	}

[Severity: Low]
Will this trigger unnecessary hardware flushes if a user or configuration
tool repeatedly sets the atr_sample_rate to 0 when it is already 0?

The devlink core in net/devlink/param.c:__devlink_nl_cmd_param_set_doit()
does not filter out unchanged values before invoking the driver's set
callback.

As a result, i40e_atr_sample_rate_set() unconditionally sets the
__I40E_FD_FLUSH_REQUESTED bit in pf->state, which then triggers a flush in
the background service task.

Would it be better to check if the new sample_rate differs from the
existing value before applying the change and requesting a flush?

> +
> +	return 0;
> +}

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