Date: Thu, 18 Aug 2011 09:18:39 +0000 (UTC) From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r224965 - user/adrian/if_ath_tx/sys/dev/ath Message-ID: <201108180918.p7I9IdcD047559@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: adrian Date: Thu Aug 18 09:18:39 2011 New Revision: 224965 URL: http://svn.freebsd.org/changeset/base/224965 Log: Flip on TX aggregate formation. No, it doesn't quite work. * Push ath_tx_setds() and ath_tx_chaindesc() back to where I had it - ie, setup just before the buffer was queued to the hardware. * Use the 11n aggregate form methods from Sam's HAL, rather than the added methods from linux/reference code. I'm still unsure which to use for now, but those methods do seem to work. the chaindesc method knows about sub-frames _and_ segments of a sub-frame; the Linux/reference code seem to assume that a TX'ed aggregate sub-frame will only take up one skb. (Or there's something else subtle going on that I haven't quite understood.) * Modify ath_tx_form_aggr() to only form the aggregate buffer list; it doesn't link the descriptors together or do any descriptor setup. * Write ath_tx_setds_11n() which sets up an aggregate descriptor set. This sets up the first/last descriptors as appropriate as well as chaining the subframes together. * Teach the aggregate scheduler function to use ath_tx_form_aggr(), then based on the result of that process, either send a single-packet (which is known to be inside the BAW), or fire off an aggregate frame list to the hardware. * Add some debugging to try and figure out what's going on. As it stands, the TX path halts very quickly. It looks like some completion handling is incorrect, as the TX stops because the BAW tracking becomes out of whack. If I disable this (ie, treat all packets as ACKed and not retry anything), TX is slow (on the order of a few megabits/sec) but the hardware doesn't lock up; so it's likely the descriptor fiddling is functioning mostly correctly. Unfortunately, I don't yet have a clear picture how I'm supposed to setup multi-segment frames inside an aggregate. As mentioned above, Linux/reference code both seem to assume an aggregate subframe fits in a whole skb. (Yes, the receive-side does indeed indicate the frames received were A-MPDU frames. No, I haven't even begun to dig deeply into the aggregate setup to see if I've done everything completely correctly. No, don't expect to use this in STA/Hostap mode just yet.) Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_debug.h user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx_ht.c Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_debug.h ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath_debug.h Thu Aug 18 08:58:10 2011 (r224964) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_debug.h Thu Aug 18 09:18:39 2011 (r224965) @@ -60,6 +60,7 @@ enum { ATH_DEBUG_SW_TX = 0x04000000, /* per-packet software TX */ ATH_DEBUG_SW_TX_BAW = 0x08000000, /* BAW handling */ ATH_DEBUG_SW_TX_CTRL = 0x10000000, /* queue control */ + ATH_DEBUG_SW_TX_AGGR = 0x20000000, /* aggregate TX */ ATH_DEBUG_FATAL = 0x80000000, /* fatal errors */ ATH_DEBUG_ANY = 0xffffffff }; Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Thu Aug 18 08:58:10 2011 (r224964) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Thu Aug 18 09:18:39 2011 (r224965) @@ -290,6 +290,9 @@ ath_tx_dmasetup(struct ath_softc *sc, st return 0; } +/* + * Chain together segments+descriptors for a non-11n frame. + */ static void ath_tx_chaindesclist(struct ath_softc *sc, struct ath_buf *bf) { @@ -321,6 +324,127 @@ ath_tx_chaindesclist(struct ath_softc *s } } +/* + * Fill in the descriptor list for a aggregate subframe. + * + * The subframe is returned with the ds_link field in the last subframe + * pointing to 0. + */ +static void +ath_tx_chaindesclist_subframe(struct ath_softc *sc, struct ath_buf *bf) +{ + struct ath_hal *ah = sc->sc_ah; + struct ath_desc *ds, *ds0; + int i; + + ds0 = ds = bf->bf_desc; + + /* + * There's no need to call ath_hal_setupfirsttxdesc here; + * That's only going to occur for the first frame in an aggregate. + */ + for (i = 0; i < bf->bf_nseg; i++, ds++) { + ds->ds_data = bf->bf_segs[i].ds_addr; + if (i == bf->bf_nseg - 1) + ds->ds_link = 0; + else + ds->ds_link = bf->bf_daddr + sizeof(*ds) * (i + 1); + + /* + * This performs the setup for an aggregate frame. + * This includes enabling the aggregate flags if needed. + */ + ath_hal_chaintxdesc(ah, ds, + bf->bf_state.bfs_pktlen, + bf->bf_state.bfs_hdrlen, + HAL_PKT_TYPE_AMPDU, /* forces aggregate bits to be set */ + bf->bf_state.bfs_keyix, + 0, /* cipher, calculated from keyix */ + bf->bf_state.bfs_ndelim, + bf->bf_segs[i].ds_len, /* segment length */ + i == 0, /* first segment */ + i == bf->bf_nseg - 1 /* last segment */ + ); + + DPRINTF(sc, ATH_DEBUG_XMIT, + "%s: %d: %08x %08x %08x %08x %08x %08x\n", + __func__, i, ds->ds_link, ds->ds_data, + ds->ds_ctl0, ds->ds_ctl1, ds->ds_hw[0], ds->ds_hw[1]); + bf->bf_lastds = ds; + } +} + +/* + * Setup segments+descriptors for an 11n aggregate. + * bf_first is the first buffer in the aggregate. + * The descriptor list must already been linked together using + * bf->bf_next. + */ +static void +ath_tx_setds_11n(struct ath_softc *sc, struct ath_buf *bf_first) +{ + struct ath_buf *bf, *bf_prev = NULL; + + DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR, "%s: nframes=%d, al=%d\n", + __func__, bf_first->bf_state.bfs_nframes, + bf_first->bf_state.bfs_al); + + /* + * Setup first descriptor of first frame. + * The sub-frame specific stuff is done + * later. + */ + ath_hal_setupfirsttxdesc(sc->sc_ah, + bf_first->bf_desc, + bf_first->bf_state.bfs_al, + bf_first->bf_state.bfs_flags, + bf_first->bf_state.bfs_txpower, + bf_first->bf_state.bfs_txrate0, + bf_first->bf_state.bfs_try0, + bf_first->bf_state.bfs_txantenna, + bf_first->bf_state.bfs_ctsrate, + bf_first->bf_state.bfs_ctsduration); + + /* + * Setup all descriptors of all subframes. + */ + bf = bf_first; + while (bf != NULL) { + DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR, + "%s: bf=%p, nseg=%d, pktlen=%d\n", + __func__, bf, bf->bf_nseg, bf->bf_state.bfs_pktlen); + + /* Sub-frame setup */ + ath_tx_chaindesclist_subframe(sc, bf); + + /* + * Link the last descriptor of the previous frame + * to the beginning descriptor of this frame. + */ + if (bf_prev != NULL) + bf_prev->bf_lastds->ds_link = bf->bf_daddr; + + /* Save a copy so we can link the next descriptor in */ + bf_prev = bf; + bf = bf->bf_next; + } + + /* + * Setup the last descriptor in the list. + * bf_prev points to the last; bf is NULL here. + */ + ath_hal_setuplasttxdesc(sc->sc_ah, bf_prev->bf_desc, bf_first->bf_desc); + + /* + * Set the first descriptor bf_lastds field to point to + * the last descriptor in the last subframe, that's where + * the status update will occur. + */ + bf_first->bf_lastds = bf_prev->bf_lastds; + + DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR, "%s: end\n", __func__); +} + static void ath_tx_handoff_mcast(struct ath_softc *sc, struct ath_txq *txq, struct ath_buf *bf) @@ -342,7 +466,7 @@ ath_tx_handoff_mcast(struct ath_softc *s *txq->axq_link = bf->bf_daddr; } ATH_TXQ_INSERT_TAIL(txq, bf, bf_list); - txq->axq_link = &bf->bf_desc[bf->bf_nseg - 1].ds_link; + txq->axq_link = &bf->bf_lastds->ds_link; } @@ -442,7 +566,7 @@ ath_tx_handoff_hw(struct ath_softc *sc, (caddr_t)bf->bf_daddr, bf->bf_desc, txq->axq_depth); } #endif /* IEEE80211_SUPPORT_TDMA */ - txq->axq_link = &bf->bf_desc[bf->bf_nseg - 1].ds_link; + txq->axq_link = &bf->bf_lastds->ds_link; ath_hal_txstart(ah, txq->axq_qnum); } } @@ -606,6 +730,10 @@ ath_tx_setds(struct ath_softc *sc, struc , bf->bf_state.bfs_ctsrate /* rts/cts rate */ , bf->bf_state.bfs_ctsduration /* rts/cts duration */ ); + + /* + * This will be overriden when the descriptor chain is written. + */ bf->bf_lastds = ds; /* XXX TODO: Setup descriptor chain */ @@ -1792,16 +1920,6 @@ ath_tx_swq(struct ath_softc *sc, struct bf->bf_state.bfs_aggr = 0; bf->bf_state.bfs_aggrburst = 0; - /* - * Program first and chain the descriptors together. - * - * These fields (along with the DMA map setup) are needed - * by the aggregate forming code, which only overrides - * the rate control setup and the aggregation fields. - */ - ath_tx_setds(sc, bf); - ath_tx_chaindesclist(sc, bf); - /* Queue frame to the tail of the software queue */ ATH_TXQ_LOCK(atid); ATH_TXQ_INSERT_TAIL(atid, bf, bf_list); @@ -2283,6 +2401,7 @@ ath_tx_retry_subframe(struct ath_softc * } ath_tx_set_retry(sc, bf); + bf->bf_next = NULL; /* Just to make sure */ STAILQ_INSERT_TAIL(bf_q, bf, bf_list); return 0; @@ -2317,6 +2436,7 @@ ath_tx_comp_aggr_error(struct ath_softc /* Update rate control module about aggregation */ /* XXX todo */ +#if 0 /* * send bar if we dropped any frames */ @@ -2335,6 +2455,7 @@ ath_tx_comp_aggr_error(struct ath_softc __func__, tid->tid); } } +#endif /* Prepend all frames to the beginning of the queue */ ATH_TXQ_LOCK(tid); @@ -2402,10 +2523,12 @@ ath_tx_aggr_comp_aggr(struct ath_softc * int ba_index; int drops = 0; + DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR, "%s: called\n", __func__); + /* * Punt cleanup to the relevant function, not our problem now */ - if (atid->cleanup_inprogress) { + if (0 && atid->cleanup_inprogress) { ath_tx_comp_cleanup_aggr(sc, bf_first); return; } @@ -2413,7 +2536,7 @@ ath_tx_aggr_comp_aggr(struct ath_softc * /* * handle errors first */ - if (ts->ts_status & HAL_TXERR_XRETRY) { + if (0 && ts->ts_status & HAL_TXERR_XRETRY) { ath_tx_comp_aggr_error(sc, bf_first, atid); return; } @@ -2432,6 +2555,10 @@ ath_tx_aggr_comp_aggr(struct ath_softc * ba[0] = ts->ts_ba_low; ba[1] = ts->ts_ba_high; + device_printf(sc->sc_dev, + "%s: txa_start=%d, tx_ok=%d, isaggr=%d, seq_st=%d, hasba=%d, ba=%.8x, %.8x\n", + __func__, tap->txa_start, tx_ok, isaggr, seq_st, hasba, ba[0], ba[1]); + /* Occasionally, the MAC sends a tx status for the wrong TID. */ if (tid != ts->ts_tid) { device_printf(sc->sc_dev, "%s: tid %d != hw tid %d\n", @@ -2449,10 +2576,17 @@ ath_tx_aggr_comp_aggr(struct ath_softc * bf = bf_first; while (bf) { + DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR, "%s: checking bf=%p seqno=%d\n", + __func__, bf, SEQNO(bf->bf_state.bfs_seqno)); + ba_index = ATH_BA_INDEX(seq_st, SEQNO(bf->bf_state.bfs_seqno)); bf_next = bf->bf_next; + /* + * For now, ACK all packets + */ if (tx_ok && ATH_BA_ISSET(ba, ba_index)) { + //if (1) { ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno)); ath_tx_default_comp(sc, bf, 0); @@ -2465,6 +2599,7 @@ ath_tx_aggr_comp_aggr(struct ath_softc * /* update rate control module about aggregate status */ /* XXX TODO */ +#if 0 /* * send bar if we dropped any frames */ @@ -2483,6 +2618,7 @@ ath_tx_aggr_comp_aggr(struct ath_softc * __func__, tid); } } +#endif /* Prepend all frames to the beginning of the queue */ ATH_TXQ_LOCK(atid); @@ -2491,6 +2627,9 @@ ath_tx_aggr_comp_aggr(struct ath_softc * STAILQ_REMOVE_HEAD(&bf_q, bf_list); } ATH_TXQ_UNLOCK(atid); + + device_printf(sc->sc_dev, "%s: finished; txa_start now %d\n", + __func__, tap->txa_start); } /* @@ -2566,6 +2705,8 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft struct ath_tid *atid = &an->an_tid[tid]; struct ieee80211_tx_ampdu *tap; struct ieee80211_node *ni = &an->an_node; + ATH_AGGR_STATUS status; + ath_bufhead bf_q; DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: tid=%d\n", __func__, tid); @@ -2576,6 +2717,8 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft __func__); for (;;) { + status = ATH_AGGR_DONE; + ATH_TXQ_LOCK(atid); /* @@ -2589,70 +2732,119 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft if (atid->paused) break; - bf = STAILQ_FIRST(&atid->axq_q); + bf = STAILQ_FIRST(&atid->axq_q); if (bf == NULL) { ATH_TXQ_UNLOCK(atid); break; } - DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: bf=%p: tid=%d\n", - __func__, bf, bf->bf_state.bfs_tid); - if (bf->bf_state.bfs_tid != tid) - device_printf(sc->sc_dev, "%s: TID: tid=%d, ac=%d, bf tid=%d\n", - __func__, tid, atid->ac, bf->bf_state.bfs_tid); - if (sc->sc_ac2q[TID_TO_WME_AC(tid)] != bf->bf_state.bfs_txq) - device_printf(sc->sc_dev, "%s: TXQ: tid=%d, ac=%d, bf tid=%d\n", - __func__, tid, atid->ac, bf->bf_state.bfs_tid); - - /* Check if seqno is outside of BAW, if so don't queue it */ - if (bf->bf_state.bfs_dobaw && - (! BAW_WITHIN(tap->txa_start, tap->txa_wnd, - SEQNO(bf->bf_state.bfs_seqno)))) { - DPRINTF(sc, ATH_DEBUG_SW_TX_BAW, - "%s: seq %d outside of %d/%d; waiting\n", - __func__, SEQNO(bf->bf_state.bfs_seqno), - tap->txa_start, tap->txa_wnd); + /* + * If the packet doesn't fall within the BAW (eg a NULL + * data frame), schedule it directly; continue. + */ + if (! bf->bf_state.bfs_dobaw) { + DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: non-baw packet\n", + __func__); + ATH_TXQ_REMOVE_HEAD(atid, bf_list); ATH_TXQ_UNLOCK(atid); - break; + bf->bf_state.bfs_aggr = 0; + /* Ensure the last descriptor link is 0 */ + bf->bf_lastds->ds_link = 0; + ath_tx_setds(sc, bf); + ath_tx_chaindesclist(sc, bf); + ath_hal_clr11n_aggr(sc->sc_ah, bf->bf_desc); + ath_tx_set_ratectrl(sc, ni, bf); + + /* Queue the packet; continue */ + goto queuepkt; } + ATH_TXQ_UNLOCK(atid); + + /* Don't lock the TID - ath_tx_form_aggr will lock as needed */ + STAILQ_INIT(&bf_q); + status = ath_tx_form_aggr(sc, an, atid, &bf_q); + + DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, + "%s: ath_tx_form_aggr() status=%d\n", __func__, status); /* - * XXX If the seqno is out of BAW, then we should pause this TID - * XXX until a completion for this TID allows the BAW to be advanced. - * XXX Otherwise it's possible that we'll simply keep getting called - * XXX for this node/TID until some TX completion has occured - * XXX and progress can be made. + * No frames to be picked up - out of BAW */ + if (STAILQ_EMPTY(&bf_q)) + break; - /* We've committed to sending it, so remove it from the list */ - ATH_TXQ_REMOVE_HEAD(atid, bf_list); - ATH_TXQ_UNLOCK(atid); + /* + * This assumes that the descriptor list in the ath_bufhead + * are already linked together via bf_next pointers. + */ + bf = STAILQ_FIRST(&bf_q); - /* Don't add packets to the BAW that don't contribute to it */ - if (bf->bf_state.bfs_dobaw) - ath_tx_addto_baw(sc, an, atid, bf); + /* + * If it's the only frame send as non-aggregate + * assume that ath_tx_form_aggr() has checked + * whether it's in the BAW and added it appropriately. + */ + if (bf->bf_state.bfs_nframes == 1) { + DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, + "%s: single-frame aggregate\n", __func__); + bf->bf_state.bfs_aggr = 0; + /* Ensure the last descriptor link is 0 */ + bf->bf_lastds->ds_link = 0; + ath_tx_setds(sc, bf); + ath_tx_chaindesclist(sc, bf); + ath_hal_clr11n_aggr(sc->sc_ah, bf->bf_desc); + ath_tx_set_ratectrl(sc, ni, bf); + } else { + DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, + "%s: multi-frame aggregate: %d frames, length %d\n", + __func__, bf->bf_state.bfs_nframes, + bf->bf_state.bfs_al); + bf->bf_state.bfs_aggr = 1; + + /* Set rate 1, 2, 3 to 0 for aggregate frames */ + bf->bf_state.bfs_rc[1].rix = + bf->bf_state.bfs_rc[2].rix = + bf->bf_state.bfs_rc[3].rix = 0; + bf->bf_state.bfs_rc[1].tries = + bf->bf_state.bfs_rc[2].tries = + bf->bf_state.bfs_rc[3].tries = 0; - txq = bf->bf_state.bfs_txq; + /* + * Setup the relevant descriptor fields + * for aggregation. The first descriptor + * already points to the rest in the chain. + */ + ath_tx_setds_11n(sc, bf); - /* Sanity check! */ - if (tid != bf->bf_state.bfs_tid) { - device_printf(sc->sc_dev, "%s: bfs_tid %d !=" - " tid %d\n", - __func__, bf->bf_state.bfs_tid, tid); + /* + * setup first desc with rate and aggr info + */ + ath_tx_set_ratectrl(sc, ni, bf); } + queuepkt: + txq = bf->bf_state.bfs_txq; - /* Set completion handler */ + /* Set completion handler, multi-frame aggregate or not */ bf->bf_comp = ath_tx_aggr_comp; - if (bf->bf_state.bfs_tid == IEEE80211_NONQOS_TID) - device_printf(sc->sc_dev, "%s: TID=16?\n", __func__); - /* Program rate control */ - ath_tx_set_ratectrl(sc, ni, bf); + if (bf->bf_state.bfs_tid == IEEE80211_NONQOS_TID) + device_printf(sc->sc_dev, "%s: TID=16?\n", __func__); - /* Punt to hardware or software txq */ + /* Punt to txq */ ATH_TXQ_LOCK(txq); ath_tx_handoff(sc, txq, bf); ATH_TXQ_UNLOCK(txq); + + /* + * Break out if ath_tx_form_aggr() indicated + * there can't be any further progress (eg BAW is full.) + * Checking for an empty txq is done above. + * + * Later on, enforce ATH_AGGR_MIN_QDEPTH to try and + * keep aggregation flowing. + */ + if (status == ATH_AGGR_BAW_CLOSED) + break; } } @@ -2708,7 +2900,9 @@ ath_tx_tid_hw_queue_norm(struct ath_soft /* Normal completion handler */ bf->bf_comp = ath_tx_normal_comp; - /* Program rate control*/ + /* Program descriptors + rate control */ + ath_tx_setds(sc, bf); + ath_tx_chaindesclist(sc, bf); ath_tx_set_ratectrl(sc, ni, bf); /* Punt to hardware or software txq */ Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx_ht.c ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx_ht.c Thu Aug 18 08:58:10 2011 (r224964) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx_ht.c Thu Aug 18 09:18:39 2011 (r224965) @@ -350,7 +350,7 @@ ath_tx_form_aggr(struct ath_softc *sc, s h_baw = tap->txa_wnd / 2; /* Calculate aggregation limit */ - aggr_limit = 8192; /* XXX just for now, for testing */ + aggr_limit = 49152; /* XXX just for now, for testing */ for (;;) { ATH_TXQ_LOCK(tid); @@ -363,6 +363,9 @@ ath_tx_form_aggr(struct ath_softc *sc, s break; } + /* Set this early just so things don't get confused */ + bf->bf_next = NULL; + /* * Don't unlock the tid lock until we're sure we are going * to queue this frame. @@ -455,18 +458,12 @@ ath_tx_form_aggr(struct ath_softc *sc, s bpad = PADBYTES(al_delta) + (bf->bf_state.bfs_ndelim << 2); /* - * link current buffer to the aggregate + * Chain the buffers together */ - if (bf_prev) { + if (bf_prev) bf_prev->bf_next = bf; - bf_prev->bf_desc->ds_link = bf->bf_daddr; - } bf_prev = bf; - /* Set aggregate flags */ - ath_hal_set11naggrmiddle(sc->sc_ah, bf->bf_desc, - bf->bf_state.bfs_ndelim); - #if 0 /* * terminate aggregation on a small packet boundary
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108180918.p7I9IdcD047559>