On Wed, May 18, 2016 at 06:24:15AM +0800, Nathanael Rensen wrote:
> On 27 April 2016 at 17:33, Stefan Sperling <s...@stsp.name> wrote:
>
> > This version includes minor tweak: When the AP goes down, we don't
> > need to send disassoc frames to nodes in COLLECT state.
>
> While testing this diff I found some additional ieee80211 code paths
> that rely on ni_associd == 0 to detect a non-associated node. These
> caused issues for rt2860.
>
> a) The rt2860 driver removes the corresponding WCID from the hardware
> table when a node dissociates. The WCID is added when the ic_newassoc()
> hook is called with the newassoc flag set. However, ieee80211_node_join()
> relies on ni_associd == 0 to determine whether to set the newassoc flag.
> If a cached node that already has a non-zero aid is reused the newassoc
> flag is not set and the WCID is not added to the hardware table.
>
> b) A node that is inactive for a long period is placed into
> IEEE80211_STA_COLLECT by ieee80211_clean_nodes() and must reassociate
> before it can exchange data. If the node believes it is still associated
> then it will continue to use the defunct association. There is code in
> ieee80211_input() to detect this and send a management frame informing
> the node that it must reassociate. The detection also relies on
> ni_associd == 0, so a previously associated node that has retained its
> ni_associd will not be instructed to reassociate and will continue to
> use the defunct association (I saw this behaviour with android phones).
>
> With the approach of retaining the aid for nodes in IEEE80211_STA_COLLECT
> ni_associd == 0 is no longer a reliable way to infer node state. I think
> it is better to rely on the node state directly. The diff below includes
> stsp@'s diff along with additional changes to use the node state directly
> to drive the logic.
>
> Nathanael
Thanks, committed with a slight tweak:
@@ -1496,7 +1496,7 @@ void
ieee80211_node_join(struct ieee80211com *ic, struct ieee80211_node *ni,
int resp)
{
- int newassoc;
+ int newassoc = (ni->ni_state != IEEE80211_STA_ASSOC);
if (ni->ni_associd == 0) {
u_int16_t aid;
Please help me with watching the lists for regression reports.
> Index: ieee80211_input.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
> retrieving revision 1.176
> diff -u -p -r1.176 ieee80211_input.c
> --- ieee80211_input.c 2 May 2016 09:35:49 -0000 1.176
> +++ ieee80211_input.c 10 May 2016 01:05:22 -0000
> @@ -447,7 +447,7 @@ ieee80211_input(struct ifnet *ifp, struc
> ic->ic_stats.is_rx_notassoc++;
> goto err;
> }
> - if (ni->ni_associd == 0) {
> + if (ni->ni_state != IEEE80211_STA_ASSOC) {
> DPRINTF(("data from unassoc src %s\n",
> ether_sprintf(wh->i_addr2)));
> IEEE80211_SEND_MGMT(ic, ni,
> Index: ieee80211_node.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 ieee80211_node.c
> --- ieee80211_node.c 12 Apr 2016 14:33:27 -0000 1.101
> +++ ieee80211_node.c 10 May 2016 01:05:22 -0000
> @@ -1496,7 +1496,10 @@ void
> ieee80211_node_join(struct ieee80211com *ic, struct ieee80211_node *ni,
> int resp)
> {
> - int newassoc;
> + int newassoc = 1;
> +
> + if (ni->ni_state == IEEE80211_STA_ASSOC)
> + newassoc = 0;
>
> if (ni->ni_associd == 0) {
> u_int16_t aid;
> @@ -1518,13 +1521,11 @@ ieee80211_node_join(struct ieee80211com
> }
> ni->ni_associd = aid | 0xc000;
> IEEE80211_AID_SET(ni->ni_associd, ic->ic_aid_bitmap);
> - newassoc = 1;
> if (ic->ic_curmode == IEEE80211_MODE_11G ||
> (ic->ic_curmode == IEEE80211_MODE_11N &&
> IEEE80211_IS_CHAN_2GHZ(ic->ic_bss->ni_chan)))
> ieee80211_node_join_11g(ic, ni);
> - } else
> - newassoc = 0;
> + }
>
> DPRINTF(("station %s %s associated at aid %d\n",
> ether_sprintf(ni->ni_macaddr), newassoc ? "newly" : "already",
> @@ -1699,8 +1700,6 @@ ieee80211_node_leave(struct ieee80211com
> if (ic->ic_node_leave != NULL)
> (*ic->ic_node_leave)(ic, ni);
>
> - IEEE80211_AID_CLR(ni->ni_associd, ic->ic_aid_bitmap);
> - ni->ni_associd = 0;
> ieee80211_node_newstate(ni, IEEE80211_STA_COLLECT);
>
> #if NBRIDGE > 0
> Index: ieee80211_proto.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_proto.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 ieee80211_proto.c
> --- ieee80211_proto.c 27 Apr 2016 11:58:10 -0000 1.66
> +++ ieee80211_proto.c 10 May 2016 01:05:22 -0000
> @@ -852,7 +852,7 @@ ieee80211_newstate(struct ieee80211com *
> case IEEE80211_M_HOSTAP:
> s = splnet();
> RB_FOREACH(ni, ieee80211_tree, &ic->ic_tree) {
> - if (ni->ni_associd == 0)
> + if (ni->ni_state != IEEE80211_STA_ASSOC)
> continue;
> IEEE80211_SEND_MGMT(ic, ni,
> IEEE80211_FC0_SUBTYPE_DISASSOC,
>
