* [PATCH 0/2] ath10k: fixes @ 2013-09-27 14:36 Michal Kazior 2013-09-27 14:36 ` [PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size Michal Kazior 2013-09-27 14:36 ` [PATCH 2/2] ath10k: fix scheduling while atomic bug Michal Kazior 0 siblings, 2 replies; 7+ messages in thread From: Michal Kazior @ 2013-09-27 14:36 UTC (permalink / raw) To: ath10k; +Cc: linux-wireless, Michal Kazior Hi, One throughput fix and one bug fix. The bug fix addresses my oversight when reworking HTC/WMI. I haven't seen the error yet but I'm quite certain this should be fixed. Michal Kazior (1): ath10k: fix scheduling while atomic bug Sujith Manoharan (1): ath10k: Fix bug in max. VHT A-MPDU size drivers/net/wireless/ath/ath10k/core.h | 4 ++- drivers/net/wireless/ath/ath10k/mac.c | 60 ++++++++++++++++++++++---------- 2 files changed, 45 insertions(+), 19 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size 2013-09-27 14:36 [PATCH 0/2] ath10k: fixes Michal Kazior @ 2013-09-27 14:36 ` Michal Kazior 2013-10-01 16:27 ` Kalle Valo 2013-09-27 14:36 ` [PATCH 2/2] ath10k: fix scheduling while atomic bug Michal Kazior 1 sibling, 1 reply; 7+ messages in thread From: Michal Kazior @ 2013-09-27 14:36 UTC (permalink / raw) To: ath10k; +Cc: linux-wireless, Sujith Manoharan, Michal Kazior From: Sujith Manoharan <c_manoha@qca.qualcomm.com> For VHT peers, the maximum A-MPDU size has to be calculated from the VHT capabilities element and not the HT-cap. The formula is the same, but a higher value is used in VHT, allowing larger aggregates to be transmitted. Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- drivers/net/wireless/ath/ath10k/mac.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 8684e03..b55b680 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -1033,14 +1033,19 @@ static void ath10k_peer_assoc_h_vht(struct ath10k *ar, struct wmi_peer_assoc_complete_arg *arg) { const struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap; + u8 ampdu_factor; if (!vht_cap->vht_supported) return; arg->peer_flags |= WMI_PEER_VHT; - arg->peer_vht_caps = vht_cap->cap; + ampdu_factor = + (vht_cap->cap & IEEE80211_VHT_CAP_MAX_A_MPDU_LENGTH_EXPONENT_MASK) >> + IEEE80211_VHT_CAP_MAX_A_MPDU_LENGTH_EXPONENT_SHIFT; + arg->peer_max_mpdu = (1 << (IEEE80211_HT_MAX_AMPDU_FACTOR + ampdu_factor)) - 1; + if (sta->bandwidth == IEEE80211_STA_RX_BW_80) arg->peer_flags |= WMI_PEER_80MHZ; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size 2013-09-27 14:36 ` [PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size Michal Kazior @ 2013-10-01 16:27 ` Kalle Valo 0 siblings, 0 replies; 7+ messages in thread From: Kalle Valo @ 2013-10-01 16:27 UTC (permalink / raw) To: Michal Kazior; +Cc: ath10k, Sujith Manoharan, linux-wireless Michal Kazior <michal.kazior@tieto.com> writes: > From: Sujith Manoharan <c_manoha@qca.qualcomm.com> > > For VHT peers, the maximum A-MPDU size has to be calculated > from the VHT capabilities element and not the HT-cap. The formula > is the same, but a higher value is used in VHT, allowing larger > aggregates to be transmitted. > > Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com> > Signed-off-by: Michal Kazior <michal.kazior@tieto.com> This kills the TCP TX throughput with D-Link DIR-865L (ath10k x86 as the client) from ~350 Mbps to ~120 Mbps. Sujith mentioned private there's a workaround for this, this patch should have that. -- Kalle Valo ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] ath10k: fix scheduling while atomic bug 2013-09-27 14:36 [PATCH 0/2] ath10k: fixes Michal Kazior 2013-09-27 14:36 ` [PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size Michal Kazior @ 2013-09-27 14:36 ` Michal Kazior 2013-10-01 16:35 ` Kalle Valo 1 sibling, 1 reply; 7+ messages in thread From: Michal Kazior @ 2013-09-27 14:36 UTC (permalink / raw) To: ath10k; +Cc: linux-wireless, Michal Kazior Recent WMI/HTC changes broke WEP with multiple keys. If WMI had no HTC TX credits to submit command for default wep index update it would trigger a bug. This simply moves the wep key index update to a worker. The key update may happen some time after first frame with a different wep key has been sent (i.e. some frames will be sent with old key). This was the case before too as WMI commands were asynchronous. Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- 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 d5da8a9..ba6fd4d 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 b55b680..e849121 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -1231,7 +1231,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, @@ -1432,6 +1432,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); @@ -1440,8 +1464,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; @@ -1453,21 +1475,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) @@ -2003,6 +2018,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; @@ -2058,7 +2075,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); @@ -2126,6 +2143,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); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug 2013-09-27 14:36 ` [PATCH 2/2] ath10k: fix scheduling while atomic bug Michal Kazior @ 2013-10-01 16:35 ` Kalle Valo 2013-10-02 5:35 ` Michal Kazior 0 siblings, 1 reply; 7+ messages in thread From: Kalle Valo @ 2013-10-01 16:35 UTC (permalink / raw) To: Michal Kazior; +Cc: ath10k, linux-wireless Michal Kazior <michal.kazior@tieto.com> writes: > Recent WMI/HTC changes broke WEP with multiple > keys. If WMI had no HTC TX credits to submit > command for default wep index update it would > trigger a bug. > > This simply moves the wep key index update to a > worker. > > The key update may happen some time after first > frame with a different wep key has been sent (i.e. > some frames will be sent with old key). This was > the case before too as WMI commands were > asynchronous. > > Signed-off-by: Michal Kazior <michal.kazior@tieto.com> This looks problematic. Basically you just delay sending the WMI command, but there's no guarantee that we actually have free credits at the time of transmission. So to me it looks like this fixes the issue just by luck. -- Kalle Valo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug 2013-10-01 16:35 ` Kalle Valo @ 2013-10-02 5:35 ` Michal Kazior 2013-10-04 6:37 ` Kalle Valo 0 siblings, 1 reply; 7+ messages in thread From: Michal Kazior @ 2013-10-02 5:35 UTC (permalink / raw) To: Kalle Valo; +Cc: ath10k, linux-wireless On 1 October 2013 18:35, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Michal Kazior <michal.kazior@tieto.com> writes: > >> Recent WMI/HTC changes broke WEP with multiple >> keys. If WMI had no HTC TX credits to submit >> command for default wep index update it would >> trigger a bug. >> >> This simply moves the wep key index update to a >> worker. >> >> The key update may happen some time after first >> frame with a different wep key has been sent (i.e. >> some frames will be sent with old key). This was >> the case before too as WMI commands were >> asynchronous. >> >> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> > > This looks problematic. Basically you just delay sending the WMI > command, but there's no guarantee that we actually have free credits at > the time of transmission. So to me it looks like this fixes the issue > just by luck. One thing at a time. This patch fixes 'scheduling while atomic' bug that was introduced with recent HTC/WMI changes. Multiple wep key handling had the same issue you point out before HTC/WMI changes. To fix the issue you mention we'd need to stop mac80211 queues, queue frame(s) with a new key on internal queue, schedule a work. The work should set new wep key index, send pending frames from internal queue and wake mac80211 queues. This is a bit more complex than it sounds though. Michał ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug 2013-10-02 5:35 ` Michal Kazior @ 2013-10-04 6:37 ` Kalle Valo 0 siblings, 0 replies; 7+ messages in thread From: Kalle Valo @ 2013-10-04 6:37 UTC (permalink / raw) To: Michal Kazior; +Cc: ath10k, linux-wireless Michal Kazior <michal.kazior@tieto.com> writes: > On 1 October 2013 18:35, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> Michal Kazior <michal.kazior@tieto.com> writes: >> >>> Recent WMI/HTC changes broke WEP with multiple >>> keys. If WMI had no HTC TX credits to submit >>> command for default wep index update it would >>> trigger a bug. >>> >>> This simply moves the wep key index update to a >>> worker. >>> >>> The key update may happen some time after first >>> frame with a different wep key has been sent (i.e. >>> some frames will be sent with old key). This was >>> the case before too as WMI commands were >>> asynchronous. >>> >>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> >> >> This looks problematic. Basically you just delay sending the WMI >> command, but there's no guarantee that we actually have free credits at >> the time of transmission. So to me it looks like this fixes the issue >> just by luck. > > One thing at a time. > > This patch fixes 'scheduling while atomic' bug that was introduced > with recent HTC/WMI changes. Ah, that's what you mean with "triggers a bug" in the commit log? Ok, even though I consider this very ugly I guess it's alright as a short term fix. But please describe the bug in more detail in the commit log, at least mention that we are sleeping in an atomic context (or something like that). -- Kalle Valo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-10-04 6:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-27 14:36 [PATCH 0/2] ath10k: fixes Michal Kazior 2013-09-27 14:36 ` [PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size Michal Kazior 2013-10-01 16:27 ` Kalle Valo 2013-09-27 14:36 ` [PATCH 2/2] ath10k: fix scheduling while atomic bug Michal Kazior 2013-10-01 16:35 ` Kalle Valo 2013-10-02 5:35 ` Michal Kazior 2013-10-04 6:37 ` Kalle Valo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).