> From: Bruce Richardson [mailto:[email protected]] > Sent: Tuesday, 16 December 2025 10.52 > > On Tue, Dec 16, 2025 at 10:25:41AM +0100, Morten Brørup wrote: > > +TO: Ethdev maintainers > > > > > From: Bruce Richardson [mailto:[email protected]] > > > Sent: Tuesday, 16 December 2025 09.48 > > > > > > On Mon, Dec 15, 2025 at 07:53:27PM +0100, Morten Brørup wrote: > > > > > From: Bruce Richardson [mailto:[email protected]] > > > > > 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:[email protected]] > > > > > > > > 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 > <[email protected]> > > > > > > > > --- > > > > > > > > > > > > > > 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). > > > > Small descriptor rings are not theoretical. > > Our application configures very small descriptor rings on unused > ports, to be able to process background traffic (e.g. slow protocols) > on those ports, but still conserve memory. > > > > E.g. the igb driver reports dev_info.rx_desc_lim.nb_min = 32, but > multiple times this value is required with default thresholds. > > The i40e driver reports dev_info.rx_desc_lim.nb_min = 64, and IIRC > more is required with default thresholds. > > > > I agree that defaults should remain optimized for performance > (maximum packets per second). > > > > The problem is rte_eth_dev_adjust_nb_rx_tx_desc() not having > sufficient information about all the thresholds to correctly calculate > its output values. I'll file a bug report. > > > > Updating the drivers to report dev_info.rx/tx_desc_lim.nb_min and > dev_info.rx/tx_desc_lim.nb_align that work with default thresholds > could be a workaround. > > > > I'm not sure I like that option. How would one then query the limits > with > non-default thresholds? Also, it doesn't inform user as to which > thresholds > are causing what limits, vs limits that are hard ones from the HW. > > Other options may be greater use of e.g. 0 as a sentinal value for > allowing > the driver to pick, or changing things internally so that the source of > the > rx_conf is passed to the drivers. However, I actually feel that the > best > option if we want a "most usable" solution here, is to document that > the > free_thresh is a hint, and that it may be adjusted by the driver if > necessary to accomodate a requested ring size. [We could log a warning > on > adjustment]
+1 to your "must usable" suggestion. (As a workaround for rte_eth_dev_adjust_nb_rx_tx_desc() not having sufficient information.) The Tx side is worse. It has requirements related to both rs_thresh and free_thresh. I suppose they could be treated as hints too.
