Re: [PATCH net 1/3] i40e: keep q_vectors array in sync with channel count changes
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-06-04 01:28:49
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 hunk ↗ jump to hunk
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 hunk ↗ jump to hunk
@@ -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 hunk ↗ jump to hunk
@@ -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.