> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf Of Jose > Ignacio Tornos Martinez > Sent: Wednesday, April 29, 2026 12:24 PM > To: [email protected] > Cc: [email protected]; Kitszel, Przemyslaw > <[email protected]>; Loktionov, Aleksandr > <[email protected]>; Keller, Jacob E <[email protected]>; > [email protected]; [email protected]; Nguyen, Anthony L > <[email protected]>; [email protected]; [email protected]; > [email protected]; [email protected]; Jose Ignacio Tornos Martinez > <[email protected]> > Subject: [Intel-wired-lan] [PATCH net v5 4/4] ice: skip unnecessary VF reset > when > setting trust > > Similar to the i40e fix, ice_set_vf_trust() unconditionally calls > ice_reset_vf() when the trust setting changes. While the delay is smaller than > i40e this reset is still unnecessary in most cases. > > Additionally, the original code has a race condition: it deletes MAC LLDP > filters > BEFORE resetting the VF. During this deletion, the VF is still ACTIVE and can > add > new MAC LLDP filters concurrently, potentially corrupting the filter list. > > When granting trust, no reset is needed - we can just set the capability flag > to > allow privileged operations. > > When revoking trust, we only need to reset (conservative approach) if the VF > has > actually configured advanced features that require cleanup (MAC LLDP filters, > promiscuous mode). For VFs in a clean state, we can safely change the trust > setting without the disruptive reset. > > When we do reset (MAC LLDP case), we fix the race condition by resetting > first to > clear VF state (which blocks new MAC LLDP filter additions), then delete > existing > filters safely. During cleanup, vf->trusted remains true so > ice_vf_is_lldp_ena() works properly. Only after cleanup do we set > vf->trusted = false. > > When we don't reset, we manually handle capability flag via helper function, > eliminating the delay. > > Fixes: 2296345416b0 ("ice: receive LLDP on trusted VFs") > Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]> > --- > v5 Address the comments from Aleksandr Loktionov: > - Error handling when ice_setup_vf_trust is called is not necessary > because ice_vf_clear_all_promisc_modes is not used due to the > conservative approach to solve AI tool review concerns > - kdoc should end with '*/' not '**/' (new function) > Address AI review (sashiko.dev) from Simon Horman: > - Adopt a conservative approach checking multiple conditions before > skipping reset: MAC LLDP filters, promiscuous mode > - Simplify helper function to only handle capability flag > - No need to export ice_vf_clear_all_promisc_modes > v4: https://lore.kernel.org/all/20260423130405.139568-5- > [email protected]/ > > drivers/net/ethernet/intel/ice/ice_sriov.c | 33 +++++++++++++++++++--- > 1 file changed, 29 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c > b/drivers/net/ethernet/intel/ice/ice_sriov.c > index 7e00e091756d..3c64ed1b41a8 100644 > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c > @@ -1364,6 +1364,23 @@ int ice_set_vf_mac(struct net_device *netdev, int > vf_id, u8 *mac) > return __ice_set_vf_mac(ice_netdev_to_pf(netdev), vf_id, mac); } > > +/** > + * ice_setup_vf_trust - Enable/disable VF trust mode without reset > + * @vf: VF to configure > + * @setting: trust setting > + * > + * Update VF flags when changing trust without performing a VF reset. > + * This is only called when it's safe to skip the reset (VF has no > +advanced > + * features configured that need cleanup). > + */ > +static void ice_setup_vf_trust(struct ice_vf *vf, bool setting) { > + if (setting) > + set_bit(ICE_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps); > + else > + clear_bit(ICE_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps); } > + > /** > * ice_set_vf_trust > * @netdev: network interface device structure @@ -1399,11 +1416,19 @@ int > ice_set_vf_trust(struct net_device *netdev, int vf_id, bool trusted) > > mutex_lock(&vf->cfg_lock); > > - while (!trusted && vf->num_mac_lldp) > - ice_vf_update_mac_lldp_num(vf, ice_get_vf_vsi(vf), false); > - > + /* Reset only if revoking trust and VF has advanced features configured > */ > + if (!trusted && > + (vf->num_mac_lldp > 0 || > + test_bit(ICE_VF_STATE_UC_PROMISC, vf->vf_states) || > + test_bit(ICE_VF_STATE_MC_PROMISC, vf->vf_states))) { > + ice_reset_vf(vf, ICE_VF_RESET_NOTIFY); > + while (vf->num_mac_lldp) > + ice_vf_update_mac_lldp_num(vf, ice_get_vf_vsi(vf), > false); > + } else { > + ice_setup_vf_trust(vf, trusted); > + } > vf->trusted = trusted; > - ice_reset_vf(vf, ICE_VF_RESET_NOTIFY); > + > dev_info(ice_pf_to_dev(pf), "VF %u is now %strusted\n", > vf_id, trusted ? "" : "un"); > > -- > 2.53.0
Tested-by: Rafal Romanowski <[email protected]>
