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
This commit is contained in:
Adrian Chadd 2012-11-15 03:00:49 +00:00
parent 28d91af30f
commit bbdf3df1c4

View File

@ -623,6 +623,31 @@ ath_tx_setds_11n(struct ath_softc *sc, struct ath_buf *bf_first)
*/
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__);
}