Skip to contentSkip to conversation

mac80211: Replace ieee80211_tx_control->key_idx with ieee80211_key_conf

PATCH Superseded Ivo van Doorn 4 participants 10 messages Apr 17, 2008 – Apr 18, 2008

Subsystems (2)
Networking drivers (wireless) mac80211
lore
Share & tools
lore

Ivo van Doorn

author
#0 lore raw

PATCH 1/1 mac80211: Replace ieee80211_tx_control->key_idx with ieee80211_key_conf

The hw_key_idx inside the ieee80211_key_conf structure does not provide all the information drivers might need to perform hardware encryption.

This is in particular true for rt2x00 who needs to know the key algorithm and whether it is a shared or pairwise key.

By passing the ieee80211_key_conf pointer it assures us that drivers can make full use of all information that it should know about a particular key.

Additionally this patch updates all drivers to grab the hw_key_idx from the ieee80211_key_conf structure.

Signed-off-by Ivo van Doorn <[email protected]>

---

drivers/net/wireless/ath5k/base.c +1 -1
index e18305b..cd20abe 100644
@@ -1323,7 +1323,7 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
 	pktlen = skb->len;
 
 	if (!(ctl->flags & IEEE80211_TXCTL_DO_NOT_ENCRYPT)) {
-		keyidx = ctl->key_idx;
+		keyidx = ctl->hw_key->hw_key_idx;
 		pktlen += ctl->icv_len;
 	}
drivers/net/wireless/b43/xmit.c +1 -1
index 19aefbf..2ee19cf 100644
@@ -235,7 +235,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16) (txctl->key_idx);
+		u8 key_idx = (u16) (txctl->hw_key->hw_key_idx);
 		struct b43_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/b43legacy/xmit.c +1 -1
index dcad249..ba50b68 100644
@@ -232,7 +232,7 @@ static int generate_txhdr_fw3(struct b43legacy_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16)(txctl->key_idx);
+		u8 key_idx = (u16)(txctl->hw_key->hw_key_idx);
 		struct b43legacy_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/iwlwifi/iwl3945-base.c +4 -3
index 1a5678f..b1250ea 100644
@@ -2391,7 +2391,8 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 				      struct sk_buff *skb_frag,
 				      int last_frag)
 {
-	struct iwl3945_hw_key *keyinfo = &priv->stations[ctl->key_idx].keyinfo;
+	struct iwl3945_hw_key *keyinfo =
+	    &priv->stations[ctl->hw_key->hw_key_idx].keyinfo;
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -2414,7 +2415,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 
 	case ALG_WEP:
 		cmd->cmd.tx.sec_ctl = TX_CMD_SEC_WEP |
-		    (ctl->key_idx & TX_CMD_SEC_MSK) << TX_CMD_SEC_SHIFT;
+		    (ctl->hw_key->hw_key_idx & TX_CMD_SEC_MSK) << TX_CMD_SEC_SHIFT;
 
 		if (keyinfo->keylen == 13)
 			cmd->cmd.tx.sec_ctl |= TX_CMD_SEC_KEY128;

@@ -2422,7 +2423,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 		memcpy(&cmd->cmd.tx.key[3], keyinfo->key, keyinfo->keylen);
 
 		IWL_DEBUG_TX("Configuring packet for WEP encryption "
-			     "with key %d\n", ctl->key_idx);
+			     "with key %d\n", ctl->hw_key->hw_key_idx);
 		break;
 
 	default:
drivers/net/wireless/iwlwifi/iwl4965-base.c +3 -3
index d7e2358..ae74042 100644
@@ -1936,7 +1936,7 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 	struct iwl_wep_key *wepkey;
 	int keyidx = 0;
 
-	BUG_ON(ctl->key_idx > 3);
+	BUG_ON(ctl->hw_key->hw_key_idx > 3);
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -1955,11 +1955,11 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 		break;
 
 	case ALG_WEP:
-		wepkey = &priv->wep_keys[ctl->key_idx];
+		wepkey = &priv->wep_keys[ctl->hw_key->hw_key_idx];
 		cmd->cmd.tx.sec_ctl = 0;
 		if (priv->default_wep_key) {
 			/* the WEP key was sent as static */
-			keyidx = ctl->key_idx;
+			keyidx = ctl->hw_key->hw_key_idx;
 			memcpy(&cmd->cmd.tx.key[3], wepkey->key,
 							wepkey->key_size);
 			if (wepkey->key_size == WEP_KEY_LEN_128)
include/net/mac80211.h +4 -2
index 4a80d74..af692f4 100644
@@ -298,9 +298,11 @@ struct ieee80211_tx_control {
 	/* retry rate for the last retries */
 	struct ieee80211_rate *alt_retry_rate;
 
+	/* Key used for hardware encryption
+	 * undefined if IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
+	struct ieee80211_key_conf *hw_key;
+
 	u32 flags;		/* tx control flags defined above */
-	u8 key_idx;		/* keyidx from hw->set_key(), undefined if
-				 * IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
 	u8 retry_limit;		/* 1 = only first attempt, 2 = one retry, ..
 				 * This could be used when set_retry_limit
 				 * is not implemented by the driver */
net/mac80211/wep.c +1 -1
index affcecd..3cbae42 100644
@@ -337,7 +337,7 @@ static int wep_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 		if (ieee80211_wep_encrypt(tx->local, skb, tx->key))
 			return -1;
 	} else {
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		if (tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) {
 			if (!ieee80211_wep_add_iv(tx->local, skb, tx->key))
 				return -1;
net/mac80211/wpa.c +4 -4
index 45709ad..42f3654 100644
@@ -228,7 +228,7 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx,
 					    0x7f),
 				      (u8) key->u.tkip.iv16);
 
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return 0;
 	}
 

@@ -256,7 +256,7 @@ ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx)
 	    !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
 	    !wpa_test) {
 		/* hwaccel - with no need for preallocated room for IV/ICV */
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return TX_CONTINUE;
 	}
 

@@ -478,7 +478,7 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx,
 
 	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 		/* hwaccel - with preallocated room for CCMP header */
-		tx->control->key_idx = key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return 0;
 	}
 

@@ -505,7 +505,7 @@ ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx)
 	    !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV)) {
 		/* hwaccel - with no need for preallocated room for CCMP "
 		 * header or MIC fields */
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return TX_CONTINUE;
 	}
#1 lore raw

drivers/net/wireless/b43/xmit.c int b43_generate_txhdr(struct b43_wldev *dev, · line 238

Quoted 10 lines · On Thursday 17 April 2008 17:28:29 Ivo van Doorn wrote:
drivers/net/wireless/b43/xmit.c +1 -1
index 19aefbf..2ee19cf 100644
@@ -235,7 +235,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16) (txctl->key_idx);
+		u8 key_idx = (u16) (txctl->hw_key->hw_key_idx);

Can we actually remove that bogus cast while we are at it? :)

drivers/net/wireless/b43legacy/xmit.c static int generate_txhdr_fw3(struct b43legacy_wldev *dev, · line 235

Quoted 13 lines
drivers/net/wireless/b43/xmit.c +0 -0
 		struct b43_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/b43legacy/xmit.c +1 -1
index dcad249..ba50b68 100644
@@ -232,7 +232,7 @@ static int generate_txhdr_fw3(struct b43legacy_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16)(txctl->key_idx);
+		u8 key_idx = (u16)(txctl->hw_key->hw_key_idx);

Same here.

Signature · 1 line
Greetings Michael.
author
#2 lore raw
Quoted 23 lines · Michael Buesch

drivers/net/wireless/b43/xmit.c int b43_generate_txhdr(struct b43_wldev *dev, · line 238

Quoted 10 lines · On Thursday 17 April 2008 17:28:29 Ivo van Doorn wrote:
drivers/net/wireless/b43/xmit.c +1 -1
index 19aefbf..2ee19cf 100644
@@ -235,7 +235,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16) (txctl->key_idx);
+		u8 key_idx = (u16) (txctl->hw_key->hw_key_idx);

Can we actually remove that bogus cast while we are at it? :)

Quoted 9 lines
drivers/net/wireless/b43/xmit.c +0 -0
 		struct b43_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/b43legacy/xmit.c +0 -0
index dcad249..ba50b68 100644
@@ -232,7 +232,7 @@ static int generate_txhdr_fw3(struct b43legacy_wldev *dev,
Quoted 6 lines

drivers/net/wireless/b43legacy/xmit.c static int generate_txhdr_fw3(struct b43legacy_wldev *dev, · line 235

Quoted 4 lines
drivers/net/wireless/b43legacy/xmit.c +1 -1
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16)(txctl->key_idx);
+		u8 key_idx = (u16)(txctl->hw_key->hw_key_idx);

Same here.

Sure, updated patch on its way. :)

Ivo

author
#3 lore raw

PATCH v2 1/1 mac80211: Replace ieee80211_tx_control->key_idx with ieee80211_key_conf

In reply to Ivo van Doorn

The hw_key_idx inside the ieee80211_key_conf structure does not provide all the information drivers might need to perform hardware encryption.

This is in particular true for rt2x00 who needs to know the key algorithm and whether it is a shared or pairwise key.

By passing the ieee80211_key_conf pointer it assures us that drivers can make full use of all information that it should know about a particular key.

Additionally this patch updates all drivers to grab the hw_key_idx from the ieee80211_key_conf structure.

v2: Removed bogus u16 cast

Signed-off-by Ivo van Doorn <[email protected]>

---

drivers/net/wireless/ath5k/base.c +1 -1
index e18305b..cd20abe 100644
@@ -1323,7 +1323,7 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
 	pktlen = skb->len;
 
 	if (!(ctl->flags & IEEE80211_TXCTL_DO_NOT_ENCRYPT)) {
-		keyidx = ctl->key_idx;
+		keyidx = ctl->hw_key->hw_key_idx;
 		pktlen += ctl->icv_len;
 	}
drivers/net/wireless/b43/xmit.c +1 -1
index 19aefbf..8849194 100644
@@ -235,7 +235,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16) (txctl->key_idx);
+		u8 key_idx = txctl->hw_key->hw_key_idx;
 		struct b43_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/b43legacy/xmit.c +1 -1
index dcad249..fc83dab 100644
@@ -232,7 +232,7 @@ static int generate_txhdr_fw3(struct b43legacy_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16)(txctl->key_idx);
+		u8 key_idx = txctl->hw_key->hw_key_idx;
 		struct b43legacy_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/iwlwifi/iwl3945-base.c +4 -3
index 1a5678f..b1250ea 100644
@@ -2391,7 +2391,8 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 				      struct sk_buff *skb_frag,
 				      int last_frag)
 {
-	struct iwl3945_hw_key *keyinfo = &priv->stations[ctl->key_idx].keyinfo;
+	struct iwl3945_hw_key *keyinfo =
+	    &priv->stations[ctl->hw_key->hw_key_idx].keyinfo;
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -2414,7 +2415,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 
 	case ALG_WEP:
 		cmd->cmd.tx.sec_ctl = TX_CMD_SEC_WEP |
-		    (ctl->key_idx & TX_CMD_SEC_MSK) << TX_CMD_SEC_SHIFT;
+		    (ctl->hw_key->hw_key_idx & TX_CMD_SEC_MSK) << TX_CMD_SEC_SHIFT;
 
 		if (keyinfo->keylen == 13)
 			cmd->cmd.tx.sec_ctl |= TX_CMD_SEC_KEY128;

@@ -2422,7 +2423,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 		memcpy(&cmd->cmd.tx.key[3], keyinfo->key, keyinfo->keylen);
 
 		IWL_DEBUG_TX("Configuring packet for WEP encryption "
-			     "with key %d\n", ctl->key_idx);
+			     "with key %d\n", ctl->hw_key->hw_key_idx);
 		break;
 
 	default:
drivers/net/wireless/iwlwifi/iwl4965-base.c +3 -3
index d7e2358..ae74042 100644
@@ -1936,7 +1936,7 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 	struct iwl_wep_key *wepkey;
 	int keyidx = 0;
 
-	BUG_ON(ctl->key_idx > 3);
+	BUG_ON(ctl->hw_key->hw_key_idx > 3);
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -1955,11 +1955,11 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 		break;
 
 	case ALG_WEP:
-		wepkey = &priv->wep_keys[ctl->key_idx];
+		wepkey = &priv->wep_keys[ctl->hw_key->hw_key_idx];
 		cmd->cmd.tx.sec_ctl = 0;
 		if (priv->default_wep_key) {
 			/* the WEP key was sent as static */
-			keyidx = ctl->key_idx;
+			keyidx = ctl->hw_key->hw_key_idx;
 			memcpy(&cmd->cmd.tx.key[3], wepkey->key,
 							wepkey->key_size);
 			if (wepkey->key_size == WEP_KEY_LEN_128)
include/net/mac80211.h +4 -2
index 4a80d74..af692f4 100644
@@ -298,9 +298,11 @@ struct ieee80211_tx_control {
 	/* retry rate for the last retries */
 	struct ieee80211_rate *alt_retry_rate;
 
+	/* Key used for hardware encryption
+	 * undefined if IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
+	struct ieee80211_key_conf *hw_key;
+
 	u32 flags;		/* tx control flags defined above */
-	u8 key_idx;		/* keyidx from hw->set_key(), undefined if
-				 * IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
 	u8 retry_limit;		/* 1 = only first attempt, 2 = one retry, ..
 				 * This could be used when set_retry_limit
 				 * is not implemented by the driver */
net/mac80211/wep.c +1 -1
index affcecd..3cbae42 100644
@@ -337,7 +337,7 @@ static int wep_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 		if (ieee80211_wep_encrypt(tx->local, skb, tx->key))
 			return -1;
 	} else {
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		if (tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) {
 			if (!ieee80211_wep_add_iv(tx->local, skb, tx->key))
 				return -1;
net/mac80211/wpa.c +4 -4
index 45709ad..42f3654 100644
@@ -228,7 +228,7 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx,
 					    0x7f),
 				      (u8) key->u.tkip.iv16);
 
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return 0;
 	}
 

@@ -256,7 +256,7 @@ ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx)
 	    !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
 	    !wpa_test) {
 		/* hwaccel - with no need for preallocated room for IV/ICV */
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return TX_CONTINUE;
 	}
 

@@ -478,7 +478,7 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx,
 
 	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 		/* hwaccel - with preallocated room for CCMP header */
-		tx->control->key_idx = key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return 0;
 	}
 

@@ -505,7 +505,7 @@ ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx)
 	    !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV)) {
 		/* hwaccel - with no need for preallocated room for CCMP "
 		 * header or MIC fields */
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return TX_CONTINUE;
 	}
maintainer
#4 lore raw

include/net/mac80211.h struct ieee80211_tx_control { · line 303

Quoted 4 lines
include/net/mac80211.h +3 -0
+	/* Key used for hardware encryption
+	 * undefined if IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
+	struct ieee80211_key_conf *hw_key;

I would prefer "NULL if <flag> is set". And we could possibly do some internal rework to get rid of the flag now (don't do that right now unless you think it's easy, I'm not too sure off-hand). Can you also add a comment about what I said wrt. accessing it inside ops->tx() only? Sorry about the mail crossing.

johannes

author
#5 lore raw

PATCH v3 1/1 mac80211: Replace ieee80211_tx_control->key_idx with ieee80211_key_conf

In reply to Ivo van Doorn

The hw_key_idx inside the ieee80211_key_conf structure does not provide all the information drivers might need to perform hardware encryption.

This is in particular true for rt2x00 who needs to know the key algorithm and whether it is a shared or pairwise key.

By passing the ieee80211_key_conf pointer it assures us that drivers can make full use of all information that it should know about a particular key.

Additionally this patch updates all drivers to grab the hw_key_idx from the ieee80211_key_conf structure.

v2: Removed bogus u16 cast
v3: Add warning about ieee80211_tx_control pointers

Signed-off-by Ivo van Doorn <[email protected]>

---

drivers/net/wireless/ath5k/base.c +1 -1
index e18305b..cd20abe 100644
@@ -1323,7 +1323,7 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
 	pktlen = skb->len;
 
 	if (!(ctl->flags & IEEE80211_TXCTL_DO_NOT_ENCRYPT)) {
-		keyidx = ctl->key_idx;
+		keyidx = ctl->hw_key->hw_key_idx;
 		pktlen += ctl->icv_len;
 	}
drivers/net/wireless/b43/xmit.c +1 -1
index 19aefbf..8849194 100644
@@ -235,7 +235,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16) (txctl->key_idx);
+		u8 key_idx = txctl->hw_key->hw_key_idx;
 		struct b43_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/b43legacy/xmit.c +1 -1
index dcad249..fc83dab 100644
@@ -232,7 +232,7 @@ static int generate_txhdr_fw3(struct b43legacy_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16)(txctl->key_idx);
+		u8 key_idx = txctl->hw_key->hw_key_idx;
 		struct b43legacy_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/iwlwifi/iwl3945-base.c +4 -3
index 1a5678f..b1250ea 100644
@@ -2391,7 +2391,8 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 				      struct sk_buff *skb_frag,
 				      int last_frag)
 {
-	struct iwl3945_hw_key *keyinfo = &priv->stations[ctl->key_idx].keyinfo;
+	struct iwl3945_hw_key *keyinfo =
+	    &priv->stations[ctl->hw_key->hw_key_idx].keyinfo;
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -2414,7 +2415,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 
 	case ALG_WEP:
 		cmd->cmd.tx.sec_ctl = TX_CMD_SEC_WEP |
-		    (ctl->key_idx & TX_CMD_SEC_MSK) << TX_CMD_SEC_SHIFT;
+		    (ctl->hw_key->hw_key_idx & TX_CMD_SEC_MSK) << TX_CMD_SEC_SHIFT;
 
 		if (keyinfo->keylen == 13)
 			cmd->cmd.tx.sec_ctl |= TX_CMD_SEC_KEY128;

@@ -2422,7 +2423,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 		memcpy(&cmd->cmd.tx.key[3], keyinfo->key, keyinfo->keylen);
 
 		IWL_DEBUG_TX("Configuring packet for WEP encryption "
-			     "with key %d\n", ctl->key_idx);
+			     "with key %d\n", ctl->hw_key->hw_key_idx);
 		break;
 
 	default:
drivers/net/wireless/iwlwifi/iwl4965-base.c +3 -3
index d7e2358..ae74042 100644
@@ -1936,7 +1936,7 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 	struct iwl_wep_key *wepkey;
 	int keyidx = 0;
 
-	BUG_ON(ctl->key_idx > 3);
+	BUG_ON(ctl->hw_key->hw_key_idx > 3);
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -1955,11 +1955,11 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 		break;
 
 	case ALG_WEP:
-		wepkey = &priv->wep_keys[ctl->key_idx];
+		wepkey = &priv->wep_keys[ctl->hw_key->hw_key_idx];
 		cmd->cmd.tx.sec_ctl = 0;
 		if (priv->default_wep_key) {
 			/* the WEP key was sent as static */
-			keyidx = ctl->key_idx;
+			keyidx = ctl->hw_key->hw_key_idx;
 			memcpy(&cmd->cmd.tx.key[3], wepkey->key,
 							wepkey->key_size);
 			if (wepkey->key_size == WEP_KEY_LEN_128)
include/net/mac80211.h +11 -4
index 4a80d74..28a8893 100644
@@ -286,8 +286,13 @@ enum mac80211_tx_control_flags {
 
 /* Transmit control fields. This data structure is passed to low-level driver
  * with each TX frame. The low-level driver is responsible for configuring
- * the hardware to use given values (depending on what is supported). */
-
+ * the hardware to use given values (depending on what is supported).
+ *
+ * NOTE: All pointers within this structure are only valid until
+ * ieee80211_ops->tx() returns. If the information is required
+ * at a later time (i.e. when defering the work to a workqueue)
+ * make sure that not only the contents of struct ieee80211_tx_control
+ * is copied, but also the data where the pointers refer to. */
 struct ieee80211_tx_control {
 	struct ieee80211_vif *vif;
 	struct ieee80211_rate *tx_rate;

@@ -298,9 +303,11 @@ struct ieee80211_tx_control {
 	/* retry rate for the last retries */
 	struct ieee80211_rate *alt_retry_rate;
 
+	/* Key used for hardware encryption
+	 * NULL if IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
+	struct ieee80211_key_conf *hw_key;
+
 	u32 flags;		/* tx control flags defined above */
-	u8 key_idx;		/* keyidx from hw->set_key(), undefined if
-				 * IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
 	u8 retry_limit;		/* 1 = only first attempt, 2 = one retry, ..
 				 * This could be used when set_retry_limit
 				 * is not implemented by the driver */
net/mac80211/wep.c +1 -1
index affcecd..3cbae42 100644
@@ -337,7 +337,7 @@ static int wep_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 		if (ieee80211_wep_encrypt(tx->local, skb, tx->key))
 			return -1;
 	} else {
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		if (tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) {
 			if (!ieee80211_wep_add_iv(tx->local, skb, tx->key))
 				return -1;
net/mac80211/wpa.c +4 -4
index 45709ad..42f3654 100644
@@ -228,7 +228,7 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx,
 					    0x7f),
 				      (u8) key->u.tkip.iv16);
 
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return 0;
 	}
 

@@ -256,7 +256,7 @@ ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx)
 	    !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
 	    !wpa_test) {
 		/* hwaccel - with no need for preallocated room for IV/ICV */
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return TX_CONTINUE;
 	}
 

@@ -478,7 +478,7 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx,
 
 	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 		/* hwaccel - with preallocated room for CCMP header */
-		tx->control->key_idx = key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return 0;
 	}
 

@@ -505,7 +505,7 @@ ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx)
 	    !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV)) {
 		/* hwaccel - with no need for preallocated room for CCMP "
 		 * header or MIC fields */
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return TX_CONTINUE;
 	}
maintainer Acked-by on v3 1/1
#6 lore raw

Hi,

I hate to do this, but

Quoted 11 lines

/* Transmit control fields. This data structure is passed to low-level driver
* with each TX frame. The low-level driver is responsible for configuring
- * the hardware to use given values (depending on what is supported). */ -
+ * the hardware to use given values (depending on what is supported).
+ *
+ * NOTE: All pointers within this structure are only valid until
+ * ieee80211_ops->tx() returns. If the information is required
+ * at a later time (i.e. when defering the work to a workqueue)
+ * make sure that not only the contents of struct ieee80211_tx_control
+ * is copied, but also the data where the pointers refer to. */

This isn't true for all pointers, the rate pointers point into the array rate array so they're valid until the phy is unregistered, the vif pointer is valid until the interface is removed via the appropriate callback (remove_interface? I forgot), and the key until that is removed via set_key().

Maybe we should add that instead? Normally, people will not need the pointers outside ->tx() though, of course.

Also, please move the */ to a new line while you're at it anyway.

Other than that,

Acked-by Johannes Berg <[email protected]>

johannes

author
#7 lore raw

PATCH v4 1/1 mac80211: Replace ieee80211_tx_control->key_idx with ieee80211_key_conf

In reply to Ivo van Doorn

The hw_key_idx inside the ieee80211_key_conf structure does not provide all the information drivers might need to perform hardware encryption.

This is in particular true for rt2x00 who needs to know the key algorithm and whether it is a shared or pairwise key.

By passing the ieee80211_key_conf pointer it assures us that drivers can make full use of all information that it should know about a particular key.

Additionally this patch updates all drivers to grab the hw_key_idx from the ieee80211_key_conf structure.

v2: Removed bogus u16 cast
v3: Add warning about ieee80211_tx_control pointers
v4: Update warning about ieee80211_tx_control pointers

Signed-off-by Ivo van Doorn <[email protected]>
Acked-by Johannes Berg <[email protected]>

---

drivers/net/wireless/ath5k/base.c +1 -1
index e18305b..cd20abe 100644
@@ -1323,7 +1323,7 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
 	pktlen = skb->len;
 
 	if (!(ctl->flags & IEEE80211_TXCTL_DO_NOT_ENCRYPT)) {
-		keyidx = ctl->key_idx;
+		keyidx = ctl->hw_key->hw_key_idx;
 		pktlen += ctl->icv_len;
 	}
drivers/net/wireless/b43/xmit.c +1 -1
index 19aefbf..8849194 100644
@@ -235,7 +235,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16) (txctl->key_idx);
+		u8 key_idx = txctl->hw_key->hw_key_idx;
 		struct b43_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/b43legacy/xmit.c +1 -1
index dcad249..fc83dab 100644
@@ -232,7 +232,7 @@ static int generate_txhdr_fw3(struct b43legacy_wldev *dev,
 
 	plcp_fragment_len = fragment_len + FCS_LEN;
 	if (use_encryption) {
-		u8 key_idx = (u16)(txctl->key_idx);
+		u8 key_idx = txctl->hw_key->hw_key_idx;
 		struct b43legacy_key *key;
 		int wlhdr_len;
 		size_t iv_len;
drivers/net/wireless/iwlwifi/iwl3945-base.c +4 -3
index 1a5678f..b1250ea 100644
@@ -2391,7 +2391,8 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 				      struct sk_buff *skb_frag,
 				      int last_frag)
 {
-	struct iwl3945_hw_key *keyinfo = &priv->stations[ctl->key_idx].keyinfo;
+	struct iwl3945_hw_key *keyinfo =
+	    &priv->stations[ctl->hw_key->hw_key_idx].keyinfo;
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -2414,7 +2415,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 
 	case ALG_WEP:
 		cmd->cmd.tx.sec_ctl = TX_CMD_SEC_WEP |
-		    (ctl->key_idx & TX_CMD_SEC_MSK) << TX_CMD_SEC_SHIFT;
+		    (ctl->hw_key->hw_key_idx & TX_CMD_SEC_MSK) << TX_CMD_SEC_SHIFT;
 
 		if (keyinfo->keylen == 13)
 			cmd->cmd.tx.sec_ctl |= TX_CMD_SEC_KEY128;

@@ -2422,7 +2423,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl3945_priv *priv,
 		memcpy(&cmd->cmd.tx.key[3], keyinfo->key, keyinfo->keylen);
 
 		IWL_DEBUG_TX("Configuring packet for WEP encryption "
-			     "with key %d\n", ctl->key_idx);
+			     "with key %d\n", ctl->hw_key->hw_key_idx);
 		break;
 
 	default:
drivers/net/wireless/iwlwifi/iwl4965-base.c +3 -3
index d7e2358..ae74042 100644
@@ -1936,7 +1936,7 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 	struct iwl_wep_key *wepkey;
 	int keyidx = 0;
 
-	BUG_ON(ctl->key_idx > 3);
+	BUG_ON(ctl->hw_key->hw_key_idx > 3);
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -1955,11 +1955,11 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 		break;
 
 	case ALG_WEP:
-		wepkey = &priv->wep_keys[ctl->key_idx];
+		wepkey = &priv->wep_keys[ctl->hw_key->hw_key_idx];
 		cmd->cmd.tx.sec_ctl = 0;
 		if (priv->default_wep_key) {
 			/* the WEP key was sent as static */
-			keyidx = ctl->key_idx;
+			keyidx = ctl->hw_key->hw_key_idx;
 			memcpy(&cmd->cmd.tx.key[3], wepkey->key,
 							wepkey->key_size);
 			if (wepkey->key_size == WEP_KEY_LEN_128)
include/net/mac80211.h +15 -4
index 4a80d74..27ef9f7 100644
@@ -286,8 +286,17 @@ enum mac80211_tx_control_flags {
 
 /* Transmit control fields. This data structure is passed to low-level driver
  * with each TX frame. The low-level driver is responsible for configuring
- * the hardware to use given values (depending on what is supported). */
-
+ * the hardware to use given values (depending on what is supported).
+ *
+ * NOTE: Be careful with using the pointers outside of the ieee80211_ops->tx()
+ * context (i.e. when defering the work to a workqueue).
+ * The vif pointer is valid until the it has been removed with the
+ * ieee80211_ops->remove_interface() callback funtion.
+ * The hw_key pointer is valid until it has been removed with the
+ * ieee80211_ops->set_key() callback function.
+ * The tx_rate and alt_retry_rate pointers are valid until the phy is
+ * deregistered.
+ */
 struct ieee80211_tx_control {
 	struct ieee80211_vif *vif;
 	struct ieee80211_rate *tx_rate;

@@ -298,9 +307,11 @@ struct ieee80211_tx_control {
 	/* retry rate for the last retries */
 	struct ieee80211_rate *alt_retry_rate;
 
+	/* Key used for hardware encryption
+	 * NULL if IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
+	struct ieee80211_key_conf *hw_key;
+
 	u32 flags;		/* tx control flags defined above */
-	u8 key_idx;		/* keyidx from hw->set_key(), undefined if
-				 * IEEE80211_TXCTL_DO_NOT_ENCRYPT is set */
 	u8 retry_limit;		/* 1 = only first attempt, 2 = one retry, ..
 				 * This could be used when set_retry_limit
 				 * is not implemented by the driver */
net/mac80211/wep.c +1 -1
index affcecd..3cbae42 100644
@@ -337,7 +337,7 @@ static int wep_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 		if (ieee80211_wep_encrypt(tx->local, skb, tx->key))
 			return -1;
 	} else {
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		if (tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) {
 			if (!ieee80211_wep_add_iv(tx->local, skb, tx->key))
 				return -1;
net/mac80211/wpa.c +4 -4
index 45709ad..42f3654 100644
@@ -228,7 +228,7 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx,
 					    0x7f),
 				      (u8) key->u.tkip.iv16);
 
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return 0;
 	}
 

@@ -256,7 +256,7 @@ ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx)
 	    !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
 	    !wpa_test) {
 		/* hwaccel - with no need for preallocated room for IV/ICV */
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return TX_CONTINUE;
 	}
 

@@ -478,7 +478,7 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx,
 
 	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 		/* hwaccel - with preallocated room for CCMP header */
-		tx->control->key_idx = key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return 0;
 	}
 

@@ -505,7 +505,7 @@ ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx)
 	    !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV)) {
 		/* hwaccel - with no need for preallocated room for CCMP "
 		 * header or MIC fields */
-		tx->control->key_idx = tx->key->conf.hw_key_idx;
+		tx->control->hw_key = &tx->key->conf;
 		return TX_CONTINUE;
 	}
maintainer
#8 lore raw
Quoted 16 lines
drivers/net/wireless/iwlwifi/iwl4965-base.c +1 -1
@@ -1936,7 +1936,7 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 	struct iwl_wep_key *wepkey;
 	int keyidx = 0;
 
-	BUG_ON(ctl->key_idx > 3);
+	BUG_ON(ctl->hw_key->hw_key_idx > 3);
 
 	switch (keyinfo->alg) {
 	case ALG_CCMP:

@@ -1955,11 +1955,11 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
 		break;
 
 	case ALG_WEP:

drivers/net/wireless/iwlwifi/iwl4965-base.c static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv, · line 1965

Quoted 10 lines
drivers/net/wireless/iwlwifi/iwl4965-base.c +2 -2
-		wepkey = &priv->wep_keys[ctl->key_idx];
+		wepkey = &priv->wep_keys[ctl->hw_key->hw_key_idx];
 		cmd->cmd.tx.sec_ctl = 0;
 		if (priv->default_wep_key) {
 			/* the WEP key was sent as static */
-			keyidx = ctl->key_idx;
+			keyidx = ctl->hw_key->hw_key_idx;
 			memcpy(&cmd->cmd.tx.key[3], wepkey->key,
 							wepkey->key_size);
 			if (wepkey->key_size == WEP_KEY_LEN_128)

All this is rather odd. I think you (Intel) may want to change it to use hw_key->keyidx and just leave the hw_key_idx alone, it seems to be a copy of keyidx in iwl4965 but an actual index as it was meant to be used in iwl3945?

johannes

#9 lore raw
Quoted 29 lines · Johannes Berg
Quoted 16 lines
drivers/net/wireless/iwlwifi/iwl4965-base.c +1 -1
@@ -1936,7 +1936,7 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
      struct iwl_wep_key *wepkey;
      int keyidx = 0;
 
-     BUG_ON(ctl->key_idx > 3);
+     BUG_ON(ctl->hw_key->hw_key_idx > 3);
 
      switch (keyinfo->alg) {
      case ALG_CCMP:

@@ -1955,11 +1955,11 @@ static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
              break;
 
      case ALG_WEP:

drivers/net/wireless/iwlwifi/iwl4965-base.c static void iwl4965_build_tx_cmd_hwcrypto(struct iwl_priv *priv, · line 1965

Quoted 10 lines
drivers/net/wireless/iwlwifi/iwl4965-base.c +2 -2
-             wepkey = &priv->wep_keys[ctl->key_idx];
+             wepkey = &priv->wep_keys[ctl->hw_key->hw_key_idx];
              cmd->cmd.tx.sec_ctl = 0;
              if (priv->default_wep_key) {
                      /* the WEP key was sent as static */
-                     keyidx = ctl->key_idx;
+                     keyidx = ctl->hw_key->hw_key_idx;
                      memcpy(&cmd->cmd.tx.key[3], wepkey->key,
                                                      wepkey->key_size);
                      if (wepkey->key_size == WEP_KEY_LEN_128)

All this is rather odd. I think you (Intel) may want to change it to use hw_key->keyidx and just leave the hw_key_idx alone, it seems to be a

Quoted 2 lines
 copy of keyidx in iwl4965 but an actual index as it was meant to be used

in iwl3945?

Sure, this patch changes the picture., in addition the code can be a bit simplified.
Will fix that.
Thanks
Tomas

Quoted 1 line