Some WMI calls were made in atomic contex, these patches fix
those. Please review.
---
Kalle Valo (2):
ath10k: fix ath10k_bss_assoc() to not sleep in atomic context
ath10k: add might_sleep() to ath10k_wmi_cmd_send()
Michal Kazior (5):
ath10k: use workqueue to set WEP TX key
ath10k: fix add_interface failure handling
ath10k: track vif list internally
ath10k: fix scheduling while atomic config bug
ath10k: remove unnecessary checks
drivers/net/wireless/ath/ath10k/core.c | 2
drivers/net/wireless/ath/ath10k/core.h | 7 +
drivers/net/wireless/ath/ath10k/mac.c | 312 +++++++++++++++++---------------
drivers/net/wireless/ath/ath10k/wmi.c | 2
4 files changed, 178 insertions(+), 145 deletions(-)
ath10k_bss_assoc() was calling ath10k_peer_assoc(), which can sleep, under
atomic rcu_read_lock() and causing scheduing while atomic errors. Workaround
that by delaying the call to ath10k_wmi_peer_assoc().
Signed-off-by: Kalle Valo <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 56 ++++++++++++++++++++++-----------
1 file changed, 37 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 4273eef..0b1cc51 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1137,26 +1137,25 @@ static void ath10k_peer_assoc_h_phymode(struct ath10k *ar,
WARN_ON(phymode == MODE_UNKNOWN);
}
-static int ath10k_peer_assoc(struct ath10k *ar,
- struct ath10k_vif *arvif,
- struct ieee80211_sta *sta,
- struct ieee80211_bss_conf *bss_conf)
+static int ath10k_peer_assoc_prepare(struct ath10k *ar,
+ struct ath10k_vif *arvif,
+ struct ieee80211_sta *sta,
+ struct ieee80211_bss_conf *bss_conf,
+ struct wmi_peer_assoc_complete_arg *arg)
{
- struct wmi_peer_assoc_complete_arg arg;
-
lockdep_assert_held(&ar->conf_mutex);
- memset(&arg, 0, sizeof(struct wmi_peer_assoc_complete_arg));
+ memset(arg, 0, sizeof(*arg));
- ath10k_peer_assoc_h_basic(ar, arvif, sta, bss_conf, &arg);
- ath10k_peer_assoc_h_crypto(ar, arvif, &arg);
- ath10k_peer_assoc_h_rates(ar, sta, &arg);
- ath10k_peer_assoc_h_ht(ar, sta, &arg);
- ath10k_peer_assoc_h_vht(ar, sta, &arg);
- ath10k_peer_assoc_h_qos(ar, arvif, sta, bss_conf, &arg);
- ath10k_peer_assoc_h_phymode(ar, arvif, sta, &arg);
+ ath10k_peer_assoc_h_basic(ar, arvif, sta, bss_conf, arg);
+ ath10k_peer_assoc_h_crypto(ar, arvif, arg);
+ ath10k_peer_assoc_h_rates(ar, sta, arg);
+ ath10k_peer_assoc_h_ht(ar, sta, arg);
+ ath10k_peer_assoc_h_vht(ar, sta, arg);
+ ath10k_peer_assoc_h_qos(ar, arvif, sta, bss_conf, arg);
+ ath10k_peer_assoc_h_phymode(ar, arvif, sta, arg);
- return ath10k_wmi_peer_assoc(ar, &arg);
+ return 0;
}
/* can be called only in mac80211 callbacks due to `key_count` usage */
@@ -1166,6 +1165,7 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
{
struct ath10k *ar = hw->priv;
struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+ struct wmi_peer_assoc_complete_arg peer_arg;
struct ieee80211_sta *ap_sta;
int ret;
@@ -1181,15 +1181,24 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
return;
}
- ret = ath10k_peer_assoc(ar, arvif, ap_sta, bss_conf);
+ ret = ath10k_peer_assoc_prepare(ar, arvif, ap_sta,
+ bss_conf, &peer_arg);
if (ret) {
- ath10k_warn("Peer assoc failed for %pM\n", bss_conf->bssid);
+ ath10k_warn("Peer assoc prepare failed for %pM\n: %d",
+ bss_conf->bssid, ret);
rcu_read_unlock();
return;
}
rcu_read_unlock();
+ ret = ath10k_wmi_peer_assoc(ar, &peer_arg);
+ if (ret) {
+ ath10k_warn("Peer assoc failed for %pM\n: %d",
+ bss_conf->bssid, ret);
+ return;
+ }
+
ath10k_dbg(ATH10K_DBG_MAC,
"mac vdev %d up (associated) bssid %pM aid %d\n",
arvif->vdev_id, bss_conf->bssid, bss_conf->aid);
@@ -1243,13 +1252,22 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
static int ath10k_station_assoc(struct ath10k *ar, struct ath10k_vif *arvif,
struct ieee80211_sta *sta)
{
+ struct wmi_peer_assoc_complete_arg peer_arg;
int ret = 0;
lockdep_assert_held(&ar->conf_mutex);
- ret = ath10k_peer_assoc(ar, arvif, sta, NULL);
+ ret = ath10k_peer_assoc_prepare(ar, arvif, sta, NULL, &peer_arg);
+ if (ret) {
+ ath10k_warn("WMI peer assoc prepare failed for %pM\n",
+ sta->addr);
+ return ret;
+ }
+
+ ret = ath10k_wmi_peer_assoc(ar, &peer_arg);
if (ret) {
- ath10k_warn("WMI peer assoc failed for %pM\n", sta->addr);
+ ath10k_warn("Peer assoc failed for STA %pM\n: %d",
+ sta->addr, ret);
return ret;
}
Kalle Valo <[email protected]> writes:
> From: Michal Kazior <[email protected]>
>
> mac80211 interface interations functions have
> peculiar locking issues. This patch introduces
> internal (to ath10k) vif list that will be used
> for vif iteration purposes.
>
> Signed-off-by: Michal Kazior <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
[...]
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -713,6 +713,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
> mutex_init(&ar->conf_mutex);
> spin_lock_init(&ar->data_lock);
>
> + INIT_LIST_HEAD(&ar->arvifs);
> INIT_LIST_HEAD(&ar->peers);
> init_waitqueue_head(&ar->peer_mapping_wq);
>
> @@ -824,6 +825,7 @@ int ath10k_core_start(struct ath10k *ar)
> goto err_disconnect_htc;
>
> ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
> + INIT_LIST_HEAD(&ar->arvifs);
>
> return 0;
Michal, why do the INIT_LIST_HEAD() twice? Isn't it enough to do it
core_start()?
--
Kalle Valo
On 15 October 2013 11:49, Kalle Valo <[email protected]> wrote:
> Kalle Valo <[email protected]> writes:
>
>> From: Michal Kazior <[email protected]>
>>
>> mac80211 interface interations functions have
>> peculiar locking issues. This patch introduces
>> internal (to ath10k) vif list that will be used
>> for vif iteration purposes.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>> Signed-off-by: Kalle Valo <[email protected]>
>
> [...]
>
>> --- a/drivers/net/wireless/ath/ath10k/core.c
>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>> @@ -713,6 +713,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
>> mutex_init(&ar->conf_mutex);
>> spin_lock_init(&ar->data_lock);
>>
>> + INIT_LIST_HEAD(&ar->arvifs);
>> INIT_LIST_HEAD(&ar->peers);
>> init_waitqueue_head(&ar->peer_mapping_wq);
>>
>> @@ -824,6 +825,7 @@ int ath10k_core_start(struct ath10k *ar)
>> goto err_disconnect_htc;
>>
>> ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
>> + INIT_LIST_HEAD(&ar->arvifs);
>>
>> return 0;
>
> Michal, why do the INIT_LIST_HEAD() twice? Isn't it enough to do it
> core_start()?
Ah, good point. It's most likely okay to just do it in
ath10k_core_start(). The one in ath10k_core_create() isn't required.
Michał
From: Michal Kazior <[email protected]>
Recent HTC/WMI changes introduced the bug. ath10k
was using _atomic iteration function with
sleepable functions.
mac80211 provides another iteration function but
it cannot be safely called in hw_config() callback
due to local->iflist_mtx being possibly acquired
already.
The patch uses internal vif list for iteration
purposes and removes/refactors no longer necessary
_iter functions.
Reported-By: Kalle Valo <[email protected]>
Signed-off-by: Michal Kazior <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 146 ++++++++++++++-------------------
1 file changed, 62 insertions(+), 84 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index c8e4180..bd42a14 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -723,35 +723,30 @@ static void ath10k_control_ibss(struct ath10k_vif *arvif,
/*
* Review this when mac80211 gains per-interface powersave support.
*/
-static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
{
- struct ath10k_generic_iter *ar_iter = data;
- struct ieee80211_conf *conf = &ar_iter->ar->hw->conf;
- struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+ struct ath10k *ar = arvif->ar;
+ struct ieee80211_conf *conf = &ar->hw->conf;
enum wmi_sta_powersave_param param;
enum wmi_sta_ps_mode psmode;
int ret;
lockdep_assert_held(&arvif->ar->conf_mutex);
- if (vif->type != NL80211_IFTYPE_STATION)
- return;
+ if (arvif->vif->type != NL80211_IFTYPE_STATION)
+ return 0;
if (conf->flags & IEEE80211_CONF_PS) {
psmode = WMI_STA_PS_MODE_ENABLED;
param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
- ret = ath10k_wmi_set_sta_ps_param(ar_iter->ar,
- arvif->vdev_id,
- param,
+ ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id, param,
conf->dynamic_ps_timeout);
if (ret) {
ath10k_warn("Failed to set inactivity time for VDEV: %d\n",
arvif->vdev_id);
- return;
+ return ret;
}
-
- ar_iter->ret = ret;
} else {
psmode = WMI_STA_PS_MODE_DISABLED;
}
@@ -759,11 +754,14 @@ static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d psmode %s\n",
arvif->vdev_id, psmode ? "enable" : "disable");
- ar_iter->ret = ath10k_wmi_set_psmode(ar_iter->ar, arvif->vdev_id,
- psmode);
- if (ar_iter->ret)
+ ret = ath10k_wmi_set_psmode(ar, arvif->vdev_id, psmode);
+ if (ret) {
ath10k_warn("Failed to set PS Mode: %d for VDEV: %d\n",
psmode, arvif->vdev_id);
+ return ret;
+ }
+
+ return 0;
}
/**********************/
@@ -1959,9 +1957,10 @@ static void ath10k_stop(struct ieee80211_hw *hw)
cancel_work_sync(&ar->restart_work);
}
-static void ath10k_config_ps(struct ath10k *ar)
+static int ath10k_config_ps(struct ath10k *ar)
{
- struct ath10k_generic_iter ar_iter;
+ struct ath10k_vif *arvif;
+ int ret = 0;
lockdep_assert_held(&ar->conf_mutex);
@@ -1970,17 +1969,17 @@ static void ath10k_config_ps(struct ath10k *ar)
* vdevs at this point we must not iterate over this interface list.
* This setting will be updated upon add_interface(). */
if (ar->state == ATH10K_STATE_RESTARTED)
- return;
-
- memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
- ar_iter.ar = ar;
+ return 0;
- ieee80211_iterate_active_interfaces_atomic(
- ar->hw, IEEE80211_IFACE_ITER_NORMAL,
- ath10k_ps_iter, &ar_iter);
+ list_for_each_entry(arvif, &ar->arvifs, list) {
+ ret = ath10k_mac_vif_setup_ps(arvif);
+ if (ret) {
+ ath10k_warn("could not setup powersave (%d)\n", ret);
+ break;
+ }
+ }
- if (ar_iter.ret)
- ath10k_warn("failed to set ps config (%d)\n", ar_iter.ret);
+ return ret;
}
static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
@@ -2882,86 +2881,65 @@ static int ath10k_cancel_remain_on_channel(struct ieee80211_hw *hw)
* Both RTS and Fragmentation threshold are interface-specific
* in ath10k, but device-specific in mac80211.
*/
-static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
-{
- struct ath10k_generic_iter *ar_iter = data;
- struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
- u32 rts = ar_iter->ar->hw->wiphy->rts_threshold;
- lockdep_assert_held(&arvif->ar->conf_mutex);
+static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
+{
+ struct ath10k *ar = hw->priv;
+ struct ath10k_vif *arvif;
+ int ret = 0;
/* During HW reconfiguration mac80211 reports all interfaces that were
* running until reconfiguration was started. Since FW doesn't have any
* vdevs at this point we must not iterate over this interface list.
* This setting will be updated upon add_interface(). */
- if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
- return;
-
- ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts_threshold %d\n",
- arvif->vdev_id, rts);
-
- ar_iter->ret = ath10k_mac_set_rts(arvif, rts);
- if (ar_iter->ret)
- ath10k_warn("Failed to set RTS threshold for VDEV: %d\n",
- arvif->vdev_id);
-}
-
-static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
-{
- struct ath10k_generic_iter ar_iter;
- struct ath10k *ar = hw->priv;
-
- memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
- ar_iter.ar = ar;
+ if (ar->state == ATH10K_STATE_RESTARTED)
+ return 0;
mutex_lock(&ar->conf_mutex);
- ieee80211_iterate_active_interfaces_atomic(
- hw, IEEE80211_IFACE_ITER_NORMAL,
- ath10k_set_rts_iter, &ar_iter);
+ list_for_each_entry(arvif, &ar->arvifs, list) {
+ ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
+ arvif->vdev_id, value);
+
+ ret = ath10k_mac_set_rts(arvif, value);
+ if (ret) {
+ ath10k_warn("could not set rts threshold for vdev %d (%d)\n",
+ arvif->vdev_id, ret);
+ break;
+ }
+ }
mutex_unlock(&ar->conf_mutex);
- return ar_iter.ret;
+ return ret;
}
-static void ath10k_set_frag_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
{
- struct ath10k_generic_iter *ar_iter = data;
- struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
- u32 frag = ar_iter->ar->hw->wiphy->frag_threshold;
-
- lockdep_assert_held(&arvif->ar->conf_mutex);
+ struct ath10k *ar = hw->priv;
+ struct ath10k_vif *arvif;
+ int ret = 0;
/* During HW reconfiguration mac80211 reports all interfaces that were
* running until reconfiguration was started. Since FW doesn't have any
* vdevs at this point we must not iterate over this interface list.
* This setting will be updated upon add_interface(). */
- if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
- return;
-
- ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation_threshold %d\n",
- arvif->vdev_id, frag);
-
- ar_iter->ret = ath10k_mac_set_frag(arvif, frag);
- if (ar_iter->ret)
- ath10k_warn("Failed to set frag threshold for VDEV: %d\n",
- arvif->vdev_id);
-}
-
-static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
-{
- struct ath10k_generic_iter ar_iter;
- struct ath10k *ar = hw->priv;
-
- memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
- ar_iter.ar = ar;
+ if (ar->state == ATH10K_STATE_RESTARTED)
+ return 0;
mutex_lock(&ar->conf_mutex);
- ieee80211_iterate_active_interfaces_atomic(
- hw, IEEE80211_IFACE_ITER_NORMAL,
- ath10k_set_frag_iter, &ar_iter);
+ list_for_each_entry(arvif, &ar->arvifs, list) {
+ ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",
+ arvif->vdev_id, value);
+
+ ret = ath10k_mac_set_rts(arvif, value);
+ if (ret) {
+ ath10k_warn("could not set fragmentation threshold for vdev %d (%d)\n",
+ arvif->vdev_id, ret);
+ break;
+ }
+ }
mutex_unlock(&ar->conf_mutex);
- return ar_iter.ret;
+ return ret;
}
static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)
Michal Kazior <[email protected]> writes:
> On 15 October 2013 11:49, Kalle Valo <[email protected]> wrote:
>> Kalle Valo <[email protected]> writes:
>>
>>> From: Michal Kazior <[email protected]>
>>>
>>> mac80211 interface interations functions have
>>> peculiar locking issues. This patch introduces
>>> internal (to ath10k) vif list that will be used
>>> for vif iteration purposes.
>>>
>>> Signed-off-by: Michal Kazior <[email protected]>
>>> Signed-off-by: Kalle Valo <[email protected]>
>>
>> [...]
>>
>> Michal, why do the INIT_LIST_HEAD() twice? Isn't it enough to do it
>> core_start()?
>
> Ah, good point. It's most likely okay to just do it in
> ath10k_core_start(). The one in ath10k_core_create() isn't required.
Ok, I removed the one from ath10k_core_create() before I commited this.
--
Kalle Valo
From: Michal Kazior <[email protected]>
Recent WMI/HTC changes made it possible for WMI
commands to sleep (if there's not enough HTC TX
credits to submit a command). TX path is in an
atomic context so calling WMI commands in it is
wrong.
This simply moves WEP key index update to a worker
and fixes the 'scheduling while atomic' bug.
This still leaves multiple WEP key handling laggy,
i.e. some frames may be TXed with an old/different
key (although recipient should still be able to RX
them).
kvalo: changed the title
Signed-off-by: Michal Kazior <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 4 ++
drivers/net/wireless/ath/ath10k/mac.c | 53 ++++++++++++++++++++++----------
2 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index ce36daa..cef5455 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -215,8 +215,10 @@ struct ath10k_vif {
struct ath10k *ar;
struct ieee80211_vif *vif;
+ struct work_struct wep_key_work;
struct ieee80211_key_conf *wep_keys[WMI_MAX_KEY_INDEX + 1];
- u8 def_wep_key_index;
+ u8 def_wep_key_idx;
+ u8 def_wep_key_newidx;
u16 tx_seq_no;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 97d7111..bc0e17b 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1239,7 +1239,7 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
/* FIXME: why don't we print error if wmi call fails? */
ret = ath10k_wmi_vdev_down(ar, arvif->vdev_id);
- arvif->def_wep_key_index = 0;
+ arvif->def_wep_key_idx = 0;
}
static int ath10k_station_assoc(struct ath10k *ar, struct ath10k_vif *arvif,
@@ -1467,6 +1467,30 @@ static void ath10k_tx_h_qos_workaround(struct ieee80211_hw *hw,
skb_pull(skb, IEEE80211_QOS_CTL_LEN);
}
+static void ath10k_tx_wep_key_work(struct work_struct *work)
+{
+ struct ath10k_vif *arvif = container_of(work, struct ath10k_vif,
+ wep_key_work);
+ int ret, keyidx = arvif->def_wep_key_newidx;
+
+ if (arvif->def_wep_key_idx == keyidx)
+ return;
+
+ ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d set keyidx %d\n",
+ arvif->vdev_id, keyidx);
+
+ ret = ath10k_wmi_vdev_set_param(arvif->ar,
+ arvif->vdev_id,
+ arvif->ar->wmi.vdev_param->def_keyid,
+ keyidx);
+ if (ret) {
+ ath10k_warn("could not update wep keyidx (%d)\n", ret);
+ return;
+ }
+
+ arvif->def_wep_key_idx = keyidx;
+}
+
static void ath10k_tx_h_update_wep_key(struct sk_buff *skb)
{
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
@@ -1475,8 +1499,6 @@ static void ath10k_tx_h_update_wep_key(struct sk_buff *skb)
struct ath10k *ar = arvif->ar;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
struct ieee80211_key_conf *key = info->control.hw_key;
- u32 vdev_param;
- int ret;
if (!ieee80211_has_protected(hdr->frame_control))
return;
@@ -1488,21 +1510,14 @@ static void ath10k_tx_h_update_wep_key(struct sk_buff *skb)
key->cipher != WLAN_CIPHER_SUITE_WEP104)
return;
- if (key->keyidx == arvif->def_wep_key_index)
- return;
-
- ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d keyidx %d\n",
- arvif->vdev_id, key->keyidx);
-
- vdev_param = ar->wmi.vdev_param->def_keyid;
- ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
- key->keyidx);
- if (ret) {
- ath10k_warn("could not update wep keyidx (%d)\n", ret);
+ if (key->keyidx == arvif->def_wep_key_idx)
return;
- }
- arvif->def_wep_key_index = key->keyidx;
+ /* FIXME: Most likely a few frames will be TXed with an old key. Simply
+ * queueing frames until key index is updated is not an option because
+ * sk_buff may need more processing to be done, e.g. offchannel */
+ arvif->def_wep_key_newidx = key->keyidx;
+ ieee80211_queue_work(ar->hw, &arvif->wep_key_work);
}
static void ath10k_tx_h_add_p2p_noa_ie(struct ath10k *ar, struct sk_buff *skb)
@@ -2023,6 +2038,8 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
arvif->ar = ar;
arvif->vif = vif;
+ INIT_WORK(&arvif->wep_key_work, ath10k_tx_wep_key_work);
+
if ((vif->type == NL80211_IFTYPE_MONITOR) && ar->monitor_present) {
ath10k_warn("Only one monitor interface allowed\n");
ret = -EBUSY;
@@ -2078,7 +2095,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
vdev_param = ar->wmi.vdev_param->def_keyid;
ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
- arvif->def_wep_key_index);
+ arvif->def_wep_key_idx);
if (ret)
ath10k_warn("Failed to set default keyid: %d\n", ret);
@@ -2147,6 +2164,8 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
mutex_lock(&ar->conf_mutex);
+ cancel_work_sync(&arvif->wep_key_work);
+
spin_lock_bh(&ar->data_lock);
if (arvif->beacon) {
dev_kfree_skb_any(arvif->beacon);
From: Michal Kazior <[email protected]>
mac80211 interface interations functions have
peculiar locking issues. This patch introduces
internal (to ath10k) vif list that will be used
for vif iteration purposes.
Signed-off-by: Michal Kazior <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 2 ++
drivers/net/wireless/ath/ath10k/core.h | 3 +++
drivers/net/wireless/ath/ath10k/mac.c | 3 +++
3 files changed, 8 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index c5561a9..ed51b51 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -713,6 +713,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
mutex_init(&ar->conf_mutex);
spin_lock_init(&ar->data_lock);
+ INIT_LIST_HEAD(&ar->arvifs);
INIT_LIST_HEAD(&ar->peers);
init_waitqueue_head(&ar->peer_mapping_wq);
@@ -824,6 +825,7 @@ int ath10k_core_start(struct ath10k *ar)
goto err_disconnect_htc;
ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+ INIT_LIST_HEAD(&ar->arvifs);
return 0;
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index cef5455..0934f76 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -205,6 +205,8 @@ struct ath10k_peer {
#define ATH10K_VDEV_SETUP_TIMEOUT_HZ (5*HZ)
struct ath10k_vif {
+ struct list_head list;
+
u32 vdev_id;
enum wmi_vdev_type vdev_type;
enum wmi_vdev_subtype vdev_subtype;
@@ -404,6 +406,7 @@ struct ath10k {
/* protects shared structure data */
spinlock_t data_lock;
+ struct list_head arvifs;
struct list_head peers;
wait_queue_head_t peer_mapping_wq;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 16e55bd..c8e4180 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2093,6 +2093,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
}
ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+ list_add(&arvif->list, &ar->arvifs);
vdev_param = ar->wmi.vdev_param->def_keyid;
ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
@@ -2175,6 +2176,7 @@ err_peer_delete:
err_vdev_delete:
ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+ list_del(&arvif->list);
err:
mutex_unlock(&ar->conf_mutex);
@@ -2201,6 +2203,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
spin_unlock_bh(&ar->data_lock);
ar->free_vdev_map |= 1 << (arvif->vdev_id);
+ list_del(&arvif->list);
if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
ret = ath10k_peer_delete(arvif->ar, arvif->vdev_id, vif->addr);
ath10k_wmi_cmd_send() will now sleep if there are no credits available.
To make it easier to catch callers in atomic context add might_sleep()
to the function.
Signed-off-by: Kalle Valo <[email protected]>
---
drivers/net/wireless/ath/ath10k/wmi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index d1e513e..77238af 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -613,6 +613,8 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
{
int ret = -EOPNOTSUPP;
+ might_sleep();
+
if (cmd_id == WMI_CMD_UNSUPPORTED) {
ath10k_warn("wmi command %d is not supported by firmware\n",
cmd_id);
From: Michal Kazior <[email protected]>
If something failed along add_interface() setup it
was possible to leak a vdev id, vdev and peer.
This could end up with leaked FW state or FW crash
(assuming add_interface() failure wasn't a result of
a crash).
kvalo: rebased, whitespace fixes
Signed-off-by: Michal Kazior <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 53 +++++++++++++++++++++++++--------
1 file changed, 40 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index bc0e17b..16e55bd 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2043,18 +2043,17 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
if ((vif->type == NL80211_IFTYPE_MONITOR) && ar->monitor_present) {
ath10k_warn("Only one monitor interface allowed\n");
ret = -EBUSY;
- goto exit;
+ goto err;
}
bit = ffs(ar->free_vdev_map);
if (bit == 0) {
ret = -EBUSY;
- goto exit;
+ goto err;
}
arvif->vdev_id = bit - 1;
arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
- ar->free_vdev_map &= ~(1 << arvif->vdev_id);
if (ar->p2p)
arvif->vdev_subtype = WMI_VDEV_SUBTYPE_P2P_DEVICE;
@@ -2090,27 +2089,33 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
arvif->vdev_subtype, vif->addr);
if (ret) {
ath10k_warn("WMI vdev create failed: ret %d\n", ret);
- goto exit;
+ goto err;
}
+ ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+
vdev_param = ar->wmi.vdev_param->def_keyid;
ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
arvif->def_wep_key_idx);
- if (ret)
+ if (ret) {
ath10k_warn("Failed to set default keyid: %d\n", ret);
+ goto err_vdev_delete;
+ }
vdev_param = ar->wmi.vdev_param->tx_encap_type;
ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
ATH10K_HW_TXRX_NATIVE_WIFI);
/* 10.X firmware does not support this VDEV parameter. Do not warn */
- if (ret && ret != -EOPNOTSUPP)
+ if (ret && ret != -EOPNOTSUPP) {
ath10k_warn("Failed to set TX encap: %d\n", ret);
+ goto err_vdev_delete;
+ }
if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
ret = ath10k_peer_create(ar, arvif->vdev_id, vif->addr);
if (ret) {
ath10k_warn("Failed to create peer for AP: %d\n", ret);
- goto exit;
+ goto err_vdev_delete;
}
}
@@ -2119,39 +2124,61 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
value = WMI_STA_PS_RX_WAKE_POLICY_WAKE;
ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
param, value);
- if (ret)
+ if (ret) {
ath10k_warn("Failed to set RX wake policy: %d\n", ret);
+ goto err_peer_delete;
+ }
param = WMI_STA_PS_PARAM_TX_WAKE_THRESHOLD;
value = WMI_STA_PS_TX_WAKE_THRESHOLD_ALWAYS;
ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
param, value);
- if (ret)
+ if (ret) {
ath10k_warn("Failed to set TX wake thresh: %d\n", ret);
+ goto err_peer_delete;
+ }
param = WMI_STA_PS_PARAM_PSPOLL_COUNT;
value = WMI_STA_PS_PSPOLL_COUNT_NO_MAX;
ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
param, value);
- if (ret)
+ if (ret) {
ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
+ goto err_peer_delete;
+ }
}
ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold);
- if (ret)
+ if (ret) {
ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
arvif->vdev_id, ret);
+ goto err_peer_delete;
+ }
ret = ath10k_mac_set_frag(arvif, ar->hw->wiphy->frag_threshold);
- if (ret)
+ if (ret) {
ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
arvif->vdev_id, ret);
+ goto err_peer_delete;
+ }
if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
ar->monitor_present = true;
-exit:
mutex_unlock(&ar->conf_mutex);
+ return 0;
+
+err_peer_delete:
+ if (arvif->vdev_type == WMI_VDEV_TYPE_AP)
+ ath10k_wmi_peer_delete(ar, arvif->vdev_id, vif->addr);
+
+err_vdev_delete:
+ ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
+ ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+
+err:
+ mutex_unlock(&ar->conf_mutex);
+
return ret;
}
Kalle Valo <[email protected]> writes:
> Some WMI calls were made in atomic contex, these patches fix
> those. Please review.
>
> ---
>
> Kalle Valo (2):
> ath10k: fix ath10k_bss_assoc() to not sleep in atomic context
> ath10k: add might_sleep() to ath10k_wmi_cmd_send()
>
> Michal Kazior (5):
> ath10k: use workqueue to set WEP TX key
> ath10k: fix add_interface failure handling
> ath10k: track vif list internally
> ath10k: fix scheduling while atomic config bug
> ath10k: remove unnecessary checks
All patches applied.
--
Kalle Valo
From: Michal Kazior <[email protected]>
mac80211 interface iteration functions that were
used originally iterated over interfaces that
weren't re-added to the driver during recovery.
Since internal vif list is now used it's safe to
remove the safe-guard as internal vif list is
based on add/remove_interface function which
guarantees that vdev is created in FW before it is
iterated over.
Signed-off-by: Michal Kazior <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index bd42a14..4273eef 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1964,13 +1964,6 @@ static int ath10k_config_ps(struct ath10k *ar)
lockdep_assert_held(&ar->conf_mutex);
- /* During HW reconfiguration mac80211 reports all interfaces that were
- * running until reconfiguration was started. Since FW doesn't have any
- * vdevs at this point we must not iterate over this interface list.
- * This setting will be updated upon add_interface(). */
- if (ar->state == ATH10K_STATE_RESTARTED)
- return 0;
-
list_for_each_entry(arvif, &ar->arvifs, list) {
ret = ath10k_mac_vif_setup_ps(arvif);
if (ret) {
@@ -2888,13 +2881,6 @@ static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
struct ath10k_vif *arvif;
int ret = 0;
- /* During HW reconfiguration mac80211 reports all interfaces that were
- * running until reconfiguration was started. Since FW doesn't have any
- * vdevs at this point we must not iterate over this interface list.
- * This setting will be updated upon add_interface(). */
- if (ar->state == ATH10K_STATE_RESTARTED)
- return 0;
-
mutex_lock(&ar->conf_mutex);
list_for_each_entry(arvif, &ar->arvifs, list) {
ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
@@ -2918,13 +2904,6 @@ static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
struct ath10k_vif *arvif;
int ret = 0;
- /* During HW reconfiguration mac80211 reports all interfaces that were
- * running until reconfiguration was started. Since FW doesn't have any
- * vdevs at this point we must not iterate over this interface list.
- * This setting will be updated upon add_interface(). */
- if (ar->state == ATH10K_STATE_RESTARTED)
- return 0;
-
mutex_lock(&ar->conf_mutex);
list_for_each_entry(arvif, &ar->arvifs, list) {
ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",