Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Sep 2017 16:59:37 +0000 (UTC)
From:      Stephen Hurd <shurd@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r323955 - head/sys/dev/bnxt
Message-ID:  <201709231659.v8NGxbo0044863@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: shurd
Date: Sat Sep 23 16:59:37 2017
New Revision: 323955
URL: https://svnweb.freebsd.org/changeset/base/323955

Log:
  bnxt: Choose better HW LRO defaults for performance
  
  1) Choose correct Firmware options for HW LRO for best performance
  2) Delete TBD and other comments which are not required.
  3) Added sysctl interface to enable / disable / modify different factors
     of HW LRO.
  4) Disabled HW LRO by default to avoid issues with packet forwarding
  
  This allows much better control over the LRO configuration via sysctls, and
  uses much better defaults.  Hardware LRO can now be enabled/disabled
  independantly from the software LRO, and the tuning parameters are exposed.
  
  manpage updates coming soon.
  
  Submitted by:	Bhargava Chenna Marreddy <bhargava.marreddy@broadcom.com>
  Reviewed by:	shurd, sbruno
  Approved by:	sbruno (mentor)
  Sponsored by:	Broadcom Limited
  Differential Revision:	https://reviews.freebsd.org/D12223

Modified:
  head/sys/dev/bnxt/bnxt.h
  head/sys/dev/bnxt/bnxt_hwrm.c
  head/sys/dev/bnxt/bnxt_hwrm.h
  head/sys/dev/bnxt/bnxt_sysctl.c
  head/sys/dev/bnxt/bnxt_sysctl.h
  head/sys/dev/bnxt/if_bnxt.c

Modified: head/sys/dev/bnxt/bnxt.h
==============================================================================
--- head/sys/dev/bnxt/bnxt.h	Sat Sep 23 16:46:30 2017	(r323954)
+++ head/sys/dev/bnxt/bnxt.h	Sat Sep 23 16:59:37 2017	(r323955)
@@ -526,6 +526,14 @@ struct bnxt_func_qcfg {
 	uint16_t alloc_vnics;
 };
 
+struct bnxt_hw_lro {
+	uint16_t enable;
+	uint16_t is_mode_gro;
+	uint16_t max_agg_segs;
+	uint16_t max_aggs;
+	uint32_t min_agg_len;
+};
+
 struct bnxt_softc {
 	device_t	dev;
 	if_ctx_t	ctx;
@@ -586,10 +594,13 @@ struct bnxt_softc {
 
 	struct sysctl_ctx_list	hw_stats;
 	struct sysctl_oid	*hw_stats_oid;
+	struct sysctl_ctx_list	hw_lro_ctx;
+	struct sysctl_oid	*hw_lro_oid;
 
 	struct bnxt_ver_info	*ver_info;
 	struct bnxt_nvram_info	*nvm_info;
 	bool wol;
+	struct bnxt_hw_lro	hw_lro;
 	uint8_t wol_filter_id;
 	uint16_t		rx_coal_usecs;
 	uint16_t		rx_coal_usecs_irq;

Modified: head/sys/dev/bnxt/bnxt_hwrm.c
==============================================================================
--- head/sys/dev/bnxt/bnxt_hwrm.c	Sat Sep 23 16:46:30 2017	(r323954)
+++ head/sys/dev/bnxt/bnxt_hwrm.c	Sat Sep 23 16:59:37 2017	(r323955)
@@ -977,29 +977,53 @@ bnxt_cfg_async_cr(struct bnxt_softc *softc)
 	return rc;
 }
 
+void
+bnxt_validate_hw_lro_settings(struct bnxt_softc *softc)
+{
+	softc->hw_lro.enable = min(softc->hw_lro.enable, 1);
+
+        softc->hw_lro.is_mode_gro = min(softc->hw_lro.is_mode_gro, 1);
+
+	softc->hw_lro.max_agg_segs = min(softc->hw_lro.max_agg_segs,
+		HWRM_VNIC_TPA_CFG_INPUT_MAX_AGG_SEGS_MAX);
+
+	softc->hw_lro.max_aggs = min(softc->hw_lro.max_aggs,
+		HWRM_VNIC_TPA_CFG_INPUT_MAX_AGGS_MAX);
+
+	softc->hw_lro.min_agg_len = min(softc->hw_lro.min_agg_len, BNXT_MAX_MTU);
+}
+
 int
-bnxt_hwrm_vnic_tpa_cfg(struct bnxt_softc *softc, struct bnxt_vnic_info *vnic,
-    uint32_t flags)
+bnxt_hwrm_vnic_tpa_cfg(struct bnxt_softc *softc)
 {
 	struct hwrm_vnic_tpa_cfg_input req = {0};
+	uint32_t flags;
 
 	bnxt_hwrm_cmd_hdr_init(softc, &req, HWRM_VNIC_TPA_CFG);
 
-	req.flags = htole32(flags);
-	req.vnic_id = htole16(vnic->id);
-	req.enables = htole32(HWRM_VNIC_TPA_CFG_INPUT_ENABLES_MAX_AGG_SEGS |
-	    HWRM_VNIC_TPA_CFG_INPUT_ENABLES_MAX_AGGS |
-	    /* HWRM_VNIC_TPA_CFG_INPUT_ENABLES_MAX_AGG_TIMER | */
-	    HWRM_VNIC_TPA_CFG_INPUT_ENABLES_MIN_AGG_LEN);
-	/* TODO: Calculate this based on ring size? */
-	req.max_agg_segs = htole16(3);
-	/* Base this in the allocated TPA start size... */
-	req.max_aggs = htole16(7);
-	/*
-	 * TODO: max_agg_timer?
-	 * req.mag_agg_timer = htole32(XXX);
-	 */
-	req.min_agg_len = htole32(0);
+	if (softc->hw_lro.enable) {
+		flags = HWRM_VNIC_TPA_CFG_INPUT_FLAGS_TPA |
+			HWRM_VNIC_TPA_CFG_INPUT_FLAGS_ENCAP_TPA |
+			HWRM_VNIC_TPA_CFG_INPUT_FLAGS_AGG_WITH_ECN |
+			HWRM_VNIC_TPA_CFG_INPUT_FLAGS_AGG_WITH_SAME_GRE_SEQ;
+		
+        	if (softc->hw_lro.is_mode_gro)
+			flags |= HWRM_VNIC_TPA_CFG_INPUT_FLAGS_GRO;
+		else
+			flags |= HWRM_VNIC_TPA_CFG_INPUT_FLAGS_RSC_WND_UPDATE;
+			
+		req.flags = htole32(flags);
+
+		req.enables = htole32(HWRM_VNIC_TPA_CFG_INPUT_ENABLES_MAX_AGG_SEGS |
+				HWRM_VNIC_TPA_CFG_INPUT_ENABLES_MAX_AGGS |
+				HWRM_VNIC_TPA_CFG_INPUT_ENABLES_MIN_AGG_LEN);
+
+		req.max_agg_segs = htole16(softc->hw_lro.max_agg_segs);
+		req.max_aggs = htole16(softc->hw_lro.max_aggs);
+		req.min_agg_len = htole32(softc->hw_lro.min_agg_len);
+	}
+
+	req.vnic_id = htole16(softc->vnic_info.id);
 
 	return hwrm_send_message(softc, &req, sizeof(req));
 }

Modified: head/sys/dev/bnxt/bnxt_hwrm.h
==============================================================================
--- head/sys/dev/bnxt/bnxt_hwrm.h	Sat Sep 23 16:46:30 2017	(r323954)
+++ head/sys/dev/bnxt/bnxt_hwrm.h	Sat Sep 23 16:59:37 2017	(r323955)
@@ -62,8 +62,8 @@ int bnxt_hwrm_set_filter(struct bnxt_softc *softc, str
 int bnxt_hwrm_rss_cfg(struct bnxt_softc *softc, struct bnxt_vnic_info *vnic,
     uint32_t hash_type);
 int bnxt_cfg_async_cr(struct bnxt_softc *softc);
-int bnxt_hwrm_vnic_tpa_cfg(struct bnxt_softc *softc,
-    struct bnxt_vnic_info *vnic, uint32_t flags);
+int bnxt_hwrm_vnic_tpa_cfg(struct bnxt_softc *softc);
+void bnxt_validate_hw_lro_settings(struct bnxt_softc *softc);
 int bnxt_hwrm_nvm_find_dir_entry(struct bnxt_softc *softc, uint16_t type,
     uint16_t *ordinal, uint16_t ext, uint16_t *index, bool use_index,
     uint8_t search_opt, uint32_t *data_length, uint32_t *item_length,

Modified: head/sys/dev/bnxt/bnxt_sysctl.c
==============================================================================
--- head/sys/dev/bnxt/bnxt_sysctl.c	Sat Sep 23 16:46:30 2017	(r323954)
+++ head/sys/dev/bnxt/bnxt_sysctl.c	Sat Sep 23 16:59:37 2017	(r323955)
@@ -84,6 +84,16 @@ bnxt_init_sysctl_ctx(struct bnxt_softc *softc)
 		return ENOMEM;
 	}
 
+	sysctl_ctx_init(&softc->hw_lro_ctx);
+	ctx = device_get_sysctl_ctx(softc->dev);
+	softc->hw_lro_oid = SYSCTL_ADD_NODE(ctx,
+	    SYSCTL_CHILDREN(device_get_sysctl_tree(softc->dev)), OID_AUTO,
+	    "hw_lro", CTLFLAG_RD, 0, "hardware lro");
+	if (!softc->hw_lro_oid) {
+		sysctl_ctx_free(&softc->hw_lro_ctx);
+		return ENOMEM;
+	}
+
 	return 0;
 }
 
@@ -114,6 +124,13 @@ bnxt_free_sysctl_ctx(struct bnxt_softc *softc)
 		else
 			softc->nvm_info->nvm_oid = NULL;
 	}
+	if (softc->hw_lro_oid != NULL) {
+		orc = sysctl_ctx_free(&softc->hw_lro_ctx);
+		if (orc)
+			rc = orc;
+		else
+			softc->hw_lro_oid = NULL;
+	}
 
 	return rc;
 }
@@ -1210,6 +1227,74 @@ bnxt_create_config_sysctls_pre(struct bnxt_softc *soft
 	return 0;
 }
 
+#define BNXT_HW_LRO_FN(fn_name, arg)			                   \
+static int						                   \
+fn_name(SYSCTL_HANDLER_ARGS) {				                   \
+	struct bnxt_softc *softc = arg1;		                   \
+	int rc;						                   \
+	int val;					                   \
+							                   \
+	if (softc == NULL)				                   \
+		return EBUSY;				                   \
+							                   \
+	val = softc->hw_lro.arg;			                   \
+	rc = sysctl_handle_int(oidp, &val, 0, req);	                   \
+	if (rc || !req->newptr)				                   \
+		return rc;				                   \
+							                   \
+	if ((if_getdrvflags(iflib_get_ifp(softc->ctx)) & IFF_DRV_RUNNING)) \
+		return EBUSY;				                   \
+							                   \
+	softc->hw_lro.arg = val;			                   \
+	bnxt_validate_hw_lro_settings(softc);		                   \
+	rc = bnxt_hwrm_vnic_tpa_cfg(softc);		                   \
+							                   \
+	return rc;					                   \
+}
+
+BNXT_HW_LRO_FN(bnxt_hw_lro_enable_disable, enable)
+BNXT_HW_LRO_FN(bnxt_hw_lro_set_mode, is_mode_gro)
+BNXT_HW_LRO_FN(bnxt_hw_lro_set_max_agg_segs, max_agg_segs)
+BNXT_HW_LRO_FN(bnxt_hw_lro_set_max_aggs, max_aggs)
+BNXT_HW_LRO_FN(bnxt_hw_lro_set_min_agg_len, min_agg_len)
+
+int
+bnxt_create_hw_lro_sysctls(struct bnxt_softc *softc)
+{
+	struct sysctl_oid *oid = softc->hw_lro_oid;
+
+	if (!oid)
+		return ENOMEM;
+
+	SYSCTL_ADD_PROC(&softc->hw_lro_ctx, SYSCTL_CHILDREN(oid), OID_AUTO,
+			"enable", CTLTYPE_INT|CTLFLAG_RWTUN, softc, 0,
+			bnxt_hw_lro_enable_disable, "A",
+			"Enable or Disable HW LRO: 0 / 1");
+
+	SYSCTL_ADD_PROC(&softc->hw_lro_ctx, SYSCTL_CHILDREN(oid), OID_AUTO,
+			"gro_mode", CTLTYPE_INT|CTLFLAG_RWTUN, softc, 0,
+			bnxt_hw_lro_set_mode, "A",
+			"Set mode: 1 = GRO mode, 0 = RSC mode");
+
+	SYSCTL_ADD_PROC(&softc->hw_lro_ctx, SYSCTL_CHILDREN(oid), OID_AUTO,
+			"max_agg_segs", CTLTYPE_INT|CTLFLAG_RWTUN, softc, 0,
+			bnxt_hw_lro_set_max_agg_segs, "A",
+			"Set Max Agg Seg Value (unit is Log2): "
+			"0 (= 1 seg) / 1 (= 2 segs) /  ... / 31 (= 2^31 segs)");
+	
+        SYSCTL_ADD_PROC(&softc->hw_lro_ctx, SYSCTL_CHILDREN(oid), OID_AUTO,
+			"max_aggs", CTLTYPE_INT|CTLFLAG_RWTUN, softc, 0,
+			bnxt_hw_lro_set_max_aggs, "A",
+			"Set Max Aggs Value (unit is Log2): "
+			"0 (= 1 agg) / 1 (= 2 aggs) /  ... / 7 (= 2^7 segs)"); 
+
+	SYSCTL_ADD_PROC(&softc->hw_lro_ctx, SYSCTL_CHILDREN(oid), OID_AUTO,
+			"min_agg_len", CTLTYPE_INT|CTLFLAG_RWTUN, softc, 0,
+			bnxt_hw_lro_set_min_agg_len, "A",
+			"Min Agg Len: 1 to 9000");
+
+	return 0;
+}
 static int
 bnxt_vlan_only_sysctl(SYSCTL_HANDLER_ARGS) {
 	struct bnxt_softc *softc = arg1;

Modified: head/sys/dev/bnxt/bnxt_sysctl.h
==============================================================================
--- head/sys/dev/bnxt/bnxt_sysctl.h	Sat Sep 23 16:46:30 2017	(r323954)
+++ head/sys/dev/bnxt/bnxt_sysctl.h	Sat Sep 23 16:59:37 2017	(r323955)
@@ -40,3 +40,4 @@ int bnxt_create_ver_sysctls(struct bnxt_softc *softc);
 int bnxt_create_nvram_sysctls(struct bnxt_nvram_info *ni);
 int bnxt_create_config_sysctls_pre(struct bnxt_softc *softc);
 int bnxt_create_config_sysctls_post(struct bnxt_softc *softc);
+int bnxt_create_hw_lro_sysctls(struct bnxt_softc *softc);

Modified: head/sys/dev/bnxt/if_bnxt.c
==============================================================================
--- head/sys/dev/bnxt/if_bnxt.c	Sat Sep 23 16:46:30 2017	(r323954)
+++ head/sys/dev/bnxt/if_bnxt.c	Sat Sep 23 16:59:37 2017	(r323955)
@@ -826,6 +826,17 @@ bnxt_attach_pre(if_ctx_t ctx)
 	/* iflib will map and release this bar */
 	scctx->isc_msix_bar = pci_msix_table_bar(softc->dev);
 
+        /* 
+         * Default settings for HW LRO (TPA):
+         *  Disable HW LRO by default
+         *  Can be enabled after taking care of 'packet forwarding'
+         */
+	softc->hw_lro.enable = 0;
+	softc->hw_lro.is_mode_gro = 0;
+	softc->hw_lro.max_agg_segs = 5; /* 2^5 = 32 segs */
+	softc->hw_lro.max_aggs = HWRM_VNIC_TPA_CFG_INPUT_MAX_AGGS_MAX;
+	softc->hw_lro.min_agg_len = 512;
+
 	/* Allocate the default completion ring */
 	softc->def_cp_ring.stats_ctx_id = HWRM_NA_SIGNATURE;
 	softc->def_cp_ring.ring.phys_id = (uint16_t)HWRM_NA_SIGNATURE;
@@ -861,6 +872,10 @@ bnxt_attach_pre(if_ctx_t ctx)
 	if (rc)
 		goto failed;
 
+	rc = bnxt_create_hw_lro_sysctls(softc);
+	if (rc)
+		goto failed;
+
 	/* Initialize the vlan list */
 	SLIST_INIT(&softc->vnic_info.vlan_tags);
 	softc->vnic_info.vlan_tag_list.idi_vaddr = NULL;
@@ -1071,15 +1086,7 @@ bnxt_init(if_ctx_t ctx)
 	if (rc)
 		goto fail;
 
-	/* 
-         * Enable LRO/TPA/GRO 
-         * TBD: 
-         *      Enable / Disable HW_LRO based on
-         *      ifconfig lro / ifconfig -lro setting
-         */
-	rc = bnxt_hwrm_vnic_tpa_cfg(softc, &softc->vnic_info,
-	    (if_getcapenable(iflib_get_ifp(ctx)) & IFCAP_LRO) ?
-	    HWRM_VNIC_TPA_CFG_INPUT_FLAGS_TPA : 0);
+	rc = bnxt_hwrm_vnic_tpa_cfg(softc);
 	if (rc)
 		goto fail;
 



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