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

Adrian Chadd adrian at FreeBSD.org
Wed Aug 10 16:13:43 UTC 2011


Author: adrian
Date: Wed Aug 10 16:13:43 2011
New Revision: 224760
URL: http://svn.freebsd.org/changeset/base/224760

Log:
  Fix the (current) major issue where BAW tracking was failing, causing the TX
  side to hang.
  
  When ath_tx_draintxq() was called, flushing hardware-queued TX frames, they'd
  be immediately freed. The bf_comp handler wasn't being called. So frames
  in an aggregation session would be freed without their seqnum being updated
  in the BAW. If enough frames were skipped, the current seqnum would be outside
  the BAW and thus TX on that TID would stop.
  
  This fixes the initial issue of hanging TX during an aggregation session.
  
  This also causes the receiver to have to slide the BAW over as there will be
  gaps in what's transmitted but without a BAR being sent to force the RX side
  to update. That's (for now) preferable to having things just hang.

Modified:
  user/adrian/if_ath_tx/sys/dev/ath/if_ath.c
  user/adrian/if_ath_tx/sys/dev/ath/if_ath_misc.h
  user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c
  user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath.c	Wed Aug 10 15:49:24 2011	(r224759)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath.c	Wed Aug 10 16:13:43 2011	(r224760)
@@ -4110,19 +4110,30 @@ ath_tx_update_stats(struct ath_softc *sc
 
 }
 
+/*
+ * The default completion. If fail is 1, this means
+ * "please don't retry the frame, and just return -1 status
+ * to the net80211 stack.
+ */
 void
-ath_tx_default_comp(struct ath_softc *sc, struct ath_buf *bf)
+ath_tx_default_comp(struct ath_softc *sc, struct ath_buf *bf, int fail)
 {
 	struct ath_tx_status *ts = &bf->bf_status.ds_txstat;
+	int st;
+
+	if (fail == 1)
+		st = -1;
+	else
+		st = ((bf->bf_txflags & HAL_TXDESC_NOACK) == 0) ?
+		    ts->ts_status : HAL_TXERR_XRETRY;
+
 	/*
 	 * Do any tx complete callback.  Note this must
 	 * be done before releasing the node reference.
 	 * This will free the mbuf, release the net80211
 	 * node and recycle the ath_buf.
 	 */
-	ath_tx_freebuf(sc, bf,
-	    ((bf->bf_txflags & HAL_TXDESC_NOACK) == 0) ?
-	        ts->ts_status : HAL_TXERR_XRETRY);
+	ath_tx_freebuf(sc, bf, st);
 }
 
 /*
@@ -4211,9 +4222,9 @@ ath_tx_processq(struct ath_softc *sc, st
 		}
 
 		if (bf->bf_comp == NULL)
-			ath_tx_default_comp(sc, bf);
+			ath_tx_default_comp(sc, bf, 0);
 		else
-			bf->bf_comp(sc, bf);
+			bf->bf_comp(sc, bf, 0);
 	}
 #ifdef IEEE80211_SUPPORT_SUPERG
 	/*
@@ -4376,16 +4387,23 @@ ath_tx_draintxq(struct ath_softc *sc, st
 	if (bf != NULL)
 		bf->bf_flags &= ~ATH_BUF_BUSY;
 	ATH_TXBUF_UNLOCK(sc);
+	/*
+	 * The TXQ lock is held here for the entire trip through the
+	 * list (rather than just being held whilst acquiring frames
+	 * off of the list) because of the (current) assumption that
+	 * the per-TID software queue stuff is locked by the hardware
+	 * queue it maps to, thus ensuring proper ordering of things.
+	 *
+	 * The chance for LOR's however, is now that much bigger. Sigh.
+	 */
+	ATH_TXQ_LOCK(txq);
 	for (ix = 0;; ix++) {
-		ATH_TXQ_LOCK(txq);
 		bf = STAILQ_FIRST(&txq->axq_q);
 		if (bf == NULL) {
 			txq->axq_link = NULL;
-			ATH_TXQ_UNLOCK(txq);
 			break;
 		}
 		ATH_TXQ_REMOVE_HEAD(txq, bf_list);
-		ATH_TXQ_UNLOCK(txq);
 #ifdef ATH_DEBUG
 		if (sc->sc_debug & ATH_DEBUG_RESET) {
 			struct ieee80211com *ic = sc->sc_ifp->if_l2com;
@@ -4397,8 +4415,18 @@ ath_tx_draintxq(struct ath_softc *sc, st
 			    bf->bf_m->m_len, 0, -1);
 		}
 #endif /* ATH_DEBUG */
-		ath_tx_freebuf(sc, bf, -1);
+		//ath_tx_freebuf(sc, bf, -1);
+		/*
+		 * Since we're now doing magic in the completion
+		 * functions, we -must- call it for aggregation
+		 * destinations or BAW tracking will get upset.
+		 */
+		if (bf->bf_comp)
+			bf->bf_comp(sc, bf, 1);
+		else
+			ath_tx_default_comp(sc, bf, 1);
 	}
+	ATH_TXQ_UNLOCK(txq);
 }
 
 static void

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_misc.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath_misc.h	Wed Aug 10 15:49:24 2011	(r224759)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_misc.h	Wed Aug 10 16:13:43 2011	(r224760)
@@ -55,7 +55,8 @@ extern struct ath_buf * _ath_getbuf_lock
 
 extern int ath_reset(struct ifnet *);
 extern void ath_tx_draintxq(struct ath_softc *sc, struct ath_txq *txq);
-extern void ath_tx_default_comp(struct ath_softc *sc, struct ath_buf *bf);
+extern void ath_tx_default_comp(struct ath_softc *sc, struct ath_buf *bf,
+	    int fail);
 extern void ath_tx_freebuf(struct ath_softc *sc, struct ath_buf *bf,
     int status);
 

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	Wed Aug 10 15:49:24 2011	(r224759)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Wed Aug 10 16:13:43 2011	(r224760)
@@ -1890,10 +1890,10 @@ ath_tx_tid_cleanup(struct ath_softc *sc,
 
 #ifdef	notyet
 /*
- * Handle completion of non-aggregate frames.
+ * Handle completion of non-aggregate session frames.
  */
 static void
-ath_tx_normal_comp(struct ath_softc *sc, struct ath_buf *bf)
+ath_tx_normal_comp(struct ath_softc *sc, struct ath_buf *bf, int fail)
 {
 	struct ieee80211_node *ni = bf->bf_node;
 	struct ath_node *an;
@@ -1905,30 +1905,35 @@ ath_tx_normal_comp(struct ath_softc *sc,
 		an = ATH_NODE(ni);
 		atid = &an->an_tid[tid];
 
-		ath_tx_default_comp(sc, bf);
+		ath_tx_default_comp(sc, bf, fail);
 	}
 }
 #endif
 
 /*
  * Handle completion of aggregate frames.
+ *
+ * Fail is set to 1 if the entry is being freed via a call to
+ * ath_tx_draintxq().
  */
 static void
-ath_tx_aggr_comp(struct ath_softc *sc, struct ath_buf *bf)
+ath_tx_aggr_comp(struct ath_softc *sc, struct ath_buf *bf, int fail)
 {
 	struct ieee80211_node *ni = bf->bf_node;
 	struct ath_node *an = ATH_NODE(ni);
 	int tid = bf->bf_state.bfs_tid;
 	struct ath_tid *atid = &an->an_tid[tid];
+
+	if (tid == IEEE80211_NONQOS_TID)
+		device_printf(sc->sc_dev, "%s: TID=16!\n", __func__);
+	DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: bf=%p: tid=%d\n",
+	    __func__, bf, bf->bf_state.bfs_tid);
+
 	/*
 	 * Not success and have retries left?
 	 *
 	 * Mark as retry, requeue at head of queue
 	 */
-	if (tid == IEEE80211_NONQOS_TID)
-		device_printf(sc->sc_dev, "%s: TID=16!\n", __func__);
-	DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: bf=%p: tid=%d\n",
-	    __func__, bf, bf->bf_state.bfs_tid);
 
 	/*
 	 * Not success and out of retries?
@@ -1944,7 +1949,7 @@ ath_tx_aggr_comp(struct ath_softc *sc, s
 	    __func__, tid, SEQNO(bf->bf_state.bfs_seqno));
 	ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno));
 
-	ath_tx_default_comp(sc, bf);
+	ath_tx_default_comp(sc, bf, fail);
 	/* bf is freed at this point */
 }
 

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Wed Aug 10 15:49:24 2011	(r224759)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Wed Aug 10 16:13:43 2011	(r224760)
@@ -167,7 +167,11 @@ struct ath_buf {
 	bus_dma_segment_t	bf_segs[ATH_MAX_SCATTER];
 
 	/* Completion function to call on TX complete (fail or not) */
-	void(* bf_comp) (struct ath_softc *sc, struct ath_buf *bf);
+	/*
+	 * "fail" here is set to 1 if the queue entries were removed
+	 * through a call to ath_tx_draintxq().
+	 */
+	void(* bf_comp) (struct ath_softc *sc, struct ath_buf *bf, int fail);
 
 	/* This state is kept to support software retries and aggregation */
 	struct {


More information about the svn-src-user mailing list