From: Linkui Xiao 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@vger.kernel.org Signed-off-by: Linkui Xiao --- 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