2009-11-25 07:05:52

by Vivek Natarajan

[permalink] [raw]
Subject: [RFC PATCH] mac80211: Send null data frame after disabling power save in Tx path.

For drivers setting IEEE80211_HW_PS_NULLFUNC_STACK, a null data frame
with PM bit off has to be sent before sending normal data frames.
If it is done in ps_disable_work, the actual data frame would be
queued first before this work is executed and hence null data frame
will be queued later. And also, this null data frame has to be sent
only after clearing CONF_PS.
Hence, directly clearing CONF_PS and sending null data frame in
ieee80211_xmit seems to function properly.

Signed-off-by: Vivek Natarajan <[email protected]>
---
net/mac80211/tx.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 943def2..51f537c 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1433,10 +1433,10 @@ static void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,

if (need_dynamic_ps(local)) {
if (local->hw.conf.flags & IEEE80211_CONF_PS) {
- ieee80211_stop_queues_by_reason(&local->hw,
- IEEE80211_QUEUE_STOP_REASON_PS);
- ieee80211_queue_work(&local->hw,
- &local->dynamic_ps_disable_work);
+ local->hw.conf.flags &= ~IEEE80211_CONF_PS;
+ ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+ if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
+ ieee80211_send_nullfunc(local, sdata, 0);
}

mod_timer(&local->dynamic_ps_timer, jiffies +
--
1.6.0.4



2009-11-25 08:37:15

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC PATCH] mac80211: Send null data frame after disabling power save in Tx path.

Vivek Natarajan <[email protected]> writes:

> For drivers setting IEEE80211_HW_PS_NULLFUNC_STACK, a null data frame
> with PM bit off has to be sent before sending normal data frames.

Why? Sending a data frame with PM bit off should be enough to inform
AP that the client is awake.

--
Kalle Valo

2009-11-25 09:50:16

by Vivek Natarajan

[permalink] [raw]
Subject: Re: [RFC PATCH] mac80211: Send null data frame after disabling power save in Tx path.

On Wed, Nov 25, 2009 at 2:07 PM, Kalle Valo <[email protected]> wrote:
> Vivek Natarajan <[email protected]> writes:
>
>> For drivers setting IEEE80211_HW_PS_NULLFUNC_STACK, a null data frame
>> with PM bit off has to be sent before sending normal data frames.
>
> Why? Sending a data frame with PM bit off should be enough to inform
> AP that the client is awake.

Since the subtype is different for null function data frame
(IEEE80211_STYPE_NULLFUNC) which is normally used to indicate the
power management status , than normal data frame
(IEEE80211_STYPE_DATA), I thought some APs might create some
functionality issues by checking the frame subtype.
After your mail, I looked at the spec and it does not mention about
null frame in this context. It says,
'STAs changing Power Management mode shall inform the AP of this fact
using the Power Management bits within the Frame Control field of
transmitted frames. '

Accordingly, the present implementation might be correct. Thanks for
pointing it out.

Vivek.

2009-11-25 11:36:16

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC PATCH] mac80211: Send null data frame after disabling power save in Tx path.

On Wed, 2009-11-25 at 12:35 +0530, Vivek Natarajan wrote:
> For drivers setting IEEE80211_HW_PS_NULLFUNC_STACK, a null data frame
> with PM bit off has to be sent before sending normal data frames.
> If it is done in ps_disable_work, the actual data frame would be
> queued first before this work is executed and hence null data frame
> will be queued later. And also, this null data frame has to be sent
> only after clearing CONF_PS.
> Hence, directly clearing CONF_PS and sending null data frame in
> ieee80211_xmit seems to function properly.
>
> Signed-off-by: Vivek Natarajan <[email protected]>
> ---
> net/mac80211/tx.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 943def2..51f537c 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1433,10 +1433,10 @@ static void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,
>
> if (need_dynamic_ps(local)) {
> if (local->hw.conf.flags & IEEE80211_CONF_PS) {
> - ieee80211_stop_queues_by_reason(&local->hw,
> - IEEE80211_QUEUE_STOP_REASON_PS);
> - ieee80211_queue_work(&local->hw,
> - &local->dynamic_ps_disable_work);
> + local->hw.conf.flags &= ~IEEE80211_CONF_PS;
> + ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
> + if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
> + ieee80211_send_nullfunc(local, sdata, 0);

Regardless of Kalle's comment I thought I'd point out that you cannot do
this anyway as _hw_config() must be able to sleep.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part