Re: [PATCH net v2 2/2] bnge: fix RSS table sizing for large RX ring counts

[netdev-bot+sashiko@kernel.org]

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] This patch adds a new helper, bnge_calc_nr_ring_pages()
  (drivers/net/ethernet/broadcom/bnge/bnge_resc.h:90-102), that keeps…

--

bnge: fix RSS table sizing for large RX ring counts

This commit changes bnge_adjust_pow_two() so it always adds one block
before rounding up to a power of two. With that change the RSS indirection
table covers every RX ring. Ring page sizing moves to a new
bnge_calc_nr_ring_pages() helper, which keeps the original rounding.

> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.h b/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> index b62a634669f62..58462524939fe 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.h
> @@ -81,6 +81,17 @@ bnge_adjust_pow_two(u32 total_ent, u16 ent_per_blk)
>  {
>  	u32 blks = total_ent / ent_per_blk;
>  
> +	if (!blks)
> +		return 1;
> +
> +	return roundup_pow_of_two(blks + 1);
> +}
> +
> +static inline u32
> +bnge_calc_nr_ring_pages(u32 total_ent, u16 ent_per_blk)
> +{
> +	u32 blks = total_ent / ent_per_blk;
> +
>  	if (blks == 0 || blks == 1)
>  		return ++blks;
>  

[Severity: Medium]
Can this new helper size a ring smaller than total_ent?

The rest of bnge_calc_nr_ring_pages() is:

	if (!is_power_of_2(blks))
		blks = roundup_pow_of_two(blks);

	return blks;

If the floored quotient is already a power of two of 2 or more, blks is
returned unchanged and the partial last page is lost. The ring then has
blks * ent_per_blk slots, which is never more than total_ent.

The commit message says "bnxt_calc_nr_ring_pages() always adds one, which
is correct". This helper has the same name but keeps the rounding that the
message calls wrong.

In bnge_set_ring_params(), the completion ring budget includes
max_tpa_v2. bnge_hwrm_vnic_qcaps() takes that value from the firmware's
max_aggs_supported without any cap:

	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_calc_nr_ring_pages(ring_size, CP_DESC_CNT);
	...
	bn->cp_bit = bn->cp_nr_pages * CP_DESC_CNT;
	bn->cp_ring_mask = bn->cp_bit - 1;

hwrm_ring_alloc_send_msg() then sets the CMPL and NQ ring lengths from
that mask:

	req->length = cpu_to_le32(bn->cp_ring_mask + 1);

Take 4K pages, rx/tx ring size 511, agg_factor 4 (TPA is on by default)
and CP_DESC_CNT 256. Then ring_size is 3577 + 2 * max_tpa_v2.

Any max_tpa_v2 from 260 to 387 gives a ring_size of 4097 to 4351. That
means blks = 16 and a 4096-entry completion ring, while bn->cp_ring_size
keeps the larger budget. The MAX_CP_PAGES clamp cannot catch this,
because the page count has already been rounded down.

With 64K pages (agg_factor 2, CP_DESC_CNT 4096), any max_tpa_v2 from 2819
to 4866 gives blks = 2. That is 8192 slots for up to 12287 budgeted
completions.

Could the completion ring overflow if the driver reaches that worst case?
I could not confirm whether shipping firmware reports max_aggs_supported
in those ranges.

The RX, RX agg and TX ring sizing use the same helper. They are only safe
today because the ring sizes are fixed at 511 and bnge has no
set_ringparam. A tx_ring_size between 512 and 767 would give
tx_ring_mask = 511, and bnge_tx_avail() would then report more free space
than the ring has.

The helper also breaks the rule encoded in BNGE_MAX_RX_DESC_CNT and
BNGE_MAX_TX_DESC_CNT, which keep the ring size below pages * DESC_CNT.

Should ring pages use the always-add-one rounding as well, the way
bnxt_calc_nr_ring_pages() does? If dropping the remainder is safe, could a
comment or the commit message explain why?

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