All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATH 0/2] Make driver_nl80211 really work
@ 2009-10-30 21:49 Maxim Levitsky
  2009-10-30 21:52 ` [PATCH 1/2] Allow scanning while in authenticated only state Maxim Levitsky
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Maxim Levitsky @ 2009-10-30 21:49 UTC (permalink / raw)
  To: linux-wireless; +Cc: hostap

Hi,

I seem to find proper fixes for problem with driver_nl80211.
Despite workaround in wpa_supplicant that sends deauth when auth command
fails, this driver still doesn't work.

Problem is that scanning is not allowed in authenticated but not
associated state. I was told that this is feature, but afterwards I
understood that it is legal for wpa_supplicant not to use deauthenticate
command.

In the other hand mac80211 holds a work list of things todo that include
pings, associations, authentications, etc...

Following the code it seems that this list is set to hold forever
entries that belong to associated but not authenticated access points.
For example ieee80211_mgd_deauth will fail if this condition isn't true.

So I modified __ieee80211_start_scan to work even if work list isn't
empty but all entries are idle.

As for second problem, the lack of ability to do authentication to
access point twice, I modified nl80211 by 'roughly' removing the check
for this condition, treating new request as if ap was never
authenticated. This seems to work very well.


Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] Allow scanning while in authenticated only state
  2009-10-30 21:49 [PATH 0/2] Make driver_nl80211 really work Maxim Levitsky
@ 2009-10-30 21:52 ` Maxim Levitsky
  2009-10-31  5:43   ` Johannes Berg
  2009-10-30 21:54 ` [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated Maxim Levitsky
  2009-10-30 22:33 ` [PATH 0/2] Make driver_nl80211 really work Maxim Levitsky
  2 siblings, 1 reply; 14+ messages in thread
From: Maxim Levitsky @ 2009-10-30 21:52 UTC (permalink / raw)
  To: linux-wireless; +Cc: hostap

>From 1d13f997f652c7e632d4ddb053df3f3fad78da23 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Fri, 30 Oct 2009 23:36:20 +0200
Subject: [PATCH 1/2] Allow scanning while in authenticated only state

Since ifmgd->work_list is {ab}used as a storage for authenticated,
but not associated access points (this is done using idle work items),
allow scanning if all work items are in this state.

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 net/mac80211/scan.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index c46ac01..c932765 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -418,6 +418,7 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+	struct ieee80211_mgd_work *wk, *tmp;
 	int rc;
 
 	if (local->scan_req)
@@ -450,9 +451,14 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
 	if (req != local->int_scan_req &&
 	    sdata->vif.type == NL80211_IFTYPE_STATION &&
 	    !list_empty(&ifmgd->work_list)) {
-		/* actually wait for the work it's doing to finish/time out */
-		set_bit(IEEE80211_STA_REQ_SCAN, &ifmgd->request);
-		return 0;
+
+		/* actually wait for the work it's doing to finish/time out*/
+		list_for_each_entry_safe(wk, tmp, &ifmgd->work_list, list) {
+			if (wk->state != IEEE80211_MGD_STATE_IDLE) {
+				set_bit(IEEE80211_STA_REQ_SCAN, &ifmgd->request);
+				return 0;
+			}
+		}
 	}
 
 	if (local->ops->hw_scan)
-- 
1.6.3.3



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated
  2009-10-30 21:49 [PATH 0/2] Make driver_nl80211 really work Maxim Levitsky
  2009-10-30 21:52 ` [PATCH 1/2] Allow scanning while in authenticated only state Maxim Levitsky
@ 2009-10-30 21:54 ` Maxim Levitsky
  2009-10-31  5:44   ` Johannes Berg
  2009-10-30 22:33 ` [PATH 0/2] Make driver_nl80211 really work Maxim Levitsky
  2 siblings, 1 reply; 14+ messages in thread
From: Maxim Levitsky @ 2009-10-30 21:54 UTC (permalink / raw)
  To: linux-wireless; +Cc: hostap

>From 02be2525b95ec3c57323dda1a9e0c6da3a9817e6 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Fri, 30 Oct 2009 23:50:27 +0200
Subject: [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated

This is permitted by standards, and used by driver_nl80211 of wpa_supplicant
Just start new authentication as if we weren't authenticated before
---
 net/wireless/mlme.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 83c2a28..3d91b4e 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -372,7 +372,7 @@ int __cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
 		if (wdev->auth_bsses[i] &&
 		    memcmp(bssid, wdev->auth_bsses[i]->pub.bssid,
 						ETH_ALEN) == 0)
-			return -EALREADY;
+			wdev->auth_bsses[i] = NULL;
 	}
 
 	memset(&req, 0, sizeof(req));
-- 
1.6.3.3



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATH 0/2] Make driver_nl80211 really work
  2009-10-30 21:49 [PATH 0/2] Make driver_nl80211 really work Maxim Levitsky
  2009-10-30 21:52 ` [PATCH 1/2] Allow scanning while in authenticated only state Maxim Levitsky
  2009-10-30 21:54 ` [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated Maxim Levitsky
@ 2009-10-30 22:33 ` Maxim Levitsky
  2 siblings, 0 replies; 14+ messages in thread
From: Maxim Levitsky @ 2009-10-30 22:33 UTC (permalink / raw)
  To: linux-wireless; +Cc: hostap

On Fri, 2009-10-30 at 23:49 +0200, Maxim Levitsky wrote: 
> Hi,
> 
> I seem to find proper fixes for problem with driver_nl80211.
> Despite workaround in wpa_supplicant that sends deauth when auth command
> fails, this driver still doesn't work.
> 
> Problem is that scanning is not allowed in authenticated but not
> associated state. I was told that this is feature, but afterwards I
> understood that it is legal for wpa_supplicant not to use deauthenticate
> command.
> 
> In the other hand mac80211 holds a work list of things todo that include
> pings, associations, authentications, etc...
> 
> Following the code it seems that this list is set to hold forever
> entries that belong to associated but not authenticated access points.
> For example ieee80211_mgd_deauth will fail if this condition isn't true.
> 
> So I modified __ieee80211_start_scan to work even if work list isn't
> empty but all entries are idle.
> 
> As for second problem, the lack of ability to do authentication to
> access point twice, I modified nl80211 by 'roughly' removing the check
> for this condition, treating new request as if ap was never
> authenticated. This seems to work very well.
> 
> 
> Best regards,
> Maxim Levitsky
> 


However, even with these patches, I found test case when it doesn't
work.
If I connect to different APs, then driver_nl80211 doesn't send deauth
at all.

On the other hand, nl80211/cfg80211 keeps track of all auth bsses, in
list that has 4 entries. As soon as this list is full, its not possible
to connect to a different AP.

Mess isn't it?

Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Allow scanning while in authenticated only state
  2009-10-30 21:52 ` [PATCH 1/2] Allow scanning while in authenticated only state Maxim Levitsky
@ 2009-10-31  5:43   ` Johannes Berg
  2009-10-31  9:46     ` Maxim Levitsky
  0 siblings, 1 reply; 14+ messages in thread
From: Johannes Berg @ 2009-10-31  5:43 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: linux-wireless, hostap

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

On Fri, 2009-10-30 at 23:52 +0200, Maxim Levitsky wrote:
> >From 1d13f997f652c7e632d4ddb053df3f3fad78da23 Mon Sep 17 00:00:00 2001
> From: Maxim Levitsky <maximlevitsky@gmail.com>
> Date: Fri, 30 Oct 2009 23:36:20 +0200
> Subject: [PATCH 1/2] Allow scanning while in authenticated only state
> 
> Since ifmgd->work_list is {ab}used as a storage for authenticated,
> but not associated access points (this is done using idle work items),
> allow scanning if all work items are in this state.
> 
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

I don't think this is needed. While not _strictly_ wrong, I still
dislike the additional complexity.

I think you just need this wpa_supplicant patch:
http://johannes.sipsolutions.net/patches/hostap/all/2009-10-26-12%3a59/deauth-on-disassoc.patch

johannes

> ---
>  net/mac80211/scan.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
> index c46ac01..c932765 100644
> --- a/net/mac80211/scan.c
> +++ b/net/mac80211/scan.c
> @@ -418,6 +418,7 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
>  {
>  	struct ieee80211_local *local = sdata->local;
>  	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
> +	struct ieee80211_mgd_work *wk, *tmp;
>  	int rc;
>  
>  	if (local->scan_req)
> @@ -450,9 +451,14 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
>  	if (req != local->int_scan_req &&
>  	    sdata->vif.type == NL80211_IFTYPE_STATION &&
>  	    !list_empty(&ifmgd->work_list)) {
> -		/* actually wait for the work it's doing to finish/time out */
> -		set_bit(IEEE80211_STA_REQ_SCAN, &ifmgd->request);
> -		return 0;
> +
> +		/* actually wait for the work it's doing to finish/time out*/
> +		list_for_each_entry_safe(wk, tmp, &ifmgd->work_list, list) {
> +			if (wk->state != IEEE80211_MGD_STATE_IDLE) {
> +				set_bit(IEEE80211_STA_REQ_SCAN, &ifmgd->request);
> +				return 0;
> +			}
> +		}
>  	}
>  
>  	if (local->ops->hw_scan)


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated
  2009-10-30 21:54 ` [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated Maxim Levitsky
@ 2009-10-31  5:44   ` Johannes Berg
  2009-10-31  9:36     ` Maxim Levitsky
  0 siblings, 1 reply; 14+ messages in thread
From: Johannes Berg @ 2009-10-31  5:44 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: linux-wireless, hostap

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

On Fri, 2009-10-30 at 23:54 +0200, Maxim Levitsky wrote:
> >From 02be2525b95ec3c57323dda1a9e0c6da3a9817e6 Mon Sep 17 00:00:00 2001
> From: Maxim Levitsky <maximlevitsky@gmail.com>
> Date: Fri, 30 Oct 2009 23:50:27 +0200
> Subject: [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated
> 
> This is permitted by standards, and used by driver_nl80211 of wpa_supplicant
> Just start new authentication as if we weren't authenticated before

NACK. This leaks the auth request here and in mac80211, so we cannot do
it this simply. Again though, the case you're running into should be
fixed by that wpa_supplicant patch.

johannes

> ---
>  net/wireless/mlme.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
> index 83c2a28..3d91b4e 100644
> --- a/net/wireless/mlme.c
> +++ b/net/wireless/mlme.c
> @@ -372,7 +372,7 @@ int __cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
>  		if (wdev->auth_bsses[i] &&
>  		    memcmp(bssid, wdev->auth_bsses[i]->pub.bssid,
>  						ETH_ALEN) == 0)
> -			return -EALREADY;
> +			wdev->auth_bsses[i] = NULL;
>  	}
>  
>  	memset(&req, 0, sizeof(req));


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated
  2009-10-31  5:44   ` Johannes Berg
@ 2009-10-31  9:36     ` Maxim Levitsky
  0 siblings, 0 replies; 14+ messages in thread
From: Maxim Levitsky @ 2009-10-31  9:36 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, hostap

On Sat, 2009-10-31 at 06:44 +0100, Johannes Berg wrote: 
> On Fri, 2009-10-30 at 23:54 +0200, Maxim Levitsky wrote:
> > >From 02be2525b95ec3c57323dda1a9e0c6da3a9817e6 Mon Sep 17 00:00:00 2001
> > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > Date: Fri, 30 Oct 2009 23:50:27 +0200
> > Subject: [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated
> > 
> > This is permitted by standards, and used by driver_nl80211 of wpa_supplicant
> > Just start new authentication as if we weren't authenticated before
> 
> NACK. This leaks the auth request here and in mac80211, so we cannot do
> it this simply. Again though, the case you're running into should be
> fixed by that wpa_supplicant patch.

I was afraid of that... oh well thanks.

Best regards,
Maxim Levitsky

> 
> johannes
> 
> > ---
> >  net/wireless/mlme.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
> > index 83c2a28..3d91b4e 100644
> > --- a/net/wireless/mlme.c
> > +++ b/net/wireless/mlme.c
> > @@ -372,7 +372,7 @@ int __cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
> >  		if (wdev->auth_bsses[i] &&
> >  		    memcmp(bssid, wdev->auth_bsses[i]->pub.bssid,
> >  						ETH_ALEN) == 0)
> > -			return -EALREADY;
> > +			wdev->auth_bsses[i] = NULL;
> >  	}
> >  
> >  	memset(&req, 0, sizeof(req));
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Allow scanning while in authenticated only state
  2009-10-31  5:43   ` Johannes Berg
@ 2009-10-31  9:46     ` Maxim Levitsky
  2009-10-31 10:02       ` Johannes Berg
  0 siblings, 1 reply; 14+ messages in thread
From: Maxim Levitsky @ 2009-10-31  9:46 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, hostap

On Sat, 2009-10-31 at 06:43 +0100, Johannes Berg wrote: 
> On Fri, 2009-10-30 at 23:52 +0200, Maxim Levitsky wrote:
> > >From 1d13f997f652c7e632d4ddb053df3f3fad78da23 Mon Sep 17 00:00:00 2001
> > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > Date: Fri, 30 Oct 2009 23:36:20 +0200
> > Subject: [PATCH 1/2] Allow scanning while in authenticated only state
> > 
> > Since ifmgd->work_list is {ab}used as a storage for authenticated,
> > but not associated access points (this is done using idle work items),
> > allow scanning if all work items are in this state.
> > 
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> 
> I don't think this is needed. While not _strictly_ wrong, I still
> dislike the additional complexity.
> 
> I think you just need this wpa_supplicant patch:
> http://johannes.sipsolutions.net/patches/hostap/all/2009-10-26-12%3a59/deauth-on-disassoc.patch

Nope, this patch doesn't help.

if I remove the bssid_changed, then it seems to work when connecting to
same AP, but still scan hangs when connecting to different.

The problem is that wpa_supplicant doesn't do deauthentication
explicitly, and I was told that this is right thing to do. This confuses
all the nl80211 and mac80211...


I also think that its best just to do both deauthentication and
disassociation in same time.

Yet, I think this path is ok, that is, when you disallow scanning, you
can be sure that wifi will be dead, and this patch catches the situation
when it for sure will disallowed  forever.


Best regards,
Maxim Levitsky


> 
> johannes
> 
> > ---
> >  net/mac80211/scan.c |   12 +++++++++---
> >  1 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
> > index c46ac01..c932765 100644
> > --- a/net/mac80211/scan.c
> > +++ b/net/mac80211/scan.c
> > @@ -418,6 +418,7 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
> >  {
> >  	struct ieee80211_local *local = sdata->local;
> >  	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
> > +	struct ieee80211_mgd_work *wk, *tmp;
> >  	int rc;
> >  
> >  	if (local->scan_req)
> > @@ -450,9 +451,14 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
> >  	if (req != local->int_scan_req &&
> >  	    sdata->vif.type == NL80211_IFTYPE_STATION &&
> >  	    !list_empty(&ifmgd->work_list)) {
> > -		/* actually wait for the work it's doing to finish/time out */
> > -		set_bit(IEEE80211_STA_REQ_SCAN, &ifmgd->request);
> > -		return 0;
> > +
> > +		/* actually wait for the work it's doing to finish/time out*/
> > +		list_for_each_entry_safe(wk, tmp, &ifmgd->work_list, list) {
> > +			if (wk->state != IEEE80211_MGD_STATE_IDLE) {
> > +				set_bit(IEEE80211_STA_REQ_SCAN, &ifmgd->request);
> > +				return 0;
> > +			}
> > +		}
> >  	}
> >  
> >  	if (local->ops->hw_scan)
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Allow scanning while in authenticated only state
  2009-10-31  9:46     ` Maxim Levitsky
@ 2009-10-31 10:02       ` Johannes Berg
  2009-10-31 10:33         ` Maxim Levitsky
  0 siblings, 1 reply; 14+ messages in thread
From: Johannes Berg @ 2009-10-31 10:02 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: linux-wireless, hostap

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

On Sat, 2009-10-31 at 11:46 +0200, Maxim Levitsky wrote:

> > I think you just need this wpa_supplicant patch:
> > http://johannes.sipsolutions.net/patches/hostap/all/2009-10-26-12%3a59/deauth-on-disassoc.patch
> 
> Nope, this patch doesn't help.
> 
> if I remove the bssid_changed, then it seems to work when connecting to
> same AP, but still scan hangs when connecting to different.

Hmm, Luis said it worked when connecting to a different AP. But he got
assoc refused and/or disassoc from the AP

> The problem is that wpa_supplicant doesn't do deauthentication
> explicitly, and I was told that this is right thing to do. This confuses
> all the nl80211 and mac80211...

I thought Jouni committed that workaround. I'm still not convinced that
wpa_supplicant is doing the right thing.

> I also think that its best just to do both deauthentication and
> disassociation in same time.

But then why do we bother?

> Yet, I think this path is ok, that is, when you disallow scanning, you
> can be sure that wifi will be dead, and this patch catches the situation
> when it for sure will disallowed  forever.

True, in some sense, but the real problem is that the authentication is
lingering along. If we really need to clean that up in the kernel then
we'd have to time out authentication structs that didn't get promoted to
association, instead of just ignoring them like your two patches
implement [1]. However, since we put control of all this into
wpa_supplicant, I don't know whether the kernel really should be
required to clean up -- I was going to write "clean up after it" but
that's not true [2], you're proposing that the kernel clean up *while*
wpa_supplicant is in control. I don't think that's right, since it might
actually want to hang on for an authentication while doing something
else, for a while at least. How can we calculate an upper bound on that
in the kernel?

Now, I can kinda agree that we should allow authentication frames to go
through while already authenticated, but that's something quite
different although your patches here would also allow that in some
hackish way (by accumulating more and more authentication structs that
are then ignored).

Now, especially since you say that this still runs into problems while
connecting to a new AP, I think that wpa_supplicant should
deauthenticate from the old AP. After all, it wants to eventually
control FT and anything we do in the kernel will come back and interfere
with that.

johannes

[1] and that'd have to be in cfg80211
[2] Except when the interface is taken down, but that is already cleaned
up by cfg80211.


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Allow scanning while in authenticated only state
  2009-10-31 10:02       ` Johannes Berg
@ 2009-10-31 10:33         ` Maxim Levitsky
  2009-10-31 10:44           ` Johannes Berg
  2009-11-01 20:10           ` Jouni Malinen
  0 siblings, 2 replies; 14+ messages in thread
From: Maxim Levitsky @ 2009-10-31 10:33 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, hostap

On Sat, 2009-10-31 at 11:02 +0100, Johannes Berg wrote: 
> On Sat, 2009-10-31 at 11:46 +0200, Maxim Levitsky wrote:
> 
> > > I think you just need this wpa_supplicant patch:
> > > http://johannes.sipsolutions.net/patches/hostap/all/2009-10-26-12%3a59/deauth-on-disassoc.patch
> > 
> > Nope, this patch doesn't help.
> > 
> > if I remove the bssid_changed, then it seems to work when connecting to
> > same AP, but still scan hangs when connecting to different.
> 
> Hmm, Luis said it worked when connecting to a different AP. But he got
> assoc refused and/or disassoc from the AP
> 
> > The problem is that wpa_supplicant doesn't do deauthentication
> > explicitly, and I was told that this is right thing to do. This confuses
> > all the nl80211 and mac80211...
> 
> I thought Jouni committed that workaround. I'm still not convinced that
> wpa_supplicant is doing the right thing.
> 
> > I also think that its best just to do both deauthentication and
> > disassociation in same time.
> 
> But then why do we bother?
> 
> > Yet, I think this path is ok, that is, when you disallow scanning, you
> > can be sure that wifi will be dead, and this patch catches the situation
> > when it for sure will disallowed  forever.
> 
> True, in some sense, but the real problem is that the authentication is
> lingering along. If we really need to clean that up in the kernel then
> we'd have to time out authentication structs that didn't get promoted to
> association, instead of just ignoring them like your two patches
> implement [1]. However, since we put control of all this into
> wpa_supplicant, I don't know whether the kernel really should be
> required to clean up -- I was going to write "clean up after it" but
> that's not true [2], you're proposing that the kernel clean up *while*
> wpa_supplicant is in control. I don't think that's right, since it might
> actually want to hang on for an authentication while doing something
> else, for a while at least. How can we calculate an upper bound on that
> in the kernel?
> 
> Now, I can kinda agree that we should allow authentication frames to go
> through while already authenticated, but that's something quite
> different although your patches here would also allow that in some
> hackish way (by accumulating more and more authentication structs that
> are then ignored).
> 
> Now, especially since you say that this still runs into problems while
> connecting to a new AP, I think that wpa_supplicant should
> deauthenticate from the old AP. After all, it wants to eventually
> control FT and anything we do in the kernel will come back and interfere
> with that.
Exactly, at least for now.

How about putting this in wpa_supplicant, and end all trouble with this
for once?

This is a workaround/hack, but at least it works....

Best regards,
Maxim Levitsky

---


commit e57bfd6e760c32177ab74c462839dd20a92343b8
Author: Maxim Levitsky <maximlevitsky@gmail.com>
Date:   Sat Oct 31 12:05:35 2009 +0200

    driver_nl80211: send deauth on disassoc

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index b3861f5..dfed87d 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -2046,12 +2046,18 @@ static int wpa_driver_nl80211_deauthenticate(void *priv, const u8 *addr,
 static int wpa_driver_nl80211_disassociate(void *priv, const u8 *addr,
                                           int reason_code)
 {
+       int err;
        struct wpa_driver_nl80211_data *drv = priv;
        if (!(drv->capa.flags & WPA_DRIVER_FLAGS_SME))
                return wpa_driver_nl80211_disconnect(drv, addr, reason_code);
        wpa_printf(MSG_DEBUG, "%s", __func__);
        drv->associated = 0;
-       return wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DISASSOCIATE,
+
+       err = wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DISASSOCIATE,
+                                      reason_code);
+       if (err)
+               return err;
+       return wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DEAUTHENTICATE,
                                       reason_code);
 }
 



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Allow scanning while in authenticated only state
  2009-10-31 10:33         ` Maxim Levitsky
@ 2009-10-31 10:44           ` Johannes Berg
  2009-11-01 20:10           ` Jouni Malinen
  1 sibling, 0 replies; 14+ messages in thread
From: Johannes Berg @ 2009-10-31 10:44 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: linux-wireless, hostap

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

On Sat, 2009-10-31 at 12:33 +0200, Maxim Levitsky wrote:

> > Now, especially since you say that this still runs into problems while
> > connecting to a new AP, I think that wpa_supplicant should
> > deauthenticate from the old AP. After all, it wants to eventually
> > control FT and anything we do in the kernel will come back and interfere
> > with that.
> Exactly, at least for now.
> 
> How about putting this in wpa_supplicant, and end all trouble with this
> for once?
> 
> This is a workaround/hack, but at least it works....

I'm all for fixing the problem in wpa_supplicant, and it should be
plenty clear that I believe that except for the little auth-while-auth
thing, the bug _is_ in wpa_supplicant.

However, it's not my decision. I'll fix the auth-while-auth eventually,
but I believe it is not really the problem in your case.

johannes

> commit e57bfd6e760c32177ab74c462839dd20a92343b8
> Author: Maxim Levitsky <maximlevitsky@gmail.com>
> Date:   Sat Oct 31 12:05:35 2009 +0200
> 
>     driver_nl80211: send deauth on disassoc
> 
> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> index b3861f5..dfed87d 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -2046,12 +2046,18 @@ static int wpa_driver_nl80211_deauthenticate(void *priv, const u8 *addr,
>  static int wpa_driver_nl80211_disassociate(void *priv, const u8 *addr,
>                                            int reason_code)
>  {
> +       int err;
>         struct wpa_driver_nl80211_data *drv = priv;
>         if (!(drv->capa.flags & WPA_DRIVER_FLAGS_SME))
>                 return wpa_driver_nl80211_disconnect(drv, addr, reason_code);
>         wpa_printf(MSG_DEBUG, "%s", __func__);
>         drv->associated = 0;
> -       return wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DISASSOCIATE,
> +
> +       err = wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DISASSOCIATE,
> +                                      reason_code);
> +       if (err)
> +               return err;
> +       return wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DEAUTHENTICATE,
>                                        reason_code);
>  }
>  
> 
> 
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Allow scanning while in authenticated only state
  2009-10-31 10:33         ` Maxim Levitsky
  2009-10-31 10:44           ` Johannes Berg
@ 2009-11-01 20:10           ` Jouni Malinen
  2009-11-02 23:17             ` Maxim Levitsky
  1 sibling, 1 reply; 14+ messages in thread
From: Jouni Malinen @ 2009-11-01 20:10 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: Johannes Berg, hostap, linux-wireless

On Sat, Oct 31, 2009 at 12:33:16PM +0200, Maxim Levitsky wrote:

> How about putting this in wpa_supplicant, and end all trouble with this
> for once?
> 
> This is a workaround/hack, but at least it works....

Could you please add a comment pointing that out and explaining that the
deauth in disassoc is there due to mac80211's inability to handle
multiple APs in authenticated-but-not-associated state?

> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
>  static int wpa_driver_nl80211_disassociate(void *priv, const u8 *addr,
>                                            int reason_code)

> -       return wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DISASSOCIATE,
> +
> +       err = wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DISASSOCIATE,
> +                                      reason_code);
> +       if (err)
> +               return err;
> +       return wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DEAUTHENTICATE,
>                                        reason_code);

There should be no need for doing both disassoc and deauth; just send
deauth only if that is needed. This will save one extra frame
transmission and speeds up roaming a bit.

Though, is this enough to handle the roaming cases where wpa_supplicant
may not try to send either disassociation or deauthentication?

-- 
Jouni Malinen                                            PGP id EFC895FA

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Allow scanning while in authenticated only state
  2009-11-01 20:10           ` Jouni Malinen
@ 2009-11-02 23:17             ` Maxim Levitsky
  2009-11-03  8:16               ` Johannes Berg
  0 siblings, 1 reply; 14+ messages in thread
From: Maxim Levitsky @ 2009-11-02 23:17 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: Johannes Berg, hostap, linux-wireless

On Sun, 2009-11-01 at 22:10 +0200, Jouni Malinen wrote: 
> On Sat, Oct 31, 2009 at 12:33:16PM +0200, Maxim Levitsky wrote:
> 
> > How about putting this in wpa_supplicant, and end all trouble with this
> > for once?
> > 
> > This is a workaround/hack, but at least it works....
> 
> Could you please add a comment pointing that out and explaining that the
> deauth in disassoc is there due to mac80211's inability to handle
> multiple APs in authenticated-but-not-associated state?
I resend this patch with proper comment.

> 
> > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> >  static int wpa_driver_nl80211_disassociate(void *priv, const u8 *addr,
> >                                            int reason_code)
> 
> > -       return wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DISASSOCIATE,
> > +
> > +       err = wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DISASSOCIATE,
> > +                                      reason_code);
> > +       if (err)
> > +               return err;
> > +       return wpa_driver_nl80211_mlme(drv, addr, NL80211_CMD_DEAUTHENTICATE,
> >                                        reason_code);
> 
> There should be no need for doing both disassoc and deauth; just send
> deauth only if that is needed. This will save one extra frame
> transmission and speeds up roaming a bit.
> 
> Though, is this enough to handle the roaming cases where wpa_supplicant
> may not try to send either disassociation or deauthentication?
Currently wpa_supplicant sends only disassociation.
deauthentication is send only in rare cases, usually due to [suspected]
authentication error.
Other that that I don't know, what I need is a clear statement about how
things should work that is:


* Should kernel allow authentication while in authenticated? I guess yes
To same AP? To different APs?

* What should kernel do if it done authentication to several APs?, but
not association.
should it timeout, or let wpa_suplicant do it?
Currently it allows 4 (or 3) such APs, and then then bugs out with
-ENOSPC


* Should kernel allow scanning while in authenticated but not associated
case? 
I have send patch to do so, I hope it will be accepted.


While 3 (or more if I didn't find all of them) problems remain, this
workaround is the only solution.


Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Allow scanning while in authenticated only state
  2009-11-02 23:17             ` Maxim Levitsky
@ 2009-11-03  8:16               ` Johannes Berg
  0 siblings, 0 replies; 14+ messages in thread
From: Johannes Berg @ 2009-11-03  8:16 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: Jouni Malinen, hostap, linux-wireless

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

On Tue, 2009-11-03 at 01:17 +0200, Maxim Levitsky wrote:

> > There should be no need for doing both disassoc and deauth; just send
> > deauth only if that is needed. This will save one extra frame
> > transmission and speeds up roaming a bit.
> > 
> > Though, is this enough to handle the roaming cases where wpa_supplicant
> > may not try to send either disassociation or deauthentication?

> Currently wpa_supplicant sends only disassociation.
> deauthentication is send only in rare cases, usually due to [suspected]
> authentication error.
> Other that that I don't know, what I need is a clear statement about how
> things should work that is:
> 
> 
> * Should kernel allow authentication while in authenticated? I guess yes
> To same AP? To different APs?

It does allow it to different APs (up to 3/4 at a time), but not to the
same AP -- I plan on fixing the latter case.

> * What should kernel do if it done authentication to several APs?, but
> not association.
> should it timeout, or let wpa_suplicant do it?
> Currently it allows 4 (or 3) such APs, and then then bugs out with
> -ENOSPC

IMHO it should not make the decision on how long authentications are
valid. Any such value might be wrong if wpa_supplicant is really slow,
say it's swapped out.

> * Should kernel allow scanning while in authenticated but not associated
> case? 
> I have send patch to do so, I hope it will be accepted.

I don't think it should, and as such don't think I want to accept the
patch. You need to make a much much better case for it if you want me to
accept it. I see no reason to ever be in a situation where there are
authentications (that aren't either killed off or used to associate) for
a long enough period of time for it to make sense to scan in that time.

johannes

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2009-11-03  8:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-30 21:49 [PATH 0/2] Make driver_nl80211 really work Maxim Levitsky
2009-10-30 21:52 ` [PATCH 1/2] Allow scanning while in authenticated only state Maxim Levitsky
2009-10-31  5:43   ` Johannes Berg
2009-10-31  9:46     ` Maxim Levitsky
2009-10-31 10:02       ` Johannes Berg
2009-10-31 10:33         ` Maxim Levitsky
2009-10-31 10:44           ` Johannes Berg
2009-11-01 20:10           ` Jouni Malinen
2009-11-02 23:17             ` Maxim Levitsky
2009-11-03  8:16               ` Johannes Berg
2009-10-30 21:54 ` [PATCH 2/2] nl80211: allow to authenticate to access point that we already authenticated Maxim Levitsky
2009-10-31  5:44   ` Johannes Berg
2009-10-31  9:36     ` Maxim Levitsky
2009-10-30 22:33 ` [PATH 0/2] Make driver_nl80211 really work Maxim Levitsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.