[PATCH iwl-net v2 1/2] ice: detach the VF representor when ice_start_vfs() fails

[PATCH iwl-net v2 1/2] ice: detach the VF representor when ice_start_vfs() fails

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

ice_start_vfs() attaches every VF it brings up to the eswitch with
ice_eswitch_attach_vf(), but the teardown path only undoes the queue
mappings and the VF VSI. Nothing calls ice_eswitch_detach_vf() for the
VFs that were attached before the failure, and the caller, ice_ena_vfs(),
goes straight to ice_free_vf_entries(), which drops its reference to
every VF.

The port representors created for those VFs therefore outlive the
failed VF creation:

  - the representor netdev stays registered and its devlink port stays
    registered too. That port is embedded in struct ice_vf, so it ends
    up pointing into the memory that ice_sriov_free_vf() releases;

  - repr->vf keeps pointing at the freed struct ice_vf, and repr->src_vsi
    at the VF VSI that ice_vf_vsi_release() tore down. The leftover
    netdev is still visible to the user, so even a plain
    "ip -s link show" of it reaches ice_repr_get_stats64(), which calls
    repr->ops.ready() -> ice_check_vf_ready_for_cfg(repr->vf) and then
    reads repr->src_vsi through ice_update_eth_stats();

  - the virtchnl ops of that VF, which ice_repr_add_vf() replaced with
    ice_virtchnl_set_repr_ops(), are never handed back to
    ice_virtchnl_set_dflt_ops();

  - pf->eswitch.reprs never becomes empty, so ice_eswitch_detach() never
    calls ice_eswitch_disable_switchdev(). pf->eswitch.is_running stays
    true, with the bridge offloads and the devlink rate topology still
    up, and ice_eswitch_release_env() is skipped, so the uplink VSI is
    left in the switchdev configuration that ice_eswitch_setup_env()
    gave it.

Detach the representor in the teardown loop the way ice_free_vfs() does,
ahead of ice_vf_vsi_release(), because ice_repr_rem_vf() and
ice_eswitch_release_repr() both need repr->src_vsi to still be valid.
Every VF the teardown loop walks completed ice_eswitch_attach_vf()
successfully, and ice_eswitch_detach_vf() already returns early for a VF
without a representor, so no extra condition is needed.

Hold vf->cfg_lock across the teardown of each VF as well, like
ice_free_vfs() does. The VFs unwound here are the ones that already
reached set_bit(ICE_VF_STATE_INIT), which is exactly what
ice_check_vf_ready_for_cfg() checks, so a host administrator can still
run "ip link set dev <pf> vf N ..." and a VF can still send a mailbox
message while the loop walks them. Both paths take cfg_lock and then run
ice_reset_vf(), which gets to ice_eswitch_update_repr() and writes
through the representor that is being freed, or reach
ice_vc_process_vf_msg() reading vf->virtchnl_ops while
ice_virtchnl_set_dflt_ops() hands them back.

Fixes: fff292b47ac1 ("ice: add VF representors one by one")
Cc: stable@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
v1:
- Link: https://lore.kernel.org/netdev/20260921031616.3390259-1-xiaolinkui@126.com/

Changes in v2:
- Hold vf->cfg_lock across the teardown of each VF, the way ice_free_vfs()
  does, so that a concurrent VF reconfiguration cannot walk through the
  representor and the virtchnl ops that are being torn down.
  (Sashiko AI review)
- Patch 2/2 is new and returns the VF MSI-X window that the same failure path
  reserves. That is a separate, pre-existing bug, so it is not folded in here.
  (Sashiko AI review)
- Not carrying over the Reviewed-by from Aleksandr Loktionov, as the code
  changed after his review.

 drivers/net/ethernet/intel/ice/ice_sriov.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index e04de0215596..95abc6704820 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -508,8 +508,13 @@ static int ice_start_vfs(struct ice_pf *pf)
 		if (it_cnt == 0)
 			break;
 
+		mutex_lock(&vf->cfg_lock);
+
+		ice_eswitch_detach_vf(pf, vf);
 		ice_dis_vf_mappings(vf);
 		ice_vf_vsi_release(vf);
+		mutex_unlock(&vf->cfg_lock);
+
 		it_cnt--;
 	}
 
-- 
2.25.1

[PATCH iwl-net v2 2/2] ice: release the VF MSI-X window when ice_start_vfs() fails

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

ice_init_vf_vsi_res() reserves the VF MSI-X window with
ice_virt_get_irqs(), which does bitmap_set() on the PF-wide
pf->virt_irq_tracker.bm, but nothing hands that window back when VF
creation fails. ice_virt_free_irqs() is not called anywhere on this
failure path: not from the release_vsi label of ice_init_vf_vsi_res(),
not from the ice_eswitch_attach_vf() failure branch, and not from the
teardown loop of ice_start_vfs(), which only undoes the queue mappings
and the VF VSI. The caller does not help either, because
err_unroll_vf_entries -> ice_free_vf_entries() -> ice_put_vf() ->
ice_sriov_free_vf() only does mutex_destroy() and kfree_rcu().

The tracker is allocated once per PF in ice_init_virt_irq_tracker() and
freed only in ice_deinit_virt_irq_tracker(), and ice_set_per_vf_res()
sizes the VFs from pf->virt_irq_tracker.num_entries rather than from the
number of free bits. So each failed "echo N > sriov_numvfs" leaks the
window reserved for every VF that got as far as ice_init_vf_vsi_res(),
and once the tracker is exhausted ice_virt_get_irqs() keeps returning
-ENOENT, which means SR-IOV cannot be enabled again without reloading
the driver. The hardware and the software bookkeeping also end up
disagreeing, because ice_dis_vf_mappings() clears
VPINT_ALLOC/VPINT_ALLOC_PCI and re-points GLINT_VECT2FUNC of exactly
those vectors back at the PF.

Return the window on the failure paths, in the order ice_free_vfs()
uses: ice_virt_free_irqs() after ice_eswitch_detach_vf() in the teardown
loop, and right after the VF VSI is released in the two branches that
fail before the loop can reach that VF.

The missing release is older than this change: the teardown loop has
never returned the window. It turns into an accumulating leak because
the tracker is now PF-wide and outlives a single SR-IOV enable.

Fixes: a203163274a4 ("ice: simplify VF MSI-X managing")
Cc: stable@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
Changes in v2:
- New patch. Returns the VF MSI-X window that ice_init_vf_vsi_res() reserves
  when ice_start_vfs() fails. The missing release is older than the
  representor bug that patch 1/2 fixes, so it stays a separate patch.
  (Sashiko AI review)

 drivers/net/ethernet/intel/ice/ice_sriov.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 95abc6704820..df84dbe12cba 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -446,8 +446,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)
@@ -457,6 +459,9 @@ 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;
 }
 
@@ -490,6 +495,8 @@ static int ice_start_vfs(struct ice_pf *pf)
 				dev_err(ice_pf_to_dev(pf), "Failed to attach VF %d to eswitch, error %d",
 					vf->vf_id, retval);
 				ice_vf_vsi_release(vf);
+				ice_virt_free_irqs(pf, vf->first_vector_idx,
+						   vf->num_msix);
 				goto teardown;
 			}
 		}
@@ -511,6 +518,7 @@ static int ice_start_vfs(struct ice_pf *pf)
 		mutex_lock(&vf->cfg_lock);
 
 		ice_eswitch_detach_vf(pf, vf);
+		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

Re: [PATCH iwl-net v2 2/2] ice: release the VF MSI-X window when ice_start_vfs() fails

[Tomasz Lichwala]

On 28.09.2026 08:53, Linkui Xiao wrote:

> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 95abc6704820..df84dbe12cba 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -446,8 +446,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)
> @@ -457,6 +459,9 @@ 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;
>  }
>  
> @@ -490,6 +495,8 @@ static int ice_start_vfs(struct ice_pf *pf)
>  				dev_err(ice_pf_to_dev(pf), "Failed to attach VF %d to eswitch, error %d",
>  					vf->vf_id, retval);
>  				ice_vf_vsi_release(vf);
> +				ice_virt_free_irqs(pf, vf->first_vector_idx,
> +						   vf->num_msix);

Nit: For consistency with the teardown: loop (which frees IRQs before releasing the VSI), consider swapping the order here too - currently this branch releases the VSI first, then frees IRQs, the opposite order. Not functionally significant, just readability.

>  				goto teardown;
>  			}
>  		}

Reviewed-by: Tomasz Lichwala <tomasz.lichwala@linux.intel.com>

RE: [PATCH iwl-net v2 2/2] ice: release the VF MSI-X window when ice_start_vfs() fails

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Linkui Xiao <xiaolinkui@126.com>
> Sent: Monday, September 28, 2026 8:53 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Linkui Xiao <xiaolinkui@kylinos.cn>;
> stable@vger.kernel.org
> Subject: [PATCH iwl-net v2 2/2] ice: release the VF MSI-X window when
> ice_start_vfs() fails
> 
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> ice_init_vf_vsi_res() reserves the VF MSI-X window with
> ice_virt_get_irqs(), which does bitmap_set() on the PF-wide
> pf->virt_irq_tracker.bm, but nothing hands that window back when VF
> creation fails. ice_virt_free_irqs() is not called anywhere on this
> failure path: not from the release_vsi label of ice_init_vf_vsi_res(),
> not from the ice_eswitch_attach_vf() failure branch, and not from the
> teardown loop of ice_start_vfs(), which only undoes the queue mappings
> and the VF VSI. The caller does not help either, because
> err_unroll_vf_entries -> ice_free_vf_entries() -> ice_put_vf() ->
> ice_sriov_free_vf() only does mutex_destroy() and kfree_rcu().
> 
> The tracker is allocated once per PF in ice_init_virt_irq_tracker()
> and freed only in ice_deinit_virt_irq_tracker(), and
> ice_set_per_vf_res() sizes the VFs from pf-
> >virt_irq_tracker.num_entries rather than from the number of free
> bits. So each failed "echo N > sriov_numvfs" leaks the window reserved
> for every VF that got as far as ice_init_vf_vsi_res(), and once the
> tracker is exhausted ice_virt_get_irqs() keeps returning -ENOENT,
> which means SR-IOV cannot be enabled again without reloading the
> driver. The hardware and the software bookkeeping also end up
> disagreeing, because ice_dis_vf_mappings() clears
> VPINT_ALLOC/VPINT_ALLOC_PCI and re-points GLINT_VECT2FUNC of exactly
> those vectors back at the PF.
> 
> Return the window on the failure paths, in the order ice_free_vfs()
> uses: ice_virt_free_irqs() after ice_eswitch_detach_vf() in the
> teardown loop, and right after the VF VSI is released in the two
> branches that fail before the loop can reach that VF.
> 
> The missing release is older than this change: the teardown loop has
> never returned the window. It turns into an accumulating leak because
> the tracker is now PF-wide and outlives a single SR-IOV enable.
> 
> Fixes: a203163274a4 ("ice: simplify VF MSI-X managing")
> Cc: stable@vger.kernel.org
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
> Changes in v2:
> - New patch. Returns the VF MSI-X window that ice_init_vf_vsi_res()
> reserves
>   when ice_start_vfs() fails. The missing release is older than the
>   representor bug that patch 1/2 fixes, so it stays a separate patch.
>   (Sashiko AI review)
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 95abc6704820..df84dbe12cba 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -446,8 +446,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)
> @@ -457,6 +459,9 @@ 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;
>  }
> 
> @@ -490,6 +495,8 @@ static int ice_start_vfs(struct ice_pf *pf)
>  				dev_err(ice_pf_to_dev(pf), "Failed to
> attach VF %d to eswitch, error %d",
>  					vf->vf_id, retval);
>  				ice_vf_vsi_release(vf);
> +				ice_virt_free_irqs(pf, vf-
> >first_vector_idx,
> +						   vf->num_msix);
>  				goto teardown;
>  			}
>  		}
> @@ -511,6 +518,7 @@ static int ice_start_vfs(struct ice_pf *pf)
>  		mutex_lock(&vf->cfg_lock);
> 
>  		ice_eswitch_detach_vf(pf, vf);
> +		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@intel.com>

Re: [PATCH iwl-net v2 1/2] ice: detach the VF representor when ice_start_vfs() fails

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH iwl-net v2 1/2] ice: detach the VF representor when ice_start_vfs() fails

[Linkui Xiao]

Hi,

Thanks for the review. Please find the missing information below.

- How the issue was discovered:
   Found during manual code inspection of the ice VF setup/teardown 
error paths. It was not reported by syzbot, a static analysis tool, or 
an LLM scan, and was not hit in production.

- Whether the issue was actually triggered:
   Not actually triggered. It is a theoretical error-path cleanup issue 
found by inspection; no stack trace or error message was observed.

- Hardware tested:
   Not tested on real hardware. The change was only compile-tested; no 
affected Intel NIC/firmware test was performed.

The same applies to patch 2/2: it was also found by the same manual code 
inspection, was not triggered at runtime, and was only compile-tested, 
not tested on real hardware.

If a v3 is needed for other review reasons, I will include this 
information in the commit messages.

Thanks,
Linkui Xiao

On 2026/9/28 14:59, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>   - How the issue was discovered, e.g. hit in production, hit during
>     development, syzbot report, manual code inspection, LLM or static
>     analysis tool scan.
> 
>   - Whether the issue was actually triggered, or is only theoretical
>     (e.g. found by code inspection). If it was triggered please include
>     the symptoms, like the stack trace or error messages.
> 
>   - What hardware the change was tested on. For driver fixes please
>     mention the device (and if relevant firmware version) used for
>     testing, or say that the change was not tested on real hardware.
> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH iwl-net v2 1/2] ice: detach the VF representor when ice_start_vfs() fails

[Tomasz Lichwala]

On 28.09.2026 08:53, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> ice_start_vfs() attaches every VF it brings up to the eswitch with
> ice_eswitch_attach_vf(), but the teardown path only undoes the queue
> mappings and the VF VSI. Nothing calls ice_eswitch_detach_vf() for the
> VFs that were attached before the failure, and the caller, ice_ena_vfs(),
> goes straight to ice_free_vf_entries(), which drops its reference to
> every VF.
> 
> The port representors created for those VFs therefore outlive the
> failed VF creation:
> 
>   - the representor netdev stays registered and its devlink port stays
>     registered too. That port is embedded in struct ice_vf, so it ends
>     up pointing into the memory that ice_sriov_free_vf() releases;
> 
>   - repr->vf keeps pointing at the freed struct ice_vf, and repr->src_vsi
>     at the VF VSI that ice_vf_vsi_release() tore down. The leftover
>     netdev is still visible to the user, so even a plain
>     "ip -s link show" of it reaches ice_repr_get_stats64(), which calls
>     repr->ops.ready() -> ice_check_vf_ready_for_cfg(repr->vf) and then
>     reads repr->src_vsi through ice_update_eth_stats();
> 
>   - the virtchnl ops of that VF, which ice_repr_add_vf() replaced with
>     ice_virtchnl_set_repr_ops(), are never handed back to
>     ice_virtchnl_set_dflt_ops();
> 
>   - pf->eswitch.reprs never becomes empty, so ice_eswitch_detach() never
>     calls ice_eswitch_disable_switchdev(). pf->eswitch.is_running stays
>     true, with the bridge offloads and the devlink rate topology still
>     up, and ice_eswitch_release_env() is skipped, so the uplink VSI is
>     left in the switchdev configuration that ice_eswitch_setup_env()
>     gave it.
> 
> Detach the representor in the teardown loop the way ice_free_vfs() does,
> ahead of ice_vf_vsi_release(), because ice_repr_rem_vf() and
> ice_eswitch_release_repr() both need repr->src_vsi to still be valid.
> Every VF the teardown loop walks completed ice_eswitch_attach_vf()
> successfully, and ice_eswitch_detach_vf() already returns early for a VF
> without a representor, so no extra condition is needed.
> 
> Hold vf->cfg_lock across the teardown of each VF as well, like
> ice_free_vfs() does. The VFs unwound here are the ones that already
> reached set_bit(ICE_VF_STATE_INIT), which is exactly what
> ice_check_vf_ready_for_cfg() checks, so a host administrator can still
> run "ip link set dev <pf> vf N ..." and a VF can still send a mailbox
> message while the loop walks them. Both paths take cfg_lock and then run
> ice_reset_vf(), which gets to ice_eswitch_update_repr() and writes
> through the representor that is being freed, or reach
> ice_vc_process_vf_msg() reading vf->virtchnl_ops while
> ice_virtchnl_set_dflt_ops() hands them back.
> 
> Fixes: fff292b47ac1 ("ice: add VF representors one by one")
> Cc: stable@vger.kernel.org
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
> v1:
> - Link: https://lore.kernel.org/netdev/20260921031616.3390259-1-xiaolinkui@126.com/
> 
> Changes in v2:
> - Hold vf->cfg_lock across the teardown of each VF, the way ice_free_vfs()
>   does, so that a concurrent VF reconfiguration cannot walk through the
>   representor and the virtchnl ops that are being torn down.
>   (Sashiko AI review)
> - Patch 2/2 is new and returns the VF MSI-X window that the same failure path
>   reserves. That is a separate, pre-existing bug, so it is not folded in here.
>   (Sashiko AI review)
> - Not carrying over the Reviewed-by from Aleksandr Loktionov, as the code
>   changed after his review.
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index e04de0215596..95abc6704820 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -508,8 +508,13 @@ static int ice_start_vfs(struct ice_pf *pf)
>  		if (it_cnt == 0)
>  			break;
>  
> +		mutex_lock(&vf->cfg_lock);
> +
> +		ice_eswitch_detach_vf(pf, vf);
>  		ice_dis_vf_mappings(vf);
>  		ice_vf_vsi_release(vf);
> +		mutex_unlock(&vf->cfg_lock);
> +
>  		it_cnt--;
>  	}
>  


Reviewed-by: Tomasz Lichwala <tomasz.lichwala@linux.intel.com>

RE: [PATCH iwl-net v2 1/2] ice: detach the VF representor when ice_start_vfs() fails

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Linkui Xiao <xiaolinkui@126.com>
> Sent: Monday, September 28, 2026 8:53 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Linkui Xiao <xiaolinkui@kylinos.cn>;
> stable@vger.kernel.org
> Subject: [PATCH iwl-net v2 1/2] ice: detach the VF representor when
> ice_start_vfs() fails
> 
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> ice_start_vfs() attaches every VF it brings up to the eswitch with
> ice_eswitch_attach_vf(), but the teardown path only undoes the queue
> mappings and the VF VSI. Nothing calls ice_eswitch_detach_vf() for the
> VFs that were attached before the failure, and the caller,
> ice_ena_vfs(), goes straight to ice_free_vf_entries(), which drops its
> reference to every VF.
> 
> The port representors created for those VFs therefore outlive the
> failed VF creation:
> 
>   - the representor netdev stays registered and its devlink port stays
>     registered too. That port is embedded in struct ice_vf, so it ends
>     up pointing into the memory that ice_sriov_free_vf() releases;
> 
>   - repr->vf keeps pointing at the freed struct ice_vf, and repr-
> >src_vsi
>     at the VF VSI that ice_vf_vsi_release() tore down. The leftover
>     netdev is still visible to the user, so even a plain
>     "ip -s link show" of it reaches ice_repr_get_stats64(), which
> calls
>     repr->ops.ready() -> ice_check_vf_ready_for_cfg(repr->vf) and then
>     reads repr->src_vsi through ice_update_eth_stats();
> 
>   - the virtchnl ops of that VF, which ice_repr_add_vf() replaced with
>     ice_virtchnl_set_repr_ops(), are never handed back to
>     ice_virtchnl_set_dflt_ops();
> 
>   - pf->eswitch.reprs never becomes empty, so ice_eswitch_detach()
> never
>     calls ice_eswitch_disable_switchdev(). pf->eswitch.is_running
> stays
>     true, with the bridge offloads and the devlink rate topology still
>     up, and ice_eswitch_release_env() is skipped, so the uplink VSI is
>     left in the switchdev configuration that ice_eswitch_setup_env()
>     gave it.
> 
> Detach the representor in the teardown loop the way ice_free_vfs()
> does, ahead of ice_vf_vsi_release(), because ice_repr_rem_vf() and
> ice_eswitch_release_repr() both need repr->src_vsi to still be valid.
> Every VF the teardown loop walks completed ice_eswitch_attach_vf()
> successfully, and ice_eswitch_detach_vf() already returns early for a
> VF without a representor, so no extra condition is needed.
> 
> Hold vf->cfg_lock across the teardown of each VF as well, like
> ice_free_vfs() does. The VFs unwound here are the ones that already
> reached set_bit(ICE_VF_STATE_INIT), which is exactly what
> ice_check_vf_ready_for_cfg() checks, so a host administrator can still
> run "ip link set dev <pf> vf N ..." and a VF can still send a mailbox
> message while the loop walks them. Both paths take cfg_lock and then
> run ice_reset_vf(), which gets to ice_eswitch_update_repr() and writes
> through the representor that is being freed, or reach
> ice_vc_process_vf_msg() reading vf->virtchnl_ops while
> ice_virtchnl_set_dflt_ops() hands them back.
> 
> Fixes: fff292b47ac1 ("ice: add VF representors one by one")
> Cc: stable@vger.kernel.org
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
> v1:
> - Link: https://lore.kernel.org/netdev/20260921031616.3390259-1-
> xiaolinkui@126.com/
> 
> Changes in v2:
> - Hold vf->cfg_lock across the teardown of each VF, the way
> ice_free_vfs()
>   does, so that a concurrent VF reconfiguration cannot walk through
> the
>   representor and the virtchnl ops that are being torn down.
>   (Sashiko AI review)
> - Patch 2/2 is new and returns the VF MSI-X window that the same
> failure path
>   reserves. That is a separate, pre-existing bug, so it is not folded
> in here.
>   (Sashiko AI review)
> - Not carrying over the Reviewed-by from Aleksandr Loktionov, as the
> code
>   changed after his review.
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index e04de0215596..95abc6704820 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -508,8 +508,13 @@ static int ice_start_vfs(struct ice_pf *pf)
>  		if (it_cnt == 0)
>  			break;
> 
> +		mutex_lock(&vf->cfg_lock);
> +
> +		ice_eswitch_detach_vf(pf, vf);
>  		ice_dis_vf_mappings(vf);
>  		ice_vf_vsi_release(vf);
> +		mutex_unlock(&vf->cfg_lock);
> +
>  		it_cnt--;
>  	}
> 
> --
> 2.25.1

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