Re: [PATCH iwl-net] ice: do not report success for a VF setting that was not applied

[Jacob Keller]

On 10/12/2026 1:29 AM, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@xxxxxxxxxx>
> 
> __ice_set_vf_mac(), ice_set_vf_trust() and ice_set_vf_port_vlan() all
> check that the VF is ready for configuration, take vf->cfg_lock and then
> call ice_reset_vf() to push the new setting into hardware. The result of
> ice_reset_vf() is not looked at, and it could not be: it returns 0 both
> when the reset ran and when it bailed out because the VF is disabled.
> 
> The check runs before the lock is taken, so a reset can disable the VF
> in between. ice_reset_all_vfs() raises ICE_VF_DIS in pf->state before it
> walks the VF table, rebuilds every VSI and clears the bit again. An
> ndo_set_vf_mac() that passed ice_check_vf_ready_for_cfg() just before
> that then waits for cfg_lock, stores the new address in vf->dev_lan_addr
> and vf->hw_lan_addr and calls ice_reset_vf(), which sees the VF
> disabled, stops its queues and returns 0 without reprogramming anything.
> __ice_set_vf_mac() reports success, so ip link and the devlink port
> function now show a MAC address the hardware never got, and the switch
> keeps enforcing the previous one until something else resets the VF.
> 
> ice_set_vf_trust() and ice_set_vf_port_vlan() have the same shape: the
> port VLAN lands in vf->port_vlan_info and the trust flag in vf->trusted
> before the reset is skipped.
> 
> Re-check ice_is_vf_disabled() once cfg_lock is held and fail with
> -EINVAL, the same code ice_check_vf_ready_for_cfg() returns for a
> disabled VF. The VF is being reset or removed at that point, so the
> caller should retry rather than be told the setting is in place.
> Changing what ice_reset_vf() returns on that path is not an option, too
> many callers rely on it meaning "nothing to do".
> 
> The race was found by code inspection of the ndo VF handlers, it was not
> reproduced at runtime and no error message was observed.
> 
Hm. My recollection is that we should set something on the PF side that
stores the new address so that the VF would get that value when next
configured.. But I guess that isn't how ice works.

Why not just move ice_check_vf_ready_for_cfg() inside the mutex though?