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 <[email protected]>
---
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
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
On Thursday, April 12, 2018 02:14 PM, Kalle Valo wrote:
> Daniel Mack <[email protected]> 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
Daniel Mack <[email protected]> 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
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
Daniel Mack <[email protected]> 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