Commit bf83b257423b for kernel
commit bf83b257423bb557b970df3d7ece32fbd85f3bd4
Author: Bryan Cochrane <bcochrane@google.com>
Date: Mon Nov 9 20:42:35 2026 -0800
bnxt_en: Take RTNL before netif_close() when aborting FW reset
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(), only a PF can reach the bnxt_get_registered_vfs()
error path that calls netif_close(), so take RTNL up front via
bnxt_rtnl_lock_sp() when BNXT_PF(bp) is true and keep bnxt_lock_sp() on
VFs. That avoids dropping the instance lock mid-reset (which would
expose a transient BNXT_STATE_IN_FW_RESET and let a concurrent devlink
health recover set BNXT_STATE_IN_SP_TASK), avoids racing with
bnxt_sriov_configure() updating active_vfs outside RTNL, and keeps VFs
from contending on RTNL during recovery while PFs already run on the
single-threaded bnxt_pf_wq.
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: stable@vger.kernel.org
Signed-off-by: Bryan Cochrane <bcochrane@google.com>
Link: https://patch.msgid.link/20261110044235.204386-1-bcochrane@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 33c4cacc8ffe..8b7ebf2eefaa 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -14838,7 +14838,10 @@ static int bnxt_get_registered_vfs(struct bnxt *bp)
void bnxt_fw_reset(struct bnxt *bp)
{
bnxt_ulp_stop(bp);
- bnxt_lock_sp(bp);
+ if (BNXT_PF(bp))
+ bnxt_rtnl_lock_sp(bp);
+ else
+ bnxt_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;
@@ -14884,7 +14887,10 @@ void bnxt_fw_reset(struct bnxt *bp)
bnxt_queue_fw_reset_work(bp, tmo);
}
fw_reset_exit:
- bnxt_unlock_sp(bp);
+ if (BNXT_PF(bp))
+ bnxt_rtnl_unlock_sp(bp);
+ else
+ bnxt_unlock_sp(bp);
}
static void bnxt_chk_missed_irq(struct bnxt *bp)
@@ -15547,9 +15553,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) {
@@ -15682,9 +15687,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);