[PATCH net 0/8][pull request] Intel Wired LAN Driver Updates 2026-09-18 (i40e)

[PATCH net 0/8][pull request] Intel Wired LAN Driver Updates 2026-09-18 (i40e)

[Tony Nguyen]

Maciej fixes various issues with VSI setup, cleanup, and rebuild paths.
Notably, keeping q_vector allocation synchronized with channel-count
changes to prevent threaded NAPI reconfiguration failures.

Chenguang Zhao fixes AF_XDP multi-buffer handling by correctly copying
and attaching fragments and cleaning up on allocation failure.

The following are changes since commit 46bc52d13594848023e681860df8700c8db14354:
  ipv4: fib: fix data-race and stale genid check around nh->nh_saddr
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue 40GbE

Chenguang Zhao (1):
  i40e: xsk: fix multi-buffer XDP_PASS skb construction

Maciej Fijalkowski (7):
  i40e: unregister netdev before clearing VSI on reinit failure
  i40e: avoid null ptr dereference in i40e_ptp_stop()
  i40e: make ring pointers unreachable before freeing via rcu
  i40e: avoid deadlock when calling unregister_netdev()
  i40e: fix potential UAF in i40e_vsi_setup()'s error path
  i40e: do not expose netdev too early
  i40e: keep q_vectors array in sync with channel count changes

 drivers/net/ethernet/intel/i40e/i40e_main.c | 129 ++++++++++++--------
 drivers/net/ethernet/intel/i40e/i40e_ptp.c  |   5 +-
 drivers/net/ethernet/intel/i40e/i40e_xsk.c  |  13 +-
 3 files changed, 88 insertions(+), 59 deletions(-)

-- 
2.47.1

[PATCH net 1/8] i40e: unregister netdev before clearing VSI on reinit failure

[Tony Nguyen]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

i40e_vsi_reinit_setup() tears down the existing VSI queue/ring backing
state before allocating replacement arrays and queue tracking. If one of
these early allocations fails, the function jumps directly to err_vsi
and calls i40e_vsi_clear().

For a registered netdev, this frees the VSI while
netdev_priv(netdev)->vsi can still point at it, leaving the registered
netdev with dangling private driver state.

Split the error path so failures after destructive reinit teardown first
unregister and free the netdev before clearing the VSI.

Fixes: d2a69fefd756 ("i40e: Fix changing previously set num_queue_pairs for PFs")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 0cd0e5597c90..de4c0737f72e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14268,7 +14268,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
 	i40e_set_num_rings_in_vsi(vsi);
 	ret = i40e_vsi_alloc_arrays(vsi, false);
 	if (ret)
-		goto err_vsi;
+		goto err_netdev;
 
 	alloc_queue_pairs = vsi->alloc_queue_pairs *
 			    (i40e_enabled_xdp_vsi(vsi) ? 2 : 1);
@@ -14278,7 +14278,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
 		dev_info(&pf->pdev->dev,
 			 "failed to get tracking for %d queues for VSI %d err %d\n",
 			 alloc_queue_pairs, vsi->seid, ret);
-		goto err_vsi;
+		goto err_netdev;
 	}
 	vsi->base_queue = ret;
 
@@ -14303,6 +14303,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
 
 err_rings:
 	i40e_vsi_free_q_vectors(vsi);
+err_netdev:
 	if (vsi->netdev_registered) {
 		vsi->netdev_registered = false;
 		unregister_netdev(vsi->netdev);
@@ -14312,7 +14313,6 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
 	if (vsi->type == I40E_VSI_MAIN)
 		i40e_devlink_destroy_port(pf);
 	i40e_aq_delete_element(&pf->hw, vsi->seid, NULL);
-err_vsi:
 	i40e_vsi_clear(vsi);
 	return NULL;
 }
-- 
2.47.1

Re: [PATCH net 1/8] i40e: unregister netdev before clearing VSI on reinit failure

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch makes the two early allocation failures in
  i40e_vsi_reinit_setup() jump to the new err_netdev label, which calls…
- [Medium] At this commit the reinit teardown at the top of
  i40e_vsi_reinit_setup() deliberately keeps the existing per-vector…

Pre-existing issues:
- [High] When i40e_vsi_reinit_setup() fails it ends in i40e_vsi_clear(),
  which does kfree(vsi) and sets pf->vsi[vsi->idx] = NULL, and returns…

--

i40e: unregister netdev before clearing VSI on reinit failure

The patch splits the error path of i40e_vsi_reinit_setup() so that the two
early allocation failures (i40e_vsi_alloc_arrays() and i40e_get_lump())
reach a new err_netdev label which unregisters and frees the netdev before
calling i40e_vsi_clear().

A few questions about the new error path below.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 0cd0e5597c90..de4c0737f72e 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -14268,7 +14268,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
>  	i40e_set_num_rings_in_vsi(vsi);
>  	ret = i40e_vsi_alloc_arrays(vsi, false);
>  	if (ret)
> -		goto err_vsi;
> +		goto err_netdev;

[Severity: Medium]
Can these two new gotos leak the struct i40e_q_vector allocations?

The teardown at the top of i40e_vsi_reinit_setup() intentionally keeps the
per-vector objects alive:

	i40e_put_lump(pf->qp_pile, vsi->base_queue, vsi->idx);
	i40e_vsi_clear_rings(vsi);

	i40e_vsi_free_arrays(vsi, false);

With free_qvectors == false, both vsi->q_vectors and every individual
struct i40e_q_vector are still allocated at this point.

The new err_netdev label is placed below i40e_vsi_free_q_vectors(), so the
i40e_vsi_alloc_arrays() and i40e_get_lump() failures skip the per-vector
destructor i40e_free_q_vector(), which is what does netif_napi_del() and
kfree_rcu(q_vector, rcu).

The final i40e_vsi_clear() then calls i40e_vsi_free_arrays(vsi, true),
which frees only the q_vectors pointer array, and kfree(vsi) — so
num_q_vectors vector objects, each with an embedded NAPI, are never freed.

Would "goto err_rings" be the intended target here, since it falls through
into the same netdev teardown block that this patch wants to reach?

For the record, the leak is gone at the end of the series: "i40e: keep
q_vectors array in sync with channel count changes" adds
i40e_vsi_free_q_vectors() to the top-of-function teardown and reroutes the
i40e_get_lump() failure to err_rings, so nothing per-vector remains
allocated when err_netdev is taken. It only exists in this intermediate
commit.

>  
>  	alloc_queue_pairs = vsi->alloc_queue_pairs *
>  			    (i40e_enabled_xdp_vsi(vsi) ? 2 : 1);
> @@ -14278,7 +14278,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
>  		dev_info(&pf->pdev->dev,
>  			 "failed to get tracking for %d queues for VSI %d err %d\n",
>  			 alloc_queue_pairs, vsi->seid, ret);
> -		goto err_vsi;
> +		goto err_netdev;
>  	}
>  	vsi->base_queue = ret;
>  
> @@ -14303,6 +14303,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
>  
>  err_rings:
>  	i40e_vsi_free_q_vectors(vsi);
> +err_netdev:
>  	if (vsi->netdev_registered) {
>  		vsi->netdev_registered = false;
>  		unregister_netdev(vsi->netdev);

[Severity: High]
Is RTNL already held when the two newly redirected failures land here?

i40e_vsi_reinit_setup() is only reached from i40e_setup_pf_switch() with
reinit == true:

	else if (reinit)
		main_vsi = i40e_vsi_reinit_setup(main_vsi);

and i40e_rebuild() wraps that call in RTNL:

	if (!lock_acquired)
		rtnl_lock();
	ret = i40e_setup_pf_switch(pf, reinit, true);

unregister_netdev() takes the lock itself:

net/core/dev.c:unregister_netdev() {
	rtnl_net_dev_lock(dev);
	unregister_netdevice(dev);
	rtnl_net_dev_unlock(dev);
}

So a kzalloc failure in i40e_vsi_alloc_arrays() or an exhausted
pf->qp_pile in i40e_get_lump(), reached via ethtool -L or an XDP attach
going through i40e_reset_and_rebuild(pf, true, true) -> i40e_rebuild() ->
i40e_setup_pf_switch(), would recursively acquire RTNL. Does that hang the
task in D state while holding RTNL, blocking further network configuration
system-wide?

Before this patch those two failures went to err_vsi and never touched
RTNL, so the exposure is new here.

This is addressed later in the same series by "i40e: avoid deadlock when
calling unregister_netdev()", which adds a lock_acquired parameter and
selects unregister_netdevice() on the RTNL-held path, so the final state of
the series is fine. Could the label split and the lock_acquired plumbing be
folded into one patch so this commit is not broken on its own?

> @@ -14312,7 +14313,6 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
>  	if (vsi->type == I40E_VSI_MAIN)
>  		i40e_devlink_destroy_port(pf);
>  	i40e_aq_delete_element(&pf->hw, vsi->seid, NULL);
> -err_vsi:
>  	i40e_vsi_clear(vsi);
>  	return NULL;
>  }

[Severity: High]
This isn't a bug introduced by this patch — the pre-patch err_vsi path also
ended in i40e_vsi_clear() — but while the error path is being reworked, can
the callers still use the freed struct i40e_vsi after a failed reinit?

i40e_vsi_clear() does kfree(vsi) and sets pf->vsi[vsi->idx] = NULL, and
i40e_vsi_reinit_setup() returns NULL. i40e_setup_pf_switch() converts that
into -EAGAIN:

	if (!main_vsi) {
		dev_info(&pf->pdev->dev, "setup of MAIN VSI failed\n");
		i40e_cloud_filter_exit(pf);
		i40e_fdir_teardown(pf);
		return -EAGAIN;
	}

but i40e_rebuild() and i40e_reset_and_rebuild() both return void, so the
failure never reaches the operation that started the rebuild.

i40e_reconfig_rss_queues() keeps using its cached pointer after the reset,
including a write:

	i40e_reset_and_rebuild(pf, true, true);
	...
	if (queue_count < vsi->rss_size) {
	...
	vsi->rss_size = min_t(int, pf->alloc_rss_size, qcount);

i40e_xdp_setup() does the same, and writes through the ring array:

	i40e_reset_and_rebuild(pf, true, true);
	...
	for (i = 0; i < vsi->num_queue_pairs; i++)
		WRITE_ONCE(vsi->rx_rings[i]->xdp_prog, vsi->xdp_prog);

Is this a use-after-free read and write on the freed VSI? It still looks
present at the end of the series, so would a complete fix either propagate
the failure out of i40e_rebuild()/i40e_reset_and_rebuild() or keep the VSI
alive until these callers are done with it?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

Re: [PATCH net 1/8] i40e: unregister netdev before clearing VSI on reinit failure

[Maciej Fijalkowski]

On Tue, Sep 22, 2026 at 01:31:05AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 (1) · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] The patch makes the two early allocation failures in
>   i40e_vsi_reinit_setup() jump to the new err_netdev label, which calls…
> - [Medium] At this commit the reinit teardown at the top of
>   i40e_vsi_reinit_setup() deliberately keeps the existing per-vector…
> 
> Pre-existing issues:
> - [High] When i40e_vsi_reinit_setup() fails it ends in i40e_vsi_clear(),
>   which does kfree(vsi) and sets pf->vsi[vsi->idx] = NULL, and returns…
> 
> --
> 
> i40e: unregister netdev before clearing VSI on reinit failure
> 
> The patch splits the error path of i40e_vsi_reinit_setup() so that the two
> early allocation failures (i40e_vsi_alloc_arrays() and i40e_get_lump())
> reach a new err_netdev label which unregisters and frees the netdev before
> calling i40e_vsi_clear().
> 
> A few questions about the new error path below.
> 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > index 0cd0e5597c90..de4c0737f72e 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > @@ -14268,7 +14268,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
> >  	i40e_set_num_rings_in_vsi(vsi);
> >  	ret = i40e_vsi_alloc_arrays(vsi, false);
> >  	if (ret)
> > -		goto err_vsi;
> > +		goto err_netdev;
> 
> [Severity: Medium]
> Can these two new gotos leak the struct i40e_q_vector allocations?
> 
> The teardown at the top of i40e_vsi_reinit_setup() intentionally keeps the
> per-vector objects alive:
> 
> 	i40e_put_lump(pf->qp_pile, vsi->base_queue, vsi->idx);
> 	i40e_vsi_clear_rings(vsi);
> 
> 	i40e_vsi_free_arrays(vsi, false);
> 
> With free_qvectors == false, both vsi->q_vectors and every individual
> struct i40e_q_vector are still allocated at this point.
> 
> The new err_netdev label is placed below i40e_vsi_free_q_vectors(), so the
> i40e_vsi_alloc_arrays() and i40e_get_lump() failures skip the per-vector
> destructor i40e_free_q_vector(), which is what does netif_napi_del() and
> kfree_rcu(q_vector, rcu).

This is fixed by patch 7, which frees the q_vectors at the start of
i40e_vsi_reinit_setup() before the allocation paths can fail. The
i40e_get_lump() failure is also routed through err_rings so that the
newly allocated q_vectors are released.

> 
> The final i40e_vsi_clear() then calls i40e_vsi_free_arrays(vsi, true),
> which frees only the q_vectors pointer array, and kfree(vsi) — so
> num_q_vectors vector objects, each with an embedded NAPI, are never freed.
> 
> Would "goto err_rings" be the intended target here, since it falls through
> into the same netdev teardown block that this patch wants to reach?
> 
> For the record, the leak is gone at the end of the series: "i40e: keep
> q_vectors array in sync with channel count changes" adds
> i40e_vsi_free_q_vectors() to the top-of-function teardown and reroutes the
> i40e_get_lump() failure to err_rings, so nothing per-vector remains
> allocated when err_netdev is taken. It only exists in this intermediate
> commit.
> 
> >  
> >  	alloc_queue_pairs = vsi->alloc_queue_pairs *
> >  			    (i40e_enabled_xdp_vsi(vsi) ? 2 : 1);
> > @@ -14278,7 +14278,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
> >  		dev_info(&pf->pdev->dev,
> >  			 "failed to get tracking for %d queues for VSI %d err %d\n",
> >  			 alloc_queue_pairs, vsi->seid, ret);
> > -		goto err_vsi;
> > +		goto err_netdev;
> >  	}
> >  	vsi->base_queue = ret;
> >  
> > @@ -14303,6 +14303,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
> >  
> >  err_rings:
> >  	i40e_vsi_free_q_vectors(vsi);
> > +err_netdev:
> >  	if (vsi->netdev_registered) {
> >  		vsi->netdev_registered = false;
> >  		unregister_netdev(vsi->netdev);
> 
> [Severity: High]
> Is RTNL already held when the two newly redirected failures land here?
> 
> i40e_vsi_reinit_setup() is only reached from i40e_setup_pf_switch() with
> reinit == true:
> 
> 	else if (reinit)
> 		main_vsi = i40e_vsi_reinit_setup(main_vsi);
> 
> and i40e_rebuild() wraps that call in RTNL:
> 
> 	if (!lock_acquired)
> 		rtnl_lock();
> 	ret = i40e_setup_pf_switch(pf, reinit, true);
> 
> unregister_netdev() takes the lock itself:
> 
> net/core/dev.c:unregister_netdev() {
> 	rtnl_net_dev_lock(dev);
> 	unregister_netdevice(dev);
> 	rtnl_net_dev_unlock(dev);
> }
> 

Patch 4 passes the lock state into i40e_vsi_reinit_setup() and uses
unregister_netdevice() when RTNL is already held.


> So a kzalloc failure in i40e_vsi_alloc_arrays() or an exhausted
> pf->qp_pile in i40e_get_lump(), reached via ethtool -L or an XDP attach
> going through i40e_reset_and_rebuild(pf, true, true) -> i40e_rebuild() ->
> i40e_setup_pf_switch(), would recursively acquire RTNL. Does that hang the
> task in D state while holding RTNL, blocking further network configuration
> system-wide?
> 
> Before this patch those two failures went to err_vsi and never touched
> RTNL, so the exposure is new here.
> 
> This is addressed later in the same series by "i40e: avoid deadlock when
> calling unregister_netdev()", which adds a lock_acquired parameter and
> selects unregister_netdevice() on the RTNL-held path, so the final state of
> the series is fine. Could the label split and the lock_acquired plumbing be
> folded into one patch so this commit is not broken on its own?
> 
> > @@ -14312,7 +14313,6 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
> >  	if (vsi->type == I40E_VSI_MAIN)
> >  		i40e_devlink_destroy_port(pf);
> >  	i40e_aq_delete_element(&pf->hw, vsi->seid, NULL);
> > -err_vsi:
> >  	i40e_vsi_clear(vsi);
> >  	return NULL;
> >  }
> 
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

[PATCH net 2/8] i40e: avoid null ptr dereference in i40e_ptp_stop()

[Tony Nguyen]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Sashiko reports:
***
If an allocation fails here during i40e_rebuild(), i40e_vsi_clear()
frees the
main VSI and sets pf->vsi[vsi->idx] = NULL, and the rebuild will abort
without
stopping the PTP clock.
Later, if the device is removed or unbound, i40e_remove()
unconditionally
calls i40e_ptp_stop(), which does:
drivers/net/ethernet/intel/i40e/i40e_ptp.c:i40e_ptp_stop() {
    ...
	struct i40e_vsi *main_vsi = i40e_pf_get_main_vsi(pf);
    ...
	dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
		 main_vsi->netdev->name);
    ...
}
Would this cause a NULL pointer dereference since main_vsi is now NULL?
***

Check if main_vsi is not null before calling dev_info().

Fixes: beb0dff1251d ("i40e: enable PTP")
Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ptp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index ff62b5f2c815..ca93df4d6785 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -1556,8 +1556,9 @@ void i40e_ptp_stop(struct i40e_pf *pf)
 	if (pf->ptp_clock) {
 		ptp_clock_unregister(pf->ptp_clock);
 		pf->ptp_clock = NULL;
-		dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
-			 main_vsi->netdev->name);
+		if (main_vsi)
+			dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
+				 main_vsi->netdev->name);
 	}
 
 	if (i40e_is_ptp_pin_dev(&pf->hw)) {
-- 
2.47.1

Re: [PATCH net 2/8] i40e: avoid null ptr dereference in i40e_ptp_stop()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Symptom-only fix: when `i40e_rebuild()` aborts because the
  main VSI could not be re-created, the PTP subsystem is left in an…

--

i40e: avoid null ptr dereference in i40e_ptp_stop()

If the main VSI cannot be re-created during a reset, i40e_rebuild() aborts
after i40e_vsi_clear() has freed the main VSI, so a later i40e_remove()
calls i40e_ptp_stop() with pf->vsi[pf->lan_vsi] == NULL.  The patch adds a
main_vsi NULL check around the dev_info() that dereferences
main_vsi->netdev->name.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> index ff62b5f2c815..ca93df4d6785 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -1556,8 +1556,9 @@ void i40e_ptp_stop(struct i40e_pf *pf)
>  	if (pf->ptp_clock) {
>  		ptp_clock_unregister(pf->ptp_clock);
>  		pf->ptp_clock = NULL;
> -		dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
> -			 main_vsi->netdev->name);
> +		if (main_vsi)
> +			dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
> +				 main_vsi->netdev->name);
>  	}

[Severity: Medium]

Is guarding the log message here enough, or does it just make the window
between the failed rebuild and the eventual remove survivable?

In that window nothing appears to touch PTP at all.  i40e_ptp_stop() has
only two callers, i40e_probe() (error path) and i40e_remove(), and none of
the abort paths in i40e_rebuild() (end_unlock / end_core_reset /
clear_recovery) call i40e_ptp_init() or i40e_ptp_stop():

drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_setup_pf_switch() {
	...
		if (!main_vsi) {
			dev_info(&pf->pdev->dev, "setup of MAIN VSI failed\n");
			i40e_cloud_filter_exit(pf);
			i40e_fdir_teardown(pf);
			return -EAGAIN;
		}
	...
}

Across the reset, the only PTP action is i40e_prep_for_reset() calling
i40e_ptp_save_hw_time(), which is a pure read.

So after the abort, pf->ptp_clock stays registered and I40E_FLAG_PTP_ENA,
pf->ptp_tx, pf->ptp_rx and pf->tstamp_config stay set, even though the main
VSI and its netdev were freed, and the register programming that
i40e_ptp_init() normally redoes after a reset never runs:

drivers/net/ethernet/intel/i40e/i40e_ptp.c:i40e_ptp_init() {
	...
		regval = rd32(hw, I40E_PRTTSYN_CTL0);
		regval |= I40E_PRTTSYN_CTL0_TSYNENA_MASK;
		wr32(hw, I40E_PRTTSYN_CTL0, regval);
	...
}

i40e_ptp_set_increment(), i40e_ptp_set_timestamp_mode(),
i40e_ptp_restore_hw_time() and i40e_ptp_set_1pps_signal_hw() are skipped in
the same way, and i40e_ptp_init()'s own kerneldoc describes it as
reconfiguring the device after a reset.

Does that leave userspace with an openable /dev/ptpN whose gettime/settime/
adjfine operate on a timesync block the reset left disabled, and PF flags
still advertising timestamping that hardware will not deliver?  A Tx
timestamp request in that state would set __I40E_PTP_TX_IN_PROGRESS for a
timestamp that never arrives, recovered only by the watchdog.

Would it be better to quiesce PTP on the rebuild-failure path, for example
unregistering the PHC or at least clearing I40E_FLAG_PTP_ENA, rather than
adding a NULL check per consumer?  As written, every future user of the main
VSI or of the PTP state in this window needs its own check.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

Re: [PATCH net 2/8] i40e: avoid null ptr dereference in i40e_ptp_stop()

[Maciej Fijalkowski]

On Tue, Sep 22, 2026 at 01:31:06AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] Symptom-only fix: when `i40e_rebuild()` aborts because the
>   main VSI could not be re-created, the PTP subsystem is left in an…
> 
> --
> 
> i40e: avoid null ptr dereference in i40e_ptp_stop()
> 
> If the main VSI cannot be re-created during a reset, i40e_rebuild() aborts
> after i40e_vsi_clear() has freed the main VSI, so a later i40e_remove()
> calls i40e_ptp_stop() with pf->vsi[pf->lan_vsi] == NULL.  The patch adds a
> main_vsi NULL check around the dev_info() that dereferences
> main_vsi->netdev->name.
> 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > index ff62b5f2c815..ca93df4d6785 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > @@ -1556,8 +1556,9 @@ void i40e_ptp_stop(struct i40e_pf *pf)
> >  	if (pf->ptp_clock) {
> >  		ptp_clock_unregister(pf->ptp_clock);
> >  		pf->ptp_clock = NULL;
> > -		dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
> > -			 main_vsi->netdev->name);
> > +		if (main_vsi)
> > +			dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
> > +				 main_vsi->netdev->name);
> >  	}
> 
> [Severity: Medium]
> 
> Is guarding the log message here enough, or does it just make the window
> between the failed rebuild and the eventual remove survivable?
> 
> In that window nothing appears to touch PTP at all.  i40e_ptp_stop() has
> only two callers, i40e_probe() (error path) and i40e_remove(), and none of
> the abort paths in i40e_rebuild() (end_unlock / end_core_reset /
> clear_recovery) call i40e_ptp_init() or i40e_ptp_stop():
> 
> drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_setup_pf_switch() {
> 	...
> 		if (!main_vsi) {
> 			dev_info(&pf->pdev->dev, "setup of MAIN VSI failed\n");
> 			i40e_cloud_filter_exit(pf);
> 			i40e_fdir_teardown(pf);
> 			return -EAGAIN;
> 		}
> 	...
> }
> 
> Across the reset, the only PTP action is i40e_prep_for_reset() calling
> i40e_ptp_save_hw_time(), which is a pure read.
> 
> So after the abort, pf->ptp_clock stays registered and I40E_FLAG_PTP_ENA,
> pf->ptp_tx, pf->ptp_rx and pf->tstamp_config stay set, even though the main
> VSI and its netdev were freed, and the register programming that
> i40e_ptp_init() normally redoes after a reset never runs:
> 
> drivers/net/ethernet/intel/i40e/i40e_ptp.c:i40e_ptp_init() {
> 	...
> 		regval = rd32(hw, I40E_PRTTSYN_CTL0);
> 		regval |= I40E_PRTTSYN_CTL0_TSYNENA_MASK;
> 		wr32(hw, I40E_PRTTSYN_CTL0, regval);
> 	...
> }
> 
> i40e_ptp_set_increment(), i40e_ptp_set_timestamp_mode(),
> i40e_ptp_restore_hw_time() and i40e_ptp_set_1pps_signal_hw() are skipped in
> the same way, and i40e_ptp_init()'s own kerneldoc describes it as
> reconfiguring the device after a reset.
> 
> Does that leave userspace with an openable /dev/ptpN whose gettime/settime/
> adjfine operate on a timesync block the reset left disabled, and PF flags
> still advertising timestamping that hardware will not deliver?  A Tx
> timestamp request in that state would set __I40E_PTP_TX_IN_PROGRESS for a
> timestamp that never arrives, recovered only by the watchdog.
> 
> Would it be better to quiesce PTP on the rebuild-failure path, for example
> unregistering the PHC or at least clearing I40E_FLAG_PTP_ENA, rather than
> adding a NULL check per consumer?  As written, every future user of the main
> VSI or of the PTP state in this window needs its own check.

I didn't want to touch ptp initially and this commit was rather to satisfy
sashiko (which was successful!) and here the broader concern is being
raised and I suppose I won't escape from addressing this :<

> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

[PATCH net 3/8] i40e: make ring pointers unreachable before freeing via rcu

[Tony Nguyen]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Sashiko reports:
***
>  err_config:
> +	i40e_vsi_free_q_vectors(vsi);
> +err_qvec:
>  	i40e_vsi_clear_rings(vsi);
This is a pre-existing issue, but can the sequence in i40e_vsi_clear_rings()
lead to an RCU ordering violation?
In i40e_vsi_clear_rings(), the rings are freed before the array pointers are
nullified:
	kfree_rcu(vsi->tx_rings[i], rcu);
	WRITE_ONCE(vsi->tx_rings[i], NULL);
Under RCU rules, a pointer must be made unreachable to new readers before it
is handed off to kfree_rcu(). Could a new RCU reader (like
i40e_get_netdev_stats_struct_tx()) fetch the pointer after kfree_rcu() is
invoked, and access freed memory if the grace period expires while the
reader is still active?
***

Save the Tx ring pointer before clearing the published ring array slots
and pass the saved pointer to kfree_rcu(). This preserves the intended
RCU ordering, where new readers can no longer discover the ring through
vsi->tx_rings/rx_rings/xdp_rings before the object is queued for
deferred freeing, while avoiding a NULL kfree_rcu() argument after the
slot has already been cleared. Since the Tx pointer is the base of the
per-queue-pair allocation block, re-reading vsi->tx_rings[i] after
WRITE_ONCE(..., NULL) would otherwise turn the free into a no-op and
leak the whole ring block.

Fixes: 9f65e15b4f98 ("i40e: Move rings from pointer to array to array of pointers")
Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index de4c0737f72e..65aa50330aac 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11693,11 +11693,13 @@ static void i40e_vsi_clear_rings(struct i40e_vsi *vsi)
 
 	if (vsi->tx_rings && vsi->tx_rings[0]) {
 		for (i = 0; i < vsi->alloc_queue_pairs; i++) {
-			kfree_rcu(vsi->tx_rings[i], rcu);
+			struct i40e_ring *tx_ring = vsi->tx_rings[i];
+
 			WRITE_ONCE(vsi->tx_rings[i], NULL);
 			WRITE_ONCE(vsi->rx_rings[i], NULL);
 			if (vsi->xdp_rings)
 				WRITE_ONCE(vsi->xdp_rings[i], NULL);
+			kfree_rcu(tx_ring, rcu);
 		}
 	}
 }
-- 
2.47.1

Re: [PATCH net 3/8] i40e: make ring pointers unreachable before freeing via rcu

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Incomplete RCU teardown: the ring-pointer arrays through which
  RCU readers discover the rings are freed with a plain kfree() and…

--

i40e: make ring pointers unreachable before freeing via rcu

The patch reorders i40e_vsi_clear_rings() so the published slots in
vsi->tx_rings/rx_rings/xdp_rings are cleared before the saved Tx ring
pointer is handed to kfree_rcu(). The Tx pointer is saved first because it
is the base of the per-queue-pair allocation block.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index de4c0737f72e..65aa50330aac 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -11693,11 +11693,13 @@ static void i40e_vsi_clear_rings(struct i40e_vsi *vsi)
>  
>  	if (vsi->tx_rings && vsi->tx_rings[0]) {
>  		for (i = 0; i < vsi->alloc_queue_pairs; i++) {
> -			kfree_rcu(vsi->tx_rings[i], rcu);
> +			struct i40e_ring *tx_ring = vsi->tx_rings[i];
> +
>  			WRITE_ONCE(vsi->tx_rings[i], NULL);
>  			WRITE_ONCE(vsi->rx_rings[i], NULL);
>  			if (vsi->xdp_rings)
>  				WRITE_ONCE(vsi->xdp_rings[i], NULL);
> +			kfree_rcu(tx_ring, rcu);

[Severity: High]
The ring objects are now unpublished before kfree_rcu(), but is the array
that readers index through itself RCU protected?

The commit message states that after this change "new readers can no longer
discover the ring through vsi->tx_rings/rx_rings/xdp_rings before the object
is queued for deferred freeing".  Does that hold on the i40e_vsi_reinit_setup()
path, where the arrays are dropped immediately afterwards?

drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_vsi_reinit_setup() {
	i40e_put_lump(pf->qp_pile, vsi->base_queue, vsi->idx);
	i40e_vsi_clear_rings(vsi);

	i40e_vsi_free_arrays(vsi, false);
	i40e_set_num_rings_in_vsi(vsi);
	ret = i40e_vsi_alloc_arrays(vsi, false);
	...
}

and i40e_vsi_free_arrays() uses a plain kfree() plus plain stores, with no
grace period:

drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_vsi_free_arrays() {
	...
	kfree(vsi->tx_rings);
	vsi->tx_rings = NULL;
	vsi->rx_rings = NULL;
	vsi->xdp_rings = NULL;
}

Meanwhile i40e_get_netdev_stats_struct() (ndo_get_stats64, reached from
dev_get_stats() without rtnl) re-evaluates the non-volatile array base on
every iteration inside the RCU section, and never checks the rx_rings or
xdp_rings bases at all:

drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_get_netdev_stats_struct() {
	if (!vsi->tx_rings)
		return;

	rcu_read_lock();
	for (i = 0; i < vsi->num_queue_pairs; i++) {
		...
		ring = READ_ONCE(vsi->tx_rings[i]);
		...
		if (i40e_enabled_xdp_vsi(vsi)) {
			ring = READ_ONCE(vsi->xdp_rings[i]);
	...
}

i40e_update_vsi_stats() does the same with no base check whatsoever:

drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_update_vsi_stats() {
	rcu_read_lock();
	for (q = 0; q < vsi->num_queue_pairs; q++) {
		/* locate Tx ring */
		p = READ_ONCE(vsi->tx_rings[q]);
	...
}

So can a reader that is already past the "if (!vsi->tx_rings)" test, or
inside the loop, reload vsi->tx_rings/rx_rings/xdp_rings after
i40e_vsi_free_arrays() nulls them and dereference NULL[i]?

And once the pointer-array slab is recycled, for instance by the
i40e_vsi_alloc_arrays() call two statements later, or by the SLUB freelist
pointer written into the freed object, can the same reader read a non-NULL
wild value out of the freed array and dereference it as struct i40e_ring
(->syncp, ->stats)?

The __I40E_VSI_DOWN test at the top of both readers is evaluated before
rcu_read_lock(), and nothing on this path drains readers that already passed
it, so it does not appear to close the window.  Should the arrays be released
through kfree_rcu() (or after a synchronize_rcu()), with the base loaded once
via rcu_dereference(), for the unpublish ordering established here to actually
take effect?

Separately, both readers bound their loops with vsi->num_queue_pairs, which
i40e_set_num_rings_in_vsi() changes between i40e_vsi_free_arrays() and
i40e_vsi_alloc_arrays().  Can that let a reader index past the end of the
freshly allocated array?

>  		}
>  	}
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

Re: [PATCH net 3/8] i40e: make ring pointers unreachable before freeing via rcu

[Maciej Fijalkowski]

On Tue, Sep 22, 2026 at 01:31:08AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] Incomplete RCU teardown: the ring-pointer arrays through which
>   RCU readers discover the rings are freed with a plain kfree() and…

this rcu'ing rings but not containers always felt a bit incomplete to me
and while i could say it is a pre-existing issue i think it will be a good
thing to address...unless it's gonna open up this Pandora's box further :<

> 
> --
> 
> i40e: make ring pointers unreachable before freeing via rcu
> 
> The patch reorders i40e_vsi_clear_rings() so the published slots in
> vsi->tx_rings/rx_rings/xdp_rings are cleared before the saved Tx ring
> pointer is handed to kfree_rcu(). The Tx pointer is saved first because it
> is the base of the per-queue-pair allocation block.
> 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > index de4c0737f72e..65aa50330aac 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > @@ -11693,11 +11693,13 @@ static void i40e_vsi_clear_rings(struct i40e_vsi *vsi)
> >  
> >  	if (vsi->tx_rings && vsi->tx_rings[0]) {
> >  		for (i = 0; i < vsi->alloc_queue_pairs; i++) {
> > -			kfree_rcu(vsi->tx_rings[i], rcu);
> > +			struct i40e_ring *tx_ring = vsi->tx_rings[i];
> > +
> >  			WRITE_ONCE(vsi->tx_rings[i], NULL);
> >  			WRITE_ONCE(vsi->rx_rings[i], NULL);
> >  			if (vsi->xdp_rings)
> >  				WRITE_ONCE(vsi->xdp_rings[i], NULL);
> > +			kfree_rcu(tx_ring, rcu);
> 
> [Severity: High]
> The ring objects are now unpublished before kfree_rcu(), but is the array
> that readers index through itself RCU protected?
> 
> The commit message states that after this change "new readers can no longer
> discover the ring through vsi->tx_rings/rx_rings/xdp_rings before the object
> is queued for deferred freeing".  Does that hold on the i40e_vsi_reinit_setup()
> path, where the arrays are dropped immediately afterwards?
> 
> drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_vsi_reinit_setup() {
> 	i40e_put_lump(pf->qp_pile, vsi->base_queue, vsi->idx);
> 	i40e_vsi_clear_rings(vsi);
> 
> 	i40e_vsi_free_arrays(vsi, false);
> 	i40e_set_num_rings_in_vsi(vsi);
> 	ret = i40e_vsi_alloc_arrays(vsi, false);
> 	...
> }
> 
> and i40e_vsi_free_arrays() uses a plain kfree() plus plain stores, with no
> grace period:
> 
> drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_vsi_free_arrays() {
> 	...
> 	kfree(vsi->tx_rings);
> 	vsi->tx_rings = NULL;
> 	vsi->rx_rings = NULL;
> 	vsi->xdp_rings = NULL;
> }
> 
> Meanwhile i40e_get_netdev_stats_struct() (ndo_get_stats64, reached from
> dev_get_stats() without rtnl) re-evaluates the non-volatile array base on
> every iteration inside the RCU section, and never checks the rx_rings or
> xdp_rings bases at all:
> 
> drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_get_netdev_stats_struct() {
> 	if (!vsi->tx_rings)
> 		return;
> 
> 	rcu_read_lock();
> 	for (i = 0; i < vsi->num_queue_pairs; i++) {
> 		...
> 		ring = READ_ONCE(vsi->tx_rings[i]);
> 		...
> 		if (i40e_enabled_xdp_vsi(vsi)) {
> 			ring = READ_ONCE(vsi->xdp_rings[i]);
> 	...
> }
> 
> i40e_update_vsi_stats() does the same with no base check whatsoever:
> 
> drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_update_vsi_stats() {
> 	rcu_read_lock();
> 	for (q = 0; q < vsi->num_queue_pairs; q++) {
> 		/* locate Tx ring */
> 		p = READ_ONCE(vsi->tx_rings[q]);
> 	...
> }
> 
> So can a reader that is already past the "if (!vsi->tx_rings)" test, or
> inside the loop, reload vsi->tx_rings/rx_rings/xdp_rings after
> i40e_vsi_free_arrays() nulls them and dereference NULL[i]?
> 
> And once the pointer-array slab is recycled, for instance by the
> i40e_vsi_alloc_arrays() call two statements later, or by the SLUB freelist
> pointer written into the freed object, can the same reader read a non-NULL
> wild value out of the freed array and dereference it as struct i40e_ring
> (->syncp, ->stats)?
> 
> The __I40E_VSI_DOWN test at the top of both readers is evaluated before
> rcu_read_lock(), and nothing on this path drains readers that already passed
> it, so it does not appear to close the window.  Should the arrays be released
> through kfree_rcu() (or after a synchronize_rcu()), with the base loaded once
> via rcu_dereference(), for the unpublish ordering established here to actually
> take effect?
> 
> Separately, both readers bound their loops with vsi->num_queue_pairs, which
> i40e_set_num_rings_in_vsi() changes between i40e_vsi_free_arrays() and
> i40e_vsi_alloc_arrays().  Can that let a reader index past the end of the
> freshly allocated array?
> 
> >  		}
> >  	}
> >  }
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

[PATCH net 4/8] i40e: avoid deadlock when calling unregister_netdev()

[Tony Nguyen]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Sashiko reports:
***
> +err_netdev:
>  	if (vsi->netdev_registered) {
>  		vsi->netdev_registered = false;
>  		unregister_netdev(vsi->netdev);
Could this result in a deadlock when called during a device rebuild?
Looking at i40e_rebuild(), it explicitly acquires the RTNL lock before
proceeding:
drivers/net/ethernet/intel/i40e/i40e_main.c:i40e_rebuild() {
    ...
	if (!lock_acquired)
		rtnl_lock();
	ret = i40e_setup_pf_switch(pf, reinit, true);
    ...
}
If i40e_setup_pf_switch() calls i40e_vsi_reinit_setup() and takes this new
err_netdev path, unregister_netdev() will unconditionally attempt to acquire
rtnl_lock(), leading to a deadlock on the non-recursive mutex.
***

Use unregister_netdevice() when the rebuild path already holds RTNL, and
keep unregister_netdev() for callers that do not. This avoids both
recursive RTNL locking and dropping RTNL in the middle of the VSI unwind
path.

Fixes: bc7d338fbb3f ("i40e: reinit flow for the main VSI")
Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 65aa50330aac..5ea8731ece08 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14245,13 +14245,15 @@ static int i40e_vsi_setup_vectors(struct i40e_vsi *vsi)
 /**
  * i40e_vsi_reinit_setup - return and reallocate resources for a VSI
  * @vsi: pointer to the vsi.
+ * @lock_acquired: indicates whether or not the lock has been acquired
  *
  * This re-allocates a vsi's queue resources.
  *
  * Returns pointer to the successfully allocated and configured VSI sw struct
  * on success, otherwise returns NULL on failure.
  **/
-static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
+static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi,
+					      bool lock_acquired)
 {
 	struct i40e_vsi *main_vsi;
 	u16 alloc_queue_pairs;
@@ -14308,7 +14310,10 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
 err_netdev:
 	if (vsi->netdev_registered) {
 		vsi->netdev_registered = false;
-		unregister_netdev(vsi->netdev);
+		if (lock_acquired)
+			unregister_netdevice(vsi->netdev);
+		else
+			unregister_netdev(vsi->netdev);
 		free_netdev(vsi->netdev);
 		vsi->netdev = NULL;
 	}
@@ -15030,7 +15035,7 @@ static int i40e_setup_pf_switch(struct i40e_pf *pf, bool reinit, bool lock_acqui
 			main_vsi = i40e_vsi_setup(pf, I40E_VSI_MAIN,
 						  uplink_seid, 0);
 		else if (reinit)
-			main_vsi = i40e_vsi_reinit_setup(main_vsi);
+			main_vsi = i40e_vsi_reinit_setup(main_vsi, lock_acquired);
 		if (!main_vsi) {
 			dev_info(&pf->pdev->dev, "setup of MAIN VSI failed\n");
 			i40e_cloud_filter_exit(pf);
-- 
2.47.1

Re: [PATCH net 4/8] i40e: avoid deadlock when calling unregister_netdev()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 0 (2) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Incomplete fix: the RTNL self-deadlock is only removed from the
  i40e_vsi_reinit_setup() arm of i40e_setup_pf_switch().

Pre-existing issues:
- [High] Dangling main-VSI references after a failed VSI reinit.
- [Medium] NULL pointer dereference when the reinit unwind re-enters
  ndo_stop after q_vectors/rings were released.
- [Medium] Memory leak in the asymmetric unwind of
  i40e_vsi_reinit_setup() as it exists at this commit: the two early…

--

i40e: avoid deadlock when calling unregister_netdev()

This adds a lock_acquired parameter to i40e_vsi_reinit_setup() so the
err_netdev unwind uses unregister_netdevice() when RTNL is already held by
the rebuild path, and unregister_netdev() otherwise. The caller in
i40e_setup_pf_switch() forwards its own lock_acquired flag.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 65aa50330aac..5ea8731ece08 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c

[ ... ]

> @@ -14308,7 +14310,10 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
>  err_netdev:
>  	if (vsi->netdev_registered) {
>  		vsi->netdev_registered = false;
> -		unregister_netdev(vsi->netdev);
> +		if (lock_acquired)
> +			unregister_netdevice(vsi->netdev);
> +		else
> +			unregister_netdev(vsi->netdev);
>  		free_netdev(vsi->netdev);
>  		vsi->netdev = NULL;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but now that this unwind can
actually run to completion instead of deadlocking, what happens to
pf->lan_vsi afterwards?

The tail of this path is:

	i40e_aq_delete_element(&pf->hw, vsi->seid, NULL);
	i40e_vsi_clear(vsi);
	return NULL;

and i40e_vsi_clear() does:

	pf->vsi[vsi->idx] = NULL;
	...
free_vsi:
	kfree(vsi);

pf->lan_vsi is never reset to I40E_NO_VSI, so i40e_pf_get_main_vsi()
returns NULL from then on.

After i40e_setup_pf_switch() returns -EAGAIN, i40e_rebuild() only clears
__I40E_RESET_FAILED, __I40E_RESET_RECOVERY_PENDING and
__I40E_TIMEOUT_RECOVERY_PENDING; neither __I40E_DOWN nor __I40E_SUSPENDED
is set, so the service task keeps running. Can the next watchdog tick or
link event then oops in i40e_link_event(), which dereferences the main VSI
with no NULL check?

	if (new_link == old_link &&
	    new_link_speed == old_link_speed &&
	    (test_bit(__I40E_VSI_DOWN, vsi->state) ||
	     new_link == netif_carrier_ok(vsi->netdev)))
		return;
	...
	i40e_print_link_message(vsi, new_link);

There is also the ethtool -L caller. i40e_reconfig_rss_queues() caches the
main VSI pointer, calls the void i40e_reset_and_rebuild(pf, true, true) and
then keeps using the pointer:

	i40e_reset_and_rebuild(pf, true, true);
	...
	if (queue_count < vsi->rss_size) {
	...
	qcount = vsi->num_queue_pairs / vsi->tc_config.numtc;

Does that dereference a VSI that this error path already kfree()d, since the
rebuild failure is not reported back to the caller?

[Severity: Medium]
This is a pre-existing issue, but the ordering between err_rings and
err_netdev looks worth a second look now that the unregister actually
proceeds under RTNL.

err_rings calls i40e_vsi_free_q_vectors(vsi), which NULLs every
vsi->q_vectors[i], and then falls through to this unregister. Both
unregister_netdevice() and unregister_netdev() reach
unregister_netdevice_many_notify() -> netif_close_many(), which calls
ndo_stop for any device that still has IFF_UP set:

net/core/dev.c:netif_close_many() {
	list_for_each_entry_safe(dev, tmp, head, close_list)
		if (!(dev->flags & IFF_UP))
			list_del_init(&dev->close_list);

	__dev_close_many(head);
}

IFF_UP is still set because the reset quiesce calls ndo_stop directly rather
than dev_close():

i40e_quiesce_vsi() {
	...
		vsi->netdev->netdev_ops->ndo_stop(vsi->netdev);
	...
}

So i40e_close() -> i40e_vsi_close() runs on the already dismantled VSI:

	if (!test_and_set_bit(__I40E_VSI_DOWN, vsi->state))
		i40e_down(vsi);

and i40e_down() -> i40e_napi_disable_all() dereferences the freed vector
array without a NULL check:

		struct i40e_q_vector *q_vector = vsi->q_vectors[q_idx];

		if (q_vector->rx.ring || q_vector->tx.ring)

__I40E_VSI_DOWN can still be clear here, since i40e_prep_for_reset()
returns early and skips i40e_pf_quiesce_all_vsi() when a reset is already
pending:

	if (test_and_set_bit(__I40E_RESET_RECOVERY_PENDING, pf->state))
		return;

Can this combination oops in i40e_napi_disable_all(), or is there something
in the ethtool/ndo entry points that prevents a reinit from starting while
another reset is pending?

[Severity: Medium]
This isn't a bug introduced by this patch either, and it looks already
addressed in this tree, but noting it for completeness: the two early
goto err_netdev paths (i40e_vsi_alloc_arrays() failure and i40e_get_lump()
failure) skip i40e_vsi_free_q_vectors(), which only the err_rings label
performs:

err_rings:
	i40e_vsi_free_q_vectors(vsi);
err_netdev:

With the older shape of this function, which kept the previously allocated
q_vectors alive across the reinit, i40e_vsi_clear() -> i40e_vsi_free_arrays()
would only free the pointer array:

	if (free_qvectors) {
		kfree(vsi->q_vectors);
		vsi->q_vectors = NULL;
	}

leaving each struct i40e_q_vector orphaned, since free_netdev() only unlinks
NAPI instances from dev->napi_list and does not free the containing
q_vector. The commit "i40e: keep q_vectors array in sync with channel count
changes" moves i40e_vsi_free_q_vectors() to the top of
i40e_vsi_reinit_setup() and re-creates the vectors via
i40e_vsi_setup_vectors(), so no live q_vector objects remain when the early
err_netdev paths run and the leak is gone.

> @@ -15030,7 +15035,7 @@ static int i40e_setup_pf_switch(struct i40e_pf *pf, bool reinit, bool lock_acqui
>  			main_vsi = i40e_vsi_setup(pf, I40E_VSI_MAIN,
>  						  uplink_seid, 0);
>  		else if (reinit)
> -			main_vsi = i40e_vsi_reinit_setup(main_vsi);
> +			main_vsi = i40e_vsi_reinit_setup(main_vsi, lock_acquired);

[Severity: High]
Should the other arm of this if/else get the same treatment? Both branches
are entered with identical lock state:

i40e_rebuild() {
	...
	if (!lock_acquired)
		rtnl_lock();
	ret = i40e_setup_pf_switch(pf, reinit, true);
	...
}

but i40e_vsi_setup() still takes RTNL itself, both on success:

	ret = register_netdev(vsi->netdev);
	if (ret)
		goto err_dl_port;
	vsi->netdev_registered = true;

and on its unwind, which is the exact pattern this patch just replaced:

err_msix:
	if (vsi->netdev_registered) {
		vsi->netdev_registered = false;
		unregister_netdev(vsi->netdev);
		free_netdev(vsi->netdev);
		vsi->netdev = NULL;
	}

Can the !main_vsi branch be reached with RTNL held? The err_netdev path
patched above ends in i40e_vsi_clear(), which sets pf->vsi[vsi->idx] = NULL
and kfree()s the VSI without resetting pf->lan_vsi, so
i40e_pf_get_main_vsi() returns NULL on the next reset. The following
rebuild would then take:

		if (!main_vsi)
			main_vsi = i40e_vsi_setup(pf, I40E_VSI_MAIN,
						  uplink_seid, 0);

with rtnl_mutex already held, and register_netdev() -> rtnl_lock() would
self-deadlock on the non-recursive mutex, which is the failure mode this
commit describes fixing.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

Re: [PATCH net 4/8] i40e: avoid deadlock when calling unregister_netdev()

[Maciej Fijalkowski]

On Tue, Sep 22, 2026 at 01:31:10AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 (1) · Medium: 0 (2) · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] Incomplete fix: the RTNL self-deadlock is only removed from the
>   i40e_vsi_reinit_setup() arm of i40e_setup_pf_switch().
> 
> Pre-existing issues:
> - [High] Dangling main-VSI references after a failed VSI reinit.
> - [Medium] NULL pointer dereference when the reinit unwind re-enters
>   ndo_stop after q_vectors/rings were released.
> - [Medium] Memory leak in the asymmetric unwind of
>   i40e_vsi_reinit_setup() as it exists at this commit: the two early…
> 
> --
> 
> i40e: avoid deadlock when calling unregister_netdev()
> 
> This adds a lock_acquired parameter to i40e_vsi_reinit_setup() so the
> err_netdev unwind uses unregister_netdevice() when RTNL is already held by
> the rebuild path, and unregister_netdev() otherwise. The caller in
> i40e_setup_pf_switch() forwards its own lock_acquired flag.
> 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > index 65aa50330aac..5ea8731ece08 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> 
> [ ... ]

> [Severity: High]
> Should the other arm of this if/else get the same treatment? Both branches
> are entered with identical lock state:
> 
> i40e_rebuild() {
> 	...
> 	if (!lock_acquired)
> 		rtnl_lock();
> 	ret = i40e_setup_pf_switch(pf, reinit, true);
> 	...
> }
> 
> but i40e_vsi_setup() still takes RTNL itself, both on success:
> 
> 	ret = register_netdev(vsi->netdev);
> 	if (ret)
> 		goto err_dl_port;
> 	vsi->netdev_registered = true;
> 
> and on its unwind, which is the exact pattern this patch just replaced:
> 
> err_msix:
> 	if (vsi->netdev_registered) {
> 		vsi->netdev_registered = false;
> 		unregister_netdev(vsi->netdev);
> 		free_netdev(vsi->netdev);
> 		vsi->netdev = NULL;
> 	}
> 
> Can the !main_vsi branch be reached with RTNL held? The err_netdev path
> patched above ends in i40e_vsi_clear(), which sets pf->vsi[vsi->idx] = NULL
> and kfree()s the VSI without resetting pf->lan_vsi, so
> i40e_pf_get_main_vsi() returns NULL on the next reset. The following
> rebuild would then take:
> 
> 		if (!main_vsi)
> 			main_vsi = i40e_vsi_setup(pf, I40E_VSI_MAIN,
> 						  uplink_seid, 0);
> 
> with rtnl_mutex already held, and register_netdev() -> rtnl_lock() would
> self-deadlock on the non-recursive mutex, which is the failure mode this
> commit describes fixing.

seems i40e_vsi_setup needs the same teaching regarding lock being held

> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

[PATCH net 5/8] i40e: fix potential UAF in i40e_vsi_setup()'s error path

[Tony Nguyen]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Sashiko pointed out an issue where error path in i40e_vsi_reinit_setup()
released ring memory but then when freeing q_vectors, the rings mapped
to q_vectors where touched which implies a regular use-after-free bug.

Apparently i40e_vsi_setup() has the same problem, so swap the allocation
and freeing order and fix the 13 year old bug.

Fixes: 41c445ff0f48 ("i40e: main driver core")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 5ea8731ece08..2b4b061302db 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14461,14 +14461,14 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 		fallthrough;
 	case I40E_VSI_FDIR:
 		/* set up vectors and rings if needed */
-		ret = i40e_vsi_setup_vectors(vsi);
-		if (ret)
-			goto err_msix;
-
 		ret = i40e_alloc_rings(vsi);
 		if (ret)
 			goto err_rings;
 
+		ret = i40e_vsi_setup_vectors(vsi);
+		if (ret)
+			goto err_qvec;
+
 		/* map all of the rings to the q_vectors */
 		i40e_vsi_map_rings_to_vectors(vsi);
 
@@ -14488,10 +14488,10 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 	return vsi;
 
 err_config:
+	i40e_vsi_free_q_vectors(vsi);
+err_qvec:
 	i40e_vsi_clear_rings(vsi);
 err_rings:
-	i40e_vsi_free_q_vectors(vsi);
-err_msix:
 	if (vsi->netdev_registered) {
 		vsi->netdev_registered = false;
 		unregister_netdev(vsi->netdev);
-- 
2.47.1

[PATCH net 6/8] i40e: do not expose netdev too early

[Tony Nguyen]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

i40e_vsi_setup() registers the netdev before rings and q_vectors are fully
allocated and mapped. Once register_netdev() returns, userspace can reach
netdev callbacks such as ndo_open(), so the VSI backing state must already
be ready.

Move register_netdev() to the end of the setup path, after ring/q_vector
allocation, ring mapping and RSS configuration. Keep freeing an allocated
but not registered netdev on the error path.

Fixes: 41c445ff0f48 ("i40e: main driver core")
Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 29 ++++++++++++---------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 2b4b061302db..82037faf960f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14449,15 +14449,6 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 				goto err_netdev;
 			SET_NETDEV_DEVLINK_PORT(vsi->netdev, &pf->devlink_port);
 		}
-		ret = register_netdev(vsi->netdev);
-		if (ret)
-			goto err_dl_port;
-		vsi->netdev_registered = true;
-		netif_carrier_off(vsi->netdev);
-#ifdef CONFIG_I40E_DCB
-		/* Setup DCB netlink interface */
-		i40e_dcbnl_setup(vsi);
-#endif /* CONFIG_I40E_DCB */
 		fallthrough;
 	case I40E_VSI_FDIR:
 		/* set up vectors and rings if needed */
@@ -14485,6 +14476,19 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 		if (ret)
 			goto err_config;
 	}
+
+	if (vsi->netdev) {
+		ret = register_netdev(vsi->netdev);
+		if (ret)
+			goto err_config;
+		vsi->netdev_registered = true;
+		netif_carrier_off(vsi->netdev);
+#ifdef CONFIG_I40E_DCB
+		/* Setup DCB netlink interface */
+		i40e_dcbnl_setup(vsi);
+#endif /* CONFIG_I40E_DCB */
+	}
+
 	return vsi;
 
 err_config:
@@ -14495,13 +14499,14 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 	if (vsi->netdev_registered) {
 		vsi->netdev_registered = false;
 		unregister_netdev(vsi->netdev);
-		free_netdev(vsi->netdev);
-		vsi->netdev = NULL;
 	}
-err_dl_port:
 	if (vsi->type == I40E_VSI_MAIN)
 		i40e_devlink_destroy_port(pf);
 err_netdev:
+	if (vsi->netdev) {
+		free_netdev(vsi->netdev);
+		vsi->netdev = NULL;
+	}
 	i40e_aq_delete_element(&pf->hw, vsi->seid, NULL);
 err_vsi:
 	i40e_vsi_clear(vsi);
-- 
2.47.1

Re: [PATCH net 6/8] i40e: do not expose netdev too early

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch narrows but does not close the window it claims to
  close for the driver's primary netdev.
- [Low] Dead error-path code left behind by the reordering: the block at
  i40e_main.c:14498-14502 (`err_rings: if (vsi->netdev_registered) {…
- [Low] Two pieces of externally visible netdev state are still installed
  after registration, contradicting the 'publish late' invariant the…

--

i40e: do not expose netdev too early

This moves register_netdev() in i40e_vsi_setup() from the middle of the
netdev setup case to the end of the function, after ring and q_vector
allocation, ring-to-vector mapping and VMDQ2 RSS configuration.  The
error path is reworked so an allocated but unregistered netdev is still
freed.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 2b4b061302db..82037faf960f 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -14449,15 +14449,6 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
>  				goto err_netdev;
>  			SET_NETDEV_DEVLINK_PORT(vsi->netdev, &pf->devlink_port);
>  		}
> -		ret = register_netdev(vsi->netdev);
> -		if (ret)
> -			goto err_dl_port;
> -		vsi->netdev_registered = true;
> -		netif_carrier_off(vsi->netdev);
> -#ifdef CONFIG_I40E_DCB
> -		/* Setup DCB netlink interface */
> -		i40e_dcbnl_setup(vsi);
> -#endif /* CONFIG_I40E_DCB */
>  		fallthrough;
>  	case I40E_VSI_FDIR:
>  		/* set up vectors and rings if needed */
> @@ -14485,6 +14476,19 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
>  		if (ret)
>  			goto err_config;
>  	}
> +
> +	if (vsi->netdev) {
> +		ret = register_netdev(vsi->netdev);
> +		if (ret)
> +			goto err_config;
> +		vsi->netdev_registered = true;

[Severity: High]
For the MAIN VSI, does this really close the window described in the
commit message?  register_netdev() is still called from inside
i40e_vsi_setup(), which runs from
i40e_probe() -> i40e_setup_pf_switch() -> i40e_vsi_setup().

After i40e_vsi_setup() returns for the MAIN VSI, i40e_setup_pf_switch()
still programs PF-wide state the datapath depends on:

	i40e_vlan_stripping_disable(main_vsi);
	i40e_fdir_sb_setup(pf);
	ret = i40e_setup_pf_filter_control(pf);
	...
	if (test_bit(I40E_FLAG_RSS_ENA, pf->flags))
		i40e_pf_config_rss(pf);
	i40e_link_event(pf);
	i40e_ptp_init(pf);

Note that only VMDQ2 gets RSS configured inside i40e_vsi_setup():

	if (test_bit(I40E_HW_CAP_RSS_AQ, pf->hw.caps) &&
	    vsi->type == I40E_VSI_VMDQ2) {
		ret = i40e_vsi_config_rss(vsi);

so the MAIN netdev is published before PFQF_HENA/HKEY/HLUT, PFQF_CTL_0 and
the VLAN strip configuration are written.  Back in i40e_probe(),
__I40E_DOWN is cleared and i40e_setup_misc_vector() arms the ICR0 admin
queue vector only after i40e_setup_pf_switch() has returned.

i40e_open() gates on:

	if (test_bit(__I40E_TESTING, pf->state) ||
	    test_bit(__I40E_BAD_EEPROM, pf->state))
		return -EBUSY;

which does not test __I40E_DOWN or any probe-complete state, and
register_netdev() drops RTNL before returning while probe holds no lock
that blocks rtnetlink.  Can a concurrent "ip link set up" (udev or
NetworkManager reacting to the freshly published ifindex) therefore reach
i40e_vsi_open() -> i40e_up_complete() while the PF's global hardware state
is still being programmed and while i40e_intr() still refuses to service
ICR0 because __I40E_DOWN is set?

There is also the probe failure case.  i40e_setup_pf_switch() returns the
(otherwise non-fatal) i40e_setup_pf_filter_control() error, and a
i40e_setup_misc_vector() failure jumps to err_vsis:

	err_vsis:
		set_bit(__I40E_DOWN, pf->state);
		i40e_ptp_stop(pf);
		i40e_clear_interrupt_scheme(pf);
		kfree(pf->vsi);

with no unregister_netdev() anywhere on that path, and err_switch_setup
then frees the PF itself.  Does this leave an already registered, possibly
already opened netdev in the system whose netdev_priv()->vsi->back points
at freed memory, so a later ndo_open, ndo_get_stats64, ethtool or close
touches freed q_vectors and a freed struct i40e_pf?

Would registering the netdev only once i40e_probe() has completed, or
gating ndo_open on a "PF ready" state, be a more complete fix than moving
registration a few statements later inside i40e_vsi_setup()?

> +		netif_carrier_off(vsi->netdev);
> +#ifdef CONFIG_I40E_DCB
> +		/* Setup DCB netlink interface */
> +		i40e_dcbnl_setup(vsi);
> +#endif /* CONFIG_I40E_DCB */
> +	}

[Severity: Low]
Since the commit declares the "publish late" invariant, should these two
steps move ahead of register_netdev() as well?

register_netdev() emits NETDEV_REGISTER/RTM_NEWLINK and drops RTNL before
returning, while alloc_etherdev_mq() leaves __LINK_STATE_NOCARRIER clear.
Can the initial link notification therefore advertise IFF_LOWER_UP, with a
carrier-down linkwatch event following immediately once
netif_carrier_off() runs?  netif_carrier_off() sets the bit before its
early return:

net/sched/sch_generic.c:netif_carrier_off() {
	if (!test_and_set_bit(__LINK_STATE_NOCARRIER, &dev->state)) {
		if (dev->reg_state == NETREG_UNINITIALIZED)
			return;
	...
}

so it looks intended to be callable before registration.

Similarly, i40e_dcbnl_setup() installs the ops and only then populates the
state:

drivers/net/ethernet/intel/i40e/i40e_dcb_nl.c:i40e_dcbnl_setup() {
	dev->dcbnl_ops = &dcbnl_ops;

	/* Set initial IEEE DCB settings */
	i40e_dcbnl_set_all(vsi);
}

Does a DCB request arriving between register_netdev() and the dcbnl_ops
assignment get -EOPNOTSUPP from dcb_doit() on a DCB-capable port:

net/dcb/dcbnl.c:dcb_doit() {
	if (!netdev->dcbnl_ops)
		return -EOPNOTSUPP;
	...
}

and is there a second window where dcbnl_ops is live but
i40e_dcbnl_set_all() has not run yet?  Assigning dcbnl_ops and clearing
carrier before register_netdev(), leaving only i40e_dcbnl_set_all()
afterwards, would avoid both windows.

> +
>  	return vsi;
>  
>  err_config:
> @@ -14495,13 +14499,14 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
>  	if (vsi->netdev_registered) {
>  		vsi->netdev_registered = false;
>  		unregister_netdev(vsi->netdev);
> -		free_netdev(vsi->netdev);
> -		vsi->netdev = NULL;
>  	}

[Severity: Low]
Is this err_rings block still reachable after the reordering?

i40e_vsi_mem_alloc() sets vsi->netdev_registered = false for every VSI, and
after this patch the only assignment of true sits after every
goto err_config/err_qvec/err_rings/err_netdev in i40e_vsi_setup(), followed
only by netif_carrier_off(), i40e_dcbnl_setup() and "return vsi".  If
register_netdev() itself fails the flag is still false, so the condition
looks always false when the unwind path runs.

Would dropping it be clearer, so a later patch adding a step after
registration does not assume the unwind already deregisters the netdev?

> -err_dl_port:
>  	if (vsi->type == I40E_VSI_MAIN)
>  		i40e_devlink_destroy_port(pf);
>  err_netdev:
> +	if (vsi->netdev) {
> +		free_netdev(vsi->netdev);
> +		vsi->netdev = NULL;
> +	}
>  	i40e_aq_delete_element(&pf->hw, vsi->seid, NULL);
>  err_vsi:
>  	i40e_vsi_clear(vsi);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

[PATCH net 7/8] i40e: keep q_vectors array in sync with channel count changes

[Tony Nguyen]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

For the main VSI, i40e_set_num_rings_in_vsi() always derives
num_q_vectors from pf->num_lan_msix. At the same time, ethtool -L stores
the user requested channel count in vsi->req_queue_pairs and the queue
setup path uses that value for the effective number of queue pairs.

This leaves queue and vector counts out of sync after shrinking channel
count via ethtool -L. The active queue configuration is reduced, but the
VSI still keeps the full PF-sized q_vector topology.

That mismatch breaks reconfiguration flows which rely on vector/NAPI
state matching the effective channel configuration. In particular,
toggling /sys/class/net/<dev>/threaded after reducing the channel count
can hang, and later channel-count changes can fail because VSI reinit
does not rebuild q_vectors to match the new vector count.

Fix this by making the main VSI num_q_vectors follow the effective
requested channel count, capped by the available MSI-X vectors. Update
i40e_vsi_reinit_setup() to rebuild q_vectors during VSI reinit so the
vector topology is refreshed together with the ring arrays when channel
count changes.

Keep alloc_queue_pairs unchanged and based on pf->num_lan_qps so the VSI
retains its full queue capacity. Also do not touch irq_pile when
rebuilding vectors.

Selftest napi_threaded.py was originally used when Jakub reported hang
on /sys/class/net/<dev>/threaded toggle. In order to make it pass on
i40e, use persistent NAPI configuration for q_vector NAPIs so NAPI
identity and threaded settings survive q_vector reallocation across
channel-count changes. This is achieved by using netif_napi_add_config()
when configuring q_vectors.

$ export NETIF=ens259f1np1
$ sudo -E env PATH="$PATH" ./tools/testing/selftests/drivers/net/napi_threaded.py
TAP version 13
1..3
ok 1 napi_threaded.napi_init
ok 2 napi_threaded.change_num_queues
ok 3 napi_threaded.enable_dev_threaded_disable_napi_threaded
Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0

Reported-by: Jakub Kicinski <kuba@kernel.org>
Closes: https://lore.kernel.org/intel-wired-lan/20260316133100.6054a11f@kernel.org/
Fixes: d2a69fefd756 ("i40e: Fix changing previously set num_queue_pairs for PFs")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 69 +++++++++++++--------
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 82037faf960f..32c0cb6caa17 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11400,10 +11400,14 @@ static void i40e_service_timer(struct timer_list *t)
 static int i40e_set_num_rings_in_vsi(struct i40e_vsi *vsi)
 {
 	struct i40e_pf *pf = vsi->back;
+	u16 qps;
 
 	switch (vsi->type) {
 	case I40E_VSI_MAIN:
 		vsi->alloc_queue_pairs = pf->num_lan_qps;
+		qps = vsi->req_queue_pairs ?
+		      min(vsi->req_queue_pairs, pf->num_lan_qps) :
+		      pf->num_lan_qps;
 		if (!vsi->num_tx_desc)
 			vsi->num_tx_desc = ALIGN(I40E_DEFAULT_NUM_DESCRIPTORS,
 						 I40E_REQ_DESCRIPTOR_MULTIPLE);
@@ -11411,7 +11415,7 @@ static int i40e_set_num_rings_in_vsi(struct i40e_vsi *vsi)
 			vsi->num_rx_desc = ALIGN(I40E_DEFAULT_NUM_DESCRIPTORS,
 						 I40E_REQ_DESCRIPTOR_MULTIPLE);
 		if (test_bit(I40E_FLAG_MSIX_ENA, pf->flags))
-			vsi->num_q_vectors = pf->num_lan_msix;
+			vsi->num_q_vectors = clamp(qps, 1, pf->num_lan_msix);
 		else
 			vsi->num_q_vectors = 1;
 
@@ -11463,12 +11467,11 @@ static int i40e_set_num_rings_in_vsi(struct i40e_vsi *vsi)
 /**
  * i40e_vsi_alloc_arrays - Allocate queue and vector pointer arrays for the vsi
  * @vsi: VSI pointer
- * @alloc_qvectors: a bool to specify if q_vectors need to be allocated.
  *
  * On error: returns error code (negative)
  * On success: returns 0
  **/
-static int i40e_vsi_alloc_arrays(struct i40e_vsi *vsi, bool alloc_qvectors)
+static int i40e_vsi_alloc_arrays(struct i40e_vsi *vsi)
 {
 	struct i40e_ring **next_rings;
 	int size;
@@ -11487,19 +11490,20 @@ static int i40e_vsi_alloc_arrays(struct i40e_vsi *vsi, bool alloc_qvectors)
 	}
 	vsi->rx_rings = next_rings;
 
-	if (alloc_qvectors) {
-		/* allocate memory for q_vector pointers */
-		size = sizeof(struct i40e_q_vector *) * vsi->num_q_vectors;
-		vsi->q_vectors = kzalloc(size, GFP_KERNEL);
-		if (!vsi->q_vectors) {
-			ret = -ENOMEM;
-			goto err_vectors;
-		}
+	/* allocate memory for q_vector pointers */
+	size = sizeof(struct i40e_q_vector *) * vsi->num_q_vectors;
+	vsi->q_vectors = kzalloc(size, GFP_KERNEL);
+	if (!vsi->q_vectors) {
+		ret = -ENOMEM;
+		goto err_vectors;
 	}
 	return ret;
 
 err_vectors:
 	kfree(vsi->tx_rings);
+	vsi->tx_rings = NULL;
+	vsi->rx_rings = NULL;
+	vsi->xdp_rings = NULL;
 	return ret;
 }
 
@@ -11572,7 +11576,7 @@ static int i40e_vsi_mem_alloc(struct i40e_pf *pf, enum i40e_vsi_type type)
 	if (ret)
 		goto err_rings;
 
-	ret = i40e_vsi_alloc_arrays(vsi, true);
+	ret = i40e_vsi_alloc_arrays(vsi);
 	if (ret)
 		goto err_rings;
 
@@ -11597,18 +11601,15 @@ static int i40e_vsi_mem_alloc(struct i40e_pf *pf, enum i40e_vsi_type type)
 /**
  * i40e_vsi_free_arrays - Free queue and vector pointer arrays for the VSI
  * @vsi: VSI pointer
- * @free_qvectors: a bool to specify if q_vectors need to be freed.
  *
  * On error: returns error code (negative)
  * On success: returns 0
  **/
-static void i40e_vsi_free_arrays(struct i40e_vsi *vsi, bool free_qvectors)
+static void i40e_vsi_free_arrays(struct i40e_vsi *vsi)
 {
 	/* free the ring and vector containers */
-	if (free_qvectors) {
-		kfree(vsi->q_vectors);
-		vsi->q_vectors = NULL;
-	}
+	kfree(vsi->q_vectors);
+	vsi->q_vectors = NULL;
 	kfree(vsi->tx_rings);
 	vsi->tx_rings = NULL;
 	vsi->rx_rings = NULL;
@@ -11668,7 +11669,7 @@ static int i40e_vsi_clear(struct i40e_vsi *vsi)
 	i40e_put_lump(pf->irq_pile, vsi->base_vector, vsi->idx);
 
 	bitmap_free(vsi->af_xdp_zc_qps);
-	i40e_vsi_free_arrays(vsi, true);
+	i40e_vsi_free_arrays(vsi);
 	i40e_clear_rss_config_user(vsi);
 
 	pf->vsi[vsi->idx] = NULL;
@@ -12042,7 +12043,8 @@ static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx)
 	cpumask_copy(&q_vector->affinity_mask, cpu_possible_mask);
 
 	if (vsi->netdev)
-		netif_napi_add(vsi->netdev, &q_vector->napi, i40e_napi_poll);
+		netif_napi_add_config(vsi->netdev, &q_vector->napi,
+				      i40e_napi_poll, v_idx);
 
 	/* tie q_vector and vsi together */
 	vsi->q_vectors[v_idx] = q_vector;
@@ -14197,8 +14199,9 @@ int i40e_vsi_release(struct i40e_vsi *vsi)
  **/
 static int i40e_vsi_setup_vectors(struct i40e_vsi *vsi)
 {
-	int ret = -ENOENT;
 	struct i40e_pf *pf = vsi->back;
+	bool reuse_irq_lump = false;
+	int ret = -ENOENT;
 
 	if (vsi->q_vectors[0]) {
 		dev_info(&pf->pdev->dev, "VSI %d has existing q_vectors\n",
@@ -14206,7 +14209,10 @@ static int i40e_vsi_setup_vectors(struct i40e_vsi *vsi)
 		return -EEXIST;
 	}
 
-	if (vsi->base_vector) {
+	if (vsi->type == I40E_VSI_MAIN && vsi->base_vector)
+		reuse_irq_lump = true;
+
+	if (vsi->base_vector && !reuse_irq_lump) {
 		dev_info(&pf->pdev->dev, "VSI %d has non-zero base vector %d\n",
 			 vsi->seid, vsi->base_vector);
 		return -EEXIST;
@@ -14226,6 +14232,10 @@ static int i40e_vsi_setup_vectors(struct i40e_vsi *vsi)
 	*/
 	if (!test_bit(I40E_FLAG_MSIX_ENA, pf->flags))
 		return ret;
+
+	if (reuse_irq_lump)
+		return ret;
+
 	if (vsi->num_q_vectors)
 		vsi->base_vector = i40e_get_lump(pf, pf->irq_pile,
 						 vsi->num_q_vectors, vsi->idx);
@@ -14266,11 +14276,20 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi,
 	pf = vsi->back;
 
 	i40e_put_lump(pf->qp_pile, vsi->base_queue, vsi->idx);
+	i40e_vsi_free_q_vectors(vsi);
 	i40e_vsi_clear_rings(vsi);
+	i40e_vsi_free_arrays(vsi);
 
-	i40e_vsi_free_arrays(vsi, false);
 	i40e_set_num_rings_in_vsi(vsi);
-	ret = i40e_vsi_alloc_arrays(vsi, false);
+	ret = i40e_vsi_alloc_arrays(vsi);
+	if (ret)
+		goto err_netdev;
+
+	/* Rebuild q_vectors during VSI reinit because the effective channel
+	 * count may change num_q_vectors. Keep vector topology aligned with the
+	 * queue configuration after ethtool's .set_channels() callback.
+	 */
+	ret = i40e_vsi_setup_vectors(vsi);
 	if (ret)
 		goto err_netdev;
 
@@ -14282,7 +14301,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi,
 		dev_info(&pf->pdev->dev,
 			 "failed to get tracking for %d queues for VSI %d err %d\n",
 			 alloc_queue_pairs, vsi->seid, ret);
-		goto err_netdev;
+		goto err_rings;
 	}
 	vsi->base_queue = ret;
 
-- 
2.47.1

Re: [PATCH net 7/8] i40e: keep q_vectors array in sync with channel count changes

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 3 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Incomplete fix: for the main VSI, i40e_set_num_rings_in_vsi()
  now sizes vsi->num_q_vectors from vsi->req_queue_pairs (clamp(qps, 1,…
- [Medium] i40e_vsi_reinit_setup() now frees every q_vector
  (i40e_vsi_free_q_vectors() -> i40e_free_q_vector(): netif_napi_del() +…
- [Medium] i40e_vsi_reinit_setup() now calls i40e_vsi_free_q_vectors()
  (netif_napi_del() on a possibly still-enabled NAPI +…
- [Medium] i40e_vsi_alloc_q_vector() now calls
  netif_napi_add_config(vsi->netdev, &q_vector->napi, i40e_napi_poll,…

Pre-existing issues:
- [High] i40e_vsi_reinit_setup() destroys the main VSI on every failure
  path (err_rings/err_netdev: unregister_netdevice()/unregister_netdev()…
- [Low] The kernel-doc block that this patch edits for
  i40e_vsi_free_arrays() (the @free_qvectors line was removed) still…

--

i40e: keep q_vectors array in sync with channel count changes

The main VSI num_q_vectors is now derived from the requested channel count
instead of pf->num_lan_msix, i40e_vsi_reinit_setup() rebuilds the q_vectors
together with the ring arrays, and q_vector NAPIs are registered with
netif_napi_add_config() so the persistent NAPI config survives q_vector
reallocation.

Closes: https://lore.kernel.org/intel-wired-lan/20260316133100.6054a11f@kernel.org/

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 82037faf960f..32c0cb6caa17 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -11400,10 +11400,14 @@ static void i40e_service_timer(struct timer_list *t)
>  static int i40e_set_num_rings_in_vsi(struct i40e_vsi *vsi)
>  {
>  	struct i40e_pf *pf = vsi->back;
> +	u16 qps;
>  
>  	switch (vsi->type) {
>  	case I40E_VSI_MAIN:
>  		vsi->alloc_queue_pairs = pf->num_lan_qps;
> +		qps = vsi->req_queue_pairs ?
> +		      min(vsi->req_queue_pairs, pf->num_lan_qps) :
> +		      pf->num_lan_qps;
>  		if (!vsi->num_tx_desc)
>  			vsi->num_tx_desc = ALIGN(I40E_DEFAULT_NUM_DESCRIPTORS,
>  						 I40E_REQ_DESCRIPTOR_MULTIPLE);
> @@ -11411,7 +11415,7 @@ static int i40e_set_num_rings_in_vsi(struct i40e_vsi *vsi)
>  			vsi->num_rx_desc = ALIGN(I40E_DEFAULT_NUM_DESCRIPTORS,
>  						 I40E_REQ_DESCRIPTOR_MULTIPLE);
>  		if (test_bit(I40E_FLAG_MSIX_ENA, pf->flags))
> -			vsi->num_q_vectors = pf->num_lan_msix;
> +			vsi->num_q_vectors = clamp(qps, 1, pf->num_lan_msix);

[Severity: High]
Can num_q_vectors still end up larger than the effective queue count when
DCB is enabled?

req_queue_pairs is the value the user asked for, but the effective layout is
recomputed later in i40e_vsi_setup_queue_map():

	num_tc_qps = num_tc_qps / numtc;
	num_tc_qps = min_t(int, num_tc_qps, i40e_pf_get_max_q_per_tc(pf));
	...
	if ((vsi->type == I40E_VSI_MAIN && numtc != 1) || ...)
		vsi->num_queue_pairs = offset;

With two enabled traffic classes and "ethtool -L <dev> combined 9", that
gives num_tc_qps = 9 / 2 = 4 and offset = 8, so num_queue_pairs becomes 8
while num_q_vectors is 9.

i40e_vsi_map_rings_to_vectors() then distributes only num_queue_pairs ring
pairs over num_q_vectors vectors, so the surplus vector is left with
rx.ring == tx.ring == NULL, and i40e_napi_enable_all() skips it:

	if (q_vector->rx.ring || q_vector->tx.ring)
		napi_enable(&q_vector->napi);

A NAPI registered by netif_napi_add*() has NAPI_STATE_SCHED set, and with
dev->threaded set a kthread is created for it regardless of the rings.
Disabling threaded afterwards reaches napi_stop_kthread():

	if ((val & NAPIF_STATE_SCHED_THREADED) ||
	    !(val & NAPIF_STATE_SCHED)) {
		...
	} else {
		msleep(20);
		continue;
	}

For that never-enabled vector SCHED is set and SCHED_THREADED is never set,
so this loop has no exit condition. Is this the same threaded-toggle hang
the commit message says is fixed?

i40e_set_channels() only rejects mqprio and caps the request by
i40e_max_channels() == vsi->alloc_queue_pairs (whose comment still says
"TODO: This code assumes DCB and FD is disabled for now"), so nothing
rejects plain DCB or requires the count to be a multiple of the TC count.
Would deriving num_q_vectors from the finalized TC queue layout rather than
req_queue_pairs avoid this?

[ ... ]

> @@ -11487,19 +11490,20 @@ static int i40e_vsi_alloc_arrays(struct i40e_vsi *vsi, bool alloc_qvectors)
>  	}
>  	vsi->rx_rings = next_rings;
>  
> -	if (alloc_qvectors) {
> -		/* allocate memory for q_vector pointers */
> -		size = sizeof(struct i40e_q_vector *) * vsi->num_q_vectors;
> -		vsi->q_vectors = kzalloc(size, GFP_KERNEL);
> -		if (!vsi->q_vectors) {
> -			ret = -ENOMEM;
> -			goto err_vectors;
> -		}
> +	/* allocate memory for q_vector pointers */
> +	size = sizeof(struct i40e_q_vector *) * vsi->num_q_vectors;
> +	vsi->q_vectors = kzalloc(size, GFP_KERNEL);
> +	if (!vsi->q_vectors) {
> +		ret = -ENOMEM;
> +		goto err_vectors;
>  	}
>  	return ret;
>  
>  err_vectors:
>  	kfree(vsi->tx_rings);
> +	vsi->tx_rings = NULL;
> +	vsi->rx_rings = NULL;
> +	vsi->xdp_rings = NULL;
>  	return ret;
>  }

[ ... ]

> @@ -11597,18 +11601,15 @@ static int i40e_vsi_mem_alloc(struct i40e_pf *pf, enum i40e_vsi_type type)
>  /**
>   * i40e_vsi_free_arrays - Free queue and vector pointer arrays for the VSI
>   * @vsi: VSI pointer
> - * @free_qvectors: a bool to specify if q_vectors need to be freed.
>   *
>   * On error: returns error code (negative)
>   * On success: returns 0
>   **/
> -static void i40e_vsi_free_arrays(struct i40e_vsi *vsi, bool free_qvectors)
> +static void i40e_vsi_free_arrays(struct i40e_vsi *vsi)

[Severity: Low]
This isn't a bug introduced by this patch, but since the kernel-doc block is
already being edited here: i40e_vsi_free_arrays() returns void, so the
"On error: returns error code (negative)" and "On success: returns 0" lines
cannot apply. Would it make sense to drop them in the same hunk?

[ ... ]

> @@ -12042,7 +12043,8 @@ static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx)
>  	cpumask_copy(&q_vector->affinity_mask, cpu_possible_mask);
>  
>  	if (vsi->netdev)
> -		netif_napi_add(vsi->netdev, &q_vector->napi, i40e_napi_poll);
> +		netif_napi_add_config(vsi->netdev, &q_vector->napi,
> +				      i40e_napi_poll, v_idx);

[Severity: Medium]
Is v_idx guaranteed to be within the netdev's napi_config array?

netif_napi_add_config_locked() stores the pointer without any bounds check:

	napi->index = index;
	napi->config = &dev->napi_config[index];

and dev->napi_config[] is sized once in alloc_netdev_mqs() from
max(num_tx_queues, num_rx_queues) and never resized. For i40e that is
vsi->alloc_queue_pairs at alloc_etherdev_mq() time in i40e_config_netdev(),
i.e. pf->num_lan_qps as it was when the netdev was created.

v_idx is bounded only by vsi->num_q_vectors, which for the main VSI is now
clamp(qps, 1, pf->num_lan_msix) recomputed on every reinit from the current
pf->num_lan_qps / pf->num_lan_msix. pf->num_lan_qps is mutated after the
netdev exists, in the limited-resources branch of i40e_init_msix():

	pf->num_lan_msix = min_t(int,
	       (vec - (pf->num_iwarp_msix + pf->num_vmdq_vsis)),
					      pf->num_lan_msix);
	pf->num_lan_qps = pf->num_lan_msix;

and i40e_init_msix() is re-run from i40e_restore_interrupt_scheme() on
resume and on recovery-mode exit. On a part where
hw.func_caps.num_tx_qp < num_online_cpus(), the probe-time num_lan_qps (and
therefore num_napi_configs) is clamped to num_tx_qp while num_lan_msix is
clamped only by the CPU count, so that assignment can raise num_lan_qps
above the netdev's fixed napi_config count. The next reinit, which this
patch makes rebuild q_vectors, would then index past the end of the
kvzalloc()'d napi_config array.

Would bounding v_idx by the netdev's queue count before calling
netif_napi_add_config() be appropriate here? Note the existing
netif_set_real_num_tx_queues() failure at open only fires after the access.

[ ... ]

> @@ -14266,11 +14276,20 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi,
>  	pf = vsi->back;
>  
>  	i40e_put_lump(pf->qp_pile, vsi->base_queue, vsi->idx);
> +	i40e_vsi_free_q_vectors(vsi);
>  	i40e_vsi_clear_rings(vsi);
> +	i40e_vsi_free_arrays(vsi);

[Severity: Medium]
Should vsi->num_q_vectors be zeroed here?

i40e_vsi_free_q_vectors() frees each q_vector and i40e_vsi_free_arrays()
then does kfree(vsi->q_vectors); vsi->q_vectors = NULL, but num_q_vectors
still describes vectors that no longer exist for the whole reinit window.

The debugfs napi command iterates with no NULL check on the array and does
not hold rtnl:

drivers/net/ethernet/intel/i40e/i40e_debugfs.c:i40e_dbg_netdev_ops_write() {
	...
	for (i = 0; i < vsi->num_q_vectors; i++)
		napi_schedule(&vsi->q_vectors[i]->napi);
	...
}

so "echo 'napi <seid>' > .../command" concurrent with the reinit window
would dereference vsi->q_vectors[i] with vsi->q_vectors == NULL.

The same shape exists in i40e_napi_disable_all():

	for (q_idx = 0; q_idx < vsi->num_q_vectors; q_idx++) {
		struct i40e_q_vector *q_vector = vsi->q_vectors[q_idx];

		if (q_vector->rx.ring || q_vector->tx.ring)

which is reachable from the err_netdev unregister path below whenever
__I40E_VSI_DOWN is not already set. Before this patch the q_vector objects
and the pointer array deliberately survived reinit
(i40e_vsi_free_arrays(vsi, false) / i40e_vsi_alloc_arrays(vsi, false)), so
this window did not exist.

[Severity: Medium]
Is the VSI always quiesced when reinit reaches this point?

i40e_vsi_free_q_vectors() -> i40e_free_q_vector() does:

	if (vsi->netdev)
		netif_napi_del(&q_vector->napi);

	vsi->q_vectors[v_idx] = NULL;

	kfree_rcu(q_vector, rcu);

while the q_vector is still the dev_id that request_irq() was given, and the
NAPI may still be enabled. The only thing that normally releases the IRQs
and disables the NAPIs first is i40e_prep_for_reset() ->
i40e_pf_quiesce_all_vsi(), and i40e_prep_for_reset() quiesces nothing when
the reset bit is already owned by someone else:

	if (test_and_set_bit(__I40E_RESET_RECOVERY_PENDING, pf->state))
		return;
	...
	i40e_pf_quiesce_all_vsi(pf);

The service task performs resets without rtnl (i40e_service_task() ->
i40e_reset_subtask() -> i40e_prep_for_reset() / i40e_reset() /
i40e_rebuild(pf, false, false)), so an rtnl-holding "ethtool -L" can find
the bit already set and walk straight into reinit with vsi->irqs_ready
still true.

In that case i40e_vsi_free_irq() later calls
free_irq(irq_num, vsi->q_vectors[i]) with a dev_id that request_irq() never
saw (or skips the vector entirely because the fresh q_vector has
num_ringpairs == 0), leaving the old handler registered against freed
memory:

	if (!vsi->irqs_ready)
		return;
	vsi->irqs_ready = false;
	for (i = 0; i < vsi->num_q_vectors; i++) {
	...
		free_irq(irq_num, vsi->q_vectors[i]);

Would it be safer for reinit to force the quiesced state (free the IRQs,
disable the NAPIs, clear irqs_ready) before rebuilding vectors?

>  
> -	i40e_vsi_free_arrays(vsi, false);
>  	i40e_set_num_rings_in_vsi(vsi);
> -	ret = i40e_vsi_alloc_arrays(vsi, false);
> +	ret = i40e_vsi_alloc_arrays(vsi);
> +	if (ret)
> +		goto err_netdev;
> +
> +	/* Rebuild q_vectors during VSI reinit because the effective channel
> +	 * count may change num_q_vectors. Keep vector topology aligned with the
> +	 * queue configuration after ethtool's .set_channels() callback.
> +	 */
> +	ret = i40e_vsi_setup_vectors(vsi);
>  	if (ret)
>  		goto err_netdev;

[Severity: High]
This isn't a bug introduced by this patch, but the two new failure points
added here land on an error path that destroys the main VSI while the caller
keeps using it.

err_netdev unregisters and free_netdev()s the netdev, deletes the switch
element and then calls i40e_vsi_clear(vsi), which kfree()s the struct
i40e_vsi and clears pf->vsi[idx], returning NULL. i40e_setup_pf_switch()
turns that into -EAGAIN, but i40e_rebuild() and i40e_reset_and_rebuild()
both return void, so the failure never reaches the caller:

i40e_set_channels()
  i40e_reconfig_rss_queues()
    i40e_reset_and_rebuild(pf, true, true)   /* void */
      i40e_rebuild()                         /* void */
        i40e_setup_pf_switch(pf, true)
          i40e_vsi_reinit_setup()            /* frees vsi, returns NULL */

and i40e_reconfig_rss_queues() then continues with its cached pointer:

	i40e_reset_and_rebuild(pf, true, true);
	...
	if (queue_count < vsi->rss_size) {
		i40e_clear_rss_config_user(vsi);
	...
	qcount = vsi->num_queue_pairs / vsi->tc_config.numtc;
	vsi->rss_size = min_t(int, pf->alloc_rss_size, qcount);

i40e_xdp_setup() has the same pattern, dereferencing vsi->num_queue_pairs,
vsi->rx_rings[i] and vsi->xdp_rings[i] after
i40e_reset_and_rebuild(pf, true, true).

Previously reinit performed no allocation of q_vectors, so an -ENOMEM could
not trigger this. With the q_vector pointer-array kzalloc() now
unconditional in i40e_vsi_alloc_arrays() and the per-vector kzalloc() in
i40e_vsi_alloc_q_vector() reached through i40e_vsi_setup_vectors(), a single
allocation failure under memory pressure destroys the main VSI and the
caller reads and writes freed memory. Could reinit keep the VSI alive on
failure, or propagate the failure so callers stop touching the destroyed
object?

>  
> @@ -14282,7 +14301,7 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi,
>  		dev_info(&pf->pdev->dev,
>  			 "failed to get tracking for %d queues for VSI %d err %d\n",
>  			 alloc_queue_pairs, vsi->seid, ret);
> -		goto err_netdev;
> +		goto err_rings;
>  	}
>  	vsi->base_queue = ret;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com

[PATCH net 8/8] i40e: xsk: fix multi-buffer XDP_PASS skb construction

[Tony Nguyen]

From: Chenguang Zhao <zhaochenguang@kylinos.cn>

When AF_XDP ZC receives a multi-buffer frame and the XDP program
returns XDP_PASS, i40e_construct_skb_zc() copies frags into a new
skb. The copy used skb_frag_page() as the memcpy source (page
metadata instead of packet data) and passed a virtual address to
__skb_fill_page_desc_noacc(), which expects a struct page *.

Use skb_frag_address() for the copy, attach frags with
skb_add_rx_frag() so len/data_len/truesize are updated, and on
dev_alloc_page() failure free the skb via the shared out path so
xsk_buff_free() still runs and previously attached pages are
released by kfree_skb.

Fixes: 1c9ba9c14658 ("i40e: xsk: add RX multi-buffer support")
Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Patryk Holda <patryk.holda@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_xsk.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 9f47388eaba5..a4247710c85b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -318,22 +318,19 @@ static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring,
 		goto out;
 
 	for (int i = 0; i < nr_frags; i++) {
-		struct skb_shared_info *skinfo = skb_shinfo(skb);
 		skb_frag_t *frag = &sinfo->frags[i];
+		unsigned int frag_size = skb_frag_size(frag);
 		struct page *page;
-		void *addr;
 
 		page = dev_alloc_page();
 		if (!page) {
 			dev_kfree_skb(skb);
-			return NULL;
+			skb = NULL;
+			goto out;
 		}
-		addr = page_to_virt(page);
 
-		memcpy(addr, skb_frag_page(frag), skb_frag_size(frag));
-
-		__skb_fill_page_desc_noacc(skinfo, skinfo->nr_frags++,
-					   addr, 0, skb_frag_size(frag));
+		memcpy(page_to_virt(page), skb_frag_address(frag), frag_size);
+		skb_add_rx_frag(skb, i, page, 0, frag_size, PAGE_SIZE);
 	}
 
 out:
-- 
2.47.1

Re: [PATCH net 0/8][pull request] Intel Wired LAN Driver Updates 2026-09-18 (i40e)

[Paolo Abeni]

On 9/18/26 23:24, Tony Nguyen wrote:
> Maciej fixes various issues with VSI setup, cleanup, and rebuild paths.
> Notably, keeping q_vector allocation synchronized with channel-count
> changes to prevent threaded NAPI reconfiguration failures.
> 
> Chenguang Zhao fixes AF_XDP multi-buffer handling by correctly copying
> and attaching fragments and cleaning up on allocation failure.
> 
> The following are changes since commit 46bc52d13594848023e681860df8700c8db14354:
>    ipv4: fib: fix data-race and stale genid check around nh->nh_saddr
> and are available in the git repository at:
>    git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue 40GbE
It looks like clashiko has found a few serious regressions, worth a respin.

/P

Re: [PATCH net 0/8][pull request] Intel Wired LAN Driver Updates 2026-09-18 (i40e)

[Paolo Abeni]

On 9/24/26 13:23, Paolo Abeni wrote:
> On 9/18/26 23:24, Tony Nguyen wrote:
>> Maciej fixes various issues with VSI setup, cleanup, and rebuild paths.
>> Notably, keeping q_vector allocation synchronized with channel-count
>> changes to prevent threaded NAPI reconfiguration failures.
>>
>> Chenguang Zhao fixes AF_XDP multi-buffer handling by correctly copying
>> and attaching fragments and cleaning up on allocation failure.
>>
>> The following are changes since commit 46bc52d13594848023e681860df8700c8db14354:
>>    ipv4: fib: fix data-race and stale genid check around nh->nh_saddr
>> and are available in the git repository at:
>>    git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue 40GbE
> It looks like clashiko has found a few serious regressions, worth a respin.
I almost forgot: please note that the current expectation is for you to
address the clashiko comments on the ML.

/P

Re: [PATCH net 0/8][pull request] Intel Wired LAN Driver Updates 2026-09-18 (i40e)

[Maciej Fijalkowski]

On Thu, Sep 24, 2026 at 01:25:14PM +0200, Paolo Abeni wrote:
> On 9/24/26 13:23, Paolo Abeni wrote:
> > On 9/18/26 23:24, Tony Nguyen wrote:
> > > Maciej fixes various issues with VSI setup, cleanup, and rebuild paths.
> > > Notably, keeping q_vector allocation synchronized with channel-count
> > > changes to prevent threaded NAPI reconfiguration failures.
> > > 
> > > Chenguang Zhao fixes AF_XDP multi-buffer handling by correctly copying
> > > and attaching fragments and cleaning up on allocation failure.
> > > 
> > > The following are changes since commit 46bc52d13594848023e681860df8700c8db14354:
> > >    ipv4: fib: fix data-race and stale genid check around nh->nh_saddr
> > > and are available in the git repository at:
> > >    git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue 40GbE
> > It looks like clashiko has found a few serious regressions, worth a respin.
> I almost forgot: please note that the current expectation is for you to
> address the clashiko comments on the ML.

Okay, there is a bunch of stuff that are reported at early stage of change
set which are later addressed - I thought bots are taking this into
consideration and therefore would skip it ?

Has the gemini LLM fell behind the others given that it reported no issues
whereas gpt/claude produced way too much of output ;)

From trying to fix one thing I'm about to be in a place where I have
refactored most of the rebuild path, which have been used without much of
a problem for (at least few) years.

It is what it is, but I have a mixed feelings for where we're at, not
mentioning where we're heading.

Maciej

> 
> /P
> 

Re: [PATCH net 0/8][pull request] Intel Wired LAN Driver Updates 2026-09-18 (i40e)

[Jakub Kicinski]

On Fri, 25 Sep 2026 14:46:44 +0200 Maciej Fijalkowski wrote:
> On Thu, Sep 24, 2026 at 01:25:14PM +0200, Paolo Abeni wrote:
> > On 9/24/26 13:23, Paolo Abeni wrote:  
> > > It looks like clashiko has found a few serious regressions, worth a respin.  
> > I almost forgot: please note that the current expectation is for you to
> > address the clashiko comments on the ML.  
> 
> Okay, there is a bunch of stuff that are reported at early stage of change
> set which are later addressed - I thought bots are taking this into
> consideration and therefore would skip it ?

We changed that because people order their code poorly, break stuff 
and then fix it back later in the series. But, indeed, it should not
report pre-existing regressions if they are fixed later.

> Has the gemini LLM fell behind the others given that it reported no issues
> whereas gpt/claude produced way too much of output ;)

We probably have a better models but we fell behind Sashiko in terms
of the pipeline itself. There was a major redesign upstream, needs some
time to rebase across it.

> From trying to fix one thing I'm about to be in a place where I have
> refactored most of the rebuild path, which have been used without much of
> a problem for (at least few) years.
> 
> It is what it is, but I have a mixed feelings for where we're at, not
> mentioning where we're heading.

Well, maybe let's go the other way? Given Linus's recent complaint
can you trim this series down to just patches which address real
issues you are able to trigger? The bigger rework can target net-next
as needed.

Re: [PATCH net 0/8][pull request] Intel Wired LAN Driver Updates 2026-09-18 (i40e)

[Maciej Fijalkowski]

On Fri, Sep 25, 2026 at 01:00:42PM -0700, Jakub Kicinski wrote:
> On Fri, 25 Sep 2026 14:46:44 +0200 Maciej Fijalkowski wrote:
> > On Thu, Sep 24, 2026 at 01:25:14PM +0200, Paolo Abeni wrote:
> > > On 9/24/26 13:23, Paolo Abeni wrote:  
> > > > It looks like clashiko has found a few serious regressions, worth a respin.  
> > > I almost forgot: please note that the current expectation is for you to
> > > address the clashiko comments on the ML.  
> > 
> > Okay, there is a bunch of stuff that are reported at early stage of change
> > set which are later addressed - I thought bots are taking this into
> > consideration and therefore would skip it ?
> 
> We changed that because people order their code poorly, break stuff 
> and then fix it back later in the series. But, indeed, it should not
> report pre-existing regressions if they are fixed later.
> 
> > Has the gemini LLM fell behind the others given that it reported no issues
> > whereas gpt/claude produced way too much of output ;)
> 
> We probably have a better models but we fell behind Sashiko in terms
> of the pipeline itself. There was a major redesign upstream, needs some
> time to rebase across it.
> 
> > From trying to fix one thing I'm about to be in a place where I have
> > refactored most of the rebuild path, which have been used without much of
> > a problem for (at least few) years.
> > 
> > It is what it is, but I have a mixed feelings for where we're at, not
> > mentioning where we're heading.
> 
> Well, maybe let's go the other way? Given Linus's recent complaint
> can you trim this series down to just patches which address real
> issues you are able to trigger? The bigger rework can target net-next
> as needed.

That would be a start from beginning. First 3 revisions were a single
patch that fixes real issue which actually you reported via
napi_threaded.py hang. In order to fix it correctly q_vector management
was changed and it touched rebuild path. Gemini-based Sashiko reported
issues and next two revisions became a patchset.

My grumblings came from a suprisingly huge mismatch between Clashiko and
Sashiko. I'm gonna try to reproduce locally with new fancy models the
reports Clashiko had and then do some back and forth AI review rounds
before sending next rev.

Hope this time we're gonna be a bit more robust with validating these
changes within Intel ;)