Re: [PATCH 2/3] iwlwifi: mvm: move TX PN assignment for TKIP to the driver

[Johannes Berg]

On Mon, 2016-02-15 at 09:24 +, Grumbach, Emmanuel wrote:
> 
> It doesn't feel right to remove this function in mac80211-next.git
> and
> touch 3 drivers along the way, but I don't mind. Johannes, what do
> you say?

I don't see any problem with that? I've touched far more drivers (like
"every single one") when doing API updates :)

johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iwlwifi: mvm: move TX PN assignment for TKIP to the driver

[Grumbach, Emmanuel]

On 02/15/2016 11:21 AM, Eliad Peller wrote:
> On Mon, Feb 15, 2016 at 11:16 AM, Grumbach, Emmanuel
>  wrote:
>>
>> On 02/15/2016 11:06 AM, Eliad Peller wrote:
>>> On Sun, Feb 14, 2016 at 9:37 PM, Grumbach, Emmanuel
>>>  wrote:
 On 02/14/2016 09:33 PM, Johannes Berg wrote:
> On Sun, 2016-02-14 at 19:34 +0200, Emmanuel Grumbach wrote:
>> Since the 3rd patch needs to be dropped anyway, let's route this one
>> through my tree as usual.
> It doesn't really have to be dropped, why? We can just make the same
> adjustment as for dvm in the patch.
>
 But I am not sure I really want to play with drivers/staging/vt6656/rxtx.c
>>> here you go:
>>>
>>> diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
>>> index b668db6..1a2dda0 100644
>>> --- a/drivers/staging/vt6655/rxtx.c
>>> +++ b/drivers/staging/vt6655/rxtx.c
>>>
>> Want to send that patch to Greg? :)
> why? can't you simply amend it to the third patch?
>
>

It doesn't feel right to remove this function in mac80211-next.git and
touch 3 drivers along the way, but I don't mind. Johannes, what do you say?
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iwlwifi: mvm: move TX PN assignment for TKIP to the driver

[Eliad Peller]

On Mon, Feb 15, 2016 at 11:16 AM, Grumbach, Emmanuel
 wrote:
>
>
> On 02/15/2016 11:06 AM, Eliad Peller wrote:
>> On Sun, Feb 14, 2016 at 9:37 PM, Grumbach, Emmanuel
>>  wrote:
>>>
>>> On 02/14/2016 09:33 PM, Johannes Berg wrote:
 On Sun, 2016-02-14 at 19:34 +0200, Emmanuel Grumbach wrote:
> Since the 3rd patch needs to be dropped anyway, let's route this one
> through my tree as usual.
 It doesn't really have to be dropped, why? We can just make the same
 adjustment as for dvm in the patch.

>>> But I am not sure I really want to play with drivers/staging/vt6656/rxtx.c
>> here you go:
>>
>> diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
>> index b668db6..1a2dda0 100644
>> --- a/drivers/staging/vt6655/rxtx.c
>> +++ b/drivers/staging/vt6655/rxtx.c
>>
> Want to send that patch to Greg? :)

why? can't you simply amend it to the third patch?

Eliad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iwlwifi: mvm: move TX PN assignment for TKIP to the driver

[Grumbach, Emmanuel]

On 02/15/2016 11:06 AM, Eliad Peller wrote:
> On Sun, Feb 14, 2016 at 9:37 PM, Grumbach, Emmanuel
>  wrote:
>>
>> On 02/14/2016 09:33 PM, Johannes Berg wrote:
>>> On Sun, 2016-02-14 at 19:34 +0200, Emmanuel Grumbach wrote:
 Since the 3rd patch needs to be dropped anyway, let's route this one
 through my tree as usual.
>>> It doesn't really have to be dropped, why? We can just make the same
>>> adjustment as for dvm in the patch.
>>>
>> But I am not sure I really want to play with drivers/staging/vt6656/rxtx.c
> here you go:
>
> diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> index b668db6..1a2dda0 100644
> --- a/drivers/staging/vt6655/rxtx.c
> +++ b/drivers/staging/vt6655/rxtx.c
> @@ -1210,7 +1210,7 @@ static void vnt_fill_txkey(struct ieee80211_hdr
> *hdr, u8 *key_buffer,
> struct sk_buff *skb, u16 payload_len,
> struct vnt_mic_hdr *mic_hdr)
>  {
> - struct ieee80211_key_seq seq;
> + u64 pn64;
>   u8 *iv = ((u8 *)hdr + ieee80211_get_hdrlen_from_skb(skb));
>
>   /* strip header and icv len from payload */
> @@ -1243,9 +1243,13 @@ static void vnt_fill_txkey(struct ieee80211_hdr
> *hdr, u8 *key_buffer,
>   mic_hdr->payload_len = cpu_to_be16(payload_len);
>   ether_addr_copy(mic_hdr->mic_addr2, hdr->addr2);
>
> - ieee80211_get_key_tx_seq(tx_key, );
> -
> - memcpy(mic_hdr->ccmp_pn, seq.ccmp.pn, IEEE80211_CCMP_PN_LEN);
> + pn64 = atomic64_read(_key->tx_pn);
> + mic_hdr->ccmp_pn[5] = pn64;
> + mic_hdr->ccmp_pn[4] = pn64 >> 8;
> + mic_hdr->ccmp_pn[3] = pn64 >> 16;
> + mic_hdr->ccmp_pn[2] = pn64 >> 24;
> + mic_hdr->ccmp_pn[1] = pn64 >> 32;
> + mic_hdr->ccmp_pn[0] = pn64 >> 40;
>
>   if (ieee80211_has_a4(hdr->frame_control))
>   mic_hdr->hlen = cpu_to_be16(28);
> diff --git a/drivers/staging/vt6656/rxtx.c b/drivers/staging/vt6656/rxtx.c
> index efb54f5..76378d2 100644
> --- a/drivers/staging/vt6656/rxtx.c
> +++ b/drivers/staging/vt6656/rxtx.c
> @@ -719,7 +719,7 @@ static void vnt_fill_txkey(struct
> vnt_usb_send_context *tx_context,
>   u16 payload_len, struct vnt_mic_hdr *mic_hdr)
>  {
>   struct ieee80211_hdr *hdr = tx_context->hdr;
> - struct ieee80211_key_seq seq;
> + u64 pn64;
>   u8 *iv = ((u8 *)hdr + ieee80211_get_hdrlen_from_skb(skb));
>
>   /* strip header and icv len from payload */
> @@ -752,9 +752,13 @@ static void vnt_fill_txkey(struct
> vnt_usb_send_context *tx_context,
>   mic_hdr->payload_len = cpu_to_be16(payload_len);
>   ether_addr_copy(mic_hdr->mic_addr2, hdr->addr2);
>
> - ieee80211_get_key_tx_seq(tx_key, );
> -
> - memcpy(mic_hdr->ccmp_pn, seq.ccmp.pn, IEEE80211_CCMP_PN_LEN);
> + pn64 = atomic64_read(_key->tx_pn);
> + mic_hdr->ccmp_pn[5] = pn64;
> + mic_hdr->ccmp_pn[4] = pn64 >> 8;
> + mic_hdr->ccmp_pn[3] = pn64 >> 16;
> + mic_hdr->ccmp_pn[2] = pn64 >> 24;
> + mic_hdr->ccmp_pn[1] = pn64 >> 32;
> + mic_hdr->ccmp_pn[0] = pn64 >> 40;
>
>   if (ieee80211_has_a4(hdr->frame_control))
>   mic_hdr->hlen = cpu_to_be16(28);
>
>
> Eliad.
>

Want to send that patch to Greg? :)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iwlwifi: mvm: move TX PN assignment for TKIP to the driver

[Eliad Peller]

On Sun, Feb 14, 2016 at 9:37 PM, Grumbach, Emmanuel
 wrote:
>
>
> On 02/14/2016 09:33 PM, Johannes Berg wrote:
>> On Sun, 2016-02-14 at 19:34 +0200, Emmanuel Grumbach wrote:
>>>
>>> Since the 3rd patch needs to be dropped anyway, let's route this one
>>> through my tree as usual.
>> It doesn't really have to be dropped, why? We can just make the same
>> adjustment as for dvm in the patch.
>>
>
> But I am not sure I really want to play with drivers/staging/vt6656/rxtx.c

here you go:

diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index b668db6..1a2dda0 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -1210,7 +1210,7 @@ static void vnt_fill_txkey(struct ieee80211_hdr
*hdr, u8 *key_buffer,
struct sk_buff *skb, u16 payload_len,
struct vnt_mic_hdr *mic_hdr)
 {
- struct ieee80211_key_seq seq;
+ u64 pn64;
  u8 *iv = ((u8 *)hdr + ieee80211_get_hdrlen_from_skb(skb));

  /* strip header and icv len from payload */
@@ -1243,9 +1243,13 @@ static void vnt_fill_txkey(struct ieee80211_hdr
*hdr, u8 *key_buffer,
  mic_hdr->payload_len = cpu_to_be16(payload_len);
  ether_addr_copy(mic_hdr->mic_addr2, hdr->addr2);

- ieee80211_get_key_tx_seq(tx_key, );
-
- memcpy(mic_hdr->ccmp_pn, seq.ccmp.pn, IEEE80211_CCMP_PN_LEN);
+ pn64 = atomic64_read(_key->tx_pn);
+ mic_hdr->ccmp_pn[5] = pn64;
+ mic_hdr->ccmp_pn[4] = pn64 >> 8;
+ mic_hdr->ccmp_pn[3] = pn64 >> 16;
+ mic_hdr->ccmp_pn[2] = pn64 >> 24;
+ mic_hdr->ccmp_pn[1] = pn64 >> 32;
+ mic_hdr->ccmp_pn[0] = pn64 >> 40;

  if (ieee80211_has_a4(hdr->frame_control))
  mic_hdr->hlen = cpu_to_be16(28);
diff --git a/drivers/staging/vt6656/rxtx.c b/drivers/staging/vt6656/rxtx.c
index efb54f5..76378d2 100644
--- a/drivers/staging/vt6656/rxtx.c
+++ b/drivers/staging/vt6656/rxtx.c
@@ -719,7 +719,7 @@ static void vnt_fill_txkey(struct
vnt_usb_send_context *tx_context,
  u16 payload_len, struct vnt_mic_hdr *mic_hdr)
 {
  struct ieee80211_hdr *hdr = tx_context->hdr;
- struct ieee80211_key_seq seq;
+ u64 pn64;
  u8 *iv = ((u8 *)hdr + ieee80211_get_hdrlen_from_skb(skb));

  /* strip header and icv len from payload */
@@ -752,9 +752,13 @@ static void vnt_fill_txkey(struct
vnt_usb_send_context *tx_context,
  mic_hdr->payload_len = cpu_to_be16(payload_len);
  ether_addr_copy(mic_hdr->mic_addr2, hdr->addr2);

- ieee80211_get_key_tx_seq(tx_key, );
-
- memcpy(mic_hdr->ccmp_pn, seq.ccmp.pn, IEEE80211_CCMP_PN_LEN);
+ pn64 = atomic64_read(_key->tx_pn);
+ mic_hdr->ccmp_pn[5] = pn64;
+ mic_hdr->ccmp_pn[4] = pn64 >> 8;
+ mic_hdr->ccmp_pn[3] = pn64 >> 16;
+ mic_hdr->ccmp_pn[2] = pn64 >> 24;
+ mic_hdr->ccmp_pn[1] = pn64 >> 32;
+ mic_hdr->ccmp_pn[0] = pn64 >> 40;

  if (ieee80211_has_a4(hdr->frame_control))
  mic_hdr->hlen = cpu_to_be16(28);


Eliad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iwlwifi: mvm: move TX PN assignment for TKIP to the driver

[Grumbach, Emmanuel]

On 02/14/2016 09:33 PM, Johannes Berg wrote:
> On Sun, 2016-02-14 at 19:34 +0200, Emmanuel Grumbach wrote:
>>  
>> Since the 3rd patch needs to be dropped anyway, let's route this one
>> through my tree as usual.
> It doesn't really have to be dropped, why? We can just make the same
> adjustment as for dvm in the patch.
>

But I am not sure I really want to play with drivers/staging/vt6656/rxtx.c
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iwlwifi: mvm: move TX PN assignment for TKIP to the driver

[Emmanuel Grumbach]

On Sun, Feb 14, 2016 at 1:56 PM, Emmanuel Grumbach
 wrote:
>
> From: Eliad Peller 
>
> If protocol offloading is configured, the fw might generate some
> frames (e.g. arp response) on its own during d3/d0i3.
>
> On d3/d0i3 exit the driver queries the updated PN (if relevant),
> and updates its keys (for the d0i3 case, this is done by
> iwl_mvm_d0i3_exit_work(), which is scheduled on d0i3 exit)
>
> While in d0i3, iwlmvm defers tx frames until d0i3 exit, and
> then continues their processing.
>
> This is problematic with TKIP, since the frame's PN has already
> been set at this stage (in contrast to CCMP, where the PN is
> being set only later on), so both the frame's PN and the upcoming
> PN update (from d0i3 exit work) might be wrong.
>
> Fix it by moving the TX PN assignment (for TKIP) to the driver,
> similarly to CCMP.
>
> Signed-off-by: Eliad Peller 
> Signed-off-by: Emmanuel Grumbach 
> ---
> Johannes, please route this one through your tree - thanks
> ---

Since the 3rd patch needs to be dropped anyway, let's route this one
through my tree as usual.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] iwlwifi: mvm: move TX PN assignment for TKIP to the driver

[Emmanuel Grumbach]

From: Eliad Peller 

If protocol offloading is configured, the fw might generate some
frames (e.g. arp response) on its own during d3/d0i3.

On d3/d0i3 exit the driver queries the updated PN (if relevant),
and updates its keys (for the d0i3 case, this is done by
iwl_mvm_d0i3_exit_work(), which is scheduled on d0i3 exit)

While in d0i3, iwlmvm defers tx frames until d0i3 exit, and
then continues their processing.

This is problematic with TKIP, since the frame's PN has already
been set at this stage (in contrast to CCMP, where the PN is
being set only later on), so both the frame's PN and the upcoming
PN update (from d0i3 exit work) might be wrong.

Fix it by moving the TX PN assignment (for TKIP) to the driver,
similarly to CCMP.

Signed-off-by: Eliad Peller 
Signed-off-by: Emmanuel Grumbach 
---
Johannes, please route this one through your tree - thanks
---
 drivers/net/wireless/intel/iwlwifi/mvm/d3.c   | 15 ++-
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/tx.c   |  2 ++
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
index d3e21d9..93e4958 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
@@ -249,16 +249,19 @@ static void iwl_mvm_wowlan_program_keys(struct 
ieee80211_hw *hw,
return;
case WLAN_CIPHER_SUITE_TKIP:
if (sta) {
+   u64 pn64;
+
tkip_sc = data->rsc_tsc->all_tsc_rsc.tkip.unicast_rsc;
tkip_tx_sc = >rsc_tsc->all_tsc_rsc.tkip.tsc;
 
rx_p1ks = data->tkip->rx_uni;
 
-   ieee80211_get_key_tx_seq(key, );
-   tkip_tx_sc->iv16 = cpu_to_le16(seq.tkip.iv16);
-   tkip_tx_sc->iv32 = cpu_to_le32(seq.tkip.iv32);
+   pn64 = atomic64_read(>tx_pn);
+   tkip_tx_sc->iv16 = cpu_to_le16(TKIP_PN_TO_IV16(pn64));
+   tkip_tx_sc->iv32 = cpu_to_le32(TKIP_PN_TO_IV32(pn64));
 
-   ieee80211_get_tkip_p1k_iv(key, seq.tkip.iv32, p1k);
+   ieee80211_get_tkip_p1k_iv(key, TKIP_PN_TO_IV32(pn64),
+ p1k);
iwl_mvm_convert_p1k(p1k, data->tkip->tx.p1k);
 
memcpy(data->tkip->mic_keys.tx,
@@ -1601,7 +1604,9 @@ static void iwl_mvm_d3_update_keys(struct ieee80211_hw 
*hw,
case WLAN_CIPHER_SUITE_TKIP:
iwl_mvm_tkip_sc_to_seq(>tkip.tsc, );
iwl_mvm_set_tkip_rx_seq(sc->tkip.unicast_rsc, key);
-   ieee80211_set_key_tx_seq(key, );
+   atomic64_set(>tx_pn,
+(u64)seq.tkip.iv16 |
+((u64)seq.tkip.iv32 << 16));
break;
}
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 1bd3f0b..2b53292 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -2585,7 +2585,7 @@ static int iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
switch (key->cipher) {
case WLAN_CIPHER_SUITE_TKIP:
key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
-   key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
+   key->flags |= IEEE80211_KEY_FLAG_PUT_IV_SPACE;
break;
case WLAN_CIPHER_SUITE_CCMP:
key->flags |= IEEE80211_KEY_FLAG_PUT_IV_SPACE;
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index 8bf48a7..ca1e485 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -299,6 +299,8 @@ static void iwl_mvm_set_tx_cmd_crypto(struct iwl_mvm *mvm,
 
case WLAN_CIPHER_SUITE_TKIP:
tx_cmd->sec_ctl = TX_CMD_SEC_TKIP;
+   pn = atomic64_inc_return(>tx_pn);
+   ieee80211_tkip_add_iv(crypto_hdr, keyconf, pn);
ieee80211_get_tkip_p2k(keyconf, skb_frag, tx_cmd->key);
break;
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html