[PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Tomas Winkler]

From: Ester Kummer <ester.kummer@intel.com>

This patch handle scanning on IBSS mode like on STA mode.
When queuing the scan work we don't refer to the return value of
ieee80211_sta_start_scan so if we are in the last scan period, we will
return 0 to ieee80211_ioctl_siwscan and not -EAGAIN, and then iwlist will
call ieee80211_ioctl_giwscan to get the scan results and will not fail.

Signed-off-by: Ester Kummer <ester.kummer@intel.com>
Acked-by: Tomas Winkler <tomas.winkler@intel.com>
---
 net/mac80211/mlme.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 2564553..72d5fe2 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -4090,7 +4090,8 @@ int ieee80211_sta_req_scan(struct ieee80211_sub_if_data *sdata, u8 *ssid, size_t
 	struct ieee80211_if_sta *ifsta = &sdata->u.sta;
 	struct ieee80211_local *local = sdata->local;
 
-	if (sdata->vif.type != IEEE80211_IF_TYPE_STA)
+	if (sdata->vif.type != IEEE80211_IF_TYPE_STA ||
+	    sdata->vif.type != IEEE80211_IF_TYPE_IBSS)
 		return ieee80211_sta_start_scan(sdata, ssid, ssid_len);
 
 	if (local->sta_sw_scanning || local->sta_hw_scanning) {
-- 
1.5.4.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Johannes Berg]

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

On Mon, 2008-09-08 at 16:31 +0300, Tomas Winkler wrote:
> From: Ester Kummer <ester.kummer@intel.com>
> 
> This patch handle scanning on IBSS mode like on STA mode.
> When queuing the scan work we don't refer to the return value of
> ieee80211_sta_start_scan so if we are in the last scan period, we will
> return 0 to ieee80211_ioctl_siwscan and not -EAGAIN, and then iwlist will
> call ieee80211_ioctl_giwscan to get the scan results and will not fail.


Can you explain why? Or can anybody else explain why we do this
difference at all? And how should mesh behave?

> Signed-off-by: Ester Kummer <ester.kummer@intel.com>
> Acked-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  net/mac80211/mlme.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 2564553..72d5fe2 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -4090,7 +4090,8 @@ int ieee80211_sta_req_scan(struct ieee80211_sub_if_data *sdata, u8 *ssid, size_t
>  	struct ieee80211_if_sta *ifsta = &sdata->u.sta;
>  	struct ieee80211_local *local = sdata->local;
>  
> -	if (sdata->vif.type != IEEE80211_IF_TYPE_STA)
> +	if (sdata->vif.type != IEEE80211_IF_TYPE_STA ||
> +	    sdata->vif.type != IEEE80211_IF_TYPE_IBSS)
>  		return ieee80211_sta_start_scan(sdata, ssid, ssid_len);

This is wrong.

a != 1 || a != 2

has to be true at all times, I think you mean &&.

johannes

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

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Johannes Berg]

On Mon, 2008-09-08 at 16:55 +0200, Johannes Berg wrote:
> On Mon, 2008-09-08 at 16:31 +0300, Tomas Winkler wrote:
> > From: Ester Kummer <ester.kummer@intel.com>
> > 
> > This patch handle scanning on IBSS mode like on STA mode.
> > When queuing the scan work we don't refer to the return value of
> > ieee80211_sta_start_scan so if we are in the last scan period, we will
> > return 0 to ieee80211_ioctl_siwscan and not -EAGAIN, and then iwlist will
> > call ieee80211_ioctl_giwscan to get the scan results and will not fail.
> 
> 
> Can you explain why? Or can anybody else explain why we do this
> difference at all? And how should mesh behave?

Actually, I do understand the difference now (explanation added below)
and if I'm guessing the problem you're having correctly your patch is
wrong. I think you want something like the patch below (never mind the
fact that it's against scan.c, I'm shuffling code)

johannes

--- everything.orig/net/mac80211/scan.c	2008-09-08 17:01:21.000000000 +0200
+++ everything/net/mac80211/scan.c	2008-09-08 17:03:28.000000000 +0200
@@ -674,24 +674,32 @@ int ieee80211_sta_start_scan(struct ieee
 
 int ieee80211_sta_req_scan(struct ieee80211_sub_if_data *sdata, u8 *ssid, size_t ssid_len)
 {
-	struct ieee80211_if_sta *ifsta = &sdata->u.sta;
 	struct ieee80211_local *local = sdata->local;
 
-	if (sdata->vif.type != IEEE80211_IF_TYPE_STA)
-		return ieee80211_sta_start_scan(sdata, ssid, ssid_len);
-
 	if (local->sta_sw_scanning || local->sta_hw_scanning) {
 		if (local->scan_sdata == sdata)
 			return 0;
 		return -EBUSY;
 	}
 
-	ifsta->scan_ssid_len = ssid_len;
-	if (ssid_len)
-		memcpy(ifsta->scan_ssid, ssid, ssid_len);
-	set_bit(IEEE80211_STA_REQ_SCAN, &ifsta->request);
-	queue_work(local->hw.workqueue, &ifsta->work);
-	return 0;
+	/*
+	 * STA has a state machine that might need to defer scanning
+	 * while it's trying to associate/authenticate, therefore we
+	 * queue it up to the state machine in that case.
+	 */
+	if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
+		struct ieee80211_if_sta *ifsta = &sdata->u.sta;
+
+		ifsta->scan_ssid_len = ssid_len;
+		if (ssid_len)
+			memcpy(ifsta->scan_ssid, ssid, ssid_len);
+		set_bit(IEEE80211_STA_REQ_SCAN, &ifsta->request);
+		queue_work(local->hw.workqueue, &ifsta->work);
+
+		return 0;
+	}
+
+	return ieee80211_sta_start_scan(sdata, ssid, ssid_len);
 }
 
 

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Johannes Berg]

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

On Mon, 2008-09-08 at 17:05 +0200, Johannes Berg wrote:

> > Can you explain why? Or can anybody else explain why we do this
> > difference at all? And how should mesh behave?
> 
> Actually, I do understand the difference now (explanation added below)
> and if I'm guessing the problem you're having correctly your patch is
> wrong. I think you want something like the patch below (never mind the
> fact that it's against scan.c, I'm shuffling code)

>  	if (local->sta_sw_scanning || local->sta_hw_scanning) {
>  		if (local->scan_sdata == sdata)
>  			return 0;
>  		return -EBUSY;
>  	}

Then again, ieee80211_sta_start_scan does that check as well, so now I'm
confused.

johannes

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

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Tomas Winkler]

On Mon, Sep 8, 2008 at 6:16 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2008-09-08 at 17:05 +0200, Johannes Berg wrote:
>
>> > Can you explain why? Or can anybody else explain why we do this
>> > difference at all? And how should mesh behave?
>>
>> Actually, I do understand the difference now (explanation added below)
>> and if I'm guessing the problem you're having correctly your patch is
>> wrong. I think you want something like the patch below (never mind the
>> fact that it's against scan.c, I'm shuffling code)
>
>>       if (local->sta_sw_scanning || local->sta_hw_scanning) {
>>               if (local->scan_sdata == sdata)
>>                       return 0;
>>               return -EBUSY;
>>       }
>
> Then again, ieee80211_sta_start_scan does that check as well, so now I'm
> confused.

The bug is that scan can be triggered in STA and IBSS internally. If
you request scan from the application (iwlist) while
internally scan is running application won't fetch scan results
because the -EBUSY is returned on scan request.
Tomas

>

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Johannes Berg]

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

On Mon, 2008-09-08 at 19:53 +0300, Tomas Winkler wrote:

> The bug is that scan can be triggered in STA and IBSS internally. If
> you request scan from the application (iwlist) while
> internally scan is running application won't fetch scan results
> because the -EBUSY is returned on scan request.

But -EBUSY won't be returned if that same interface is already scanning,
then 0 will be returned, so I don't understand?

johannes

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

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Tomas Winkler]

On Mon, Sep 8, 2008 at 9:13 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2008-09-08 at 19:53 +0300, Tomas Winkler wrote:
>
>> The bug is that scan can be triggered in STA and IBSS internally. If
>> you request scan from the application (iwlist) while
>> internally scan is running application won't fetch scan results
>> because the -EBUSY is returned on scan request.
>
> But -EBUSY won't be returned if that same interface is already scanning,
> then 0 will be returned, so I don't understand?

Me either, but empirically the fix from Esti cured the problem. The
problem is seen only in IBSS. Will dig into this tomorrow.
Thanks
Tomas

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Tomas Winkler]

On Tue, Sep 9, 2008 at 1:24 AM, Tomas Winkler <tomasw@gmail.com> wrote:
> On Mon, Sep 8, 2008 at 9:13 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Mon, 2008-09-08 at 19:53 +0300, Tomas Winkler wrote:
>>
>>> The bug is that scan can be triggered in STA and IBSS internally. If
>>> you request scan from the application (iwlist) while
>>> internally scan is running application won't fetch scan results
>>> because the -EBUSY is returned on scan request.
>>
>> But -EBUSY won't be returned if that same interface is already scanning,
>> then 0 will be returned, so I don't understand?
>
> Me either, but empirically the fix from Esti cured the problem. The
> problem is seen only in IBSS. Will dig into this tomorrow.

Okay so here is the scenario. Iwlwifi reject scan that is triggered
withing fixed period after
association to leave time to finish EAPOL exchange.
We never hit the problem in STA mode because of this code

 if (sdata->vif.type != IEEE80211_IF_TYPE_STA)
		return ieee80211_sta_start_scan(dev, ssid, ssid_len);

which makes it go through the work queue, just pure timing luck.
I will remove this delay for IBSS, there is no association and no 1X
is going on.

For BSS I suggest to enforce similar delay already mac80211, iwlwifi I
just return empty scan completion and buffered results will be
returned.

Tomas

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Johannes Berg]

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

On Thu, 2008-09-11 at 00:32 +0300, Tomas Winkler wrote:

> Okay so here is the scenario. Iwlwifi reject scan that is triggered
> withing fixed period after
> association to leave time to finish EAPOL exchange.

Why is iwlwifi thinking it's associating in an IBSS?

> We never hit the problem in STA mode because of this code
> 
>  if (sdata->vif.type != IEEE80211_IF_TYPE_STA)
> 		return ieee80211_sta_start_scan(dev, ssid, ssid_len);
> 
> which makes it go through the work queue, just pure timing luck.

Well the remainder of the code makes it go through, but yeah.

> I will remove this delay for IBSS, there is no association and no 1X
> is going on.

Ok, makes sense.

> For BSS I suggest to enforce similar delay already mac80211, iwlwifi I
> just return empty scan completion and buffered results will be
> returned.

We might want to enforce a similar delay like that in mac80211 instead
of iwlwifi, yeah.

johannes

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

Re: [PATCH 1/1] mac80211: scan on IBSS mode like on STA mode

[Tomas Winkler]

On Mon, Sep 8, 2008 at 5:55 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2008-09-08 at 16:31 +0300, Tomas Winkler wrote:
>> From: Ester Kummer <ester.kummer@intel.com>
>>
>> This patch handle scanning on IBSS mode like on STA mode.
>> When queuing the scan work we don't refer to the return value of
>> ieee80211_sta_start_scan so if we are in the last scan period, we will
>> return 0 to ieee80211_ioctl_siwscan and not -EAGAIN, and then iwlist will
>> call ieee80211_ioctl_giwscan to get the scan results and will not fail.
>
>
> Can you explain why? Or can anybody else explain why we do this
> difference at all? And how should mesh behave?
>
>> Signed-off-by: Ester Kummer <ester.kummer@intel.com>
>> Acked-by: Tomas Winkler <tomas.winkler@intel.com>
>> ---
>>  net/mac80211/mlme.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
>> index 2564553..72d5fe2 100644
>> --- a/net/mac80211/mlme.c
>> +++ b/net/mac80211/mlme.c
>> @@ -4090,7 +4090,8 @@ int ieee80211_sta_req_scan(struct ieee80211_sub_if_data *sdata, u8 *ssid, size_t
>>       struct ieee80211_if_sta *ifsta = &sdata->u.sta;
>>       struct ieee80211_local *local = sdata->local;
>>
>> -     if (sdata->vif.type != IEEE80211_IF_TYPE_STA)
>> +     if (sdata->vif.type != IEEE80211_IF_TYPE_STA ||
>> +         sdata->vif.type != IEEE80211_IF_TYPE_IBSS)
>>               return ieee80211_sta_start_scan(sdata, ssid, ssid_len);
>
> This is wrong.
>
> a != 1 || a != 2
>
> has to be true at all times, I think you mean &&.

It definitely should be &&. Copied it wrongly from the original patch.
Tomas