On Wed, Sep 10, 2014 at 02:42:43PM +0200, Marcin Piotr Pawlowski wrote:
> Yes, I think that it could be is possible to double clean the node cache.
> 
> Updated diff with suggestion from Stefan.

Hi,

this patch works for me too. 

But is the problem not specific to the
iwn(4) driver or at least only specific to drivers which override the
default state machine and not calling into the original state machine when
IEEE80211_S_SCAN is entered?

ieee80211_begin_scan() calls ieee80211_free_allnodes() which prevents
the problem for drivers which use the default scanning infrastructure.

So would it make sense to move the call to ieee80211_clean_cached() into
iwn(4) like in the modified patch below?

I experimented a bit with calling ieee80211_free_allnodes() here as it
is called in default scanning infrastructure but this does not work for
me (yet) :)

Cheers,
Fabian


Index: dev/pci/if_iwn.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.135
diff -u -p -r1.135 if_iwn.c
--- dev/pci/if_iwn.c    10 Sep 2014 07:22:09 -0000      1.135
+++ dev/pci/if_iwn.c    10 Sep 2014 18:04:18 -0000
@@ -1750,6 +1750,9 @@ iwn_newstate(struct ieee80211com *ic, en
                /* Make the link LED blink while we're scanning. */
                iwn_set_led(sc, IWN_LED_LINK, 10, 10);
 
+               if (ic->ic_state != IEEE80211_S_SCAN)
+                       ieee80211_clean_cached(ic);
+
                if ((error = iwn_scan(sc, IEEE80211_CHAN_2GHZ)) != 0) {
                        printf("%s: could not initiate scan\n",
                            sc->sc_dev.dv_xname);
Index: net80211/ieee80211_node.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
retrieving revision 1.82
diff -u -p -r1.82 ieee80211_node.c
--- net80211/ieee80211_node.c   8 Aug 2014 15:16:39 -0000       1.82
+++ net80211/ieee80211_node.c   10 Sep 2014 18:04:19 -0000
@@ -1134,6 +1134,21 @@ ieee80211_free_allnodes(struct ieee80211
                ieee80211_node_cleanup(ic, ic->ic_bss); /* for station mode */
 }
 
+void
+ieee80211_clean_cached(struct ieee80211com *ic)
+{
+       struct ieee80211_node *ni, *next_ni;
+       int s;
+
+       s = splnet();
+       for (ni = RB_MIN(ieee80211_tree, &ic->ic_tree);
+           ni != NULL; ni = next_ni) {
+               next_ni = RB_NEXT(ieee80211_tree, &ic->ic_tree, ni);
+               if (ni->ni_state == IEEE80211_STA_CACHE)
+                       ieee80211_free_node(ic, ni);
+       }
+       splx(s);
+}
 /*
  * Timeout inactive nodes.
  *
Index: net80211/ieee80211_node.h
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
retrieving revision 1.45
diff -u -p -r1.45 ieee80211_node.h
--- net80211/ieee80211_node.h   20 Mar 2014 13:19:06 -0000      1.45
+++ net80211/ieee80211_node.h   10 Sep 2014 18:04:20 -0000
@@ -325,6 +325,7 @@ extern      void ieee80211_free_allnodes(stru
 typedef void ieee80211_iter_func(void *, struct ieee80211_node *);
 extern void ieee80211_iterate_nodes(struct ieee80211com *ic,
                ieee80211_iter_func *, void *);
+extern void ieee80211_clean_cached(struct ieee80211com *ic);
 extern void ieee80211_clean_nodes(struct ieee80211com *, int);
 extern int ieee80211_setup_rates(struct ieee80211com *,
            struct ieee80211_node *, const u_int8_t *, const u_int8_t *, int);

> 
> Best regards,
> mpp
> 
> On 09/10/14 14:34, Stefan Sperling wrote:
> > On Wed, Sep 10, 2014 at 02:06:15PM +0200, Marcin Piotr Pawlowski wrote:
> >> On 09/10/14 10:15, Stefan Sperling wrote:
> >>> On Tue, Sep 09, 2014 at 10:17:59PM +0200, Fabian Raetz wrote:
> >>>> Hm interesting ... i can reproduce it here with an 2.4GHz AP.
> >>>> The entry isn't cleared when scanning and the interface is up.
> >>>>
> >>>> Scanning when the interface is down works correct for me.
> >>>> I will take a look at it tommorow :)
> >>>>
> >>>> 2014-09-09 21:48 GMT+02:00 Stuart Henderson <st...@openbsd.org>:
> >>>>
> >>>>> I have just noticed one thing with this; the 5GHz AP which I powered up 
> >>>>> for
> >>>>> a test is still showing in my current scan result long after I turned it
> >>>>> off
> >>>>> again. Caching issue somewhere?
> >>>>>
> >>>>>
> >>>
> >>> Try this with other wifi cards, too. The AP could be lingering
> >>> in the net80211 node cache which is device independent.
> >>>
> >>
> >> I think that the hypothesis with net80211 node cache was correct. And
> >> following diff solves this issue for me.
> >>
> >> Best regards,
> >> mpp
> > 
> >> Index: ieee80211_ioctl.c
> >> ===================================================================
> >> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
> >> retrieving revision 1.35
> >> diff -u -r1.35 ieee80211_ioctl.c
> >> --- ieee80211_ioctl.c      10 Jul 2014 14:32:28 -0000      1.35
> >> +++ ieee80211_ioctl.c      10 Sep 2014 12:04:13 -0000
> >> @@ -645,6 +645,7 @@
> >>                    error = ENETDOWN;
> >>                    break;
> >>            }
> >> +          ieee80211_clean_cached(ic);
> > 
> > Perhaps I'm wrong but couldn't this remove cached nodes while
> > a scan is already in progress?
> > 
> > Wouldn't it be better to only clean the cache before the
> > ieee80211_new_state(ic, IEEE80211_S_SCAN, -1); call
> > within the following if statement?
> > 
> >>            if ((ic->ic_scan_lock & IEEE80211_SCAN_REQUEST) == 0) {
> >>                    if (ic->ic_scan_lock & IEEE80211_SCAN_LOCKED)
> >>                            ic->ic_scan_lock |= IEEE80211_SCAN_RESUME;
> >> Index: ieee80211_node.c
> >> ===================================================================
> >> RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
> >> retrieving revision 1.82
> >> diff -u -r1.82 ieee80211_node.c
> >> --- ieee80211_node.c       8 Aug 2014 15:16:39 -0000       1.82
> >> +++ ieee80211_node.c       10 Sep 2014 12:04:13 -0000
> >> @@ -1134,6 +1134,21 @@
> >>            ieee80211_node_cleanup(ic, ic->ic_bss); /* for station mode */
> >>  }
> >>  
> >> +void
> >> +ieee80211_clean_cached(struct ieee80211com *ic)
> >> +{
> >> +  struct ieee80211_node *ni, *next_ni;
> >> +  int s;
> >> +
> >> +  s = splnet();
> >> +  for (ni = RB_MIN(ieee80211_tree, &ic->ic_tree);
> >> +      ni != NULL; ni = next_ni) {
> >> +          next_ni = RB_NEXT(ieee80211_tree, &ic->ic_tree, ni);
> >> +          if (ni->ni_state == IEEE80211_STA_CACHE)
> >> +                  ieee80211_free_node(ic, ni);
> >> +  }
> >> +  splx(s);
> >> +}
> >>  /*
> >>   * Timeout inactive nodes.
> >>   *
> >> Index: ieee80211_node.h
> >> ===================================================================
> >> RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
> >> retrieving revision 1.45
> >> diff -u -r1.45 ieee80211_node.h
> >> --- ieee80211_node.h       20 Mar 2014 13:19:06 -0000      1.45
> >> +++ ieee80211_node.h       10 Sep 2014 12:04:13 -0000
> >> @@ -325,6 +325,7 @@
> >>  typedef void ieee80211_iter_func(void *, struct ieee80211_node *);
> >>  extern    void ieee80211_iterate_nodes(struct ieee80211com *ic,
> >>            ieee80211_iter_func *, void *);
> >> +extern    void ieee80211_clean_cached(struct ieee80211com *ic);
> >>  extern    void ieee80211_clean_nodes(struct ieee80211com *, int);
> >>  extern    int ieee80211_setup_rates(struct ieee80211com *,
> >>        struct ieee80211_node *, const u_int8_t *, const u_int8_t *, int);
> > 
> 

> Index: ieee80211_ioctl.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
> retrieving revision 1.35
> diff -u -r1.35 ieee80211_ioctl.c
> --- ieee80211_ioctl.c 10 Jul 2014 14:32:28 -0000      1.35
> +++ ieee80211_ioctl.c 10 Sep 2014 12:39:36 -0000
> @@ -649,8 +649,10 @@
>                       if (ic->ic_scan_lock & IEEE80211_SCAN_LOCKED)
>                               ic->ic_scan_lock |= IEEE80211_SCAN_RESUME;
>                       ic->ic_scan_lock |= IEEE80211_SCAN_REQUEST;
> -                     if (ic->ic_state != IEEE80211_S_SCAN)
> +                     if (ic->ic_state != IEEE80211_S_SCAN) {
> +                             ieee80211_clean_cached(ic);
>                               ieee80211_new_state(ic, IEEE80211_S_SCAN, -1);
> +                     }
>               }
>               /* Let the userspace process wait for completion */
>               error = tsleep(&ic->ic_scan_lock, PCATCH, "80211scan",
> Index: ieee80211_node.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
> retrieving revision 1.82
> diff -u -r1.82 ieee80211_node.c
> --- ieee80211_node.c  8 Aug 2014 15:16:39 -0000       1.82
> +++ ieee80211_node.c  10 Sep 2014 12:39:36 -0000
> @@ -1134,6 +1134,21 @@
>               ieee80211_node_cleanup(ic, ic->ic_bss); /* for station mode */
>  }
>  
> +void
> +ieee80211_clean_cached(struct ieee80211com *ic)
> +{
> +     struct ieee80211_node *ni, *next_ni;
> +     int s;
> +
> +     s = splnet();
> +     for (ni = RB_MIN(ieee80211_tree, &ic->ic_tree);
> +         ni != NULL; ni = next_ni) {
> +             next_ni = RB_NEXT(ieee80211_tree, &ic->ic_tree, ni);
> +             if (ni->ni_state == IEEE80211_STA_CACHE)
> +                     ieee80211_free_node(ic, ni);
> +     }
> +     splx(s);
> +}
>  /*
>   * Timeout inactive nodes.
>   *
> Index: ieee80211_node.h
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
> retrieving revision 1.45
> diff -u -r1.45 ieee80211_node.h
> --- ieee80211_node.h  20 Mar 2014 13:19:06 -0000      1.45
> +++ ieee80211_node.h  10 Sep 2014 12:39:36 -0000
> @@ -325,6 +325,7 @@
>  typedef void ieee80211_iter_func(void *, struct ieee80211_node *);
>  extern       void ieee80211_iterate_nodes(struct ieee80211com *ic,
>               ieee80211_iter_func *, void *);
> +extern       void ieee80211_clean_cached(struct ieee80211com *ic);
>  extern       void ieee80211_clean_nodes(struct ieee80211com *, int);
>  extern       int ieee80211_setup_rates(struct ieee80211com *,
>           struct ieee80211_node *, const u_int8_t *, const u_int8_t *, int);

Reply via email to