Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 10 May 2019 00:41:43 +0000 (UTC)
From:      Eric Joyner <erj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r347418 - head/sys/net
Message-ID:  <201905100041.x4A0fhNT083122@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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.
  
  Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
  Reviewed by:	marius@
  MFC after:	1 week
  Sponsored by:	Intel Corporation
  Differential Revision:	https://reviews.freebsd.org/D19880

Modified:
  head/sys/net/iflib.c

Modified: head/sys/net/iflib.c
==============================================================================
--- head/sys/net/iflib.c	Fri May 10 00:03:32 2019	(r347417)
+++ head/sys/net/iflib.c	Fri May 10 00:41:42 2019	(r347418)
@@ -4387,9 +4387,6 @@ iflib_reset_qvalues(if_ctx_t ctx)
 
 	scctx->isc_txrx_budget_bytes_max = IFLIB_MAX_TX_BYTES;
 	scctx->isc_tx_qdepth = IFLIB_DEFAULT_TX_QDEPTH;
-	/*
-	 * XXX sanity check that ntxd & nrxd are a power of 2
-	 */
 	if (ctx->ifc_sysctl_ntxqs != 0)
 		scctx->isc_ntxqsets = ctx->ifc_sysctl_ntxqs;
 	if (ctx->ifc_sysctl_nrxqs != 0)
@@ -4420,6 +4417,11 @@ iflib_reset_qvalues(if_ctx_t ctx)
 				      i, scctx->isc_nrxd[i], sctx->isc_nrxd_max[i]);
 			scctx->isc_nrxd[i] = sctx->isc_nrxd_max[i];
 		}
+		if (!powerof2(scctx->isc_nrxd[i])) {
+			device_printf(dev, "nrxd%d: %d is not a power of 2 - using default value of %d\n",
+				      i, scctx->isc_nrxd[i], sctx->isc_nrxd_default[i]);
+			scctx->isc_nrxd[i] = sctx->isc_nrxd_default[i];
+		}
 	}
 
 	for (i = 0; i < sctx->isc_ntxqs; i++) {
@@ -4433,6 +4435,11 @@ iflib_reset_qvalues(if_ctx_t ctx)
 				      i, scctx->isc_ntxd[i], sctx->isc_ntxd_max[i]);
 			scctx->isc_ntxd[i] = sctx->isc_ntxd_max[i];
 		}
+		if (!powerof2(scctx->isc_ntxd[i])) {
+			device_printf(dev, "ntxd%d: %d is not a power of 2 - using default value of %d\n",
+				      i, scctx->isc_ntxd[i], sctx->isc_ntxd_default[i]);
+			scctx->isc_ntxd[i] = sctx->isc_ntxd_default[i];
+		}
 	}
 }
 
@@ -4543,7 +4550,7 @@ iflib_device_register(device_t dev, void *sc, if_share
 	if_softc_ctx_t scctx;
 	kobjop_desc_t kobj_desc;
 	kobj_method_t *kobj_method;
-	int err, i, msix, rid;
+	int err, msix, rid;
 	uint16_t main_rxq, main_txq;
 
 	ctx = malloc(sizeof(* ctx), M_IFLIB, M_WAITOK|M_ZERO);
@@ -4598,23 +4605,6 @@ iflib_device_register(device_t dev, void *sc, if_share
 	/* XXX change for per-queue sizes */
 	device_printf(dev, "Using %d TX descriptors and %d RX descriptors\n",
 	    scctx->isc_ntxd[main_txq], scctx->isc_nrxd[main_rxq]);
-	for (i = 0; i < sctx->isc_nrxqs; i++) {
-		if (!powerof2(scctx->isc_nrxd[i])) {
-			/* round down instead? */
-			device_printf(dev,
-			    "# RX descriptors must be a power of 2\n");
-			err = EINVAL;
-			goto fail_iflib_detach;
-		}
-	}
-	for (i = 0; i < sctx->isc_ntxqs; i++) {
-		if (!powerof2(scctx->isc_ntxd[i])) {
-			device_printf(dev,
-			    "# TX descriptors must be a power of 2");
-			err = EINVAL;
-			goto fail_iflib_detach;
-		}
-	}
 
 	if (scctx->isc_tx_nsegments > scctx->isc_ntxd[main_txq] /
 	    MAX_SINGLE_PACKET_FRACTION)
@@ -4790,7 +4780,6 @@ fail_intr_free:
 fail_queues:
 	iflib_tx_structures_free(ctx);
 	iflib_rx_structures_free(ctx);
-fail_iflib_detach:
 	IFDI_DETACH(ctx);
 fail_unlock:
 	CTX_UNLOCK(ctx);
@@ -4833,9 +4822,6 @@ iflib_pseudo_register(device_t dev, if_shared_ctx_t sc
 	scctx = &ctx->ifc_softc_ctx;
 	ifp = ctx->ifc_ifp;
 
-	/*
-	 * XXX sanity check that ntxd & nrxd are a power of 2
-	 */
 	iflib_reset_qvalues(ctx);
 	CTX_LOCK(ctx);
 	if ((err = IFDI_ATTACH_PRE(ctx)) != 0) {
@@ -4899,23 +4885,6 @@ iflib_pseudo_register(device_t dev, if_shared_ctx_t sc
 	/* XXX change for per-queue sizes */
 	device_printf(dev, "Using %d TX descriptors and %d RX descriptors\n",
 	    scctx->isc_ntxd[main_txq], scctx->isc_nrxd[main_rxq]);
-	for (i = 0; i < sctx->isc_nrxqs; i++) {
-		if (!powerof2(scctx->isc_nrxd[i])) {
-			/* round down instead? */
-			device_printf(dev,
-			    "# RX descriptors must be a power of 2\n");
-			err = EINVAL;
-			goto fail_iflib_detach;
-		}
-	}
-	for (i = 0; i < sctx->isc_ntxqs; i++) {
-		if (!powerof2(scctx->isc_ntxd[i])) {
-			device_printf(dev,
-			    "# TX descriptors must be a power of 2");
-			err = EINVAL;
-			goto fail_iflib_detach;
-		}
-	}
 
 	if (scctx->isc_tx_nsegments > scctx->isc_ntxd[main_txq] /
 	    MAX_SINGLE_PACKET_FRACTION)
@@ -5305,6 +5274,8 @@ iflib_module_event_handler(module_t mod, int what, voi
 static void
 _iflib_assert(if_shared_ctx_t sctx)
 {
+	int i;
+
 	MPASS(sctx->isc_tx_maxsize);
 	MPASS(sctx->isc_tx_maxsegsize);
 
@@ -5312,12 +5283,25 @@ _iflib_assert(if_shared_ctx_t sctx)
 	MPASS(sctx->isc_rx_nsegments);
 	MPASS(sctx->isc_rx_maxsegsize);
 
-	MPASS(sctx->isc_nrxd_min[0]);
-	MPASS(sctx->isc_nrxd_max[0]);
-	MPASS(sctx->isc_nrxd_default[0]);
-	MPASS(sctx->isc_ntxd_min[0]);
-	MPASS(sctx->isc_ntxd_max[0]);
-	MPASS(sctx->isc_ntxd_default[0]);
+	MPASS(sctx->isc_nrxqs >= 1 && sctx->isc_nrxqs <= 8);
+	for (i = 0; i < sctx->isc_nrxqs; i++) {
+		MPASS(sctx->isc_nrxd_min[i]);
+		MPASS(powerof2(sctx->isc_nrxd_min[i]));
+		MPASS(sctx->isc_nrxd_max[i]);
+		MPASS(powerof2(sctx->isc_nrxd_max[i]));
+		MPASS(sctx->isc_nrxd_default[i]);
+		MPASS(powerof2(sctx->isc_nrxd_default[i]));
+	}
+
+	MPASS(sctx->isc_ntxqs >= 1 && sctx->isc_ntxqs <= 8);
+	for (i = 0; i < sctx->isc_ntxqs; i++) {
+		MPASS(sctx->isc_ntxd_min[i]);
+		MPASS(powerof2(sctx->isc_ntxd_min[i]));
+		MPASS(sctx->isc_ntxd_max[i]);
+		MPASS(powerof2(sctx->isc_ntxd_max[i]));
+		MPASS(sctx->isc_ntxd_default[i]);
+		MPASS(powerof2(sctx->isc_ntxd_default[i]));
+	}
 }
 
 static void

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905100041.x4A0fhNT083122>