RE: [PATCH net v4 4/4] ice: skip unnecessary VF reset when setting trust

Loktionov, Aleksandr
2026-04-23T13:17:27Z

> -----Original Message-----
> From: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> Sent: Thursday, April 23, 2026 3:04 PM
> To: netdev@vger.kernel.org
> Cc: intel-wired-lan@lists.osuosl.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>; horms@kernel.org;
> jesse.brandeburg@intel.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Jose Ignacio
> Tornos Martinez <jtornosm@redhat.com>
> Subject: [PATCH net v4 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 need to:
> 1. Clear the capability flag to block privileged operations 2. Disable
> promiscuous mode if it was enabled (trusted VFs can enable it) 3. Only
> reset if MAC LLDP filters exist (to clean them up)
>
> 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 and
> promiscuous mode via helper function.
>
> The ice driver already has logic to clean up MAC LLDP filters when
> removing trust. After this cleanup, the VF reset is only necessary if
> there were actually filters to remove (num_mac_lldp was non-zero).
>
> This saves time and eliminates unnecessary service disruption when
> changing VF trust settings in most cases, while properly handling
> filter cleanup.
>
> Fixes: 2296345416b0 ("ice: receive LLDP on trusted VFs")
For me it looks like cc: stable@vger.kernel.org must be added

> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
> v4:
> - Address AI review (sashiko.dev) from Simon Horman:
> vf->trusted ordering bug
> - Fix upstream race condition when comparing with i40e code
> - Apply capability flag and promiscuous mode fixes from i40e AI
> review
> - Add helper function ice_setup_vf_trust() for non-reset path
> - Export ice_vf_clear_all_promisc_modes() for code reuse
> v3: https://lore.kernel.org/all/20260414110006.124286-5-
> jtornosm@redhat.com/
>
> drivers/net/ethernet/intel/ice/ice_sriov.c | 41 +++++++++++++++++++-
> - drivers/net/ethernet/intel/ice/ice_vf_lib.c | 2 +-
> drivers/net/ethernet/intel/ice/ice_vf_lib.h | 1 +
> 3 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 7e00e091756d..d0da7f6adc23 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -1364,6 +1364,34 @@ 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
> + *
> + * Manually handle capability flag and promiscuous mode when changing
> +trust
> + * without performing a VF reset.
> + * When reset is performed, this is not necessary as the reset
> +procedure
> + * already handles this.
> + **/
> +static void ice_setup_vf_trust(struct ice_vf *vf, bool setting) {
> + struct ice_vsi *vsi;
> +
> + if (setting) {
> + set_bit(ICE_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps);
> + } else {
> + clear_bit(ICE_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps);
> +
> + if (test_bit(ICE_VF_STATE_UC_PROMISC, vf->vf_states) ||
> + test_bit(ICE_VF_STATE_MC_PROMISC, vf->vf_states)) {
> + vsi = ice_get_vf_vsi(vf);
> + if (vsi)
> + ice_vf_clear_all_promisc_modes(vf, vsi);
You declare ice_vf_clear_all_promisc_modes() returning int, but ignore the return value.
Looks suspicious isn't it?

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

> + }
> + }
> +}
> +
> /**
> * ice_set_vf_trust
> * @netdev: network interface device structure @@ -1399,11 +1427,16
> @@ 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 with MAC LLDP filters */
> + if (!trusted && vf->num_mac_lldp) {
> + 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");
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> index c8bc952f05cd..81bbf30e5c29 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> @@ -623,7 +623,7 @@ ice_vf_get_promisc_masks(struct ice_vf *vf, struct
> ice_vsi *vsi,
> *
> * Clear all promiscuous/allmulticast filters for a VF
> */
> -static int
> +int
> ice_vf_clear_all_promisc_modes(struct ice_vf *vf, struct ice_vsi
> *vsi) {
> struct ice_pf *pf = vf->pf;
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> index 7a9c75d1d07c..a3501bd92311 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> @@ -310,6 +310,7 @@ bool ice_is_any_vf_in_unicast_promisc(struct
> ice_pf *pf); void ice_vf_get_promisc_masks(struct ice_vf *vf, struct
> ice_vsi *vsi,
> u8 *ucast_m, u8 *mcast_m);
> +int ice_vf_clear_all_promisc_modes(struct ice_vf *vf, struct ice_vsi
> +*vsi);
> int
> ice_vf_set_vsi_promisc(struct ice_vf *vf, struct ice_vsi *vsi, u8
> promisc_m); int
> --
> 2.53.0