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

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

[Vivek Natarajan]

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 <vnatarajan@atheros.com>
---
 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

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

[Kalle Valo]

Vivek Natarajan <vnatarajan@atheros.com> 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

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

[Vivek Natarajan]

On Wed, Nov 25, 2009 at 2:07 PM, Kalle Valo <kalle.valo@iki.fi> wrote:
> Vivek Natarajan <vnatarajan@atheros.com> 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.

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]

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 <vnatarajan@atheros.com>
> ---
>  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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]