Re: [PATCH net v3 4/5] ice: skip unnecessary VF reset when setting trust

Simon Horman
2026-04-16T13:55:43Z

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
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. The ice driver already
has logic to clean up MAC LLDP filters when removing trust, which is
the only operation that requires filter synchronization. After this
cleanup, the VF reset is only necessary if there were actually filters
to remove.

> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -1399,14 +1399,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);
> -
> 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");
>
> + /* Only reset VF if removing trust and there are MAC LLDP filters
> + * to clean up. Reset is needed to ensure filter removal completes.
> + */
> + if (!trusted && vf->num_mac_lldp) {
> + while (vf->num_mac_lldp)
> + ice_vf_update_mac_lldp_num(vf, ice_get_vf_vsi(vf), false);
> + ice_reset_vf(vf, ICE_VF_RESET_NOTIFY);
> + }
> +

Does setting vf->trusted before the cleanup loop break the filter
removal? Looking at the cleanup mechanism, ice_vf_update_mac_lldp_num()
calls ice_vf_is_lldp_ena() which checks both num_mac_lldp AND
vf->trusted:

ice_vf_lib.h:ice_vf_is_lldp_ena():
return vf->num_mac_lldp && vf->trusted;

With the new ordering where vf->trusted is set to false before the loop:

ice_vf_update_mac_lldp_num() logic:
was_ena = ice_vf_is_lldp_ena(vf); // evaluates to (num > 0 && FALSE) = FALSE
vf->num_mac_lldp--;
is_ena = ice_vf_is_lldp_ena(vf); // evaluates to FALSE

if (was_ena != is_ena) // FALSE != FALSE is FALSE
ice_vsi_cfg_sw_lldp(vsi, false, false); // NEVER called

But in the old code, vf->trusted was still TRUE during the loop:

was_ena = (num > 0 && TRUE) = TRUE
vf->num_mac_lldp--;
is_ena = (0 && TRUE) = FALSE // when last filter removed

if (was_ena != is_ena) // TRUE != FALSE is TRUE
ice_vsi_cfg_sw_lldp(...) // called to remove LLDP filters

So the ETH_P_LLDP packet filters are not removed from the hardware
switch, leaving stale filter rules. Should vf->trusted be set after the
cleanup loop instead?

> mutex_unlock(&vf->cfg_lock);
>