Before roaming to another AP we should explicitly stop Rx BA sessions
by sending the appropriate 'ADD_STA' commands to firmware, in addition
to clearing Rx BA buffers. This is similar to a recent change in iwm(4).
See the iwm patch description for testing instructions:
https://marc.info/?l=openbsd-tech&m=163329420019842&w=2

It seems there is no need to stop Tx BA sessions in this driver.
Note that Tx aggregation sessions are handled entirely in firmware on
iwx(4) devices, which is substantially different from iwm(4).
I have already tried sending commands to disable Tx aggregation queues,
like iwm(4) will do now. This causes fatal firmware errors on iwx.
And everything seems to be working fine with Tx queues left enabled.

While here, remove a pointless STA_ACTIVE check; if we are in RUN state
then our firmware station (which represents the AP) is active by definition.

ok?

diff 58be466d62dc3469b7024e02971f96cadae4041e 
2611d0ab4f1e42a3f2c5db88a7a8cf0f1d94ef39
blob - 55d0375ff3f9ff3ec8c1c37fc03de2eb4f9e5bff
blob + 2cb39f6f37c89487e80cae7c423309688622031f
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -7531,19 +7531,34 @@ iwx_run_stop(struct iwx_softc *sc)
 {
        struct ieee80211com *ic = &sc->sc_ic;
        struct iwx_node *in = (void *)ic->ic_bss;
-       int err;
+       struct ieee80211_node *ni = &in->in_ni;
+       int err, i;
 
        splassert(IPL_NET);
 
-       if (sc->sc_flags & IWX_FLAG_STA_ACTIVE) {
-               err = iwx_flush_sta(sc, in);
-               if (err) {
-                       printf("%s: could not flush Tx path (error %d)\n",
-                           DEVNAME(sc), err);
-                       return err;
-               }
+       err = iwx_flush_sta(sc, in);
+       if (err) {
+               printf("%s: could not flush Tx path (error %d)\n",
+                   DEVNAME(sc), err);
+               return err;
        }
 
+       /*
+        * Stop Rx BA sessions now. We cannot rely on the BA task
+        * for this when moving out of RUN state since it runs in a
+        * separate thread.
+        * Note that in->in_ni (struct ieee80211_node) already represents
+        * our new access point in case we are roaming between APs.
+        * This means we cannot rely on struct ieee802111_node to tell
+        * us which BA sessions exist.
+        */
+       for (i = 0; i < nitems(sc->sc_rxba_data); i++) {
+               struct iwx_rxba_data *rxba = &sc->sc_rxba_data[i];
+               if (rxba->baid == IWX_RX_REORDER_DATA_INVALID_BAID)
+                       continue;
+               iwx_sta_rx_agg(sc, ni, rxba->tid, 0, 0, 0, 0);
+       }
+
        err = iwx_sf_config(sc, IWX_SF_INIT_OFF);
        if (err)
                return err;
@@ -7869,7 +7884,6 @@ iwx_newstate(struct ieee80211com *ic, enum ieee80211_s
 {
        struct ifnet *ifp = IC2IFP(ic);
        struct iwx_softc *sc = ifp->if_softc;
-       int i;
 
        /*
         * Prevent attemps to transition towards the same state, unless
@@ -7887,10 +7901,6 @@ iwx_newstate(struct ieee80211com *ic, enum ieee80211_s
                memset(sc->setkey_arg, 0, sizeof(sc->setkey_arg));
                sc->setkey_cur = sc->setkey_tail = sc->setkey_nkeys = 0;
                iwx_del_task(sc, systq, &sc->mac_ctxt_task);
-               for (i = 0; i < nitems(sc->sc_rxba_data); i++) {
-                       struct iwx_rxba_data *rxba = &sc->sc_rxba_data[i];
-                       iwx_clear_reorder_buffer(sc, rxba);
-               }
        }
 
        sc->ns_nstate = nstate;

Reply via email to