[PATCH] mac80211: Always send a null data frame if TIM bit is set.

[PATCH] mac80211: Always send a null data frame if TIM bit is set.

[Vivek Natarajan]

If the AP thinks we are in power save state eventhough we are not truly
in that state, it sets the TIM bit and does not send a data frame unless
we send a null data frame to correct the state in the AP.
This might happen if the null data frame for wake up is lost in the air
after we disable power save.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
 net/mac80211/mlme.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 7f23858..ac10b21 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1457,8 +1457,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 	ieee80211_sta_wmm_params(local, ifmgd, elems.wmm_param,
 				 elems.wmm_param_len);
 
-	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK &&
-	    local->hw.conf.flags & IEEE80211_CONF_PS) {
+	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) {
 		directed_tim = ieee80211_check_tim(&elems, ifmgd->aid);
 
 		if (directed_tim) {
-- 
1.6.0.1

Re: [PATCH] mac80211: Always send a null data frame if TIM bit is set.

[Kalle Valo]

Vivek Natarajan <vivek.natraj@gmail.com> writes:

> If the AP thinks we are in power save state eventhough we are not truly
> in that state, it sets the TIM bit and does not send a data frame unless
> we send a null data frame to correct the state in the AP.
> This might happen if the null data frame for wake up is lost in the air
> after we disable power save.

Not a bad idea. Of course the proper fix would be that the stack is
notified when the null frames transmission fails, but we don't have
that yet.

> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -1457,8 +1457,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
>  	ieee80211_sta_wmm_params(local, ifmgd, elems.wmm_param,
>  				 elems.wmm_param_len);
>  
> -	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK &&
> -	    local->hw.conf.flags & IEEE80211_CONF_PS) {
> +	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) {
>  		directed_tim = ieee80211_check_tim(&elems, ifmgd->aid);

Please add a comment why IEEE80211_CONF_PS is not checked.

-- 
Kalle Valo

Re: [PATCH] mac80211: Always send a null data frame if TIM bit is set.

[Vivek Natarajan]

On Mon, Mar 2, 2009 at 8:20 PM, Vivek Natarajan <vivek.natraj@gmail.com=
> wrote:
> If the AP thinks we are in power save state eventhough we are not tru=
ly
> in that state, it sets the TIM bit and does not send a data frame unl=
ess
> we send a null data frame to correct the state in the AP.
> This might happen if the null data frame for wake up is lost in the a=
ir
> after we disable power save.
>
> Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
> ---
> =A0net/mac80211/mlme.c | =A0 =A03 +--
> =A01 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 7f23858..ac10b21 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -1457,8 +1457,7 @@ static void ieee80211_rx_mgmt_beacon(struct iee=
e80211_sub_if_data *sdata,
> =A0 =A0 =A0 =A0ieee80211_sta_wmm_params(local, ifmgd, elems.wmm_param=
,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 elems=
=2Ewmm_param_len);
>
> - =A0 =A0 =A0 if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK &&
> - =A0 =A0 =A0 =A0 =A0 local->hw.conf.flags & IEEE80211_CONF_PS) {
> + =A0 =A0 =A0 if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0directed_tim =3D ieee80211_check_tim(&=
elems, ifmgd->aid);
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (directed_tim) {
	                 if (local->hw.conf.dynamic_ps_timeout > 0) {
				local->hw.conf.flags &=3D ~IEEE80211_CONF_PS;
				ieee80211_hw_config(local,
						    IEEE80211_CONF_CHANGE_PS);
				ieee80211_send_nullfunc(local, sdata, 0);
			} else {
				local->pspolling =3D true;
				ieee80211_send_pspoll(local, sdata);

Although the above patch is a right approach, it opens up an issue
with pspolling.

In the above chunk of code, if the timeout is zero, instead of sending
a null frame to
correct the state in the AP, we may keep on sending ps poll frames (
And the ps state
in the AP will never be corrected)

I have not tested pspolling with ath9k. But this seems to be an issue.
Thoughts?

VIvek.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: Always send a null data frame if TIM bit is set.

[Kalle Valo]

Vivek Natarajan <vivek.natraj@gmail.com> writes:

> Although the above patch is a right approach, it opens up an issue
> with pspolling.
>
> In the above chunk of code, if the timeout is zero, instead of
> sending a null frame to correct the state in the AP, we may keep on
> sending ps poll frames ( And the ps state in the AP will never be
> corrected)
>
> I have not tested pspolling with ath9k. But this seems to be an issue.
> Thoughts?

Maybe it's better to have a separate test for the case that TIM bit is
set but we are not in power save and just send a nullfunc frame
without psm bit. And leave the original power save code as it was.

I really would like to have some frame ack callback to mac80211, even
not all drivers support that. Otherwise we cannot reliably go to power
save because the AP might not receive our nullfunc frame. I have to
think about this and I'll try to propose something later on.

-- 
Kalle Valo

Re: [PATCH] mac80211: Always send a null data frame if TIM bit is set.

[Johannes Berg]

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

On Thu, 2009-04-02 at 08:52 +0300, Kalle Valo wrote:

> I really would like to have some frame ack callback to mac80211, even
> not all drivers support that. Otherwise we cannot reliably go to power
> save because the AP might not receive our nullfunc frame. I have to
> think about this and I'll try to propose something later on.

I suppose we could well assume that we do get TX status for those
devices that do support power save -- which devices would that affect?

johannes

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

Re: [PATCH] mac80211: Always send a null data frame if TIM bit is set.

[Ivo van Doorn]

On Thursday 02 April 2009, Johannes Berg wrote:
> On Thu, 2009-04-02 at 08:52 +0300, Kalle Valo wrote:
> 
> > I really would like to have some frame ack callback to mac80211, even
> > not all drivers support that. Otherwise we cannot reliably go to power
> > save because the AP might not receive our nullfunc frame. I have to
> > think about this and I'll try to propose something later on.
> 
> I suppose we could well assume that we do get TX status for those
> devices that do support power save -- which devices would that affect?

rt2500usb and rt73usb don't provide a valid TX status but do support
power save at he moment. I have to add that I'm in contact with somebody
who is looking into supporting Ack-based TX status reporting in mac80211
which could cure this problem. I have directed him to the linux-wireless list
so he will probably soon send his ideas to the list.

In any case, for now we can assume rt2500usb and rt73usb to be a problem.

Ivo