Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 2 Aug 2011 02:46:03 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r224588 - head/sys/dev/ath
Message-ID:  <201108020246.p722k3uZ023066@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Tue Aug  2 02:46:03 2011
New Revision: 224588
URL: http://svn.freebsd.org/changeset/base/224588

Log:
  Fix a corner case in RXEOL handling which was likely introduced by yours
  truly.
  
  Before 802.11n, the RX descriptor list would employ the "self-linked tail
  descriptor" trick which linked the last descriptor back to itself.
  This way, the RX engine would never hit the "end" of the list and stop
  processing RX (and assert RXEOL) as it never hit a descriptor whose next
  pointer was 0. It would just keep overwriting the last descriptor until
  the software freed up some more RX descriptors and chained them onto the
  end.
  
  For 802.11n, this needs to stop as a self-linked RX descriptor tickles the
  block-ack logic into ACK'ing whatever frames are received into that
  self-linked descriptor - so in very busy periods, you could end up with
  A-MPDU traffic that is ACKed but never received by the 802.11 stack.
  This would cause some confusion as the ADDBA windows would suddenly
  be out of sync.
  
  So when that occured here, the last descriptor would be hit and the PCU
  logic would stop. It would only start again when the RX descriptor list
  was updated and the PCU RX engine was re-tickled. That wasn't being done,
  so RXEOL would be continuously asserted and no RX would continue.
  
  This patch introduces a new flag - sc->sc_kickpcu - which when set,
  signals the RX task to kick the PCU after its processed whatever packets
  it can. This way completed packets aren't discarded.
  
  In case some other task gets called which resets the hardware, don't
  update sc->sc_imask - instead, just update the hardware interrupt mask
  directly and let either ath_rx_proc() or ath_reset() restore the imask
  to its former setting.
  
  Note: this bug was only triggered when doing a whole lot of frame snooping
  with serial console IO in the RX task. This would defer interrupt processing
  enough to cause an RX descriptor overflow. It doesn't happen in normal
  conditions.
  
  Approved by: re (kib, blanket)

Modified:
  head/sys/dev/ath/if_ath.c
  head/sys/dev/ath/if_athvar.h

Modified: head/sys/dev/ath/if_ath.c
==============================================================================
--- head/sys/dev/ath/if_ath.c	Tue Aug  2 01:48:45 2011	(r224587)
+++ head/sys/dev/ath/if_ath.c	Tue Aug  2 02:46:03 2011	(r224588)
@@ -1389,6 +1389,7 @@ ath_intr(void *arg)
 			}
 		}
 		if (status & HAL_INT_RXEOL) {
+			int imask = sc->sc_imask;
 			/*
 			 * NB: the hardware should re-read the link when
 			 *     RXE bit is written, but it doesn't work at
@@ -1398,10 +1399,22 @@ ath_intr(void *arg)
 			/*
 			 * Disable RXEOL/RXORN - prevent an interrupt
 			 * storm until the PCU logic can be reset.
+			 * In case the interface is reset some other
+			 * way before "sc_kickpcu" is called, don't
+			 * modify sc_imask - that way if it is reset
+			 * by a call to ath_reset() somehow, the
+			 * interrupt mask will be correctly reprogrammed.
 			 */
-			sc->sc_imask &= ~(HAL_INT_RXEOL | HAL_INT_RXORN);
-			ath_hal_intrset(ah, sc->sc_imask);
+			imask &= ~(HAL_INT_RXEOL | HAL_INT_RXORN);
+			ath_hal_intrset(ah, imask);
+			/*
+			 * Enqueue an RX proc, to handled whatever
+			 * is in the RX queue.
+			 * This will then kick the PCU.
+			 */
+			taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask);
 			sc->sc_rxlink = NULL;
+			sc->sc_kickpcu = 1;
 		}
 		if (status & HAL_INT_TXURN) {
 			sc->sc_stats.ast_txurn++;
@@ -3776,6 +3789,25 @@ rx_next:
 	if (ath_dfs_tasklet_needed(sc, sc->sc_curchan))
 		taskqueue_enqueue(sc->sc_tq, &sc->sc_dfstask);
 
+	/*
+	 * Now that all the RX frames were handled that
+	 * need to be handled, kick the PCU if there's
+	 * been an RXEOL condition.
+	 */
+	if (sc->sc_kickpcu) {
+		sc->sc_kickpcu = 0;
+		ath_stoprecv(sc);
+		sc->sc_imask |= (HAL_INT_RXEOL | HAL_INT_RXORN);
+		if (ath_startrecv(sc) != 0) {
+			if_printf(ifp,
+			    "%s: couldn't restart RX after RXEOL; resetting\n",
+			    __func__);
+			ath_reset(ifp);
+			return;
+		}
+		ath_hal_intrset(ah, sc->sc_imask);
+	}
+
 	if ((ifp->if_drv_flags & IFF_DRV_OACTIVE) == 0) {
 #ifdef IEEE80211_SUPPORT_SUPERG
 		ieee80211_ff_age_all(ic, 100);

Modified: head/sys/dev/ath/if_athvar.h
==============================================================================
--- head/sys/dev/ath/if_athvar.h	Tue Aug  2 01:48:45 2011	(r224587)
+++ head/sys/dev/ath/if_athvar.h	Tue Aug  2 02:46:03 2011	(r224588)
@@ -254,7 +254,8 @@ struct ath_softc {
 				sc_tdma	    : 1,/* TDMA in use */
 				sc_setcca   : 1,/* set/clr CCA with TDMA */
 				sc_resetcal : 1,/* reset cal state next trip */
-				sc_rxslink  : 1;/* do self-linked final descriptor */
+				sc_rxslink  : 1,/* do self-linked final descriptor */
+				sc_kickpcu  : 1;/* kick PCU RX on next RX proc */
 	uint32_t		sc_eerd;	/* regdomain from EEPROM */
 	uint32_t		sc_eecc;	/* country code from EEPROM */
 						/* rate tables */



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