From f858e9281c60d0730ab20ed94eef5c52e66795ee Mon Sep 17 00:00:00 2001 From: Adrian Chadd Date: Sat, 22 May 2021 16:39:16 -0700 Subject: [PATCH] [ath] Handle STA + AP beacon programming without stomping over HW AP beacon programming I've been using STA+AP modes at home for a couple years now and I've been finding and fixing a lot of weird corner cases. This is the eventual patchset I've landed on. * Don't force beacon resync in STA mode if we're using sw beacon tracking. This stops a variety of stomping issues when the STA VAP is reconfigured; the AP hardware beacons were being stomped on! * Use the first AP VAP to configure beacons on, rather than the first VAP. This prevents weird behaviour in ath_beacon_config() when the hardware is being reconfigured and the STA VAP was the first one created. * Ensure the beacon interval / timing programming is within the AR9300 HAL bounds by masking off any flags that may have been there before shifting the value up to 1/8 TUs rather than the 1 TU resolution the previous chips used. Now I don't get weird beacon reprogramming during startup, STA state changes and hardware recovery which showed up as HI-LARIOUS beacon configurations and STAs that would just disconnect from the AP very frequently. Tested: * AR9344/AR9380, STA and AP and STA+AP modes --- sys/dev/ath/if_ath.c | 34 ++++++++++++++++---------- sys/dev/ath/if_ath_beacon.c | 48 +++++++++++++++++++++++++++++-------- sys/dev/ath/if_ath_rx.c | 1 + 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/sys/dev/ath/if_ath.c b/sys/dev/ath/if_ath.c index 698b8d1a2853..4bc5c31812e7 100644 --- a/sys/dev/ath/if_ath.c +++ b/sys/dev/ath/if_ath.c @@ -2450,13 +2450,15 @@ ath_bmiss_vap(struct ieee80211vap *vap) ath_power_setpower(sc, HAL_PM_AWAKE, 0); ath_power_restore_power_state(sc); ATH_UNLOCK(sc); + DPRINTF(sc, ATH_DEBUG_BEACON, "%s: forced awake; force syncbeacon=1\n", __func__); - - /* - * Attempt to force a beacon resync. - */ - sc->sc_syncbeacon = 1; + if ((vap->iv_flags_ext & IEEE80211_FEXT_SWBMISS) == 0) { + /* + * Attempt to force a beacon resync. + */ + sc->sc_syncbeacon = 1; + } ATH_VAP(vap)->av_bmiss(vap); } @@ -6061,19 +6063,25 @@ ath_newstate(struct ieee80211vap *vap, enum ieee80211_state nstate, int arg) * In that case, we may not receive an actual * beacon to update the beacon timer and thus we * won't get notified of the missing beacons. + * + * Also, don't do any of this if we're not running + * with hardware beacon support, as that'll interfere + * with an AP VAP. */ if (ostate != IEEE80211_S_RUN && ostate != IEEE80211_S_SLEEP) { - DPRINTF(sc, ATH_DEBUG_BEACON, - "%s: STA; syncbeacon=1\n", __func__); - sc->sc_syncbeacon = 1; + + if ((vap->iv_flags_ext & IEEE80211_FEXT_SWBMISS) == 0) { + DPRINTF(sc, ATH_DEBUG_BEACON, + "%s: STA; syncbeacon=1\n", __func__); + sc->sc_syncbeacon = 1; + if (csa_run_transition) + ath_beacon_config(sc, vap); + } /* Quiet time handling - ensure we resync */ memset(&avp->quiet_ie, 0, sizeof(avp->quiet_ie)); - if (csa_run_transition) - ath_beacon_config(sc, vap); - /* * PR: kern/175227 * @@ -6086,7 +6094,9 @@ ath_newstate(struct ieee80211vap *vap, enum ieee80211_state nstate, int arg) * timer fires (too often), leading to a STA * disassociation. */ - sc->sc_beacons = 1; + if ((vap->iv_flags_ext & IEEE80211_FEXT_SWBMISS) == 0) { + sc->sc_beacons = 1; + } } break; case IEEE80211_M_MONITOR: diff --git a/sys/dev/ath/if_ath_beacon.c b/sys/dev/ath/if_ath_beacon.c index ac1244c5f8e7..6da36a176319 100644 --- a/sys/dev/ath/if_ath_beacon.c +++ b/sys/dev/ath/if_ath_beacon.c @@ -448,9 +448,11 @@ ath_beacon_proc(void *arg, int pending) * many consecutive beacons reset the device. */ if (ath_hal_numtxpending(ah, sc->sc_bhalq) != 0) { + sc->sc_bmisscount++; sc->sc_stats.ast_be_missed++; ath_beacon_miss(sc); + DPRINTF(sc, ATH_DEBUG_BEACON, "%s: missed %u consecutive beacons\n", __func__, sc->sc_bmisscount); @@ -935,15 +937,33 @@ ath_beacon_config(struct ath_softc *sc, struct ieee80211vap *vap) u_int32_t nexttbtt_u8, intval_u8; u_int64_t tsf, tsf_beacon; - if (vap == NULL) - vap = TAILQ_FIRST(&ic->ic_vaps); /* XXX */ /* - * Just ensure that we aren't being called when the last - * VAP is destroyed. + * Find the first VAP that we /can/ use a beacon configuration for. + * If it's a STA VAP then if it has SWBMISS set we should ignore it. + * + * Yes, ideally we'd not have a STA without SWBMISS followed by an + * AP STA, and yes this isn't ready for P2P/TSF2 logic on AR9300 and + * later chips. */ if (vap == NULL) { - device_printf(sc->sc_dev, "%s: called with no VAPs\n", - __func__); + IEEE80211_LOCK(ic); + TAILQ_FOREACH(vap, &ic->ic_vaps, iv_next) { + /* A STA VAP w/ SWBMISS set can't be used for beaconing */ + if ((vap->iv_opmode == IEEE80211_M_STA) && + ((vap->iv_flags_ext & IEEE80211_FEXT_SWBMISS) != 0)) + continue; + break; + } + IEEE80211_UNLOCK(ic); + } + + if (vap == NULL) { + device_printf(sc->sc_dev, "called with no valid vaps?\n"); + return; + } + + if ((vap->iv_flags_ext & IEEE80211_FEXT_SWBMISS) != 0) { + device_printf(sc->sc_dev, "called on VAP with SWBMISS set?\n"); return; } @@ -1004,8 +1024,7 @@ ath_beacon_config(struct ath_softc *sc, struct ieee80211vap *vap) else 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) { HAL_BEACON_STATE bs; int dtimperiod, dtimcount; @@ -1196,8 +1215,8 @@ ath_beacon_config(struct ath_softc *sc, struct ieee80211vap *vap) * nexttbtt and intval is TU/8. */ if (sc->sc_isedma) { - nexttbtt_u8 = (nexttbtt << 3); - intval_u8 = (intval << 3); + nexttbtt_u8 = (nexttbtt << 3) & HAL_BEACON_PERIOD_TU8; + intval_u8 = (intval << 3) & HAL_BEACON_PERIOD_TU8; if (intval & HAL_BEACON_ENA) intval_u8 |= HAL_BEACON_ENA; if (intval & HAL_BEACON_RESET_TSF) @@ -1216,6 +1235,15 @@ ath_beacon_config(struct ath_softc *sc, struct ieee80211vap *vap) } ieee80211_free_node(ni); + tsf = ath_hal_gettsf64(ah); + DPRINTF(sc, ATH_DEBUG_BEACON, + "%s: nexttbtt %u intval %u (%u), tsf64=%llu tsfbeacon=%llu delta=%lld\n", + __func__, nexttbtt, intval, ni->ni_intval, + (unsigned long long) tsf, + (unsigned long long) tsf_beacon, + (long long) tsf - + (long long) tsf_beacon); + ATH_LOCK(sc); ath_power_restore_power_state(sc); ATH_UNLOCK(sc); diff --git a/sys/dev/ath/if_ath_rx.c b/sys/dev/ath/if_ath_rx.c index 58da21f09638..588194a942db 100644 --- a/sys/dev/ath/if_ath_rx.c +++ b/sys/dev/ath/if_ath_rx.c @@ -463,6 +463,7 @@ ath_recv_mgmt(struct ieee80211_node *ni, struct mbuf *m, sc->sc_syncbeacon && (!sc->sc_swbmiss) && ni == vap->iv_bss && + ((vap->iv_flags_ext & IEEE80211_FEXT_SWBMISS) == 0) && (vap->iv_state == IEEE80211_S_RUN || vap->iv_state == IEEE80211_S_SLEEP)) { DPRINTF(sc, ATH_DEBUG_BEACON, "%s: syncbeacon=1; syncing\n",