Hello Mike,
On Tue, Dec 20, 2022 at 12:24 AM Mike Pattrick <m...@redhat.com> wrote:
>
> iavf admin queue commands aren't thread-safe. Bugs surrounding this
> issue can manifest in a variety of ways but frequently pend_cmd is
> over written. Simultaneously executing commands can result in a
> misconfigured device or DPDK sleeping in a thread for 2 second.
>
> Despite this limitation, vf commands may be executed from both
> iavf_dev_alarm_handler() in a control thread and the applications main
> thread. This is trivial to simulate in the testpmd application by
> creating a bond of vf's in active backup mode, and then turning the
> bond off and then on again repeatedly.
>
> Previously [1] was proposed as a potential solution, but this commit
> has been reverted. I propose adding locks until a more complete
> solution is available.
- Hum, this commit is still in the main branch, and I did reproduce
the issue with a E810 nic.
[1] is not enough for the race on pend_cmd (probably some alarm is
still racing with the control thread).
So I think a fix is needed regardless of [1] fate.
- Your patch does make the race disappear, but then, about using a
lock on pend_cmd, I wonder what the point to set it atomically is.
- The same issue probably affects the recently copy/pasted^Wintroduced
driver idpf.
>
> [1] commit cb5c1b91f76f ("net/iavf: add thread for event callbacks")
>
> Fixes: 48de41ca11f0 ("net/avf: enable link status update")
> Fixes: 84108425054a ("net/iavf: support asynchronous virtual channel message")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Mike Pattrick <m...@redhat.com>
> ---
[snip]
Doing a git grep -nC 1 iavf_execute_vf_cmd, I see some unprotected calls.
This is probably a rebase issue if you tested on 21.11.
--
David Marchand
