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: [email protected]
Signed-off-by: Felix Fietkau <[email protected]>
---
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)
On Tue, Mar 11, 2014 at 10:36 AM, Felix Fietkau <[email protected]> wrote:
> On 2014-03-11 10:28, Helmut Schaa wrote:
>> On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <[email protected]> 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: [email protected]
>>> Signed-off-by: Felix Fietkau <[email protected]>
>>> ---
>>
>> 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 <[email protected]>
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 <[email protected]>
---
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
On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <[email protected]> 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: [email protected]
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014-03-11 10:28, Helmut Schaa wrote:
> On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <[email protected]> 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: [email protected]
>> Signed-off-by: Felix Fietkau <[email protected]>
>> ---
>
> 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
On 2014-03-11 15:22, Helmut Schaa wrote:
> On Tue, Mar 11, 2014 at 10:36 AM, Felix Fietkau <[email protected]> wrote:
>> On 2014-03-11 10:28, Helmut Schaa wrote:
>>> On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <[email protected]> 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: [email protected]
>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>> ---
>>>
>>> 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
On Tue, Mar 11, 2014 at 4:16 PM, Felix Fietkau <[email protected]> wrote:
> On 2014-03-11 15:22, Helmut Schaa wrote:
>> On Tue, Mar 11, 2014 at 10:36 AM, Felix Fietkau <[email protected]> wrote:
>>> On 2014-03-11 10:28, Helmut Schaa wrote:
>>>> On Sat, Feb 22, 2014 at 1:48 PM, Felix Fietkau <[email protected]> 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: [email protected]
>>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>>> ---
>>>>
>>>> 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