* [PATCH net-next v2 1/6] bnge: Extract bnapi allocation and cleanup into helpers
2026-09-22 5:28 [PATCH net-next v2 0/6] bnge: Support async events while down Bhargava Marreddy
@ 2026-09-22 5:28 ` Bhargava Marreddy
2026-09-24 17:07 ` netdev-bot+sashiko
2026-09-22 5:29 ` [PATCH net-next v2 2/6] bnge: Extract per-NQ ring allocation and init helpers Bhargava Marreddy
` (4 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Bhargava Marreddy @ 2026-09-22 5:28 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
Cc: netdev, linux-kernel, michael.chan, pavan.chebbi,
vsrama-krishna.nemani, vikas.gupta, Bhargava Marreddy,
Rajashekar Hudumula
Factor bn->bnapi allocation and free logic out of bnge_{alloc,free}_core()
into new bnge_{alloc,free}_bnapi_mem() helpers.
Prepare the driver to move bnapi allocation out of the open/close path,
and add NULL guards for rx_ring and tx_ring in core driver cleanup paths.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
---
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 38 ++++++++++++++++---
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index a4288f0258f8..c1810a309888 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -825,6 +825,9 @@ static void bnge_free_tpa_info(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i, j;
+ if (!bn->rx_ring)
+ return;
+
for (i = 0; i < bd->rx_nr_rings; i++) {
struct bnge_rx_ring_info *rxr = &bn->rx_ring[i];
@@ -881,6 +884,9 @@ static void bnge_free_rx_rings(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i;
+ if (!bn->rx_ring)
+ return;
+
bnge_free_tpa_info(bn);
for (i = 0; i < bd->rx_nr_rings; i++) {
struct bnge_rx_ring_info *rxr = &bn->rx_ring[i];
@@ -1024,6 +1030,9 @@ static void bnge_free_tx_rings(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i;
+ if (!bn->tx_ring)
+ return;
+
for (i = 0; i < bd->tx_nr_rings; i++) {
struct bnge_tx_ring_info *txr = &bn->tx_ring[i];
struct bnge_ring_struct *ring;
@@ -1195,6 +1204,12 @@ static int bnge_init_ring_grps(struct bnge_net *bn)
return 0;
}
+static void bnge_free_bnapi_mem(struct bnge_net *bn)
+{
+ kfree(bn->bnapi);
+ bn->bnapi = NULL;
+}
+
static void bnge_free_core(struct bnge_net *bn)
{
bnge_free_vnic_attributes(bn);
@@ -1211,15 +1226,13 @@ static void bnge_free_core(struct bnge_net *bn)
bn->tx_ring = NULL;
kfree(bn->rx_ring);
bn->rx_ring = NULL;
- kfree(bn->bnapi);
- bn->bnapi = NULL;
+ bnge_free_bnapi_mem(bn);
}
-static int bnge_alloc_core(struct bnge_net *bn)
+static int bnge_alloc_bnapi_mem(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
- int i, j, size, arr_size;
- int rc = -ENOMEM;
+ int i, size, arr_size;
void *bnapi;
arr_size = L1_CACHE_ALIGN(sizeof(struct bnge_napi *) *
@@ -1227,7 +1240,7 @@ static int bnge_alloc_core(struct bnge_net *bn)
size = L1_CACHE_ALIGN(sizeof(struct bnge_napi));
bnapi = kzalloc(arr_size + size * bd->nq_nr_rings, GFP_KERNEL);
if (!bnapi)
- return rc;
+ return -ENOMEM;
bn->bnapi = bnapi;
bnapi += arr_size;
@@ -1241,6 +1254,19 @@ static int bnge_alloc_core(struct bnge_net *bn)
nqr->ring_struct.ring_mem.flags = BNGE_RMEM_RING_PTE_FLAG;
}
+ return 0;
+}
+
+static int bnge_alloc_core(struct bnge_net *bn)
+{
+ struct bnge_dev *bd = bn->bd;
+ int i, j, rc;
+
+ rc = bnge_alloc_bnapi_mem(bn);
+ if (rc)
+ return rc;
+
+ rc = -ENOMEM;
bn->rx_ring = kzalloc_objs(struct bnge_rx_ring_info, bd->rx_nr_rings);
if (!bn->rx_ring)
goto err_free_core;
--
2.47.3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next v2 1/6] bnge: Extract bnapi allocation and cleanup into helpers
2026-09-22 5:28 ` [PATCH net-next v2 1/6] bnge: Extract bnapi allocation and cleanup into helpers Bhargava Marreddy
@ 2026-09-24 17:07 ` netdev-bot+sashiko
0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-24 17:07 UTC (permalink / raw)
To: bhargava.marreddy
Cc: davem, edumazet, kuba, pabeni, andrew+netdev, horms, netdev,
linux-kernel, michael.chan, pavan.chebbi, vsrama-krishna.nemani,
vikas.gupta, dharmender.garg
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The commit message describes the three new NULL guards as
preparatory hardening ("add NULL guards for rx_ring and tx_ring in…
--
bnge: Extract bnapi allocation and cleanup into helpers
This moves the bn->bnapi allocation and free code out of
bnge_alloc_core()/bnge_free_core() into bnge_alloc_bnapi_mem() and
bnge_free_bnapi_mem(), and adds NULL checks for bn->rx_ring and
bn->tx_ring in the ring cleanup helpers.
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> index a4288f0258f87..c1810a3098883 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> @@ -825,6 +825,9 @@ static void bnge_free_tpa_info(struct bnge_net *bn)
> struct bnge_dev *bd = bn->bd;
> int i, j;
>
> + if (!bn->rx_ring)
> + return;
> +
> for (i = 0; i < bd->rx_nr_rings; i++) {
> struct bnge_rx_ring_info *rxr = &bn->rx_ring[i];
>
> @@ -881,6 +884,9 @@ static void bnge_free_rx_rings(struct bnge_net *bn)
> struct bnge_dev *bd = bn->bd;
> int i;
>
> + if (!bn->rx_ring)
> + return;
> +
> bnge_free_tpa_info(bn);
> for (i = 0; i < bd->rx_nr_rings; i++) {
> struct bnge_rx_ring_info *rxr = &bn->rx_ring[i];
> @@ -1024,6 +1030,9 @@ static void bnge_free_tx_rings(struct bnge_net *bn)
> struct bnge_dev *bd = bn->bd;
> int i;
>
> + if (!bn->tx_ring)
> + return;
> +
[Severity: Medium]
Are these three new NULL checks really just preparation, or do they fix a
reachable NULL pointer dereference that exists today?
The commit message says only:
Prepare the driver to move bnapi allocation out of the open/close path,
and add NULL guards for rx_ring and tx_ring in core driver cleanup paths.
Before this patch, bnge_alloc_core() allocates bn->bnapi, then bn->rx_ring,
then bn->tx_ring, and every failure goes to err_free_core: which calls
bnge_free_core(). bnge_free_core() unconditionally calls
bnge_free_tx_rings() and bnge_free_rx_rings(), so if the bn->rx_ring
allocation fails both pointers are still NULL:
drivers/net/ethernet/broadcom/bnge/bnge_netdev.c:bnge_free_tx_rings() {
for (i = 0; i < bd->tx_nr_rings; i++) {
struct bnge_tx_ring_info *txr = &bn->tx_ring[i];
...
bnge_free_ring(bd, &ring->ring_mem);
}
and bnge_free_ring() dereferences that near-NULL address right away:
drivers/net/ethernet/broadcom/bnge/bnge_rmem.c:bnge_free_ring() {
if (!rmem->pg_arr)
goto skip_pages;
The rx side looks the same: bnge_free_rx_rings() -> bnge_free_tpa_info()
reads rxr->rx_tpa_idx_map, and bnge_free_rx_rings() itself calls
page_pool_destroy(rxr->page_pool), both off a NULL bn->rx_ring.
Since bnge_alloc_core() runs from the ndo_open path, an allocation failure
during "ip link set dev ethX up" would oops instead of returning -ENOMEM.
git blame points the affected loops at commit 0259379037cab9 ("bng_en: Add
initial support for RX and TX rings"). Would it make sense to split these
guards into their own patch with a Fixes: 0259379037cab9 tag, so it can be
picked up for stable? No commit in this series (10cfa109c8..63494c308b19)
carries such a tag or mentions the crash.
Note that bnge_free_rx_ring_pair_bufs() already had the same
if (!bn->rx_ring) return; check, which suggests the guard is functional
rather than cosmetic.
> for (i = 0; i < bd->tx_nr_rings; i++) {
> struct bnge_tx_ring_info *txr = &bn->tx_ring[i];
> struct bnge_ring_struct *ring;
[ ... ]
> @@ -1241,6 +1254,19 @@ static int bnge_alloc_core(struct bnge_net *bn)
> nqr->ring_struct.ring_mem.flags = BNGE_RMEM_RING_PTE_FLAG;
> }
>
> + return 0;
> +}
> +
> +static int bnge_alloc_core(struct bnge_net *bn)
> +{
> + struct bnge_dev *bd = bn->bd;
> + int i, j, rc;
> +
> + rc = bnge_alloc_bnapi_mem(bn);
> + if (rc)
> + return rc;
> +
> + rc = -ENOMEM;
> bn->rx_ring = kzalloc_objs(struct bnge_rx_ring_info, bd->rx_nr_rings);
> if (!bn->rx_ring)
> goto err_free_core;
This is the branch that reaches bnge_free_core() with both bn->rx_ring and
bn->tx_ring still NULL.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922052904.59237-1-bhargava.marreddy%40broadcom.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v2 2/6] bnge: Extract per-NQ ring allocation and init helpers
2026-09-22 5:28 [PATCH net-next v2 0/6] bnge: Support async events while down Bhargava Marreddy
2026-09-22 5:28 ` [PATCH net-next v2 1/6] bnge: Extract bnapi allocation and cleanup into helpers Bhargava Marreddy
@ 2026-09-22 5:29 ` Bhargava Marreddy
2026-09-22 5:29 ` [PATCH net-next v2 3/6] bnge: Move bnapi and ring_grp allocation to probe/remove Bhargava Marreddy
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-09-22 5:29 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
Cc: netdev, linux-kernel, michael.chan, pavan.chebbi,
vsrama-krishna.nemani, vikas.gupta, Bhargava Marreddy,
Dharmender Garg, Akhilesh Samineni
Factor the per-NQ loop body in bnge_hwrm_ring_alloc() and
bnge_init_ring_struct() into standalone bnge_hwrm_nq_ring_alloc() and
bnge_init_nq_ring_struct() helpers.
This is a pure refactor with no functional changes, preparing NQ0 setup
to run independently during probe rather than through the open path loop.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>
---
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 60 ++++++++++++-------
.../net/ethernet/broadcom/bnge/bnge_rmem.c | 25 +++++---
.../net/ethernet/broadcom/bnge/bnge_rmem.h | 4 +-
3 files changed, 56 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index c1810a309888..dfd68b830ded 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -1918,6 +1918,41 @@ static int bnge_hwrm_rx_ring_alloc(struct bnge_net *bn,
return 0;
}
+static int bnge_hwrm_nq_ring_alloc(struct bnge_net *bn, int index)
+{
+ struct bnge_napi *bnapi = bn->bnapi[index];
+ struct bnge_nq_ring_info *nqr;
+ struct bnge_ring_struct *ring;
+ u32 type = HWRM_RING_ALLOC_NQ;
+ struct bnge_dev *bd = bn->bd;
+ unsigned int vector;
+ u32 map_idx;
+ int rc;
+
+ nqr = &bnapi->nq_ring;
+ ring = &nqr->ring_struct;
+ map_idx = ring->map_idx;
+ vector = bd->irq_tbl[map_idx].vector;
+
+ disable_irq_nosync(vector);
+ rc = hwrm_ring_alloc_send_msg(bn, ring, type, map_idx);
+ if (rc) {
+ enable_irq(vector);
+ return rc;
+ }
+ bnge_set_db(bn, &nqr->nq_db, type, map_idx, ring->fw_ring_id);
+ bnge_db_nq(bn, &nqr->nq_db, nqr->nq_raw_cons);
+ enable_irq(vector);
+ bn->grp_info[index].nq_fw_ring_id = (u16)ring->fw_ring_id;
+ if (!index) {
+ rc = bnge_hwrm_set_async_event_cr(bd, ring->fw_ring_id);
+ if (rc)
+ netdev_warn(bn->netdev, "Failed to set async event completion ring.\n");
+ }
+
+ return 0;
+}
+
static int bnge_hwrm_ring_alloc(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
@@ -1926,30 +1961,9 @@ static int bnge_hwrm_ring_alloc(struct bnge_net *bn)
agg_rings = !!(bnge_is_agg_reqd(bd));
for (i = 0; i < bd->nq_nr_rings; i++) {
- struct bnge_napi *bnapi = bn->bnapi[i];
- struct bnge_nq_ring_info *nqr = &bnapi->nq_ring;
- struct bnge_ring_struct *ring = &nqr->ring_struct;
- u32 type = HWRM_RING_ALLOC_NQ;
- u32 map_idx = ring->map_idx;
- unsigned int vector;
-
- vector = bd->irq_tbl[map_idx].vector;
- disable_irq_nosync(vector);
- rc = hwrm_ring_alloc_send_msg(bn, ring, type, map_idx);
- if (rc) {
- enable_irq(vector);
+ rc = bnge_hwrm_nq_ring_alloc(bn, i);
+ if (rc)
goto err_out;
- }
- bnge_set_db(bn, &nqr->nq_db, type, map_idx, ring->fw_ring_id);
- bnge_db_nq(bn, &nqr->nq_db, nqr->nq_raw_cons);
- enable_irq(vector);
- bn->grp_info[i].nq_fw_ring_id = (u16)ring->fw_ring_id;
-
- if (!i) {
- rc = bnge_hwrm_set_async_event_cr(bd, ring->fw_ring_id);
- if (rc)
- netdev_warn(bn->netdev, "Failed to set async event completion ring.\n");
- }
}
for (i = 0; i < bd->tx_nr_rings; i++) {
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
index b066ee887a09..e0ddb2800c54 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
@@ -423,6 +423,21 @@ int bnge_alloc_ctx_mem(struct bnge_dev *bd)
return 0;
}
+void bnge_init_nq_ring_struct(struct bnge_net *bn,
+ struct bnge_nq_ring_info *nqr)
+{
+ struct bnge_ring_mem_info *rmem;
+ struct bnge_ring_struct *ring;
+
+ ring = &nqr->ring_struct;
+ rmem = &ring->ring_mem;
+ rmem->nr_pages = bn->cp_nr_pages;
+ rmem->page_size = HW_CMPD_RING_SIZE;
+ rmem->pg_arr = (void **)nqr->desc_ring;
+ rmem->dma_arr = nqr->desc_mapping;
+ rmem->vmem_size = 0;
+}
+
void bnge_init_ring_struct(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
@@ -431,19 +446,11 @@ void bnge_init_ring_struct(struct bnge_net *bn)
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_napi *bnapi = bn->bnapi[i];
struct bnge_ring_mem_info *rmem;
- struct bnge_nq_ring_info *nqr;
struct bnge_rx_ring_info *rxr;
struct bnge_tx_ring_info *txr;
struct bnge_ring_struct *ring;
- nqr = &bnapi->nq_ring;
- ring = &nqr->ring_struct;
- rmem = &ring->ring_mem;
- rmem->nr_pages = bn->cp_nr_pages;
- rmem->page_size = HW_CMPD_RING_SIZE;
- rmem->pg_arr = (void **)nqr->desc_ring;
- rmem->dma_arr = nqr->desc_mapping;
- rmem->vmem_size = 0;
+ bnge_init_nq_ring_struct(bn, &bnapi->nq_ring);
rxr = bnapi->rx_ring;
if (!rxr)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.h b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.h
index bb0c79a1ee60..a5a1d346ac93 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.h
@@ -7,6 +7,7 @@
struct bnge_ctx_mem_type;
struct bnge_dev;
struct bnge_net;
+struct bnge_nq_ring_info;
#define PTU_PTE_VALID 0x1UL
#define PTU_PTE_LAST 0x2UL
@@ -198,5 +199,6 @@ void bnge_free_ring(struct bnge_dev *bd, struct bnge_ring_mem_info *rmem);
int bnge_alloc_ctx_mem(struct bnge_dev *bd);
void bnge_free_ctx_mem(struct bnge_dev *bd);
void bnge_init_ring_struct(struct bnge_net *bn);
-
+void bnge_init_nq_ring_struct(struct bnge_net *bn,
+ struct bnge_nq_ring_info *nqr);
#endif /* _BNGE_RMEM_H_ */
--
2.47.3
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH net-next v2 3/6] bnge: Move bnapi and ring_grp allocation to probe/remove
2026-09-22 5:28 [PATCH net-next v2 0/6] bnge: Support async events while down Bhargava Marreddy
2026-09-22 5:28 ` [PATCH net-next v2 1/6] bnge: Extract bnapi allocation and cleanup into helpers Bhargava Marreddy
2026-09-22 5:29 ` [PATCH net-next v2 2/6] bnge: Extract per-NQ ring allocation and init helpers Bhargava Marreddy
@ 2026-09-22 5:29 ` Bhargava Marreddy
2026-09-22 5:29 ` [PATCH net-next v2 4/6] bnge: Drop obsolete bn->bnapi NULL checks in open/close paths Bhargava Marreddy
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-09-22 5:29 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
Cc: netdev, linux-kernel, michael.chan, pavan.chebbi,
vsrama-krishna.nemani, vikas.gupta, Bhargava Marreddy,
Rajashekar Hudumula, Dharmender Garg, Kiran Kella
Allocate bn->bnapi and bn->grp_info during probe and free them during
remove instead of reallocating on every open/close.
To support this shift:
- Pass an irq_re_init flag to bnge_init_ring_grps() so open can reset
per-group state without reallocating bn->grp_info.
- Have bnge_free_core() clear rx_ring/tx_ring back-pointers via the new
bnge_clear_bnapi_queues() rather than freeing bn->bnapi.
- Replace "!bn->bnapi" checks in bnge_get_queue_stats_rx() and
bnge_get_ethtool_stats() with !netif_running(dev) checks to avoid
reading sw_stats memory freed on close.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Kiran Kella <kiran.kella@broadcom.com>
---
.../net/ethernet/broadcom/bnge/bnge_ethtool.c | 2 +-
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 107 ++++++++++++------
2 files changed, 71 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_ethtool.c b/drivers/net/ethernet/broadcom/bnge/bnge_ethtool.c
index 2467e44de291..ad9956bc4408 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_ethtool.c
@@ -346,7 +346,7 @@ static void bnge_get_ethtool_stats(struct net_device *dev,
u32 tpa_stats;
u32 i, j = 0;
- if (!bn->bnapi) {
+ if (!netif_running(dev)) {
j += bnge_get_num_ring_stats(bd);
goto skip_ring_stats;
}
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index dfd68b830ded..367109dad496 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -1185,14 +1185,18 @@ static void bnge_free_ring_grps(struct bnge_net *bn)
bn->grp_info = NULL;
}
-static int bnge_init_ring_grps(struct bnge_net *bn)
+static int bnge_init_ring_grps(struct bnge_net *bn, bool irq_re_init)
{
struct bnge_dev *bd = bn->bd;
int i;
- bn->grp_info = kzalloc_objs(struct bnge_ring_grp_info, bd->nq_nr_rings);
- if (!bn->grp_info)
- return -ENOMEM;
+ if (irq_re_init) {
+ bn->grp_info = kzalloc_objs(struct bnge_ring_grp_info,
+ bd->nq_nr_rings);
+ if (!bn->grp_info)
+ return -ENOMEM;
+ }
+
for (i = 0; i < bd->nq_nr_rings; i++) {
bn->grp_info[i].fw_stats_ctx = INVALID_HW_RING_ID;
bn->grp_info[i].fw_grp_id = INVALID_HW_RING_ID;
@@ -1210,25 +1214,6 @@ static void bnge_free_bnapi_mem(struct bnge_net *bn)
bn->bnapi = NULL;
}
-static void bnge_free_core(struct bnge_net *bn)
-{
- bnge_free_vnic_attributes(bn);
- bnge_free_tx_rings(bn);
- bnge_free_rx_rings(bn);
- bnge_free_nq_tree(bn);
- bnge_free_nq_arrays(bn);
- bnge_free_ring_stats(bn);
- bnge_free_ring_grps(bn);
- bnge_free_vnics(bn);
- kfree(bn->tx_ring_map);
- bn->tx_ring_map = NULL;
- kfree(bn->tx_ring);
- bn->tx_ring = NULL;
- kfree(bn->rx_ring);
- bn->rx_ring = NULL;
- bnge_free_bnapi_mem(bn);
-}
-
static int bnge_alloc_bnapi_mem(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
@@ -1257,16 +1242,52 @@ static int bnge_alloc_bnapi_mem(struct bnge_net *bn)
return 0;
}
-static int bnge_alloc_core(struct bnge_net *bn)
+static void bnge_clear_bnapi_queues(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
- int i, j, rc;
+ int i;
- rc = bnge_alloc_bnapi_mem(bn);
- if (rc)
- return rc;
+ if (!bn->bnapi)
+ return;
+
+ for (i = 0; i < bd->nq_nr_rings; i++) {
+ struct bnge_napi *bnapi = bn->bnapi[i];
+ int j;
+
+ if (!bnapi)
+ continue;
+
+ bnapi->rx_ring = NULL;
+ for (j = 0; j < BNGE_MAX_TXR_PER_NAPI; j++)
+ bnapi->tx_ring[j] = NULL;
+ }
+}
+
+static void bnge_free_core(struct bnge_net *bn)
+{
+ bnge_free_vnic_attributes(bn);
+ bnge_free_tx_rings(bn);
+ bnge_free_rx_rings(bn);
+ bnge_free_nq_tree(bn);
+ bnge_free_nq_arrays(bn);
+ bnge_free_ring_stats(bn);
+ bnge_free_vnics(bn);
+
+ kfree(bn->tx_ring_map);
+ bn->tx_ring_map = NULL;
+ kfree(bn->tx_ring);
+ bn->tx_ring = NULL;
+ kfree(bn->rx_ring);
+ bn->rx_ring = NULL;
+
+ bnge_clear_bnapi_queues(bn);
+}
+
+static int bnge_alloc_core(struct bnge_net *bn)
+{
+ struct bnge_dev *bd = bn->bd;
+ int i, j, rc = -ENOMEM;
- rc = -ENOMEM;
bn->rx_ring = kzalloc_objs(struct bnge_rx_ring_info, bd->rx_nr_rings);
if (!bn->rx_ring)
goto err_free_core;
@@ -1402,6 +1423,7 @@ static void bnge_init_nq_tree(struct bnge_net *bn)
struct bnge_nq_ring_info *nqr = &bn->bnapi[i]->nq_ring;
struct bnge_ring_struct *ring = &nqr->ring_struct;
+ nqr->nq_raw_cons = 0;
ring->fw_ring_id = INVALID_HW_RING_ID_32BIT;
for (j = 0; j < nqr->cp_ring_count; j++) {
struct bnge_cp_ring_info *cpr = &nqr->cp_ring_arr[j];
@@ -2847,19 +2869,15 @@ static int bnge_init_nic(struct bnge_net *bn)
bnge_init_tx_rings(bn);
- rc = bnge_init_ring_grps(bn);
- if (rc)
- goto err_free_rx_ring_pair_bufs;
+ bnge_init_ring_grps(bn, false);
bnge_init_vnics(bn);
rc = bnge_init_chip(bn);
if (rc)
- goto err_free_ring_grps;
+ goto err_free_rx_ring_pair_bufs;
return rc;
-err_free_ring_grps:
- bnge_free_ring_grps(bn);
err_free_rx_ring_pair_bufs:
bnge_free_rx_ring_pair_bufs(bn);
return rc;
@@ -3235,7 +3253,7 @@ static void bnge_get_queue_stats_rx(struct net_device *dev, int i,
struct bnge_nq_ring_info *nqr;
u64 *sw;
- if (!bn->bnapi)
+ if (!netif_running(dev))
return;
nqr = &bn->bnapi[i]->nq_ring;
@@ -3557,14 +3575,27 @@ int bnge_netdev_alloc(struct bnge_dev *bd, int max_irqs)
spin_lock_init(&bn->stats_lock);
netdev->request_ops_lock = true;
+
+ rc = bnge_alloc_bnapi_mem(bn);
+ if (rc)
+ goto err_free_port_stats;
+
+ rc = bnge_init_ring_grps(bn, true);
+ if (rc)
+ goto err_free_bnapi_mem;
+
rc = register_netdev(netdev);
if (rc) {
dev_err(bd->dev, "Register netdev failed rc: %d\n", rc);
- goto err_free_port_stats;
+ goto err_free_ring_grps;
}
return 0;
+err_free_ring_grps:
+ bnge_free_ring_grps(bn);
+err_free_bnapi_mem:
+ bnge_free_bnapi_mem(bn);
err_free_port_stats:
bnge_free_port_stats(bn);
err_free_workq:
@@ -3589,6 +3620,8 @@ void bnge_netdev_free(struct bnge_dev *bd)
destroy_workqueue(bn->bnge_pf_wq);
bnge_free_port_stats(bn);
+ bnge_free_ring_grps(bn);
+ bnge_free_bnapi_mem(bn);
free_netdev(netdev);
bd->netdev = NULL;
--
2.47.3
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH net-next v2 4/6] bnge: Drop obsolete bn->bnapi NULL checks in open/close paths
2026-09-22 5:28 [PATCH net-next v2 0/6] bnge: Support async events while down Bhargava Marreddy
` (2 preceding siblings ...)
2026-09-22 5:29 ` [PATCH net-next v2 3/6] bnge: Move bnapi and ring_grp allocation to probe/remove Bhargava Marreddy
@ 2026-09-22 5:29 ` Bhargava Marreddy
2026-09-22 5:29 ` [PATCH net-next v2 5/6] bnge: Quiesce NQ0 around ring teardown in bnge_free_core() Bhargava Marreddy
2026-09-22 5:29 ` [PATCH net-next v2 6/6] bnge: Create NQ0 during probe and keep active across open/close Bhargava Marreddy
5 siblings, 0 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-09-22 5:29 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
Cc: netdev, linux-kernel, michael.chan, pavan.chebbi,
vsrama-krishna.nemani, vikas.gupta, Bhargava Marreddy,
Rajashekar Hudumula, Dharmender Garg
Since bn->bnapi is now allocated during probe in bnge_netdev_alloc() and
freed in bnge_netdev_free(), it remains non-NULL for the lifetime of the
open/close path helpers.
Remove the dead "if (!bn->bnapi)" guards in bnge_free_ring_stats(),
bnge_clear_bnapi_queues(), bnge_disable_int(), and bnge_hwrm_ring_free().
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
---
drivers/net/ethernet/broadcom/bnge/bnge_netdev.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index 367109dad496..9e606fd1a580 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -85,9 +85,6 @@ static void bnge_free_ring_stats(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i;
- if (!bn->bnapi)
- return;
-
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_napi *bnapi = bn->bnapi[i];
struct bnge_nq_ring_info *nqr = &bnapi->nq_ring;
@@ -1247,9 +1244,6 @@ static void bnge_clear_bnapi_queues(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i;
- if (!bn->bnapi)
- return;
-
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_napi *bnapi = bn->bnapi[i];
int j;
@@ -2382,9 +2376,6 @@ static void bnge_disable_int(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i;
- if (!bn->bnapi)
- return;
-
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_napi *bnapi = bn->bnapi[i];
struct bnge_nq_ring_info *nqr;
@@ -2576,9 +2567,6 @@ static void bnge_hwrm_ring_free(struct bnge_net *bn, bool close_path)
struct bnge_dev *bd = bn->bd;
int i;
- if (!bn->bnapi)
- return;
-
for (i = 0; i < bd->tx_nr_rings; i++)
bnge_hwrm_tx_ring_free(bn, &bn->tx_ring[i], close_path);
--
2.47.3
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH net-next v2 5/6] bnge: Quiesce NQ0 around ring teardown in bnge_free_core()
2026-09-22 5:28 [PATCH net-next v2 0/6] bnge: Support async events while down Bhargava Marreddy
` (3 preceding siblings ...)
2026-09-22 5:29 ` [PATCH net-next v2 4/6] bnge: Drop obsolete bn->bnapi NULL checks in open/close paths Bhargava Marreddy
@ 2026-09-22 5:29 ` Bhargava Marreddy
2026-09-22 5:29 ` [PATCH net-next v2 6/6] bnge: Create NQ0 during probe and keep active across open/close Bhargava Marreddy
5 siblings, 0 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-09-22 5:29 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
Cc: netdev, linux-kernel, michael.chan, pavan.chebbi,
vsrama-krishna.nemani, vikas.gupta, Bhargava Marreddy,
Rajashekar Hudumula
Add bnge_quiesce_nq0() and bnge_resume_nq0() helpers to temporarily disable
and re-enable NQ0 NAPI. Use these helpers to bracket ring teardown in
bnge_free_core() and ring rebuild in bnge_alloc_core().
Both helpers currently no-op, since no bnge_napi has BNGE_NAPI_NQ0 set
yet. The next patch creates NQ0 during probe and sets this flag, at
which point these helpers become load-bearing: because NQ0 will remain
active while administratively down, its NAPI is not disabled by the
standard close paths, so temporarily quiescing it during core resource
free/rebuild windows is required to prevent stray completions from
accessing uninitialized or freed memory.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
---
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 76 ++++++++++++++-----
.../net/ethernet/broadcom/bnge/bnge_netdev.h | 11 +++
2 files changed, 68 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index 9e606fd1a580..1c12282d7e88 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -442,6 +442,25 @@ static void bnge_sp_task(struct work_struct *work)
netdev_unlock(bn->netdev);
}
+static void bnge_db_nq_arm(struct bnge_net *bn,
+ struct bnge_db_info *db, u32 idx)
+{
+ bnge_writeq(bn->bd, db->db_key64 | DBR_TYPE_NQ_ARM |
+ DB_RING_IDX(db, idx), db->doorbell);
+}
+
+static void bnge_db_nq(struct bnge_net *bn, struct bnge_db_info *db, u32 idx)
+{
+ bnge_writeq(bn->bd, db->db_key64 | DBR_TYPE_NQ_MASK |
+ DB_RING_IDX(db, idx), db->doorbell);
+}
+
+static void bnge_db_cq(struct bnge_net *bn, struct bnge_db_info *db, u32 idx)
+{
+ bnge_writeq(bn->bd, db->db_key64 | DBR_TYPE_CQ_ARMALL |
+ DB_RING_IDX(db, idx), db->doorbell);
+}
+
static void bnge_free_nq_desc_arr(struct bnge_nq_ring_info *nqr)
{
struct bnge_ring_struct *ring = &nqr->ring_struct;
@@ -563,6 +582,40 @@ static void bnge_free_nq_tree(struct bnge_net *bn)
}
}
+static void bnge_quiesce_nq0(struct bnge_net *bn)
+{
+ struct bnge_napi *bnapi = bn->bnapi[BNGE_NQ0_NAPI_IDX];
+ struct bnge_nq_ring_info *nqr = &bnapi->nq_ring;
+ struct bnge_ring_struct *ring;
+ struct bnge_dev *bd = bn->bd;
+
+ if (!BNGE_NQ0_NAPI(bnapi))
+ return;
+
+ if (test_and_set_bit(BNGE_NAPI_QUIESCED, &bnapi->flags))
+ return;
+
+ ring = &nqr->ring_struct;
+ napi_disable_locked(&bnapi->napi);
+ synchronize_irq(bd->irq_tbl[ring->map_idx].vector);
+ bnge_db_nq(bn, &nqr->nq_db, nqr->nq_raw_cons);
+}
+
+static void bnge_resume_nq0(struct bnge_net *bn)
+{
+ struct bnge_napi *bnapi = bn->bnapi[BNGE_NQ0_NAPI_IDX];
+ struct bnge_nq_ring_info *nqr = &bnapi->nq_ring;
+
+ if (!BNGE_NQ0_NAPI(bnapi))
+ return;
+
+ if (!test_and_clear_bit(BNGE_NAPI_QUIESCED, &bnapi->flags))
+ return;
+
+ napi_enable_locked(&bnapi->napi);
+ bnge_db_nq_arm(bn, &nqr->nq_db, nqr->nq_raw_cons);
+}
+
static int alloc_one_cp_ring(struct bnge_net *bn,
struct bnge_cp_ring_info *cpr)
{
@@ -1260,6 +1313,7 @@ static void bnge_clear_bnapi_queues(struct bnge_net *bn)
static void bnge_free_core(struct bnge_net *bn)
{
bnge_free_vnic_attributes(bn);
+ bnge_quiesce_nq0(bn);
bnge_free_tx_rings(bn);
bnge_free_rx_rings(bn);
bnge_free_nq_tree(bn);
@@ -1275,6 +1329,7 @@ static void bnge_free_core(struct bnge_net *bn)
bn->rx_ring = NULL;
bnge_clear_bnapi_queues(bn);
+ bnge_resume_nq0(bn);
}
static int bnge_alloc_core(struct bnge_net *bn)
@@ -1352,10 +1407,12 @@ static int bnge_alloc_core(struct bnge_net *bn)
if (rc)
goto err_free_core;
+ bnge_quiesce_nq0(bn);
rc = bnge_alloc_nq_tree(bn);
if (rc)
goto err_free_core;
+ bnge_resume_nq0(bn);
bn->vnic_info[BNGE_VNIC_DEFAULT].flags |= BNGE_VNIC_RSS_FLAG |
BNGE_VNIC_MCAST_FLAG |
BNGE_VNIC_UCAST_FLAG;
@@ -1379,25 +1436,6 @@ u32 bnge_cp_ring_for_tx(struct bnge_tx_ring_info *txr)
return txr->tx_cpr->ring_struct.fw_ring_id;
}
-static void bnge_db_nq_arm(struct bnge_net *bn,
- struct bnge_db_info *db, u32 idx)
-{
- bnge_writeq(bn->bd, db->db_key64 | DBR_TYPE_NQ_ARM |
- DB_RING_IDX(db, idx), db->doorbell);
-}
-
-static void bnge_db_nq(struct bnge_net *bn, struct bnge_db_info *db, u32 idx)
-{
- bnge_writeq(bn->bd, db->db_key64 | DBR_TYPE_NQ_MASK |
- DB_RING_IDX(db, idx), db->doorbell);
-}
-
-static void bnge_db_cq(struct bnge_net *bn, struct bnge_db_info *db, u32 idx)
-{
- bnge_writeq(bn->bd, db->db_key64 | DBR_TYPE_CQ_ARMALL |
- DB_RING_IDX(db, idx), db->doorbell);
-}
-
static int bnge_cp_num_to_irq_num(struct bnge_net *bn, int n)
{
struct bnge_napi *bnapi = bn->bnapi[n];
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.h b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.h
index 476b5bab96fe..d6e65fc7d038 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.h
@@ -539,6 +539,15 @@ struct bnge_tx_ring_info {
struct bnge_ring_struct tx_ring_struct;
};
+#define BNGE_NQ0_NAPI_IDX 0
+
+enum bnge_napi_flag {
+ BNGE_NAPI_NQ0,
+ BNGE_NAPI_QUIESCED,
+};
+
+#define BNGE_NQ0_NAPI(bnapi) (test_bit(BNGE_NAPI_NQ0, &(bnapi)->flags))
+
struct bnge_napi {
struct napi_struct napi;
struct bnge_net *bn;
@@ -555,6 +564,8 @@ struct bnge_napi {
#define BNGE_TX_CMP_EVENT 0x10
bool in_reset;
bool tx_fault;
+
+ unsigned long flags;
};
#define INVALID_STATS_CTX_ID -1
--
2.47.3
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH net-next v2 6/6] bnge: Create NQ0 during probe and keep active across open/close
2026-09-22 5:28 [PATCH net-next v2 0/6] bnge: Support async events while down Bhargava Marreddy
` (4 preceding siblings ...)
2026-09-22 5:29 ` [PATCH net-next v2 5/6] bnge: Quiesce NQ0 around ring teardown in bnge_free_core() Bhargava Marreddy
@ 2026-09-22 5:29 ` Bhargava Marreddy
2026-09-24 17:07 ` netdev-bot+sashiko
5 siblings, 1 reply; 9+ messages in thread
From: Bhargava Marreddy @ 2026-09-22 5:29 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
Cc: netdev, linux-kernel, michael.chan, pavan.chebbi,
vsrama-krishna.nemani, vikas.gupta, Bhargava Marreddy,
Dharmender Garg
Set up NQ0 during probe and free it at remove. Skip NQ0 in open/close
ring, IRQ, and NAPI helpers once BNGE_NAPI_NQ0 is set.
bnge_reinit_nq0() re-creates NQ0 on open if its MSI-X vector no longer
matches the aux device's current reservation.
Also guard bnge_napi_poll() against stale completions for rings freed by
bnge_free_nq_tree().
Since NQ0 now needs its own MSI-X vector, fix bnge_alloc_irqs() to
require "aux_msix + min" vectors instead of just "aux_msix".
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
---
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 255 +++++++++++++++++-
.../net/ethernet/broadcom/bnge/bnge_resc.c | 2 +-
.../net/ethernet/broadcom/bnge/bnge_rmem.c | 3 +-
.../net/ethernet/broadcom/bnge/bnge_txrx.c | 15 ++
4 files changed, 261 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index 1c12282d7e88..c19b51221fa8 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -26,6 +26,10 @@
#include "bnge_rmem.h"
#include "bnge_txrx.h"
+static void bnge_free_nq0(struct bnge_net *bn, bool locked);
+static int bnge_setup_nq0(struct bnge_net *bn, bool locked);
+static int bnge_reinit_nq0(struct bnge_net *bn);
+
#define BNGE_RING_TO_TC_OFF(bd, tx) \
((tx) % (bd)->tx_nr_rings_per_tc)
@@ -527,6 +531,9 @@ static void bnge_free_nq_arrays(struct bnge_net *bn)
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_napi *bnapi = bn->bnapi[i];
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
+
bnge_free_nq_desc_arr(&bnapi->nq_ring);
}
}
@@ -539,6 +546,9 @@ static int bnge_alloc_nq_arrays(struct bnge_net *bn)
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_napi *bnapi = bn->bnapi[i];
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
+
rc = bnge_alloc_nq_desc_arr(&bnapi->nq_ring, bn->cp_nr_pages);
if (rc)
goto err_free_nq_arrays;
@@ -564,7 +574,8 @@ static void bnge_free_nq_tree(struct bnge_net *bn)
nqr = &bnapi->nq_ring;
ring = &nqr->ring_struct;
- bnge_free_ring(bd, &ring->ring_mem);
+ if (!BNGE_NQ0_NAPI(bnapi))
+ bnge_free_ring(bd, &ring->ring_mem);
if (!nqr->cp_ring_arr)
continue;
@@ -664,11 +675,13 @@ static int bnge_alloc_nq_tree(struct bnge_net *bn)
nqr->bnapi = bnapi;
ring = &nqr->ring_struct;
- rc = bnge_alloc_ring(bd, &ring->ring_mem);
- if (rc)
- goto err_free_nq_tree;
+ if (!BNGE_NQ0_NAPI(bnapi)) {
+ rc = bnge_alloc_ring(bd, &ring->ring_mem);
+ if (rc)
+ goto err_free_nq_tree;
- ring->map_idx = ulp_msix + i;
+ ring->map_idx = ulp_msix + i;
+ }
if (i < bd->rx_nr_rings) {
cp_count++;
@@ -1252,6 +1265,10 @@ static int bnge_init_ring_grps(struct bnge_net *bn, bool irq_re_init)
bn->grp_info[i].fw_grp_id = INVALID_HW_RING_ID;
bn->grp_info[i].rx_fw_ring_id = INVALID_HW_RING_ID;
bn->grp_info[i].agg_fw_ring_id = INVALID_HW_RING_ID;
+
+ if (BNGE_NQ0_NAPI(bn->bnapi[i]))
+ continue;
+
bn->grp_info[i].nq_fw_ring_id = INVALID_HW_RING_ID;
}
@@ -1270,6 +1287,9 @@ static int bnge_alloc_bnapi_mem(struct bnge_net *bn)
int i, size, arr_size;
void *bnapi;
+ if (!bd->nq_nr_rings)
+ return -ENOMEM;
+
arr_size = L1_CACHE_ALIGN(sizeof(struct bnge_napi *) *
bd->nq_nr_rings);
size = L1_CACHE_ALIGN(sizeof(struct bnge_napi));
@@ -1407,6 +1427,10 @@ static int bnge_alloc_core(struct bnge_net *bn)
if (rc)
goto err_free_core;
+ rc = bnge_reinit_nq0(bn);
+ if (rc)
+ goto err_free_core;
+
bnge_quiesce_nq0(bn);
rc = bnge_alloc_nq_tree(bn);
if (rc)
@@ -1455,8 +1479,11 @@ static void bnge_init_nq_tree(struct bnge_net *bn)
struct bnge_nq_ring_info *nqr = &bn->bnapi[i]->nq_ring;
struct bnge_ring_struct *ring = &nqr->ring_struct;
- nqr->nq_raw_cons = 0;
- ring->fw_ring_id = INVALID_HW_RING_ID_32BIT;
+ if (!BNGE_NQ0_NAPI(bn->bnapi[i])) {
+ nqr->nq_raw_cons = 0;
+ ring->fw_ring_id = INVALID_HW_RING_ID_32BIT;
+ }
+
for (j = 0; j < nqr->cp_ring_count; j++) {
struct bnge_cp_ring_info *cpr = &nqr->cp_ring_arr[j];
@@ -1983,6 +2010,9 @@ static int bnge_hwrm_nq_ring_alloc(struct bnge_net *bn, int index)
u32 map_idx;
int rc;
+ if (BNGE_NQ0_NAPI(bnapi))
+ return 0;
+
nqr = &bnapi->nq_ring;
ring = &nqr->ring_struct;
map_idx = ring->map_idx;
@@ -2007,6 +2037,171 @@ static int bnge_hwrm_nq_ring_alloc(struct bnge_net *bn, int index)
return 0;
}
+static void bnge_free_nq0(struct bnge_net *bn, bool locked)
+{
+ struct bnge_nq_ring_info *nqr;
+ struct bnge_ring_struct *ring;
+ struct bnge_dev *bd = bn->bd;
+ struct bnge_napi *bnapi;
+ struct bnge_irq *irq;
+
+ if (!bn->bnapi)
+ return;
+
+ bnapi = bn->bnapi[BNGE_NQ0_NAPI_IDX];
+ if (!BNGE_NQ0_NAPI(bnapi))
+ return;
+
+ nqr = &bnapi->nq_ring;
+ ring = &nqr->ring_struct;
+ irq = &bd->irq_tbl[ring->map_idx];
+
+ if (locked)
+ napi_disable_locked(&bnapi->napi);
+ else
+ napi_disable(&bnapi->napi);
+
+ synchronize_irq(irq->vector);
+ bnge_db_nq(bn, &nqr->nq_db, nqr->nq_raw_cons);
+
+ if (irq->have_cpumask) {
+ irq_set_affinity_hint(irq->vector, NULL);
+ free_cpumask_var(irq->cpu_mask);
+ irq->have_cpumask = 0;
+ }
+ free_irq(irq->vector, bnapi);
+ irq->requested = 0;
+
+ if (locked)
+ __netif_napi_del_locked(&bnapi->napi);
+ else
+ netif_napi_del(&bnapi->napi);
+
+ synchronize_net();
+
+ hwrm_ring_free_send_msg(bn, ring,
+ RING_FREE_REQ_RING_TYPE_NQ,
+ INVALID_HW_RING_ID);
+ ring->fw_ring_id = INVALID_HW_RING_ID;
+ if (bn->grp_info)
+ bn->grp_info[0].nq_fw_ring_id = INVALID_HW_RING_ID;
+
+ clear_bit(BNGE_NAPI_QUIESCED, &bnapi->flags);
+ clear_bit(BNGE_NAPI_NQ0, &bnapi->flags);
+
+ bnge_free_ring(bd, &ring->ring_mem);
+ bnge_free_nq_desc_arr(nqr);
+}
+
+static int bnge_setup_nq0(struct bnge_net *bn, bool locked)
+{
+ struct bnge_nq_ring_info *nqr;
+ struct bnge_ring_struct *ring;
+ struct bnge_dev *bd = bn->bd;
+ struct bnge_napi *bnapi;
+ struct bnge_irq *irq;
+ int map_idx, rc;
+
+ bnapi = bn->bnapi[BNGE_NQ0_NAPI_IDX];
+ if (BNGE_NQ0_NAPI(bnapi))
+ return 0;
+
+ nqr = &bnapi->nq_ring;
+ ring = &nqr->ring_struct;
+ rc = bnge_alloc_nq_desc_arr(&bnapi->nq_ring, bn->cp_nr_pages);
+ if (rc)
+ return -ENOMEM;
+
+ nqr->nq_raw_cons = 0;
+ bnge_init_nq_ring_struct(bn, nqr);
+ rc = bnge_alloc_ring(bd, &ring->ring_mem);
+ if (rc)
+ goto err_free_nq_desc_arr;
+
+ map_idx = bnge_aux_get_msix(bd);
+ ring->map_idx = map_idx;
+ irq = &bd->irq_tbl[map_idx];
+ irq->handler = bnge_msix;
+
+ if (locked)
+ netif_napi_add_config_locked(bn->netdev, &bnapi->napi,
+ bnge_napi_poll, bnapi->index);
+ else
+ netif_napi_add_config(bn->netdev, &bnapi->napi,
+ bnge_napi_poll, bnapi->index);
+
+ snprintf(irq->name, sizeof(bd->irq_tbl[0].name), "%s-%s-%d", "bnge",
+ "nq", map_idx);
+ rc = request_irq(irq->vector, irq->handler, 0, irq->name, bnapi);
+ if (rc)
+ goto err_del_napi;
+
+ if (locked)
+ netif_napi_set_irq_locked(&bnapi->napi, irq->vector);
+ else
+ netif_napi_set_irq(&bnapi->napi, irq->vector);
+ irq->requested = 1;
+
+ if (zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL)) {
+ int numa_node = dev_to_node(&bd->pdev->dev);
+ int cpu = cpumask_local_spread(BNGE_NQ0_NAPI_IDX, numa_node);
+
+ irq->have_cpumask = 1;
+ cpumask_set_cpu(cpu, irq->cpu_mask);
+ if (irq_set_affinity_hint(irq->vector, irq->cpu_mask)) {
+ netdev_warn(bn->netdev,
+ "Set affinity failed, IRQ = %d\n",
+ irq->vector);
+ }
+ }
+
+ rc = bnge_hwrm_nq_ring_alloc(bn, BNGE_NQ0_NAPI_IDX);
+ if (rc)
+ goto err_free_irq;
+
+ if (locked)
+ napi_enable_locked(&bnapi->napi);
+ else
+ napi_enable(&bnapi->napi);
+
+ bnge_db_nq_arm(bn, &nqr->nq_db, nqr->nq_raw_cons);
+
+ set_bit(BNGE_NAPI_NQ0, &bnapi->flags);
+
+ return 0;
+
+err_free_irq:
+ if (irq->have_cpumask) {
+ irq_set_affinity_hint(irq->vector, NULL);
+ free_cpumask_var(irq->cpu_mask);
+ irq->have_cpumask = 0;
+ }
+ free_irq(irq->vector, bnapi);
+ irq->requested = 0;
+err_del_napi:
+ if (locked)
+ __netif_napi_del_locked(&bnapi->napi);
+ else
+ netif_napi_del(&bnapi->napi);
+ bnge_free_ring(bd, &ring->ring_mem);
+err_free_nq_desc_arr:
+ bnge_free_nq_desc_arr(nqr);
+ return rc;
+}
+
+static int bnge_reinit_nq0(struct bnge_net *bn)
+{
+ struct bnge_napi *bnapi = bn->bnapi[BNGE_NQ0_NAPI_IDX];
+ struct bnge_dev *bd = bn->bd;
+
+ if (BNGE_NQ0_NAPI(bnapi) &&
+ bnapi->nq_ring.ring_struct.map_idx == bnge_aux_get_msix(bd))
+ return 0;
+
+ bnge_free_nq0(bn, true);
+ return bnge_setup_nq0(bn, true);
+}
+
static int bnge_hwrm_ring_alloc(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
@@ -2419,6 +2614,9 @@ static void bnge_disable_int(struct bnge_net *bn)
struct bnge_nq_ring_info *nqr;
struct bnge_ring_struct *ring;
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
+
nqr = &bnapi->nq_ring;
ring = &nqr->ring_struct;
@@ -2434,8 +2632,12 @@ static void bnge_disable_int_sync(struct bnge_net *bn)
bnge_disable_int(bn);
for (i = 0; i < bd->nq_nr_rings; i++) {
- int map_idx = bnge_cp_num_to_irq_num(bn, i);
+ int map_idx;
+ if (BNGE_NQ0_NAPI(bn->bnapi[i]))
+ continue;
+
+ map_idx = bnge_cp_num_to_irq_num(bn, i);
synchronize_irq(bd->irq_tbl[map_idx].vector);
}
}
@@ -2449,6 +2651,9 @@ static void bnge_enable_int(struct bnge_net *bn)
struct bnge_napi *bnapi = bn->bnapi[i];
struct bnge_nq_ring_info *nqr;
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
+
nqr = &bnapi->nq_ring;
bnge_db_nq_arm(bn, &nqr->nq_db, nqr->nq_raw_cons);
}
@@ -2465,6 +2670,8 @@ static void bnge_disable_napi(struct bnge_net *bn)
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_napi *bnapi = bn->bnapi[i];
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
napi_disable_locked(&bnapi->napi);
}
}
@@ -2481,6 +2688,8 @@ static void bnge_enable_napi(struct bnge_net *bn)
bnapi->in_reset = false;
bnapi->tx_fault = 0;
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
napi_enable_locked(&bnapi->napi);
}
}
@@ -2629,6 +2838,9 @@ static void bnge_hwrm_ring_free(struct bnge_net *bn, bool close_path)
for (j = 0; j < nqr->cp_ring_count && nqr->cp_ring_arr; j++)
bnge_hwrm_cp_ring_free(bn, &nqr->cp_ring_arr[j]);
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
+
ring = &nqr->ring_struct;
if (ring->fw_ring_id != INVALID_HW_RING_ID_32BIT) {
hwrm_ring_free_send_msg(bn, ring,
@@ -2690,6 +2902,9 @@ static void bnge_free_irq(struct bnge_net *bn)
for (i = 0; i < bd->nq_nr_rings; i++) {
int map_idx = bnge_cp_num_to_irq_num(bn, i);
+ if (BNGE_NQ0_NAPI(bn->bnapi[i]))
+ continue;
+
irq = &bd->irq_tbl[map_idx];
if (irq->requested) {
if (irq->have_cpumask) {
@@ -2718,6 +2933,9 @@ static int bnge_request_irq(struct bnge_net *bn)
int map_idx = bnge_cp_num_to_irq_num(bn, i);
struct bnge_irq *irq = &bd->irq_tbl[map_idx];
+ if (BNGE_NQ0_NAPI(bn->bnapi[i]))
+ continue;
+
rc = request_irq(irq->vector, irq->handler, 0, irq->name,
bn->bnapi[i]);
if (rc)
@@ -2732,12 +2950,10 @@ static int bnge_request_irq(struct bnge_net *bn)
irq->have_cpumask = 1;
cpumask_set_cpu(cpumask_local_spread(i, numa_node),
irq->cpu_mask);
- rc = irq_set_affinity_hint(irq->vector, irq->cpu_mask);
- if (rc) {
+ if (irq_set_affinity_hint(irq->vector, irq->cpu_mask)) {
netdev_warn(bn->netdev,
"Set affinity failed, IRQ = %d\n",
irq->vector);
- goto err_free_irq;
}
}
}
@@ -2857,6 +3073,10 @@ static void bnge_init_napi(struct bnge_net *bn)
for (i = 0; i < bd->nq_nr_rings; i++) {
bnapi = bn->bnapi[i];
+
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
+
netif_napi_add_config_locked(bn->netdev, &bnapi->napi,
bnge_napi_poll, bnapi->index);
}
@@ -2875,6 +3095,9 @@ static void bnge_del_napi(struct bnge_net *bn)
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_napi *bnapi = bn->bnapi[i];
+ if (BNGE_NQ0_NAPI(bnapi))
+ continue;
+
__netif_napi_del_locked(&bnapi->napi);
}
@@ -3610,14 +3833,20 @@ int bnge_netdev_alloc(struct bnge_dev *bd, int max_irqs)
if (rc)
goto err_free_bnapi_mem;
+ rc = bnge_setup_nq0(bn, false);
+ if (rc)
+ goto err_free_ring_grps;
+
rc = register_netdev(netdev);
if (rc) {
dev_err(bd->dev, "Register netdev failed rc: %d\n", rc);
- goto err_free_ring_grps;
+ goto err_free_nq0;
}
return 0;
+err_free_nq0:
+ bnge_free_nq0(bn, false);
err_free_ring_grps:
bnge_free_ring_grps(bn);
err_free_bnapi_mem:
@@ -3640,6 +3869,8 @@ void bnge_netdev_free(struct bnge_dev *bd)
unregister_netdev(netdev);
+ bnge_free_nq0(bn, false);
+
timer_shutdown_sync(&bn->timer);
cancel_work_sync(&bn->sp_task);
bn->sp_event = 0;
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
index 4711dd4945ff..7c26943c5cb3 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
@@ -382,7 +382,7 @@ int bnge_alloc_irqs(struct bnge_dev *bd)
irqs_demand = pci_alloc_irq_vectors(bd->pdev, min, irqs_demand,
PCI_IRQ_MSIX);
aux_msix = bnge_aux_get_msix(bd);
- if (irqs_demand < 0 || irqs_demand < aux_msix) {
+ if (irqs_demand < 0 || irqs_demand < aux_msix + min) {
rc = -ENODEV;
goto err_free_irqs;
}
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
index e0ddb2800c54..22fd4eeb3213 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
@@ -450,7 +450,8 @@ void bnge_init_ring_struct(struct bnge_net *bn)
struct bnge_tx_ring_info *txr;
struct bnge_ring_struct *ring;
- bnge_init_nq_ring_struct(bn, &bnapi->nq_ring);
+ if (!BNGE_NQ0_NAPI(bnapi))
+ bnge_init_nq_ring_struct(bn, &bnapi->nq_ring);
rxr = bnapi->rx_ring;
if (!rxr)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
index 7d45e057f2e8..dcec6fdced30 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
@@ -1375,6 +1375,21 @@ int bnge_napi_poll(struct napi_struct *napi, int budget)
break;
idx = BNGE_NQ_HDL_IDX(idx);
+
+ /* NQ0 keeps running while administratively down to
+ * process async events, but its cp_ring_arr is torn
+ * down (and cp_ring_count zeroed) by
+ * bnge_free_nq_tree() while down. Guard against a
+ * stray/late CQ notification arriving in that state
+ * instead of dereferencing a freed or out-of-range
+ * cp_ring_arr.
+ */
+ if (unlikely(!nqr->cp_ring_arr ||
+ idx >= nqr->cp_ring_count)) {
+ raw_cons = NEXT_RAW_CMP(raw_cons);
+ continue;
+ }
+
cpr = &nqr->cp_ring_arr[idx];
cpr->had_nqe_notify = 1;
cpr->toggle = NQE_CN_TOGGLE(type);
--
2.47.3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next v2 6/6] bnge: Create NQ0 during probe and keep active across open/close
2026-09-22 5:29 ` [PATCH net-next v2 6/6] bnge: Create NQ0 during probe and keep active across open/close Bhargava Marreddy
@ 2026-09-24 17:07 ` netdev-bot+sashiko
0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-24 17:07 UTC (permalink / raw)
To: bhargava.marreddy
Cc: davem, edumazet, kuba, pabeni, andrew+netdev, horms, netdev,
linux-kernel, michael.chan, pavan.chebbi, vsrama-krishna.nemani,
vikas.gupta, dharmender.garg
Thank you for your contribution! Sashiko AI review found 9 potential
issue(s) to consider.
Critical: 0 · High: 2 · Medium: 6 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] bnge_close_core() no longer quiesces NQ0 before tearing down
shared ring state, so NQ0's NAPI can run concurrently with the…
- [High] bnge_setup_nq0() uses the runtime-mutable aux MSI-X count as an
unvalidated index into bd->irq_tbl.
- [Medium] bnge_setup_nq0() can double-allocate NQ0's descriptor arrays
and leak the first pair.
- [Medium] NQ0 is re-armed in bnge_alloc_core() before the freshly
attached completion rings have usable doorbells.
- [Medium] The patch silently converts an irq_set_affinity_hint() failure
in bnge_request_irq() from fatal to a warning (the `goto err_free_irq`…
- [Medium] The commit message mischaracterizes the bnge_alloc_irqs()
change and hides a second fix.
- [Medium] bnge_reinit_nq0() destroys a working NQ0 before creating the
replacement and has no rollback: it calls bnge_free_nq0(bn, true)…
- [Medium] A failure of bnge_hwrm_set_async_event_cr() during NQ0 setup
is now permanent.
- [Low] bnge_free_nq0() stores the 16-bit sentinel INVALID_HW_RING_ID
(0xffff) into the 32-bit ring->fw_ring_id, whereas every other writer…
--
bnge: Create NQ0 during probe and keep active across open/close
This moves NQ0 creation out of open/close into probe/remove, marks it with
BNGE_NAPI_NQ0 so the generic ring/IRQ/NAPI helpers skip it, and adds
bnge_reinit_nq0() to re-create it when the aux MSI-X reservation changes.
It also adds a guard in bnge_napi_poll() for completions arriving while
NQ0's completion rings are torn down.
A few questions about the new NQ0 lifetime below.
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> index 1c12282d7e88f..c19b51221fa8e 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
[ ... ]
> @@ -539,6 +546,9 @@ static int bnge_alloc_nq_arrays(struct bnge_net *bn)
> for (i = 0; i < bd->nq_nr_rings; i++) {
> struct bnge_napi *bnapi = bn->bnapi[i];
>
> + if (BNGE_NQ0_NAPI(bnapi))
> + continue;
> +
> rc = bnge_alloc_nq_desc_arr(&bnapi->nq_ring, bn->cp_nr_pages);
> if (rc)
> goto err_free_nq_arrays;
[ ... ]
> @@ -1270,6 +1287,9 @@ static int bnge_alloc_bnapi_mem(struct bnge_net *bn)
> int i, size, arr_size;
> void *bnapi;
>
> + if (!bd->nq_nr_rings)
> + return -ENOMEM;
> +
> arr_size = L1_CACHE_ALIGN(sizeof(struct bnge_napi *) *
> bd->nq_nr_rings);
> size = L1_CACHE_ALIGN(sizeof(struct bnge_napi));
> @@ -1407,6 +1427,10 @@ static int bnge_alloc_core(struct bnge_net *bn)
> if (rc)
> goto err_free_core;
>
> + rc = bnge_reinit_nq0(bn);
> + if (rc)
> + goto err_free_core;
> +
> bnge_quiesce_nq0(bn);
> rc = bnge_alloc_nq_tree(bn);
> if (rc)
[Severity: Medium]
bnge_quiesce_nq0() and bnge_resume_nq0() only bracket
bnge_alloc_nq_tree(), so NQ0 is re-armed as soon as the new
nqr->cp_ring_arr is published. Is that too early?
cp_ring_arr comes from kzalloc_objs() in bnge_alloc_nq_tree(), so
cpr->cp_db.doorbell is NULL until bnge_set_db() runs much later, during
bnge_init_nic() -> bnge_hwrm_ring_alloc():
db->doorbell = bd->bar1 + bd->db_offset;
NQ0's nq_raw_cons and fw_ring_id are deliberately preserved across
close/open by bnge_init_nq_tree(), and the NQ ring is never drained. If an
unconsumed CQ_NOTIFICATION from the previous incarnation is still pending,
it passes the new guard in bnge_napi_poll() (cp_ring_arr is non-NULL and
idx is in range), __bnge_poll_work() sets cpr->had_work_done = 1, and
__bnge_poll_cqs_done() then writes:
bnge_writeq(bn->bd,
db->db_key64 | dbr_type | DB_TOGGLE(tgl) |
DB_RING_IDX(db, cpr->cp_raw_cons),
db->doorbell);
with db->doorbell == NULL. Is such a stale entry always consumed before
the next open, or can it survive into this window?
> @@ -1455,8 +1479,11 @@ static void bnge_init_nq_tree(struct bnge_net *bn)
> struct bnge_nq_ring_info *nqr = &bn->bnapi[i]->nq_ring;
> struct bnge_ring_struct *ring = &nqr->ring_struct;
>
> - nqr->nq_raw_cons = 0;
> - ring->fw_ring_id = INVALID_HW_RING_ID_32BIT;
> + if (!BNGE_NQ0_NAPI(bn->bnapi[i])) {
> + nqr->nq_raw_cons = 0;
> + ring->fw_ring_id = INVALID_HW_RING_ID_32BIT;
> + }
> +
> for (j = 0; j < nqr->cp_ring_count; j++) {
> struct bnge_cp_ring_info *cpr = &nqr->cp_ring_arr[j];
>
[ ... ]
> @@ -1983,6 +2010,9 @@ static int bnge_hwrm_nq_ring_alloc(struct bnge_net *bn, int index)
> u32 map_idx;
> int rc;
>
> + if (BNGE_NQ0_NAPI(bnapi))
> + return 0;
> +
> nqr = &bnapi->nq_ring;
> ring = &nqr->ring_struct;
> map_idx = ring->map_idx;
> @@ -2007,6 +2037,171 @@ static int bnge_hwrm_nq_ring_alloc(struct bnge_net *bn, int index)
> return 0;
> }
>
> +static void bnge_free_nq0(struct bnge_net *bn, bool locked)
> +{
> + struct bnge_nq_ring_info *nqr;
> + struct bnge_ring_struct *ring;
> + struct bnge_dev *bd = bn->bd;
> + struct bnge_napi *bnapi;
> + struct bnge_irq *irq;
> +
> + if (!bn->bnapi)
> + return;
> +
> + bnapi = bn->bnapi[BNGE_NQ0_NAPI_IDX];
> + if (!BNGE_NQ0_NAPI(bnapi))
> + return;
> +
> + nqr = &bnapi->nq_ring;
> + ring = &nqr->ring_struct;
> + irq = &bd->irq_tbl[ring->map_idx];
> +
> + if (locked)
> + napi_disable_locked(&bnapi->napi);
> + else
> + napi_disable(&bnapi->napi);
> +
> + synchronize_irq(irq->vector);
> + bnge_db_nq(bn, &nqr->nq_db, nqr->nq_raw_cons);
> +
> + if (irq->have_cpumask) {
> + irq_set_affinity_hint(irq->vector, NULL);
> + free_cpumask_var(irq->cpu_mask);
> + irq->have_cpumask = 0;
> + }
> + free_irq(irq->vector, bnapi);
> + irq->requested = 0;
> +
> + if (locked)
> + __netif_napi_del_locked(&bnapi->napi);
> + else
> + netif_napi_del(&bnapi->napi);
> +
> + synchronize_net();
> +
> + hwrm_ring_free_send_msg(bn, ring,
> + RING_FREE_REQ_RING_TYPE_NQ,
> + INVALID_HW_RING_ID);
> + ring->fw_ring_id = INVALID_HW_RING_ID;
[Severity: Low]
Should this be INVALID_HW_RING_ID_32BIT? fw_ring_id is u32 and the two
sentinels have different values:
#define INVALID_HW_RING_ID ((u16)-1)
#define INVALID_HW_RING_ID_32BIT (U32_MAX)
Every other writer of ring->fw_ring_id uses the 32-bit constant, and every
reader compares against it, for example in bnge_hwrm_ring_free():
if (ring->fw_ring_id != INVALID_HW_RING_ID_32BIT) {
hwrm_ring_free_send_msg(bn, ring,
RING_FREE_REQ_RING_TYPE_NQ,
and in bnge_disable_int():
if (ring->fw_ring_id != INVALID_HW_RING_ID_32BIT)
bnge_db_nq(bn, &nqr->nq_db, nqr->nq_raw_cons);
Since bnge_free_nq0() also clears BNGE_NAPI_NQ0, index 0 stops being
skipped by those loops while fw_ring_id holds 0x0000ffff, which both tests
read as a live ring.
This looks latent today: the bit-clear state only arises after a failed
open (so ndo_close cannot run) and the next open overwrites the field. It
would become reachable as soon as an internal close/reopen path such as
ethtool -L/-G or reset recovery is added.
> + if (bn->grp_info)
> + bn->grp_info[0].nq_fw_ring_id = INVALID_HW_RING_ID;
> +
> + clear_bit(BNGE_NAPI_QUIESCED, &bnapi->flags);
> + clear_bit(BNGE_NAPI_NQ0, &bnapi->flags);
> +
> + bnge_free_ring(bd, &ring->ring_mem);
> + bnge_free_nq_desc_arr(nqr);
> +}
> +
> +static int bnge_setup_nq0(struct bnge_net *bn, bool locked)
> +{
> + struct bnge_nq_ring_info *nqr;
> + struct bnge_ring_struct *ring;
> + struct bnge_dev *bd = bn->bd;
> + struct bnge_napi *bnapi;
> + struct bnge_irq *irq;
> + int map_idx, rc;
> +
> + bnapi = bn->bnapi[BNGE_NQ0_NAPI_IDX];
> + if (BNGE_NQ0_NAPI(bnapi))
> + return 0;
> +
> + nqr = &bnapi->nq_ring;
> + ring = &nqr->ring_struct;
> + rc = bnge_alloc_nq_desc_arr(&bnapi->nq_ring, bn->cp_nr_pages);
> + if (rc)
> + return -ENOMEM;
[Severity: Medium]
Can this leak NQ0's descriptor arrays?
bnge_alloc_nq_desc_arr() is called unconditionally here, but index 0 is
only skipped by bnge_alloc_nq_arrays() while BNGE_NAPI_NQ0 is set. If an
earlier bnge_reinit_nq0() freed NQ0 (clearing the bit) and the following
bnge_setup_nq0() failed, the bit stays clear, so on the next open:
bnge_alloc_core()
bnge_alloc_nq_arrays() /* allocates desc_ring/desc_mapping for 0 */
bnge_reinit_nq0()
bnge_setup_nq0()
bnge_alloc_nq_desc_arr() /* allocates them again */
and bnge_alloc_nq_desc_arr() just assigns without checking:
nqr->desc_ring = kzalloc_objs(*nqr->desc_ring, n);
if (!nqr->desc_ring)
return -ENOMEM;
nqr->desc_mapping = kzalloc_objs(*nqr->desc_mapping, n);
Once BNGE_NAPI_NQ0 is set again, bnge_free_nq_arrays() skips index 0, so
is the first pair ever freed?
> +
> + nqr->nq_raw_cons = 0;
> + bnge_init_nq_ring_struct(bn, nqr);
> + rc = bnge_alloc_ring(bd, &ring->ring_mem);
> + if (rc)
> + goto err_free_nq_desc_arr;
> +
> + map_idx = bnge_aux_get_msix(bd);
> + ring->map_idx = map_idx;
> + irq = &bd->irq_tbl[map_idx];
> + irq->handler = bnge_msix;
[Severity: High]
Can this write past bd->irq_tbl?
map_idx comes straight from bnge_aux_get_msix(bd) (bd->aux_num_msix) with
no check against bd->irqs_acquired, and the entry is then written
(irq->handler, snprintf() into irq->name, irq->requested, irq->cpu_mask)
and read (irq->vector passed to request_irq()).
bd->irq_tbl is allocated in bnge_alloc_irqs() with irqs_demand entries
(unless dynamic MSI-X is available), and only those entries get a valid
.vector. bd->aux_num_msix is recomputed at runtime in
bnge_reserve_rings():
resv_msix = hw_resc->resv_irqs - bd->nq_nr_rings;
aux_msix = min_t(u16, resv_msix, aux_msix);
bnge_aux_set_msix_num(bd, aux_msix);
where the default is derived from num_online_cpus(), and it is never
clamped against bd->irqs_acquired. If CPUs are brought online after probe
and a reservation is re-triggered (ethtool -L / -G), can aux_num_msix grow
past the value bnge_alloc_irqs() validated, so that the next open reaches
bnge_setup_nq0() with map_idx >= bd->irqs_acquired? bnge_free_nq0()
repeats the same access through the stored ring->map_idx.
> +
> + if (locked)
> + netif_napi_add_config_locked(bn->netdev, &bnapi->napi,
> + bnge_napi_poll, bnapi->index);
> + else
> + netif_napi_add_config(bn->netdev, &bnapi->napi,
> + bnge_napi_poll, bnapi->index);
> +
> + snprintf(irq->name, sizeof(bd->irq_tbl[0].name), "%s-%s-%d", "bnge",
> + "nq", map_idx);
> + rc = request_irq(irq->vector, irq->handler, 0, irq->name, bnapi);
> + if (rc)
> + goto err_del_napi;
> +
> + if (locked)
> + netif_napi_set_irq_locked(&bnapi->napi, irq->vector);
> + else
> + netif_napi_set_irq(&bnapi->napi, irq->vector);
> + irq->requested = 1;
> +
> + if (zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL)) {
> + int numa_node = dev_to_node(&bd->pdev->dev);
> + int cpu = cpumask_local_spread(BNGE_NQ0_NAPI_IDX, numa_node);
> +
> + irq->have_cpumask = 1;
> + cpumask_set_cpu(cpu, irq->cpu_mask);
> + if (irq_set_affinity_hint(irq->vector, irq->cpu_mask)) {
> + netdev_warn(bn->netdev,
> + "Set affinity failed, IRQ = %d\n",
> + irq->vector);
> + }
> + }
> +
> + rc = bnge_hwrm_nq_ring_alloc(bn, BNGE_NQ0_NAPI_IDX);
> + if (rc)
> + goto err_free_irq;
[Severity: Medium]
With NQ0 now persistent, does a failure to program the async event ring
become permanent?
bnge_hwrm_nq_ring_alloc() only warns on that failure and returns success:
rc = bnge_hwrm_set_async_event_cr(bd, ring->fw_ring_id);
if (rc)
netdev_warn(bn->netdev, "Failed to set async event completion ring.\n");
so bnge_setup_nq0() proceeds to arm NQ0 and set BNGE_NAPI_NQ0. After that
bnge_reinit_nq0() returns early whenever the MSI-X index is unchanged, and
bnge_hwrm_nq_ring_alloc() returns 0 immediately for index 0 because
BNGE_NQ0_NAPI(bnapi) is true, so the FUNC_CFG async_event_cr programming is
never retried. Before this patch the NQ0 allocation was redone on every
open, so a down/up cycle recovered; now a single transient HWRM failure at
probe would leave link status and other async events undelivered for the
life of the driver instance.
> +
> + if (locked)
> + napi_enable_locked(&bnapi->napi);
> + else
> + napi_enable(&bnapi->napi);
> +
> + bnge_db_nq_arm(bn, &nqr->nq_db, nqr->nq_raw_cons);
> +
> + set_bit(BNGE_NAPI_NQ0, &bnapi->flags);
> +
> + return 0;
> +
> +err_free_irq:
> + if (irq->have_cpumask) {
> + irq_set_affinity_hint(irq->vector, NULL);
> + free_cpumask_var(irq->cpu_mask);
> + irq->have_cpumask = 0;
> + }
> + free_irq(irq->vector, bnapi);
> + irq->requested = 0;
> +err_del_napi:
> + if (locked)
> + __netif_napi_del_locked(&bnapi->napi);
> + else
> + netif_napi_del(&bnapi->napi);
> + bnge_free_ring(bd, &ring->ring_mem);
> +err_free_nq_desc_arr:
> + bnge_free_nq_desc_arr(nqr);
> + return rc;
> +}
> +
> +static int bnge_reinit_nq0(struct bnge_net *bn)
> +{
> + struct bnge_napi *bnapi = bn->bnapi[BNGE_NQ0_NAPI_IDX];
> + struct bnge_dev *bd = bn->bd;
> +
> + if (BNGE_NQ0_NAPI(bnapi) &&
> + bnapi->nq_ring.ring_struct.map_idx == bnge_aux_get_msix(bd))
> + return 0;
> +
> + bnge_free_nq0(bn, true);
> + return bnge_setup_nq0(bn, true);
> +}
[Severity: Medium]
The working NQ0 is destroyed before the replacement exists, and there is no
rollback if bnge_setup_nq0() fails.
bnge_free_nq0() frees the IRQ, deletes the NAPI, frees the firmware ring
and clears BNGE_NAPI_NQ0. If bnge_setup_nq0() then hits a transient
failure (bnge_alloc_nq_desc_arr() or bnge_alloc_ring() returning -ENOMEM,
request_irq() failing, or bnge_hwrm_nq_ring_alloc() failing), the device is
left with no NQ0 at all: bnge_alloc_core() propagates the error, open()
fails, BNGE_NAPI_NQ0 stays clear and bnge_free_core() does not restore it.
The commit message describes the reinit as:
"bnge_reinit_nq0() re-creates NQ0 on open if its MSI-X vector no
longer matches the aux device's current reservation."
Is losing the always-on async event ring until some later successful open
intended here, or should the existing NQ0 be kept until the new one is
fully set up?
> +
> static int bnge_hwrm_ring_alloc(struct bnge_net *bn)
> {
> struct bnge_dev *bd = bn->bd;
> @@ -2419,6 +2614,9 @@ static void bnge_disable_int(struct bnge_net *bn)
> struct bnge_nq_ring_info *nqr;
> struct bnge_ring_struct *ring;
>
> + if (BNGE_NQ0_NAPI(bnapi))
> + continue;
> +
> nqr = &bnapi->nq_ring;
> ring = &nqr->ring_struct;
>
> @@ -2434,8 +2632,12 @@ static void bnge_disable_int_sync(struct bnge_net *bn)
>
> bnge_disable_int(bn);
> for (i = 0; i < bd->nq_nr_rings; i++) {
> - int map_idx = bnge_cp_num_to_irq_num(bn, i);
> + int map_idx;
>
> + if (BNGE_NQ0_NAPI(bn->bnapi[i]))
> + continue;
> +
> + map_idx = bnge_cp_num_to_irq_num(bn, i);
> synchronize_irq(bd->irq_tbl[map_idx].vector);
> }
> }
[ ... ]
> @@ -2465,6 +2670,8 @@ static void bnge_disable_napi(struct bnge_net *bn)
> for (i = 0; i < bd->nq_nr_rings; i++) {
> struct bnge_napi *bnapi = bn->bnapi[i];
>
> + if (BNGE_NQ0_NAPI(bnapi))
> + continue;
> napi_disable_locked(&bnapi->napi);
> }
> }
[Severity: High]
With NQ0 skipped here and in bnge_disable_int()/bnge_disable_int_sync(),
what still serializes bnge_napi_poll() on NQ0 against the close path?
bnge_close_core() runs:
bnge_shutdown_nic(bn); /* frees CQ/RX/TX rings in firmware */
bnge_disable_napi(bn); /* now skips NQ0 */
...
bnge_free_all_rings_bufs(bn);
bnge_free_irq(bn); /* now skips NQ0's vector */
bnge_del_napi(bn);
bnge_free_core(bn); /* only here does bnge_quiesce_nq0() run */
Through that whole window NQ0's vector stays requested and the NQ stays
armed, nqr->cp_ring_arr is still populated, and bnge_msix() schedules
unconditionally:
napi_schedule(&bnapi->napi);
So a poll landing there passes the new guard in bnge_napi_poll()
(cp_ring_arr non-NULL, idx < cp_ring_count) and enters __bnge_poll_work()
on completion rings whose firmware ids bnge_shutdown_nic() just
invalidated. Can that poll touch rxr->rx_buf_ring / txr->tx_buf_ring
entries that bnge_free_all_rings_bufs() is freeing and dma-unmapping
concurrently, and write cp_db doorbells for rings already freed in
firmware?
bnge_hwrm_ring_free() states the requirement it relies on:
/* The completion rings are about to be freed. After that the
* IRQ doorbell will not work anymore. So we need to disable
* IRQ here.
*/
bnge_disable_int_sync(bn);
and that NQ doorbell mask plus synchronize_irq() no longer happens for NQ0.
> @@ -2629,6 +2838,9 @@ static void bnge_hwrm_ring_free(struct bnge_net *bn, bool close_path)
> for (j = 0; j < nqr->cp_ring_count && nqr->cp_ring_arr; j++)
> bnge_hwrm_cp_ring_free(bn, &nqr->cp_ring_arr[j]);
>
> + if (BNGE_NQ0_NAPI(bnapi))
> + continue;
> +
> ring = &nqr->ring_struct;
> if (ring->fw_ring_id != INVALID_HW_RING_ID_32BIT) {
> hwrm_ring_free_send_msg(bn, ring,
[ ... ]
> @@ -2690,6 +2902,9 @@ static void bnge_free_irq(struct bnge_net *bn)
> for (i = 0; i < bd->nq_nr_rings; i++) {
> int map_idx = bnge_cp_num_to_irq_num(bn, i);
>
> + if (BNGE_NQ0_NAPI(bn->bnapi[i]))
> + continue;
> +
> irq = &bd->irq_tbl[map_idx];
> if (irq->requested) {
> if (irq->have_cpumask) {
[ ... ]
> @@ -2732,12 +2950,10 @@ static int bnge_request_irq(struct bnge_net *bn)
> irq->have_cpumask = 1;
> cpumask_set_cpu(cpumask_local_spread(i, numa_node),
> irq->cpu_mask);
> - rc = irq_set_affinity_hint(irq->vector, irq->cpu_mask);
> - if (rc) {
> + if (irq_set_affinity_hint(irq->vector, irq->cpu_mask)) {
> netdev_warn(bn->netdev,
> "Set affinity failed, IRQ = %d\n",
> irq->vector);
> - goto err_free_irq;
> }
> }
> }
[Severity: Medium]
This changes an irq_set_affinity_hint() failure in bnge_request_irq() from
fatal to a warning, which the commit message does not mention.
On CONFIG_SMP=n the helper is a stub that always fails:
include/linux/interrupt.h:
static inline int irq_set_affinity_hint(unsigned int irq,
const struct cpumask *m)
{
return -EINVAL;
}
while zalloc_cpumask_var() returns true on !CONFIG_CPUMASK_OFFSTACK, so
before this hunk bnge_request_irq() always took the err_free_irq path and
ndo_open() could not succeed on a uniprocessor build. Could this be split
into its own patch with a Fixes: tag so it is visible for backporting?
[ ... ]
> @@ -3610,14 +3833,20 @@ int bnge_netdev_alloc(struct bnge_dev *bd, int max_irqs)
> if (rc)
> goto err_free_bnapi_mem;
>
> + rc = bnge_setup_nq0(bn, false);
> + if (rc)
> + goto err_free_ring_grps;
> +
> rc = register_netdev(netdev);
> if (rc) {
> dev_err(bd->dev, "Register netdev failed rc: %d\n", rc);
> - goto err_free_ring_grps;
> + goto err_free_nq0;
> }
>
> return 0;
>
> +err_free_nq0:
> + bnge_free_nq0(bn, false);
> err_free_ring_grps:
> bnge_free_ring_grps(bn);
> err_free_bnapi_mem:
[ ... ]
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
> index 4711dd4945ffe..7c26943c5cb35 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
> @@ -382,7 +382,7 @@ int bnge_alloc_irqs(struct bnge_dev *bd)
> irqs_demand = pci_alloc_irq_vectors(bd->pdev, min, irqs_demand,
> PCI_IRQ_MSIX);
> aux_msix = bnge_aux_get_msix(bd);
> - if (irqs_demand < 0 || irqs_demand < aux_msix) {
> + if (irqs_demand < 0 || irqs_demand < aux_msix + min) {
> rc = -ENODEV;
> goto err_free_irqs;
> }
[Severity: Medium]
The commit message says:
"Since NQ0 now needs its own MSI-X vector, fix bnge_alloc_irqs() to
require "aux_msix + min" vectors instead of just "aux_msix"."
Does NQ0 consume an additional vector? bnge_setup_nq0() uses
map_idx = bnge_aux_get_msix(bd), which is the same ulp_msix + 0 vector that
NQ index 0 always used, and bnge_alloc_nq_tree() still assigns
ring->map_idx = ulp_msix + i for the remaining NQs, so total consumption
stays aux_msix + nq_nr_rings.
What the new test looks like it really changes is that a
pci_alloc_irq_vectors() return of exactly aux_msix is now rejected.
Previously bnge_adjust_rings() was called with a zero NQ budget, and
bnge_fix_rings_count()'s shared-channel branch set rx = tx = 0 and returned
success, so bnge_alloc_irqs() could return success with
bd->nq_nr_rings == 0.
Is that also the reason for the undocumented hunk in
bnge_alloc_bnapi_mem()?
if (!bd->nq_nr_rings)
return -ENOMEM;
Without it, kzalloc(0) returns ZERO_SIZE_PTR and the new NQ0 helpers
dereference bn->bnapi[0]. Could the real trigger and this second hunk be
described in the changelog, with a Fixes: tag?
[ ... ]
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
> index 7d45e057f2e82..dcec6fdced301 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
> @@ -1375,6 +1375,21 @@ int bnge_napi_poll(struct napi_struct *napi, int budget)
> break;
>
> idx = BNGE_NQ_HDL_IDX(idx);
> +
> + /* NQ0 keeps running while administratively down to
> + * process async events, but its cp_ring_arr is torn
> + * down (and cp_ring_count zeroed) by
> + * bnge_free_nq_tree() while down. Guard against a
> + * stray/late CQ notification arriving in that state
> + * instead of dereferencing a freed or out-of-range
> + * cp_ring_arr.
> + */
> + if (unlikely(!nqr->cp_ring_arr ||
> + idx >= nqr->cp_ring_count)) {
> + raw_cons = NEXT_RAW_CMP(raw_cons);
> + continue;
> + }
> +
> cpr = &nqr->cp_ring_arr[idx];
> cpr->had_nqe_notify = 1;
> cpr->toggle = NQE_CN_TOGGLE(type);
This guard covers the state after bnge_free_nq_tree() has run, but not the
earlier part of close (where cp_ring_arr is still valid while the firmware
rings and buffers behind it are being freed) nor the early part of open
(where cp_ring_arr is valid but cp_db is not yet programmed). Both windows
are described in the comments above.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922052904.59237-1-bhargava.marreddy%40broadcom.com
^ permalink raw reply [flat|nested] 9+ messages in thread