Prepare for software retransmission of non-aggregate frames but ensure

it's disabled.

The previous commit to enable CLRDMASK setting didn't do it at all
correctly for non-aggregate sessions - so the CLRDMASK bit would be
cleared and never re-set.

* move ath_tx_update_clrdmask() to be called by functions that setup
  descriptors and queue frames to the hardware, rather than scattered
  everywhere.

* Force CLRDMASK to be set on all non-aggregate session frames being
  transmitted.

* Use ath_tx_normal_comp() now on non-aggregate sessoin frames
  that are queued via ath_tx_xmit_normal().  That way the TID hwq is
  updated and they can trigger (eventual) filter frame queue resets
  and software retransmits.

There's still a bit more work to do in this area to reverse the silly
short-sightedness on my part, however it's likely going to be better
to fix this now than just reverting the patch.

Thanks to people on the freebsd-wireless@ mailing list for promptly
pointing this out.
This commit is contained in:
Adrian Chadd 2012-09-24 06:42:20 +00:00
parent 94eefcf1dc
commit 0c54de88e6
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=240883

View File

@ -1259,6 +1259,22 @@ ath_tx_do_ratelookup(struct ath_softc *sc, struct ath_buf *bf)
bf->bf_state.bfs_txrate0 = rate;
}
/*
* Update the CLRDMASK bit in the ath_buf if it needs to be set.
*/
static void
ath_tx_update_clrdmask(struct ath_softc *sc, struct ath_tid *tid,
struct ath_buf *bf)
{
ATH_TID_LOCK_ASSERT(sc, tid);
if (tid->clrdmask == 1) {
bf->bf_state.bfs_txflags |= HAL_TXDESC_CLRDMASK;
tid->clrdmask = 0;
}
}
/*
* Transmit the given frame to the hardware.
*
@ -1274,9 +1290,25 @@ static void
ath_tx_xmit_normal(struct ath_softc *sc, struct ath_txq *txq,
struct ath_buf *bf)
{
struct ath_node *an = ATH_NODE(bf->bf_node);
struct ath_tid *tid = &an->an_tid[bf->bf_state.bfs_tid];
ATH_TXQ_LOCK_ASSERT(txq);
/*
* For now, just enable CLRDMASK. ath_tx_xmit_normal() does
* set a completion handler however it doesn't (yet) properly
* handle the strict ordering requirements needed for normal,
* non-aggregate session frames.
*
* Once this is implemented, only set CLRDMASK like this for
* frames that must go out - eg management/raw frames.
*/
bf->bf_state.bfs_txflags |= HAL_TXDESC_CLRDMASK;
/* See if clrdmask needs to be set */
ath_tx_update_clrdmask(sc, tid, bf);
/* Setup the descriptor before handoff */
ath_tx_do_ratelookup(sc, bf);
ath_tx_calc_duration(sc, bf);
@ -1285,8 +1317,11 @@ ath_tx_xmit_normal(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_rate_fill_rcflags(sc, bf);
ath_tx_setds(sc, bf);
/* XXX Bump TID HWQ counter */
/* XXX Assign a completion handler */
/* Track per-TID hardware queue depth correctly */
tid->hwq_depth++;
/* Assign the completion handler */
bf->bf_comp = ath_tx_normal_comp;
/* Hand off to hardware */
ath_tx_handoff(sc, txq, bf);
@ -2484,22 +2519,6 @@ ath_tx_tid_unsched(struct ath_softc *sc, struct ath_tid *tid)
TAILQ_REMOVE(&txq->axq_tidq, tid, axq_qelem);
}
/*
* Update the CLRDMASK bit in the ath_buf if it needs to be set.
*/
static void
ath_tx_update_clrdmask(struct ath_softc *sc, struct ath_tid *tid,
struct ath_buf *bf)
{
ATH_TID_LOCK_ASSERT(sc, tid);
if (tid->clrdmask == 1) {
bf->bf_state.bfs_txflags |= HAL_TXDESC_CLRDMASK;
tid->clrdmask = 0;
}
}
/*
* Assign a sequence number manually to the given frame.
*
@ -2738,7 +2757,6 @@ ath_tx_swq(struct ath_softc *sc, struct ieee80211_node *ni, struct ath_txq *txq,
} else if (txq->axq_depth < sc->sc_hwq_limit) {
/* AMPDU not running, attempt direct dispatch */
DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: xmit_normal\n", __func__);
ath_tx_update_clrdmask(sc, atid, bf);
ath_tx_xmit_normal(sc, txq, bf);
} else {
/* Busy; queue */
@ -3432,6 +3450,19 @@ ath_tx_txq_drain(struct ath_softc *sc, struct ath_txq *txq)
/*
* Handle completion of non-aggregate session frames.
*
* This (currently) doesn't implement software retransmission of
* non-aggregate frames!
*
* Software retransmission of non-aggregate frames needs to obey
* the strict sequence number ordering, and drop any frames that
* will fail this.
*
* For now, filtered frames and frame transmission will cause
* all kinds of issues. So we don't support them.
*
* So anyone queuing frames via ath_tx_normal_xmit() or
* ath_tx_hw_queue_norm() must override and set CLRDMASK.
*/
void
ath_tx_normal_comp(struct ath_softc *sc, struct ath_buf *bf, int fail)
@ -3450,10 +3481,23 @@ ath_tx_normal_comp(struct ath_softc *sc, struct ath_buf *bf, int fail)
atid->hwq_depth--;
#if 0
/*
* If the frame was filtered, stick it on the filter frame
* queue and complain about it. It shouldn't happen!
*/
if ((ts->ts_status & HAL_TXERR_FILT) ||
(ts->ts_status != 0 && atid->isfiltered)) {
device_printf(sc->sc_dev,
"%s: isfiltered=%d, ts_status=%d: huh?\n",
__func__,
atid->isfiltered,
ts->ts_status);
ath_tx_tid_filt_comp_buf(sc, atid, bf);
}
#endif
if (atid->isfiltered)
device_printf(sc->sc_dev, "%s: isfiltered=1, normal_comp?\n",
__func__);
device_printf(sc->sc_dev, "%s: filtered?!\n", __func__);
if (atid->hwq_depth < 0)
device_printf(sc->sc_dev, "%s: hwq_depth < 0: %d\n",
__func__, atid->hwq_depth);
@ -3471,7 +3515,6 @@ ath_tx_normal_comp(struct ath_softc *sc, struct ath_buf *bf, int fail)
*/
if (atid->isfiltered)
ath_tx_tid_filt_comp_complete(sc, atid);
ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
/*
@ -4766,6 +4809,12 @@ ath_tx_tid_hw_queue_norm(struct ath_softc *sc, struct ath_node *an,
/* Normal completion handler */
bf->bf_comp = ath_tx_normal_comp;
/*
* Override this for now, until the non-aggregate
* completion handler correctly handles software retransmits.
*/
bf->bf_state.bfs_txflags |= HAL_TXDESC_CLRDMASK;
/* Update CLRDMASK just before this frame is queued */
ath_tx_update_clrdmask(sc, tid, bf);