Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 19 May 2020 10:21:32 +0200
From:      "Kristof Provost" <kp@FreeBSD.org>
To:        "Eric Joyner" <erj@FreeBSD.org>, "Jacob Keller" <jacob.e.keller@intel.com>, shurd@FreeBSD.org
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r347418 - head/sys/net
Message-ID:  <3BBFB371-EA44-4EE9-8A55-542CDE273CC4@FreeBSD.org>
In-Reply-To: <201905100041.x4A0fhNT083122@repo.freebsd.org>
References:  <201905100041.x4A0fhNT083122@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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
From owner-svn-src-head@freebsd.org  Tue May 19 08:43:18 2020
Return-Path: <owner-svn-src-head@freebsd.org>
Delivered-To: svn-src-head@mailman.nyi.freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1])
 by mailman.nyi.freebsd.org (Postfix) with ESMTP id 292922FFC07;
 Tue, 19 May 2020 08:43:18 +0000 (UTC)
 (envelope-from manu@FreeBSD.org)
Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org
 [IPv6:2610:1c1:1:606c::19:3])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256
 client-signature RSA-PSS (4096 bits) client-digest SHA256)
 (Client CN "mxrelay.nyi.freebsd.org",
 Issuer "Let's Encrypt Authority X3" (verified OK))
 by mx1.freebsd.org (Postfix) with ESMTPS id 49R8Y209fFz4CNH;
 Tue, 19 May 2020 08:43:18 +0000 (UTC)
 (envelope-from manu@FreeBSD.org)
Received: from repo.freebsd.org (repo.freebsd.org
 [IPv6:2610:1c1:1:6068::e6a:0])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (Client did not present a certificate)
 by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 011538420;
 Tue, 19 May 2020 08:43:18 +0000 (UTC)
 (envelope-from manu@FreeBSD.org)
Received: from repo.freebsd.org ([127.0.1.37])
 by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 04J8hHAP031773;
 Tue, 19 May 2020 08:43:17 GMT (envelope-from manu@FreeBSD.org)
Received: (from manu@localhost)
 by repo.freebsd.org (8.15.2/8.15.2/Submit) id 04J8hHb1031772;
 Tue, 19 May 2020 08:43:17 GMT (envelope-from manu@FreeBSD.org)
Message-Id: <202005190843.04J8hHb1031772@repo.freebsd.org>
X-Authentication-Warning: repo.freebsd.org: manu set sender to
 manu@FreeBSD.org using -f
From: Emmanuel Vadot <manu@FreeBSD.org>
Date: Tue, 19 May 2020 08:43:17 +0000 (UTC)
To: src-committers@freebsd.org, svn-src-all@freebsd.org,
 svn-src-head@freebsd.org
Subject: svn commit: r361245 - head/sys/compat/linuxkpi/common/include/linux
X-SVN-Group: head
X-SVN-Commit-Author: manu
X-SVN-Commit-Paths: head/sys/compat/linuxkpi/common/include/linux
X-SVN-Commit-Revision: 361245
X-SVN-Commit-Repository: base
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-BeenThere: svn-src-head@freebsd.org
X-Mailman-Version: 2.1.33
Precedence: list
List-Id: SVN commit messages for the src tree for head/-current
 <svn-src-head.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-head>,
 <mailto:svn-src-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head/>;
List-Post: <mailto:svn-src-head@freebsd.org>
List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-head>,
 <mailto:svn-src-head-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 19 May 2020 08:43:18 -0000

Author: manu
Date: Tue May 19 08:43:17 2020
New Revision: 361245
URL: https://svnweb.freebsd.org/changeset/base/361245

Log:
  linuxkpi: Add __init_waitqueue_head
  
  The only difference with init_waitqueue_head is that the name and the
  lock class key are provided but we don't use those so use init_waitqueue_head
  directly.
  
  Sponsored-by: The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D24861

Modified:
  head/sys/compat/linuxkpi/common/include/linux/wait.h

Modified: head/sys/compat/linuxkpi/common/include/linux/wait.h
==============================================================================
--- head/sys/compat/linuxkpi/common/include/linux/wait.h	Tue May 19 07:47:59 2020	(r361244)
+++ head/sys/compat/linuxkpi/common/include/linux/wait.h	Tue May 19 08:43:17 2020	(r361245)
@@ -119,6 +119,8 @@ extern wait_queue_func_t default_wake_function;
 	INIT_LIST_HEAD(&(wqh)->task_list);				\
 } while (0)
 
+#define	__init_waitqueue_head(wqh, name, lk) init_waitqueue_head(wqh)
+
 void linux_init_wait_entry(wait_queue_t *, int);
 void linux_wake_up(wait_queue_head_t *, unsigned int, int, bool);
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3BBFB371-EA44-4EE9-8A55-542CDE273CC4>