* [PATCH 3.14] ath9k: fix ps-poll responses under a-mpdu sessions
@ 2014-02-22 12:48 Felix Fietkau
2014-03-11 9:28 ` Helmut Schaa
0 siblings, 1 reply; 6+ messages in thread
From: Felix Fietkau @ 2014-02-22 12:48 UTC (permalink / raw)
To: linux-wireless; +Cc: linville
When passing tx frames to the U-APSD queue for powersave poll responses,
the ath_atx_tid pointer needs to be passed to ath_tx_setup_buffer for
proper sequence number accounting.
This fixes high latency and connection stability issues with ath9k
running as AP and a few kinds of mobile phones as client, when PS-Poll
is heavily used
Cc: stable@vger.kernel.org
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
drivers/net/wireless/ath/ath9k/xmit.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 4f4ce83..f042a18 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2186,14 +2186,15 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
txq->stopped = true;
}
+ if (txctl->an)
+ tid = ath_get_skb_tid(sc, txctl->an, skb);
+
if (info->flags & IEEE80211_TX_CTL_PS_RESPONSE) {
ath_txq_unlock(sc, txq);
txq = sc->tx.uapsdq;
ath_txq_lock(sc, txq);
} else if (txctl->an &&
ieee80211_is_data_present(hdr->frame_control)) {
- tid = ath_get_skb_tid(sc, txctl->an, skb);
-
WARN_ON(tid->ac->txq != txctl->txq);
if (info->flags & IEEE80211_TX_CTL_CLEAR_PS_FILT)
--
1.8.3.4 (Apple Git-47)
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3.14] ath9k: fix ps-poll responses under a-mpdu sessions
2014-02-22 12:48 [PATCH 3.14] ath9k: fix ps-poll responses under a-mpdu sessions Felix Fietkau
@ 2014-03-11 9:28 ` Helmut Schaa
2014-03-11 9:36 ` Felix Fietkau
0 siblings, 1 reply; 6+ messages in thread
From: Helmut Schaa @ 2014-03-11 9:28 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, John Linville
On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> When passing tx frames to the U-APSD queue for powersave poll responses,
> the ath_atx_tid pointer needs to be passed to ath_tx_setup_buffer for
> proper sequence number accounting.
>
> This fixes high latency and connection stability issues with ath9k
> running as AP and a few kinds of mobile phones as client, when PS-Poll
> is heavily used
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
Hi Felix,
this commit introduced a regression for me when using Intel Win7
clients on a ath9k AP.
I was not able to track the exact issue down yet :( but it seems to be
related to the Intel
client constantly tearing down the BA session and entering/leaving PS mode.
Any idea?
Thanks,
Helmut
> drivers/net/wireless/ath/ath9k/xmit.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 4f4ce83..f042a18 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -2186,14 +2186,15 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
> txq->stopped = true;
> }
>
> + if (txctl->an)
> + tid = ath_get_skb_tid(sc, txctl->an, skb);
> +
> if (info->flags & IEEE80211_TX_CTL_PS_RESPONSE) {
> ath_txq_unlock(sc, txq);
> txq = sc->tx.uapsdq;
> ath_txq_lock(sc, txq);
> } else if (txctl->an &&
> ieee80211_is_data_present(hdr->frame_control)) {
> - tid = ath_get_skb_tid(sc, txctl->an, skb);
> -
> WARN_ON(tid->ac->txq != txctl->txq);
>
> if (info->flags & IEEE80211_TX_CTL_CLEAR_PS_FILT)
> --
> 1.8.3.4 (Apple Git-47)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3.14] ath9k: fix ps-poll responses under a-mpdu sessions
2014-03-11 9:28 ` Helmut Schaa
@ 2014-03-11 9:36 ` Felix Fietkau
2014-03-11 14:22 ` Helmut Schaa
0 siblings, 1 reply; 6+ messages in thread
From: Felix Fietkau @ 2014-03-11 9:36 UTC (permalink / raw)
To: Helmut Schaa; +Cc: linux-wireless, John Linville
On 2014-03-11 10:28, Helmut Schaa wrote:
> On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <nbd@openwrt.org> wrote:
>> When passing tx frames to the U-APSD queue for powersave poll responses,
>> the ath_atx_tid pointer needs to be passed to ath_tx_setup_buffer for
>> proper sequence number accounting.
>>
>> This fixes high latency and connection stability issues with ath9k
>> running as AP and a few kinds of mobile phones as client, when PS-Poll
>> is heavily used
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>> ---
>
> Hi Felix,
>
> this commit introduced a regression for me when using Intel Win7
> clients on a ath9k AP.
>
> I was not able to track the exact issue down yet :( but it seems to be
> related to the Intel
> client constantly tearing down the BA session and entering/leaving PS mode.
>
> Any idea?
Please make some packet captures and describe more clearly what the
regression is. Do you see connections stalling, big latencies, etc?
- Felix
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3.14] ath9k: fix ps-poll responses under a-mpdu sessions
2014-03-11 9:36 ` Felix Fietkau
@ 2014-03-11 14:22 ` Helmut Schaa
2014-03-11 15:16 ` Felix Fietkau
0 siblings, 1 reply; 6+ messages in thread
From: Helmut Schaa @ 2014-03-11 14:22 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, John Linville
On Tue, Mar 11, 2014 at 10:36 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2014-03-11 10:28, Helmut Schaa wrote:
>> On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> When passing tx frames to the U-APSD queue for powersave poll responses,
>>> the ath_atx_tid pointer needs to be passed to ath_tx_setup_buffer for
>>> proper sequence number accounting.
>>>
>>> This fixes high latency and connection stability issues with ath9k
>>> running as AP and a few kinds of mobile phones as client, when PS-Poll
>>> is heavily used
>>>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>> ---
>>
>> Hi Felix,
>>
>> this commit introduced a regression for me when using Intel Win7
>> clients on a ath9k AP.
>>
>> I was not able to track the exact issue down yet :( but it seems to be
>> related to the Intel
>> client constantly tearing down the BA session and entering/leaving PS mode.
>>
>> Any idea?
> Please make some packet captures and describe more clearly what the
> regression is. Do you see connections stalling, big latencies, etc?
>From what I can see with this patch action frames (like ADDBA and DELBA) get
sequence numbers from TID 0 assigned instead of a seq number from the global
counter. And that seems to "confuse" the client.
The following patch solves the issue for me and seems to still keep
Felix original intention ...
Thoughts?
>From 67282f91e649d946617276b94ee9d48c25fe1521 Mon Sep 17 00:00:00 2001
From: Helmut Schaa <helmut.schaa@googlemail.com>
Date: Tue, 11 Mar 2014 15:11:49 +0100
Subject: [PATCH] ath9k: Fix sequence number assignment for non-data frames
Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
---
drivers/net/wireless/ath/ath9k/xmit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c
b/drivers/net/wireless/ath/ath9k/xmit.c
index fafacfe..bda3432 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2187,7 +2187,8 @@ int ath_tx_start(struct ieee80211_hw *hw, struct
sk_buff *skb,
txq->stopped = true;
}
- if (txctl->an)
+ if (txctl->an &&
+ ieee80211_is_data_present(hdr->frame_control))
tid = ath_get_skb_tid(sc, txctl->an, skb);
if (info->flags & IEEE80211_TX_CTL_PS_RESPONSE) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3.14] ath9k: fix ps-poll responses under a-mpdu sessions
2014-03-11 14:22 ` Helmut Schaa
@ 2014-03-11 15:16 ` Felix Fietkau
2014-03-11 15:37 ` Helmut Schaa
0 siblings, 1 reply; 6+ messages in thread
From: Felix Fietkau @ 2014-03-11 15:16 UTC (permalink / raw)
To: Helmut Schaa; +Cc: linux-wireless, John Linville
On 2014-03-11 15:22, Helmut Schaa wrote:
> On Tue, Mar 11, 2014 at 10:36 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> On 2014-03-11 10:28, Helmut Schaa wrote:
>>> On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <nbd@openwrt.org> wrote:
>>>> When passing tx frames to the U-APSD queue for powersave poll responses,
>>>> the ath_atx_tid pointer needs to be passed to ath_tx_setup_buffer for
>>>> proper sequence number accounting.
>>>>
>>>> This fixes high latency and connection stability issues with ath9k
>>>> running as AP and a few kinds of mobile phones as client, when PS-Poll
>>>> is heavily used
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>>> ---
>>>
>>> Hi Felix,
>>>
>>> this commit introduced a regression for me when using Intel Win7
>>> clients on a ath9k AP.
>>>
>>> I was not able to track the exact issue down yet :( but it seems to be
>>> related to the Intel
>>> client constantly tearing down the BA session and entering/leaving PS mode.
>>>
>>> Any idea?
>> Please make some packet captures and describe more clearly what the
>> regression is. Do you see connections stalling, big latencies, etc?
>
> From what I can see with this patch action frames (like ADDBA and DELBA) get
> sequence numbers from TID 0 assigned instead of a seq number from the global
> counter. And that seems to "confuse" the client.
>
> The following patch solves the issue for me and seems to still keep
> Felix original intention ...
Looks good to me, also because it ensures that action/mgmt frames are
sent out faster, even with filled data queues.
To really make sure that this issue does not happen again, we should
also have a check in ath_tx_setup_buffer where it actually assigns the
seqno.
This will become useful in case we need to buffer non-data packets (e.g.
for things like P2P powersave).
- Felix
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3.14] ath9k: fix ps-poll responses under a-mpdu sessions
2014-03-11 15:16 ` Felix Fietkau
@ 2014-03-11 15:37 ` Helmut Schaa
0 siblings, 0 replies; 6+ messages in thread
From: Helmut Schaa @ 2014-03-11 15:37 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, John Linville
On Tue, Mar 11, 2014 at 4:16 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2014-03-11 15:22, Helmut Schaa wrote:
>> On Tue, Mar 11, 2014 at 10:36 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> On 2014-03-11 10:28, Helmut Schaa wrote:
>>>> On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <nbd@openwrt.org> wrote:
>>>>> When passing tx frames to the U-APSD queue for powersave poll responses,
>>>>> the ath_atx_tid pointer needs to be passed to ath_tx_setup_buffer for
>>>>> proper sequence number accounting.
>>>>>
>>>>> This fixes high latency and connection stability issues with ath9k
>>>>> running as AP and a few kinds of mobile phones as client, when PS-Poll
>>>>> is heavily used
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>>>> ---
>>>>
>>>> Hi Felix,
>>>>
>>>> this commit introduced a regression for me when using Intel Win7
>>>> clients on a ath9k AP.
>>>>
>>>> I was not able to track the exact issue down yet :( but it seems to be
>>>> related to the Intel
>>>> client constantly tearing down the BA session and entering/leaving PS mode.
>>>>
>>>> Any idea?
>>> Please make some packet captures and describe more clearly what the
>>> regression is. Do you see connections stalling, big latencies, etc?
>>
>> From what I can see with this patch action frames (like ADDBA and DELBA) get
>> sequence numbers from TID 0 assigned instead of a seq number from the global
>> counter. And that seems to "confuse" the client.
>>
>> The following patch solves the issue for me and seems to still keep
>> Felix original intention ...
> Looks good to me, also because it ensures that action/mgmt frames are
> sent out faster, even with filled data queues.
> To really make sure that this issue does not happen again, we should
> also have a check in ath_tx_setup_buffer where it actually assigns the
> seqno.
Good point, I'll add that too.
Helmut
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-11 15:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-22 12:48 [PATCH 3.14] ath9k: fix ps-poll responses under a-mpdu sessions Felix Fietkau
2014-03-11 9:28 ` Helmut Schaa
2014-03-11 9:36 ` Felix Fietkau
2014-03-11 14:22 ` Helmut Schaa
2014-03-11 15:16 ` Felix Fietkau
2014-03-11 15:37 ` Helmut Schaa
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).