From owner-svn-src-head@FreeBSD.ORG Mon Dec 2 03:40:52 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2D1C4DCC; Mon, 2 Dec 2013 03:40:52 +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)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 005F96D9B; Mon, 2 Dec 2013 03:40:52 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id rB23epT5092011; Mon, 2 Dec 2013 03:40:51 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.7/8.14.5/Submit) id rB23ep3t092010; Mon, 2 Dec 2013 03:40:51 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201312020340.rB23ep3t092010@svn.freebsd.org> From: Adrian Chadd Date: Mon, 2 Dec 2013 03:40:51 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r258823 - 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.16 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, 02 Dec 2013 03:40:52 -0000 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];