Date: Sat, 31 Mar 2012 11:28:57 -0700 From: Adrian Chadd <adrian@freebsd.org> To: freebsd-wireless@freebsd.org Subject: Request for testing: BAR TX handling for ath(4) Message-ID: <CAJ-VmonZZZoY5iRDGLrtNjFLy5Fc5NajhkfdqSzi=s4SF2n5rQ@mail.gmail.com>
index | next in thread | raw e-mail
[-- Attachment #1 --]
Hi all,
This patch is my first (dirty) attempt at implementing BAR TX for FreeBSD.
What it does:
* adds two state flags for ath_tid - one that indicates that a BAR is
pending, another that indicates that a BAR TX has been attempted.
* whenever a frame is dropped, transition into the BAR pending state
and pause the queue - but it's paused once, on the state transition,
since multiple frames inside the BAW may be dropped;
* .. then, don't transmit anything from that TID until all the frames
in the hardware queue for that TID are completed.
* Check whether we need to send the BAR upon completion of any frame
it an aggregation session (aggregate or unaggregate) - as there may
have been more than one frame pending on the hardware queue, and only
one out of a larger set may have failed.
I'm not entirely comfortable with this solution - especially with the
silly locking - but it does work.
The main downside is that the call to ieee80211_send_bar() can fail if
the ic_raw_xmit function or the mbuf allocation functions return
errors. Now, what I'm seeing with live tests is that sometimes, the
sender side will fail one of these and the BAR attempt will fail. For
now this code just ignores it and continues along it's merry way. It
_should_ drop out of aggregation.
If ieee80211_send_bar() does fail to successfully transmit the BAR, it
will eventually transition the session out of aggregation. However, if
the initial call to ieee80211_send_bar() fails, then the driver has to
handle it itself.
I'll fix the behaviour or use of ieee80211_send_bar() in a future
commit. I'd just like to get the BAR suspend/resume stuff tidied up
and in the tree so I'm one step closer to enabling ath 11n by default.
TL;DR - if you're testing out FreeBSD ath(4) 802.11n support, please
enable this patch and let me know what happens.
Thanks!
Adrian
[-- Attachment #2 --]
Index: sys/dev/ath/if_athvar.h
===================================================================
--- sys/dev/ath/if_athvar.h (revision 233673)
+++ sys/dev/ath/if_athvar.h (working copy)
@@ -106,6 +106,8 @@
TAILQ_ENTRY(ath_tid) axq_qelem;
int sched;
int paused; /* >0 if the TID has been paused */
+ int bar_wait; /* waiting for BAR */
+ int bar_tx; /* BAR TXed */
/*
* Is the TID being cleaned up after a transition
Index: sys/dev/ath/if_ath_tx.c
===================================================================
--- sys/dev/ath/if_ath_tx.c (revision 233673)
+++ sys/dev/ath/if_ath_tx.c (working copy)
@@ -2597,11 +2597,11 @@
static void
ath_tx_tid_pause(struct ath_softc *sc, struct ath_tid *tid)
{
- ATH_TXQ_LOCK(sc->sc_ac2q[tid->ac]);
+
+ ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]);
tid->paused++;
DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: paused = %d\n",
__func__, tid->paused);
- ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]);
}
/*
@@ -2628,6 +2628,146 @@
}
/*
+ * Suspend the queue because we need to TX a BAR.
+ */
+static void
+ath_tx_tid_bar_suspend(struct ath_softc *sc, struct ath_tid *tid)
+{
+ ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]);
+
+ device_printf(sc->sc_dev, "%s: tid=%p, called\n", __func__, tid);
+
+ /* We shouldn't be called when bar_tx is 1 */
+ if (tid->bar_tx) {
+ device_printf(sc->sc_dev, "%s: bar_tx is 1?!\n",
+ __func__);
+ }
+
+ /* If we've already been called, just be patient. */
+ if (tid->bar_wait)
+ return;
+
+ /* Wait! */
+ tid->bar_wait = 1;
+
+ /* Only one pause, no matter how many frames fail */
+ ath_tx_tid_pause(sc, tid);
+}
+
+/*
+ * We've finished with BAR handling - either we succeeded or
+ * failed. Either way, unsuspend TX.
+ */
+static void
+ath_tx_tid_bar_unsuspend(struct ath_softc *sc, struct ath_tid *tid)
+{
+ ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]);
+
+ device_printf(sc->sc_dev, "%s: tid=%p, called\n", __func__, tid);
+
+ if (tid->bar_tx == 0 || tid->bar_wait == 0) {
+ device_printf(sc->sc_dev, "%s: bar_tx=%d, bar_wait=%d: ?\n",
+ __func__, tid->bar_tx, tid->bar_wait);
+ }
+
+ tid->bar_tx = tid->bar_wait = 0;
+ ath_tx_tid_resume(sc, tid);
+}
+
+/*
+ * Return whether we're ready to TX a BAR frame.
+ *
+ * Requires the TID lock be held.
+ */
+static int
+ath_tx_tid_bar_tx_ready(struct ath_softc *sc, struct ath_tid *tid)
+{
+
+ ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]);
+
+ if (tid->bar_wait == 0 || tid->hwq_depth > 0)
+ return (0);
+
+ return (1);
+}
+
+/*
+ * Check whether the current TID is ready to have a BAR
+ * TXed and if so, do the TX.
+ *
+ * Since the TID/TXQ lock can't be held during a call to
+ * ieee80211_send_bar(), we have to do the dirty thing of unlocking it,
+ * sending the BAR and locking it again.
+ *
+ * Eventually, the code to send the BAR should be broken out
+ * from this routine so the lock doesn't have to be reacquired
+ * just to be immediately dropped by the caller.
+ */
+static void
+ath_tx_tid_bar_tx(struct ath_softc *sc, struct ath_tid *tid)
+{
+ struct ieee80211_tx_ampdu *tap;
+
+ ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]);
+
+ device_printf(sc->sc_dev, "%s: tid=%p, called\n", __func__, tid);
+
+ tap = ath_tx_get_tx_tid(tid->an, tid->tid);
+
+ /*
+ * This is an error condition!
+ */
+ if (tid->bar_wait == 0 || tid->bar_tx == 1) {
+ device_printf(sc->sc_dev,
+ "%s: tid=%p, bar_tx=%d, bar_wait=%d: ?\n",
+ __func__,
+ tid,
+ tid->bar_tx,
+ tid->bar_wait);
+ return;
+ }
+
+ /* Don't do anything if we still have pending frames */
+ if (tid->hwq_depth > 0) {
+ device_printf(sc->sc_dev,
+ "%s: tid=%p, hwq_depth=%d, waiting\n",
+ __func__,
+ tid,
+ tid->hwq_depth);
+ return;
+ }
+
+ /* We're now about to TX */
+ tid->bar_tx = 1;
+
+ /*
+ * Calculate new BAW left edge, now that all frames have either
+ * succeeded or failed.
+ *
+ * XXX verify this is _actually_ the valid value to begin at!
+ */
+ device_printf(sc->sc_dev, "%s: tid=%p, new BAW left edge=%d\n",
+ __func__, tid, tap->txa_start);
+
+ /* Try sending the BAR frame */
+ /* We can't hold the lock here! */
+
+ ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]);
+ if (ieee80211_send_bar(&tid->an->an_node, tap, tap->txa_start) == 0) {
+ /* Success? Now we wait for notification that it's done */
+ ATH_TXQ_LOCK(sc->sc_ac2q[tid->ac]);
+ return;
+ }
+
+ /* Failure? For now, warn loudly and continue */
+ ATH_TXQ_LOCK(sc->sc_ac2q[tid->ac]);
+ device_printf(sc->sc_dev, "%s: tid=%p, failed to TX BAR, continue!\n",
+ __func__, tid);
+ ath_tx_tid_bar_unsuspend(sc, tid);
+}
+
+
+/*
* Free any packets currently pending in the software TX queue.
*
* This will be called when a node is being deleted.
@@ -3076,7 +3216,6 @@
int tid = bf->bf_state.bfs_tid;
struct ath_tid *atid = &an->an_tid[tid];
struct ieee80211_tx_ampdu *tap;
- int txseq;
ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]);
@@ -3117,18 +3256,14 @@
}
bf->bf_state.bfs_dobaw = 0;
- /* Send BAR frame */
- /*
- * This'll end up going into net80211 and back out
- * again, via ic->ic_raw_xmit().
- */
- txseq = tap->txa_start;
- ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
+ /* Suspend the TX queue and get ready to send the BAR */
+ ath_tx_tid_bar_suspend(sc, atid);
- device_printf(sc->sc_dev,
- "%s: TID %d: send BAR; seq %d\n", __func__, tid, txseq);
+ /* Send the BAR if there are no other frames waiting */
+ if (ath_tx_tid_bar_tx_ready(sc, atid))
+ ath_tx_tid_bar_tx(sc, atid);
- /* XXX TODO: send BAR */
+ ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
/* Free buffer, bf is free after this call */
ath_tx_default_comp(sc, bf, 0);
@@ -3148,6 +3283,9 @@
*/
ATH_TXQ_INSERT_HEAD(atid, bf, bf_list);
ath_tx_tid_sched(sc, atid);
+ /* Send the BAR if there are no other frames waiting */
+ if (ath_tx_tid_bar_tx_ready(sc, atid))
+ ath_tx_tid_bar_tx(sc, atid);
ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
}
@@ -3277,17 +3415,20 @@
* in the ifnet TX context or raw TX context.)
*/
if (drops) {
- int txseq = tap->txa_start;
- ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]);
- device_printf(sc->sc_dev,
- "%s: TID %d: send BAR; seq %d\n",
- __func__, tid->tid, txseq);
-
- /* XXX TODO: send BAR */
- } else {
- ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]);
+ /* Suspend the TX queue and get ready to send the BAR */
+ ath_tx_tid_bar_suspend(sc, tid);
}
+ ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]);
+
+ /*
+ * Send BAR if required
+ */
+ ATH_TXQ_LOCK(sc->sc_ac2q[tid->ac]);
+ if (ath_tx_tid_bar_tx_ready(sc, tid))
+ ath_tx_tid_bar_tx(sc, tid);
+ ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]);
+
/* Complete frames which errored out */
while ((bf = TAILQ_FIRST(&bf_cq)) != NULL) {
TAILQ_REMOVE(&bf_cq, bf, bf_list);
@@ -3327,6 +3468,10 @@
atid->cleanup_inprogress = 0;
ath_tx_tid_resume(sc, atid);
}
+
+ /* Send BAR if required */
+ if (ath_tx_tid_bar_tx_ready(sc, atid))
+ ath_tx_tid_bar_tx(sc, atid);
ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
/* Handle frame completion */
@@ -3541,9 +3686,10 @@
* send bar if we dropped any frames
*/
if (drops) {
- device_printf(sc->sc_dev,
- "%s: TID %d: send BAR; seq %d\n", __func__, tid, txseq);
- /* XXX TODO: send BAR */
+ /* Suspend the TX queue and get ready to send the BAR */
+ ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]);
+ ath_tx_tid_bar_suspend(sc, atid);
+ ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
}
/* Prepend all frames to the beginning of the queue */
@@ -3558,6 +3704,14 @@
DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR,
"%s: txa_start now %d\n", __func__, tap->txa_start);
+ /*
+ * Send BAR if required
+ */
+ ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]);
+ if (ath_tx_tid_bar_tx_ready(sc, atid))
+ ath_tx_tid_bar_tx(sc, atid);
+ ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
+
/* Do deferred completion */
while ((bf = TAILQ_FIRST(&bf_cq)) != NULL) {
TAILQ_REMOVE(&bf_cq, bf, bf_list);
@@ -3651,6 +3805,12 @@
__func__, SEQNO(bf->bf_state.bfs_seqno));
}
+ /*
+ * Send BAR if required
+ */
+ if (ath_tx_tid_bar_tx_ready(sc, atid))
+ ath_tx_tid_bar_tx(sc, atid);
+
ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
ath_tx_default_comp(sc, bf, fail);
@@ -4079,7 +4239,9 @@
* it'll be "after" the left edge of the BAW and thus it'll
* fall within it.
*/
+ ATH_TXQ_LOCK(sc->sc_ac2q[atid->tid]);
ath_tx_tid_pause(sc, atid);
+ ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->tid]);
DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL,
"%s: called; dialogtoken=%d, baparamset=%d, batimeout=%d\n",
@@ -4165,7 +4327,9 @@
DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: called\n", __func__);
/* Pause TID traffic early, so there aren't any races */
+ ATH_TXQ_LOCK(sc->sc_ac2q[atid->tid]);
ath_tx_tid_pause(sc, atid);
+ ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->tid]);
/* There's no need to hold the TXQ lock here */
sc->sc_addba_stop(ni, tap);
@@ -4212,7 +4376,7 @@
*/
if (status == 0 || attempts == 50) {
ATH_TXQ_LOCK(sc->sc_ac2q[atid->ac]);
- ath_tx_tid_resume(sc, atid);
+ ath_tx_tid_bar_unsuspend(sc, atid);
ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
}
}
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmonZZZoY5iRDGLrtNjFLy5Fc5NajhkfdqSzi=s4SF2n5rQ>