Date: Mon, 8 Sep 2014 03:16:28 +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: r271247 - head/sys/dev/iwn Message-ID: <201409080316.s883GS9Y086899@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: adrian Date: Mon Sep 8 03:16:28 2014 New Revision: 271247 URL: http://svnweb.freebsd.org/changeset/base/271247 Log: (more) correctly account TX completion status for A-MPDU session frames. The rules turn out to be: * for non-aggregation session TX queues - it's either sent or not sent. * for aggregation session TX queues - if nframes=1, then the status reflects the completed transmission. * however, for nframes > 1, then this is just a status reflecting what the initial transmission did. The compressed BA (immediate or delayed) may not have yet been received, so the actual frame status is in the compressed BA updates. Whilst here, I fiddled with debugging and formatting a bit. There's also RTS attempts (what the atheros chips call "short retries") which weren't being logged and they aren't yet being used in the rate control statistics updates. For now, at least log them. TODO: * This still isn't 100% correct! So I have to tinker with this some more. (The failures aren't always failures..) * Extend the rate control API in net80211 so it can take both short and long retry counts. Tested: * Intel 5100, STA mode Modified: head/sys/dev/iwn/if_iwn.c Modified: head/sys/dev/iwn/if_iwn.c ============================================================================== --- head/sys/dev/iwn/if_iwn.c Mon Sep 8 03:12:42 2014 (r271246) +++ head/sys/dev/iwn/if_iwn.c Mon Sep 8 03:16:28 2014 (r271247) @@ -3125,6 +3125,7 @@ iwn_rx_compressed_ba(struct iwn_softc *s KASSERT(ni != NULL, ("no node")); KASSERT(m != NULL, ("no mbuf")); + DPRINTF(sc, IWN_DEBUG_XMIT, "%s: freeing m=%p\n", __func__, m); ieee80211_tx_complete(ni, m, 1); txq->queued--; @@ -3151,9 +3152,11 @@ iwn_rx_compressed_ba(struct iwn_softc *s return; /* - * XXX does this correctly process an almost empty bitmap? - * (since it bails out when it sees an empty bitmap, but there - * may be failed bits there..) + * Walk the bitmap and calculate how many successful and failed + * attempts are made. + * + * Yes, the rate control code doesn't know these are A-MPDU + * subframes and that it's okay to fail some of these. */ ni = tap->txa_ni; bitmap = (le64toh(ba->bitmap) >> shift) & wn->agg[tid].bitmap; @@ -3172,7 +3175,8 @@ iwn_rx_compressed_ba(struct iwn_softc *s bitmap >>= 1; } - DPRINTF(sc, IWN_DEBUG_TRACE | IWN_DEBUG_XMIT, "->%s: end; %d ok; %d err\n",__func__, tx_ok, tx_err); + DPRINTF(sc, IWN_DEBUG_TRACE | IWN_DEBUG_XMIT, + "->%s: end; %d ok; %d err\n",__func__, tx_ok, tx_err); } @@ -3423,9 +3427,12 @@ iwn4965_tx_done(struct iwn_softc *sc, st ring = &sc->txq[qid]; DPRINTF(sc, IWN_DEBUG_XMIT, "%s: " - "qid %d idx %d retries %d nkill %d rate %x duration %d status %x\n", - __func__, desc->qid, desc->idx, stat->ackfailcnt, - stat->btkillcnt, stat->rate, le16toh(stat->duration), + "qid %d idx %d RTS retries %d ACK retries %d nkill %d rate %x duration %d status %x\n", + __func__, desc->qid, desc->idx, + stat->rtsfailcnt, + stat->ackfailcnt, + stat->btkillcnt, + stat->rate, le16toh(stat->duration), le32toh(stat->status)); bus_dmamap_sync(ring->data_dmat, data->map, BUS_DMASYNC_POSTREAD); @@ -3450,9 +3457,12 @@ iwn5000_tx_done(struct iwn_softc *sc, st ring = &sc->txq[qid]; DPRINTF(sc, IWN_DEBUG_XMIT, "%s: " - "qid %d idx %d retries %d nkill %d rate %x duration %d status %x\n", - __func__, desc->qid, desc->idx, stat->ackfailcnt, - stat->btkillcnt, stat->rate, le16toh(stat->duration), + "qid %d idx %d RTS retries %d ACK retries %d nkill %d rate %x duration %d status %x\n", + __func__, desc->qid, desc->idx, + stat->rtsfailcnt, + stat->ackfailcnt, + stat->btkillcnt, + stat->rate, le16toh(stat->duration), le32toh(stat->status)); #ifdef notyet @@ -3595,6 +3605,8 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, uint8_t tid; int bit, i, lastidx, *res, seqno, shift, start; + /* XXX TODO: status is le16 field! Grr */ + DPRINTF(sc, IWN_DEBUG_TRACE, "->%s begin\n", __func__); DPRINTF(sc, IWN_DEBUG_XMIT, "%s: nframes=%d, status=0x%08x\n", __func__, @@ -3606,6 +3618,18 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, wn = (void *)tap->txa_ni; ni = tap->txa_ni; + /* + * XXX TODO: ACK and RTS failures would be nice here! + */ + + /* + * A-MPDU single frame status - if we failed to transmit it + * in A-MPDU, then it may be a permanent failure. + * + * XXX TODO: check what the Linux iwlwifi driver does here; + * there's some permanent and temporary failures that may be + * handled differently. + */ if (nframes == 1) { if ((*status & 0xff) != 1 && (*status & 0xff) != 2) { #ifdef NOT_YET @@ -3616,24 +3640,26 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, * notification is pushed up to the rate control * layer. */ - ieee80211_ratectl_tx_complete(ni->ni_vap, ni, - IEEE80211_RATECTL_TX_FAILURE, &nframes, NULL); + ieee80211_ratectl_tx_complete(ni->ni_vap, + ni, + IEEE80211_RATECTL_TX_FAILURE, + &ackfailcnt, + NULL); + } else { + /* + * If nframes=1, then we won't be getting a BA for + * this frame. Ensure that we correctly update the + * rate control code with how many retries were + * needed to send it. + */ + ieee80211_ratectl_tx_complete(ni->ni_vap, + ni, + IEEE80211_RATECTL_TX_SUCCESS, + &ackfailcnt, + NULL); } } - /* - * We succeeded with some frames, so let's update how many - * retries were needed for this frame. - * - * XXX we can't yet pass tx_complete tx_cnt and success_cnt, - * le sigh. - */ - ieee80211_ratectl_tx_complete(ni->ni_vap, - ni, - IEEE80211_RATECTL_TX_SUCCESS, - &ackfailcnt, - NULL); - bitmap = 0; start = idx; for (i = 0; i < nframes; i++) { @@ -3671,6 +3697,7 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, ssn = tap->txa_start & 0xfff; } + /* This is going nframes DWORDS into the descriptor? */ seqno = le32toh(*(status + nframes)) & 0xfff; for (lastidx = (seqno & 0xff); ring->read != lastidx;) { data = &ring->data[ring->read]; @@ -3684,7 +3711,7 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, KASSERT(ni != NULL, ("no node")); KASSERT(m != NULL, ("no mbuf")); - + DPRINTF(sc, IWN_DEBUG_XMIT, "%s: freeing m=%p\n", __func__, m); ieee80211_tx_complete(ni, m, 1); ring->queued--;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201409080316.s883GS9Y086899>