[PATCH net] eth: fbnic: fix ordering of heartbeat vs ownership

[PATCH net] eth: fbnic: fix ordering of heartbeat vs ownership

[Jakub Kicinski]

When requesting ownership of the NIC (MAC/PHY control), we set up
the heartbeat to look stale:

  /* Initialize heartbeat, set last response to 1 second in the past
   * so that we will trigger a timeout if the firmware doesn't respond
   */
  fbd->last_heartbeat_response = req_time - HZ;
  fbd->last_heartbeat_request = req_time;

The response handler then sets:

  fbd->last_heartbeat_response = jiffies;

for which we wait via:

  fbnic_fw_init_heartbeat() -> fbnic_fw_heartbeat_current()

The scheme is a bit odd, but it should work in principle.

Fix the ordering of operations. We have to set up the stale heartbeat
before we send the message. Otherwise if the response is very fast
we will override it. This triggers on QEMU if we run on the core
that handles the IRQ, and results in ndo_open failing with ETIMEDOUT.

The change in ordering doesn't impact releasing the ownership.
Both ndo_stop and heartbeat check are under rtnl_lock.

Fixes: 20d2e88cc746 ("eth: fbnic: Add initial messaging to notify FW of our presence")
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
index 0c6812fcf185..283d25fae79e 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
@@ -526,15 +526,10 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership)
 			goto free_message;
 	}
 
-	err = fbnic_mbx_map_tlv_msg(fbd, msg);
-	if (err)
-		goto free_message;
-
 	/* Initialize heartbeat, set last response to 1 second in the past
 	 * so that we will trigger a timeout if the firmware doesn't respond
 	 */
 	fbd->last_heartbeat_response = req_time - HZ;
-
 	fbd->last_heartbeat_request = req_time;
 
 	/* Set prev_firmware_time to 0 to avoid triggering firmware crash
@@ -542,6 +537,10 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership)
 	 */
 	fbd->prev_firmware_time = 0;
 
+	err = fbnic_mbx_map_tlv_msg(fbd, msg);
+	if (err)
+		goto free_message;
+
 	/* Set heartbeat detection based on if we are taking ownership */
 	fbd->fw_heartbeat_enabled = take_ownership;
 
-- 
2.54.0

Re: [PATCH net] eth: fbnic: fix ordering of heartbeat vs ownership

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]

On Mon, Jun 22, 2026 at 9:18 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> When requesting ownership of the NIC (MAC/PHY control), we set up
> the heartbeat to look stale:
>
>   /* Initialize heartbeat, set last response to 1 second in the past
>    * so that we will trigger a timeout if the firmware doesn't respond
>    */
>   fbd->last_heartbeat_response = req_time - HZ;
>   fbd->last_heartbeat_request = req_time;
>
> The response handler then sets:
>
>   fbd->last_heartbeat_response = jiffies;
>
> for which we wait via:
>
>   fbnic_fw_init_heartbeat() -> fbnic_fw_heartbeat_current()
>
> The scheme is a bit odd, but it should work in principle.
>
> Fix the ordering of operations. We have to set up the stale heartbeat
> before we send the message. Otherwise if the response is very fast
> we will override it. This triggers on QEMU if we run on the core
> that handles the IRQ, and results in ndo_open failing with ETIMEDOUT.
>
> The change in ordering doesn't impact releasing the ownership.
> Both ndo_stop and heartbeat check are under rtnl_lock.
>
> Fixes: 20d2e88cc746 ("eth: fbnic: Add initial messaging to notify FW of our presence")
> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

Re: [PATCH net] eth: fbnic: fix ordering of heartbeat vs ownership

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 22 Jun 2026 08:47:53 -0700 you wrote:
> When requesting ownership of the NIC (MAC/PHY control), we set up
> the heartbeat to look stale:
> 
>   /* Initialize heartbeat, set last response to 1 second in the past
>    * so that we will trigger a timeout if the firmware doesn't respond
>    */
>   fbd->last_heartbeat_response = req_time - HZ;
>   fbd->last_heartbeat_request = req_time;
> 
> [...]

Here is the summary with links:
  - [net] eth: fbnic: fix ordering of heartbeat vs ownership
    https://git.kernel.org/netdev/net/c/d87363b0edfc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html