svn commit: r224867 - user/adrian/if_ath_tx/sys/dev/ath

Sun Aug 14 16:03:26 UTC 2011

Author: adrian
Date: Sun Aug 14 16:03:26 2011
New Revision: 224867
URL: http://svn.freebsd.org/changeset/base/224867

Log:
  Begin to attempt to handle sending BAR frames. This doesn't work yet and will
  panic the kernel.
  
  BAR frames should be generated to resync the remote side with where the block-ack
  window left edge is - eg, after some packet loss.
  
  The problem here is that said BAR frames will be generated from:
  
  * the TX packet completion handler; and
  * via a timer callout if the BAR TX wasn't successful.
  
  The former will occur with the hardware TXQ lock held, as that's currently
  how I've implemented the locking (ie, the hardware TXQ protects the TXQ and
  the TID state.) This results in lock recursion (as we're recursing back
  through net80211 and then back into the driver via ic->ic_raw_xmit()) and
  thus TX'ing a BAR frame will very much fail.
  
  The problem here (among other things) is that TX'ing a BAR frame should
  occur after all the hardware-queued frames for that AC, in case there's
  some other packets (which may or may not be successfully transmitted
  in the future), and in the same hardware queue. This means we can't just
  use the normal software queue method, as the TID should be paused when
  a BAR frame is TX'ed. Hence, the direct dispatch to hardware.
  
  Anyway, this is all quite messy. I think I'm going to have to bite the
  bullet and (re) introduce more fine-grained locking in a subsequent commit
  before supporting this in a clean fashion is even possible.
  
  (The other option is to modify the ieee80211_send_bar()) function to just
  schedule a callback even and have the TX occur from there, rather than
  recurse through the driver. I may end up doing that at a later date, but
  I really need to fix the locking anyway. So, I'll just do that first.)

Modified:
  user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c
==============================================================================

--- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Sun Aug 14 14:36:32 2011	(r224866)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Sun Aug 14 16:03:26 2011	(r224867)
@@ -1010,7 +1010,7 @@ ath_tx_start(struct ath_softc *sc, struc
 	const struct ieee80211_frame *wh;
 	int is_ampdu, is_ampdu_tx, is_ampdu_pending;
 	ieee80211_seq seqno;
-	uint8_t subtype;
+	uint8_t type, subtype;
 
 	/*
 	 * Determine the target hardware queue.
@@ -1032,6 +1032,7 @@ ath_tx_start(struct ath_softc *sc, struc
 	txq = sc->sc_ac2q[pri];
 	wh = mtod(m0, struct ieee80211_frame *);
 	ismcast = IEEE80211_IS_MULTICAST(wh->i_addr1);
+	type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK;
 	subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK;
 
 	/* A-MPDU TX */
@@ -1097,9 +1098,38 @@ ath_tx_start(struct ath_softc *sc, struc
 	 * destination hardware queue. Don't bother software
 	 * queuing it.
 	 */
+	/*
+	 * If it's a BAR frame, do a direct dispatch to the
+	 * destination hardware queue. Don't bother software
+	 * queuing it, as the TID will now be paused.
+	 */
 	if (ismcast)
 		ath_tx_handoff_mcast(sc, txq, bf);
-	else {
+	else if (type == IEEE80211_FC0_TYPE_CTL &&
+		    subtype == IEEE80211_FC0_SUBTYPE_BAR) {
+		/*
+		 * XXX The following is dirty but needed for now.
+		 *
+		 * Because of the current way locking is implemented,
+		 * we may end up here because of a call to
+		 * ieee80211_send_bar() from a call inside the completion
+		 * handler. This will have the hardware TXQ locked,
+		 * protecting the TXQ and the node TID state in question.
+		 *
+		 * So we (a) can't SWQ queue to it, as it'll be queued
+		 * on the same TID which will be paused, and (b) the TXQ
+		 * will be locked anyway, so grabbing the lock will cause
+		 * recursion.
+		 *
+		 * The longer term issue is that the TXQ lock is being held
+		 * for so damned long, and that must be addressed before this
+		 * stuff is merged into -HEAD.
+		 */
+		device_printf(sc->sc_dev, "%s: BAR: TX'ing direct\n", __func__);
+		ATH_TXQ_LOCK(txq);
+		ath_tx_handoff(sc, txq, bf);
+		ATH_TXQ_UNLOCK(txq);
+	} else {
 		ATH_TXQ_LOCK(txq);
 		/* add to software queue */
 		ath_tx_swq(sc, ni, txq, bf);
@@ -1108,15 +1138,16 @@ ath_tx_start(struct ath_softc *sc, struc
 		ATH_TXQ_UNLOCK(txq);
 	}
 #else
-
 	/*
 	 * For now, since there's no software queue,
 	 * direct-dispatch to the hardware.
 	 */
+	ATH_TXQ_LOCK(txq);
 	if (ismcast)
 		ath_tx_handoff_mcast(sc, txq, bf);
 	else
 		ath_tx_handoff_hw(sc, txq, bf);
+	ATH_TXQ_UNLOCK(txq);
 #endif
 
 	return 0;
@@ -2093,9 +2124,12 @@ ath_tx_aggr_retry_unaggr(struct ath_soft
 	int tid = bf->bf_state.bfs_tid;
 	struct ath_tid *atid = &an->an_tid[tid];
 	struct ath_txq *txq = sc->sc_ac2q[atid->ac];
+	struct ieee80211_tx_ampdu *tap;
 
 	ATH_TXQ_LOCK_ASSERT(txq);
 
+	tap = ath_tx_get_tx_tid(an, tid);
+
 	if (bf->bf_state.bfs_retries >= SWMAX_RETRIES) {
 		sc->sc_stats.ast_tx_swretrymax++;
 
@@ -2107,8 +2141,18 @@ ath_tx_aggr_retry_unaggr(struct ath_soft
 		/* Pause the TID */
 
 		/* Send BAR frame */
+		/*
+		 * XXX This causes the kernel to recurse
+		 * XXX back into the net80211 layer, then back out
+		 * XXX to the TX queue (via ic->ic_raw_xmit()).
+		 * XXX Because the TXQ lock is held here,
+		 * XXX this will cause a lock recurse panic.
+		 */
 		device_printf(sc->sc_dev, "%s: TID %d: send BAR\n",
 		    __func__, tid);
+#if 0
+		ieee80211_send_bar(ni, tap, ni->ni_txseqs[tid]);
+#endif
 
 		/* Free buffer, bf is free after this call */
 		ath_tx_default_comp(sc, bf, 0);
