* [mac80211] UAPSD: WLAN_STA_PS flag cleared prematurely?
@ 2012-03-16 9:26 Marco Porsch
2012-03-16 12:44 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Marco Porsch @ 2012-03-16 9:26 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
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?
Regards,
Marco
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [mac80211] UAPSD: WLAN_STA_PS flag cleared prematurely?
2012-03-16 9:26 [mac80211] UAPSD: WLAN_STA_PS flag cleared prematurely? Marco Porsch
@ 2012-03-16 12:44 ` Johannes Berg
2012-03-16 13:38 ` Marco Porsch
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2012-03-16 12:44 UTC (permalink / raw)
To: Marco Porsch; +Cc: linux-wireless
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?
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [mac80211] UAPSD: WLAN_STA_PS flag cleared prematurely?
2012-03-16 12:44 ` Johannes Berg
@ 2012-03-16 13:38 ` Marco Porsch
2012-03-16 13:40 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Marco Porsch @ 2012-03-16 13:38 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
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?
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);
}
}
Open issues:
Which tid?
Would this Null now be sent after or before the last frame?
What if the EOSP frame is lost?
Regards,
Marco
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [mac80211] UAPSD: WLAN_STA_PS flag cleared prematurely?
2012-03-16 13:38 ` Marco Porsch
@ 2012-03-16 13:40 ` Johannes Berg
2012-03-16 13:58 ` Marco Porsch
2012-03-16 14:30 ` [PATCH] mac80211: end service period only after sending last buffered frame Marco Porsch
0 siblings, 2 replies; 6+ messages in thread
From: Johannes Berg @ 2012-03-16 13:40 UTC (permalink / raw)
To: Marco Porsch; +Cc: linux-wireless
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;
}
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [mac80211] UAPSD: WLAN_STA_PS flag cleared prematurely?
2012-03-16 13:40 ` Johannes Berg
@ 2012-03-16 13:58 ` Marco Porsch
2012-03-16 14:30 ` [PATCH] mac80211: end service period only after sending last buffered frame Marco Porsch
1 sibling, 0 replies; 6+ messages in thread
From: Marco Porsch @ 2012-03-16 13:58 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
[-- 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 --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mac80211: end service period only after sending last buffered frame
2012-03-16 13:40 ` Johannes Berg
2012-03-16 13:58 ` Marco Porsch
@ 2012-03-16 14:30 ` Marco Porsch
1 sibling, 0 replies; 6+ messages in thread
From: Marco Porsch @ 2012-03-16 14:30 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, Marco Porsch
Signed-off-by: Marco Porsch <marco.porsch@etit.tu-chemnitz.de>
---
net/mac80211/sta_info.c | 16 +++++++++-------
1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index f468eb1..9b63cd7 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1257,13 +1257,15 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
ieee80211_is_qos_nullfunc(hdr->frame_control))
qoshdr = ieee80211_get_qos_ctl(hdr);
- /* 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;
+ /* end service period after last 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;
+ }
if (qoshdr)
tids |= BIT(*qoshdr & IEEE80211_QOS_CTL_TID_MASK);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-16 14:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-16 9:26 [mac80211] UAPSD: WLAN_STA_PS flag cleared prematurely? Marco Porsch
2012-03-16 12:44 ` Johannes Berg
2012-03-16 13:38 ` Marco Porsch
2012-03-16 13:40 ` Johannes Berg
2012-03-16 13:58 ` Marco Porsch
2012-03-16 14:30 ` [PATCH] mac80211: end service period only after sending last buffered frame Marco Porsch
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).