Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 14 Oct 2018 05:09:44 +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: r339354 - in head/sys/dev: e1000 ixgbe
Message-ID:  <201810140509.w9E59iKD057619@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: erj
Date: Sun Oct 14 05:09:43 2018
New Revision: 339354
URL: https://svnweb.freebsd.org/changeset/base/339354

Log:
  em/igb/ix(4): Port two Tx/Rx fixes made to ixl in r339338
  
  - Fix assert/panic on receive when Jumbo Frames are enabled.
  
  From the commit I made to ixl:
  "It turns out that *_isc_rxd_available is supposed to return how many
  packets are available to be cleaned on the rx ring. This patch removes
  a section of code where if the budget argument is 1, the function would return
  one if there was a descriptor available, not necessarily a packet.
  
  This is okay in regular mtu 1500 traffic since the max frame size is less
  than the configured receive buffer size (2048), but this doesn't work when
  received packets can span more than one  descriptor, as is the case when the
  mtu is 9000 and the receive buffer size is 4096."
  
  - Fix possible Tx hang because *_isc_txd_credits_update returns incorrect result
  
  From the commit by Krzysztof Galazka to ixl: "Function isc_txd_update_credits
  called with clear set to false should return 1 if there are TX descriptors
  already handled by HW. It was always returning 0 causing troubles with UDP TX
  traffic."
  
  PR:             231659
  Reported by:    lev@
  Approved by:	re (gjb@)
  Sponsored by:   Intel Corporation

Modified:
  head/sys/dev/e1000/em_txrx.c
  head/sys/dev/e1000/igb_txrx.c
  head/sys/dev/ixgbe/ix_txrx.c

Modified: head/sys/dev/e1000/em_txrx.c
==============================================================================
--- head/sys/dev/e1000/em_txrx.c	Sun Oct 14 01:16:48 2018	(r339353)
+++ head/sys/dev/e1000/em_txrx.c	Sun Oct 14 05:09:43 2018	(r339354)
@@ -446,9 +446,14 @@ em_isc_txd_credits_update(void *arg, uint16_t txqid, b
 	status = txr->tx_base[cur].upper.fields.status;
 	updated = !!(status & E1000_TXD_STAT_DD);
 
-	if (clear == false || updated == 0)
-		return (updated);
+	if (!updated)
+		return (0);
 
+	/* If clear is false just let caller know that there
+	 * are descriptors to reclaim */
+	if (!clear)
+		return (1);
+
 	prev = txr->tx_cidx_processed;
 	ntxd = scctx->isc_ntxd[0];
 	do {
@@ -553,22 +558,14 @@ lem_isc_rxd_available(void *arg, uint16_t rxqid, qidx_
 	u32 staterr = 0;
 	int cnt, i;
 
-	if (budget == 1) {
-		rxd = (struct e1000_rx_desc *)&rxr->rx_base[idx];
-		staterr = rxd->status;
-		return (staterr & E1000_RXD_STAT_DD);
-	}
-
 	for (cnt = 0, i = idx; cnt < scctx->isc_nrxd[0] && cnt <= budget;) {
 		rxd = (struct e1000_rx_desc *)&rxr->rx_base[i];
 		staterr = rxd->status;
 
 		if ((staterr & E1000_RXD_STAT_DD) == 0)
 			break;
-
 		if (++i == scctx->isc_nrxd[0])
 			i = 0;
-
 		if (staterr & E1000_RXD_STAT_EOP)
 			cnt++;
 	}
@@ -586,26 +583,16 @@ em_isc_rxd_available(void *arg, uint16_t rxqid, qidx_t
 	u32 staterr = 0;
 	int cnt, i;
 
-	if (budget == 1) {
-		rxd = &rxr->rx_base[idx];
-		staterr = le32toh(rxd->wb.upper.status_error);
-		return (staterr & E1000_RXD_STAT_DD);
-	}
-
 	for (cnt = 0, i = idx; cnt < scctx->isc_nrxd[0] && cnt <= budget;) {
 		rxd = &rxr->rx_base[i];
 		staterr = le32toh(rxd->wb.upper.status_error);
 
 		if ((staterr & E1000_RXD_STAT_DD) == 0)
 			break;
-		
-		if (++i == scctx->isc_nrxd[0]) {
+		if (++i == scctx->isc_nrxd[0])
 			i = 0;
-		}
-
 		if (staterr & E1000_RXD_STAT_EOP)
 			cnt++;
-
 	}
 	return (cnt);
 }

Modified: head/sys/dev/e1000/igb_txrx.c
==============================================================================
--- head/sys/dev/e1000/igb_txrx.c	Sun Oct 14 01:16:48 2018	(r339353)
+++ head/sys/dev/e1000/igb_txrx.c	Sun Oct 14 05:09:43 2018	(r339354)
@@ -321,9 +321,14 @@ igb_isc_txd_credits_update(void *arg, uint16_t txqid, 
 	status = ((union e1000_adv_tx_desc *)&txr->tx_base[cur])->wb.status;
 	updated = !!(status & E1000_TXD_STAT_DD);
 
-	if (!clear || !updated)
-		return (updated);
+	if (!updated)
+		return (0);
 
+	/* If clear is false just let caller know that there
+	 * are descriptors to reclaim */
+	if (!clear)
+		return (1);
+
 	prev = txr->tx_cidx_processed;
 	ntxd = scctx->isc_ntxd[0];
 	do {
@@ -392,28 +397,18 @@ igb_isc_rxd_available(void *arg, uint16_t rxqid, qidx_
 	struct rx_ring *rxr = &que->rxr;
 	union e1000_adv_rx_desc *rxd;
 	u32 staterr = 0;
-	int cnt, i, iter;
+	int cnt, i;
 
-	if (budget == 1) {
-		rxd = (union e1000_adv_rx_desc *)&rxr->rx_base[idx];
-		staterr = le32toh(rxd->wb.upper.status_error);
-		return (staterr & E1000_RXD_STAT_DD);
-	}
-
-	for (iter = cnt = 0, i = idx; iter < scctx->isc_nrxd[0] && iter <= budget;) {
+	for (cnt = 0, i = idx; cnt < scctx->isc_nrxd[0] && cnt <= budget;) {
 		rxd = (union e1000_adv_rx_desc *)&rxr->rx_base[i];
 		staterr = le32toh(rxd->wb.upper.status_error);
 
 		if ((staterr & E1000_RXD_STAT_DD) == 0)
 			break;
-
-		if (++i == scctx->isc_nrxd[0]) {
+		if (++i == scctx->isc_nrxd[0])
 			i = 0;
-		}
-
 		if (staterr & E1000_RXD_STAT_EOP)
 			cnt++;
-		iter++;
 	}
 	return (cnt);
 }

Modified: head/sys/dev/ixgbe/ix_txrx.c
==============================================================================
--- head/sys/dev/ixgbe/ix_txrx.c	Sun Oct 14 01:16:48 2018	(r339353)
+++ head/sys/dev/ixgbe/ix_txrx.c	Sun Oct 14 05:09:43 2018	(r339354)
@@ -285,9 +285,14 @@ ixgbe_isc_txd_credits_update(void *arg, uint16_t txqid
 	status = txr->tx_base[cur].wb.status;
 	updated = !!(status & IXGBE_TXD_STAT_DD);
 
-	if (clear == false || updated == 0)
-		return (updated);
+	if (!updated)
+		return (0);
 
+	/* If clear is false just let caller know that there
+	 * are descriptors to reclaim */
+	if (!clear)
+		return (1);
+
 	prev = txr->tx_cidx_processed;
 	ntxd = scctx->isc_ntxd[0];
 	do {
@@ -362,17 +367,8 @@ ixgbe_isc_rxd_available(void *arg, uint16_t qsidx, qid
 	u32                      staterr;
 	int                      cnt, i, nrxd;
 
-	if (budget == 1) {
-		rxd = &rxr->rx_base[pidx];
-		staterr = le32toh(rxd->wb.upper.status_error);
-
-		return (staterr & IXGBE_RXD_STAT_DD);
-	}
-
 	nrxd = sc->shared->isc_nrxd[0];
-	// em has cnt < nrxd. off by 1 here or there?
-//	for (cnt = 0, i = pidx; cnt < nrxd && cnt <= budget;) {
-	for (cnt = 0, i = pidx; cnt < nrxd-1 && cnt <= budget;) {
+	for (cnt = 0, i = pidx; cnt < nrxd && cnt <= budget;) {
 		rxd = &rxr->rx_base[i];
 		staterr = le32toh(rxd->wb.upper.status_error);
 
@@ -383,7 +379,6 @@ ixgbe_isc_rxd_available(void *arg, uint16_t qsidx, qid
 		if (staterr & IXGBE_RXD_STAT_EOP)
 			cnt++;
 	}
-
 	return (cnt);
 } /* ixgbe_isc_rxd_available */
 



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