* [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).