Re: [PATCH net v2 1/3] i40e: skip unnecessary VF reset when setting trust

[Jacob Keller <jacob.e.keller@intel.com>]

On 10/6/2026 10:33 PM, Jose Ignacio Tornos Martinez wrote:
> Hi Jacob,
> 
>> Since the usual case is going to be a situation where we don't expect
>> to exceed these limits, couldn't this check be expanded so we would
>> still reset in the case where the filters need to be removed? I guess
>> its overkill and maybe we accept the extra filters.
> 
> I think accepting the extra filters in this rare case is a reasonable
> trade-off. In the usual case the VF will be well within the untrusted
> limits (bonding setup typically adds 1-2 MAC filters). The reset adds
> ~10 second delay, so we only want it when truly necessary.
> 

Makes sense.

>> This doesn't address the case where filters have MAC addresses
>> different from the admin-set MAC? I am not sure if that still leaves
>> a security hole or not.
> 
> The PF enforces the admin-set MAC restriction on every new add request
> from an untrusted VF (i40e_check_vf_permission() rejects it). Existing
> filters with different MACs remain in hardware, but the VF cannot add
> new ones. I think this is not a security hole: the filters were
> legitimately installed while the VF was trusted, and the VF cannot
> escalate from this state.
> 

True.

>> This is an intentional change that I think should at least be called
>> out in the commit message. This *does* mean a VF which was previously
>> trusted could remain exceeding its untrusted limit in perpetuity
>> until it resets or self-removes the filters.
>>
>> That does mean the VF will technically violate the trusted boundary,
>> but it can't get worse, and it was previously accepted while under
>> trust. I think.
>>
>> I think this is acceptable, but we may want to clarify the
>> functionally changed behavior here so that it is clear.
> 
> Agreed, this is an intentional trade-off that was considered during
> development. As you say, the VF cannot make it worse, and the filters
> were accepted while trusted. And of course, a VF reset cleans
> everything up completely. If you think it would be helpful, I can send
> a follow-up adding a comment in the code documenting this trade-off.
> 

The patches already merged, and I think its ok as-is, the commit message
is pretty clear on what changed. I don't know if a code comment would
actually clarify for anyone that matters. If we had any documentation on
the behavior of the trusted flags that would be the best place for it..
but I don't think that is very well covered in any user manual.

Thanks,
Jake

> Thanks
> 
> Best regards
> José Ignacio
>