2008-09-08 13:31:29

by Tomas Winkler

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

From: Ester Kummer <[email protected]>

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 <[email protected]>
Acked-by: Tomas Winkler <[email protected]>
---
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.



2008-09-08 22:24:59

by Tomas Winkler

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

On Mon, Sep 8, 2008 at 9:13 PM, Johannes Berg <[email protected]> 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

2008-09-08 18:13:22

by Johannes Berg

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

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


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-10 21:37:47

by Johannes Berg

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

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


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-08 16:53:06

by Tomas Winkler

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

On Mon, Sep 8, 2008 at 6:16 PM, Johannes Berg <[email protected]> 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

>

2008-09-08 16:42:05

by Tomas Winkler

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

On Mon, Sep 8, 2008 at 5:55 PM, Johannes Berg <[email protected]> wrote:
> On Mon, 2008-09-08 at 16:31 +0300, Tomas Winkler wrote:
>> From: Ester Kummer <[email protected]>
>>
>> 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 <[email protected]>
>> Acked-by: Tomas Winkler <[email protected]>
>> ---
>> 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

2008-09-08 14:55:49

by Johannes Berg

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

On Mon, 2008-09-08 at 16:31 +0300, Tomas Winkler wrote:
> From: Ester Kummer <[email protected]>
>
> 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 <[email protected]>
> Acked-by: Tomas Winkler <[email protected]>
> ---
> 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


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-08 15:17:05

by Johannes Berg

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

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


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-10 21:32:53

by Tomas Winkler

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

On Tue, Sep 9, 2008 at 1:24 AM, Tomas Winkler <[email protected]> wrote:
> On Mon, Sep 8, 2008 at 9:13 PM, Johannes Berg <[email protected]> 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

2008-09-08 15:05:42

by Johannes Berg

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

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 <[email protected]>
> >
> > 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);
}