Re: [PATCH net-next 2/2] net/mlx5e: Clamp page_pool size to max

[Dragos Tatulea]

On Tue, Sep 23, 2025 at 05:23:05PM -0700, Jakub Kicinski wrote:
> On Tue, 23 Sep 2025 08:23:10 -0700 Jakub Kicinski wrote:
> > On Tue, 23 Sep 2025 15:12:33 +0000 Dragos Tatulea wrote:
> > > On Tue, Sep 23, 2025 at 07:23:56AM -0700, Jakub Kicinski wrote:  
> > > > Please do some testing. A PP cache of 32k is just silly, you should
> > > > probably use a smaller limit.    
> > > You mean clamping the pool_size to a certain limit so that the page_pool
> > > ring size doesn't cover a full RQ when the RQ ring size is too large?  
> > 
> > Yes, 8k ring will take milliseconds to drain. We don't really need
> > milliseconds of page cache. By the time the driver processed the full
> > ring we must have gone thru 128 NAPI cycles, and the application
> > most likely already stated freeing the pages.
> > 
> > If my math is right at 80Gbps per ring and 9k MTU it takes more than a
> > 1usec to receive a frame. So 8msec to just _receive_ a full ring worth
> > of data. At Meta we mostly use large rings to cover up scheduler and
> > IRQ masking latency.
> 
> On second thought, let's just clamp it to 16k in the core and remove
> the error. Clearly the expectations of the API are too intricate,
> most drivers just use ring size as the cache size.
Makes sense. For my peace of mind I want to do some packet rate tests
to see that there is no perf difference and compare the page_pool stats.

Should the page_pool should print a warning when it clamps?

Also, checking for size > 32K and clamping to 16K looks a bit weird...
Should the limit be lowered to 16K alltogether?

Thanks,
Dragos