When a scan begins we currently toss away everything we have
learned about access points in the previous scan iteration.
This behaviour gets in the way of some things.

For instance, I am working on another diff to show reasons for association
failures in ifconfig output ("wrong channel", "wrong WPA key", "wrong BSSID").
Because the scan loop keeps deleting existing nodes it gets in the way of
such features. Whenever ifconfig wants to read information about an AP we
have failed to associate to the AP's node has been freed already, and has
perhaps been re-allocated upon reception of a new beacon.

There are several ways in which nodes will still get freed with this diff:

1) This diff adds a new way of timing out inactive nodes which don't
send a beacon within 10 scan iterations. This should get rid of stale
APs if we're scanning for some time in a changing environment (should
cover laptops walking around in buildings looking for APs to connect to).

2) If we fail to associate a few times, the corresponding node is removed.
This mechanism already exists in -current code and is not changed here.
See how ni_fails is handled in ieee80211_node_choose_bss().

3) If net80211 transitions back to INIT state (e.g. because of a
user-initiated configuration change) all nodes are removed.

4) When a background scan starts all nodes will be removed.
This could be revisited later. I have left it as-is for now.
Background scan only occurs in RUN state and would be unwise to mix
such changes into this diff, which aims to fix SCAN state.


This isn't a new idea.
I can recall conversations about this with various people.

My question is whether this diff breaks anything for anyone.


diff refs/heads/master refs/heads/keepnodes
blob - 461f33f2561841520e61dd3de64a857413d1b224
blob + 4634efcc61bcc44bea8a44abb4590cd6df303384
--- sys/dev/ic/bwfm.c
+++ sys/dev/ic/bwfm.c
@@ -2610,7 +2610,7 @@ bwfm_newstate(struct ieee80211com *ic, enum ieee80211_
                        return 0;
                }
                ieee80211_set_link_state(ic, LINK_STATE_DOWN);
-               ieee80211_free_allnodes(ic, 1);
+               ieee80211_node_cleanup(ic, ic->ic_bss);
                ic->ic_state = nstate;
                splx(s);
                return 0;
blob - 0eb9dc07a0a75583f80579cc2d4c285dd1dc36b2
blob + 0caa61779fed20f6e96a337a7ec0278efaaa72b5
--- sys/dev/ic/pgt.c
+++ sys/dev/ic/pgt.c
@@ -170,7 +170,7 @@ void         node_mark_active_ap(void *, struct 
ieee80211_nod
 void    node_mark_active_adhoc(void *, struct ieee80211_node *);
 void    pgt_watchdog(struct ifnet *);
 int     pgt_init(struct ifnet *);
-void    pgt_update_hw_from_sw(struct pgt_softc *, int, int);
+void    pgt_update_hw_from_sw(struct pgt_softc *, int);
 void    pgt_hostap_handle_mlme(struct pgt_softc *, uint32_t,
             struct pgt_obj_mlme *);
 void    pgt_update_sw_from_hw(struct pgt_softc *,
@@ -544,8 +544,7 @@ trying_again:
                sc->sc_flags &= ~flag;
                if (ic->ic_if.if_flags & IFF_RUNNING)
                        pgt_update_hw_from_sw(sc,
-                           ic->ic_state != IEEE80211_S_INIT,
-                           ic->ic_opmode != IEEE80211_M_MONITOR);
+                           ic->ic_state != IEEE80211_S_INIT);
        }
 
        ic->ic_if.if_flags &= ~IFF_RUNNING;
@@ -2015,7 +2014,7 @@ pgt_media_change(struct ifnet *ifp)
 
         error = ieee80211_media_change(ifp);
         if (error == ENETRESET) {
-                pgt_update_hw_from_sw(sc, 0, 0);
+                pgt_update_hw_from_sw(sc, 0);
                 error = 0;
         }
 
@@ -2367,7 +2366,7 @@ pgt_ioctl(struct ifnet *ifp, u_long cmd, caddr_t req)
        }
 
        if (error == ENETRESET) {
-               pgt_update_hw_from_sw(sc, 0, 0);
+               pgt_update_hw_from_sw(sc, 0);
                error = 0;
        }
        splx(s);
@@ -2501,8 +2500,7 @@ pgt_init(struct ifnet *ifp)
 
        if (!(sc->sc_flags & (SC_DYING | SC_UNINITIALIZED)))
                pgt_update_hw_from_sw(sc,
-                   ic->ic_state != IEEE80211_S_INIT,
-                   ic->ic_opmode != IEEE80211_M_MONITOR);
+                   ic->ic_state != IEEE80211_S_INIT);
 
        ifp->if_flags |= IFF_RUNNING;
        ifq_clr_oactive(&ifp->if_snd);
@@ -2522,7 +2520,7 @@ pgt_init(struct ifnet *ifp)
  * back to the BSS had before.
  */
 void
-pgt_update_hw_from_sw(struct pgt_softc *sc, int keepassoc, int keepnodes)
+pgt_update_hw_from_sw(struct pgt_softc *sc, int keepassoc)
 {
        struct ieee80211com *ic = &sc->sc_ic;
        struct arpcom *ac = &ic->ic_ac;
@@ -2769,8 +2767,6 @@ badopmode:
        splx(s);
 
        if (success) {
-               if (shouldbeup && keepnodes)
-                       sc->sc_flags |= SC_NOFREE_ALLNODES;
                if (shouldbeup)
                        ieee80211_new_state(ic, IEEE80211_S_SCAN, -1);
                else
@@ -2942,11 +2938,7 @@ pgt_newstate(struct ieee80211com *ic, enum ieee80211_s
        case IEEE80211_S_SCAN:
                ic->ic_if.if_timer = 1;
                ic->ic_mgt_timer = 0;
-               if (sc->sc_flags & SC_NOFREE_ALLNODES)
-                       sc->sc_flags &= ~SC_NOFREE_ALLNODES;
-               else
-                       ieee80211_free_allnodes(ic, 1);
-
+               ieee80211_node_cleanup(ic, ic->ic_bss);
                ieee80211_set_link_state(ic, LINK_STATE_DOWN);
 #ifndef IEEE80211_STA_ONLY
                /* Just use any old channel; we override it anyway. */
@@ -3262,7 +3254,7 @@ pgt_activate(struct device *self, int act)
        case DVACT_SUSPEND:
                if (ifp->if_flags & IFF_RUNNING) {
                        pgt_stop(sc, SC_NEEDS_RESET);
-                       pgt_update_hw_from_sw(sc, 0, 0);
+                       pgt_update_hw_from_sw(sc, 0);
                }
                if (sc->sc_power != NULL)
                        (*sc->sc_power)(sc, act);
@@ -3283,10 +3275,10 @@ pgt_wakeup(struct pgt_softc *sc)
                (*sc->sc_power)(sc, DVACT_RESUME);
 
        pgt_stop(sc, SC_NEEDS_RESET);
-       pgt_update_hw_from_sw(sc, 0, 0);
+       pgt_update_hw_from_sw(sc, 0);
 
        if (ifp->if_flags & IFF_UP) {
                pgt_init(ifp);
-               pgt_update_hw_from_sw(sc, 0, 0);
+               pgt_update_hw_from_sw(sc, 0);
        }
 }
blob - 038e6a63dfff113b525bb1e9a30c935996535569
blob + 83b42976022d4ed42a9165720139abe5c1508324
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -5701,7 +5701,7 @@ iwm_scan(struct iwm_softc *sc)
                    ieee80211_state_name[IEEE80211_S_SCAN]);
        if ((sc->sc_flags & IWM_FLAG_BGSCAN) == 0) {
                ieee80211_set_link_state(ic, LINK_STATE_DOWN);
-               ieee80211_free_allnodes(ic, 1);
+               ieee80211_node_cleanup(ic, ic->ic_bss);
        }
        ic->ic_state = IEEE80211_S_SCAN;
        iwm_led_blink_start(sc);
blob - be1f18695ea4d1c2dedcd7e1630b97f3f72a7cd6
blob + ef5f81699e90f2d4c4202b25880c9120907e5ec3
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -1825,7 +1825,7 @@ iwn_newstate(struct ieee80211com *ic, enum ieee80211_s
                            ieee80211_state_name[nstate]);
                if ((sc->sc_flags & IWN_FLAG_BGSCAN) == 0) {
                        ieee80211_set_link_state(ic, LINK_STATE_DOWN);
-                       ieee80211_free_allnodes(ic, 1);
+                       ieee80211_node_cleanup(ic, ic->ic_bss);
                }
                ic->ic_state = nstate;
                return 0;
blob - b67bcfd77b19bceb5ee8b7ceffc1bfe7aab92598
blob + 3075074c587ff95f69675e252c4b58f3aaf63e37
--- sys/dev/pci/if_wpi.c
+++ sys/dev/pci/if_wpi.c
@@ -1058,7 +1058,7 @@ wpi_newstate(struct ieee80211com *ic, enum ieee80211_s
                            ieee80211_state_name[ic->ic_state],
                            ieee80211_state_name[nstate]);
                ieee80211_set_link_state(ic, LINK_STATE_DOWN);
-               ieee80211_free_allnodes(ic, 1);
+               ieee80211_node_cleanup(ic, ic->ic_bss);
                ic->ic_state = nstate;
                return 0;
 
blob - a2476c29743019bca8b9183c32be0971f992b178
blob + e8064dda864afdda3814234882fd125c430799ce
--- sys/dev/usb/if_atu.c
+++ sys/dev/usb/if_atu.c
@@ -1210,7 +1210,7 @@ atu_newstate(struct ieee80211com *ic, enum ieee80211_s
        case IEEE80211_S_SCAN:
                memcpy(ic->ic_chan_scan, ic->ic_chan_active,
                    sizeof(ic->ic_chan_active));
-               ieee80211_free_allnodes(ic, 1);
+               ieee80211_node_cleanup(ic, ic->ic_bss);
 
                /* tell the event thread that we want a scan */
                sc->sc_cmd = ATU_C_SCAN;
blob - ec3e0f61e963ea403c9d8b88e675fbe42d5aeaeb
blob + 905396f216158f9497fc28be19737e61595adce3
--- sys/net80211/ieee80211_node.c
+++ sys/net80211/ieee80211_node.c
@@ -74,7 +74,6 @@ void ieee80211_setup_node(struct ieee80211com *, struc
     const u_int8_t *);
 void ieee80211_free_node(struct ieee80211com *, struct ieee80211_node *);
 struct ieee80211_node *ieee80211_alloc_node_helper(struct ieee80211com *);
-void ieee80211_node_cleanup(struct ieee80211com *, struct ieee80211_node *);
 void ieee80211_node_switch_bss(struct ieee80211com *, struct ieee80211_node *);
 void ieee80211_node_addba_request(struct ieee80211_node *, int);
 void ieee80211_node_addba_request_ac_be_to(void *);
@@ -92,6 +91,7 @@ void ieee80211_node_leave_11g(struct ieee80211com *, s
 void ieee80211_inact_timeout(void *);
 void ieee80211_node_cache_timeout(void *);
 #endif
+void ieee80211_clean_inactive_nodes(struct ieee80211com *, int);
 
 #ifndef IEEE80211_STA_ONLY
 void
@@ -780,6 +780,19 @@ ieee80211_reset_scan(struct ifnet *ifp)
 }
 
 /*
+ * Increase a node's inactitivy counter.
+ * This counter get reset to zero if a frame is received.
+ * This function is intended for station mode only.
+ * See ieee80211_node_cache_timeout() for hostap mode.
+ */
+void
+ieee80211_node_raise_inact(void *arg, struct ieee80211_node *ni)
+{
+       if (ni->ni_refcnt == 0 && ni->ni_inact < IEEE80211_INACT_SCAN)
+               ni->ni_inact++;
+}
+
+/*
  * Begin an active scan.
  */
 void
@@ -807,14 +820,12 @@ ieee80211_begin_scan(struct ifnet *ifp)
                        (ic->ic_flags & IEEE80211_F_ASCAN) ?
                                "active" : "passive");
 
-       /*
-        * Flush any previously seen AP's. Note that the latter 
-        * assumes we don't act as both an AP and a station,
-        * otherwise we'll potentially flush state of stations
-        * associated with us.
-        */
-       ieee80211_free_allnodes(ic, 1);
 
+       if (ic->ic_opmode == IEEE80211_M_STA) {
+               ieee80211_node_cleanup(ic, ic->ic_bss);
+               ieee80211_iterate_nodes(ic, ieee80211_node_raise_inact, NULL);
+       }
+
        /*
         * Reset the current mode. Setting the current mode will also
         * reset scan state.
@@ -1297,6 +1308,9 @@ ieee80211_end_scan(struct ifnet *ifp)
        if (ic->ic_scan_count)
                ic->ic_flags &= ~IEEE80211_F_ASCAN;
 
+       if (ic->ic_opmode == IEEE80211_M_STA)
+               ieee80211_clean_inactive_nodes(ic, IEEE80211_INACT_SCAN);
+
        ni = RBT_MIN(ieee80211_tree, &ic->ic_tree);
 
 #ifndef IEEE80211_STA_ONLY
@@ -2130,6 +2144,29 @@ ieee80211_clean_nodes(struct ieee80211com *ic, int cac
                    "possible nodes leak\n", ifp->if_xname, nnodes,
                    ic->ic_nnodes);
 #endif
+       splx(s);
+}
+
+void
+ieee80211_clean_inactive_nodes(struct ieee80211com *ic, int inact_max)
+{
+       struct ieee80211_node *ni, *next_ni;
+       u_int gen = ic->ic_scangen++;   /* NB: ok 'cuz single-threaded*/
+       int s;
+
+       s = splnet();
+       for (ni = RBT_MIN(ieee80211_tree, &ic->ic_tree);
+           ni != NULL; ni = next_ni) {
+               next_ni = RBT_NEXT(ieee80211_tree, ni);
+               if (ni->ni_scangen == gen)      /* previously handled */
+                       continue;
+               ni->ni_scangen = gen;
+               if (ni->ni_refcnt > 0 || ni->ni_inact < inact_max)
+                       continue;
+               ieee80211_free_node(ic, ni);
+               ic->ic_stats.is_node_timeout++;
+       }
+
        splx(s);
 }
 
blob - af6ad346c608189e4689050262a477a7a2e75cd5
blob + 84c5ecabaf666eb20d0935b0d96cdd3af88084c4
--- sys/net80211/ieee80211_node.h
+++ sys/net80211/ieee80211_node.h
@@ -41,6 +41,7 @@
 #define        IEEE80211_INACT_MAX     (300/IEEE80211_INACT_WAIT)
 #define        IEEE80211_CACHE_SIZE    100
 #define        IEEE80211_CACHE_WAIT    30
+#define        IEEE80211_INACT_SCAN    10              /* for station mode */
 
 struct ieee80211_rateset {
        u_int8_t                rs_nrates;
@@ -491,6 +492,7 @@ struct ieee80211_node *
                const char *, u_int8_t);
 void ieee80211_release_node(struct ieee80211com *,
                struct ieee80211_node *);
+void ieee80211_node_cleanup(struct ieee80211com *, struct ieee80211_node *);
 void ieee80211_free_allnodes(struct ieee80211com *, int);
 void ieee80211_iterate_nodes(struct ieee80211com *,
                ieee80211_iter_func *, void *);



Reply via email to