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

On 10/6/2026 10:33 PM, Jose Ignacio Tornos Martinez wrote:

Hi Jacob,

quoted

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.

quoted

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.

quoted

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