[PATCH 1/6] ath5k: ath5k_setup_channels cleanup and whitespace

[PATCH 1/6] ath5k: ath5k_setup_channels cleanup and whitespace

[Bruno Randolf]

Remove useless test_bit - it's not going to happen because of the way this
function is called only when that bit is set.

And fix some whitespace.

Signed-off-by: Bruno Randolf <br1@einfach.org>
Acked-by: Bob Copeland <me@bobcopeland.com>
Acked-by: Nick Kossifidis <mickflemm@gmail.com>

---

Has this been lost from my last series?
---
 drivers/net/wireless/ath/ath5k/base.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 14377ac..dae0bdc 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -262,21 +262,16 @@ static bool ath5k_is_standard_channel(short chan, enum ieee80211_band band)
 }
 
 static unsigned int
-ath5k_setup_channels(struct ath5k_hw *ah,
-		struct ieee80211_channel *channels,
-		unsigned int mode,
-		unsigned int max)
+ath5k_setup_channels(struct ath5k_hw *ah, struct ieee80211_channel *channels,
+		unsigned int mode, unsigned int max)
 {
 	unsigned int count, size, chfreq, freq, ch;
 	enum ieee80211_band band;
 
-	if (!test_bit(mode, ah->ah_modes))
-		return 0;
-
 	switch (mode) {
 	case AR5K_MODE_11A:
 		/* 1..220, but 2GHz frequencies are filtered by check_channel */
-		size = 220 ;
+		size = 220;
 		chfreq = CHANNEL_5GHZ;
 		band = IEEE80211_BAND_5GHZ;
 		break;

[PATCH 2/6] ath5k: Use local variable for capabilities

[Bruno Randolf]

Shorten some lines and make code more readable.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 drivers/net/wireless/ath/ath5k/caps.c |   39 +++++++++++++++------------------
 1 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/caps.c b/drivers/net/wireless/ath/ath5k/caps.c
index 31cad80..39baee1 100644
--- a/drivers/net/wireless/ath/ath5k/caps.c
+++ b/drivers/net/wireless/ath/ath5k/caps.c
@@ -32,23 +32,24 @@
  */
 int ath5k_hw_set_capabilities(struct ath5k_hw *ah)
 {
+	struct ath5k_capabilities *caps = &ah->ah_capabilities;
 	u16 ee_header;
 
 	/* Capabilities stored in the EEPROM */
-	ee_header = ah->ah_capabilities.cap_eeprom.ee_header;
+	ee_header = caps->cap_eeprom.ee_header;
 
 	if (ah->ah_version == AR5K_AR5210) {
 		/*
 		 * Set radio capabilities
 		 * (The AR5110 only supports the middle 5GHz band)
 		 */
-		ah->ah_capabilities.cap_range.range_5ghz_min = 5120;
-		ah->ah_capabilities.cap_range.range_5ghz_max = 5430;
-		ah->ah_capabilities.cap_range.range_2ghz_min = 0;
-		ah->ah_capabilities.cap_range.range_2ghz_max = 0;
+		caps->cap_range.range_5ghz_min = 5120;
+		caps->cap_range.range_5ghz_max = 5430;
+		caps->cap_range.range_2ghz_min = 0;
+		caps->cap_range.range_2ghz_max = 0;
 
 		/* Set supported modes */
-		__set_bit(AR5K_MODE_11A, ah->ah_capabilities.cap_mode);
+		__set_bit(AR5K_MODE_11A, caps->cap_mode);
 	} else {
 		/*
 		 * XXX The tranceiver supports frequencies from 4920 to 6100GHz
@@ -67,12 +68,11 @@ int ath5k_hw_set_capabilities(struct ath5k_hw *ah)
 
 		if (AR5K_EEPROM_HDR_11A(ee_header)) {
 			/* 4920 */
-			ah->ah_capabilities.cap_range.range_5ghz_min = 5005;
-			ah->ah_capabilities.cap_range.range_5ghz_max = 6100;
+			caps->cap_range.range_5ghz_min = 5005;
+			caps->cap_range.range_5ghz_max = 6100;
 
 			/* Set supported modes */
-			__set_bit(AR5K_MODE_11A,
-					ah->ah_capabilities.cap_mode);
+			__set_bit(AR5K_MODE_11A, caps->cap_mode);
 		}
 
 		/* Enable  802.11b if a 2GHz capable radio (2111/5112) is
@@ -81,32 +81,29 @@ int ath5k_hw_set_capabilities(struct ath5k_hw *ah)
 		    (AR5K_EEPROM_HDR_11G(ee_header) &&
 		     ah->ah_version != AR5K_AR5211)) {
 			/* 2312 */
-			ah->ah_capabilities.cap_range.range_2ghz_min = 2412;
-			ah->ah_capabilities.cap_range.range_2ghz_max = 2732;
+			caps->cap_range.range_2ghz_min = 2412;
+			caps->cap_range.range_2ghz_max = 2732;
 
 			if (AR5K_EEPROM_HDR_11B(ee_header))
-				__set_bit(AR5K_MODE_11B,
-						ah->ah_capabilities.cap_mode);
+				__set_bit(AR5K_MODE_11B, caps->cap_mode);
 
 			if (AR5K_EEPROM_HDR_11G(ee_header) &&
 			    ah->ah_version != AR5K_AR5211)
-				__set_bit(AR5K_MODE_11G,
-						ah->ah_capabilities.cap_mode);
+				__set_bit(AR5K_MODE_11G, caps->cap_mode);
 		}
 	}
 
 	/* Set number of supported TX queues */
 	if (ah->ah_version == AR5K_AR5210)
-		ah->ah_capabilities.cap_queues.q_tx_num =
-			AR5K_NUM_TX_QUEUES_NOQCU;
+		caps->cap_queues.q_tx_num = AR5K_NUM_TX_QUEUES_NOQCU;
 	else
-		ah->ah_capabilities.cap_queues.q_tx_num = AR5K_NUM_TX_QUEUES;
+		caps->cap_queues.q_tx_num = AR5K_NUM_TX_QUEUES;
 
 	/* newer hardware has PHY error counters */
 	if (ah->ah_mac_srev >= AR5K_SREV_AR5213A)
-		ah->ah_capabilities.cap_has_phyerr_counters = true;
+		caps->cap_has_phyerr_counters = true;
 	else
-		ah->ah_capabilities.cap_has_phyerr_counters = false;
+		caps->cap_has_phyerr_counters = false;
 
 	return 0;
 }

[PATCH 3/6] ath: Add function to check if 4.9GHz channels are allowed

[Bruno Randolf]

This adds a helper function to ath/regd.c which can be asked if 4.9GHz channels
are allowed for a given regulatory domain code. This keeps the knowledge of
regdomains and defines like MKK9_MKKC in one place. I'm passing the regdomain
code instead of the ath_regulatory structure because this needs to be called
quite early in the driver inititalization where ath_regulatory is not available
yet in ath5k.

I'm using MKK9_MKKC only because this is the regdomain in the 802.11j enabled
sample cards we got from our vendor. I found some hints in HAL code that this
is used by Atheros to indicate 4.9GHz channels support and that there might be
other domain codes as well, but as I don't have any documentation I'm just
putting in what I need right now. It can be extended later.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 drivers/net/wireless/ath/regd.c |    7 +++++++
 drivers/net/wireless/ath/regd.h |    1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index 2b14775..f828f29 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -158,6 +158,13 @@ ieee80211_regdomain *ath_world_regdomain(struct ath_regulatory *reg)
 	}
 }
 
+bool ath_is_49ghz_allowed(u16 regdomain)
+{
+	/* possibly more */
+	return regdomain == MKK9_MKKC;
+}
+EXPORT_SYMBOL(ath_is_49ghz_allowed);
+
 /* Frequency is one where radar detection is required */
 static bool ath_is_radar_freq(u16 center_freq)
 {
diff --git a/drivers/net/wireless/ath/regd.h b/drivers/net/wireless/ath/regd.h
index 345dd97..172f63f 100644
--- a/drivers/net/wireless/ath/regd.h
+++ b/drivers/net/wireless/ath/regd.h
@@ -250,6 +250,7 @@ enum CountryCode {
 };
 
 bool ath_is_world_regd(struct ath_regulatory *reg);
+bool ath_is_49ghz_allowed(u16 redomain);
 int ath_regd_init(struct ath_regulatory *reg, struct wiphy *wiphy,
 		  int (*reg_notifier)(struct wiphy *wiphy,
 		  struct regulatory_request *request));

[PATCH 4/6] ath5k: Enable 802.11j 4.9GHz frequencies

[Bruno Randolf]

This enables 4.9GHz frequencies in ath5k if they are allowed as indicated by
the regulatory domain code. Currently this is MKK9_MKKC (0xfe).

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 drivers/net/wireless/ath/ath5k/caps.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/caps.c b/drivers/net/wireless/ath/ath5k/caps.c
index 39baee1..f77e8a7 100644
--- a/drivers/net/wireless/ath/ath5k/caps.c
+++ b/drivers/net/wireless/ath/ath5k/caps.c
@@ -57,9 +57,8 @@ int ath5k_hw_set_capabilities(struct ath5k_hw *ah)
 		 * XXX current ieee80211 implementation because the IEEE
 		 * XXX channel mapping does not support negative channel
 		 * XXX numbers (2312MHz is channel -19). Of course, this
-		 * XXX doesn't matter because these channels are out of range
-		 * XXX but some regulation domains like MKK (Japan) will
-		 * XXX support frequencies somewhere around 4.8GHz.
+		 * XXX doesn't matter because these channels are out of the
+		 * XXX legal range.
 		 */
 
 		/*
@@ -67,8 +66,10 @@ int ath5k_hw_set_capabilities(struct ath5k_hw *ah)
 		 */
 
 		if (AR5K_EEPROM_HDR_11A(ee_header)) {
-			/* 4920 */
-			caps->cap_range.range_5ghz_min = 5005;
+			if (ath_is_49ghz_allowed(caps->cap_eeprom.ee_regdomain))
+				caps->cap_range.range_5ghz_min = 4920;
+			else
+				caps->cap_range.range_5ghz_min = 5005;
 			caps->cap_range.range_5ghz_max = 6100;
 
 			/* Set supported modes */

[PATCH 5/6] ath9k: Remove unused IEEE80211_WEP_NKID

[Bruno Randolf]

IEEE80211_WEP_NKID is not used in ath9k any more since the key handling code
has been moved to ath/.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 drivers/net/wireless/ath/ath9k/ath9k.h  |    1 -
 drivers/net/wireless/ath/ath9k/common.h |    2 --
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index dab0271..3472b84 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -537,7 +537,6 @@ struct ath_ant_comb {
 #define ATH_CABQ_READY_TIME     80      /* % of beacon interval */
 #define ATH_MAX_SW_RETRIES      10
 #define ATH_CHAN_MAX            255
-#define IEEE80211_WEP_NKID      4       /* number of key ids */
 
 #define ATH_TXPOWER_MAX         100     /* .5 dBm units */
 #define ATH_RATE_DUMMY_MARKER   0
diff --git a/drivers/net/wireless/ath/ath9k/common.h b/drivers/net/wireless/ath/ath9k/common.h
index a126bdd..4c7020b 100644
--- a/drivers/net/wireless/ath/ath9k/common.h
+++ b/drivers/net/wireless/ath/ath9k/common.h
@@ -23,8 +23,6 @@
 
 /* Common header for Atheros 802.11n base driver cores */
 
-#define IEEE80211_WEP_NKID 4
-
 #define WME_NUM_TID             16
 #define WME_BA_BMP_SIZE         64
 #define WME_MAX_BA              WME_BA_BMP_SIZE

[PATCH 6/6] ath: Fix WEP hardware encryption

[Bruno Randolf]

In AP mode hardware encryption for WEP was not used on the RX side because
there was a mismatch in the key indices. The key index in the WLAN header is
0-3 while the hardware key cache was configured for key indices >= 4. This is
ok for transmit but received packets were not decrypted in HW and therefore
mac80211 had to decrypt them in SW - this can be easily seen by adding some
debug prints in mac80211/wep.c (around line 296). I have noticed it by watching
the system CPU load under high traffic.

This patch configures WEP keys into the "standard" key indices 0-4 which were
reserved for that.

Tested with ath5k on a 600MHz mips board the difference is ~15% CPU load (with
this patch) vs. ~45% CPU load (before) when the AP receives 28Mbps of WEP
encrypted data.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 drivers/net/wireless/ath/key.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/key.c b/drivers/net/wireless/ath/key.c
index 5d465e5..5748dae 100644
--- a/drivers/net/wireless/ath/key.c
+++ b/drivers/net/wireless/ath/key.c
@@ -465,6 +465,16 @@ int ath_key_config(struct ath_common *common,
 	hk.kv_len = key->keylen;
 	memcpy(hk.kv_val, key->key, key->keylen);
 
+	/* set WEP keys directly to index */
+	if ((key->cipher == WLAN_CIPHER_SUITE_WEP40 ||
+	     key->cipher == WLAN_CIPHER_SUITE_WEP104) && key->keyidx < 4) {
+		ret = ath_hw_set_keycache_entry(common, key->keyidx, &hk, NULL);
+		if (!ret)
+			return -EIO;
+		set_bit(key->keyidx, common->keymap);
+		return key->keyidx;
+	}
+
 	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
 		switch (vif->type) {
 		case NL80211_IFTYPE_AP:
@@ -541,10 +551,8 @@ EXPORT_SYMBOL(ath_key_config);
 void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key)
 {
 	ath_hw_keyreset(common, key->hw_key_idx);
-	if (key->hw_key_idx < IEEE80211_WEP_NKID)
-		return;
-
 	clear_bit(key->hw_key_idx, common->keymap);
+
 	if (key->cipher != WLAN_CIPHER_SUITE_TKIP)
 		return;
 

Re: [PATCH 6/6] ath: Fix WEP hardware encryption

[Jouni Malinen]

On Tue, Jan 25, 2011 at 01:15:49PM +0900, Bruno Randolf wrote:
> In AP mode hardware encryption for WEP was not used on the RX side because
> there was a mismatch in the key indices. The key index in the WLAN header is
> 0-3 while the hardware key cache was configured for key indices >= 4. This is
> ok for transmit but received packets were not decrypted in HW and therefore
> mac80211 had to decrypt them in SW - this can be easily seen by adding some
> debug prints in mac80211/wep.c (around line 296). I have noticed it by watching
> the system CPU load under high traffic.
> 
> This patch configures WEP keys into the "standard" key indices 0-4 which were
> reserved for that.

Have you tested this with multiple vifs? I would assume it would break
things pretty completely for all but one vif.. As such, I'm not sure
that it would be that good of an idea to try to improve on something
like WEP which is known to be completely insecure and unusable with
802.11n when it is likely to break use cases that are much more relevant
in modern, post-WEP, time..

If you really think you need this, there would need to be code somewhere
kicking out the default keys from key cache if another vif is added.
When working with the key cache implementation for ath9k, the extra
effort did not seem to be justifiable for WEP and it is now couple of
years from that and surely there is even less justification now.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 6/6] ath: Fix WEP hardware encryption

[Bruno Randolf]

On Wed January 26 2011 03:32:05 Jouni Malinen wrote:
> On Tue, Jan 25, 2011 at 01:15:49PM +0900, Bruno Randolf wrote:
> > In AP mode hardware encryption for WEP was not used on the RX side
> > because there was a mismatch in the key indices. The key index in the
> > WLAN header is 0-3 while the hardware key cache was configured for key
> > indices >= 4. This is ok for transmit but received packets were not
> > decrypted in HW and therefore mac80211 had to decrypt them in SW - this
> > can be easily seen by adding some debug prints in mac80211/wep.c (around
> > line 296). I have noticed it by watching the system CPU load under high
> > traffic.
> > 
> > This patch configures WEP keys into the "standard" key indices 0-4 which
> > were reserved for that.
> 
> Have you tested this with multiple vifs? I would assume it would break
> things pretty completely for all but one vif.. As such, I'm not sure
> that it would be that good of an idea to try to improve on something
> like WEP which is known to be completely insecure and unusable with
> 802.11n when it is likely to break use cases that are much more relevant
> in modern, post-WEP, time..
> 
> If you really think you need this, there would need to be code somewhere
> kicking out the default keys from key cache if another vif is added.
> When working with the key cache implementation for ath9k, the extra
> effort did not seem to be justifiable for WEP and it is now couple of
> years from that and surely there is even less justification now.

Please let's not go into the discussion about the usefulness of WEP - there is 
no need to convince me. But unfortunately there are users and customers who 
still want to use WEP for a variety of reasons. Yes it's legacy, yes it's 
flawed especially with regards to multiple vifs, but we still need to support 
it.

Even without my patch, WEP does not work with multiple vifs. 
(As far as i understand it, with WEP the lookup is just done based on the key 
index in the WLAN header field. mac80211 (or is it hostapd?) sets up both keys 
for both interfaces with a key index of 0, which causes the lookup to go to 
the same key for both vifs. I guess mac80211 or hostapd would need to be 
changed to use different key indices for different vif WEP keys, but then of 
course we can only use 4 different WEP keys in sum, and not 4 different WEP 
keys per vif. No big deal imho.)

My patch just adds a special case for WEP, so it does not break anything for 
the other use cases. It improves the performance for the one vif case where 
WEP works right now.

bruno

Re: [PATCH 6/6] ath: Fix WEP hardware encryption

[Johannes Berg]

On Wed, 2011-01-26 at 11:38 +0900, Bruno Randolf wrote:

> Even without my patch, WEP does not work with multiple vifs. 
> (As far as i understand it, with WEP the lookup is just done based on the key 
> index in the WLAN header field. mac80211 (or is it hostapd?) sets up both keys 
> for both interfaces with a key index of 0, which causes the lookup to go to 
> the same key for both vifs. I guess mac80211 or hostapd would need to be 
> changed to use different key indices for different vif WEP keys, but then of 
> course we can only use 4 different WEP keys in sum, and not 4 different WEP 
> keys per vif. No big deal imho.)

Are you saying software WEP in mac80211 is broken with multiple VIFs? I
find that hard to believe :-)

johannes

Re: [ath5k-devel] [PATCH 6/6] ath: Fix WEP hardware encryption

[Bruno Randolf]

On Wed January 26 2011 17:29:48 Johannes Berg wrote:
> On Wed, 2011-01-26 at 11:38 +0900, Bruno Randolf wrote:
> > Even without my patch, WEP does not work with multiple vifs.
> > (As far as i understand it, with WEP the lookup is just done based on the
> > key index in the WLAN header field. mac80211 (or is it hostapd?) sets up
> > both keys for both interfaces with a key index of 0, which causes the
> > lookup to go to the same key for both vifs. I guess mac80211 or hostapd
> > would need to be changed to use different key indices for different vif
> > WEP keys, but then of course we can only use 4 different WEP keys in
> > sum, and not 4 different WEP keys per vif. No big deal imho.)
> 
> Are you saying software WEP in mac80211 is broken with multiple VIFs? I
> find that hard to believe :-)

No, i'm talking about the combination of HW encryption in ath5k and mac80211. 
I never tested mac80211 software WEP alone with multiple VIFs.

bruno

Re: [ath5k-devel] [PATCH 6/6] ath: Fix WEP hardware encryption

[Johannes Berg]

On Wed, 2011-01-26 at 18:21 +0900, Bruno Randolf wrote:
> On Wed January 26 2011 17:29:48 Johannes Berg wrote:
> > On Wed, 2011-01-26 at 11:38 +0900, Bruno Randolf wrote:
> > > Even without my patch, WEP does not work with multiple vifs.
> > > (As far as i understand it, with WEP the lookup is just done based on the
> > > key index in the WLAN header field. mac80211 (or is it hostapd?) sets up
> > > both keys for both interfaces with a key index of 0, which causes the
> > > lookup to go to the same key for both vifs. I guess mac80211 or hostapd
> > > would need to be changed to use different key indices for different vif
> > > WEP keys, but then of course we can only use 4 different WEP keys in
> > > sum, and not 4 different WEP keys per vif. No big deal imho.)
> > 
> > Are you saying software WEP in mac80211 is broken with multiple VIFs? I
> > find that hard to believe :-)
> 
> No, i'm talking about the combination of HW encryption in ath5k and mac80211. 
> I never tested mac80211 software WEP alone with multiple VIFs.

But wasn't Jouni saying that ath9k currently uses SW crypto so that it
doesn't have to worry about any of this -- and that that would be
broken?

johannes

Re: [PATCH 6/6] ath: Fix WEP hardware encryption

[Jouni Malinen]

On Wed, Jan 26, 2011 at 11:38:53AM +0900, Bruno Randolf wrote:

> Even without my patch, WEP does not work with multiple vifs. 

Are you sure about that? Why would there be any issues in using software
crypto for decrypting WEP frames while everything else is done in
hardware? I'm really interested in cases where only one of the vifs is
using WEP, but this should work even with multiple WEP vifs. The key is
in mac80211 using more details of the frame header in selecting which
key to use than the hardware key cache.

> My patch just adds a special case for WEP, so it does not break anything for 
> the other use cases. It improves the performance for the one vif case where 
> WEP works right now.

As far as I can tell, it will break all multi-vif cases where at least
one of the vifs is using WEP (which would be one of the only acceptable
uses of WEP as a temporary upgrade path while providing more reasonable
security on other vifs). As such, I would have to NAK this patch in its
current form.

To make this acceptable, the patch would need to handle a case where
multiple vifs are added (which may happen either before or after the WEP
keys would be set to default key indexes) and prevent the use of those
key indexes (which would include removing the already configured keys in
case of vif added after the WEP configuration on another vif).

Another alternative could be to figure out whether some of the new key
cache functionality could be used to avoid the problems in some cases,
but that may not be feasible with all the hardware revisions supported
by ath5k.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 6/6] ath: Fix WEP hardware encryption

[Bruno Randolf]

On Wed January 26 2011 18:37:00 Jouni Malinen wrote:
> On Wed, Jan 26, 2011 at 11:38:53AM +0900, Bruno Randolf wrote:
> > Even without my patch, WEP does not work with multiple vifs.
> 
> Are you sure about that? 

Yes, I'm sure. Please test it yourself, if you don't believe me. :)
I'm using ath5k, not ath9k, BTW.

> Why would there be any issues in using software
> crypto for decrypting WEP frames while everything else is done in
> hardware? 

I don't know why it doesn't work at this point, but it doesn't and this looks 
suspicious:

root@RMR1:~# cat /sys/kernel/debug/ieee80211/phy0/keys/0/hw_key_idx 
4
root@RMR1:~# cat /sys/kernel/debug/ieee80211/phy0/keys/0/keyidx 
0
root@RMR1:~# cat /sys/kernel/debug/ieee80211/phy0/keys/1/hw_key_idx 
68
root@RMR1:~# cat /sys/kernel/debug/ieee80211/phy0/keys/1/keyidx 
0

> > My patch just adds a special case for WEP, so it does not break anything
> > for the other use cases. It improves the performance for the one vif
> > case where WEP works right now.
> 
> As far as I can tell, it will break all multi-vif cases where at least
> one of the vifs is using WEP (which would be one of the only acceptable
> uses of WEP as a temporary upgrade path while providing more reasonable
> security on other vifs). As such, I would have to NAK this patch in its
> current form.

Why do you believe it would break something? It just uses key indices 0-3, 
which are not used for anything else.

I tested it with 1 WEP vif and 3 WPA vifs and it works.

> To make this acceptable, the patch would need to handle a case where
> multiple vifs are added (which may happen either before or after the WEP
> keys would be set to default key indexes) and prevent the use of those
> key indexes (which would include removing the already configured keys in
> case of vif added after the WEP configuration on another vif).

Given the fact that it works i think this is not necessary.

bruno

Re: [PATCH 6/6] ath: Fix WEP hardware encryption

[Vasanthakumar Thiagarajan]

On Wed, Jan 26, 2011 at 04:06:02PM +0530, Bruno Randolf wrote:
> On Wed January 26 2011 18:37:00 Jouni Malinen wrote:
> > On Wed, Jan 26, 2011 at 11:38:53AM +0900, Bruno Randolf wrote:
> > > Even without my patch, WEP does not work with multiple vifs.
> > 
> > Are you sure about that? 
> 
> Yes, I'm sure. Please test it yourself, if you don't believe me. :)
> I'm using ath5k, not ath9k, BTW.

Using a key cache entry without seeing it's availability would
certainly break multi vif operation with WEP in at least one
interface. Did you test the following

1. Associate sta vif1 with TKIP and
2. Bring up ap vif2 with WEP and configure the key at index 1.

With your patch, the AP wep key would probably override the GTK of STA
interface. 

Vasanth

Re: [PATCH 6/6] ath: Fix WEP hardware encryption

[Bruno Randolf]

On Thu January 27 2011 14:51:05 Vasanthakumar Thiagarajan wrote:
> On Wed, Jan 26, 2011 at 04:06:02PM +0530, Bruno Randolf wrote:
> > On Wed January 26 2011 18:37:00 Jouni Malinen wrote:
> > > On Wed, Jan 26, 2011 at 11:38:53AM +0900, Bruno Randolf wrote:
> > > > Even without my patch, WEP does not work with multiple vifs.
> > > 
> > > Are you sure about that?
> > 
> > Yes, I'm sure. Please test it yourself, if you don't believe me. :)
> > I'm using ath5k, not ath9k, BTW.
> 
> Using a key cache entry without seeing it's availability would
> certainly break multi vif operation with WEP in at least one
> interface. Did you test the following
> 
> 1. Associate sta vif1 with TKIP and
> 2. Bring up ap vif2 with WEP and configure the key at index 1.
> 
> With your patch, the AP wep key would probably override the GTK of STA
> interface.

Ok, right. I have to admit that i did not think about this case and just about 
multiple AP interfaces. So please just drop my patch for now.

bruno