d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Wrap the whole (software) TX path from ifnet dequeue to software queue

Browse Source 
(or direct dispatch) behind the TXQ lock (which, remember, is doubling
as the TID lock too for now.)

This ensures that:

 (a) the sequence number and the CCMP PN allocation is done together;
 (b) overlapping transmit paths don't interleave frames, so we don't
     end up with the original issue that triggered kern/166190.

     Ie, that we don't end up with seqno A, B in thread 1, C, D in
     thread 2, and they being queued to the software queue as "A C D B"
     or similar, leading to the BAW stalls.

This has been tested:

* both STA and AP modes with INVARIANTS and WITNESS;
* TCP and UDP TX;
* both STA->AP and AP->STA.

STA is a Routerstation Pro (single CPU MIPS) and the AP is a dual-core
Centrino.

PR:		kern/166190
This commit is contained in:
Adrian Chadd 2012-06-11 07:44:16 +00:00
parent 4b6db4043f
commit 7561cb5c8b
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=236880
1 changed files with 71 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
sys/dev/ath/if_ath_tx.c
View File

@ -1171,6 +1171,15 @@ ath_tx_normal_setup(struct ath_softc *sc, struct ieee80211_node *ni,
struct ath_node *an;
u_int pri;
/*
* To ensure that both sequence numbers and the CCMP PN handling
* is "correct", make sure that the relevant TID queue is locked.
* Otherwise the CCMP PN and seqno may appear out of order, causing
* re-ordered frames to have out of order CCMP PN's, resulting
* in many, many frame drops.
*/
ATH_TXQ_LOCK_ASSERT(txq);
wh = mtod(m0, struct ieee80211_frame *);
iswep = wh->i_fc[1] & IEEE80211_FC1_WEP;
ismcast = IEEE80211_IS_MULTICAST(wh->i_addr1);
@ -1506,11 +1515,18 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
/* XXX should just bzero the bf_state? */
bf->bf_state.bfs_dobaw = 0;
/*
* Acquire the TXQ lock early, so both the encap and seqno
* are allocated together.
*/
ATH_TXQ_LOCK(txq);
/* A-MPDU TX? Manually set sequence number */
/* Don't do it whilst pending; the net80211 layer still assigns them */
/* XXX do we need locking here? */
/*
* Don't do it whilst pending; the net80211 layer still
* assigns them.
*/
if (is_ampdu_tx) {
ATH_TXQ_LOCK(txq);
/*
* Always call; this function will
* handle making sure that null data frames
@ -1526,7 +1542,6 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
subtype != IEEE80211_FC0_SUBTYPE_QOS_NULL) {
bf->bf_state.bfs_dobaw = 1;
}
ATH_TXQ_UNLOCK(txq);
}
/*
@ -1545,7 +1560,7 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
r = ath_tx_normal_setup(sc, ni, bf, m0, txq);
if (r != 0)
return r;
goto done;
/* At this point m0 could have changed! */
m0 = bf->bf_m;
@ -1570,16 +1585,12 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
if (txq == &avp->av_mcastq) {
DPRINTF(sc, ATH_DEBUG_SW_TX,
"%s: bf=%p: mcastq: TX'ing\n", __func__, bf);
ATH_TXQ_LOCK(txq);
ath_tx_xmit_normal(sc, txq, bf);
ATH_TXQ_UNLOCK(txq);
} else if (type == IEEE80211_FC0_TYPE_CTL &&
subtype == IEEE80211_FC0_SUBTYPE_BAR) {
DPRINTF(sc, ATH_DEBUG_SW_TX,
"%s: BAR: TX'ing direct\n", __func__);
ATH_TXQ_LOCK(txq);
ath_tx_xmit_normal(sc, txq, bf);
ATH_TXQ_UNLOCK(txq);
} else {
/* add to software queue */
DPRINTF(sc, ATH_DEBUG_SW_TX,
@ -1591,10 +1602,10 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
* For now, since there's no software queue,
* direct-dispatch to the hardware.
*/
ATH_TXQ_LOCK(txq);
ath_tx_xmit_normal(sc, txq, bf);
ATH_TXQ_UNLOCK(txq);
#endif
done:
ATH_TXQ_UNLOCK(txq);
return 0;
}
@ -1630,10 +1641,29 @@ ath_tx_raw_start(struct ath_softc *sc, struct ieee80211_node *ni,
/* XXX honor IEEE80211_BPF_DATAPAD */
pktlen = m0->m_pkthdr.len - (hdrlen & 3) + IEEE80211_CRC_LEN;
DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: ismcast=%d\n",
__func__, ismcast);
pri = params->ibp_pri & 3;
/* Override pri if the frame isn't a QoS one */
if (! IEEE80211_QOS_HAS_SEQ(wh))
pri = ath_tx_getac(sc, m0);
/* XXX If it's an ADDBA, override the correct queue */
do_override = ath_tx_action_frame_override_queue(sc, ni, m0, &o_tid);
/* Map ADDBA to the correct priority */
if (do_override) {
#if 0
device_printf(sc->sc_dev,
"%s: overriding tid %d pri %d -> %d\n",
__func__, o_tid, pri, TID_TO_WME_AC(o_tid));
#endif
pri = TID_TO_WME_AC(o_tid);
}
ATH_TXQ_LOCK(sc->sc_ac2q[pri]);
/* Handle encryption twiddling if needed */
if (! ath_tx_tag_crypto(sc, ni,
m0, params->ibp_flags & IEEE80211_BPF_CRYPTO, 0,
@ -1688,11 +1718,6 @@ ath_tx_raw_start(struct ath_softc *sc, struct ieee80211_node *ni,
if (flags & (HAL_TXDESC_RTSENA|HAL_TXDESC_CTSENA))
bf->bf_state.bfs_ctsrate0 = params->ibp_ctsrate;
pri = params->ibp_pri & 3;
/* Override pri if the frame isn't a QoS one */
if (! IEEE80211_QOS_HAS_SEQ(wh))
pri = ath_tx_getac(sc, m0);
/*
* NB: we mark all packets as type PSPOLL so the h/w won't
* set the sequence number, duration, etc.
@ -1774,19 +1799,6 @@ ath_tx_raw_start(struct ath_softc *sc, struct ieee80211_node *ni,
/* NB: no buffered multicast in power save support */
/* XXX If it's an ADDBA, override the correct queue */
do_override = ath_tx_action_frame_override_queue(sc, ni, m0, &o_tid);
/* Map ADDBA to the correct priority */
if (do_override) {
#if 0
device_printf(sc->sc_dev,
"%s: overriding tid %d pri %d -> %d\n",
__func__, o_tid, pri, TID_TO_WME_AC(o_tid));
#endif
pri = TID_TO_WME_AC(o_tid);
}
/*
* If we're overiding the ADDBA destination, dump directly
* into the hardware queue, right after any pending
@ -1796,13 +1808,12 @@ ath_tx_raw_start(struct ath_softc *sc, struct ieee80211_node *ni,
__func__, do_override);
if (do_override) {
ATH_TXQ_LOCK(sc->sc_ac2q[pri]);
ath_tx_xmit_normal(sc, sc->sc_ac2q[pri], bf);
ATH_TXQ_UNLOCK(sc->sc_ac2q[pri]);
} else {
/* Queue to software queue */
ath_tx_swq(sc, ni, sc->sc_ac2q[pri], bf);
}
ATH_TXQ_UNLOCK(sc->sc_ac2q[pri]);
return 0;
}
@ -2032,6 +2043,15 @@ ath_tx_addto_baw(struct ath_softc *sc, struct ath_node *an,
if (bf->bf_state.bfs_isretried)
return;
if (! bf->bf_state.bfs_dobaw) {
device_printf(sc->sc_dev,
"%s: dobaw=0, seqno=%d, window %d:%d\n",
__func__,
SEQNO(bf->bf_state.bfs_seqno),
tap->txa_start,
tap->txa_wnd);
}
tap = ath_tx_get_tx_tid(an, tid->tid);
if (bf->bf_state.bfs_addedbaw)
@ -2042,6 +2062,20 @@ ath_tx_addto_baw(struct ath_softc *sc, struct ath_node *an,
tap->txa_start, tap->txa_wnd, tid->baw_head,
tid->baw_tail);
/*
* Verify that the given sequence number is not outside of the
* BAW. Complain loudly if that's the case.
*/
if (! BAW_WITHIN(tap->txa_start, tap->txa_wnd,
SEQNO(bf->bf_state.bfs_seqno))) {
device_printf(sc->sc_dev,
"%s: bf=%p: outside of BAW?? tid=%d, seqno %d; window %d:%d; "
"baw head=%d tail=%d\n",
__func__, bf, tid->tid, SEQNO(bf->bf_state.bfs_seqno),
tap->txa_start, tap->txa_wnd, tid->baw_head,
tid->baw_tail);
}
/*
* ni->ni_txseqs[] is the currently allocated seqno.
* the txa state contains the current baw start.
@ -2265,6 +2299,8 @@ ath_tx_tid_seqno_assign(struct ath_softc *sc, struct ieee80211_node *ni,
if (! IEEE80211_QOS_HAS_SEQ(wh))
return -1;
ATH_TID_LOCK_ASSERT(sc, &(ATH_NODE(ni)->an_tid[tid]));
/*
* Is it a QOS NULL Data frame? Give it a sequence number from
* the default TID (IEEE80211_NONQOS_TID.)
@ -2276,6 +2312,7 @@ ath_tx_tid_seqno_assign(struct ath_softc *sc, struct ieee80211_node *ni,
*/
subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK;
if (subtype == IEEE80211_FC0_SUBTYPE_QOS_NULL) {
/* XXX no locking for this TID? This is a bit of a problem. */
seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID];
INCR(ni->ni_txseqs[IEEE80211_NONQOS_TID], IEEE80211_SEQ_RANGE);
} else {
@ -2369,6 +2406,8 @@ ath_tx_swq(struct ath_softc *sc, struct ieee80211_node *ni, struct ath_txq *txq,
int pri, tid;
struct mbuf *m0 = bf->bf_m;
ATH_TXQ_LOCK_ASSERT(txq);
/* Fetch the TID - non-QoS frames get assigned to TID 16 */
wh = mtod(m0, struct ieee80211_frame *);
pri = ath_tx_getac(sc, m0);
@ -2391,7 +2430,6 @@ ath_tx_swq(struct ath_softc *sc, struct ieee80211_node *ni, struct ath_txq *txq,
* If the TID is paused or the traffic it outside BAW, software
* queue it.
*/
ATH_TXQ_LOCK(txq);
if (atid->paused) {
/* TID is paused, queue */
DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: paused\n", __func__);
@ -2439,7 +2477,6 @@ ath_tx_swq(struct ath_softc *sc, struct ieee80211_node *ni, struct ath_txq *txq,
ATH_TXQ_INSERT_TAIL(atid, bf, bf_list);
ath_tx_tid_sched(sc, atid);
}
ATH_TXQ_UNLOCK(txq);
}
/*