Hi,

For those who are testing out 802.11n development - please update and
test this for me.  This work ties the software TX mechanism into the
net80211 node power state, so whenever a wireless node sends a "going
to sleep!" frame, the ath driver pauses TX. Same with wakeup/resume.

Devices which are doing ps-poll will still be badly broken,
unfortunately. And I have to fix TIM handling so the TIM bitmap takes
the power save queue _and_ the driver software queue into account when
deciding if to set or clear the TIM bitmap. That, and CABQ + MORE data
bit stuff being set.

Please report any issues you may have. The first thing to do is to
turn on power save (wlandebug +power) and see if it's going in/out of
powersave, along with whether it mentions ps-poll or not. I'm going to
address ps-poll but first everything else has to work (TIM handling,
software queue notification and CABQ/MORE data.)

Thanks,



Adrian

---------- Forwarded message ----------
From: Adrian Chadd <adr...@freebsd.org>
Date: 3 October 2012 16:23
Subject: svn commit: r241170 - head/sys/dev/ath
To: src-committ...@freebsd.org, svn-src-...@freebsd.org,
svn-src-h...@freebsd.org


Author: adrian
Date: Wed Oct  3 23:23:45 2012
New Revision: 241170
URL: http://svn.freebsd.org/changeset/base/241170

Log:
  Pause and unpause the software queues for a given node based on the
  net80211 node power save state.

  * Add an ATH_NODE_UNLOCK_ASSERT() check
  * Add a new node field - an_is_powersave
  * Pause/unpause the queue based on the node state
  * Attempt to handle net80211 concurrency issues so the queue
    doesn't get paused/unpaused more than once at a time from
    the net80211 power save code.

  Whilst here (and breaking my usual rule), set CLRDMASK when a queue
  is unpaused, regardless of whether the queue has some pending traffic.
  This means the first frame from that TID (now or later) will hvae
  CLRDMASK set.

  Also whilst here, bump the swretrymax counters whenever the
  filtered frames code expires a frame.  Again, breaking my rule, but
  this is just a statistics thing rather than a functional change.

  This doesn't fix ps-poll (but it doesn't break it too much worse
  than it is at the present) or correcting the TID updates.
  That's next on the list.

  Tested:
        * AR9220 AP (Atheros AP96 reference design)
        * Macbook Pro and LG Optimus 1 Android phone, both setting
          and clearing power save state (but not using PS-POLL.)

Modified:
  head/sys/dev/ath/if_ath.c
  head/sys/dev/ath/if_ath_debug.h
  head/sys/dev/ath/if_ath_tx.c
  head/sys/dev/ath/if_ath_tx.h
  head/sys/dev/ath/if_athvar.h

Modified: head/sys/dev/ath/if_ath.c
==============================================================================
--- head/sys/dev/ath/if_ath.c   Wed Oct  3 22:02:16 2012        (r241169)
+++ head/sys/dev/ath/if_ath.c   Wed Oct  3 23:23:45 2012        (r241170)
@@ -199,6 +199,7 @@ static void ath_setcurmode(struct ath_so
 static void    ath_announce(struct ath_softc *);

 static void    ath_dfs_tasklet(void *, int);
+static void    ath_node_powersave(struct ieee80211_node *, int);

 #ifdef IEEE80211_SUPPORT_TDMA
 #include <dev/ath/if_ath_tdma.h>
@@ -1138,6 +1139,9 @@ ath_vap_create(struct ieee80211com *ic,
        avp->av_bmiss = vap->iv_bmiss;
        vap->iv_bmiss = ath_bmiss_vap;

+       avp->av_node_ps = vap->iv_node_ps;
+       vap->iv_node_ps = ath_node_powersave;
+
        /* Set default parameters */

        /*
@@ -5335,6 +5339,37 @@ ath_dfs_tasklet(void *p, int npending)
        }
 }

+/*
+ * Enable/disable power save.  This must be called with
+ * no TX driver locks currently held, so it should only
+ * be called from the RX path (which doesn't hold any
+ * TX driver locks.)
+ */
+static void
+ath_node_powersave(struct ieee80211_node *ni, int enable)
+{
+       struct ath_node *an = ATH_NODE(ni);
+       struct ieee80211com *ic = ni->ni_ic;
+       struct ath_softc *sc = ic->ic_ifp->if_softc;
+       struct ath_vap *avp = ATH_VAP(ni->ni_vap);
+
+       ATH_NODE_UNLOCK_ASSERT(an);
+       /* XXX and no TXQ locks should be held here */
+
+       DPRINTF(sc, ATH_DEBUG_NODE_PWRSAVE, "%s: ni=%p, enable=%d\n",
+           __func__, ni, enable);
+
+       /* Suspend or resume software queue handling */
+       if (enable)
+               ath_tx_node_sleep(sc, an);
+       else
+               ath_tx_node_wakeup(sc, an);
+
+       /* Update net80211 state */
+       avp->av_node_ps(ni, enable);
+}
+
+
 MODULE_VERSION(if_ath, 1);
 MODULE_DEPEND(if_ath, wlan, 1, 1, 1);          /* 802.11 media layer */
 #if    defined(IEEE80211_ALQ) || defined(AH_DEBUG_ALQ)

Modified: head/sys/dev/ath/if_ath_debug.h
==============================================================================
--- head/sys/dev/ath/if_ath_debug.h     Wed Oct  3 22:02:16 2012
 (r241169)
+++ head/sys/dev/ath/if_ath_debug.h     Wed Oct  3 23:23:45 2012
 (r241170)
@@ -66,6 +66,7 @@ enum {
        ATH_DEBUG_SW_TX_BAR     = 0x100000000ULL,       /* BAR TX */
        ATH_DEBUG_EDMA_RX       = 0x200000000ULL,       /* RX EDMA state */
        ATH_DEBUG_SW_TX_FILT    = 0x400000000ULL,       /* SW TX FF */
+       ATH_DEBUG_NODE_PWRSAVE  = 0x800000000ULL,       /* node powersave */

        ATH_DEBUG_ANY           = 0xffffffffffffffffULL
 };

Modified: head/sys/dev/ath/if_ath_tx.c
==============================================================================
--- head/sys/dev/ath/if_ath_tx.c        Wed Oct  3 22:02:16 2012
 (r241169)
+++ head/sys/dev/ath/if_ath_tx.c        Wed Oct  3 23:23:45 2012
 (r241170)
@@ -111,6 +111,9 @@ __FBSDID("$FreeBSD$");
  */
 #define        ATH_NONQOS_TID_AC       WME_AC_VO

+#if 0
+static int ath_tx_node_is_asleep(struct ath_softc *sc, struct ath_node *an);
+#endif
 static int ath_tx_ampdu_pending(struct ath_softc *sc, struct ath_node *an,
     int tid);
 static int ath_tx_ampdu_running(struct ath_softc *sc, struct ath_node *an,
@@ -2902,9 +2905,17 @@ ath_tx_tid_resume(struct ath_softc *sc,
        DPRINTF(sc, ATH_DEBUG_SW_TX_CTRL, "%s: unpaused = %d\n",
            __func__, tid->paused);

-       if (tid->paused || tid->axq_depth == 0) {
+       if (tid->paused)
+               return;
+
+       /*
+        * Override the clrdmask configuration for the next frame
+        * from this TID, just to get the ball rolling.
+        */
+       tid->clrdmask = 1;
+
+       if (tid->axq_depth == 0)
                return;
-       }

        /* XXX isfiltered shouldn't ever be 0 at this point */
        if (tid->isfiltered == 1) {
@@ -2912,12 +2923,6 @@ ath_tx_tid_resume(struct ath_softc *sc,
                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]);
@@ -3021,6 +3026,7 @@ ath_tx_tid_filt_comp_single(struct ath_s
         * Don't allow a filtered frame to live forever.
         */
        if (bf->bf_state.bfs_retries > SWMAX_RETRIES) {
+               sc->sc_stats.ast_tx_swretrymax++;
                DPRINTF(sc, ATH_DEBUG_SW_TX_FILT,
                    "%s: bf=%p, seqno=%d, exceeded retries\n",
                    __func__,
@@ -3073,6 +3079,7 @@ ath_tx_tid_filt_comp_aggr(struct ath_sof
                 * Don't allow a filtered frame to live forever.
                 */
                if (bf->bf_state.bfs_retries > SWMAX_RETRIES) {
+               sc->sc_stats.ast_tx_swretrymax++;
                        DPRINTF(sc, ATH_DEBUG_SW_TX_FILT,
                            "%s: bf=%p, seqno=%d, exceeded retries\n",
                            __func__,
@@ -5282,6 +5289,145 @@ ath_addba_response_timeout(struct ieee80
        ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]);
 }

+#if 0
+/*
+ * Check if a node is asleep or not.
+ */
+static int
+ath_tx_node_is_asleep(struct ath_softc *sc, struct ath_node *an)
+{
+
+       ATH_NODE_LOCK_ASSERT(an);
+
+       return (an->an_is_powersave);
+}
+#endif
+
+/*
+ * Mark a node as currently "in powersaving."
+ * This suspends all traffic on the node.
+ *
+ * This must be called with the node/tx locks free.
+ *
+ * XXX TODO: the locking silliness below is due to how the node
+ * locking currently works.  Right now, the node lock is grabbed
+ * to do rate control lookups and these are done with the TX
+ * queue lock held.  This means the node lock can't be grabbed
+ * first here or a LOR will occur.
+ *
+ * Eventually (hopefully!) the TX path code will only grab
+ * the TXQ lock when transmitting and the ath_node lock when
+ * doing node/TID operations.  There are other complications -
+ * the sched/unsched operations involve walking the per-txq
+ * 'active tid' list and this requires both locks to be held.
+ */
+void
+ath_tx_node_sleep(struct ath_softc *sc, struct ath_node *an)
+{
+       struct ath_tid *atid;
+       struct ath_txq *txq;
+       int tid;
+
+       ATH_NODE_UNLOCK_ASSERT(an);
+
+       /*
+        * It's possible that a parallel call to ath_tx_node_wakeup()
+        * will unpause these queues.
+        *
+        * The node lock can't just be grabbed here, as there's places
+        * in the driver where the node lock is grabbed _within_ a
+        * TXQ lock.
+        * So, we do this delicately and unwind state if needed.
+        *
+        * + Pause all the queues
+        * + Grab the node lock
+        * + If the queue is already asleep, unpause and quit
+        * + else just mark as asleep.
+        *
+        * A parallel sleep() call will just pause and then
+        * find they're already paused, so undo it.
+        *
+        * A parallel wakeup() call will check if asleep is 1
+        * and if it's not (ie, it's 0), it'll treat it as already
+        * being awake. If it's 1, it'll mark it as 0 and then
+        * unpause everything.
+        *
+        * (Talk about a delicate hack.)
+        */
+
+       /* Suspend all traffic on the node */
+       for (tid = 0; tid < IEEE80211_TID_SIZE; tid++) {
+               atid = &an->an_tid[tid];
+               txq = sc->sc_ac2q[atid->ac];
+
+               ATH_TXQ_LOCK(txq);
+               ath_tx_tid_pause(sc, atid);
+               ATH_TXQ_UNLOCK(txq);
+       }
+
+       ATH_NODE_LOCK(an);
+
+       /* In case of concurrency races from net80211.. */
+       if (an->an_is_powersave == 1) {
+               ATH_NODE_UNLOCK(an);
+               device_printf(sc->sc_dev,
+                   "%s: an=%p: node was already asleep\n",
+                   __func__, an);
+               for (tid = 0; tid < IEEE80211_TID_SIZE; tid++) {
+                       atid = &an->an_tid[tid];
+                       txq = sc->sc_ac2q[atid->ac];
+
+                       ATH_TXQ_LOCK(txq);
+                       ath_tx_tid_resume(sc, atid);
+                       ATH_TXQ_UNLOCK(txq);
+               }
+               return;
+       }
+
+       /* Mark node as in powersaving */
+       an->an_is_powersave = 1;
+
+       ATH_NODE_UNLOCK(an);
+}
+
+/*
+ * Mark a node as currently "awake."
+ * This resumes all traffic to the node.
+ */
+void
+ath_tx_node_wakeup(struct ath_softc *sc, struct ath_node *an)
+{
+       struct ath_tid *atid;
+       struct ath_txq *txq;
+       int tid;
+
+       ATH_NODE_UNLOCK_ASSERT(an);
+       ATH_NODE_LOCK(an);
+
+       /* In case of concurrency races from net80211.. */
+       if (an->an_is_powersave == 0) {
+               ATH_NODE_UNLOCK(an);
+               device_printf(sc->sc_dev,
+                   "%s: an=%p: node was already awake\n",
+                   __func__, an);
+               return;
+       }
+
+       /* Mark node as awake */
+       an->an_is_powersave = 0;
+
+       ATH_NODE_UNLOCK(an);
+
+       for (tid = 0; tid < IEEE80211_TID_SIZE; tid++) {
+               atid = &an->an_tid[tid];
+               txq = sc->sc_ac2q[atid->ac];
+
+               ATH_TXQ_LOCK(txq);
+               ath_tx_tid_resume(sc, atid);
+               ATH_TXQ_UNLOCK(txq);
+       }
+}
+
 static int
 ath_legacy_dma_txsetup(struct ath_softc *sc)
 {

Modified: head/sys/dev/ath/if_ath_tx.h
==============================================================================
--- head/sys/dev/ath/if_ath_tx.h        Wed Oct  3 22:02:16 2012
 (r241169)
+++ head/sys/dev/ath/if_ath_tx.h        Wed Oct  3 23:23:45 2012
 (r241170)
@@ -124,6 +124,12 @@ extern     void ath_addba_response_timeout(s
     struct ieee80211_tx_ampdu *tap);

 /*
+ * AP mode power save handling (of stations)
+ */
+extern void ath_tx_node_sleep(struct ath_softc *sc, struct ath_node *an);
+extern void ath_tx_node_wakeup(struct ath_softc *sc, struct ath_node *an);
+
+/*
  * Setup path
  */
 #define        ath_txdma_setup(_sc)                    \

Modified: head/sys/dev/ath/if_athvar.h
==============================================================================
--- head/sys/dev/ath/if_athvar.h        Wed Oct  3 22:02:16 2012
 (r241169)
+++ head/sys/dev/ath/if_athvar.h        Wed Oct  3 23:23:45 2012
 (r241170)
@@ -170,6 +170,7 @@ struct ath_node {
        struct ieee80211_node an_node;  /* base class */
        u_int8_t        an_mgmtrix;     /* min h/w rate index */
        u_int8_t        an_mcastrix;    /* mcast h/w rate index */
+       uint32_t        an_is_powersave;        /* node is sleeping */
        struct ath_buf  *an_ff_buf[WME_NUM_AC]; /* ff staging area */
        struct ath_tid  an_tid[IEEE80211_TID_SIZE];     /* per-TID state */
        char            an_name[32];    /* eg "wlan0_a1" */
@@ -332,6 +333,8 @@ struct ath_txq {
 #define        ATH_NODE_LOCK(_an)              mtx_lock(&(_an)->an_mtx)
 #define        ATH_NODE_UNLOCK(_an)            mtx_unlock(&(_an)->an_mtx)
 #define        ATH_NODE_LOCK_ASSERT(_an)
mtx_assert(&(_an)->an_mtx, MA_OWNED)
+#define        ATH_NODE_UNLOCK_ASSERT(_an)
mtx_assert(&(_an)->an_mtx,      \
+                                           MA_NOTOWNED)

 #define        ATH_TXQ_LOCK_INIT(_sc, _tq) do { \
        snprintf((_tq)->axq_name, sizeof((_tq)->axq_name), "%s_txq%u", \
@@ -379,6 +382,7 @@ struct ath_vap {
        int             (*av_newstate)(struct ieee80211vap *,
                                enum ieee80211_state, int);
        void            (*av_bmiss)(struct ieee80211vap *);
+       void            (*av_node_ps)(struct ieee80211_node *, int);
 };
 #define        ATH_VAP(vap)    ((struct ath_vap *)(vap))
_______________________________________________
freebsd-wireless@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"

Reply via email to