RE: [PATCH iwl-net v3 3/3] ice: free the VF MSI-X vectors when VF start fails

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Linkui Xiao <xiaolinkui@xxxxxxx>
> Sent: Thursday, October 8, 2026 2:58 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; andrew+netdev@xxxxxxx;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx
> Cc: intel-wired-lan@xxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Linkui Xiao <xiaolinkui@xxxxxxxxxx>;
> stable@xxxxxxxxxxxxxxx
> Subject: [PATCH iwl-net v3 3/3] ice: free the VF MSI-X vectors when VF
> start fails
> 
> From: Linkui Xiao <xiaolinkui@xxxxxxxxxx>
> 
> ice_init_vf_vsi_res() reserves vf->num_msix vectors out of
> pf->virt_irq_tracker with ice_virt_get_irqs() as its very first step,
> but neither of the two error paths below it gives them back. A NULL from
> ice_vf_vsi_setup() returns -ENOMEM straight away, and the release_vsi
> label only releases the VSI.
> 
> ice_start_vfs() leaks the same vectors. Its teardown loop undoes the
> queue mappings and the VF VSI of the VFs it already started, and the
> eswitch attach failure path releases the VSI of the VF it is working on,
> but neither calls ice_virt_free_irqs(). The caller then runs
> ice_free_vf_entries(), which drops the last reference on every VF, so
> nothing further down the error path can release the reservation either.
> 
> The tracker bitmap is only freed in ice_deinit_virt_irq_tracker(), so
> the leaked vectors stay reserved for the whole lifetime of the driver
> instance. Every failed "echo N > sriov_numvfs" permanently shrinks the
> pool that ice_set_per_vf_res() divides up, and after enough retries
> ice_virt_get_irqs() fails with -ENOENT for good even though the hardware
> vectors are idle. ice_dis_vf_mappings() meanwhile re-points
> GLINT_VECT2FUNC of exactly those vectors back at the PF while the bitmap
> still books them to the VF.
> 
> Release the vectors on all three paths, the way ice_free_vfs() does for
> a VF that is torn down normally.
> 
> Found by code inspection of the VF setup and teardown error paths. It
> was not triggered and no stack trace or error message was observed.
> Compile-tested only, not run on hardware.
> 
> Fixes: 4d38cb44bd32 ("ice: manage VFs MSI-X using resource tracking")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Linkui Xiao <xiaolinkui@xxxxxxxxxx>
> ---
> Changes in v3:
> - Correct the Fixes: tag. The leak starts at 4d38cb44bd32, which
> replaced the
>   computed first vector index with a reservation from the bitmap and
> left the
>   error paths below it unchanged; a203163274a4 only renamed that helper
> and
>   moved the tracker, so it is not the commit that introduced the leak.
> - Include how the issue was found, that it has not been triggered, and
> that the
>   change is compile tested only, as netdev-bot asked for.
> - Free the IRQs before the VSI is released on the eswitch attach failure
>   path, as Tomasz asked for, and keep the teardown loop in that same
> order.
>   The teardown loop hunk now sits inside vf->cfg_lock instead of next to
>   the detach, so the Reviewed-by tags from Tomasz Lichwala and Aleksandr
>   Loktionov are not carried over.
>  drivers/net/ethernet/intel/ice/ice_sriov.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 470aec8849b6..444d05a9bcb0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -458,8 +458,10 @@ static int ice_init_vf_vsi_res(struct ice_vf *vf)
>  		return -ENOMEM;
> 
>  	vsi = ice_vf_vsi_setup(vf);
> -	if (!vsi)
> -		return -ENOMEM;
> +	if (!vsi) {
> +		err = -ENOMEM;
> +		goto free_irqs;
> +	}
> 
>  	err = ice_vf_init_host_cfg(vf, vsi);
>  	if (err)
> @@ -469,6 +471,8 @@ static int ice_init_vf_vsi_res(struct ice_vf *vf)
> 
>  release_vsi:
>  	ice_vf_vsi_release(vf);
> +free_irqs:
> +	ice_virt_free_irqs(pf, vf->first_vector_idx, vf->num_msix);
>  	return err;
>  }
> 
> @@ -501,6 +505,8 @@ static int ice_start_vfs(struct ice_pf *pf)
>  			if (retval) {
>  				dev_err(ice_pf_to_dev(pf), "Failed to attach
> VF %d to eswitch, error %d",
>  					vf->vf_id, retval);
> +				ice_virt_free_irqs(pf, vf->first_vector_idx,
> +						   vf->num_msix);
>  				ice_vf_vsi_release(vf);
>  				goto teardown;
>  			}
> @@ -537,6 +543,7 @@ static int ice_start_vfs(struct ice_pf *pf)
>  		ice_eswitch_detach_vf(pf, vf);
> 
>  		mutex_lock(&vf->cfg_lock);
> +		ice_virt_free_irqs(pf, vf->first_vector_idx, vf-
> >num_msix);
>  		ice_dis_vf_mappings(vf);
>  		ice_vf_vsi_release(vf);
>  		mutex_unlock(&vf->cfg_lock);
> --
> 2.25.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@xxxxxxxxx>