* [PATCH net-next v3 1/5] bnge: factor out per-NQ alloc/free helpers
2026-10-13 20:06 [PATCH net-next v3 0/5] bnge: Make NQ0 persistent across open/close Bhargava Marreddy
@ 2026-10-13 20:06 ` Bhargava Marreddy
2026-10-13 20:06 ` [PATCH net-next v3 2/5] bnge: add bnge_err()/bnge_warn()/bnge_dbg() logging helpers Bhargava Marreddy
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-10-13 20:06 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, Rajashekar Hudumula, Ramakrishna Koduri
The descriptor array, ring memory, firmware ring and IRQ of an NQ are
allocated and released inside loops over all NQs, and the NQ ring memory
is set up with the completion rings.
Factor the per-NQ parts out into bnge_alloc_nq()/bnge_free_nq(),
bnge_hwrm_nq_alloc()/bnge_hwrm_nq_free() and
bnge_nq_request_irq()/bnge_nq_free_irq() so a single NQ can be created
and destroyed independently of the others. Rename
bnge_{alloc,free}_nq_arrays() to bnge_{alloc,free}_nqs().
The NQ ring_mem setup, ring allocation and fw_ring_id initialization
move into bnge_alloc_nq(), so the NQ ring memory is allocated with the
NQ descriptor arrays rather than with the completion rings. The
fw_ring_id initialization thus moves from bnge_init_nic(), which runs
on every open, to bnge_alloc_core(), which also runs on every open.
The IRQ map index is also set there; the value is unchanged. The
affinity hint is now derived from bnapi->index, which equals the loop
index used before. The error path of bnge_alloc_nqs() now unwinds only
the NQs allocated so far.
Also rename bnge_{alloc,free,init}_nq_tree() to
bnge_{alloc,free,init}_cp_rings(), since they no longer touch the NQ
itself, only the completion rings hanging off it.
One behavioral difference: bnge_hwrm_ring_free() now resets
grp_info[i].nq_fw_ring_id to INVALID_HW_RING_ID for every NQ, not only
for the NQs that had a valid firmware ring ID. Otherwise there is no
functional change.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
---
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 284 ++++++++++--------
.../net/ethernet/broadcom/bnge/bnge_rmem.c | 10 -
2 files changed, 163 insertions(+), 131 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index 25f844e6f6d3..8cd0d8bd68c2 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -503,61 +503,89 @@ static int bnge_alloc_cp_desc_arr(struct bnge_cp_ring_info *cpr, int n)
return -ENOMEM;
}
-static void bnge_free_nq_arrays(struct bnge_net *bn)
+static void bnge_free_nq(struct bnge_net *bn, struct bnge_napi *bnapi)
{
- struct bnge_dev *bd = bn->bd;
- int i;
+ struct bnge_nq_ring_info *nqr = &bnapi->nq_ring;
- for (i = 0; i < bd->nq_nr_rings; i++) {
- struct bnge_napi *bnapi = bn->bnapi[i];
+ bnge_free_ring(bn->bd, &nqr->ring_struct.ring_mem);
+ bnge_free_nq_desc_arr(nqr);
+}
+
+static int bnge_alloc_nq(struct bnge_net *bn, struct bnge_napi *bnapi)
+{
+ struct bnge_nq_ring_info *nqr = &bnapi->nq_ring;
+ struct bnge_ring_mem_info *rmem;
+ struct bnge_ring_struct *ring;
+ int rc;
+
+ ring = &nqr->ring_struct;
+ rmem = &ring->ring_mem;
- bnge_free_nq_desc_arr(&bnapi->nq_ring);
+ rc = bnge_alloc_nq_desc_arr(nqr, bn->cp_nr_pages);
+ if (rc)
+ return rc;
+
+ nqr->bnapi = bnapi;
+ 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;
+ rmem->flags = BNGE_RMEM_RING_PTE_FLAG;
+ rc = bnge_alloc_ring(bn->bd, rmem);
+ if (rc) {
+ bnge_free_nq_desc_arr(nqr);
+ return rc;
}
+
+ ring->fw_ring_id = INVALID_HW_RING_ID_32BIT;
+ ring->map_idx = bnge_aux_get_msix(bn->bd) + bnapi->index;
+ return 0;
}
-static int bnge_alloc_nq_arrays(struct bnge_net *bn)
+static void bnge_free_nqs(struct bnge_net *bn)
+{
+ struct bnge_dev *bd = bn->bd;
+ int i;
+
+ for (i = 0; i < bd->nq_nr_rings; i++)
+ bnge_free_nq(bn, bn->bnapi[i]);
+}
+
+static int bnge_alloc_nqs(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
int i, rc;
for (i = 0; i < bd->nq_nr_rings; i++) {
- struct bnge_napi *bnapi = bn->bnapi[i];
-
- rc = bnge_alloc_nq_desc_arr(&bnapi->nq_ring, bn->cp_nr_pages);
+ rc = bnge_alloc_nq(bn, bn->bnapi[i]);
if (rc)
- goto err_free_nq_arrays;
+ goto err_free_nqs;
}
return 0;
-err_free_nq_arrays:
- bnge_free_nq_arrays(bn);
+err_free_nqs:
+ while (--i >= 0)
+ bnge_free_nq(bn, bn->bnapi[i]);
return rc;
}
-static void bnge_free_nq_tree(struct bnge_net *bn)
+static void bnge_free_cp_rings(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
int i;
for (i = 0; i < bd->nq_nr_rings; i++) {
- struct bnge_napi *bnapi = bn->bnapi[i];
- struct bnge_nq_ring_info *nqr;
- struct bnge_ring_struct *ring;
+ struct bnge_nq_ring_info *nqr = &bn->bnapi[i]->nq_ring;
int j;
- nqr = &bnapi->nq_ring;
- ring = &nqr->ring_struct;
-
- bnge_free_ring(bd, &ring->ring_mem);
-
if (!nqr->cp_ring_arr)
continue;
for (j = 0; j < nqr->cp_ring_count; j++) {
struct bnge_cp_ring_info *cpr = &nqr->cp_ring_arr[j];
- ring = &cpr->ring_struct;
- bnge_free_ring(bd, &ring->ring_mem);
+ bnge_free_ring(bd, &cpr->ring_struct.ring_mem);
bnge_free_cp_desc_arr(cpr);
}
kfree(nqr->cp_ring_arr);
@@ -594,31 +622,21 @@ static int alloc_one_cp_ring(struct bnge_net *bn,
return rc;
}
-static int bnge_alloc_nq_tree(struct bnge_net *bn)
+static int bnge_alloc_cp_rings(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
- int i, j, ulp_msix, rc;
+ int i, j, rc;
int tcs = 1;
- ulp_msix = bnge_aux_get_msix(bd);
for (i = 0, j = 0; i < bd->nq_nr_rings; i++) {
bool sh = !!(bd->flags & BNGE_EN_SHARED_CHNL);
struct bnge_napi *bnapi = bn->bnapi[i];
struct bnge_nq_ring_info *nqr;
struct bnge_cp_ring_info *cpr;
- struct bnge_ring_struct *ring;
int cp_count = 0, k;
int rx = 0, tx = 0;
nqr = &bnapi->nq_ring;
- nqr->bnapi = bnapi;
- ring = &nqr->ring_struct;
-
- rc = bnge_alloc_ring(bd, &ring->ring_mem);
- if (rc)
- goto err_free_nq_tree;
-
- ring->map_idx = ulp_msix + i;
if (i < bd->rx_nr_rings) {
cp_count++;
@@ -634,7 +652,7 @@ static int bnge_alloc_nq_tree(struct bnge_net *bn)
nqr->cp_ring_arr = kzalloc_objs(*cpr, cp_count);
if (!nqr->cp_ring_arr) {
rc = -ENOMEM;
- goto err_free_nq_tree;
+ goto err_free_cp_rings;
}
nqr->cp_ring_count = cp_count;
@@ -643,7 +661,7 @@ static int bnge_alloc_nq_tree(struct bnge_net *bn)
cpr = &nqr->cp_ring_arr[k];
rc = alloc_one_cp_ring(bn, cpr);
if (rc)
- goto err_free_nq_tree;
+ goto err_free_cp_rings;
cpr->bnapi = bnapi;
cpr->cp_idx = k;
@@ -663,8 +681,8 @@ static int bnge_alloc_nq_tree(struct bnge_net *bn)
}
return 0;
-err_free_nq_tree:
- bnge_free_nq_tree(bn);
+err_free_cp_rings:
+ bnge_free_cp_rings(bn);
return rc;
}
@@ -1200,8 +1218,8 @@ 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_cp_rings(bn);
+ bnge_free_nqs(bn);
bnge_free_ring_stats(bn);
bnge_free_ring_grps(bn);
bnge_free_vnics(bn);
@@ -1232,13 +1250,9 @@ static int bnge_alloc_core(struct bnge_net *bn)
bn->bnapi = bnapi;
bnapi += arr_size;
for (i = 0; i < bd->nq_nr_rings; i++, bnapi += size) {
- struct bnge_nq_ring_info *nqr;
-
bn->bnapi[i] = bnapi;
bn->bnapi[i]->index = i;
bn->bnapi[i]->bn = bn;
- nqr = &bn->bnapi[i]->nq_ring;
- nqr->ring_struct.ring_mem.flags = BNGE_RMEM_RING_PTE_FLAG;
}
bn->rx_ring = kzalloc_objs(struct bnge_rx_ring_info, bd->rx_nr_rings);
@@ -1297,7 +1311,7 @@ static int bnge_alloc_core(struct bnge_net *bn)
if (rc)
goto err_free_ring_stats;
- rc = bnge_alloc_nq_arrays(bn);
+ rc = bnge_alloc_nqs(bn);
if (rc)
goto err_free_vnics;
@@ -1305,13 +1319,13 @@ static int bnge_alloc_core(struct bnge_net *bn)
rc = bnge_alloc_rx_rings(bn);
if (rc)
- goto err_free_nq_arrays;
+ goto err_free_nqs;
rc = bnge_alloc_tx_rings(bn);
if (rc)
goto err_free_rx_rings;
- rc = bnge_alloc_nq_tree(bn);
+ rc = bnge_alloc_cp_rings(bn);
if (rc)
goto err_free_tx_rings;
@@ -1320,17 +1334,17 @@ static int bnge_alloc_core(struct bnge_net *bn)
BNGE_VNIC_UCAST_FLAG;
rc = bnge_alloc_vnic_attributes(bn);
if (rc)
- goto err_free_nq_tree;
+ goto err_free_cp_rings;
return 0;
-err_free_nq_tree:
- bnge_free_nq_tree(bn);
+err_free_cp_rings:
+ bnge_free_cp_rings(bn);
err_free_tx_rings:
bnge_free_tx_rings(bn);
err_free_rx_rings:
bnge_free_rx_rings(bn);
-err_free_nq_arrays:
- bnge_free_nq_arrays(bn);
+err_free_nqs:
+ bnge_free_nqs(bn);
err_free_vnics:
bnge_free_vnics(bn);
err_free_ring_stats:
@@ -1389,21 +1403,18 @@ static int bnge_cp_num_to_irq_num(struct bnge_net *bn, int n)
return nqr->ring_struct.map_idx;
}
-static void bnge_init_nq_tree(struct bnge_net *bn)
+static void bnge_init_cp_rings(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
int i, j;
for (i = 0; i < bd->nq_nr_rings; i++) {
struct bnge_nq_ring_info *nqr = &bn->bnapi[i]->nq_ring;
- struct bnge_ring_struct *ring = &nqr->ring_struct;
- 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];
- ring = &cpr->ring_struct;
- ring->fw_ring_id = INVALID_HW_RING_ID_32BIT;
+ cpr->ring_struct.fw_ring_id = INVALID_HW_RING_ID_32BIT;
}
}
}
@@ -1914,6 +1925,44 @@ static int bnge_hwrm_rx_ring_alloc(struct bnge_net *bn,
return 0;
}
+static int bnge_hwrm_nq_alloc(struct bnge_net *bn, struct bnge_napi *bnapi)
+{
+ struct bnge_nq_ring_info *nqr = &bnapi->nq_ring;
+ u32 type = HWRM_RING_ALLOC_NQ;
+ struct bnge_ring_struct *ring;
+ unsigned int vector;
+ u32 map_idx;
+ int rc;
+
+ ring = &nqr->ring_struct;
+ map_idx = ring->map_idx;
+
+ vector = bn->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);
+
+ return 0;
+}
+
+static void bnge_hwrm_nq_free(struct bnge_net *bn, struct bnge_napi *bnapi)
+{
+ struct bnge_ring_struct *ring = &bnapi->nq_ring.ring_struct;
+
+ if (ring->fw_ring_id == INVALID_HW_RING_ID_32BIT)
+ return;
+
+ 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_32BIT;
+}
+
static int bnge_hwrm_ring_alloc(struct bnge_net *bn)
{
struct bnge_dev *bd = bn->bd;
@@ -1923,22 +1972,11 @@ 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;
+ struct bnge_ring_struct *ring = &bnapi->nq_ring.ring_struct;
- 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_alloc(bn, bnapi);
+ 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) {
@@ -2566,23 +2604,14 @@ static void bnge_hwrm_ring_free(struct bnge_net *bn, bool close_path)
bnge_disable_int_sync(bn);
for (i = 0; i < bd->nq_nr_rings; i++) {
- struct bnge_napi *bnapi = bn->bnapi[i];
- struct bnge_nq_ring_info *nqr;
- struct bnge_ring_struct *ring;
+ struct bnge_nq_ring_info *nqr = &bn->bnapi[i]->nq_ring;
int j;
- nqr = &bnapi->nq_ring;
for (j = 0; j < nqr->cp_ring_count && nqr->cp_ring_arr; j++)
bnge_hwrm_cp_ring_free(bn, &nqr->cp_ring_arr[j]);
- ring = &nqr->ring_struct;
- if (ring->fw_ring_id != INVALID_HW_RING_ID_32BIT) {
- 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_32BIT;
- bn->grp_info[i].nq_fw_ring_id = INVALID_HW_RING_ID;
- }
+ bnge_hwrm_nq_free(bn, bn->bnapi[i]);
+ bn->grp_info[i].nq_fw_ring_id = INVALID_HW_RING_ID;
}
}
@@ -2627,27 +2656,60 @@ static void bnge_hwrm_resource_free(struct bnge_net *bn, bool close_path)
bnge_hwrm_stat_ctx_free(bn);
}
-static void bnge_free_irq(struct bnge_net *bn)
+static void bnge_nq_free_irq(struct bnge_net *bn, struct bnge_napi *bnapi)
+{
+ int map_idx = bnapi->nq_ring.ring_struct.map_idx;
+ struct bnge_irq *irq = &bn->bd->irq_tbl[map_idx];
+
+ if (irq->requested) {
+ 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;
+}
+
+static int bnge_nq_request_irq(struct bnge_net *bn, struct bnge_napi *bnapi)
{
+ int map_idx = bnapi->nq_ring.ring_struct.map_idx;
struct bnge_dev *bd = bn->bd;
struct bnge_irq *irq;
- int i;
+ int rc;
- for (i = 0; i < bd->nq_nr_rings; i++) {
- int map_idx = bnge_cp_num_to_irq_num(bn, i);
+ irq = &bd->irq_tbl[map_idx];
- irq = &bd->irq_tbl[map_idx];
- if (irq->requested) {
- 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, bn->bnapi[i]);
- }
+ rc = request_irq(irq->vector, irq->handler, 0, irq->name, bnapi);
+ if (rc)
+ return rc;
+
+ netif_napi_set_irq_locked(&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);
- irq->requested = 0;
+ irq->have_cpumask = 1;
+ cpumask_set_cpu(cpumask_local_spread(bnapi->index, numa_node),
+ 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);
}
+ return 0;
+}
+
+static void bnge_free_irq(struct bnge_net *bn)
+{
+ struct bnge_dev *bd = bn->bd;
+ int i;
+
+ for (i = 0; i < bd->nq_nr_rings; i++)
+ bnge_nq_free_irq(bn, bn->bnapi[i]);
}
static int bnge_request_irq(struct bnge_net *bn)
@@ -2661,29 +2723,9 @@ static int bnge_request_irq(struct bnge_net *bn)
return rc;
}
for (i = 0; i < bd->nq_nr_rings; i++) {
- int map_idx = bnge_cp_num_to_irq_num(bn, i);
- struct bnge_irq *irq = &bd->irq_tbl[map_idx];
-
- rc = request_irq(irq->vector, irq->handler, 0, irq->name,
- bn->bnapi[i]);
+ rc = bnge_nq_request_irq(bn, bn->bnapi[i]);
if (rc)
goto err_free_irq;
-
- netif_napi_set_irq_locked(&bn->bnapi[i]->napi, irq->vector);
- irq->requested = 1;
-
- if (zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL)) {
- int numa_node = dev_to_node(&bd->pdev->dev);
-
- irq->have_cpumask = 1;
- cpumask_set_cpu(cpumask_local_spread(i, numa_node),
- 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);
- }
- }
}
return 0;
@@ -2830,7 +2872,7 @@ static int bnge_init_nic(struct bnge_net *bn)
{
int rc;
- bnge_init_nq_tree(bn);
+ bnge_init_cp_rings(bn);
bnge_init_rx_rings(bn);
rc = bnge_alloc_rx_ring_pair_bufs(bn);
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
index b066ee887a09..bbae84b4157e 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
@@ -431,20 +431,10 @@ 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;
-
rxr = bnapi->rx_ring;
if (!rxr)
goto skip_rx;
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net-next v3 2/5] bnge: add bnge_err()/bnge_warn()/bnge_dbg() logging helpers
2026-10-13 20:06 [PATCH net-next v3 0/5] bnge: Make NQ0 persistent across open/close Bhargava Marreddy
2026-10-13 20:06 ` [PATCH net-next v3 1/5] bnge: factor out per-NQ alloc/free helpers Bhargava Marreddy
@ 2026-10-13 20:06 ` Bhargava Marreddy
2026-10-13 20:06 ` [PATCH net-next v3 3/5] bnge: add bnge_for_each_open_nq() iterator Bhargava Marreddy
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-10-13 20:06 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, Kumar Mudide, Ramakrishna Koduri
netdev_err() and netdev_warn() print "(unnamed net_device)
(uninitialized)" instead of the interface name while the netdev has not
been registered yet. Later patches in this series run NQ0 init before
register_netdev() in probe, so messages from the HWRM ring alloc/free
paths and from NQ0's NAPI would be logged that way.
Add bnge_err(), bnge_warn() and bnge_dbg(), which take a struct bnge_dev
and log through the netdev once it has been registered, including while
it is being unregistered and after, where it keeps its name. Before
that, fall back to dev_err()/dev_warn()/dev_dbg() on the PCI device.
Convert the HWRM ring alloc/free error messages, the IRQ affinity hint
warning and the async event debug message, which are reachable from
those paths. No other message is changed.
No functional change.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Kumar Mudide <kumar.mudide@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
---
drivers/net/ethernet/broadcom/bnge/bnge.h | 18 ++++++++++++++++++
.../net/ethernet/broadcom/bnge/bnge_hwrm_lib.c | 8 +++++---
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 5 ++---
drivers/net/ethernet/broadcom/bnge/bnge_txrx.c | 4 ++--
4 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge.h b/drivers/net/ethernet/broadcom/bnge/bnge.h
index 4479ccd071f5..e76ba4e063bb 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge.h
@@ -220,6 +220,24 @@ struct bnge_dev {
u32 phy_flags;
};
+/* The netdev has no name until register_netdev(), so log through the PCI
+ * device before that (e.g. during probe).
+ */
+#define bnge_log(level, bd, fmt, ...) \
+ do { \
+ struct bnge_dev *__bd = (bd); \
+ \
+ if (__bd->netdev && \
+ READ_ONCE(__bd->netdev->reg_state) != NETREG_UNINITIALIZED)\
+ netdev_##level(__bd->netdev, fmt, ##__VA_ARGS__);\
+ else \
+ dev_##level(__bd->dev, fmt, ##__VA_ARGS__); \
+ } while (0)
+
+#define bnge_err(bd, fmt, ...) bnge_log(err, bd, fmt, ##__VA_ARGS__)
+#define bnge_warn(bd, fmt, ...) bnge_log(warn, bd, fmt, ##__VA_ARGS__)
+#define bnge_dbg(bd, fmt, ...) bnge_log(dbg, bd, fmt, ##__VA_ARGS__)
+
static inline bool bnge_is_roce_en(struct bnge_dev *bd)
{
return bd->flags & BNGE_EN_ROCE;
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c b/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
index 651c5e783516..fb960bff85fa 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
@@ -1302,7 +1302,8 @@ int hwrm_ring_free_send_msg(struct bnge_net *bn,
bnge_hwrm_req_drop(bd, req);
exit:
if (rc) {
- netdev_err(bd->netdev, "hwrm_ring_free type %d failed. rc:%d\n", ring_type, rc);
+ bnge_err(bd, "hwrm_ring_free type %d failed. rc:%d\n",
+ ring_type, rc);
return -EIO;
}
return 0;
@@ -1395,7 +1396,7 @@ int hwrm_ring_alloc_send_msg(struct bnge_net *bn,
req->int_mode = RING_ALLOC_REQ_INT_MODE_MSIX;
break;
default:
- netdev_err(bn->netdev, "hwrm alloc invalid ring type %d\n", ring_type);
+ bnge_err(bd, "hwrm alloc invalid ring type %d\n", ring_type);
return -EINVAL;
}
@@ -1406,7 +1407,8 @@ int hwrm_ring_alloc_send_msg(struct bnge_net *bn,
exit:
if (rc) {
- netdev_err(bd->netdev, "hwrm_ring_alloc type %d failed. rc:%d\n", ring_type, rc);
+ bnge_err(bd, "hwrm_ring_alloc type %d failed. rc:%d\n",
+ ring_type, rc);
return -EIO;
}
ring->fw_ring_id = ring_id;
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index 8cd0d8bd68c2..f8f81f3f7cd9 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -2696,9 +2696,8 @@ static int bnge_nq_request_irq(struct bnge_net *bn, struct bnge_napi *bnapi)
cpumask_set_cpu(cpumask_local_spread(bnapi->index, numa_node),
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);
+ bnge_warn(bd, "Set affinity failed, IRQ = %d\n",
+ irq->vector);
}
return 0;
}
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
index 472b02cc43b3..2f382514c6ea 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
@@ -1135,8 +1135,8 @@ static void bnge_async_event_process(struct bnge_net *bn,
u32 data1 = le32_to_cpu(cmpl->event_data1);
u32 data2 = le32_to_cpu(cmpl->event_data2);
- netdev_dbg(bn->netdev, "hwrm event 0x%x {0x%x, 0x%x}\n",
- event_id, data1, data2);
+ bnge_dbg(bn->bd, "hwrm event 0x%x {0x%x, 0x%x}\n",
+ event_id, data1, data2);
switch (event_id) {
case ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE:
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net-next v3 3/5] bnge: add bnge_for_each_open_nq() iterator
2026-10-13 20:06 [PATCH net-next v3 0/5] bnge: Make NQ0 persistent across open/close Bhargava Marreddy
2026-10-13 20:06 ` [PATCH net-next v3 1/5] bnge: factor out per-NQ alloc/free helpers Bhargava Marreddy
2026-10-13 20:06 ` [PATCH net-next v3 2/5] bnge: add bnge_err()/bnge_warn()/bnge_dbg() logging helpers Bhargava Marreddy
@ 2026-10-13 20:06 ` Bhargava Marreddy
2026-10-13 20:06 ` [PATCH net-next v3 4/5] bnge: make irq_tbl layout depend on BNGE_FIRST_OPEN_NQ Bhargava Marreddy
2026-10-13 20:06 ` [PATCH net-next v3 5/5] bnge: make NQ0 persistent across open/stop cycles Bhargava Marreddy
4 siblings, 0 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-10-13 20:06 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, Kumar Mudide, Ramakrishna Koduri
Add BNGE_FIRST_OPEN_NQ and bnge_for_each_open_nq(), and use them in the
loops that will need to skip the NQ that is created at probe once it is
split out. The "open" NQs are the ones created in ndo_open() and
destroyed in ndo_stop(), i.e. all NQs from index BNGE_FIRST_OPEN_NQ on.
The loop that carves the bnge_napi instances out of the bn->bnapi
allocation in bnge_alloc_core() is converted as well; its allocation
size will be reduced by BNGE_FIRST_OPEN_NQ when the probe-time NQ moves
out of it.
The loops over the completion rings, over the per-NQ stats and ring
group state, and the interrupt disable/sync loops are intentionally
left alone, as they still cover all NQs. The latter must mask and
synchronize the IRQ of the probe-time NQ before its completion rings
are freed.
In bnge_hwrm_ring_free(), split the NQ free out of the completion ring
loop so that it can use the iterator, mirroring bnge_hwrm_ring_alloc().
This changes the order of the firmware commands: previously each NQ was
freed right after its own completion rings, now all completion rings
are freed first and all NQs after. Each completion ring is still freed
before the NQ it belongs to.
BNGE_FIRST_OPEN_NQ is 0 for now, i.e. every NQ is an open NQ, so the
macro expands to the loops it replaces. Apart from the firmware command
order above, no functional change.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Kumar Mudide <kumar.mudide@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
---
drivers/net/ethernet/broadcom/bnge/bnge.h | 9 ++++++
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 29 ++++++++++---------
2 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge.h b/drivers/net/ethernet/broadcom/bnge/bnge.h
index e76ba4e063bb..fdd8bca01440 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge.h
@@ -238,6 +238,15 @@ struct bnge_dev {
#define bnge_warn(bd, fmt, ...) bnge_log(warn, bd, fmt, ##__VA_ARGS__)
#define bnge_dbg(bd, fmt, ...) bnge_log(dbg, bd, fmt, ##__VA_ARGS__)
+/* Index of the first NQ created in ndo_open() and destroyed in ndo_stop().
+ * NQs below it are created at probe and destroyed at remove. It starts at 0,
+ * i.e. all NQs are open NQs, until NQ0 moves to probe.
+ */
+#define BNGE_FIRST_OPEN_NQ 0
+
+#define bnge_for_each_open_nq(i, bd) \
+ for ((i) = BNGE_FIRST_OPEN_NQ; (i) < (bd)->nq_nr_rings; (i)++)
+
static inline bool bnge_is_roce_en(struct bnge_dev *bd)
{
return bd->flags & BNGE_EN_ROCE;
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index f8f81f3f7cd9..0d98223cd703 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -548,7 +548,7 @@ static void bnge_free_nqs(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i;
- for (i = 0; i < bd->nq_nr_rings; i++)
+ bnge_for_each_open_nq(i, bd)
bnge_free_nq(bn, bn->bnapi[i]);
}
@@ -557,7 +557,7 @@ static int bnge_alloc_nqs(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i, rc;
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
rc = bnge_alloc_nq(bn, bn->bnapi[i]);
if (rc)
goto err_free_nqs;
@@ -565,7 +565,7 @@ static int bnge_alloc_nqs(struct bnge_net *bn)
return 0;
err_free_nqs:
- while (--i >= 0)
+ while (--i >= BNGE_FIRST_OPEN_NQ)
bnge_free_nq(bn, bn->bnapi[i]);
return rc;
}
@@ -1249,10 +1249,11 @@ static int bnge_alloc_core(struct bnge_net *bn)
bn->bnapi = bnapi;
bnapi += arr_size;
- for (i = 0; i < bd->nq_nr_rings; i++, bnapi += size) {
+ bnge_for_each_open_nq(i, bd) {
bn->bnapi[i] = bnapi;
bn->bnapi[i]->index = i;
bn->bnapi[i]->bn = bn;
+ bnapi += size;
}
bn->rx_ring = kzalloc_objs(struct bnge_rx_ring_info, bd->rx_nr_rings);
@@ -1970,7 +1971,7 @@ static int bnge_hwrm_ring_alloc(struct bnge_net *bn)
int i, rc = 0;
agg_rings = !!(bnge_is_agg_reqd(bd));
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
struct bnge_napi *bnapi = bn->bnapi[i];
struct bnge_ring_struct *ring = &bnapi->nq_ring.ring_struct;
@@ -2426,7 +2427,7 @@ static void bnge_enable_int(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i;
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
struct bnge_napi *bnapi = bn->bnapi[i];
struct bnge_nq_ring_info *nqr;
@@ -2443,7 +2444,7 @@ static void bnge_disable_napi(struct bnge_net *bn)
if (test_and_set_bit(BNGE_STATE_NAPI_DISABLED, &bn->state))
return;
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
struct bnge_napi *bnapi = bn->bnapi[i];
napi_disable_locked(&bnapi->napi);
@@ -2456,7 +2457,7 @@ static void bnge_enable_napi(struct bnge_net *bn)
int i;
clear_bit(BNGE_STATE_NAPI_DISABLED, &bn->state);
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
struct bnge_napi *bnapi = bn->bnapi[i];
bnapi->in_reset = false;
@@ -2609,7 +2610,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]);
+ }
+ bnge_for_each_open_nq(i, bd) {
bnge_hwrm_nq_free(bn, bn->bnapi[i]);
bn->grp_info[i].nq_fw_ring_id = INVALID_HW_RING_ID;
}
@@ -2622,7 +2625,7 @@ static void bnge_setup_msix(struct bnge_net *bn)
int len, i;
len = sizeof(bd->irq_tbl[0].name);
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
int map_idx = bnge_cp_num_to_irq_num(bn, i);
char *attr;
@@ -2707,7 +2710,7 @@ static void bnge_free_irq(struct bnge_net *bn)
struct bnge_dev *bd = bn->bd;
int i;
- for (i = 0; i < bd->nq_nr_rings; i++)
+ bnge_for_each_open_nq(i, bd)
bnge_nq_free_irq(bn, bn->bnapi[i]);
}
@@ -2721,7 +2724,7 @@ static int bnge_request_irq(struct bnge_net *bn)
netdev_err(bn->netdev, "bnge_setup_interrupts err: %d\n", rc);
return rc;
}
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
rc = bnge_nq_request_irq(bn, bn->bnapi[i]);
if (rc)
goto err_free_irq;
@@ -2840,7 +2843,7 @@ static void bnge_init_napi(struct bnge_net *bn)
struct bnge_napi *bnapi;
int i;
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
bnapi = bn->bnapi[i];
netif_napi_add_config_locked(bn->netdev, &bnapi->napi,
bnge_napi_poll, bnapi->index);
@@ -2857,7 +2860,7 @@ static void bnge_del_napi(struct bnge_net *bn)
for (i = 0; i < bd->tx_nr_rings; i++)
netif_queue_set_napi(bn->netdev, i, NETDEV_QUEUE_TYPE_TX, NULL);
- for (i = 0; i < bd->nq_nr_rings; i++) {
+ bnge_for_each_open_nq(i, bd) {
struct bnge_napi *bnapi = bn->bnapi[i];
__netif_napi_del_locked(&bnapi->napi);
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net-next v3 4/5] bnge: make irq_tbl layout depend on BNGE_FIRST_OPEN_NQ
2026-10-13 20:06 [PATCH net-next v3 0/5] bnge: Make NQ0 persistent across open/close Bhargava Marreddy
` (2 preceding siblings ...)
2026-10-13 20:06 ` [PATCH net-next v3 3/5] bnge: add bnge_for_each_open_nq() iterator Bhargava Marreddy
@ 2026-10-13 20:06 ` Bhargava Marreddy
2026-10-13 20:06 ` [PATCH net-next v3 5/5] bnge: make NQ0 persistent across open/stop cycles Bhargava Marreddy
4 siblings, 0 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-10-13 20:06 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, Rajashekar Hudumula, Ramakrishna Koduri
NQ0 will be created at probe, before the aux (RoCE) device has been
given its MSI-X vectors, and must keep its vector for the lifetime of
the driver. So the probe NQs must come first in irq_tbl, followed by the
aux vectors and then the open NQs, instead of putting all NQs after the
aux vectors.
Describe the irq_tbl layout once, next to BNGE_FIRST_OPEN_NQ, and add
bnge_nq_irq_idx() and bnge_aux_irq_idx() to compute the irq_tbl index of
an NQ and of an aux vector from it. bnge_fill_msix_vecs() uses the
latter to hand the aux device the vectors that follow the probe NQs.
The ring_idx passed to the aux device changes meaning accordingly: it is
the irq_tbl index of the vector, not an index within the aux range. This
matches how NQ rings are created, where the ring index is used as the
logical ID that ties the ring to its doorbell and MSI-X vector. The aux
driver uses the values it is given, so it needs no change. The total
number of vectors is unchanged.
BNGE_FIRST_OPEN_NQ is 0 for now, so the layout is unchanged. No
functional change.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
---
drivers/net/ethernet/broadcom/bnge/bnge.h | 19 +++++++++++++++++++
.../net/ethernet/broadcom/bnge/bnge_auxr.c | 4 ++--
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 2 +-
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge.h b/drivers/net/ethernet/broadcom/bnge/bnge.h
index fdd8bca01440..13452caa9081 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge.h
@@ -241,6 +241,10 @@ struct bnge_dev {
/* Index of the first NQ created in ndo_open() and destroyed in ndo_stop().
* NQs below it are created at probe and destroyed at remove. It starts at 0,
* i.e. all NQs are open NQs, until NQ0 moves to probe.
+ *
+ * Layout of bd->irq_tbl: the NQs created at probe (none yet) come first,
+ * then the aux (RoCE) vectors, then the NQs created in open. The ring_idx
+ * the aux device receives is its irq_tbl index.
*/
#define BNGE_FIRST_OPEN_NQ 0
@@ -290,4 +294,19 @@ static inline void bnge_db_write(struct bnge_dev *bd, struct bnge_db_info *db,
bool bnge_aux_registered(struct bnge_dev *bd);
u16 bnge_aux_get_msix(struct bnge_dev *bd);
+/* irq_tbl index of the i-th aux vector, also used as its ring_idx */
+static inline int bnge_aux_irq_idx(int i)
+{
+ return BNGE_FIRST_OPEN_NQ + i;
+}
+
+/* irq_tbl index of NQ nq_idx */
+static inline int bnge_nq_irq_idx(struct bnge_dev *bd, int nq_idx)
+{
+ if (nq_idx < BNGE_FIRST_OPEN_NQ)
+ return nq_idx;
+
+ return bnge_aux_get_msix(bd) + nq_idx;
+}
+
#endif /* _BNGE_H_ */
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
index 0955b488b6fe..0393bfea7b65 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
@@ -34,9 +34,9 @@ static void bnge_fill_msix_vecs(struct bnge_dev *bd,
}
num_msix = auxr_dev->auxr_info->msix_requested;
for (i = 0; i < num_msix; i++) {
- info[i].vector = bd->irq_tbl[i].vector;
+ info[i].vector = bd->irq_tbl[bnge_aux_irq_idx(i)].vector;
info[i].db_offset = bd->db_offset;
- info[i].ring_idx = i;
+ info[i].ring_idx = bnge_aux_irq_idx(i);
}
}
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index 0d98223cd703..dc6cf3525034 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -539,7 +539,7 @@ static int bnge_alloc_nq(struct bnge_net *bn, struct bnge_napi *bnapi)
}
ring->fw_ring_id = INVALID_HW_RING_ID_32BIT;
- ring->map_idx = bnge_aux_get_msix(bn->bd) + bnapi->index;
+ ring->map_idx = bnge_nq_irq_idx(bn->bd, bnapi->index);
return 0;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net-next v3 5/5] bnge: make NQ0 persistent across open/stop cycles
2026-10-13 20:06 [PATCH net-next v3 0/5] bnge: Make NQ0 persistent across open/close Bhargava Marreddy
` (3 preceding siblings ...)
2026-10-13 20:06 ` [PATCH net-next v3 4/5] bnge: make irq_tbl layout depend on BNGE_FIRST_OPEN_NQ Bhargava Marreddy
@ 2026-10-13 20:06 ` Bhargava Marreddy
2026-10-18 15:32 ` Bhargava Chenna Marreddy
2026-10-20 20:34 ` Jakub Kicinski
4 siblings, 2 replies; 9+ messages in thread
From: Bhargava Marreddy @ 2026-10-13 20:06 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, Rajashekar Hudumula, Ramakrishna Koduri
NQ0 is currently created in ndo_open(), so firmware async events are not
received while the interface is down. This change lets NQ0 receive async
events in the down state, but sp_task bails out unless BNGE_STATE_OPEN is
set. Down-state event processing is left to subsequent patches.
Focus this patch strictly on making NQ0 persistent across
ndo_stop/ndo_open cycles:
- probe: Allocate NQ0, register its NAPI, request its IRQ, allocate
the HWRM ring, configure it as the async event completion
ring (a failure is only warned about, as before), then
enable its NAPI and arm it.
- open: Allocate/create NQ1..N and attach queue 0's RX/TX and completion
rings to NQ0.
- close: Destroy NQ1..N and detach rings from NQ0, leaving NQ0 to handle
async events only.
- remove: Destroy NQ0 before the timer and workqueue, as NQ0 queues
sp_task on async events.
NQ0's NAPI keeps running while the interface is down, so quiesce it
while the host ring structures (Rx, Tx, etc.) are allocated on open and
freed on close. Async events NQ0 receives while down are ignored by
sp_task, so open also clears sp_event while NQ0 is quiesced, and resumes
NQ0 only after BNGE_STATE_OPEN is set, together with the other NQs.
Introduce bnge_nq_drain() to drop pending NQEs on NQ0 once the completion
rings they refer to are gone, while still handling HWRM and async event
completions. Call it with NQ0's NAPI disabled.
For the same reason, bnge_napi_poll() now ignores a CQ notification whose
index is beyond the current completion ring count, instead of indexing
the freed cp_ring_arr.
Only the NQ, its IRQ and its NAPI persist. NQ0 is skipped, via
bnge_for_each_open_nq(), when enabling NQ interrupts, adding/deleting
and enabling/disabling NAPI, requesting/freeing IRQs, naming IRQs,
allocating/freeing NQ rings and the firmware NQ, and its firmware ring
ID is carried over in grp_info[0]. Its completion rings, cp_ring_arr and
stats remain per-open.
Give NQ0 a stable <pci_name>-nq0 IRQ name and retain the existing names
for NQs created during open. As a result, the vector serving queue 0 is
listed in /proc/interrupts under the <pci_name>-nq0 name instead of
<ifname>-TxRx-0, and stays requested while the interface is down.
Set BNGE_FIRST_OPEN_NQ to 1, shifting aux (RoCE) vectors to irq_tbl[1..]
as prepared by earlier patches.
The async event completion ring is now configured once at probe rather
than on every open. This relies on firmware keeping NQ0 allocated, and
the async event completion ring configured, across FUNC_DRV_IF_CHANGE
down/up. Recreating NQ0 after a firmware reset is not handled here.
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
---
drivers/net/ethernet/broadcom/bnge/bnge.h | 13 +-
.../net/ethernet/broadcom/bnge/bnge_netdev.c | 185 ++++++++++++++++--
.../net/ethernet/broadcom/bnge/bnge_netdev.h | 2 +
.../net/ethernet/broadcom/bnge/bnge_txrx.c | 42 ++++
.../net/ethernet/broadcom/bnge/bnge_txrx.h | 1 +
5 files changed, 225 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge.h b/drivers/net/ethernet/broadcom/bnge/bnge.h
index 13452caa9081..950f40d69916 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge.h
@@ -239,14 +239,15 @@ struct bnge_dev {
#define bnge_dbg(bd, fmt, ...) bnge_log(dbg, bd, fmt, ##__VA_ARGS__)
/* Index of the first NQ created in ndo_open() and destroyed in ndo_stop().
- * NQs below it are created at probe and destroyed at remove. It starts at 0,
- * i.e. all NQs are open NQs, until NQ0 moves to probe.
+ * NQs below it are created at probe and destroyed at remove. Exactly one NQ,
+ * NQ0 (async events and queue 0), is created at probe.
*
- * Layout of bd->irq_tbl: the NQs created at probe (none yet) come first,
- * then the aux (RoCE) vectors, then the NQs created in open. The ring_idx
- * the aux device receives is its irq_tbl index.
+ * Layout of bd->irq_tbl, with A = aux MSI-X count: NQ0 uses irq_tbl[0],
+ * the aux (RoCE) device irq_tbl[1..A], and the NQs created in open
+ * irq_tbl[A + 1..]. The ring_idx the aux device receives is its irq_tbl
+ * index.
*/
-#define BNGE_FIRST_OPEN_NQ 0
+#define BNGE_FIRST_OPEN_NQ 1
#define bnge_for_each_open_nq(i, bd) \
for ((i) = BNGE_FIRST_OPEN_NQ; (i) < (bd)->nq_nr_rings; (i)++)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index dc6cf3525034..c6649a2c5876 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -1209,10 +1209,20 @@ static int bnge_init_ring_grps(struct bnge_net *bn)
bn->grp_info[i].agg_fw_ring_id = INVALID_HW_RING_ID;
bn->grp_info[i].nq_fw_ring_id = INVALID_HW_RING_ID;
}
+ /* NQ0 stays allocated in FW across open/close */
+ bn->grp_info[0].nq_fw_ring_id =
+ (u16)bn->nq0->nq_ring.ring_struct.fw_ring_id;
return 0;
}
+static void bnge_nq0_clear_rings(struct bnge_net *bn)
+{
+ bn->nq0->rx_ring = NULL;
+ memset(bn->nq0->tx_ring, 0, sizeof(bn->nq0->tx_ring));
+ bn->nq0->events = 0;
+}
+
static void bnge_free_core(struct bnge_net *bn)
{
bnge_free_vnic_attributes(bn);
@@ -1229,6 +1239,7 @@ static void bnge_free_core(struct bnge_net *bn)
bn->tx_ring = NULL;
kfree(bn->rx_ring);
bn->rx_ring = NULL;
+ bnge_nq0_clear_rings(bn);
kfree(bn->bnapi);
bn->bnapi = NULL;
}
@@ -1243,12 +1254,15 @@ static int bnge_alloc_core(struct bnge_net *bn)
arr_size = L1_CACHE_ALIGN(sizeof(struct bnge_napi *) *
bd->nq_nr_rings);
size = L1_CACHE_ALIGN(sizeof(struct bnge_napi));
- bnapi = kzalloc(arr_size + size * bd->nq_nr_rings, GFP_KERNEL);
+ bnapi = kzalloc(arr_size +
+ size * (bd->nq_nr_rings - BNGE_FIRST_OPEN_NQ),
+ GFP_KERNEL);
if (!bnapi)
return rc;
bn->bnapi = bnapi;
bnapi += arr_size;
+ bn->bnapi[0] = bn->nq0;
bnge_for_each_open_nq(i, bd) {
bn->bnapi[i] = bnapi;
bn->bnapi[i]->index = i;
@@ -1360,6 +1374,7 @@ static int bnge_alloc_core(struct bnge_net *bn)
kfree(bn->rx_ring);
bn->rx_ring = NULL;
err_free_bnapi:
+ bnge_nq0_clear_rings(bn);
kfree(bn->bnapi);
bn->bnapi = NULL;
return rc;
@@ -1973,18 +1988,12 @@ static int bnge_hwrm_ring_alloc(struct bnge_net *bn)
agg_rings = !!(bnge_is_agg_reqd(bd));
bnge_for_each_open_nq(i, bd) {
struct bnge_napi *bnapi = bn->bnapi[i];
- struct bnge_ring_struct *ring = &bnapi->nq_ring.ring_struct;
rc = bnge_hwrm_nq_alloc(bn, bnapi);
if (rc)
goto err_out;
- 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");
- }
+ bn->grp_info[i].nq_fw_ring_id =
+ (u16)bnapi->nq_ring.ring_struct.fw_ring_id;
}
for (i = 0; i < bd->tx_nr_rings; i++) {
@@ -2625,6 +2634,9 @@ static void bnge_setup_msix(struct bnge_net *bn)
int len, i;
len = sizeof(bd->irq_tbl[0].name);
+ /* NQ0 keeps the "<pci_name>-nq0" name set in bnge_nq0_init(). Only the
+ * NQs created in open are named after the netdev here.
+ */
bnge_for_each_open_nq(i, bd) {
int map_idx = bnge_cp_num_to_irq_num(bn, i);
char *attr;
@@ -2870,6 +2882,129 @@ static void bnge_del_napi(struct bnge_net *bn)
synchronize_net();
}
+static void bnge_nq0_quiesce(struct bnge_net *bn)
+{
+ napi_disable_locked(&bn->nq0->napi);
+ /* Wait for RCU grace period after napi_disable_locked(): a poll can
+ * still be arming the NQ doorbell after it clears NAPI_STATE_SCHED.
+ */
+ synchronize_net();
+ bnge_db_nq(bn, &bn->nq0->nq_ring.nq_db, bn->nq0->nq_ring.nq_raw_cons);
+}
+
+static void bnge_nq0_resume(struct bnge_net *bn)
+{
+ struct bnge_nq_ring_info *nqr = &bn->nq0->nq_ring;
+
+ bn->nq0->in_reset = false;
+ bn->nq0->tx_fault = 0;
+ napi_enable_locked(&bn->nq0->napi);
+ bnge_db_nq_arm(bn, &nqr->nq_db, nqr->nq_raw_cons);
+}
+
+static int bnge_nq0_init(struct bnge_net *bn)
+{
+ struct net_device *dev = bn->netdev;
+ struct bnge_nq_ring_info *nqr;
+ struct bnge_dev *bd = bn->bd;
+ struct bnge_napi *bnapi;
+ struct bnge_irq *irq;
+ int rc;
+
+ bnapi = kzalloc_obj(*bnapi);
+ if (!bnapi)
+ return -ENOMEM;
+
+ /* NQ0 helpers hard-code NQ index 0 */
+ BUILD_BUG_ON(BNGE_FIRST_OPEN_NQ != 1);
+
+ bnapi->bn = bn;
+ bnapi->index = 0;
+ bn->nq0 = bnapi;
+ nqr = &bnapi->nq_ring;
+
+ rc = bnge_alloc_nq(bn, bnapi);
+ if (rc)
+ goto err_free_bnapi;
+
+ irq = &bd->irq_tbl[nqr->ring_struct.map_idx];
+ irq->handler = bnge_msix;
+ snprintf(irq->name, sizeof(irq->name), "%s-nq0", pci_name(bd->pdev));
+
+ /* The netdev is not registered yet, but the *_locked() NAPI helpers
+ * expect the instance lock to be held.
+ */
+ netdev_lock(dev);
+ netif_napi_add_config_locked(dev, &bnapi->napi, bnge_napi_poll,
+ bnapi->index);
+
+ rc = bnge_nq_request_irq(bn, bnapi);
+ if (rc)
+ goto err_del_napi;
+
+ rc = bnge_hwrm_nq_alloc(bn, bnapi);
+ if (rc)
+ goto err_free_irq;
+
+ rc = bnge_hwrm_set_async_event_cr(bd, nqr->ring_struct.fw_ring_id);
+ if (rc) {
+ bnge_err(bd, "Failed to set async event completion ring: %d\n",
+ rc);
+ goto err_free_irq;
+ }
+
+ /* Enable NAPI only now that the NQ doorbell is set up, so an early
+ * interrupt cannot make the poll arm an uninitialized doorbell.
+ */
+ napi_enable_locked(&bnapi->napi);
+ bnge_db_nq_arm(bn, &nqr->nq_db, nqr->nq_raw_cons);
+ netdev_unlock(dev);
+ return 0;
+
+err_free_irq:
+ bnge_nq_free_irq(bn, bnapi);
+ __netif_napi_del_locked(&bnapi->napi);
+ netdev_unlock(dev);
+ /* Same order as bnge_nq0_uninit(): no poll may touch the NQ once the
+ * FW ring is freed.
+ */
+ synchronize_net();
+ bnge_hwrm_nq_free(bn, bnapi);
+ bnge_free_nq(bn, bnapi);
+ goto err_free_bnapi;
+err_del_napi:
+ __netif_napi_del_locked(&bnapi->napi);
+ netdev_unlock(dev);
+ synchronize_net();
+ bnge_free_nq(bn, bnapi);
+err_free_bnapi:
+ kfree(bnapi);
+ bn->nq0 = NULL;
+ return rc;
+}
+
+static void bnge_nq0_uninit(struct bnge_net *bn)
+{
+ struct net_device *dev = bn->netdev;
+ struct bnge_napi *bnapi = bn->nq0;
+
+ netdev_lock(dev);
+ napi_disable_locked(&bnapi->napi);
+ bnge_nq_free_irq(bn, bnapi);
+ __netif_napi_del_locked(&bnapi->napi);
+ netdev_unlock(dev);
+ /* Wait for RCU grace period after removing the NAPI instance, and for
+ * a poll that may still be arming the NQ doorbell, before destroying
+ * the FW ring.
+ */
+ synchronize_net();
+
+ bnge_hwrm_nq_free(bn, bnapi);
+ bnge_free_nq(bn, bnapi);
+ kfree(bnapi);
+ bn->nq0 = NULL;
+}
+
static int bnge_init_nic(struct bnge_net *bn)
{
int rc;
@@ -2976,10 +3111,17 @@ static int bnge_open_core(struct bnge_net *bn)
goto err_if_change;
}
+ bnge_nq0_quiesce(bn);
+
+ /* Drop async events NQ0 received while down. Its NAPI is quiesced, so
+ * nothing can set bits concurrently.
+ */
+ WRITE_ONCE(bn->sp_event, 0);
+
rc = bnge_alloc_core(bn);
if (rc) {
netdev_err(bn->netdev, "bnge_alloc_core err: %d\n", rc);
- goto err_if_change;
+ goto err_nq0_resume;
}
bnge_init_napi(bn);
@@ -3011,6 +3153,8 @@ static int bnge_open_core(struct bnge_net *bn)
set_bit(BNGE_STATE_STATS_ENABLE, &bn->state);
bnge_enable_int(bn);
+ /* Resume NQ0 with the other NQs, once BNGE_STATE_OPEN is set */
+ bnge_nq0_resume(bn);
bnge_tx_enable(bn);
@@ -3022,10 +3166,13 @@ static int bnge_open_core(struct bnge_net *bn)
return 0;
err_free_irq:
+ bnge_nq_drain(bn, bn->nq0);
bnge_free_irq(bn);
err_del_napi:
bnge_del_napi(bn);
bnge_free_core(bn);
+err_nq0_resume:
+ bnge_nq0_resume(bn);
err_if_change:
bnge_hwrm_if_change(bd, false);
return rc;
@@ -3238,6 +3385,8 @@ static void bnge_close_core(struct bnge_net *bn)
timer_delete_sync(&bn->timer);
bnge_shutdown_nic(bn);
bnge_disable_napi(bn);
+ bnge_nq0_quiesce(bn);
+ bnge_nq_drain(bn, bn->nq0);
/* Save ring stats before shutdown */
spin_lock_bh(&bn->stats_lock);
@@ -3250,6 +3399,7 @@ static void bnge_close_core(struct bnge_net *bn)
bnge_del_napi(bn);
bnge_free_core(bn);
+ bnge_nq0_resume(bn);
}
static int bnge_close(struct net_device *dev)
@@ -3259,7 +3409,6 @@ static int bnge_close(struct net_device *dev)
bnge_close_core(bn);
bnge_hwrm_shutdown_link(bn->bd);
bnge_hwrm_if_change(bn->bd, false);
- bn->sp_event = 0;
return 0;
}
@@ -3593,14 +3742,23 @@ int bnge_netdev_alloc(struct bnge_dev *bd, int max_irqs)
spin_lock_init(&bn->stats_lock);
netdev->request_ops_lock = true;
+
+ rc = bnge_nq0_init(bn);
+ if (rc) {
+ dev_err(bd->dev, "NQ0 init failed rc: %d\n", rc);
+ goto err_free_port_stats;
+ }
+
rc = register_netdev(netdev);
if (rc) {
dev_err(bd->dev, "Register netdev failed rc: %d\n", rc);
- goto err_free_port_stats;
+ goto err_uninit_nq0;
}
return 0;
+err_uninit_nq0:
+ bnge_nq0_uninit(bn);
err_free_port_stats:
bnge_free_port_stats(bn);
err_free_workq:
@@ -3620,6 +3778,9 @@ void bnge_netdev_free(struct bnge_dev *bd)
unregister_netdev(netdev);
+ /* NQ0 can queue sp_task; destroy it before the timer and workqueue */
+ bnge_nq0_uninit(bn);
+
timer_shutdown_sync(&bn->timer);
cancel_work_sync(&bn->sp_task);
bn->sp_event = 0;
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.h b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.h
index 476b5bab96fe..118a2e4f5331 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.h
@@ -315,6 +315,8 @@ struct bnge_net {
u32 hash_seed;
u64 toeplitz_prefix;
+ /* Lives from probe to remove. bnapi[0] points to it. */
+ struct bnge_napi *nq0;
struct bnge_napi **bnapi;
struct bnge_rx_ring_info *rx_ring;
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
index 2f382514c6ea..9324ee889034 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.c
@@ -1375,6 +1375,13 @@ int bnge_napi_poll(struct napi_struct *napi, int budget)
break;
idx = BNGE_NQ_HDL_IDX(idx);
+ /* NQ0 outlives its completion rings: ignore a stale
+ * notification for a CQ that no longer exists.
+ */
+ if (unlikely(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);
@@ -1396,6 +1403,41 @@ int bnge_napi_poll(struct napi_struct *napi, int budget)
return work_done;
}
+/* Consume pending NQ entries without polling the CP rings. The NQ's NAPI must
+ * be disabled (or quiesced) by the caller. HWRM events are handled, CQ
+ * notifications are dropped. cpr->had_nqe_notify is not reset, so the CP rings
+ * must be freed or re-initialised afterwards.
+ */
+void bnge_nq_drain(struct bnge_net *bn, struct bnge_napi *bnapi)
+{
+ struct bnge_nq_ring_info *nqr = &bnapi->nq_ring;
+ u32 raw_cons = nqr->nq_raw_cons;
+
+ while (1) {
+ u32 cons = RING_CMP(bn, raw_cons);
+ struct nqe_cn *nqcmp;
+ u16 type;
+
+ nqcmp = &nqr->desc_ring[CP_RING(cons)][CP_IDX(cons)];
+ if (!NQ_CMP_VALID(bn, nqcmp, raw_cons))
+ break;
+
+ /* The valid test of the entry must be done first before
+ * reading any further.
+ */
+ dma_rmb();
+
+ type = le16_to_cpu(nqcmp->type);
+ if (NQE_CN_TYPE(type) != NQ_CN_TYPE_CQ_NOTIFICATION)
+ bnge_hwrm_handler(bn, (struct tx_cmp *)nqcmp);
+ raw_cons = NEXT_RAW_CMP(raw_cons);
+ }
+
+ nqr->has_more_work = 0;
+ nqr->nq_raw_cons = raw_cons;
+ BNGE_DB_NQ(bn->bd, &nqr->nq_db, raw_cons);
+}
+
static u16 bnge_xmit_get_cfa_action(struct sk_buff *skb)
{
struct metadata_dst *md_dst = skb_metadata_dst(skb);
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.h b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.h
index bd0aa6c221a4..5779b78fb6a2 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_txrx.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_txrx.h
@@ -120,6 +120,7 @@ irqreturn_t bnge_msix(int irq, void *dev_instance);
netdev_tx_t bnge_start_xmit(struct sk_buff *skb, struct net_device *dev);
void bnge_reuse_rx_data(struct bnge_rx_ring_info *rxr, u16 cons, void *data);
int bnge_napi_poll(struct napi_struct *napi, int budget);
+void bnge_nq_drain(struct bnge_net *bn, struct bnge_napi *bnapi);
netdev_features_t bnge_features_check(struct sk_buff *skb,
struct net_device *dev,
netdev_features_t features);
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net-next v3 5/5] bnge: make NQ0 persistent across open/stop cycles
2026-10-13 20:06 ` [PATCH net-next v3 5/5] bnge: make NQ0 persistent across open/stop cycles Bhargava Marreddy
@ 2026-10-18 15:32 ` Bhargava Chenna Marreddy
2026-10-18 17:20 ` Bhargava Chenna Marreddy
2026-10-20 20:34 ` Jakub Kicinski
1 sibling, 1 reply; 9+ messages in thread
From: Bhargava Chenna Marreddy @ 2026-10-18 15:32 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
Cc: netdev, linux-kernel, michael.chan, pavan.chebbi,
vsrama-krishna.nemani, vikas.gupta, Dharmender Garg,
Rajashekar Hudumula, Ramakrishna Koduri
[-- Attachment #1: Type: text/plain, Size: 3727 bytes --]
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Source: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013200645.1346299-1-bhargava.marreddy@broadcom.com
On Wed, Oct 14, 2026 at 1:38 AM Bhargava Marreddy
<bhargava.marreddy@broadcom.com> wrote:
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> index dc6cf3525034..c6649a2c5876 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> @@ -1209,10 +1209,20 @@ static int bnge_init_ring_grps(struct bnge_net *bn)
> bn->grp_info[i].agg_fw_ring_id = INVALID_HW_RING_ID;
> bn->grp_info[i].nq_fw_ring_id = INVALID_HW_RING_ID;
> }
> + /* NQ0 stays allocated in FW across open/close */
> + bn->grp_info[0].nq_fw_ring_id =
> + (u16)bn->nq0->nq_ring.ring_struct.fw_ring_id;
Discovered by: gpt-6-astra, opus-5-5 · Ranked Medium by: gpt-6-astra
bnge_nq0_init() now allocates NQ0 in firmware and sets it as the async
event completion ring only once. After that, every open reuses the
probe-time fw_ring_id here, and bnge_nq0_resume() re-arms the cached NQ0
doorbell.
On the way back up, bnge_hwrm_if_change(bd, true) returns only rc and
does not check any response flags:
if (up)
req->flags = cpu_to_le32(FUNC_DRV_IF_CHANGE_REQ_FLAGS_UP);
return bnge_hwrm_req_send(bd, req);
Suppose firmware resets or reclaims the function's NQ while the
interface is down. Would queue 0's completion ring then be allocated
against a stale nq_fw_ring_id? NQ0 doorbell writes would target a ring
firmware no longer owns, and async events would stop with no error.
The commit message acknowledges this ("Recreating NQ0 after a firmware
reset is not handled here"). Other probe-only firmware state would also
be stale in that case, such as the context memory from
bnge_alloc_ctx_mem() and the registration from
bnge_hwrm_func_drv_rgtr(). So the driver does not handle a firmware
reset while down today either.
Does firmware guarantee that it keeps NQ0 and the async event CR
configuration across FUNC_DRV_IF_CHANGE down/up?
> +
> + rc = bnge_hwrm_set_async_event_cr(bd, nqr->ring_struct.fw_ring_id);
> + if (rc) {
> + bnge_err(bd, "Failed to set async event completion ring: %d\n",
> + rc);
> + goto err_free_irq;
> + }
The commit message says probe will "configure it as the async event
completion ring (a failure is only warned about, as before)". Does that
still match the code?
Here, if bnge_hwrm_set_async_event_cr() fails, the error is logged with
bnge_err() and the code jumps to err_free_irq. That tears NQ0 down and
returns rc. bnge_netdev_alloc() then treats the error as fatal:
rc = bnge_nq0_init(bn);
if (rc) {
dev_err(bd->dev, "NQ0 init failed rc: %d\n", rc);
goto err_free_port_stats;
}
So register_netdev() is never called and probe fails.
Before this patch, bnge_hwrm_ring_alloc() only warned and kept going:
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");
}
Before, a firmware rejection of this FUNC_CFG request still left a
working interface, just without async events. Now the port gets no
netdev at all.
Should this failure stay warn-only, as the commit message says? If it
is meant to be fatal at probe, could the commit message say so?
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5496 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next v3 5/5] bnge: make NQ0 persistent across open/stop cycles
2026-10-18 15:32 ` Bhargava Chenna Marreddy
@ 2026-10-18 17:20 ` Bhargava Chenna Marreddy
0 siblings, 0 replies; 9+ messages in thread
From: Bhargava Chenna Marreddy @ 2026-10-18 17:20 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
Cc: netdev, linux-kernel, michael.chan, pavan.chebbi,
vsrama-krishna.nemani, vikas.gupta, Dharmender Garg,
Rajashekar Hudumula, Ramakrishna Koduri
[-- Attachment #1: Type: text/plain, Size: 4766 bytes --]
On Sun, Oct 18, 2026 at 9:02 PM Bhargava Chenna Marreddy
<bhargava.marreddy@broadcom.com> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> Source: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013200645.1346299-1-bhargava.marreddy@broadcom.com
>
> On Wed, Oct 14, 2026 at 1:38 AM Bhargava Marreddy
> <bhargava.marreddy@broadcom.com> wrote:
> > diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> > index dc6cf3525034..c6649a2c5876 100644
> > --- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> > +++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
> > @@ -1209,10 +1209,20 @@ static int bnge_init_ring_grps(struct bnge_net *bn)
> > bn->grp_info[i].agg_fw_ring_id = INVALID_HW_RING_ID;
> > bn->grp_info[i].nq_fw_ring_id = INVALID_HW_RING_ID;
> > }
> > + /* NQ0 stays allocated in FW across open/close */
> > + bn->grp_info[0].nq_fw_ring_id =
> > + (u16)bn->nq0->nq_ring.ring_struct.fw_ring_id;
>
> Discovered by: gpt-6-astra, opus-5-5 · Ranked Medium by: gpt-6-astra
>
> bnge_nq0_init() now allocates NQ0 in firmware and sets it as the async
> event completion ring only once. After that, every open reuses the
> probe-time fw_ring_id here, and bnge_nq0_resume() re-arms the cached NQ0
> doorbell.
>
> On the way back up, bnge_hwrm_if_change(bd, true) returns only rc and
> does not check any response flags:
>
> if (up)
> req->flags = cpu_to_le32(FUNC_DRV_IF_CHANGE_REQ_FLAGS_UP);
> return bnge_hwrm_req_send(bd, req);
>
> Suppose firmware resets or reclaims the function's NQ while the
> interface is down. Would queue 0's completion ring then be allocated
> against a stale nq_fw_ring_id? NQ0 doorbell writes would target a ring
> firmware no longer owns, and async events would stop with no error.
>
> The commit message acknowledges this ("Recreating NQ0 after a firmware
> reset is not handled here"). Other probe-only firmware state would also
> be stale in that case, such as the context memory from
> bnge_alloc_ctx_mem() and the registration from
> bnge_hwrm_func_drv_rgtr(). So the driver does not handle a firmware
> reset while down today either.
>
> Does firmware guarantee that it keeps NQ0 and the async event CR
> configuration across FUNC_DRV_IF_CHANGE down/up?
Yes. Firmware does not touch NQ0 or async event support across
FUNC_DRV_IF_CHANGE down/up transitions, so the fw_ring_id cached during probe
remains valid across open/stop cycles.
A full firmware reset is a different case. NQ0, context memory, and driver
registration would all need to be recreated—which the driver does not support
today, with or without this patch. That recovery path is out of scope for this
series, as noted in the commit message.
>
> > +
> > + rc = bnge_hwrm_set_async_event_cr(bd, nqr->ring_struct.fw_ring_id);
> > + if (rc) {
> > + bnge_err(bd, "Failed to set async event completion ring: %d\n",
> > + rc);
> > + goto err_free_irq;
> > + }
>
> The commit message says probe will "configure it as the async event
> completion ring (a failure is only warned about, as before)". Does that
> still match the code?
>
> Here, if bnge_hwrm_set_async_event_cr() fails, the error is logged with
> bnge_err() and the code jumps to err_free_irq. That tears NQ0 down and
> returns rc. bnge_netdev_alloc() then treats the error as fatal:
>
> rc = bnge_nq0_init(bn);
> if (rc) {
> dev_err(bd->dev, "NQ0 init failed rc: %d\n", rc);
> goto err_free_port_stats;
> }
>
> So register_netdev() is never called and probe fails.
>
> Before this patch, bnge_hwrm_ring_alloc() only warned and kept going:
>
> 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");
> }
>
> Before, a firmware rejection of this FUNC_CFG request still left a
> working interface, just without async events. Now the port gets no
> netdev at all.
>
> Should this failure stay warn-only, as the commit message says? If it
> is meant to be fatal at probe, could the commit message say so?
The failing probe is intentional. Without async event support, the driver
cannot receive firmware async events, so a netdev coming up without it would be
only partially functional. The current commit message is inaccurate on this
point, which I can fix in a future revision.
Thanks,
Bhargava, Marreddy
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5496 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next v3 5/5] bnge: make NQ0 persistent across open/stop cycles
2026-10-13 20:06 ` [PATCH net-next v3 5/5] bnge: make NQ0 persistent across open/stop cycles Bhargava Marreddy
2026-10-18 15:32 ` Bhargava Chenna Marreddy
@ 2026-10-20 20:34 ` Jakub Kicinski
1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-10-20 20:34 UTC (permalink / raw)
To: Bhargava Marreddy
Cc: davem, edumazet, pabeni, andrew+netdev, horms, netdev,
linux-kernel, michael.chan, pavan.chebbi, vsrama-krishna.nemani,
vikas.gupta, Dharmender Garg, Rajashekar Hudumula,
Ramakrishna Koduri
On Wed, 14 Oct 2026 01:36:45 +0530 Bhargava Marreddy wrote:
> NQ0's NAPI keeps running while the interface is down
Sounds like abuse of netdev infra. Creative reuse of this sort makes it
extremely painful to add stack-level features.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 9+ messages in thread