Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 13 Aug 2011 10:43:56 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   svn commit: r224813 - user/adrian/if_ath_tx/sys/dev/ath
Message-ID:  <201108131043.p7DAhu7S010342@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: adrian
Date: Sat Aug 13 10:43:56 2011
New Revision: 224813
URL: http://svn.freebsd.org/changeset/base/224813

Log:
  This deviates a bit from the reference code; I may end up backing this out
  at a later stage.
  
  This unifies the "is this packet supposed to be considered as part of the
  BAW?" logic. I've added a bit in the ath_buf.bf_state struct which indicates
  whether it's going to be considered for inclusion or not.
  
  Packets that aren't part of the BAW calculation are thus not passed to the
  BAW add or update code.
  
  Eventually, the aggregate forming code will also not include this packet
  as part of an aggregate and will be sent as a non-aggregate packet.
  
  This fixes the hangs, interface resets and general unhappiness that has been
  happening with my recent changes.

Modified:
  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_tx.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Sat Aug 13 10:43:21 2011	(r224812)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Sat Aug 13 10:43:56 2011	(r224813)
@@ -1010,6 +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;
 
 	/*
 	 * Determine the target hardware queue.
@@ -1031,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);
+	subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK;
 
 	/* A-MPDU TX */
 	is_ampdu_tx = ath_tx_ampdu_running(sc, ATH_NODE(ni), tid);
@@ -1043,15 +1045,27 @@ ath_tx_start(struct ath_softc *sc, struc
 	/* Multicast frames go onto the software multicast queue */
 	if (ismcast)
 		txq = &avp->av_mcastq;
- 	if ((! is_ampdu) && (vap->iv_ps_sta || avp->av_mcastq.axq_depth))
+
+	if ((! is_ampdu) && (vap->iv_ps_sta || avp->av_mcastq.axq_depth))
 		txq = &avp->av_mcastq;
 
 	/* Do the generic frame setup */
 
 	/* A-MPDU TX? Manually set sequence number */
 	/* Don't do it whilst pending; the net80211 layer still assigns them */
-	if (is_ampdu_tx)
+	if (is_ampdu_tx) {
+		/*
+		 * Always set a seqno; this function will
+		 * handle making sure that null data frames
+		 * don't get a sequence number from the current
+		 * TID and thus mess with the BAW.
+		 */
 		seqno = ath_tx_tid_seqno_assign(sc, ni, bf, m0);
+		if (IEEE80211_QOS_HAS_SEQ(wh) &&
+		    subtype != IEEE80211_FC0_SUBTYPE_QOS_NULL) {
+			bf->bf_state.bfs_dobaw = 1;
+		}
+	}
 
 	/*
 	 * If needed, the sequence number has been assigned.
@@ -1708,6 +1722,7 @@ ath_tx_swq(struct ath_softc *sc, struct 
 	bf->bf_state.bfs_tid = tid;
 	bf->bf_state.bfs_txq = txq;
 	bf->bf_state.bfs_pri = pri;
+	bf->bf_state.bfs_dobaw = 0;
 
 	/* Queue frame to the tail of the software queue */
 	ATH_TXQ_INSERT_TAIL(atid, bf, bf_list);
@@ -1839,7 +1854,8 @@ ath_tx_tid_free_pkts(struct ath_softc *s
 		 * If the current TID is running AMPDU, update
 		 * the BAW.
 		 */
-		if (ath_tx_ampdu_running(sc, an, tid))
+		if (ath_tx_ampdu_running(sc, an, tid) &&
+		    bf->bf_state.bfs_dobaw)
 			ath_tx_update_baw(sc, an, atid,
 			    SEQNO(bf->bf_state.bfs_seqno));
 		ATH_TXQ_REMOVE_HEAD(atid, bf_list);
@@ -1994,8 +2010,9 @@ ath_tx_cleanup(struct ath_softc *sc, str
 			bf_next = STAILQ_NEXT(bf, bf_list);
 			STAILQ_REMOVE(&atid->axq_q, bf, ath_buf, bf_list);
 			atid->axq_depth--;
-			ath_tx_update_baw(sc, an, atid,
-			    SEQNO(bf->bf_state.bfs_seqno));
+			if (bf->bf_state.bfs_dobaw)
+				ath_tx_update_baw(sc, an, atid,
+				    SEQNO(bf->bf_state.bfs_seqno));
 			/*
 			 * Call the default completion handler with "fail" just
 			 * so upper levels are suitably notified about this.
@@ -2083,7 +2100,9 @@ ath_tx_aggr_retry_unaggr(struct ath_soft
 		sc->sc_stats.ast_tx_swretrymax++;
 
 		/* Update BAW anyway */
-		ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno));
+		if (bf->bf_state.bfs_dobaw)
+			ath_tx_update_baw(sc, an, atid,
+			    SEQNO(bf->bf_state.bfs_seqno));
 
 		/* Free buffer, bf is free after this call */
 		ath_tx_default_comp(sc, bf, 0);
@@ -2150,7 +2169,8 @@ ath_tx_aggr_comp(struct ath_softc *sc, s
 	/* Success? Complete */
 	DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: TID=%d, seqno %d\n",
 	    __func__, tid, SEQNO(bf->bf_state.bfs_seqno));
-	ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno));
+	if (bf->bf_state.bfs_dobaw)
+		ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno));
 
 	ath_tx_default_comp(sc, bf, fail);
 	/* bf is freed at this point */
@@ -2168,9 +2188,6 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft
 	struct ath_txq *txq;
 	struct ath_tid *atid = &an->an_tid[tid];
 	struct ieee80211_tx_ampdu *tap;
-	int check_baw;
-	uint8_t subtype;
-	const struct ieee80211_frame *wh;
 
 	DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: tid=%d\n", __func__, tid);
 
@@ -2181,7 +2198,6 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft
 		    __func__);
 
 	for (;;) {
-		check_baw = 1;
                 bf = STAILQ_FIRST(&atid->axq_q);
 		if (bf == NULL) {
 			break;
@@ -2195,22 +2211,8 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft
 			device_printf(sc->sc_dev, "%s: TXQ: tid=%d, ac=%d, bf tid=%d\n",
 			    __func__, tid, atid->ac, bf->bf_state.bfs_tid);
 
-		/*
-		 * Some packets aren't going to fall within the BAW.
-		 * If they're non-sequence QOS packets that sit inside this TID,
-		 * or they're a null data frame.
-		 * This is quite messy and I should make the seqno code and
-		 * this code share the same decision logic.
-		 */
-		wh = mtod(bf->bf_m, const struct ieee80211_frame *);
-		if (! IEEE80211_QOS_HAS_SEQ(wh))
-			check_baw = 0;
-		subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK;
-		if (subtype == IEEE80211_FC0_SUBTYPE_QOS_NULL)
-			check_baw = 0;
-
-		/* XXX check if seqno is outside of BAW, if so don't queue it */
-		if (check_baw == 1 &&
+		/* Check if seqno is outside of BAW, if so don't queue it */
+		if (bf->bf_state.bfs_dobaw &&
 		    (! BAW_WITHIN(tap->txa_start, tap->txa_wnd,
 		    SEQNO(bf->bf_state.bfs_seqno)))) {
 			DPRINTF(sc, ATH_DEBUG_SW_TX_BAW,
@@ -2221,7 +2223,7 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft
 		}
 
 		/* Don't add packets to the BAW that don't contribute to it */
-		if (check_baw == 1)
+		if (bf->bf_state.bfs_dobaw)
 			ath_tx_addto_baw(sc, an, atid, bf);
 
 		/*

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Sat Aug 13 10:43:21 2011	(r224812)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Sat Aug 13 10:43:56 2011	(r224813)
@@ -207,6 +207,7 @@ struct ath_buf {
 		int bfs_aggr:1;		/* part of aggregate? */
 		int bfs_aggrburst:1;	/* part of aggregate burst? */
 		int bfs_isretried:1;	/* retried frame? */
+		int bfs_dobaw:1;	/* actually check against BAW? */
 	} bf_state;
 };
 typedef STAILQ_HEAD(, ath_buf) ath_bufhead;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108131043.p7DAhu7S010342>