Re: [PATCH] net/intel: improve Rx descriptor ring size checks

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Dec 15, 2025 at 07:53:27PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Monday, 15 December 2025 19.21
> > 
> > On Mon, Dec 15, 2025 at 05:58:33PM +0000, Bruce Richardson wrote:
> > > On Mon, Dec 15, 2025 at 06:54:50PM +0100, Morten Brørup wrote:
> > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > > Sent: Monday, 15 December 2025 18.36
> > > > >
> > > > > The default Rx ring size checks did not account for the fact that
> > the
> > > > > port would not work correctly if the Rx ring size was only twice
> > the
> > > > > free threshold size or less, so add in a new check for this. This
> > would
> > > > > generally only apply in cases where very small rings sizes are
> > being
> > > > > used, for example, with default Rx free thresh of 32, only ring
> > size of
> > > > > 64 would cause issues.
> > > > >
> > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > > ---
> > > >
> > > > Does dev_info.rx_desc_lim.nb_min returned by rte_eth_dev_info_get()
> > need correction too?
> > > >
> > > The minimum number of descriptors stays the same, however, if
> > choosing the
> > > minimum number of descriptors you may need to reduce the
> > rx_free_thresh
> > > value.
> > >
> > However, I think you raise a good point. I'll see about adding a
> > specific
> > error message in case the user is using the default threshold and
> > setting
> > the min ring size.
> 
> The applications need some generic code sequence that always works, on all NICs.
> 
> E.g. if an application uses rte_eth_dev_adjust_nb_rx_tx_desc() to move a requested crazy number of descriptors within bounds, and uses the default values for all other parameters, it should work.
> 

This is surprisingly difficult to make working with the way things are set
up right now. For example, if the user wants defaults for config settings
and passes in NULL to the ethdev API, the ethdev library queries the
defaults from the driver and fills those in before calling the relevant
ring setup functions. Therefore, the driver level has no knowledge of
whether the user explicitly requested a value which happens to match the
default, or if the user just wants a working default value.

Another option would be to set the default low enough that it would work
with any ring size possible, but that would then cause a perf impact for
apps which don't need such low values (as an extreme example, think on a
theoretical driver which allows ring sizes of as small as 16 or 8, a free
threshold to work there is likely suboptimal for more normal ring sizes of
e.g. 1k or 2k).

> Nonetheless, more detailed error messages are always helpful. :-)
> 
Yes, for now I think giving a meaningful error message in the few cases of
really small ring sizes is the best approach I can take. Given the issue
has been present for a long time without issue, and given that the error
will always repeatedly occur at init time with the offending values, I
believe its sufficient.

/Bruce