svn commit: r309223 - head/sys/dev/ath

[Adrian Chadd]

Author: adrian
Date: Mon Nov 28 02:59:33 2016
New Revision: 309223
URL: https://svnweb.freebsd.org/changeset/base/309223

Log:
  [ath] fix target beacon interval programming for STA mode when in powersave.
  
  This bug has been bugging me for quite some time.  I finally sat down
  with enough coffee to figure it out.
  
  The short of it - rounding up to the next intval multiple of the TSF value
  only works if the AP is transmitting all its beacons on an interval of
  the TSF.  If it isn't - for example, doing staggered beacons on a multi-VAP
  setup with a single hardware TSF - then weird things occur.
  
  The long of it -
  
  When powersave is enabled, the MAC and PHY are partially powered off.
  They can't receive any packets (or transmit, for that matter.)
  The target beacon timer programming will wake up the MAC/PHY just before
  the beacon is supposed to be received (well, strictly speaking, at DTIM
  so it can see the TIM - traffic information map - telling the STA whether
  any traffic is there for it) and it happens automatically.
  
  However, this relies on the target beacon time being programmed correctly.
  If it isn't then the hardware will wake up and not hear any beacons -
  and then it'll be asleep for said beacons.  After enough of this, net80211
  will give up and assume the AP went away.
  
  This should fix both TSFOOR interrupts and disconnects from APs with powersave
  enabled.
  
  The annoying bit is that it only happens if APs stagger things or start
  on a non-zero TSF.  So, this would sometimes be fine and sometimes not be
  fine.
  
  What:
  
  * I don't know (yet) why the code rounds up to the next intval.
    For now, just disable rounding it and trust the value we get.
  
  TODO:
  
  * If we do see a beacon miss in STA mode then we should transition
    out of sleep for a while so we can hear beacons to resync against.
    I'd love a patch from someone to enable that particular behaviour.
    Note - that doesn't require that net80211 brings the chip out of
    sleep state - only that we wake the chip up through to full-on and
    then let it go to sleep again when we've seen a beacon.  The wifi
    stack and AP can still completely just stay believing we're in sleep
    mode.
  
  Tested:
  
  * AR9485, STA mode, powersave enabled
  
  MFC after:	1 week
  Relnotes:	Yes

Modified:
  head/sys/dev/ath/if_ath_beacon.c

Modified: head/sys/dev/ath/if_ath_beacon.c
==============================================================================
--- head/sys/dev/ath/if_ath_beacon.c	Mon Nov 28 02:51:55 2016	(r309222)
+++ head/sys/dev/ath/if_ath_beacon.c	Mon Nov 28 02:59:33 2016	(r309223)
@@ -964,10 +964,30 @@ ath_beacon_config(struct ath_softc *sc, 
 		/* NB: the beacon interval is kept internally in TU's */
 		intval = ni->ni_intval & HAL_BEACON_PERIOD;
 	}
+
+
+	/*
+	 * Note: rounding up to the next intval can cause problems.
+	 *
+	 * In STA mode with powersave enabled, beacons are only received
+	 * whenever the beacon timer fires to wake up the hardware.
+	 * Now, if this is rounded up to the next intval, it assumes
+	 * that the AP has started transmitting beacons at TSF values that
+	 * are multiples of intval, versus say being 25 TU off.
+	 *
+	 * I'm not sure why nexttbtt is rounded up to the intval.
+	 * If we sync against a beacon that is way out, we should
+	 * take a beacon miss and re-sync against the next beacon.
+	 *
+	 * So for now - don't round up if we're in STA mode.
+	 * Maybe later (when someone eventually does powersave+IBSS,
+	 * powersave+MBSS) this can be flipped on for those too.
+	 */
 	if (nexttbtt == 0)		/* e.g. for ap mode */
 		nexttbtt = intval;
-	else if (intval)		/* NB: can be 0 for monitor mode */
+	else if ((ic->ic_opmode != IEEE80211_M_STA) && intval)		/* NB: can be 0 for monitor mode */
 		nexttbtt = roundup(nexttbtt, intval);
+
 	DPRINTF(sc, ATH_DEBUG_BEACON, "%s: nexttbtt %u intval %u (%u)\n",
 		__func__, nexttbtt, intval, ni->ni_intval);
 	if (ic->ic_opmode == IEEE80211_M_STA && !sc->sc_swbmiss) {