svn commit: r258823 - head/sys/dev/iwn

[Adrian Chadd]

Author: adrian
Date: Mon Dec  2 03:40:51 2013
New Revision: 258823
URL: http://svnweb.freebsd.org/changeset/base/258823

Log:
  Add some code to double-check whether we're correctly populating the
  TX ring according to what the firmware requires.
  
  The firmware requires A-MPDU sub-frames to be at a very specific ring
  offset - that is, the ring slot offset should be (seqno % 256.)
  
  This holds for every NIC I've tested thus far except the 4965,
  which starts erroring out here shortly before the firmware panics.
  Which is good, it's doing what it's supposed to (read: capture that
  we've screwed up somewhere.)
  
  The specifics about getting this stuff right:
  
  * the initial seqno allocation should match up with the ringid.
  * .. yes, this means we can start at a ring offset that isn't zero.
  * .. because we program the start seqno in the firmware message
    to setup the AC.
  * The initial seqno allocation may be non-zero _and_ frames may be
    being transmitted during a-mpdu negotiation.  I faced similar
    issues on ath(4) and had to software queue frames to that node+TID
    during A-MPDU negotiation.
  * seqno allocation should be in lockstep with ring increments.
  * If we fail to transmit some segment, no, we shouldn't reuse that
    ring slot.  We should just transmit a BAR (which we aren't yet
    doing, sigh) and move onto the next seqno.
  * In theory there shouldn't be any holes in the seqno space when
    we are transmitting frames.
  
  Tested:
  
  * 4965 (throws problems, so yes we have to fix this);
  * 5100 (seems ok);
  * 6200 (seems ok);
  * 2200 (seems ok);
  * 2230 (seems ok).

Modified:
  head/sys/dev/iwn/if_iwn.c

Modified: head/sys/dev/iwn/if_iwn.c
==============================================================================
--- head/sys/dev/iwn/if_iwn.c	Mon Dec  2 03:36:44 2013	(r258822)
+++ head/sys/dev/iwn/if_iwn.c	Mon Dec  2 03:40:51 2013	(r258823)
@@ -4070,6 +4070,7 @@ iwn_tx_data(struct iwn_softc *sc, struct
 	}
 	ac = M_WME_GETAC(m);
 	if (m->m_flags & M_AMPDU_MPDU) {
+		uint16_t seqno;
 		struct ieee80211_tx_ampdu *tap = &ni->ni_tx_ampdu[ac];
 
 		if (!IEEE80211_AMPDU_RUNNING(tap)) {
@@ -4077,9 +4078,27 @@ iwn_tx_data(struct iwn_softc *sc, struct
 			return EINVAL;
 		}
 
+		/*
+		 * Queue this frame to the hardware ring that we've
+		 * negotiated AMPDU TX on.
+		 *
+		 * Note that the sequence number must match the TX slot
+		 * being used!
+		 */
 		ac = *(int *)tap->txa_private;
+		seqno = ni->ni_txseqs[tid];
 		*(uint16_t *)wh->i_seq =
-		    htole16(ni->ni_txseqs[tid] << IEEE80211_SEQ_SEQ_SHIFT);
+		    htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT);
+		ring = &sc->txq[ac];
+		if ((seqno % 256) != ring->cur) {
+			device_printf(sc->sc_dev,
+			    "%s: m=%p: seqno (%d) (%d) != ring index (%d) !\n",
+			    __func__,
+			    m,
+			    seqno,
+			    seqno % 256,
+			    ring->cur);
+		}
 		ni->ni_txseqs[tid]++;
 	}
 	ring = &sc->txq[ac];