2010-01-18 23:17:48

by Samuel Ortiz

Subject: [PATCH] libertas: Set/clear WPA keys before the WEP ones

With the v10 firmware running on 8688 HW, clearing WPA keys after setting the
WEP key prevents us from being able to associate with WEP APs.
Swapping the calling order for assoc_helper_wpa_keys() and
assoc_helper_wep_keys fixes that issue.

Signed-off-by: Samuel Ortiz <[email protected]>
---
drivers/net/wireless/libertas/assoc.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/libertas/assoc.c b/drivers/net/wireless/libertas/assoc.c
index 5e650f3..fb3dff0 100644
--- a/drivers/net/wireless/libertas/assoc.c
+++ b/drivers/net/wireless/libertas/assoc.c
@@ -2052,13 +2052,6 @@ void lbs_association_worker(struct work_struct *work)
goto out;
}

- if ( test_bit(ASSOC_FLAG_WEP_KEYS, &assoc_req->flags)
- || test_bit(ASSOC_FLAG_WEP_TX_KEYIDX, &assoc_req->flags)) {
- ret = assoc_helper_wep_keys(priv, assoc_req);
- if (ret)
- goto out;
- }
-
if (test_bit(ASSOC_FLAG_SECINFO, &assoc_req->flags)) {
ret = assoc_helper_secinfo(priv, assoc_req);
if (ret)
@@ -2078,6 +2071,14 @@ void lbs_association_worker(struct work_struct *work)
goto out;
}

+ if ( test_bit(ASSOC_FLAG_WEP_KEYS, &assoc_req->flags)
+ || test_bit(ASSOC_FLAG_WEP_TX_KEYIDX, &assoc_req->flags)) {
+ ret = assoc_helper_wep_keys(priv, assoc_req);
+ if (ret)
+ goto out;
+ }
+
+
/* SSID/BSSID should be the _last_ config option set, because they
* trigger the association attempt.
*/
--
1.6.3.3

--
Intel Open Source Technology Centre
http://oss.intel.com/

2010-01-21 18:00:37

by John W. Linville

Subject: Re: [PATCH v2] libertas: Set/clear WPA keys before the WEP ones

On Thu, Jan 21, 2010 at 06:26:19PM +0100, Samuel Ortiz wrote:
>
> With the v10 firmware running on 8688 HW, clearing WPA keys after setting the
> WEP key prevents us from being able to associate with WEP APs.
> Swapping the calling order for assoc_helper_wpa_keys() and
> assoc_helper_wep_keys() fixes that issue.
>
> Acked-by: Dan Williams <[email protected]>
> Signed-off-by: Samuel Ortiz <[email protected]>

I already applied the first version of the patch. Could you send
one w/ just the new changes?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-01-21 17:22:31

by Samuel Ortiz

Subject: Re: [PATCH] libertas: Set/clear WPA keys before the WEP ones

On Tue, Jan 19, 2010 at 05:48:12PM -0800, Dan Williams wrote:
> On Tue, 2010-01-19 at 00:19 +0100, Samuel Ortiz wrote:
> > With the v10 firmware running on 8688 HW, clearing WPA keys after setting the
> > WEP key prevents us from being able to associate with WEP APs.
> > Swapping the calling order for assoc_helper_wpa_keys() and
> > assoc_helper_wep_keys fixes that issue.
> >
> > Signed-off-by: Samuel Ortiz <[email protected]>
>
> Acked-by: Dan Williams <[email protected]>
>
> Tested on both sd8686 (v9 fw) and usb8388 (v5 OLPC fw). Switching
> between WEP and WPA-PSK on-the-fly works as expected with this patch.
>
> Though maybe we want to add a comment about this specific issue that
> says something like:
>
> /* v10 FW wants WPA keys to be set/cleared after WEP key operations,
> * otherwise it will fail to correctly associate to WEP networks.
> * Other firmware versions don't appear to care.
> */
Yes, I can add these comments. Thanks for testing it, I'll send a new version
of the patch.

Cheers,
Samuel.


> > ---
> > drivers/net/wireless/libertas/assoc.c | 15 ++++++++-------
> > 1 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/libertas/assoc.c b/drivers/net/wireless/libertas/assoc.c
> > index 5e650f3..fb3dff0 100644
> > --- a/drivers/net/wireless/libertas/assoc.c
> > +++ b/drivers/net/wireless/libertas/assoc.c
> > @@ -2052,13 +2052,6 @@ void lbs_association_worker(struct work_struct *work)
> > goto out;
> > }
> >
> > - if ( test_bit(ASSOC_FLAG_WEP_KEYS, &assoc_req->flags)
> > - || test_bit(ASSOC_FLAG_WEP_TX_KEYIDX, &assoc_req->flags)) {
> > - ret = assoc_helper_wep_keys(priv, assoc_req);
> > - if (ret)
> > - goto out;
> > - }
> > -
> > if (test_bit(ASSOC_FLAG_SECINFO, &assoc_req->flags)) {
> > ret = assoc_helper_secinfo(priv, assoc_req);
> > if (ret)
> > @@ -2078,6 +2071,14 @@ void lbs_association_worker(struct work_struct *work)
> > goto out;
> > }
> >
> > + if ( test_bit(ASSOC_FLAG_WEP_KEYS, &assoc_req->flags)
> > + || test_bit(ASSOC_FLAG_WEP_TX_KEYIDX, &assoc_req->flags)) {
> > + ret = assoc_helper_wep_keys(priv, assoc_req);
> > + if (ret)
> > + goto out;
> > + }
> > +
> > +
> > /* SSID/BSSID should be the _last_ config option set, because they
> > * trigger the association attempt.
> > */
> > --
> > 1.6.3.3
> >
>

--
Intel Open Source Technology Centre
http://oss.intel.com/

2010-01-21 17:24:49

by Samuel Ortiz

Subject: [PATCH v2] libertas: Set/clear WPA keys before the WEP ones

With the v10 firmware running on 8688 HW, clearing WPA keys after setting the
WEP key prevents us from being able to associate with WEP APs.
Swapping the calling order for assoc_helper_wpa_keys() and
assoc_helper_wep_keys() fixes that issue.

Acked-by: Dan Williams <[email protected]>
Signed-off-by: Samuel Ortiz <[email protected]>
---
drivers/net/wireless/libertas/assoc.c | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/libertas/assoc.c b/drivers/net/wireless/libertas/assoc.c
index 5e650f3..e15a83a 100644
--- a/drivers/net/wireless/libertas/assoc.c
+++ b/drivers/net/wireless/libertas/assoc.c
@@ -2052,13 +2052,6 @@ void lbs_association_worker(struct work_struct *work)
goto out;
}

- if ( test_bit(ASSOC_FLAG_WEP_KEYS, &assoc_req->flags)
- || test_bit(ASSOC_FLAG_WEP_TX_KEYIDX, &assoc_req->flags)) {
- ret = assoc_helper_wep_keys(priv, assoc_req);
- if (ret)
- goto out;
- }
-
if (test_bit(ASSOC_FLAG_SECINFO, &assoc_req->flags)) {
ret = assoc_helper_secinfo(priv, assoc_req);
if (ret)
@@ -2071,6 +2064,10 @@ void lbs_association_worker(struct work_struct *work)
goto out;
}

+ /* v10 FW wants WPA keys to be set/cleared before WEP key operations,
+ * otherwise it will fail to correctly associate to WEP networks.
+ * Other firmware versions don't appear to care.
+ */
if (test_bit(ASSOC_FLAG_WPA_MCAST_KEY, &assoc_req->flags)
|| test_bit(ASSOC_FLAG_WPA_UCAST_KEY, &assoc_req->flags)) {
ret = assoc_helper_wpa_keys(priv, assoc_req);
@@ -2078,6 +2075,14 @@ void lbs_association_worker(struct work_struct *work)
goto out;
}

+ if (test_bit(ASSOC_FLAG_WEP_KEYS, &assoc_req->flags)
+ || test_bit(ASSOC_FLAG_WEP_TX_KEYIDX, &assoc_req->flags)) {
+ ret = assoc_helper_wep_keys(priv, assoc_req);
+ if (ret)
+ goto out;
+ }
+
+
/* SSID/BSSID should be the _last_ config option set, because they
* trigger the association attempt.
*/
--
1.6.3.3

--
Intel Open Source Technology Centre
http://oss.intel.com/

2010-01-20 01:48:14

by Dan Williams

Subject: Re: [PATCH] libertas: Set/clear WPA keys before the WEP ones

On Tue, 2010-01-19 at 00:19 +0100, Samuel Ortiz wrote:
> With the v10 firmware running on 8688 HW, clearing WPA keys after setting the
> WEP key prevents us from being able to associate with WEP APs.
> Swapping the calling order for assoc_helper_wpa_keys() and
> assoc_helper_wep_keys fixes that issue.
>
> Signed-off-by: Samuel Ortiz <[email protected]>

Acked-by: Dan Williams <[email protected]>

Tested on both sd8686 (v9 fw) and usb8388 (v5 OLPC fw). Switching
between WEP and WPA-PSK on-the-fly works as expected with this patch.

Though maybe we want to add a comment about this specific issue that
says something like:

/* v10 FW wants WPA keys to be set/cleared after WEP key operations,
* otherwise it will fail to correctly associate to WEP networks.
* Other firmware versions don't appear to care.
*/

> ---
> drivers/net/wireless/libertas/assoc.c | 15 ++++++++-------
> 1 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/assoc.c b/drivers/net/wireless/libertas/assoc.c
> index 5e650f3..fb3dff0 100644
> --- a/drivers/net/wireless/libertas/assoc.c
> +++ b/drivers/net/wireless/libertas/assoc.c
> @@ -2052,13 +2052,6 @@ void lbs_association_worker(struct work_struct *work)
> goto out;
> }
>
> - if ( test_bit(ASSOC_FLAG_WEP_KEYS, &assoc_req->flags)
> - || test_bit(ASSOC_FLAG_WEP_TX_KEYIDX, &assoc_req->flags)) {
> - ret = assoc_helper_wep_keys(priv, assoc_req);
> - if (ret)
> - goto out;
> - }
> -
> if (test_bit(ASSOC_FLAG_SECINFO, &assoc_req->flags)) {
> ret = assoc_helper_secinfo(priv, assoc_req);
> if (ret)
> @@ -2078,6 +2071,14 @@ void lbs_association_worker(struct work_struct *work)
> goto out;
> }
>
> + if ( test_bit(ASSOC_FLAG_WEP_KEYS, &assoc_req->flags)
> + || test_bit(ASSOC_FLAG_WEP_TX_KEYIDX, &assoc_req->flags)) {
> + ret = assoc_helper_wep_keys(priv, assoc_req);
> + if (ret)
> + goto out;
> + }
> +
> +
> /* SSID/BSSID should be the _last_ config option set, because they
> * trigger the association attempt.
> */
> --
> 1.6.3.3
>