netdev - Re: [PATCH net-next] eth: bnxt: take page size into account for page pool recycling rings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACKFLikrdeF=sp4U9zT_3LgsQuS9PQcoPmTdiBLT3G4fGrvi2w@mail.gmail.com>
Date: Thu, 26 Jun 2025 23:48:18 -0700
From: Michael Chan <michael.chan@...adcom.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, 
	pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org, 
	pavan.chebbi@...adcom.com
Subject: Re: [PATCH net-next] eth: bnxt: take page size into account for page
 pool recycling rings

On Thu, Jun 26, 2025 at 3:40 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Thu, 26 Jun 2025 14:52:17 -0700 Michael Chan wrote:
> > >  {
> > > +       const unsigned int agg_size_fac = PAGE_SIZE / BNXT_RX_PAGE_SIZE;
> > > +       const unsigned int rx_size_fac = PAGE_SIZE / SZ_4K;
> > >         struct page_pool_params pp = { 0 };
> > >         struct page_pool *pool;
> > >
> > > -       pp.pool_size = bp->rx_agg_ring_size;
> > > +       pp.pool_size = bp->rx_agg_ring_size / agg_size_fac;
> >
> > The bp->rx_agg_ring_size has already taken the system PAGE_SIZE into
> > consideration to some extent in bnxt_set_ring_params().  The
> > jumbo_factor and agg_factor will be smaller when PAGE_SIZE is larger.
> > Will this overcompensate?
>
> My understanding is basically that bnxt_set_ring_params() operates
> on BNXT_RX_PAGE_SIZE so it takes care of 4k .. 32k range pretty well.
> But for 64k pages we will use 32k buffers, so 2 agg ring entries
> per system page. If our heuristic is that we want the same number
> of pages on the device ring as in the pp cache we should divide
> the cache size by two. Hope that makes sense.

Ah, got it.  agg_size_fac will be at most 2 when PAGE_SIZE is 64K.

>
> My initial temptation was to say that agg ring is always shown to
> the user in 4kB units, regardless of system page. The driver would
> divide and multiply the parameter in the ethtool callbacks. Otherwise
> even with this patch existing configs for bnxt have to be adjusted
> based on system page size :( But I suspect you may have existing users
> on systems with 64kB pages, so this would be too risky? WDYT?

Agreed.  Too risky and too confusing.  Thanks.

Reviewed-by: Michael Chan <michael.chan@...adcom.com>

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4196 bytes)