svn commit: r347418 - head/sys/net

[Kristof Provost]

On 10 May 2019, at 2:41, Eric Joyner wrote:
> Author: erj
> Date: Fri May 10 00:41:42 2019
> New Revision: 347418
> URL: https://svnweb.freebsd.org/changeset/base/347418
>
> Log:
>   iflib: use default ntxd and nrxd when user value is not power of 2
>
>   From Jake:
>   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.
>
This commit appears to trigger a panic I see on a system with a Broadcom 
BCM57416 (if_bnxt) nic.

It trips over the power of two assertion:

	panic: Assertion powerof2(sctx->isc_nrxd_max[i]) failed at 
/usr/src/sys/net/iflib.c:5320
	Tracing pid 0 tid 100000 td 0xffffffff81c8c640
	kdb_enter() at kdb_enter+0x37/frame 0xffffffff825be990
	vpanic() at vpanic+0x19e/frame 0xffffffff825be9e0
	panic() at panic+0x43/frame 0xffffffff825bea40
	iflib_register() at iflib_register+0x340/frame 0xffffffff825bea80
	iflib_device_register() at iflib_device_register+0x9f/frame 
0xffffffff825bee10
	iflib_device_attach() at iflib_device_attach+0xb5/frame 
0xffffffff825bee40
	device_attach() at device_attach+0x3ca/frame 0xffffffff825bee80
	device_probe_and_attach() at device_probe_and_attach+0x70/frame 
0xffffffff825beeb0
	bus_generic_attach() at bus_generic_attach+0x18/frame 
0xffffffff825beed0
	pci_attach() at pci_attach+0xe0/frame 0xffffffff825bef10
	acpi_pci_attach() at acpi_pci_attach+0x19/frame 0xffffffff825bf150
	device_attach() at device_attach+0x3ca/frame 0xffffffff825bf190
	device_probe_and_attach() at device_probe_and_attach+0x70/frame 
0xffffffff825bf1c0
	bus_generic_attach() at bus_generic_attach+0x18/frame 
0xffffffff825bf1e0
	acpi_pcib_acpi_attach() at acpi_pcib_acpi_attach+0x431/frame 
0xffffffff825bf250
	device_attach() at device_attach+0x3ca/frame 0xffffffff825bf290
	device_probe_and_attach() at device_probe_and_attach+0x70/frame 
0xffffffff825bf2c0
	bus_generic_attach() at bus_generic_attach+0x18/frame 
0xffffffff825bf2e0
	acpi_attach() at acpi_attach+0xbb7/frame 0xffffffff825bf370
	device_attach() at device_attach+0x3ca/frame 0xffffffff825bf3b0
	device_probe_and_attach() at device_probe_and_attach+0x70/frame 
0xffffffff825bf3e0
	bus_generic_attach() at bus_generic_attach+0x18/frame 
0xffffffff825bf400
	device_attach() at device_attach+0x3ca/frame 0xffffffff825bf440
	device_probe_and_attach() at device_probe_and_attach+0x70/frame 
0xffffffff825bf470
	bus_generic_new_pass() at bus_generic_new_pass+0xed/frame 
0xffffffff825bf4a0
	bus_set_pass() at bus_set_pass+0x46/frame 0xffffffff825bf4d0
	configure() at configure+0x9/frame 0xffffffff825bf4e0
	mi_startup() at mi_startup+0xec/frame 0xffffffff825bf530
	btext() at btext+0x2c

The if_bnxt driver initialises `.isc_nrxd_max = {INT32_MAX, INT32_MAX, 
INT32_MAX},`, so presumably that’s the cause.
I don’t know what a sane value would be though. I’ve defaulted to 
4096 (because that’s what some other iflib users seems to do) for now, 
and that seems to work. It doesn’t panic and I can get traffic through 
it at least:

	diff --git a/sys/dev/bnxt/if_bnxt.c b/sys/dev/bnxt/if_bnxt.c
	index 50827106024..3958d95cab9 100644
	--- a/sys/dev/bnxt/if_bnxt.c
	+++ b/sys/dev/bnxt/if_bnxt.c
	@@ -316,11 +316,11 @@ static struct if_shared_ctx bnxt_sctx_init = {
	        .isc_nrxd_default = {PAGE_SIZE / sizeof(struct cmpl_base) * 8,
	            PAGE_SIZE / sizeof(struct rx_prod_pkt_bd),
	            PAGE_SIZE / sizeof(struct rx_prod_pkt_bd)},
	-       .isc_nrxd_max = {INT32_MAX, INT32_MAX, INT32_MAX},
	+       .isc_nrxd_max = {4096, 4096, 4096},
	        .isc_ntxd_min = {16, 16, 16},
	        .isc_ntxd_default = {PAGE_SIZE / sizeof(struct cmpl_base) * 2,
	            PAGE_SIZE / sizeof(struct tx_bd_short)},
	-       .isc_ntxd_max = {INT32_MAX, INT32_MAX, INT32_MAX},
	+       .isc_ntxd_max = {4096, 4096, 4096},

	        .isc_admin_intrcnt = 1,
	        .isc_vendor_info = bnxt_vendor_info_array,


Best regards,
Kristof