On 10/12/2026 1:29 AM, Linkui Xiao wrote: > From: Linkui Xiao <[email protected]> > > __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?
