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

[Vikas Gupta]

AI review looks valid. I will send new revision.

Thanks,
Vikas

On Mon, Oct 19, 2026 at 6:09 PM <netdev-bot+sashiko@xxxxxxxxxx> wrote:
>
> 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

pw-bot: cr

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature