From owner-svn-src-all@FreeBSD.ORG Fri Sep 7 00:24:27 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id C3150106566B; Fri, 7 Sep 2012 00:24:27 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 95AF68FC08; Fri, 7 Sep 2012 00:24:27 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q870ORlL008442; Fri, 7 Sep 2012 00:24:27 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q870ORgZ008440; Fri, 7 Sep 2012 00:24:27 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201209070024.q870ORgZ008440@svn.freebsd.org> From: Adrian Chadd Date: Fri, 7 Sep 2012 00:24:27 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r240180 - head/sys/dev/ath X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Sep 2012 00:24:27 -0000 Author: adrian Date: Fri Sep 7 00:24:27 2012 New Revision: 240180 URL: http://svn.freebsd.org/changeset/base/240180 Log: Ensure that single-frame aggregate session frames are retransmitted with the correct configuration. Occasionally an aggregate TX would fail and the first frame would be retransmitted as a non-AMPDU frame. Since bfs_aggr=1 and bfs_nframes > 1 (from the previous AMPDU attempt), the aggr completion function would be called and be very confused about what's going on. Noticed by: Kim PR: kern/171394 Modified: head/sys/dev/ath/if_ath_tx.c Modified: head/sys/dev/ath/if_ath_tx.c ============================================================================== --- head/sys/dev/ath/if_ath_tx.c Fri Sep 7 00:20:46 2012 (r240179) +++ head/sys/dev/ath/if_ath_tx.c Fri Sep 7 00:24:27 2012 (r240180) @@ -2528,6 +2528,25 @@ ath_tx_xmit_aggr(struct ath_softc *sc, s return; } + /* + * This is a temporary check and should be removed once + * all the relevant code paths have been fixed. + * + * During aggregate retries, it's possible that the head + * frame will fail (which has the bfs_aggr and bfs_nframes + * fields set for said aggregate) and will be retried as + * a single frame. In this instance, the values should + * be reset or the completion code will get upset with you. + */ + if (bf->bf_state.bfs_aggr != 0 || bf->bf_state.bfs_nframes > 1) { + device_printf(sc->sc_dev, "%s: bfs_aggr=%d, bfs_nframes=%d\n", + __func__, + bf->bf_state.bfs_aggr, + bf->bf_state.bfs_nframes); + bf->bf_state.bfs_aggr = 0; + bf->bf_state.bfs_nframes = 1; + } + /* Direct dispatch to hardware */ ath_tx_do_ratelookup(sc, bf); ath_tx_calc_duration(sc, bf); @@ -2624,6 +2643,16 @@ ath_tx_swq(struct ath_softc *sc, struct if (txq->axq_depth < sc->sc_hwq_limit) { bf = TAILQ_FIRST(&atid->axq_q); ATH_TXQ_REMOVE(atid, bf, bf_list); + + /* + * Ensure it's definitely treated as a non-AMPDU + * frame - this information may have been left + * over from a previous attempt. + */ + bf->bf_state.bfs_aggr = 0; + bf->bf_state.bfs_nframes = 1; + + /* Queue to the hardware */ ath_tx_xmit_aggr(sc, an, txq, bf); DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: xmit_aggr\n", @@ -4018,7 +4047,23 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft "%s: non-baw packet\n", __func__); ATH_TXQ_REMOVE(tid, bf, bf_list); + + if (bf->bf_state.bfs_nframes > 1) + device_printf(sc->sc_dev, + "%s: aggr=%d, nframes=%d\n", + __func__, + bf->bf_state.bfs_aggr, + bf->bf_state.bfs_nframes); + + /* + * This shouldn't happen - such frames shouldn't + * ever have been queued as an aggregate in the + * first place. However, make sure the fields + * are correctly setup just to be totally sure. + */ bf->bf_state.bfs_aggr = 0; + bf->bf_state.bfs_nframes = 1; + ath_tx_do_ratelookup(sc, bf); ath_tx_calc_duration(sc, bf); ath_tx_calc_protection(sc, bf);