mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net 0/2] bnge fixes
@ 2026-10-12  5:43 Vikas Gupta
  2026-10-12  5:43 ` [PATCH net 1/2] bnge: fix RSS table sizing for large RX ring counts Vikas Gupta
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Vikas Gupta @ 2026-10-12  5:43 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
  Cc: netdev, linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde, Vikas Gupta

Hi,

This series fixes RSS indirection table and context sizing for large RX
ring counts. Patch 1 caps the hardware RSS context count and RX rings
to the RSS table size, preventing out-of-bounds writes and an oversized
memset(). Patch 2 fixes the indirection table sizing so rings above
the table size still receive default RSS traffic.

Thanks,
Vikas

Dharmender Garg (2):
  bnge: fix RSS table sizing for large RX ring counts
  bnge: cap RSS contexts and RX rings to the RSS table size

 drivers/net/ethernet/broadcom/bnge/bnge_resc.c | 6 ++++--
 drivers/net/ethernet/broadcom/bnge/bnge_resc.h | 9 +++------
 2 files changed, 7 insertions(+), 8 deletions(-)

-- 
2.52.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 1/2] bnge: fix RSS table sizing for large RX ring counts
  2026-10-12  5:43 [PATCH net 0/2] bnge fixes Vikas Gupta
@ 2026-10-12  5:43 ` Vikas Gupta
  2026-10-13  5:44   ` netdev-bot+sashiko
  2026-10-12  5:43 ` [PATCH net 2/2] bnge: cap RSS contexts and RX rings to the RSS table size Vikas Gupta
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Vikas Gupta @ 2026-10-12  5:43 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
  Cc: netdev, linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde, Vikas Gupta

From: Dharmender Garg <dharmender.garg@broadcom.com>

bnge_adjust_pow_two() adds one block before rounding up to a power of
two only when the quotient is 0 or 1. A larger quotient that is already
a power of two is returned unchanged, so 129-192 RX rings get a
128-entry RSS indirection table and 257-320 rings a 256-entry one. The
rings above the table size never receive default RSS traffic.
bnxt_calc_nr_ring_pages() always adds one, which is correct.

Always add one block before rounding up. The same helper sizes the ring
pages; their count only changes when the number of entries divided by
the entries per page is a power of two of 2 or more, which does not
happen with the default ring sizes.

Fixes: 627c67f038d2 ("bng_en: Add resource management support")
Signed-off-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnge/bnge_resc.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.h b/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
index b62a634669f6..c0b6992d7f82 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
@@ -81,13 +81,10 @@ bnge_adjust_pow_two(u32 total_ent, u16 ent_per_blk)
 {
 	u32 blks = total_ent / ent_per_blk;
 
-	if (blks == 0 || blks == 1)
-		return ++blks;
+	if (!blks)
+		return 1;
 
-	if (!is_power_of_2(blks))
-		blks = roundup_pow_of_two(blks);
-
-	return blks;
+	return roundup_pow_of_two(blks + 1);
 }
 
 #define BNGE_MAX_ROCE_MSIX		64
-- 
2.52.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 2/2] bnge: cap RSS contexts and RX rings to the RSS table size
  2026-10-12  5:43 [PATCH net 0/2] bnge fixes Vikas Gupta
  2026-10-12  5:43 ` [PATCH net 1/2] bnge: fix RSS table sizing for large RX ring counts Vikas Gupta
@ 2026-10-12  5:43 ` Vikas Gupta
  2026-10-13  5:44   ` netdev-bot+sashiko
  2026-10-12  5:50 ` [PATCH net 0/2] bnge fixes netdev-bot+sinfo
  2026-10-12 11:51 ` Björn Töpel
  3 siblings, 1 reply; 9+ messages in thread
From: Vikas Gupta @ 2026-10-12  5:43 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
  Cc: netdev, linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde, Vikas Gupta

From: Dharmender Garg <dharmender.garg@broadcom.com>

bnge_cal_nr_rss_ctxs() has no upper bound. The VNIC holds at most
BNGE_RSS_TABLE_MAX_TBL (8) RSS contexts, in fw_rss_cos_lb_ctx[] and in
a 512-entry indirection table and DMA buffer. With 577 or more RX rings
the function returns more than 8, so bnge_setup_vnic() writes past
vnic->fw_rss_cos_lb_ctx[], bnge_set_dflt_rss_indir_tbl() and
bnge_fill_hw_rss_tbl() write past the indirection table and its DMA
buffer, and the pad computation in bnge_set_dflt_rss_indir_tbl() and
bnge_modify_rss() underflows into a huge memset().

Cap the number of contexts at BNGE_RSS_TABLE_MAX_TBL and the number of
RX rings at BNGE_MAX_RSS_TABLE_ENTRIES, so the sizes derived from the
ring count always fit the arrays.

Fixes: 627c67f038d2 ("bng_en: Add resource management support")
Signed-off-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnge/bnge_resc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
index 4711dd4945ff..b540eed12fc6 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
@@ -182,8 +182,9 @@ int bnge_cal_nr_rss_ctxs(u16 rx_rings)
 	if (!rx_rings)
 		return 0;
 
-	return bnge_adjust_pow_two(rx_rings - 1,
-				   BNGE_RSS_TABLE_ENTRIES);
+	return min_t(int, bnge_adjust_pow_two(rx_rings - 1,
+					      BNGE_RSS_TABLE_ENTRIES),
+		     BNGE_RSS_TABLE_MAX_TBL);
 }
 
 static u16 bnge_rss_ctxs_in_use(struct bnge_dev *bd,
@@ -459,6 +460,7 @@ static void _bnge_get_max_rings(struct bnge_dev *bd, u16 *max_rx,
 	}
 
 	*max_rx = min_t(int, *max_rx, max_ring_grps);
+	*max_rx = min_t(int, *max_rx, BNGE_MAX_RSS_TABLE_ENTRIES);
 }
 
 static int bnge_get_max_rings(struct bnge_dev *bd, u16 *max_rx,
-- 
2.52.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 0/2] bnge fixes
  2026-10-12  5:43 [PATCH net 0/2] bnge fixes Vikas Gupta
  2026-10-12  5:43 ` [PATCH net 1/2] bnge: fix RSS table sizing for large RX ring counts Vikas Gupta
  2026-10-12  5:43 ` [PATCH net 2/2] bnge: cap RSS contexts and RX rings to the RSS table size Vikas Gupta
@ 2026-10-12  5:50 ` netdev-bot+sinfo
  2026-10-12  5:56   ` Vikas Gupta
  2026-10-12 11:51 ` Björn Töpel
  3 siblings, 1 reply; 9+ messages in thread
From: netdev-bot+sinfo @ 2026-10-12  5:50 UTC (permalink / raw)
  To: Vikas Gupta
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev, horms, netdev,
	linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 0/2] bnge fixes
  2026-10-12  5:50 ` [PATCH net 0/2] bnge fixes netdev-bot+sinfo
@ 2026-10-12  5:56   ` Vikas Gupta
  0 siblings, 0 replies; 9+ messages in thread
From: Vikas Gupta @ 2026-10-12  5:56 UTC (permalink / raw)
  To: netdev-bot+sinfo
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev, horms, netdev,
	linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde

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

Hi,
  We found these issues during the Sashiko code review.
  The fixes were tested on the Thor Ultra platform.

Thanks,
Vikas

On Mon, Oct 12, 2026 at 11:20 AM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
>  - What hardware the change was tested on. For driver fixes please
>    mention the device (and if relevant firmware version) used for
>    testing, or say that the change was not tested on real hardware.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 0/2] bnge fixes
  2026-10-12  5:43 [PATCH net 0/2] bnge fixes Vikas Gupta
                   ` (2 preceding siblings ...)
  2026-10-12  5:50 ` [PATCH net 0/2] bnge fixes netdev-bot+sinfo
@ 2026-10-12 11:51 ` Björn Töpel
  3 siblings, 0 replies; 9+ messages in thread
From: Björn Töpel @ 2026-10-12 11:51 UTC (permalink / raw)
  To: Vikas Gupta, davem, edumazet, kuba, pabeni, andrew+netdev, horms
  Cc: netdev, linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde, Vikas Gupta

Vikas Gupta <vikas.gupta@broadcom.com> writes:

> Hi,
>
> This series fixes RSS indirection table and context sizing for large RX
> ring counts. Patch 1 caps the hardware RSS context count and RX rings
> to the RSS table size, preventing out-of-bounds writes and an oversized
> memset(). Patch 2 fixes the indirection table sizing so rings above
> the table size still receive default RSS traffic.
>
> Thanks,
> Vikas
>
> Dharmender Garg (2):
>   bnge: fix RSS table sizing for large RX ring counts
>   bnge: cap RSS contexts and RX rings to the RSS table size

LGTM. Tiny nit: If you respin for some reason, the patch order should be
reversed -- capping before the rounding fix?

Reviewed-by: Björn Töpel <bjorn@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] bnge: fix RSS table sizing for large RX ring counts
  2026-10-12  5:43 ` [PATCH net 1/2] bnge: fix RSS table sizing for large RX ring counts Vikas Gupta
@ 2026-10-13  5:44   ` netdev-bot+sashiko
  2026-10-13  6:42     ` Vikas Gupta
  0 siblings, 1 reply; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13  5:44 UTC (permalink / raw)
  To: vikas.gupta
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev, horms, netdev,
	linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · 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).

- [High] This commit opens a heap and DMA out-of-bounds write window for
  513-576 RX rings.
- [Medium] The commit message is wrong about the completion ring.

--

bnge: fix RSS table sizing for large RX ring counts

This changes bnge_adjust_pow_two() to always add one block before rounding
up to a power of two. RX ring counts whose quotient is already a power of
two now get an RSS indirection table that covers all rings. The same
helper also sizes the RX, TX, agg and completion ring pages.

> Always add one block before rounding up. The same helper sizes the ring
> pages; their count only changes when the number of entries divided by
> the entries per page is a power of two of 2 or more, which does not
> happen with the default ring sizes.

[Severity: Medium]
Is this accurate for the completion ring? It holds for the RX, TX and agg
rings at the default depth of 511. The completion ring, though, is sized
from more than the default depths:

bnge_netdev.c:bnge_set_ring_params() {
    ...
	max_rx_cmpl = bn->rx_ring_size;

	if (bn->priv_flags & BNGE_NET_EN_TPA)
		max_rx_cmpl += bd->max_tpa_v2;
	ring_size = max_rx_cmpl * 2 + agg_ring_size + bn->tx_ring_size;
	bn->cp_ring_size = ring_size;

	bn->cp_nr_pages = bnge_adjust_pow_two(ring_size, CP_DESC_CNT);
    ...
}

Take BNGE_PAGE_SIZE of 8192 (for example arm64 with 16K pages), TPA off
and default 511/511 rings. Then ring_size is 2 * 511 + 511 = 1533 and
CP_DESC_CNT is 512, so the quotient is 2. The old code returned 2 pages,
which gives a cp_bit of 1024 (fewer slots than cp_ring_size). The new code
returns 4.

With 4K pages and TPA on, agg_ring_size is 2044 and ring_size is
3577 + 2 * max_tpa_v2. max_tpa_v2 comes from firmware max_aggs_supported.
For values of 260..387, the quotient over a CP_DESC_CNT of 256 is 16, and
the page count goes from 16 to 32.

Could the commit message mention this change to completion ring sizing?

> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.h b/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> index b62a634669f62..c0b6992d7f828 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> @@ -81,13 +81,10 @@ bnge_adjust_pow_two(u32 total_ent, u16 ent_per_blk)
>  {
>  	u32 blks = total_ent / ent_per_blk;
>  
> -	if (blks == 0 || blks == 1)
> -		return ++blks;
> +	if (!blks)
> +		return 1;
>  
> -	if (!is_power_of_2(blks))
> -		blks = roundup_pow_of_two(blks);
> -
> -	return blks;
> +	return roundup_pow_of_two(blks + 1);

[Severity: High]
For 513..576 RX rings, bnge_cal_nr_rss_ctxs() calls
bnge_adjust_pow_two(rx_rings - 1, 64), which gives a quotient of 8. The
old code returned 8, but this now returns 16. Nothing clamps the result
to BNGE_RSS_TABLE_MAX_TBL (8):

bnge_resc.c:bnge_cal_nr_rss_ctxs() {
	...
	return bnge_adjust_pow_two(rx_rings - 1,
				   BNGE_RSS_TABLE_ENTRIES);
}

Can this overflow bd->rss_indir_tbl[]? bnge_alloc_rss_indir_tbl()
allocates BNGE_MAX_RSS_TABLE_ENTRIES (512) entries. On the first
reservation, bnge_reserve_rings()->bnge_set_dflt_rss_indir_tbl() gets a
max_entries of 16 * 64 = 1024:

bnge_resc.c:bnge_set_dflt_rss_indir_tbl() {
	...
	for (i = 0; i < max_entries; i++)
		rss_indir_tbl[i] = ethtool_rxfh_indir_default(i,
							      bd->rx_nr_rings);

	pad = bd->rss_indir_tbl_entries - max_entries;
	if (pad)
		memset(&rss_indir_tbl[i], 0, pad * sizeof(*rss_indir_tbl));
}

The loop writes 512 entries past the end. Then the u16 pad wraps to
512 - 1024 = 65024, so the memset() clears about 254 KB beyond that.

The other RSS consumers also look sized for at most 8 contexts:

- bnge_setup_vnic() calls bnge_hwrm_vnic_ctx_alloc(bd, vnic, i) for
  i < 16. That stores into vnic->fw_rss_cos_lb_ctx[], which has only
  BNGE_MAX_CTX_PER_VNIC (8) entries.

- bnge_fill_hw_rss_tbl() uses a tbl_size of 1024. It reads
  rss_indir_tbl[] past 512 entries and writes 4096 bytes into
  vnic->rss_table. bnge_alloc_vnic_attributes() sizes that buffer from
  BNGE_MAX_RSS_TABLE_SIZE (2048 bytes plus the hash key).

- bnge_hwrm_vnic_set_rss() reads fw_rss_cos_lb_ctx[8..15]. It also moves
  ring_grp_tbl_addr past the end of the coherent allocation, so firmware
  would DMA read outside the buffer.

Configurations with 577 or more RX rings already overflowed before this
patch, but this patch lowers the threshold to 513. Hitting it needs at
least 513 default RSS queues from bnge_net_init_dflt_rings() and firmware
that grants that many rings.

The next patch in the series, "bnge: cap RSS contexts and RX rings to the
RSS table size", closes this. It adds a min_t() against
BNGE_RSS_TABLE_MAX_TBL in bnge_cal_nr_rss_ctxs() and caps *max_rx at
BNGE_MAX_RSS_TABLE_ENTRIES in _bnge_get_max_rings().

Could that cap be folded into this patch, or moved ahead of it, so this
commit can be bisected or backported safely on its own?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012054325.1782819-1-vikas.gupta%40broadcom.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] bnge: cap RSS contexts and RX rings to the RSS table size
  2026-10-12  5:43 ` [PATCH net 2/2] bnge: cap RSS contexts and RX rings to the RSS table size Vikas Gupta
@ 2026-10-13  5:44   ` netdev-bot+sashiko
  0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13  5:44 UTC (permalink / raw)
  To: vikas.gupta
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev, horms, netdev,
	linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the overflow happens with "577 or more RX
  rings".
- [Low] The commit message says the pad computation "in
  bnge_set_dflt_rss_indir_tbl() and bnge_modify_rss() underflows into a…

--

bnge: cap RSS contexts and RX rings to the RSS table size

This patch caps the value returned by bnge_cal_nr_rss_ctxs() at
BNGE_RSS_TABLE_MAX_TBL. It also limits max_rx in _bnge_get_max_rings() to
BNGE_MAX_RSS_TABLE_ENTRIES, so the RSS context count and the indirection
table size fit the arrays, which hold 8 contexts and 512 entries.

> bnge_cal_nr_rss_ctxs() has no upper bound. The VNIC holds at most
> BNGE_RSS_TABLE_MAX_TBL (8) RSS contexts, in fw_rss_cos_lb_ctx[] and in
> a 512-entry indirection table and DMA buffer. With 577 or more RX rings
> the function returns more than 8, so bnge_setup_vnic() writes past

[Severity: Low]
Is 577 the right threshold here? Here is bnge_adjust_pow_two() in
bnge_resc.h:

    u32 blks = total_ent / ent_per_blk;

    if (!blks)
        return 1;

    return roundup_pow_of_two(blks + 1);

With rx_rings = 512, bnge_cal_nr_rss_ctxs() passes 511, so blks = 7 and
the result is 8 contexts.

With rx_rings = 513 it passes 512, so blks = 8 and roundup_pow_of_two(9)
returns 16 contexts.

So the overflows described here seem to start at 513 RX rings. 577 is
one of the values in the 513..1024 range that also give 16. The code
change caps max_rx at 512, so it covers that whole range. Only the number
in the message looks wrong.

Could the message say 513 instead of 577?

> vnic->fw_rss_cos_lb_ctx[], bnge_set_dflt_rss_indir_tbl() and
> bnge_fill_hw_rss_tbl() write past the indirection table and its DMA
> buffer, and the pad computation in bnge_set_dflt_rss_indir_tbl() and
> bnge_modify_rss() underflows into a huge memset().

[Severity: Low]
Where is bnge_modify_rss() defined? I couldn't find a function by that
name under drivers/net/ethernet/broadcom/. bnge also doesn't seem to have
a set_rxfh or set_channels path where one would live.

The only pad underflow site in bnge seems to be in
bnge_set_dflt_rss_indir_tbl():

    pad = bd->rss_indir_tbl_entries - max_entries;
    if (pad)
        memset(&rss_indir_tbl[i], 0, pad * sizeof(*rss_indir_tbl));

Was this reference carried over from bnxt? Should the message list only
the sites that exist in bnge, i.e. bnge_setup_vnic(),
bnge_fill_hw_rss_tbl() and bnge_set_dflt_rss_indir_tbl()?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012054325.1782819-1-vikas.gupta%40broadcom.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] bnge: fix RSS table sizing for large RX ring counts
  2026-10-13  5:44   ` netdev-bot+sashiko
@ 2026-10-13  6:42     ` Vikas Gupta
  0 siblings, 0 replies; 9+ messages in thread
From: Vikas Gupta @ 2026-10-13  6:42 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev, horms, netdev,
	linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde

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

Hi,
  I'll send a v2 of this series with the patch order swapped.

Thanks,
Vikas

On Tue, Oct 13, 2026 at 11:14 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · 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).
>
> - [High] This commit opens a heap and DMA out-of-bounds write window for
>   513-576 RX rings.
> - [Medium] The commit message is wrong about the completion ring.
>
> --
>
> bnge: fix RSS table sizing for large RX ring counts
>
> This changes bnge_adjust_pow_two() to always add one block before rounding
> up to a power of two. RX ring counts whose quotient is already a power of
> two now get an RSS indirection table that covers all rings. The same
> helper also sizes the RX, TX, agg and completion ring pages.
>
> > Always add one block before rounding up. The same helper sizes the ring
> > pages; their count only changes when the number of entries divided by
> > the entries per page is a power of two of 2 or more, which does not
> > happen with the default ring sizes.
>
> [Severity: Medium]
> Is this accurate for the completion ring? It holds for the RX, TX and agg
> rings at the default depth of 511. The completion ring, though, is sized
> from more than the default depths:
>
> bnge_netdev.c:bnge_set_ring_params() {
>     ...
>         max_rx_cmpl = bn->rx_ring_size;
>
>         if (bn->priv_flags & BNGE_NET_EN_TPA)
>                 max_rx_cmpl += bd->max_tpa_v2;
>         ring_size = max_rx_cmpl * 2 + agg_ring_size + bn->tx_ring_size;
>         bn->cp_ring_size = ring_size;
>
>         bn->cp_nr_pages = bnge_adjust_pow_two(ring_size, CP_DESC_CNT);
>     ...
> }
>
> Take BNGE_PAGE_SIZE of 8192 (for example arm64 with 16K pages), TPA off
> and default 511/511 rings. Then ring_size is 2 * 511 + 511 = 1533 and
> CP_DESC_CNT is 512, so the quotient is 2. The old code returned 2 pages,
> which gives a cp_bit of 1024 (fewer slots than cp_ring_size). The new code
> returns 4.
>
> With 4K pages and TPA on, agg_ring_size is 2044 and ring_size is
> 3577 + 2 * max_tpa_v2. max_tpa_v2 comes from firmware max_aggs_supported.
> For values of 260..387, the quotient over a CP_DESC_CNT of 256 is 16, and
> the page count goes from 16 to 32.
>
> Could the commit message mention this change to completion ring sizing?
>
> > diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.h b/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> > index b62a634669f62..c0b6992d7f828 100644
> > --- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> > +++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> > @@ -81,13 +81,10 @@ bnge_adjust_pow_two(u32 total_ent, u16 ent_per_blk)
> >  {
> >       u32 blks = total_ent / ent_per_blk;
> >
> > -     if (blks == 0 || blks == 1)
> > -             return ++blks;
> > +     if (!blks)
> > +             return 1;
> >
> > -     if (!is_power_of_2(blks))
> > -             blks = roundup_pow_of_two(blks);
> > -
> > -     return blks;
> > +     return roundup_pow_of_two(blks + 1);
>
> [Severity: High]
> For 513..576 RX rings, bnge_cal_nr_rss_ctxs() calls
> bnge_adjust_pow_two(rx_rings - 1, 64), which gives a quotient of 8. The
> old code returned 8, but this now returns 16. Nothing clamps the result
> to BNGE_RSS_TABLE_MAX_TBL (8):
>
> bnge_resc.c:bnge_cal_nr_rss_ctxs() {
>         ...
>         return bnge_adjust_pow_two(rx_rings - 1,
>                                    BNGE_RSS_TABLE_ENTRIES);
> }
>
> Can this overflow bd->rss_indir_tbl[]? bnge_alloc_rss_indir_tbl()
> allocates BNGE_MAX_RSS_TABLE_ENTRIES (512) entries. On the first
> reservation, bnge_reserve_rings()->bnge_set_dflt_rss_indir_tbl() gets a
> max_entries of 16 * 64 = 1024:
>
> bnge_resc.c:bnge_set_dflt_rss_indir_tbl() {
>         ...
>         for (i = 0; i < max_entries; i++)
>                 rss_indir_tbl[i] = ethtool_rxfh_indir_default(i,
>                                                               bd->rx_nr_rings);
>
>         pad = bd->rss_indir_tbl_entries - max_entries;
>         if (pad)
>                 memset(&rss_indir_tbl[i], 0, pad * sizeof(*rss_indir_tbl));
> }
>
> The loop writes 512 entries past the end. Then the u16 pad wraps to
> 512 - 1024 = 65024, so the memset() clears about 254 KB beyond that.
>
> The other RSS consumers also look sized for at most 8 contexts:
>
> - bnge_setup_vnic() calls bnge_hwrm_vnic_ctx_alloc(bd, vnic, i) for
>   i < 16. That stores into vnic->fw_rss_cos_lb_ctx[], which has only
>   BNGE_MAX_CTX_PER_VNIC (8) entries.
>
> - bnge_fill_hw_rss_tbl() uses a tbl_size of 1024. It reads
>   rss_indir_tbl[] past 512 entries and writes 4096 bytes into
>   vnic->rss_table. bnge_alloc_vnic_attributes() sizes that buffer from
>   BNGE_MAX_RSS_TABLE_SIZE (2048 bytes plus the hash key).
>
> - bnge_hwrm_vnic_set_rss() reads fw_rss_cos_lb_ctx[8..15]. It also moves
>   ring_grp_tbl_addr past the end of the coherent allocation, so firmware
>   would DMA read outside the buffer.
>
> Configurations with 577 or more RX rings already overflowed before this
> patch, but this patch lowers the threshold to 513. Hitting it needs at
> least 513 default RSS queues from bnge_net_init_dflt_rings() and firmware
> that grants that many rings.
>
> The next patch in the series, "bnge: cap RSS contexts and RX rings to the
> RSS table size", closes this. It adds a min_t() against
> BNGE_RSS_TABLE_MAX_TBL in bnge_cal_nr_rss_ctxs() and caps *max_rx at
> BNGE_MAX_RSS_TABLE_ENTRIES in _bnge_get_max_rings().
>
> Could that cap be folded into this patch, or moved ahead of it, so this
> commit can be bisected or backported safely on its own?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012054325.1782819-1-vikas.gupta%40broadcom.com

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-13  6:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-12  5:43 [PATCH net 0/2] bnge fixes Vikas Gupta
2026-10-12  5:43 ` [PATCH net 1/2] bnge: fix RSS table sizing for large RX ring counts Vikas Gupta
2026-10-13  5:44   ` netdev-bot+sashiko
2026-10-13  6:42     ` Vikas Gupta
2026-10-12  5:43 ` [PATCH net 2/2] bnge: cap RSS contexts and RX rings to the RSS table size Vikas Gupta
2026-10-13  5:44   ` netdev-bot+sashiko
2026-10-12  5:50 ` [PATCH net 0/2] bnge fixes netdev-bot+sinfo
2026-10-12  5:56   ` Vikas Gupta
2026-10-12 11:51 ` Björn Töpel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®