> -----Original Message-----
> From: Stephen Hemminger <[email protected]>
> Sent: Friday, May 22, 2026 1:01 PM
> To: [email protected]; [email protected]
> Cc: Stephen Hemminger <[email protected]>; [email protected];
> Jiawen Wu <[email protected]>; Zaiyu Wang
> <[email protected]>; Stephen Hemminger
> <[email protected]>; [email protected]; Jiawen Wu
> <[email protected]>; Zaiyu Wang <[email protected]>
> Subject: [PATCH] net/txgbe: fix AML mailbox lock acquisition
>
> The try-lock spin loop in txgbe_host_interface_command_aml() has the
condition
> inverted. rte_atomic32_test_and_set returns non-zero on successful
acquisition (0
> -> 1), zero when the lock was already held. Walk through the two cases:
>
> swfw_busy was 0 (free): test_and_set returns 1, sets to 1, loop
> body runs and sleeps 1ms. Next iteration finds 1, returns 0, loop
> exits. Lock held, after an unnecessary 1ms sleep.
>
> swfw_busy was 1 (busy): test_and_set returns 0, loop exits
> immediately. The caller proceeds without holding the lock,
> racing with the in-flight host interface command.
>
> Invert the condition so the loop spins while the lock remains held and
exits only
> when acquisition succeeds, matching the intent of the surrounding timeout
> machinery.
>
> Fixes: 6a139ade82e7 ("net/txgbe: add new SW-FW mailbox interface")
> Cc: [email protected]
>
> Signed-off-by: Stephen Hemminger <[email protected]>
> ---
> drivers/net/txgbe/base/txgbe_mng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/txgbe/base/txgbe_mng.c
> b/drivers/net/txgbe/base/txgbe_mng.c
> index a1974820b6..dcd8f58a68 100644
> --- a/drivers/net/txgbe/base/txgbe_mng.c
> +++ b/drivers/net/txgbe/base/txgbe_mng.c
> @@ -185,7 +185,7 @@ txgbe_host_interface_command_aml(struct txgbe_hw
> *hw, u32 *buffer,
> }
>
> /* try to get lock */
> - while (rte_atomic32_test_and_set(&hw->swfw_busy)) {
> + while (rte_atomic32_test_and_set(&hw->swfw_busy) == 0) {
> timeout--;
> if (!timeout)
> return TXGBE_ERR_TIMEOUT;
> --
> 2.53.0
Thanks for the patch and the detailed analysis.
The fix is correct. This code was originally ported from our out-of-tree
Linux kernel driver, where it used a classic test_and_set_bit() that returns
the old bit value (0 on success). When adapting it for DPDK, we overlooked
that rte_atomic32_test_and_set() returns the opposite semantic.
Acked-by: Zaiyu Wang <[email protected]>
