[PATCH net] bnxt_en: Take RTNL before netif_close() when aborting FW reset
flat view
HOTtoday
From: Bryan Cochrane <hidden>
Date: 2026-10-29 03:28:39
Also in:
lkml, stable
Subsystem:
broadcom bnxt_en 50 gigabit ethernet driver, networking drivers, the rest · Maintainers:
Michael Chan, Pavan Chebbi, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
netif_close() must be called with RTNL held in addition to the netdev instance lock: __dev_close_many() asserts it, and the NETDEV_GOING_DOWN and NETDEV_DOWN notifier chains it runs rely on it for serialization. Commit 004b5008016a ("eth: bnxt: remove most dependencies on RTNL") replaced rtnl_lock() with netdev_lock() in bnxt_fw_reset_task() and bnxt_fw_reset(). That is sufficient for the paths that only tear the NIC down internally with __bnxt_close_nic(), but the abort paths close the netdev via bnxt_fw_reset_abort() -> netif_close(), and the error path in bnxt_fw_reset() calls netif_close() directly. Those now run without RTNL and trigger a WARN_ONCE() cascade when a firmware reset is aborted: RTNL: assertion failed at net/core/dev.c (1740) WARNING: net/core/dev.c:1740 at __dev_close_many+0x1be/0x1f0 Workqueue: bnxt_pf_wq bnxt_fw_reset_task [bnxt_en] Call Trace: netif_close_many+0xb5/0x190 netif_close+0xa2/0xd0 bnxt_fw_reset_task.cold+0x8d/0x213 [bnxt_en] process_one_work+0x19d/0x3a0 worker_thread+0x1af/0x320 kthread+0xe3/0x120 followed by the same assertion from call_netdevice_notifiers_info(), fib_rules_event(), inetdev_event(), rtnl_fill_ifinfo(), devlink_compat_phys_port_name_get() and ip_mc_down(). This was observed on a machine in real use with a BCM57504 (4 PFs, FW 236.1.153.0) running v7.1.6: the firmware crashed, announced a "Fatal firmware reset event" and then failed to come back within the advertised max wait, so bnxt_fw_reset_task() aborted the reset on all ports: bnxt_en 0000:c1:00.1 eno16905np1: Firmware reset aborted bnxt_en 0000:c1:00.1 eno16905np1: fw_health_status 0x448100 Take RTNL (before the netdev instance lock, matching the OPENING state which already uses rtnl_trylock()) in the fw_reset_abort path of bnxt_fw_reset_task() and route the POLL_VF abort through that same path. In bnxt_fw_reset() go back to bnxt_rtnl_lock_sp() so that its netif_close() error path is covered as well; bnxt_reset() and bnxt_rx_ring_reset() already take RTNL from bnxt_sp_task() the same way. The bug was found by inspecting the kernel log of the affected machine and the driver source; the fix was prepared with the help of an AI coding assistant (Gemini based) and reviewed by the submitter. It is build tested (allmodconfig with W=1, plus the same change on top of v7.1.6 built as a module for the affected BCM57504 host). The aborted reset path itself only runs when the firmware fails to return from a reset, which could not be reproduced on demand. Fixes: 004b5008016a ("eth: bnxt: remove most dependencies on RTNL") Cc: <redacted> Signed-off-by: Bryan Cochrane <redacted> --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 0682060b048c..0f37639b70ba 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c@@ -14798,7 +14798,7 @@ static int bnxt_get_registered_vfs(struct bnxt *bp) void bnxt_fw_reset(struct bnxt *bp) { bnxt_ulp_stop(bp); - bnxt_lock_sp(bp); + bnxt_rtnl_lock_sp(bp); if (test_bit(BNXT_STATE_OPEN, &bp->state) && !test_bit(BNXT_STATE_IN_FW_RESET, &bp->state)) { struct bnxt_ptp_cfg *ptp = bp->ptp_cfg;
@@ -14844,7 +14844,7 @@ void bnxt_fw_reset(struct bnxt *bp) bnxt_queue_fw_reset_work(bp, tmo); } fw_reset_exit: - bnxt_unlock_sp(bp); + bnxt_rtnl_unlock_sp(bp); } static void bnxt_chk_missed_irq(struct bnxt *bp)
@@ -15495,9 +15495,8 @@ static void bnxt_fw_reset_task(struct work_struct *work) bp->fw_reset_timestamp = jiffies; netdev_lock(bp->dev); if (test_bit(BNXT_STATE_ABORT_ERR, &bp->state)) { - bnxt_fw_reset_abort(bp, rc); netdev_unlock(bp->dev); - goto ulp_start; + goto fw_reset_abort; } bnxt_fw_reset_close(bp); if (bp->fw_cap & BNXT_FW_CAP_ERR_RECOVER_RELOAD) {
@@ -15630,9 +15629,11 @@ static void bnxt_fw_reset_task(struct work_struct *work) netdev_err(bp->dev, "fw_health_status 0x%x\n", sts); } fw_reset_abort: + rtnl_lock(); netdev_lock(bp->dev); bnxt_fw_reset_abort(bp, rc); netdev_unlock(bp->dev); + rtnl_unlock(); ulp_start: if (!rc) bnxt_ulp_start(bp);
base-commit: fcc00b15cb21a6de76826e5012b2c758505f5e65 -- 2.56.0.313.g2479322e68-goog