[PATCH 0/2] ath10k: fixes

[PATCH 0/2] ath10k: fixes

[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

[PATCH 0/2] ath10k: fixes

[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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size

[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

[PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size

[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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 2/2] ath10k: fix scheduling while atomic bug

[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

[PATCH 2/2] ath10k: fix scheduling while atomic bug

[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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size

[Kalle Valo]

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

Re: [PATCH 1/2] ath10k: Fix bug in max. VHT A-MPDU size

[Kalle Valo]

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug

[Kalle Valo]

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

Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug

[Kalle Valo]

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug

[Michal Kazior]

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ł

Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug

[Michal Kazior]

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ł

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug

[Kalle Valo]

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

Re: [PATCH 2/2] ath10k: fix scheduling while atomic bug

[Kalle Valo]

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k