Date: Thu, 15 Nov 2012 03:00:49 +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: r243047 - head/sys/dev/ath Message-ID: <201211150300.qAF30nmY001743@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: adrian Date: Thu Nov 15 03:00:49 2012 New Revision: 243047 URL: http://svnweb.freebsd.org/changeset/base/243047 Log: Make sure the final descriptor in an aggregate has rate control information. This was broken by me when merging the 802.11n aggregate descriptor chain setup with the default descriptor chain setup, in preparation for supporting AR9380 NICs. The corner case here is quite specific - if you queue an aggregate frame with >1 frames in it, and the last subframe has only one descriptor making it up, then that descriptor won't have the rate control information copied into it. Look at what happens inside ar5416FillTxDesc() if both firstSeg and lastSeg are set to 1. Then when ar5416ProcTxDesc() goes to fill out ts_rate based on the transmit index, it looks at the rate control fields in that descriptor and dutifully sets it to be 0. It doesn't happen for non-aggregate frames - if they have one descriptor, the first descriptor already has rate control info. I removed the call to ath_hal_setuplasttxdesc() when I migrated the code to use the "new" style aggregate chain routines from the HAL. But I missed this particular corner case. This is a bit inefficient with MIPS boards as it involves a few redundant writes into non-cachable memory. I'll chase that up when it matters. Tested: * AR9280 STA mode, TCP iperf traffic * Rui Paulo <rpaulo@> first reported this and has verified it on his AR9160 based AP. PR: kern/173636 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 Thu Nov 15 00:51:57 2012 (r243046) +++ head/sys/dev/ath/if_ath_tx.c Thu Nov 15 03:00:49 2012 (r243047) @@ -623,6 +623,31 @@ ath_tx_setds_11n(struct ath_softc *sc, s */ bf_first->bf_last = bf_prev; + /* + * For non-AR9300 NICs, which require the rate control + * in the final descriptor - let's set that up now. + * + * This is because the filltxdesc() HAL call doesn't + * populate the last segment with rate control information + * if firstSeg is also true. For non-aggregate frames + * that is fine, as the first frame already has rate control + * info. But if the last frame in an aggregate has one + * descriptor, both firstseg and lastseg will be true and + * the rate info isn't copied. + * + * This is inefficient on MIPS/ARM platforms that have + * non-cachable memory for TX descriptors, but we'll just + * make do for now. + * + * As to why the rate table is stashed in the last descriptor + * rather than the first descriptor? Because proctxdesc() + * is called on the final descriptor in an MPDU or A-MPDU - + * ie, the one that gets updated by the hardware upon + * completion. That way proctxdesc() doesn't need to know + * about the first _and_ last TX descriptor. + */ + ath_hal_setuplasttxdesc(sc->sc_ah, bf_prev->bf_lastds, ds0); + DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR, "%s: end\n", __func__); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201211150300.qAF30nmY001743>