On Thu, Jun 25, 2026 at 05:14:29PM +0200, Maciej Fijalkowski wrote:
> 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 <[email protected]>
> ---
> 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 a04683004a56..471fa7f7b643 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -14274,7 +14274,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);
> @@ -14284,7 +14284,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;
>
> @@ -14309,6 +14309,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);
Sashiko says:
---
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.
---
which is another valid concern. I'll take a stab at addressing this, but
looking at a bigger picture, we don't propagate errors from rebuild path,
so I wouldn't be surprised that in the next iteration Sashiko would point
it out. I'd say that would be a too big refactor for this series.
> @@ -14318,7 +14319,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.43.0
>
