Re: [PATCH net v4] bnxt_en: Bound SW TPA IDs to prevent crashes

[Joe Damato]

On Tue, Sep 01, 2026 at 10:16:54AM +0200, Paolo Abeni wrote:
> On 8/28/26 9:08 PM, Joe Damato wrote:

[...]

> > ---
> > v4:
> >   - Moved bp->max_tpa_roundup_size init out of the early return path and
> >     documented that TPA is unsupported there, as suggested by Michael.
> 
> Clashiko quite convincingly elaborates that the above is not enough:
> 
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828190900.1767611-1-joe%40dama.to
> 
> Could you please have a look?

I looked. There are 4 separate things Clashiko reports, so in order from top
to bottom:

1.) "This isn't a bug introduced by this patch, but the line right below the last
context line here still uses the raw firmware ID as a table index: ..."

This seems impossible. If FW emits an ID > 1024 that's a firmware bug and if
we wanted to guard against that possiblity that would be a separate patch and
a different Fixes.

2.) "This isn't a bug introduced by this patch, but the loop count here changes
from bp->max_tpa to bp->max_tpa_roundup_size,  ...."

I mentioned previously in my v3 that this is real, but is pre-existing and is
a different Fixes unrelated to this change.

3.) "The new assignment sits after the max_tpa_v2 == 0 early return, so on a P5+
device whose firmware reports max_aggs_supported == 0, does
bp->max_tpa_roundup_size stay 0 while BNXT_FLAG_TPA is set? ..."

Michael commented in the v3 that this is a false positive, hence why I changed
the code from what it was in the v3 to this.

4.) "This isn't a bug introduced by this patch, but the loop bound grows here
(for example 64 instead of 48 when max_tpa is not a power of two), so there
are more mandatory allocations that can fail, and the failure is not
propagated by two callers...."

I mentioned previously in the v3 that this is real, but this is pre-existing
and unrelated to this change. Fixing this would be a separate patch with a
different Fixes.

Michael: please feel free to confirm (or let me know if I'm wrong here), but I
think this patch can be merged.