On 6/3/2026 6:27 PM, Jakub Kicinski wrote:

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
i40e: keep q_vectors array in sync with channel count changes

This patch makes the main VSI's num_q_vectors track the effective
requested channel count and rebuilds q_vectors during VSI reinit so the
vector topology is refreshed together with the ring arrays. It also
switches to netif_napi_add_config() so NAPI identity survives q_vector
reallocation across channel-count changes.

Link: https://lore.kernel.org/intel-wired-lan/20260316133100.6054a11f@kernel.org/ (local)

quoted

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 6d4f9218dc68..c3fbe14ff2d3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c

[ ... ]

quoted

@@ -14264,12 +14264,26 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
  
  	pf = vsi->back;
  
+	if (test_bit(I40E_FLAG_MSIX_ENA, pf->flags)) {
+		i40e_put_lump(pf->irq_pile, vsi->base_vector, vsi->idx);
+		vsi->base_vector = 0;
+	}
+
  	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_vsi;
+
+	/* 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_vsi;

[High]
Can the err_vsi path here cause a use-after-free of vsi via netdev_priv?

Reinit is invoked from i40e_rebuild() -> i40e_setup_pf_switch(pf, true,
...) on an already-registered netdev (vsi->netdev_registered == true),
and struct i40e_netdev_priv keeps a pointer at the vsi. The err_vsi
label only calls i40e_vsi_clear() which kfrees the vsi without
unregistering vsi->netdev, destroying the devlink port, or deleting
the FW VSI element:

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

     bitmap_free(vsi->af_xdp_zc_qps);
     i40e_vsi_free_arrays(vsi);
     i40e_clear_rss_config_user(vsi);
     ...
     kfree(vsi);
     ...
}

If i40e_vsi_alloc_arrays() or the newly added i40e_vsi_setup_vectors()
fails (e.g. kzalloc of q_vectors, kzalloc of an individual q_vector,
or i40e_get_lump(irq_pile)), would a subsequent ndo_* callback on the
still-registered netdev dereference the freed vsi through
netdev_priv()?

Two new failure points the patch routes into err_vsi were not reachable
before: the unconditional q_vectors kzalloc in i40e_vsi_alloc_arrays()
and the entire i40e_vsi_setup_vectors() call.

quoted

@@ -14281,7 +14295,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_lump;
  	}
  	vsi->base_queue = ret;
  

@@ -14305,7 +14319,6 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
  	return vsi;
  
  err_rings:
-	i40e_vsi_free_q_vectors(vsi);
  	if (vsi->netdev_registered) {
  		vsi->netdev_registered = false;
  		unregister_netdev(vsi->netdev);

@@ -14315,6 +14328,8 @@ 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_lump:
+	i40e_vsi_free_q_vectors(vsi);
  err_vsi:
  	i40e_vsi_clear(vsi);
  	return NULL;

Should the new err_lump label, and the existing err_vsi exits from the
two allocation steps above, instead unwind through the err_rings block
(unregister_netdev / free_netdev / i40e_devlink_destroy_port /
i40e_aq_delete_element) the way i40e_vsi_setup()'s err_msix path does?

The pre-patch code had the same defective err_vsi target for the
qp_pile and arrays paths, but the patch adds two new failure points
(the unconditional q_vectors kzalloc and the new
i40e_vsi_setup_vectors() call) that route into it during reset
rebuild, where vsi->netdev is already registered.

Hi Jakub,

This does seem valid, but as mentioned by Sashiko the pre-patch code has 
the same target/issue. There's a recent submission [1], with changes 
requested, that should cover this. Did you want to take this now or wait 
and have it sent with this other one?

Thanks,
Tony

[1] 
https://lore.kernel.org/intel-wired-lan/eb74ae1c-3027-42f5-ad5b-a6f2c2cd6a98@onemain.com/ (local)