Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH] wcn36xx: pass correct BSS index when deleting BSS keys
@ 2018-04-12 11:13 Daniel Mack
  2018-04-12 11:46 ` Loic Poulain
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Mack @ 2018-04-12 11:13 UTC (permalink / raw)
  To: linux-wireless; +Cc: wcn36xx, kvalo, rfried, bjorn.andersson, Daniel Mack

The firmware message to delete BSS keys expects a BSS index to be passed.
This field is currently hard-coded to 0. Fix this by passing in the index
we received from the firmware when the BSS was configured.

Also, AFAIU, when a BSS is deleted, the firmware apparently drops all the
keys associated with it. Trying to remove the key explicitly afterwards
will hence lead to the following message:

  wcn36xx: ERROR hal_remove_bsskey response failed err=16

This is now suppressed with an extra check for the BSS index validity.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 10 +++++++---
 drivers/net/wireless/ath/wcn36xx/smd.c  |  6 ++++--
 drivers/net/wireless/ath/wcn36xx/smd.h  |  2 ++
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 0bc9283e7d8d..1b17c35a7944 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -547,6 +547,7 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		} else {
 			wcn36xx_smd_set_bsskey(wcn,
 				vif_priv->encrypt_type,
+				vif_priv->bss_index,
 				key_conf->keyidx,
 				key_conf->keylen,
 				key);
@@ -564,10 +565,13 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		break;
 	case DISABLE_KEY:
 		if (!(IEEE80211_KEY_FLAG_PAIRWISE & key_conf->flags)) {
+			if (vif_priv->bss_index != WCN36XX_HAL_BSS_INVALID_IDX)
+				wcn36xx_smd_remove_bsskey(wcn,
+					vif_priv->encrypt_type,
+					vif_priv->bss_index,
+					key_conf->keyidx);
+
 			vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE;
-			wcn36xx_smd_remove_bsskey(wcn,
-				vif_priv->encrypt_type,
-				key_conf->keyidx);
 		} else {
 			sta_priv->is_data_encrypted = false;
 			/* do not remove key if disassociated */
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 9827a1e1124b..297a785d0785 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -1636,6 +1636,7 @@ int wcn36xx_smd_set_stakey(struct wcn36xx *wcn,
 
 int wcn36xx_smd_set_bsskey(struct wcn36xx *wcn,
 			   enum ani_ed_type enc_type,
+			   u8 bssidx,
 			   u8 keyidx,
 			   u8 keylen,
 			   u8 *key)
@@ -1645,7 +1646,7 @@ int wcn36xx_smd_set_bsskey(struct wcn36xx *wcn,
 
 	mutex_lock(&wcn->hal_mutex);
 	INIT_HAL_MSG(msg_body, WCN36XX_HAL_SET_BSSKEY_REQ);
-	msg_body.bss_idx = 0;
+	msg_body.bss_idx = bssidx;
 	msg_body.enc_type = enc_type;
 	msg_body.num_keys = 1;
 	msg_body.keys[0].id = keyidx;
@@ -1706,6 +1707,7 @@ int wcn36xx_smd_remove_stakey(struct wcn36xx *wcn,
 
 int wcn36xx_smd_remove_bsskey(struct wcn36xx *wcn,
 			      enum ani_ed_type enc_type,
+			      u8 bssidx,
 			      u8 keyidx)
 {
 	struct wcn36xx_hal_remove_bss_key_req_msg msg_body;
@@ -1713,7 +1715,7 @@ int wcn36xx_smd_remove_bsskey(struct wcn36xx *wcn,
 
 	mutex_lock(&wcn->hal_mutex);
 	INIT_HAL_MSG(msg_body, WCN36XX_HAL_RMV_BSSKEY_REQ);
-	msg_body.bss_idx = 0;
+	msg_body.bss_idx = bssidx;
 	msg_body.enc_type = enc_type;
 	msg_body.key_id = keyidx;
 
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 8076edf40ac8..61bb8d43138c 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -97,6 +97,7 @@ int wcn36xx_smd_set_stakey(struct wcn36xx *wcn,
 			   u8 sta_index);
 int wcn36xx_smd_set_bsskey(struct wcn36xx *wcn,
 			   enum ani_ed_type enc_type,
+			   u8 bssidx,
 			   u8 keyidx,
 			   u8 keylen,
 			   u8 *key);
@@ -106,6 +107,7 @@ int wcn36xx_smd_remove_stakey(struct wcn36xx *wcn,
 			      u8 sta_index);
 int wcn36xx_smd_remove_bsskey(struct wcn36xx *wcn,
 			      enum ani_ed_type enc_type,
+			      u8 bssidx,
 			      u8 keyidx);
 int wcn36xx_smd_enter_bmps(struct wcn36xx *wcn, struct ieee80211_vif *vif);
 int wcn36xx_smd_exit_bmps(struct wcn36xx *wcn, struct ieee80211_vif *vif);
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] wcn36xx: pass correct BSS index when deleting BSS keys
  2018-04-12 11:13 [PATCH] wcn36xx: pass correct BSS index when deleting BSS keys Daniel Mack
@ 2018-04-12 11:46 ` Loic Poulain
  2018-04-12 12:04   ` Daniel Mack
  0 siblings, 1 reply; 6+ messages in thread
From: Loic Poulain @ 2018-04-12 11:46 UTC (permalink / raw)
  To: Daniel Mack
  Cc: linux-wireless, wcn36xx, Kalle Valo, Ramon Fried, Bjorn Andersson

Hi Daniel,

> @@ -564,10 +565,13 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>                 break;
>         case DISABLE_KEY:
>                 if (!(IEEE80211_KEY_FLAG_PAIRWISE & key_conf->flags)) {
> +                       if (vif_priv->bss_index != WCN36XX_HAL_BSS_INVALID_IDX)
> +                               wcn36xx_smd_remove_bsskey(wcn,
> +                                       vif_priv->encrypt_type,
> +                                       vif_priv->bss_index,
> +                                       key_conf->keyidx);
> +
>                         vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE;
> -                       wcn36xx_smd_remove_bsskey(wcn,
> -                               vif_priv->encrypt_type,
> -                               key_conf->keyidx);

Note that moving vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE after
key removal also fixes an issue I observed in AP mode:
wcn36xx: ERROR hal_remove_bsskey response failed err=6

Indeed, trying to remove a key with non-matching encrypt type fails,
keeping the right encrypt_type for removal fixes the issue.

Patch looks good.

Regards,
Loic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] wcn36xx: pass correct BSS index when deleting BSS keys
  2018-04-12 11:46 ` Loic Poulain
@ 2018-04-12 12:04   ` Daniel Mack
  2018-04-12 12:14     ` Kalle Valo
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Mack @ 2018-04-12 12:04 UTC (permalink / raw)
  To: Loic Poulain
  Cc: wcn36xx, Ramon Fried, linux-wireless, Kalle Valo, Bjorn Andersson

On Thursday, April 12, 2018 01:46 PM, Loic Poulain wrote:
> Hi Daniel,
> 
>> @@ -564,10 +565,13 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>                 break;
>>         case DISABLE_KEY:
>>                 if (!(IEEE80211_KEY_FLAG_PAIRWISE & key_conf->flags)) {
>> +                       if (vif_priv->bss_index != WCN36XX_HAL_BSS_INVALID_IDX)
>> +                               wcn36xx_smd_remove_bsskey(wcn,
>> +                                       vif_priv->encrypt_type,
>> +                                       vif_priv->bss_index,
>> +                                       key_conf->keyidx);
>> +
>>                         vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE;
>> -                       wcn36xx_smd_remove_bsskey(wcn,
>> -                               vif_priv->encrypt_type,
>> -                               key_conf->keyidx);
> 
> Note that moving vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE after
> key removal also fixes an issue I observed in AP mode:
> wcn36xx: ERROR hal_remove_bsskey response failed err=6

Yeah, sorry. I did that intentionally, but missed to mention it in the
commit log.


Thanks,
Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] wcn36xx: pass correct BSS index when deleting BSS keys
  2018-04-12 12:04   ` Daniel Mack
@ 2018-04-12 12:14     ` Kalle Valo
  2018-04-12 12:16       ` Daniel Mack
  0 siblings, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2018-04-12 12:14 UTC (permalink / raw)
  To: Daniel Mack
  Cc: Loic Poulain, wcn36xx, Ramon Fried, linux-wireless,
	Bjorn Andersson

Daniel Mack <daniel@zonque.org> writes:

> On Thursday, April 12, 2018 01:46 PM, Loic Poulain wrote:
>> Hi Daniel,
>> 
>>> @@ -564,10 +565,13 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>                 break;
>>>         case DISABLE_KEY:
>>>                 if (!(IEEE80211_KEY_FLAG_PAIRWISE & key_conf->flags)) {
>>> +                       if (vif_priv->bss_index != WCN36XX_HAL_BSS_INVALID_IDX)
>>> +                               wcn36xx_smd_remove_bsskey(wcn,
>>> +                                       vif_priv->encrypt_type,
>>> +                                       vif_priv->bss_index,
>>> +                                       key_conf->keyidx);
>>> +
>>>                         vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE;
>>> -                       wcn36xx_smd_remove_bsskey(wcn,
>>> -                               vif_priv->encrypt_type,
>>> -                               key_conf->keyidx);
>> 
>> Note that moving vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE after
>> key removal also fixes an issue I observed in AP mode:
>> wcn36xx: ERROR hal_remove_bsskey response failed err=6
>
> Yeah, sorry. I did that intentionally, but missed to mention it in the
> commit log.

I can add that to the commit log, just tell me what to add.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] wcn36xx: pass correct BSS index when deleting BSS keys
  2018-04-12 12:14     ` Kalle Valo
@ 2018-04-12 12:16       ` Daniel Mack
  2018-04-12 12:38         ` Kalle Valo
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Mack @ 2018-04-12 12:16 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Loic Poulain, wcn36xx, Ramon Fried, linux-wireless,
	Bjorn Andersson

On Thursday, April 12, 2018 02:14 PM, Kalle Valo wrote:
> Daniel Mack <daniel@zonque.org> writes:
> 
>> On Thursday, April 12, 2018 01:46 PM, Loic Poulain wrote:
>>> Hi Daniel,
>>>
>>>> @@ -564,10 +565,13 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>>                 break;
>>>>         case DISABLE_KEY:
>>>>                 if (!(IEEE80211_KEY_FLAG_PAIRWISE & key_conf->flags)) {
>>>> +                       if (vif_priv->bss_index != WCN36XX_HAL_BSS_INVALID_IDX)
>>>> +                               wcn36xx_smd_remove_bsskey(wcn,
>>>> +                                       vif_priv->encrypt_type,
>>>> +                                       vif_priv->bss_index,
>>>> +                                       key_conf->keyidx);
>>>> +
>>>>                         vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE;
>>>> -                       wcn36xx_smd_remove_bsskey(wcn,
>>>> -                               vif_priv->encrypt_type,
>>>> -                               key_conf->keyidx);
>>>
>>> Note that moving vif_priv->encrypt_type = WCN36XX_HAL_ED_NONE after
>>> key removal also fixes an issue I observed in AP mode:
>>> wcn36xx: ERROR hal_remove_bsskey response failed err=6
>>
>> Yeah, sorry. I did that intentionally, but missed to mention it in the
>> commit log.
> 
> I can add that to the commit log, just tell me what to add.

I'll resend, hang on :)

Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] wcn36xx: pass correct BSS index when deleting BSS keys
  2018-04-12 12:16       ` Daniel Mack
@ 2018-04-12 12:38         ` Kalle Valo
  0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2018-04-12 12:38 UTC (permalink / raw)
  To: Daniel Mack
  Cc: Loic Poulain, wcn36xx, Ramon Fried, linux-wireless,
	Bjorn Andersson

Daniel Mack <daniel@zonque.org> writes:

>>> Yeah, sorry. I did that intentionally, but missed to mention it in the
>>> commit log.
>> 
>> I can add that to the commit log, just tell me what to add.
>
> I'll resend, hang on :)

Even better, thanks.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-04-12 12:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-12 11:13 [PATCH] wcn36xx: pass correct BSS index when deleting BSS keys Daniel Mack
2018-04-12 11:46 ` Loic Poulain
2018-04-12 12:04   ` Daniel Mack
2018-04-12 12:14     ` Kalle Valo
2018-04-12 12:16       ` Daniel Mack
2018-04-12 12:38         ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox