[PATCH] mac80211: Look out for some other AP when disassoc is received.

[PATCH] mac80211: Look out for some other AP when disassoc is received.

[Vivek Natarajan]

When a disassoc packet is received with a reason code of 'AP leaving the BSS',
mac80211 should start looking for some other AP instead of trying to
associate with the same AP.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
 net/mac80211/mlme.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 6d5ed9c..8d0f995 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -855,7 +855,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 
 	ieee80211_sta_send_apinfo(sdata, ifsta);
 
-	if (self_disconnected)
+	if (self_disconnected || reason == WLAN_REASON_DISASSOC_STA_HAS_LEFT)
 		ifsta->state = IEEE80211_STA_MLME_DISABLED;
 
 	sta_info_unlink(&sta);
@@ -1174,7 +1174,7 @@ static void ieee80211_rx_mgmt_disassoc(struct ieee80211_sub_if_data *sdata,
 				      IEEE80211_RETRY_AUTH_INTERVAL);
 	}
 
-	ieee80211_set_disassoc(sdata, ifsta, false, false, 0);
+	ieee80211_set_disassoc(sdata, ifsta, false, false, reason_code);
 }
 
 
-- 
1.6.0.1

Re: [PATCH] mac80211: Look out for some other AP when disassoc is received.

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1482 bytes --]

On Wed, 2008-11-19 at 16:03 -0800, Vivek Natarajan wrote:
> When a disassoc packet is received with a reason code of 'AP leaving the BSS',
> mac80211 should start looking for some other AP instead of trying to
> associate with the same AP.

Umm, it doesn't try to associated with the same AP. And the patch you're
doing is kinda pointless, the reason codes are very often simply wrong
or "due to some local policy decision". Also, the reason code that's
being passed is the one we send ourselves, not the one we received.

> Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
> ---
>  net/mac80211/mlme.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 6d5ed9c..8d0f995 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -855,7 +855,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>  
>  	ieee80211_sta_send_apinfo(sdata, ifsta);
>  
> -	if (self_disconnected)
> +	if (self_disconnected || reason == WLAN_REASON_DISASSOC_STA_HAS_LEFT)
>  		ifsta->state = IEEE80211_STA_MLME_DISABLED;
>  
>  	sta_info_unlink(&sta);
> @@ -1174,7 +1174,7 @@ static void ieee80211_rx_mgmt_disassoc(struct ieee80211_sub_if_data *sdata,
>  				      IEEE80211_RETRY_AUTH_INTERVAL);
>  	}
>  
> -	ieee80211_set_disassoc(sdata, ifsta, false, false, 0);
> +	ieee80211_set_disassoc(sdata, ifsta, false, false, reason_code);
>  }
>  
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mac80211: Look out for some other AP when disassoc is received.

[Vivek Natarajan]

On Wed, Nov 19, 2008 at 5:25 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2008-11-19 at 16:03 -0800, Vivek Natarajan wrote:
>> When a disassoc packet is received with a reason code of 'AP leaving the BSS',
>> mac80211 should start looking for some other AP instead of trying to
>> associate with the same AP.
>
> Umm, it doesn't try to associated with the same AP. And the patch you're
> doing is kinda pointless, the reason codes are very often simply wrong
> or "due to some local policy decision".

Here is the test scenario that exposed this issue:
* There are two APs with the same SSID so that the sta can roam if one is down.
* When the AP to which the station is connected is brought down or restarted,
  it sends( should send according to 802.11 spec) a disassoc frame with a
  reason code of 8 which means the sender is leaving the BSS.
* After receiving a disassoc, mac80211 moves to ASSOCIATE state in
which scanning
   is disabled. So, for the first scan request from
wpa_supplicant(scan duration of 30sec),
   it tries to send probe   request to the same AP instead of scanning
for other APs.
   Then it moves to DISABLED  state since there will be no probe response.
* The next scan request comes only after 30sec and now it will scan
for all available APs
   since scanning  is supported in DISABLED state.
    So, it takes atleast 30sec to connect to some other AP.

> Also, the reason code that's being passed is the one we send ourselves,
> not the one we received.

I have passed the reason code from ieee80211_rx_mgmt_disassoc which is called
only after we receive a disassoc frame from the AP.

--
Vivek.

Re: [PATCH] mac80211: Look out for some other AP when disassoc is received.

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]

On Wed, 2008-11-19 at 18:03 +0530, Vivek Natarajan wrote:

> Here is the test scenario that exposed this issue:
> * There are two APs with the same SSID so that the sta can roam if one is down.
> * When the AP to which the station is connected is brought down or restarted,
>   it sends( should send according to 802.11 spec) a disassoc frame with a
>   reason code of 8 which means the sender is leaving the BSS.
> * After receiving a disassoc, mac80211 moves to ASSOCIATE state in which scanning
>    is disabled. So, for the first scan request from wpa_supplicant(scan duration of 30sec),
>    it tries to send probe   request to the same AP instead of scanning for other APs.
>    Then it moves to DISABLED  state since there will be no probe response.
> * The next scan request comes only after 30sec and now it will scan for all available APs
>    since scanning  is supported in DISABLED state.
>     So, it takes atleast 30sec to connect to some other AP.

Ok, I understand the scenario, I think. But I don't understand why going
to DISABLED fixes this. In DISABLED the mlme does nothing at all, I
think, so how can that fix it?

We can only request a new scan if we get into ieee80211_sta_config_auth,
which we can only get into by setting IEEE80211_STA_REQ_AUTH, which in
turn we get from ieee80211_sta_req_auth. Therefore, going into disabled
won't help at all.

Ok, I think I see, let me take a step back: you're not just using the
mlme code in the kernel but wpa supplicant as well? That has a 30 second
scan timeout? Seems a bit of a race condition thing, we block scans for
a short time while trying to probe/associate and wpa supplicant requests
one exactly then.

However, I still think going to disabled is wrong. We should be in a
different state and if we detect a problem with the AP, got disconnected
and it didn't respond to probes, then we could go back to scanning if we
had previously been associated. Not if we haven't ever been associated
though because then we could get caught in an endless loop.

I think you've also not accounted for the possibility that the AP simply
lost power for some reason and didn't send a deauth frame at all.

Unfortunately, I don't see a good way to fix this. I've long meant to
completely rewrite the MLME state machine which would have made this
easier, never got around to it though. Taking into account what I've
just said, do you have another idea?

> > Also, the reason code that's being passed is the one we send ourselves,
> > not the one we received.
> 
> I have passed the reason code from ieee80211_rx_mgmt_disassoc which is called
> only after we receive a disassoc frame from the AP.

Yes, but the parameter we pass to that function used to be for the
reason code we send, I have no trouble making it both depending on the
other parameters though, but I guess we should document that if we
really need to do it.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mac80211: Look out for some other AP when disassoc is received.

[Vivek Natarajan]

 Johannes Berg <johannes@sipsolutions.net> wrote:

>> * After receiving a disassoc, mac80211 moves to ASSOCIATE state in which scanning
>>    is disabled. So, for the first scan request from wpa_supplicant(scan duration of 30sec),
>>    it tries to send probe   request to the same AP instead of scanning for other APs.
>>    Then it moves to DISABLED  state since there will be no probe response.
>> * The next scan request comes only after 30sec and now it will scan for all available APs
>>    since scanning  is supported in DISABLED state.
>>     So, it takes atleast 30sec to connect to some other AP.

> we block scans for a short time while trying to probe/associate and
> wpa supplicant request one exactly then.
> However, I still think going to disabled is wrong. We should be in a
> different state

If the AP has explicitly told us that it is leaving the bss, what is the
point in retrying in associate state? DISABLED state would be ideal
for fresh scanning & assocation. What do you think?

> Ok, I understand the scenario, I think. But I don't understand why going
> to DISABLED fixes this. In DISABLED the mlme does nothing at all, I
> think, so how can that fix it?
> I think you've also not accounted for the possibility that the AP simply
> lost power for some reason and didn't send a deauth frame at all.

If AP loses power, probe request times out and we directly move to
disabled state. Hence we don't spend time in ASSOCIATE/DIRECT-PROBE
state. The same behaviour is expected when we receive a disassoc
frame with reason code as "AP leaving BSS". There should not be any
intermediate state like ASSOCIATE.

Thanks for your time,
 Vivek.

Re: [PATCH] mac80211: Look out for some other AP when disassoc is received.

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]

On Fri, 2008-11-21 at 17:16 +0530, Vivek Natarajan wrote:
> Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> >> * After receiving a disassoc, mac80211 moves to ASSOCIATE state in which scanning
> >>    is disabled. So, for the first scan request from wpa_supplicant(scan duration of 30sec),
> >>    it tries to send probe   request to the same AP instead of scanning for other APs.
> >>    Then it moves to DISABLED  state since there will be no probe response.
> >> * The next scan request comes only after 30sec and now it will scan for all available APs
> >>    since scanning  is supported in DISABLED state.
> >>     So, it takes atleast 30sec to connect to some other AP.
> 
> > we block scans for a short time while trying to probe/associate and
> > wpa supplicant request one exactly then.
> > However, I still think going to disabled is wrong. We should be in a
> > different state
> 
> If the AP has explicitly told us that it is leaving the bss, what is the
> point in retrying in associate state? DISABLED state would be ideal
> for fresh scanning & assocation. What do you think?

Well, the thing is that if we go to disabled state then we don't
actually scan and associate again. Only if wpa supplicant is running
does that happen.

> > Ok, I understand the scenario, I think. But I don't understand why going
> > to DISABLED fixes this. In DISABLED the mlme does nothing at all, I
> > think, so how can that fix it?
> > I think you've also not accounted for the possibility that the AP simply
> > lost power for some reason and didn't send a deauth frame at all.
> 
> If AP loses power, probe request times out and we directly move to
> disabled state. Hence we don't spend time in ASSOCIATE/DIRECT-PROBE
> state. The same behaviour is expected when we receive a disassoc
> frame with reason code as "AP leaving BSS". There should not be any
> intermediate state like ASSOCIATE.

Oh. You're right, we do go directly to disabled. I thought we somehow
went back to 'request auth' (which is not a proper state, unfortunately)

Hmm. I guess what your patch is doing is the best thing at this point
then until we actually rewrite the roaming code.

Can you add a comment to _set_disassoc that the "reason" variable passed
is either our own reason (when self_disconnected is true) or the remote
reason (when self_disconnected is false) please? Also, I haven't
checked, but there may be other places where we could pass the remote
reason.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] mac80211: Look out for some other AP when disassoc is received.

[Vivek Natarajan]

From: Vivek Natarajan <vnatarajan@atheros.com>
Date: Fri, 21 Nov 2008 11:15:19 +0530
Subject: [PATCH] mac80211: Look out for some other AP when disassoc is received.

When a disassoc packet is received with a reason code of
'AP leaving the BSS', mac80211 should start looking for some
other AP instead of trying to associate with the same AP.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
 net/mac80211/mlme.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 90ad5a4..ee15e75 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -808,6 +808,10 @@ static void ieee80211_authenticate(struct ieee80211_sub_if_data *sdata,
 	mod_timer(&ifsta->timer, jiffies + IEEE80211_AUTH_TIMEOUT);
 }
 
+/*
+ * The disassoc 'reason' argument can be either our own reason
+ * if self disconnected or a reason code from the AP.
+ */
 static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 				   struct ieee80211_if_sta *ifsta, bool deauth,
 				   bool self_disconnected, u16 reason)
@@ -854,7 +858,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 
 	ieee80211_sta_send_apinfo(sdata, ifsta);
 
-	if (self_disconnected)
+	if (self_disconnected || reason == WLAN_REASON_DISASSOC_STA_HAS_LEFT)
 		ifsta->state = IEEE80211_STA_MLME_DISABLED;
 
 	sta_info_unlink(&sta);
@@ -1175,7 +1179,7 @@ static void ieee80211_rx_mgmt_disassoc(struct ieee80211_sub_if_data *sdata,
 				      IEEE80211_RETRY_AUTH_INTERVAL);
 	}
 
-	ieee80211_set_disassoc(sdata, ifsta, false, false, 0);
+	ieee80211_set_disassoc(sdata, ifsta, false, false, reason_code);
 }
 
 
-- 
1.5.6.4

[PATCH] mac80211: Look out for some other AP when disassoc is received.

[Vivek Natarajan]

When a disassoc packet is received with a reason code of
'AP leaving the BSS', mac80211 should start looking for some
other AP instead of trying to associate with the same AP.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
 net/mac80211/mlme.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 90ad5a4..ee15e75 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -808,6 +808,10 @@ static void ieee80211_authenticate(struct ieee80211_sub_if_data *sdata,
 	mod_timer(&ifsta->timer, jiffies + IEEE80211_AUTH_TIMEOUT);
 }
 
+/*
+ * The disassoc 'reason' argument can be either our own reason
+ * if self disconnected or a reason code from the AP.
+ */
 static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 				   struct ieee80211_if_sta *ifsta, bool deauth,
 				   bool self_disconnected, u16 reason)
@@ -854,7 +858,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 
 	ieee80211_sta_send_apinfo(sdata, ifsta);
 
-	if (self_disconnected)
+	if (self_disconnected || reason == WLAN_REASON_DISASSOC_STA_HAS_LEFT)
 		ifsta->state = IEEE80211_STA_MLME_DISABLED;
 
 	sta_info_unlink(&sta);
@@ -1175,7 +1179,7 @@ static void ieee80211_rx_mgmt_disassoc(struct ieee80211_sub_if_data *sdata,
 				      IEEE80211_RETRY_AUTH_INTERVAL);
 	}
 
-	ieee80211_set_disassoc(sdata, ifsta, false, false, 0);
+	ieee80211_set_disassoc(sdata, ifsta, false, false, reason_code);
 }
 
 
-- 
1.5.6.4

Re: [PATCH] mac80211: Look out for some other AP when disassoc is received.

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2165 bytes --]

On Fri, 2008-11-21 at 20:13 -0800, Vivek Natarajan wrote:
> When a disassoc packet is received with a reason code of
> 'AP leaving the BSS', mac80211 should start looking for some
> other AP instead of trying to associate with the same AP.

Can we edit this commit log message? mac80211 will not start looking for
another AP in DISABLED state. How about

When a disassoc packet is received from the AP with a reason code of
'leaving the BSS', mac80211 should go into DISABLED state just as it
would do if the AP suddenly went away for some reason, as that is what
will happen shortly after the AP leaves anyway.

(is that correct?)

Other than that,
Acked-by: Johannes Berg <johannes@sipsolutions.net>

> Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
> ---
>  net/mac80211/mlme.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 90ad5a4..ee15e75 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -808,6 +808,10 @@ static void ieee80211_authenticate(struct ieee80211_sub_if_data *sdata,
>  	mod_timer(&ifsta->timer, jiffies + IEEE80211_AUTH_TIMEOUT);
>  }
>  
> +/*
> + * The disassoc 'reason' argument can be either our own reason
> + * if self disconnected or a reason code from the AP.
> + */
>  static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>  				   struct ieee80211_if_sta *ifsta, bool deauth,
>  				   bool self_disconnected, u16 reason)
> @@ -854,7 +858,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>  
>  	ieee80211_sta_send_apinfo(sdata, ifsta);
>  
> -	if (self_disconnected)
> +	if (self_disconnected || reason == WLAN_REASON_DISASSOC_STA_HAS_LEFT)
>  		ifsta->state = IEEE80211_STA_MLME_DISABLED;
>  
>  	sta_info_unlink(&sta);
> @@ -1175,7 +1179,7 @@ static void ieee80211_rx_mgmt_disassoc(struct ieee80211_sub_if_data *sdata,
>  				      IEEE80211_RETRY_AUTH_INTERVAL);
>  	}
>  
> -	ieee80211_set_disassoc(sdata, ifsta, false, false, 0);
> +	ieee80211_set_disassoc(sdata, ifsta, false, false, reason_code);
>  }
>  
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] mac80211: Look out for some other AP when disassoc is received.

[Vivek Natarajan]

When a disassoc packet is received from the AP with a reason code of
'leaving the BSS', mac80211 should go into DISABLED state just as it
would do if the AP suddenly went away for some reason, as that is what
will happen shortly after the AP leaves anyway.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
Acked-by: Johannes Berg <johannes@sipsolutions.net>
---
 net/mac80211/mlme.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 90ad5a4..ee15e75 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -808,6 +808,10 @@ static void ieee80211_authenticate(struct ieee80211_sub_if_data *sdata,
 	mod_timer(&ifsta->timer, jiffies + IEEE80211_AUTH_TIMEOUT);
 }
 
+/*
+ * The disassoc 'reason' argument can be either our own reason
+ * if self disconnected or a reason code from the AP.
+ */
 static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 				   struct ieee80211_if_sta *ifsta, bool deauth,
 				   bool self_disconnected, u16 reason)
@@ -854,7 +858,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 
 	ieee80211_sta_send_apinfo(sdata, ifsta);
 
-	if (self_disconnected)
+	if (self_disconnected || reason == WLAN_REASON_DISASSOC_STA_HAS_LEFT)
 		ifsta->state = IEEE80211_STA_MLME_DISABLED;
 
 	sta_info_unlink(&sta);
@@ -1175,7 +1179,7 @@ static void ieee80211_rx_mgmt_disassoc(struct ieee80211_sub_if_data *sdata,
 				      IEEE80211_RETRY_AUTH_INTERVAL);
 	}
 
-	ieee80211_set_disassoc(sdata, ifsta, false, false, 0);
+	ieee80211_set_disassoc(sdata, ifsta, false, false, reason_code);
 }
 
 
-- 
1.5.6.4