libertas: Set/clear WPA keys before the WEP ones

PATCH Superseded Samuel Ortiz 3 participants 5 messages Jan 18, 2010 – Jan 21, 2010 Networking drivers (wireless)

Share & tools

Samuel Ortiz

author

2010-01-18 23:17 UTC #0 lore raw

PATCH 1/1 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(-)

drivers/net/wireless/libertas/assoc.c +8 -7

index 5e650f3..fb3dff0 100644
@@ -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.
 	 */

Signature · 5 lines

1.6.3.3

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

Dan Williams

Acked-by on v1 1/1

2010-01-20 01:48 UTC #1 lore raw

Quoted 6 lines · Samuel Ortiz

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.
*/

Quoted 40 lines

---
 drivers/net/wireless/libertas/assoc.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

drivers/net/wireless/libertas/assoc.c +8 -7

index 5e650f3..fb3dff0 100644
@@ -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.
 	 */

Signature · 1 line

1.6.3.3

Samuel Ortiz

author

2010-01-21 17:22 UTC #2 lore raw

Quoted 13 lines · Dan Williams

Quoted 6 lines · Samuel Ortiz
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.

Quoted 7 lines

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.

Quoted 41 lines

Quoted 40 lines

---
 drivers/net/wireless/libertas/assoc.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

drivers/net/wireless/libertas/assoc.c +8 -7

index 5e650f3..fb3dff0 100644
@@ -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.
 	 */

Signature · 1 line

1.6.3.3

Signature · 2 lines

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

Samuel Ortiz

author

2010-01-21 17:24 UTC #3 lore raw

PATCH v2 1/1 libertas: Set/clear WPA keys before the WEP ones

In reply to Samuel Ortiz

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(-)

drivers/net/wireless/libertas/assoc.c +12 -7

index 5e650f3..e15a83a 100644
@@ -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.
 	 */

Signature · 5 lines

1.6.3.3

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

John W. Linville

2010-01-21 18:00 UTC #4 lore raw

Quoted 8 lines · Samuel Ortiz

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

Signature · 2 lines

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