Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 28 Aug 2014 07:44:59 +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: r270742 - head/sys/dev/iwn
Message-ID:  <201408280744.s7S7ixh5088562@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: adrian
Date: Thu Aug 28 07:44:59 2014
New Revision: 270742
URL: http://svnweb.freebsd.org/changeset/base/270742

Log:
  Inform the rate control code if a single frame AMPDU transmission succeeds
  but has some retries.
  
  Without this, single frame transmission in AMPDU will always look like
  it succeeded fine, and thus AMRR will think it's totally fine to just
  keep upping the rate upwards.
  
  Now, this is still not quite right!  For multi-frame aggregates the
  completion happens in two parts - the TX done and the BA received.
  The driver is currently double accounting those a little - there's no
  way to say to the rate control code "I completed X frames, Y worked fine,
  there were Z retries." And it's a bit odd with iwn, as the firmware
  retransmits frames for us so we don't get to see how many retransmits
  happened; only that it took longer than normal.  I may have to extend
  the rate control API to properly track that.
  
  So this may keep the rate lower than it should be, but that's better
  than keeping it higher than it should be.
  
  Tested:
  
  * 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	Thu Aug 28 06:16:36 2014	(r270741)
+++ head/sys/dev/iwn/if_iwn.c	Thu Aug 28 07:44:59 2014	(r270742)
@@ -213,7 +213,7 @@ static void	iwn5000_tx_done(struct iwn_s
 		    struct iwn_rx_data *);
 static void	iwn_tx_done(struct iwn_softc *, struct iwn_rx_desc *, int,
 		    uint8_t);
-static void	iwn_ampdu_tx_done(struct iwn_softc *, int, int, int, void *);
+static void	iwn_ampdu_tx_done(struct iwn_softc *, int, int, int, int, void *);
 static void	iwn_cmd_done(struct iwn_softc *, struct iwn_rx_desc *);
 static void	iwn_notif_intr(struct iwn_softc *);
 static void	iwn_wakeup_intr(struct iwn_softc *);
@@ -3150,6 +3150,11 @@ iwn_rx_compressed_ba(struct iwn_softc *s
 	if (wn->agg[tid].nframes > (64 - shift))
 		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..)
+	 */
 	ni = tap->txa_ni;
 	bitmap = (le64toh(ba->bitmap) >> shift) & wn->agg[tid].bitmap;
 	for (i = 0; bitmap; i++) {
@@ -3426,7 +3431,7 @@ iwn4965_tx_done(struct iwn_softc *sc, st
 	bus_dmamap_sync(ring->data_dmat, data->map, BUS_DMASYNC_POSTREAD);
 	if (qid >= sc->firstaggqueue) {
 		iwn_ampdu_tx_done(sc, qid, desc->idx, stat->nframes,
-		    &stat->status);
+		    stat->ackfailcnt, &stat->status);
 	} else {
 		iwn_tx_done(sc, desc, stat->ackfailcnt,
 		    le32toh(stat->status) & 0xff);
@@ -3458,7 +3463,7 @@ iwn5000_tx_done(struct iwn_softc *sc, st
 	bus_dmamap_sync(ring->data_dmat, data->map, BUS_DMASYNC_POSTREAD);
 	if (qid >= sc->firstaggqueue) {
 		iwn_ampdu_tx_done(sc, qid, desc->idx, stat->nframes,
-		    &stat->status);
+		    stat->ackfailcnt, &stat->status);
 	} else {
 		iwn_tx_done(sc, desc, stat->ackfailcnt,
 		    le16toh(stat->status) & 0xff);
@@ -3573,7 +3578,7 @@ iwn_cmd_done(struct iwn_softc *sc, struc
 
 static void
 iwn_ampdu_tx_done(struct iwn_softc *sc, int qid, int idx, int nframes,
-    void *stat)
+    int ackfailcnt, void *stat)
 {
 	struct iwn_ops *ops = &sc->ops;
 	struct ifnet *ifp = sc->sc_ifp;
@@ -3591,6 +3596,15 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, 
 	int bit, i, lastidx, *res, seqno, shift, start;
 
 	DPRINTF(sc, IWN_DEBUG_TRACE, "->%s begin\n", __func__);
+	DPRINTF(sc, IWN_DEBUG_XMIT, "%s: nframes=%d, status=0x%08x\n",
+	    __func__,
+	    nframes,
+	    *status);
+
+	tap = sc->qid2tap[qid];
+	tid = tap->txa_tid;
+	wn = (void *)tap->txa_ni;
+	ni = tap->txa_ni;
 
 	if (nframes == 1) {
 		if ((*status & 0xff) != 1 && (*status & 0xff) != 2) {
@@ -3602,15 +3616,24 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, 
 			 * notification is pushed up to the rate control
 			 * layer.
 			 */
-			tap = sc->qid2tap[qid];
-			tid = tap->txa_tid;
-			wn = (void *)tap->txa_ni;
-			ni = tap->txa_ni;
 			ieee80211_ratectl_tx_complete(ni->ni_vap, ni,
 			    IEEE80211_RATECTL_TX_FAILURE, &nframes, 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++) {

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201408280744.s7S7ixh5088562>