| Message ID | CAGXE3d-ku6EZsAo6ZksXxU4g9KTANnphmSGz3H_NUMfkaJGnew@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 -- 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
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 -- 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
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);