Re: small net80211 node cache fix
On Wed, Jul 18, 2012 at 12:33:12PM +0100, Stuart Henderson wrote:
> On 2012/07/18 12:51, Stefan Sperling wrote:
> > Node cache eviction is too agressive, possibly kicking off associated
> > stations for no good reason. I missed that associated stations are in
> > state IEEE80211_S_RUN rather than IEEE80211_S_ASSOC (which means "trying
> > to associate").
>
> there's ieee80211_node_state with the IEEE80211_STA_* enum ..
>
> enum ieee80211_node_state {
> IEEE80211_STA_CACHE,/* cached node */
> IEEE80211_STA_BSS, /* ic->ic_bss, the network we joined */
> IEEE80211_STA_AUTH, /* successfully authenticated */
> IEEE80211_STA_ASSOC,/* successfully associated */
> IEEE80211_STA_COLLECT /* This node remains in the cache while
> * the driver sends a de-auth message;
> * afterward it should be freed to make room
> * for a new node.
> */
> };
>
> and also ieee80211_state with IEEE80211_S_*,
>
> enum ieee80211_state {
> IEEE80211_S_INIT= 0,/* default state */
> IEEE80211_S_SCAN= 1,/* scanning */
> IEEE80211_S_AUTH= 2,/* try to authenticate */
> IEEE80211_S_ASSOC = 3,/* try to assoc */
> IEEE80211_S_RUN = 4 /* associated */
> };
>
> so I'm not sure about this, it looks like the diff changes things so that
> nodes in STA_COLLECT are no longer kicked off?
Oh my, yes, I was looking at the wrong enum. Thanks for catching this!
> > Also compile the debug message shown when a node is evicted from the
> > cache to make such problems easier to spot (won't affect install media
> > as it is protected by IEEE80211_STA_ONLY).
>
> OK for this bit.
I'll commit just the debug print so. Thanks again.
Re: small net80211 node cache fix
On 2012/07/18 12:51, Stefan Sperling wrote:
> Node cache eviction is too agressive, possibly kicking off associated
> stations for no good reason. I missed that associated stations are in
> state IEEE80211_S_RUN rather than IEEE80211_S_ASSOC (which means "trying
> to associate").
there's ieee80211_node_state with the IEEE80211_STA_* enum ..
enum ieee80211_node_state {
IEEE80211_STA_CACHE,/* cached node */
IEEE80211_STA_BSS, /* ic->ic_bss, the network we joined */
IEEE80211_STA_AUTH, /* successfully authenticated */
IEEE80211_STA_ASSOC,/* successfully associated */
IEEE80211_STA_COLLECT /* This node remains in the cache while
* the driver sends a de-auth message;
* afterward it should be freed to make room
* for a new node.
*/
};
and also ieee80211_state with IEEE80211_S_*,
enum ieee80211_state {
IEEE80211_S_INIT= 0,/* default state */
IEEE80211_S_SCAN= 1,/* scanning */
IEEE80211_S_AUTH= 2,/* try to authenticate */
IEEE80211_S_ASSOC = 3,/* try to assoc */
IEEE80211_S_RUN = 4 /* associated */
};
so I'm not sure about this, it looks like the diff changes things so that
nodes in STA_COLLECT are no longer kicked off?
> Also compile the debug message shown when a node is evicted from the
> cache to make such problems easier to spot (won't affect install media
> as it is protected by IEEE80211_STA_ONLY).
OK for this bit.
>
> Index: ieee80211_node.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 ieee80211_node.c
> --- ieee80211_node.c 16 Jul 2012 14:51:46 - 1.70
> +++ ieee80211_node.c 18 Jul 2012 10:24:59 -
> @@ -1176,12 +1176,12 @@ ieee80211_clean_nodes(struct ieee80211co
> ic->ic_state == IEEE80211_S_RUN) {
> if (cache_timeout) {
> if (ni->ni_state != IEEE80211_STA_COLLECT &&
> - (ni->ni_state == IEEE80211_STA_ASSOC ||
> + (ni->ni_state >= IEEE80211_STA_ASSOC ||
> ni->ni_inact < IEEE80211_INACT_MAX))
> continue;
> } else {
> if (ic->ic_opmode == IEEE80211_M_HOSTAP &&
> - ((ni->ni_state == IEEE80211_STA_ASSOC &&
> + ((ni->ni_state >= IEEE80211_STA_ASSOC &&
> ni->ni_inact < IEEE80211_INACT_MAX) ||
> (ni->ni_state == IEEE80211_STA_AUTH &&
>ni->ni_inact == 0)))
> @@ -1194,9 +1194,10 @@ ieee80211_clean_nodes(struct ieee80211co
> continue;
> }
> }
> + if (ifp->if_flags & IFF_DEBUG)
> + printf("%s: station %s purged from node cache\n",
> + ifp->if_xname, ether_sprintf(ni->ni_macaddr));
> #endif
> - DPRINTF(("station %s purged from LRU cache\n",
> - ether_sprintf(ni->ni_macaddr)));
> /*
>* If we're hostap and the node is authenticated, send
>* a deauthentication frame. The node will be freed when
small net80211 node cache fix
Node cache eviction is too agressive, possibly kicking off associated
stations for no good reason. I missed that associated stations are in
state IEEE80211_S_RUN rather than IEEE80211_S_ASSOC (which means "trying
to associate").
Also compile the debug message shown when a node is evicted from the
cache to make such problems easier to spot (won't affect install media
as it is protected by IEEE80211_STA_ONLY).
Index: ieee80211_node.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
retrieving revision 1.70
diff -u -p -r1.70 ieee80211_node.c
--- ieee80211_node.c16 Jul 2012 14:51:46 - 1.70
+++ ieee80211_node.c18 Jul 2012 10:24:59 -
@@ -1176,12 +1176,12 @@ ieee80211_clean_nodes(struct ieee80211co
ic->ic_state == IEEE80211_S_RUN) {
if (cache_timeout) {
if (ni->ni_state != IEEE80211_STA_COLLECT &&
- (ni->ni_state == IEEE80211_STA_ASSOC ||
+ (ni->ni_state >= IEEE80211_STA_ASSOC ||
ni->ni_inact < IEEE80211_INACT_MAX))
continue;
} else {
if (ic->ic_opmode == IEEE80211_M_HOSTAP &&
- ((ni->ni_state == IEEE80211_STA_ASSOC &&
+ ((ni->ni_state >= IEEE80211_STA_ASSOC &&
ni->ni_inact < IEEE80211_INACT_MAX) ||
(ni->ni_state == IEEE80211_STA_AUTH &&
ni->ni_inact == 0)))
@@ -1194,9 +1194,10 @@ ieee80211_clean_nodes(struct ieee80211co
continue;
}
}
+ if (ifp->if_flags & IFF_DEBUG)
+ printf("%s: station %s purged from node cache\n",
+ ifp->if_xname, ether_sprintf(ni->ni_macaddr));
#endif
- DPRINTF(("station %s purged from LRU cache\n",
- ether_sprintf(ni->ni_macaddr)));
/*
* If we're hostap and the node is authenticated, send
* a deauthentication frame. The node will be freed when
