RE: [Intel-wired-lan] [PATCH net v5 4/4] ice: skip unnecessary VF reset when setting trust

-----Original Message-----
From: Intel-wired-lan <redacted> On Behalf Of Jose
Ignacio Tornos Martinez
Sent: Wednesday, April 29, 2026 12:24 PM
To: netdev@vger.kernel.org
Cc: intel-wired-lan@lists.osuosl.org; Kitszel, Przemyslaw
[off-list ref]; Loktionov, Aleksandr
[off-list ref]; Keller, Jacob E [off-list ref];
horms@kernel.org; jesse.brandeburg@intel.com; Nguyen, Anthony L
[off-list ref]; davem@davemloft.net; edumazet@google.com;
kuba@kernel.org; pabeni@redhat.com; Jose Ignacio Tornos Martinez
[off-list ref]
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 <redacted>
---
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-
jtornosm@redhat.com/

 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