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>