I'm seeing scenarios where the TID pauses, tying up all ath_buf entries (and mbufs, further up in the network stack) until the STA is forcibly removed. Here's a snippet from the AP log: ath1: ath_tx_tid_drain: node 0xc1d8a000: bf=0xc1cd417c: addbaw=1, dobaw=1, seqno_assign=1, seqno_required=1, seqno=1560, retry=10 ath1: ath_tx_tid_drain: node 0xc1d8a000: bf=0xc1cd417c: tid txq_depth=78 hwq_depth=0, bar_wait=1 ath1: ath_tx_tid_drain: node 0xc1d8a000: bf=0xc1cd417c: tid 0: txq_depth=0, txq_aggr_depth=0, sched=0, paused=1, hwq_depth=0, incomp=0, baw_head=123, baw_tail=33 txa_start=1537, ni_txseqs=1575 After doing a whole lot of digging, I found this: * ieee80211_bar_send() is called from the wifi driver. If it fails there then the wifi driver will have to consider the BAR failed and continue. * if it succeeds, then the wifi driver should hold TXing any further frames until it completes. * If it does succeed, then bar_start_timer() is called. * If the TX completes succesfully, then everything is ok * But then, if it fails - either it's told that it fails or it times out, bar_timeout() is (eventually) called. The timer here has completed, so it won't be called again. * It then calls ieee80211_send_bar(), which should try sending again. * .. but if the call to ieee80211_send_bar() fails, the timer is never restarted - its only restarted _if_ ieee80211_send_bar() actually successfully completed. Fix: I'm not entirely sure. It may be good enough to just call bar_start_timer() from bar_timeout(), just to enforce that the timer needs to be kicked along whether or not ieee80211_send_bar() succeeded. The other problem is resource exhaustion. At this point, all ieee80211_send_bar() calls may fail because the driver is completely out of internal buffers (here ath_buf), or the network stack has exhausted all mbufs. So it likely requires a few fixes in a few places. How-To-Repeat: Fire up 802.11n on ath(4) in hostap mode; wait for TX failures to occur.
Responsible Changed From-To: freebsd-bugs->freebsd-wireless Over to maintainer(s).
Author: adrian Date: Tue May 22 19:37:12 2012 New Revision: 235801 URL: http://svn.freebsd.org/changeset/base/235801 Log: Fix some corner cases in the ieee80211_send_bar() handling. * If the first call succeeded but failed to transmit, a timer would reschedule it via bar_timeout(). Unfortunately bar_timeout() didn't check the return value from the ieee80211_send_bar() reattempt and if that failed (eg the driver ic_raw_xmit() failed), it would never re-arm the timer. * If BARPEND is cleared (which ieee80211_send_bar() will do if it can't TX), then re-arming the timer isn't enough - once bar_timeout() occurs, it'll see BARPEND is 0 and not run through the rest of the routine. So when rearming the timer, also set that flag. * If the TX wasn't occuring, bar_tx_complete() wouldn't be called and the driver callback wouldn't be called either. So the driver had no idea that the BAR TX attempt had failed. In the ath(4) case, TX would stay paused. (There's no callback to indicate that BAR TX had failed or not; only a "BAR TX was attempted". That's a separate, later problem.) So call the driver callback (ic_bar_response()) before the ADDBA session is torn down, so it has a chance of being notified that things didn't quite go to plan. I've verified that yes, this does suspend traffic for ath(4), retry BAR TX even if the driver is failing ic_raw_xmit(), and then eventually giving up and sending a DELBA. I'll address the "out of ath_buf" issue in ath(4) in a subsequent commit - this commit just fixes the edge case where any driver is (way) out of internal buffers/descriptors and fails frame TX. PR: kern/168170 Reviewed by: bschmidt MFC after: 1 month Modified: head/sys/net80211/ieee80211_ht.c Modified: head/sys/net80211/ieee80211_ht.c ============================================================================== --- head/sys/net80211/ieee80211_ht.c Tue May 22 18:31:56 2012 (r235800) +++ head/sys/net80211/ieee80211_ht.c Tue May 22 19:37:12 2012 (r235801) @@ -2166,6 +2166,9 @@ ieee80211_ampdu_stop(struct ieee80211_no } } +/* XXX */ +static void bar_start_timer(struct ieee80211_tx_ampdu *tap); + static void bar_timeout(void *arg) { @@ -2184,11 +2187,34 @@ bar_timeout(void *arg) return; /* XXX ? */ if (tap->txa_attempts >= ieee80211_bar_maxtries) { + struct ieee80211com *ic = ni->ni_ic; + ni->ni_vap->iv_stats.is_ampdu_bar_tx_fail++; + /* + * If (at least) the last BAR TX timeout was due to + * an ieee80211_send_bar() failures, then we need + * to make sure we notify the driver that a BAR + * TX did occur and fail. This gives the driver + * a chance to undo any queue pause that may + * have occured. + */ + ic->ic_bar_response(ni, tap, 1); ieee80211_ampdu_stop(ni, tap, IEEE80211_REASON_TIMEOUT); } else { ni->ni_vap->iv_stats.is_ampdu_bar_tx_retry++; - ieee80211_send_bar(ni, tap, tap->txa_seqpending); + if (ieee80211_send_bar(ni, tap, tap->txa_seqpending) != 0) { + /* + * If ieee80211_send_bar() fails here, the + * timer may have stopped and/or the pending + * flag may be clear. Because of this, + * fake the BARPEND and reset the timer. + * A retransmission attempt will then occur + * during the next timeout. + */ + /* XXX locking */ + tap->txa_flags |= IEEE80211_AGGR_BARPEND; + bar_start_timer(tap); + } } } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: adrian Date: Wed Jun 13 05:39:16 2012 New Revision: 236993 URL: http://svn.freebsd.org/changeset/base/236993 Log: Replace the direct sc_txbuf manipulation with a pair of functions. This is preparation work for having a separate ath_buf queue for management traffic. PR: kern/168170 Modified: head/sys/dev/ath/if_ath.c head/sys/dev/ath/if_ath_misc.h head/sys/dev/ath/if_ath_tx.c Modified: head/sys/dev/ath/if_ath.c ============================================================================== --- head/sys/dev/ath/if_ath.c Wed Jun 13 05:02:51 2012 (r236992) +++ head/sys/dev/ath/if_ath.c Wed Jun 13 05:39:16 2012 (r236993) @@ -2358,7 +2358,7 @@ ath_start(struct ifnet *ifp) IFQ_DEQUEUE(&ifp->if_snd, m); if (m == NULL) { ATH_TXBUF_LOCK(sc); - TAILQ_INSERT_HEAD(&sc->sc_txbuf, bf, bf_list); + ath_returnbuf_head(sc, bf); ATH_TXBUF_UNLOCK(sc); break; } @@ -2401,7 +2401,7 @@ ath_start(struct ifnet *ifp) bf->bf_m = NULL; bf->bf_node = NULL; ATH_TXBUF_LOCK(sc); - TAILQ_INSERT_HEAD(&sc->sc_txbuf, bf, bf_list); + ath_returnbuf_head(sc, bf); ath_txfrag_cleanup(sc, &frags, ni); ATH_TXBUF_UNLOCK(sc); if (ni != NULL) @@ -3631,6 +3631,24 @@ ath_txq_sched_tasklet(void *arg, int npe ATH_PCU_UNLOCK(sc); } +void +ath_returnbuf_tail(struct ath_softc *sc, struct ath_buf *bf) +{ + + ATH_TXBUF_LOCK_ASSERT(sc); + + TAILQ_INSERT_TAIL(&sc->sc_txbuf, bf, bf_list); +} + +void +ath_returnbuf_head(struct ath_softc *sc, struct ath_buf *bf) +{ + + ATH_TXBUF_LOCK_ASSERT(sc); + + TAILQ_INSERT_HEAD(&sc->sc_txbuf, bf, bf_list); +} + /* * Return a buffer to the pool and update the 'busy' flag on the * previous 'tail' entry. @@ -3653,7 +3671,7 @@ ath_freebuf(struct ath_softc *sc, struct ATH_TXBUF_LOCK(sc); ath_tx_update_busy(sc); - TAILQ_INSERT_TAIL(&sc->sc_txbuf, bf, bf_list); + ath_returnbuf_tail(sc, bf); ATH_TXBUF_UNLOCK(sc); } Modified: head/sys/dev/ath/if_ath_misc.h ============================================================================== --- head/sys/dev/ath/if_ath_misc.h Wed Jun 13 05:02:51 2012 (r236992) +++ head/sys/dev/ath/if_ath_misc.h Wed Jun 13 05:39:16 2012 (r236993) @@ -55,6 +55,8 @@ extern struct ath_buf * _ath_getbuf_lock extern struct ath_buf * ath_buf_clone(struct ath_softc *sc, const struct ath_buf *bf); extern void ath_freebuf(struct ath_softc *sc, struct ath_buf *bf); +extern void ath_returnbuf_head(struct ath_softc *sc, struct ath_buf *bf); +extern void ath_returnbuf_tail(struct ath_softc *sc, struct ath_buf *bf); extern int ath_reset(struct ifnet *, ATH_RESET_TYPE); extern void ath_tx_draintxq(struct ath_softc *sc, struct ath_txq *txq); Modified: head/sys/dev/ath/if_ath_tx.c ============================================================================== --- head/sys/dev/ath/if_ath_tx.c Wed Jun 13 05:02:51 2012 (r236992) +++ head/sys/dev/ath/if_ath_tx.c Wed Jun 13 05:39:16 2012 (r236993) @@ -184,7 +184,7 @@ ath_txfrag_cleanup(struct ath_softc *sc, TAILQ_FOREACH_SAFE(bf, frags, bf_list, next) { /* NB: bf assumed clean */ TAILQ_REMOVE(frags, bf, bf_list); - TAILQ_INSERT_HEAD(&sc->sc_txbuf, bf, bf_list); + ath_returnbuf_head(sc, bf); ieee80211_node_decref(ni); } } @@ -1916,7 +1916,7 @@ ath_raw_xmit(struct ieee80211_node *ni, return 0; bad2: ATH_TXBUF_LOCK(sc); - TAILQ_INSERT_HEAD(&sc->sc_txbuf, bf, bf_list); + ath_returnbuf_head(sc, bf); ATH_TXBUF_UNLOCK(sc); bad: ATH_PCU_LOCK(sc); @@ -3137,7 +3137,7 @@ ath_tx_retry_clone(struct ath_softc *sc, * the list.) */ ATH_TXBUF_LOCK(sc); - TAILQ_INSERT_HEAD(&sc->sc_txbuf, nbf, bf_list); + ath_returnbuf_head(sc, bf); ATH_TXBUF_UNLOCK(sc); return NULL; } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: adrian Date: Wed Jun 13 05:41:00 2012 New Revision: 236994 URL: http://svn.freebsd.org/changeset/base/236994 Log: Oops, return the newly allocated buffer to the queue, not the completed buffer. PR: kern/168170 Modified: head/sys/dev/ath/if_ath_tx.c Modified: head/sys/dev/ath/if_ath_tx.c ============================================================================== --- head/sys/dev/ath/if_ath_tx.c Wed Jun 13 05:39:16 2012 (r236993) +++ head/sys/dev/ath/if_ath_tx.c Wed Jun 13 05:41:00 2012 (r236994) @@ -3137,7 +3137,7 @@ ath_tx_retry_clone(struct ath_softc *sc, * the list.) */ ATH_TXBUF_LOCK(sc); - ath_returnbuf_head(sc, bf); + ath_returnbuf_head(sc, nbf); ATH_TXBUF_UNLOCK(sc); return NULL; } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: adrian Date: Wed Jun 13 06:57:55 2012 New Revision: 237000 URL: http://svn.freebsd.org/changeset/base/237000 Log: Implement a separate, smaller pool of ath_buf entries for use by management traffic. * Create sc_mgmt_txbuf and sc_mgmt_txdesc, initialise/free them appropriately. * Create an enum to represent buffer types in the API. * Extend ath_getbuf() and _ath_getbuf_locked() to take the above enum. * Right now anything sent via ic_raw_xmit() allocates via ATH_BUFTYPE_MGMT. This may not be very useful. * Add ATH_BUF_MGMT flag (ath_buf.bf_flags) which indicates the current buffer is a mgmt buffer and should go back onto the mgmt free list. * Extend 'txagg' to include debugging output for both normal and mgmt txbufs. * When checking/clearing ATH_BUF_BUSY, do it on both TX pools. Tested: * STA mode, with heavy UDP injection via iperf. This filled the TX queue however BARs were still going out successfully. TODO: * Initialise the mgmt buffers with ATH_BUF_MGMT and then ensure the right type is being allocated and freed on the appropriate list. That'd save a write operation (to bf->bf_flags) on each buffer alloc/free. * Test on AP mode, ensure that BAR TX and probe responses go out nicely when the main TX queue is filled (eg with paused traffic to a TID, awaiting a BAR to complete.) PR: kern/168170 Modified: head/sys/dev/ath/if_ath.c head/sys/dev/ath/if_ath_misc.h head/sys/dev/ath/if_ath_sysctl.c head/sys/dev/ath/if_ath_tx.c head/sys/dev/ath/if_athvar.h Modified: head/sys/dev/ath/if_ath.c ============================================================================== --- head/sys/dev/ath/if_ath.c Wed Jun 13 06:46:00 2012 (r236999) +++ head/sys/dev/ath/if_ath.c Wed Jun 13 06:57:55 2012 (r237000) @@ -246,6 +246,10 @@ static int ath_txbuf = ATH_TXBUF; /* # SYSCTL_INT(_hw_ath, OID_AUTO, txbuf, CTLFLAG_RW, &ath_txbuf, 0, "tx buffers allocated"); TUNABLE_INT("hw.ath.txbuf", &ath_txbuf); +static int ath_txbuf_mgmt = ATH_MGMT_TXBUF; /* # mgmt tx buffers to allocate */ +SYSCTL_INT(_hw_ath, OID_AUTO, txbuf_mgmt, CTLFLAG_RW, &ath_txbuf_mgmt, + 0, "tx (mgmt) buffers allocated"); +TUNABLE_INT("hw.ath.txbuf_mgmt", &ath_txbuf_mgmt); int ath_bstuck_threshold = 4; /* max missed beacons */ SYSCTL_INT(_hw_ath, OID_AUTO, bstuck, CTLFLAG_RW, &ath_bstuck_threshold, @@ -2212,13 +2216,17 @@ ath_reset_vap(struct ieee80211vap *vap, } struct ath_buf * -_ath_getbuf_locked(struct ath_softc *sc) +_ath_getbuf_locked(struct ath_softc *sc, ath_buf_type_t btype) { struct ath_buf *bf; ATH_TXBUF_LOCK_ASSERT(sc); - bf = TAILQ_FIRST(&sc->sc_txbuf); + if (btype == ATH_BUFTYPE_MGMT) + bf = TAILQ_FIRST(&sc->sc_txbuf_mgmt); + else + bf = TAILQ_FIRST(&sc->sc_txbuf); + if (bf == NULL) { sc->sc_stats.ast_tx_getnobuf++; } else { @@ -2228,18 +2236,29 @@ _ath_getbuf_locked(struct ath_softc *sc) } } - if (bf != NULL && (bf->bf_flags & ATH_BUF_BUSY) == 0) - TAILQ_REMOVE(&sc->sc_txbuf, bf, bf_list); - else + if (bf != NULL && (bf->bf_flags & ATH_BUF_BUSY) == 0) { + if (btype == ATH_BUFTYPE_MGMT) + TAILQ_REMOVE(&sc->sc_txbuf_mgmt, bf, bf_list); + else + TAILQ_REMOVE(&sc->sc_txbuf, bf, bf_list); + } else bf = NULL; if (bf == NULL) { + /* XXX should check which list, mgmt or otherwise */ DPRINTF(sc, ATH_DEBUG_XMIT, "%s: %s\n", __func__, TAILQ_FIRST(&sc->sc_txbuf) == NULL ? "out of xmit buffers" : "xmit buffer busy"); return NULL; } + /* XXX TODO: should do this at buffer list initialisation */ + /* XXX (then, ensure the buffer has the right flag set) */ + if (btype == ATH_BUFTYPE_MGMT) + bf->bf_flags |= ATH_BUF_MGMT; + else + bf->bf_flags &= (~ATH_BUF_MGMT); + /* Valid bf here; clear some basic fields */ bf->bf_next = NULL; /* XXX just to be sure */ bf->bf_last = NULL; /* XXX again, just to be sure */ @@ -2274,7 +2293,9 @@ ath_buf_clone(struct ath_softc *sc, cons { struct ath_buf *tbf; - tbf = ath_getbuf(sc); + tbf = ath_getbuf(sc, + (bf->bf_flags & ATH_BUF_MGMT) ? + ATH_BUFTYPE_MGMT : ATH_BUFTYPE_NORMAL); if (tbf == NULL) return NULL; /* XXX failure? Why? */ @@ -2302,12 +2323,18 @@ ath_buf_clone(struct ath_softc *sc, cons } struct ath_buf * -ath_getbuf(struct ath_softc *sc) +ath_getbuf(struct ath_softc *sc, ath_buf_type_t btype) { struct ath_buf *bf; ATH_TXBUF_LOCK(sc); - bf = _ath_getbuf_locked(sc); + bf = _ath_getbuf_locked(sc, btype); + /* + * If a mgmt buffer was requested but we're out of those, + * try requesting a normal one. + */ + if (bf == NULL && btype == ATH_BUFTYPE_MGMT) + bf = _ath_getbuf_locked(sc, ATH_BUFTYPE_NORMAL); ATH_TXBUF_UNLOCK(sc); if (bf == NULL) { struct ifnet *ifp = sc->sc_ifp; @@ -2351,7 +2378,7 @@ ath_start(struct ifnet *ifp) /* * Grab a TX buffer and associated resources. */ - bf = ath_getbuf(sc); + bf = ath_getbuf(sc, ATH_BUFTYPE_NORMAL); if (bf == NULL) break; @@ -2857,11 +2884,26 @@ ath_desc_alloc(struct ath_softc *sc) return error; } + error = ath_descdma_setup(sc, &sc->sc_txdma_mgmt, &sc->sc_txbuf_mgmt, + "tx_mgmt", ath_txbuf_mgmt, ATH_TXDESC); + if (error != 0) { + ath_descdma_cleanup(sc, &sc->sc_rxdma, &sc->sc_rxbuf); + ath_descdma_cleanup(sc, &sc->sc_txdma, &sc->sc_txbuf); + return error; + } + + /* + * XXX mark txbuf_mgmt frames with ATH_BUF_MGMT, so the + * flag doesn't have to be set in ath_getbuf_locked(). + */ + error = ath_descdma_setup(sc, &sc->sc_bdma, &sc->sc_bbuf, "beacon", ATH_BCBUF, 1); if (error != 0) { - ath_descdma_cleanup(sc, &sc->sc_txdma, &sc->sc_txbuf); ath_descdma_cleanup(sc, &sc->sc_rxdma, &sc->sc_rxbuf); + ath_descdma_cleanup(sc, &sc->sc_txdma, &sc->sc_txbuf); + ath_descdma_cleanup(sc, &sc->sc_txdma_mgmt, + &sc->sc_txbuf_mgmt); return error; } return 0; @@ -2877,6 +2919,9 @@ ath_desc_free(struct ath_softc *sc) ath_descdma_cleanup(sc, &sc->sc_txdma, &sc->sc_txbuf); if (sc->sc_rxdma.dd_desc_len != 0) ath_descdma_cleanup(sc, &sc->sc_rxdma, &sc->sc_rxbuf); + if (sc->sc_txdma_mgmt.dd_desc_len != 0) + ath_descdma_cleanup(sc, &sc->sc_txdma_mgmt, + &sc->sc_txbuf_mgmt); } static struct ieee80211_node * @@ -3323,12 +3368,14 @@ ath_tx_update_busy(struct ath_softc *sc) * descriptor. */ ATH_TXBUF_LOCK_ASSERT(sc); + last = TAILQ_LAST(&sc->sc_txbuf_mgmt, ath_bufhead_s); + if (last != NULL) + last->bf_flags &= ~ATH_BUF_BUSY; last = TAILQ_LAST(&sc->sc_txbuf, ath_bufhead_s); if (last != NULL) last->bf_flags &= ~ATH_BUF_BUSY; } - /* * Process completed xmit descriptors from the specified queue. * Kick the packet scheduler if needed. This can occur from this @@ -3637,7 +3684,10 @@ ath_returnbuf_tail(struct ath_softc *sc, ATH_TXBUF_LOCK_ASSERT(sc); - TAILQ_INSERT_TAIL(&sc->sc_txbuf, bf, bf_list); + if (bf->bf_flags & ATH_BUF_MGMT) + TAILQ_INSERT_TAIL(&sc->sc_txbuf_mgmt, bf, bf_list); + else + TAILQ_INSERT_TAIL(&sc->sc_txbuf, bf, bf_list); } void @@ -3646,7 +3696,10 @@ ath_returnbuf_head(struct ath_softc *sc, ATH_TXBUF_LOCK_ASSERT(sc); - TAILQ_INSERT_HEAD(&sc->sc_txbuf, bf, bf_list); + if (bf->bf_flags & ATH_BUF_MGMT) + TAILQ_INSERT_HEAD(&sc->sc_txbuf_mgmt, bf, bf_list); + else + TAILQ_INSERT_HEAD(&sc->sc_txbuf, bf, bf_list); } /* @@ -3727,6 +3780,9 @@ ath_tx_draintxq(struct ath_softc *sc, st bf = TAILQ_LAST(&sc->sc_txbuf, ath_bufhead_s); if (bf != NULL) bf->bf_flags &= ~ATH_BUF_BUSY; + bf = TAILQ_LAST(&sc->sc_txbuf_mgmt, ath_bufhead_s); + if (bf != NULL) + bf->bf_flags &= ~ATH_BUF_BUSY; ATH_TXBUF_UNLOCK(sc); for (ix = 0;; ix++) { Modified: head/sys/dev/ath/if_ath_misc.h ============================================================================== --- head/sys/dev/ath/if_ath_misc.h Wed Jun 13 06:46:00 2012 (r236999) +++ head/sys/dev/ath/if_ath_misc.h Wed Jun 13 06:57:55 2012 (r237000) @@ -50,10 +50,13 @@ extern int ath_tx_findrix(const struct ath_softc *sc, uint8_t rate); -extern struct ath_buf * ath_getbuf(struct ath_softc *sc); -extern struct ath_buf * _ath_getbuf_locked(struct ath_softc *sc); +extern struct ath_buf * ath_getbuf(struct ath_softc *sc, + ath_buf_type_t btype); +extern struct ath_buf * _ath_getbuf_locked(struct ath_softc *sc, + ath_buf_type_t btype); extern struct ath_buf * ath_buf_clone(struct ath_softc *sc, const struct ath_buf *bf); +/* XXX change this to NULL the buffer pointer? */ extern void ath_freebuf(struct ath_softc *sc, struct ath_buf *bf); extern void ath_returnbuf_head(struct ath_softc *sc, struct ath_buf *bf); extern void ath_returnbuf_tail(struct ath_softc *sc, struct ath_buf *bf); Modified: head/sys/dev/ath/if_ath_sysctl.c ============================================================================== --- head/sys/dev/ath/if_ath_sysctl.c Wed Jun 13 06:46:00 2012 (r236999) +++ head/sys/dev/ath/if_ath_sysctl.c Wed Jun 13 06:57:55 2012 (r237000) @@ -377,6 +377,19 @@ ath_sysctl_txagg(SYSCTL_HANDLER_ARGS) printf("Total TX buffers: %d; Total TX buffers busy: %d\n", t, i); + i = t = 0; + ATH_TXBUF_LOCK(sc); + TAILQ_FOREACH(bf, &sc->sc_txbuf_mgmt, bf_list) { + if (bf->bf_flags & ATH_BUF_BUSY) { + printf("Busy: %d\n", t); + i++; + } + t++; + } + ATH_TXBUF_UNLOCK(sc); + printf("Total mgmt TX buffers: %d; Total mgmt TX buffers busy: %d\n", + t, i); + return 0; } Modified: head/sys/dev/ath/if_ath_tx.c ============================================================================== --- head/sys/dev/ath/if_ath_tx.c Wed Jun 13 06:46:00 2012 (r236999) +++ head/sys/dev/ath/if_ath_tx.c Wed Jun 13 06:57:55 2012 (r237000) @@ -203,7 +203,8 @@ ath_txfrag_setup(struct ath_softc *sc, a ATH_TXBUF_LOCK(sc); for (m = m0->m_nextpkt; m != NULL; m = m->m_nextpkt) { - bf = _ath_getbuf_locked(sc); + /* XXX non-management? */ + bf = _ath_getbuf_locked(sc, ATH_BUFTYPE_NORMAL); if (bf == NULL) { /* out of buffers, cleanup */ device_printf(sc->sc_dev, "%s: no buffer?\n", __func__); @@ -1878,7 +1879,7 @@ ath_raw_xmit(struct ieee80211_node *ni, /* * Grab a TX buffer and associated resources. */ - bf = ath_getbuf(sc); + bf = ath_getbuf(sc, ATH_BUFTYPE_MGMT); if (bf == NULL) { sc->sc_stats.ast_tx_nobuf++; m_freem(m); Modified: head/sys/dev/ath/if_athvar.h ============================================================================== --- head/sys/dev/ath/if_athvar.h Wed Jun 13 06:46:00 2012 (r236999) +++ head/sys/dev/ath/if_athvar.h Wed Jun 13 06:57:55 2012 (r237000) @@ -44,6 +44,14 @@ #define ATH_TIMEOUT 1000 /* + * There is a separate TX ath_buf pool for management frames. + * This ensures that management frames such as probe responses + * and BAR frames can be transmitted during periods of high + * TX activity. + */ +#define ATH_MGMT_TXBUF 32 + +/* * 802.11n requires more TX and RX buffers to do AMPDU. */ #ifdef ATH_ENABLE_11N @@ -172,6 +180,11 @@ struct ath_node { ((((x)%(mul)) >= ((mul)/2)) ? ((x) + ((mul) - 1)) / (mul) : (x)/(mul)) #define ATH_RSSI(x) ATH_EP_RND(x, HAL_RSSI_EP_MULTIPLIER) +typedef enum { + ATH_BUFTYPE_NORMAL = 0, + ATH_BUFTYPE_MGMT = 1, +} ath_buf_type_t; + struct ath_buf { TAILQ_ENTRY(ath_buf) bf_list; struct ath_buf * bf_next; /* next buffer in the aggregate */ @@ -243,6 +256,7 @@ struct ath_buf { }; typedef TAILQ_HEAD(ath_bufhead_s, ath_buf) ath_bufhead; +#define ATH_BUF_MGMT 0x00000001 /* (tx) desc is a mgmt desc */ #define ATH_BUF_BUSY 0x00000002 /* (tx) desc owned by h/w */ /* @@ -487,6 +501,8 @@ struct ath_softc { struct ath_descdma sc_txdma; /* TX descriptors */ ath_bufhead sc_txbuf; /* transmit buffer */ + struct ath_descdma sc_txdma_mgmt; /* mgmt TX descriptors */ + ath_bufhead sc_txbuf_mgmt; /* mgmt transmit buffer */ struct mtx sc_txbuflock; /* txbuf lock */ char sc_txname[12]; /* e.g. "ath0_buf" */ u_int sc_txqsetup; /* h/w queues setup */ _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: adrian Date: Thu Jun 14 00:51:53 2012 New Revision: 237038 URL: http://svn.freebsd.org/changeset/base/237038 Log: Implement a global (all non-mgmt traffic) TX ath_buf limitation when ath_start() is called. This (defaults to 10 frames) gives for a little headway in the TX ath_buf allocation, so buffer cloning is still possible. This requires a lot omre experimenting and tuning. It also doesn't stop a node/TID from consuming all of the available ath_buf's, especially when the node is going through high packet loss or only talking at a low TX rate. It also doesn't stop a paused TID from taking all of the ath_bufs. I'll look at fixing that up in subsequent commits. PR: kern/168170 Modified: head/sys/dev/ath/if_ath.c head/sys/dev/ath/if_ath_sysctl.c head/sys/dev/ath/if_athvar.h Modified: head/sys/dev/ath/if_ath.c ============================================================================== --- head/sys/dev/ath/if_ath.c Wed Jun 13 22:53:56 2012 (r237037) +++ head/sys/dev/ath/if_ath.c Thu Jun 14 00:51:53 2012 (r237038) @@ -2239,8 +2239,22 @@ _ath_getbuf_locked(struct ath_softc *sc, if (bf != NULL && (bf->bf_flags & ATH_BUF_BUSY) == 0) { if (btype == ATH_BUFTYPE_MGMT) TAILQ_REMOVE(&sc->sc_txbuf_mgmt, bf, bf_list); - else + else { TAILQ_REMOVE(&sc->sc_txbuf, bf, bf_list); + sc->sc_txbuf_cnt--; + + /* + * This shuldn't happen; however just to be + * safe print a warning and fudge the txbuf + * count. + */ + if (sc->sc_txbuf_cnt < 0) { + device_printf(sc->sc_dev, + "%s: sc_txbuf_cnt < 0?\n", + __func__); + sc->sc_txbuf_cnt = 0; + } + } } else bf = NULL; @@ -2367,6 +2381,7 @@ ath_start(struct ifnet *ifp) "%s: sc_inreset_cnt > 0; bailing\n", __func__); ATH_PCU_UNLOCK(sc); IF_LOCK(&ifp->if_snd); + sc->sc_stats.ast_tx_qstop++; ifp->if_drv_flags |= IFF_DRV_OACTIVE; IF_UNLOCK(&ifp->if_snd); return; @@ -2375,6 +2390,17 @@ ath_start(struct ifnet *ifp) ATH_PCU_UNLOCK(sc); for (;;) { + ATH_TXBUF_LOCK(sc); + if (sc->sc_txbuf_cnt <= sc->sc_txq_data_minfree) { + /* XXX increment counter? */ + ATH_TXBUF_UNLOCK(sc); + IF_LOCK(&ifp->if_snd); + ifp->if_drv_flags |= IFF_DRV_OACTIVE; + IF_UNLOCK(&ifp->if_snd); + break; + } + ATH_TXBUF_UNLOCK(sc); + /* * Grab a TX buffer and associated resources. */ @@ -2883,6 +2909,7 @@ ath_desc_alloc(struct ath_softc *sc) ath_descdma_cleanup(sc, &sc->sc_rxdma, &sc->sc_rxbuf); return error; } + sc->sc_txbuf_cnt = ath_txbuf; error = ath_descdma_setup(sc, &sc->sc_txdma_mgmt, &sc->sc_txbuf_mgmt, "tx_mgmt", ath_txbuf_mgmt, ATH_TXDESC); @@ -3686,8 +3713,17 @@ ath_returnbuf_tail(struct ath_softc *sc, if (bf->bf_flags & ATH_BUF_MGMT) TAILQ_INSERT_TAIL(&sc->sc_txbuf_mgmt, bf, bf_list); - else + else { TAILQ_INSERT_TAIL(&sc->sc_txbuf, bf, bf_list); + sc->sc_txbuf_cnt++; + if (sc->sc_txbuf_cnt > ath_txbuf) { + device_printf(sc->sc_dev, + "%s: sc_txbuf_cnt > %d?\n", + __func__, + ath_txbuf); + sc->sc_txbuf_cnt = ath_txbuf; + } + } } void @@ -3698,8 +3734,17 @@ ath_returnbuf_head(struct ath_softc *sc, if (bf->bf_flags & ATH_BUF_MGMT) TAILQ_INSERT_HEAD(&sc->sc_txbuf_mgmt, bf, bf_list); - else + else { TAILQ_INSERT_HEAD(&sc->sc_txbuf, bf, bf_list); + sc->sc_txbuf_cnt++; + if (sc->sc_txbuf_cnt > ATH_TXBUF) { + device_printf(sc->sc_dev, + "%s: sc_txbuf_cnt > %d?\n", + __func__, + ATH_TXBUF); + sc->sc_txbuf_cnt = ATH_TXBUF; + } + } } /* Modified: head/sys/dev/ath/if_ath_sysctl.c ============================================================================== --- head/sys/dev/ath/if_ath_sysctl.c Wed Jun 13 22:53:56 2012 (r237037) +++ head/sys/dev/ath/if_ath_sysctl.c Thu Jun 14 00:51:53 2012 (r237038) @@ -374,8 +374,8 @@ ath_sysctl_txagg(SYSCTL_HANDLER_ARGS) t++; } ATH_TXBUF_UNLOCK(sc); - printf("Total TX buffers: %d; Total TX buffers busy: %d\n", - t, i); + printf("Total TX buffers: %d; Total TX buffers busy: %d (%d)\n", + t, i, sc->sc_txbuf_cnt); i = t = 0; ATH_TXBUF_LOCK(sc); @@ -620,12 +620,11 @@ ath_sysctlattach(struct ath_softc *sc) "tid_hwq_hi", CTLFLAG_RW, &sc->sc_tid_hwq_hi, 0, ""); -#if 0 SYSCTL_ADD_INT(ctx, SYSCTL_CHILDREN(tree), OID_AUTO, "txq_data_minfree", CTLFLAG_RW, &sc->sc_txq_data_minfree, 0, "Minimum free buffers before adding a data frame" " to the TX queue"); -#endif + SYSCTL_ADD_INT(ctx, SYSCTL_CHILDREN(tree), OID_AUTO, "txq_mcastq_maxdepth", CTLFLAG_RW, &sc->sc_txq_mcastq_maxdepth, 0, Modified: head/sys/dev/ath/if_athvar.h ============================================================================== --- head/sys/dev/ath/if_athvar.h Wed Jun 13 22:53:56 2012 (r237037) +++ head/sys/dev/ath/if_athvar.h Thu Jun 14 00:51:53 2012 (r237038) @@ -501,6 +501,7 @@ struct ath_softc { struct ath_descdma sc_txdma; /* TX descriptors */ ath_bufhead sc_txbuf; /* transmit buffer */ + int sc_txbuf_cnt; /* how many buffers avail */ struct ath_descdma sc_txdma_mgmt; /* mgmt TX descriptors */ ath_bufhead sc_txbuf_mgmt; /* mgmt transmit buffer */ struct mtx sc_txbuflock; /* txbuf lock */ _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
batch change: For bugs that match the following - Status Is In progress AND - Untouched since 2018-01-01. AND - Affects Base System OR Documentation DO: Reset to open status. Note: I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
There was a commit referencing this bug, but it's still not closed and has been inactive for some time. Closing as fixed. Please re-open it if the issue hasn't been completely resolved.