Skip site navigation (1)Skip section navigation (2)
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>