The branch main has been updated by adrian:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=e8de31caceaa36caf5d7b4355072f148e2433b82

commit e8de31caceaa36caf5d7b4355072f148e2433b82
Author:     Adrian Chadd <[email protected]>
AuthorDate: 2022-04-12 20:20:28 +0000
Commit:     Adrian Chadd <[email protected]>
CommitDate: 2022-04-22 05:49:01 +0000

    net80211: Fix traffic hang on STA/AP VAPs on a multi-VAP interface
    
    This took an embarrasingly long time to find.
    
    The state changes for a radio with a STA /and/ AP VAP gets a bit messy.
    The AP maps are marked as waiting, waiting for the STA AP to find a
    channel to use before the AP VAPs become active.
    
    However, the code path that clears the OACTIVE flag on a VAP only runs
    during a successful run of ieee80211_newstate_cb().
    
    So here is how it goes:
    
    * the STA VAP goes down and needs to scan;
    * the AP vap goes RUN->INIT; but it doesn't YET call 
ieee80211_newstate_cb();
    * meanwhile - a send on the AP VAP causes the VAP to set the OACTIVE flag 
here;
    * then the STA VAP finishes scan and goes to RUN;
    * which will call wakeupwaiting() as part of the STA VAP transition to RUN;
    * .. then the AP VAP goes INIT->RUN directly via a call to hostap_newstate
      in wakeupwaiting rather than it being through the deferred path;
    * /then/ the ieee80211_newstate_cb() is called, but it sees the state go
      RUN->RUN;
    * .. which results in the OACTIVE flag never being cleared.
    
    This clears the OACTIVE flag when a VAP transitions RUN->RUN; the
    driver layer or net80211 layer can set it if required in a subsequent
    transmit.
    
    Differential Revision: https://reviews.freebsd.org/D34920
    
    Reviewed by: bz
---
 sys/net80211/ieee80211_proto.c | 47 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/sys/net80211/ieee80211_proto.c b/sys/net80211/ieee80211_proto.c
index 2228983050a2..d2bde99ce79c 100644
--- a/sys/net80211/ieee80211_proto.c
+++ b/sys/net80211/ieee80211_proto.c
@@ -2469,6 +2469,29 @@ wakeupwaiting(struct ieee80211vap *vap0)
                        vap->iv_flags_ext &= ~IEEE80211_FEXT_SCANWAIT;
                        /* NB: sta's cannot go INIT->RUN */
                        /* NB: iv_newstate may drop the lock */
+
+                       /*
+                        * This is problematic if the interface has OACTIVE
+                        * set.  Only the deferred ieee80211_newstate_cb()
+                        * will end up actually /clearing/ the OACTIVE
+                        * flag on a state transition to RUN from a non-RUN
+                        * state.
+                        *
+                        * But, we're not actually deferring this callback;
+                        * and when the deferred call occurs it shows up as
+                        * a RUN->RUN transition!  So the flag isn't/wasn't
+                        * cleared!
+                        *
+                        * I'm also not sure if it's correct to actually
+                        * do the transitions here fully through the deferred
+                        * paths either as other things can be invoked as
+                        * part of that state machine.
+                        *
+                        * So just keep this in mind when looking at what
+                        * the markwaiting/wakeupwaiting routines are doing
+                        * and how they invoke vap state changes.
+                        */
+
                        vap->iv_newstate(vap,
                            vap->iv_opmode == IEEE80211_M_STA ?
                                IEEE80211_S_SCAN : IEEE80211_S_RUN, 0);
@@ -2543,6 +2566,30 @@ ieee80211_newstate_cb(void *xvap, int npending)
                goto done;
        }
 
+       /*
+        * Handle the case of a RUN->RUN transition occuring when STA + AP
+        * VAPs occur on the same radio.
+        *
+        * The mark and wakeup waiting routines call iv_newstate() directly,
+        * but they do not end up deferring state changes here.
+        * Thus, although the VAP newstate method sees a transition
+        * of RUN->INIT->RUN, the deferred path here only sees a RUN->RUN
+        * transition.  If OACTIVE is set then it is never cleared.
+        *
+        * So, if we're here and the state is RUN, just clear OACTIVE.
+        * At some point if the markwaiting/wakeupwaiting paths end up
+        * also invoking the deferred state updates then this will
+        * be no-op code - and also if OACTIVE is finally retired, it'll
+        * also be no-op code.
+        */
+       if (nstate == IEEE80211_S_RUN) {
+               /*
+                * Unblock the VAP queue; a RUN->RUN state can happen
+                * on a STA+AP setup on the AP vap.  See wakeupwaiting().
+                */
+               vap->iv_ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
+       }
+
        /* No actual transition, skip post processing */
        if (ostate == nstate)
                goto done;

Reply via email to