From owner-svn-src-head@FreeBSD.ORG Mon Sep 8 03:16:28 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D64634A8; Mon, 8 Sep 2014 03:16:28 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C13981078; Mon, 8 Sep 2014 03:16:28 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id s883GSHR086901; Mon, 8 Sep 2014 03:16:28 GMT (envelope-from adrian@FreeBSD.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id s883GS9Y086899; Mon, 8 Sep 2014 03:16:28 GMT (envelope-from adrian@FreeBSD.org) Message-Id: <201409080316.s883GS9Y086899@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: adrian set sender to adrian@FreeBSD.org using -f From: Adrian Chadd Date: Mon, 8 Sep 2014 03:16:28 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r271247 - head/sys/dev/iwn X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Sep 2014 03:16:29 -0000 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--;