Re: [mac80211] UAPSD: WLAN_STA_PS flag cleared prematurely?

[Marco Porsch <marco.porsch@etit.tu-chemnitz.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2248 bytes --]

On 03/16/12 14:40, Johannes Berg wrote:
> On Fri, 2012-03-16 at 14:38 +0100, Marco Porsch wrote:
>> On 03/16/12 13:44, Johannes Berg wrote:
>>> On Fri, 2012-03-16 at 10:26 +0100, Marco Porsch wrote:
>>>> Hi all,
>>>>
>>>> in sta_info.c : ieee80211_sta_ps_deliver_response the
>>>> IEEE80211_TX_STATUS_EOSP is set for all to-be-sent frames, not only for
>>>> the last. But only the last buffered frame actually gets the EOSP flag.
>>>>
>>>>
>>>> /* set EOSP for the frame */
>>>> if (reason == IEEE80211_FRAME_RELEASE_UAPSD&&
>>>>        qoshdr&&   skb_queue_empty(&frames))
>>>> 	*qoshdr |= IEEE80211_QOS_CTL_EOSP;
>>>>
>>>> info->flags |= IEEE80211_TX_STATUS_EOSP |
>>>>           IEEE80211_TX_CTL_REQ_TX_STATUS;
>>>>
>>>>
>>>> Consequence is, that the WLAN_STA_SP flag gets cleared (multiple times)
>>>> in ieee80211_tx_status before the last frame with EOSP has been sent.
>>>> Is this correct?
>>>
>>> Looks like the bug is above, the EOSP/TX_STATUS should only be set for
>>> the last frame?
>>
>> I Agree. But what about the case, when the last frame is not a QoS
>> frame? Can this even happen - or is U-APSD only for QoS STA?
>
> uAPSD can only be used by a QoS STA.
>
>> Then we would have to manually append a QoS Null with the EOSP flag +
>> TX_STATUS?
>>
>> So like this:
>>
>> /* set EOSP for the _last_ frame or appended a QoS Null when needed */
>> if (reason == IEEE80211_FRAME_RELEASE_UAPSD&&
>>       skb_queue_empty(&frames)) {
>> 	if (qoshdr) {
>> 		*qoshdr |= IEEE80211_QOS_CTL_EOSP;
>>
>> 		info->flags |= IEEE80211_TX_STATUS_EOSP |
>> 		               IEEE80211_TX_CTL_REQ_TX_STATUS;
>> 	} else {
>> 		ieee80211_send_null_response(sdata, tid, reason);
>> 	}
>> }
>
> No, I think it should be more like this:
>
>
> /* set EOSP for the frame */
>
> if (skb_queue_empty(&frames)) {
> 	if (reason == IEEE80211_FRAME_RELEASE_UAPSD&&  qoshdr)
> 	        *qoshdr |= IEEE80211_QOS_CTL_EOSP;
>
> 	info->flags |= IEEE80211_TX_STATUS_EOSP |
> 			IEEE80211_TX_CTL_REQ_TX_STATUS;
> }

Oh, yes, I see. In case of reason==IEEE80211_FRAME_RELEASE_PSPOLL, it is 
intended to not send the EOSP, but still set IEEE80211_TX_STATUS_EOSP.

Who sends the patch?

Regards,
Marco

[-- Attachment #1.2: marco_porsch.vcf --]
[-- Type: text/x-vcard, Size: 432 bytes --]

begin:vcard
fn:Marco Porsch
n:Porsch;Marco
org:Chemnitz University of Technology;Communication Networks
adr;quoted-printable;quoted-printable:Fakult=C3=A4t f=C3=BCr Elektrotechnik und Informationstechnik , Professur=
	 Kommunikationsnetze;;Technische Universit=C3=A4t Chemnitz;Chemnitz;;D - 09107;Germany
email;internet:marco.porsch@etit.tu-chemnitz.de
title:Dipl.-Ing.
tel;work:+49 371 531 37523
version:2.1
end:vcard


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5088 bytes --]