Introduce the CLRDMASK gating based on tid->clrdmask, enabling filtered

frames to occur.

* Create a new function which will set the bf_flags CLRDMASK bit
  if required.
* For raw frames, always set CLRDMASK.
* For BAR, ADDBA frames, always set CLRDMASK.
* For everything else, check if CLRDMASK needs to be set before
  calling tx_setds() or tx_setds11n().
* When unpausing a queue or drain/resetting it, set tid->clrdmask=1
  just to ensure traffic starts flowing.

What I need to do:

* Modify that function to _clear_ the CLRDMASK if it's not required,
  or retried frames may have CLRDMASK set when they don't need to.
  (Which isn't a huge deal, but..)

Whilst I'm here:

* ath_tx_normal_xmit() should really act like the AMPDU session TX
  functions - any incomplete frames will end up being assigned
  ath_tx_normal_comp() which will decrement tid->hwq_depth - but that
  won't have been incremented.

  So whilst I'm here, add a comment to do that.

* Fix the debug print function to be slightly clearer about things;
  it's not a good sign when I can't interpret my own debugging output.

I've done some testing on AR9280/AR5416/AR9160 STA and AP modes.

1 files changed, 81 insertions, 9 deletions

diff --git a/sys/dev/ath/if_ath_tx.c b/sys/dev/ath/if_ath_tx.c
index de61a6b..aa750d8 100644
--- a/sys/dev/ath/if_ath_tx.c
+++ b/sys/dev/ath/if_ath_tx.c
@@ -1285,6 +1285,9 @@ 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 */
+
 	/* Hand off to hardware */
 	ath_tx_handoff(sc, txq, bf);
 }
@@ -1384,7 +1387,8 @@ ath_tx_normal_setup(struct ath_softc *sc, struct ieee80211_node *ni,
 	}
 
 	an = ATH_NODE(ni);
-	flags = HAL_TXDESC_CLRDMASK;		/* XXX needed for crypto errs */
+	//flags = HAL_TXDESC_CLRDMASK;		/* XXX needed for crypto errs */
+	flags = 0;
 	ismrr = 0;				/* default no multi-rate retry*/
 	pri = M_WME_GETAC(m0);			/* honor classification */
 	/* XXX use txparams instead of fixed values */
@@ -1598,12 +1602,15 @@ ath_tx_normal_setup(struct ath_softc *sc, struct ieee80211_node *ni,
 }
 
 /*
- * Direct-dispatch the current frame to the hardware.
+ * Queue a frame to the hardware or software queue.
  *
  * This can be called by the net80211 code.
  *
  * XXX what about locking? Or, push the seqno assign into the
  * XXX aggregate scheduler so its serialised?
+ *
+ * XXX When sending management frames via ath_raw_xmit(),
+ *     should CLRDMASK be set unconditionally?
  */
 int
 ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
@@ -1774,11 +1781,13 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
 	if (txq == &avp->av_mcastq) {
 		DPRINTF(sc, ATH_DEBUG_SW_TX,
 		    "%s: bf=%p: mcastq: TX'ing\n", __func__, bf);
+		bf->bf_state.bfs_txflags |= HAL_TXDESC_CLRDMASK;
 		ath_tx_xmit_normal(sc, txq, bf);
 	} else if (type == IEEE80211_FC0_TYPE_CTL &&
 		    subtype == IEEE80211_FC0_SUBTYPE_BAR) {
 		DPRINTF(sc, ATH_DEBUG_SW_TX,
 		    "%s: BAR: TX'ing direct\n", __func__);
+		bf->bf_state.bfs_txflags |= HAL_TXDESC_CLRDMASK;
 		ath_tx_xmit_normal(sc, txq, bf);
 	} else {
 		/* add to software queue */
@@ -1791,6 +1800,7 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni,
 	 * For now, since there's no software queue,
 	 * direct-dispatch to the hardware.
 	 */
+	bf->bf_state.bfs_txflags |= HAL_TXDESC_CLRDMASK;
 	ath_tx_xmit_normal(sc, txq, bf);
 #endif
 done:
@@ -1874,6 +1884,7 @@ ath_tx_raw_start(struct ath_softc *sc, struct ieee80211_node *ni,
 	wh = mtod(m0, struct ieee80211_frame *);
 	bf->bf_node = ni;			/* NB: held reference */
 
+	/* Always enable CLRDMASK for raw frames for now.. */
 	flags = HAL_TXDESC_CLRDMASK;		/* XXX needed for crypto errs */
 	flags |= HAL_TXDESC_INTREQ;		/* force interrupt */
 	if (params->ibp_flags & IEEE80211_BPF_RTS)
@@ -2002,6 +2013,7 @@ ath_tx_raw_start(struct ath_softc *sc, struct ieee80211_node *ni,
 	    __func__, do_override);
 
 	if (do_override) {
+		bf->bf_state.bfs_txflags |= HAL_TXDESC_CLRDMASK;
 		ath_tx_xmit_normal(sc, sc->sc_ac2q[pri], bf);
 	} else {
 		/* Queue to software queue */
@@ -2467,6 +2479,22 @@ ath_tx_tid_unsched(struct ath_softc *sc, struct ath_tid *tid)
 }
 
 /*
+ * 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.
  *
  * This should only be called for A-MPDU TX frames.
@@ -2582,6 +2610,9 @@ ath_tx_xmit_aggr(struct ath_softc *sc, struct ath_node *an,
 		bf->bf_state.bfs_nframes = 1;
 	}
 
+	/* Update CLRDMASK just before this frame is queued */
+	ath_tx_update_clrdmask(sc, tid, bf);
+
 	/* Direct dispatch to hardware */
 	ath_tx_do_ratelookup(sc, bf);
 	ath_tx_calc_duration(sc, bf);
@@ -2701,6 +2732,7 @@ 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 */
@@ -2789,6 +2821,12 @@ ath_tx_tid_resume(struct ath_softc *sc, struct ath_tid *tid)
 		return;
 	}
 
+	/*
+	 * Override the clrdmask configuration for the next frame,
+	 * just to get the ball rolling.
+	 */
+	tid->clrdmask = 1;
+
 	ath_tx_tid_sched(sc, tid);
 	/* Punt some frames to the hardware if needed */
 	//ath_txq_sched(sc, sc->sc_ac2q[tid->ac]);
@@ -3109,6 +3147,12 @@ ath_tx_tid_bar_tx(struct ath_softc *sc, struct ath_tid *tid)
 	tid->bar_tx = 1;
 
 	/*
+	 * Override the clrdmask configuration for the next frame,
+	 * just to get the ball rolling.
+	 */
+	tid->clrdmask = 1;
+
+	/*
 	 * Calculate new BAW left edge, now that all frames have either
 	 * succeeded or failed.
 	 *
@@ -3189,6 +3233,12 @@ ath_tx_tid_drain_print(struct ath_softc *sc, struct ath_node *an,
 	    SEQNO(bf->bf_state.bfs_seqno),
 	    bf->bf_state.bfs_retries);
 	device_printf(sc->sc_dev,
+	    "%s: node %p: bf=%p: txq axq_depth=%d, axq_aggr_depth=%d\n",
+	        __func__, ni, bf,
+		txq->axq_depth,
+		txq->axq_aggr_depth);
+
+	device_printf(sc->sc_dev,
 	    "%s: node %p: bf=%p: tid txq_depth=%d hwq_depth=%d, bar_wait=%d, isfiltered=%d\n",
 	    __func__, ni, bf,
 	    tid->axq_depth,
@@ -3196,13 +3246,13 @@ ath_tx_tid_drain_print(struct ath_softc *sc, struct ath_node *an,
 	    tid->bar_wait,
 	    tid->isfiltered);
 	device_printf(sc->sc_dev,
-	    "%s: node %p: tid %d: txq_depth=%d, "
-	    "txq_aggr_depth=%d, sched=%d, paused=%d, "
-	    "hwq_depth=%d, incomp=%d, baw_head=%d, "
+	    "%s: node %p: tid %d: "
+	    "sched=%d, paused=%d, "
+	    "incomp=%d, baw_head=%d, "
 	    "baw_tail=%d txa_start=%d, ni_txseqs=%d\n",
-	     __func__, ni, tid->tid, txq->axq_depth,
-	     txq->axq_aggr_depth, tid->sched, tid->paused,
-	     tid->hwq_depth, tid->incomp, tid->baw_head,
+	     __func__, ni, tid->tid,
+	     tid->sched, tid->paused,
+	     tid->incomp, tid->baw_head,
 	     tid->baw_tail, tap == NULL ? -1 : tap->txa_start,
 	     ni->ni_txseqs[tid->tid]);
 
@@ -3274,6 +3324,15 @@ ath_tx_tid_drain(struct ath_softc *sc, struct ath_node *an,
 	}
 
 	/*
+	 * Override the clrdmask configuration for the next frame
+	 * in case there is some future transmission, just to get
+	 * the ball rolling.
+	 *
+	 * This won't hurt things if the TID is about to be freed.
+	 */
+	tid->clrdmask = 1;
+
+	/*
 	 * Now that it's completed, grab the TID lock and update
 	 * the sequence number and BAW window.
 	 * Because sequence numbers have been assigned to frames
@@ -4511,6 +4570,9 @@ ath_tx_tid_hw_queue_aggr(struct ath_softc *sc, struct ath_node *an,
 			bf->bf_state.bfs_aggr = 0;
 			bf->bf_state.bfs_nframes = 1;
 
+			/* Update CLRDMASK just before this frame is queued */
+			ath_tx_update_clrdmask(sc, tid, bf);
+
 			ath_tx_do_ratelookup(sc, bf);
 			ath_tx_calc_duration(sc, bf);
 			ath_tx_calc_protection(sc, bf);
@@ -4573,6 +4635,10 @@ ath_tx_tid_hw_queue_aggr(struct ath_softc *sc, struct ath_node *an,
 		if (bf->bf_state.bfs_nframes == 1) {
 			DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR,
 			    "%s: single-frame aggregate\n", __func__);
+
+			/* Update CLRDMASK just before this frame is queued */
+			ath_tx_update_clrdmask(sc, tid, bf);
+
 			bf->bf_state.bfs_aggr = 0;
 			bf->bf_state.bfs_ndelim = 0;
 			ath_tx_setds(sc, bf);
@@ -4591,6 +4657,9 @@ ath_tx_tid_hw_queue_aggr(struct ath_softc *sc, struct ath_node *an,
 			sc->sc_aggr_stats.aggr_pkts[bf->bf_state.bfs_nframes]++;
 			sc->sc_aggr_stats.aggr_aggr_pkt++;
 
+			/* Update CLRDMASK just before this frame is queued */
+			ath_tx_update_clrdmask(sc, tid, bf);
+
 			/*
 			 * Calculate the duration/protection as required.
 			 */
@@ -4654,7 +4723,7 @@ ath_tx_tid_hw_queue_norm(struct ath_softc *sc, struct ath_node *an,
 	DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: node %p: TID %d: called\n",
 	    __func__, an, tid->tid);
 
-	ATH_TXQ_LOCK_ASSERT(txq);
+	ATH_TID_LOCK_ASSERT(sc, tid);
 
 	/* Check - is AMPDU pending or running? then print out something */
 	if (ath_tx_ampdu_pending(sc, an, tid->tid))
@@ -4691,6 +4760,9 @@ ath_tx_tid_hw_queue_norm(struct ath_softc *sc, struct ath_node *an,
 		/* Normal completion handler */
 		bf->bf_comp = ath_tx_normal_comp;
 
+		/* Update CLRDMASK just before this frame is queued */
+		ath_tx_update_clrdmask(sc, tid, bf);
+
 		/* Program descriptors + rate control */
 		ath_tx_do_ratelookup(sc, bf);
 		ath_tx_calc_duration(sc, bf);