Re: [PATCH net 3/8] idpf: fix possible race in idpf_vport_stop()

[Tantilov, Emil S]

On 10/3/2025 10:43 AM, Jakub Kicinski wrote:
> On Wed, 01 Oct 2025 17:14:13 -0700 Jacob Keller wrote:
> > From: Emil Tantilov <emil.s.tantilov@xxxxxxxxx>
> >
> > Make sure to clear the IDPF_VPORT_UP bit on entry. The idpf_vport_stop()
> > function is void and once called, the vport teardown is guaranteed to
> > happen. Previously the bit was cleared at the end of the function, which
> > opened it up to possible races with all instances in the driver where
> > operations were conditional on this bit being set. For example, on rmmod
> > callbacks in the middle of idpf_vport_stop() end up attempting to remove
> > MAC address filter already removed by the function:
> > idpf 0000:83:00.0: Received invalid MAC filter payload (op 536) (len 0)
>
> Argh, please stop using the flag based state machines. They CANNOT
> replace locking. If there was proper locking in place it wouldn't
> have mattered when we clear the flag.

This patch is resolving a bug in the current logic of how the flag is 
used (not being atomic and not being cleared properly). I don't think
there is an existing lock in place to address this issue, though we are
looking to refactor the code over time to remove and/or limit how these
flags are used.
> I'm guessing false negatives are okay in how you use the UP flag?
> The commit message doesn't really explain why.

I am not sure I understand the question completely, if you mean that the
flag is cleared before the vport is actually brought down, I did touch
on it at the beginning of the description, once we enter
idpf_vport_stop() we know for sure that the vport will be brought down 
and that is how the present checks in the driver for the UP flag are
being used. Specifically in the scenario that exposed this issue, the 
driver will send a message in the middle of idpf_vport_down() after
checking the flag.

Thanks,
Emil