RE: [PATCH] net/txgbe: fix AML mailbox lock acquisition

["Zaiyu Wang" <zaiyuwang@trustnetic.com>]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Friday, May 22, 2026 1:01 PM
> To: dev@dpdk.org; dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; stable@dpdk.org;
> Jiawen Wu <jiawenwu@trustnetic.com>; Zaiyu Wang
> <zaiyuwang@trustnetic.com>; Stephen Hemminger
> <stephen@networkplumber.org>; stable@dpdk.org; Jiawen Wu
> <jiawenwu@trustnetic.com>; Zaiyu Wang <zaiyuwang@trustnetic.com>
> 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: stable@dpdk.org
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  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 <zaiyuwang@trustnetic.com>