[PATCH 1/2] mac80211: Extra debug info for station cleanup case.

[PATCH 1/2] mac80211: Extra debug info for station cleanup case.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 net/mac80211/sta_info.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index decd30c..83238b8 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -877,7 +877,9 @@ static void __sta_info_destroy_part2(struct sta_info *sta)
 	if (sta->uploaded) {
 		ret = drv_sta_state(local, sdata, sta, IEEE80211_STA_NONE,
 				    IEEE80211_STA_NOTEXIST);
-		WARN_ON_ONCE(ret != 0);
+		if (WARN_ON_ONCE(ret != 0))
+			sdata_info(sdata, "sta-info-destroy: drv-sta-state error: %i, sta: %pM\n",
+				   ret, sta->sta.addr);
 	}
 
 	sta_dbg(sdata, "Removed STA %pM\n", sta->sta.addr);
-- 
1.7.11.7

[PATCH 2/2] mac80211: Support tx-hw-crypt with rx-sw-crypt.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Some NICs can be made to offload the tx encryption and
run the rx encryption in software.  This can be used to
allow connecting several stations to the same AP, since most
hardware appears to hash only on the remote peer's MAC.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 include/net/mac80211.h | 4 ++++
 net/mac80211/tkip.c    | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f4ab2fb..dc552c7 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1203,6 +1203,9 @@ struct ieee80211_vif *wdev_to_ieee80211_vif(struct wireless_dev *wdev);
  *	fall back to software crypto. Note that this flag deals only with
  *	RX, if your crypto engine can't deal with TX you can also set the
  *	%IEEE80211_KEY_FLAG_SW_MGMT_TX flag to encrypt such frames in SW.
+ * @IEEE80211_KEY_FLAG_SW_RX_CRYPT: This flag indicates hardware will not
+ *	do any decrypt for received packets, though it may have uploaded
+ *	the hardware key to be used for encrypting transmitted frames.
  */
 enum ieee80211_key_flags {
 	IEEE80211_KEY_FLAG_GENERATE_IV	= 1<<1,
@@ -1211,6 +1214,7 @@ enum ieee80211_key_flags {
 	IEEE80211_KEY_FLAG_SW_MGMT_TX	= 1<<4,
 	IEEE80211_KEY_FLAG_PUT_IV_SPACE = 1<<5,
 	IEEE80211_KEY_FLAG_RX_MGMT	= 1<<6,
+	IEEE80211_KEY_FLAG_SW_RX_CRYPT	= 1<<7,
 };
 
 /**
diff --git a/net/mac80211/tkip.c b/net/mac80211/tkip.c
index 0ae2077..83ac9d9 100644
--- a/net/mac80211/tkip.c
+++ b/net/mac80211/tkip.c
@@ -283,7 +283,8 @@ int ieee80211_tkip_decrypt_data(struct crypto_cipher *tfm,
 		tkip_mixing_phase1(tk, &key->u.tkip.rx[queue], ta, iv32);
 	}
 	if (key->local->ops->update_tkip_key &&
-	    key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE &&
+	    (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE &&
+	     !(key->conf.flags & IEEE80211_KEY_FLAG_SW_RX_CRYPT)) &&
 	    key->u.tkip.rx[queue].state != TKIP_STATE_PHASE1_HW_UPLOADED) {
 		struct ieee80211_sub_if_data *sdata = key->sdata;
 
-- 
1.7.11.7

Re: [PATCH 2/2] mac80211: Support tx-hw-crypt with rx-sw-crypt.

[Johannes Berg]

On Wed, 2014-01-22 at 14:54 -0800, greearb@candelatech.com wrote:

> + * @IEEE80211_KEY_FLAG_SW_RX_CRYPT: This flag indicates hardware will not
> + *	do any decrypt for received packets, though it may have uploaded
> + *	the hardware key to be used for encrypting transmitted frames.

This isn't needed, all you need to do is return 0 from the set_key
callback in the driver, without programming the key into the device. And
then ignore tkip key updates for such a key.

johannes

Re: [PATCH 1/2] mac80211: Extra debug info for station cleanup case.

[Johannes Berg]

On Wed, 2014-01-22 at 14:54 -0800, greearb@candelatech.com wrote:
> -		WARN_ON_ONCE(ret != 0);
> +		if (WARN_ON_ONCE(ret != 0))
> +			sdata_info(sdata, "sta-info-destroy: drv-sta-state error: %i, sta: %pM\n",
> +				   ret, sta->sta.addr);

You should put the string into the warning

WARN_ONCE(ret, "sta-info-destroy: ...", ...)

johannes

Re: [PATCH 1/2] mac80211: Extra debug info for station cleanup case.

[Ben Greear]

On 01/22/2014 03:11 PM, Johannes Berg wrote:
> On Wed, 2014-01-22 at 14:54 -0800, greearb@candelatech.com wrote:
>> -		WARN_ON_ONCE(ret != 0);
>> +		if (WARN_ON_ONCE(ret != 0))
>> +			sdata_info(sdata, "sta-info-destroy: drv-sta-state error: %i, sta: %pM\n",
>> +				   ret, sta->sta.addr);
> 
> You should put the string into the warning
> 
> WARN_ONCE(ret, "sta-info-destroy: ...", ...)

I wanted it to kernel splat once and print out a message, and after that, only print
messages.

I think your suggestion will only splat _and_ print once?

Thanks,
Ben

> 
> johannes
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 2/2] mac80211: Support tx-hw-crypt with rx-sw-crypt.

[Ben Greear]

On 01/22/2014 03:10 PM, Johannes Berg wrote:
> On Wed, 2014-01-22 at 14:54 -0800, greearb@candelatech.com wrote:
> 
>> + * @IEEE80211_KEY_FLAG_SW_RX_CRYPT: This flag indicates hardware will not
>> + *	do any decrypt for received packets, though it may have uploaded
>> + *	the hardware key to be used for encrypting transmitted frames.
> 
> This isn't needed, all you need to do is return 0 from the set_key
> callback in the driver, without programming the key into the device. And
> then ignore tkip key updates for such a key.

I want and need to program the key into the device so that it can do the
encryption for transmit.  But, I need software to do full decryption on
receive.

That is the only way I could get ath10k to do any sort of software crypt,
and in fact, that is the most efficient way to get the features I need
so I might try getting ath9k to support the same mode some day...

I'll be posting ath10k patches to use this soon, but the dark magic
is down in the firmware in this case.

Thanks,
Ben

> 
> johannes
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 2/2] mac80211: Support tx-hw-crypt with rx-sw-crypt.

[Johannes Berg]

On Wed, 2014-01-22 at 16:05 -0800, Ben Greear wrote:

> >> + * @IEEE80211_KEY_FLAG_SW_RX_CRYPT: This flag indicates hardware will not
> >> + *	do any decrypt for received packets, though it may have uploaded
> >> + *	the hardware key to be used for encrypting transmitted frames.
> > 
> > This isn't needed, all you need to do is return 0 from the set_key
> > callback in the driver, without programming the key into the device. And
> > then ignore tkip key updates for such a key.
> 
> I want and need to program the key into the device so that it can do the
> encryption for transmit.  But, I need software to do full decryption on
> receive.

Ok so the "without programming the key into the device" part was iwlwifi
specific (on TX the key is given explicitly with that device.) However,
the same still applies - just replace that by "without enabling it for
RX" or so.

> That is the only way I could get ath10k to do any sort of software crypt,
> and in fact, that is the most efficient way to get the features I need
> so I might try getting ath9k to support the same mode some day...
> 
> I'll be posting ath10k patches to use this soon, but the dark magic
> is down in the firmware in this case.

Ok, whatever, what I'm saying is that mac80211 doesn't care about
whether you enabled the key for RX or not if you return 0 from
set_key(). All it cares about it that if you return 0 the key must be
available for TX. You therefore don't need to make any mac80211 changes.

johannes

Re: [PATCH 1/2] mac80211: Extra debug info for station cleanup case.

[Johannes Berg]

On Wed, 2014-01-22 at 15:58 -0800, Ben Greear wrote:

> > You should put the string into the warning
> > 
> > WARN_ONCE(ret, "sta-info-destroy: ...", ...)
> 
> I wanted it to kernel splat once and print out a message, and after that, only print
> messages.
> 
> I think your suggestion will only splat _and_ print once?

Hm, that's true, but it shouldn't be happening anyway? So why would you
care about subsequent instances - even the first is a bug?

johannes

Re: [PATCH 1/2] mac80211: Extra debug info for station cleanup case.

[Ben Greear]

On 01/23/2014 12:20 AM, Johannes Berg wrote:
> On Wed, 2014-01-22 at 15:58 -0800, Ben Greear wrote:
>
>>> You should put the string into the warning
>>>
>>> WARN_ONCE(ret, "sta-info-destroy: ...", ...)
>>
>> I wanted it to kernel splat once and print out a message, and after that, only print
>> messages.
>>
>> I think your suggestion will only splat _and_ print once?
>
> Hm, that's true, but it shouldn't be happening anyway? So why would you
> care about subsequent instances - even the first is a bug?

Nice to know how often it happens, and under what (other) circumstances it might
be happening.  As you know, these things are not always easy to fix even if you
have a kernel splat....

Thanks,
Ben

>
> johannes
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 2/2] mac80211: Support tx-hw-crypt with rx-sw-crypt.

[Ben Greear]

On 01/23/2014 12:19 AM, Johannes Berg wrote:
> On Wed, 2014-01-22 at 16:05 -0800, Ben Greear wrote:
>
>>>> + * @IEEE80211_KEY_FLAG_SW_RX_CRYPT: This flag indicates hardware will not
>>>> + *	do any decrypt for received packets, though it may have uploaded
>>>> + *	the hardware key to be used for encrypting transmitted frames.
>>>
>>> This isn't needed, all you need to do is return 0 from the set_key
>>> callback in the driver, without programming the key into the device. And
>>> then ignore tkip key updates for such a key.
>>
>> I want and need to program the key into the device so that it can do the
>> encryption for transmit.  But, I need software to do full decryption on
>> receive.
>
> Ok so the "without programming the key into the device" part was iwlwifi
> specific (on TX the key is given explicitly with that device.) However,
> the same still applies - just replace that by "without enabling it for
> RX" or so.
>
>> That is the only way I could get ath10k to do any sort of software crypt,
>> and in fact, that is the most efficient way to get the features I need
>> so I might try getting ath9k to support the same mode some day...
>>
>> I'll be posting ath10k patches to use this soon, but the dark magic
>> is down in the firmware in this case.
>
> Ok, whatever, what I'm saying is that mac80211 doesn't care about
> whether you enabled the key for RX or not if you return 0 from
> set_key(). All it cares about it that if you return 0 the key must be
> available for TX. You therefore don't need to make any mac80211 changes.

The ieee80211_tkip_decrypt_data method is explicitly checking if the key is uploaded to hardware.
I need it to think that it is not uploaded to hardware as far as that check
is concerned, otherwise packets will not decrypt properly.

There are other places that check that key-uploaded-to-hardware flag,
so I don't think I can just upload the key and then not set that flag?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 2/2] mac80211: Support tx-hw-crypt with rx-sw-crypt.

[Johannes Berg]

On Thu, 2014-01-23 at 06:52 -0800, Ben Greear wrote:

> The ieee80211_tkip_decrypt_data method is explicitly checking if the key is uploaded to hardware.

It is, but only for calling drv_update_tkip_key(), which you can make
the driver ignore.

> I need it to think that it is not uploaded to hardware as far as that check
> is concerned, otherwise packets will not decrypt properly.

I don't believe this is true. The stack was always designed so that if
you didn't decrypt the packet, you just don't set the flags indicating
it was decrypted and it'll be decrypted in software.

johannes

Re: [PATCH 2/2] mac80211: Support tx-hw-crypt with rx-sw-crypt.

[Ben Greear]

On 01/23/2014 06:55 AM, Johannes Berg wrote:
> On Thu, 2014-01-23 at 06:52 -0800, Ben Greear wrote:
> 
>> The ieee80211_tkip_decrypt_data method is explicitly checking if the key is uploaded to hardware.
> 
> It is, but only for calling drv_update_tkip_key(), which you can make
> the driver ignore.
> 
>> I need it to think that it is not uploaded to hardware as far as that check
>> is concerned, otherwise packets will not decrypt properly.
> 
> I don't believe this is true. The stack was always designed so that if
> you didn't decrypt the packet, you just don't set the flags indicating
> it was decrypted and it'll be decrypted in software.

Ok, I see now.  I must have been having some other random issue when
I made that change and saw (more) success.

I disabled it from my now-working code and it continues to work.

What's more, ath10k does not even implement that method, so it was
not doing anything anyway.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com