iflib: use default ntxd and nrxd when user value is not power of 2

[jacob.e.keller_intel.com]

Summary

A user may set a sysctl to override the default number of Tx or Rx
descriptors. However, certain calculations in the iflib core expect the
number of descriptors to be a power of 2.

Update _iflib_assert to verify that all of the shared context parameters
for the number of descriptors are powers of 2.

Modify iflib_reset_qvalues to check that the provided isc_nrxd value is
a power of 2. If it's not, print a warning message and then use the
default value.

An alternative might be to try rounding the number down instead.
However, this creates problems in case the rounded down value is below
the minimum value that the driver would support.

[jacob.e.keller_intel.com]

I have a couple of thoughts I had while writing this.

sys/net/iflib.c

4397–4401 ↗

(On Diff #56079)

Should we move the minimum check to after the round down? or should we add a 2nd minimum check? I.e. in the off chance that the minimum value the driver specifies isn't a power of 2, we'd round down below what the minimum hardware support is.

4408 ↗

(On Diff #56079)

I feel like this should be some sort of macro we can call, but I'm not sure where to put it. Also, I don't know how to make it size-agnostic.

[marius]

Yes, the case of the driver providing isc_n{r,t}xd_min which aren't a power of 2 must be handled as well for iflib(4) to actually do the right thing. This is getting kind of ridiculous, though, as rounding up isc_n{r,t}xd_min to a power of 2 in turn may exceed sc_n{r,t}xd_max and so far nothing guarantees that sc_n{r,t}xd_max is a power 2 either, same for sc_n{r,t}xd_default. As much as I understand the desire for iflib(4) to figure out applicable values within the ranges supplied by the driver with the driver just taking actual hardware limitations and not iflib(4) constraints into account, IMO such magic would just be too complex without much real gain beyond some convenience for driver developers. Moreover, these ring size constraints aren't the only restriction imposed by iflib(4) onto the driver/hardware. For example, given that iflib(4) only offers a shared DMA alignment constraint (isc_q_align) for both RX and TX descriptors, ixl(4) nowadays specifies a 4096-byte alignment for both while according to the pre-iflib(4) counterpart, that alignment is only required for the RX descriptors with 128 bytes being sufficient for the TX ones. By contrast, something that iflib(4) really should deal gracefully with rather than iflib_device_register() just failing is a user supplying non-power-of-2 ring sizes via the override_n{r,t}xds SYSCTLs.
Thus, what I'd suggest instead is to simply assert in _iflib_assert() via MPASS() that all isc_n{r,t}xd_{default,max,min} are a power of 2. Actually, it's not that uncommon for Ethernet MACs to also only support ring sizes which are a power of 2 (at least Apple GMAC, National Semiconductor Saturn, Sun CAS, ERI and GEM come to my mind), probably for the same optimization reasons iflib(4) only supports power-of-2-sizes, too. If a particular MAC happens to really support some more odd-ish ring sizes, just take note of that fact with a comment in the driver. However, code should be added to iflib_reset_qvalues() which resets isc_n{r,t}xd to isc_n{r,t}xd_default in case the user-supplied values aren't a power of 2.

[jacob.e.keller_intel.com]

Yea, I'll add MPASS to check that the values are power of 2.

The rounding code for the override value should be kept, since iflib doesn't support non-power-of-2 values, and it's rather annoying to cause iflib to just fail to load if the user configures a value that isn't a power of 2.

I agree there isn't really a reason to check the driver values, as we should expect the driver to handle that.

[jacob.e.keller_intel.com]

I would be fine modifying the iflib code to avoid the power of 2 limitation entirely, but I don't think there is significant gain for that, because we would have to add a configuration knob indicating the increment. Additionally, since some hardware likely does require power-of-2 values, we'd want to have a way for those drivers to express that restriction as well. power-of-2 restriction can't be expressed with just an increment value, so that becomes a lot trickier.

[jacob.e.keller_intel.com]

Modify _iflib_assert to verify driver parameters are power of 2

[marius]

sys/net/iflib.c

4354 ↗

(On Diff #56496)

As previously suggested, please just go with sctx->isc_nrxd_default[i] in case the user-supplied value isn't a power of 2 in order to avoid the complexity of dealing with case that the rounded down value happens to be smaller than sctx->isc_nrxd_min[i].

4373 ↗

(On Diff #56496)

Likewise for sctx->isc_nrxd*

[jacob.e.keller_intel.com]

use the default value instead of rounding down

[jacob.e.keller_intel.com]

Ok, I updated this revision to just use the default value now instead of rounding down.

[marius]

Looks good to me now, thanks!