This patch enables mac80211 to send a null frame and also to
check for tim in the beacon if power save is enabled.
Signed-off-by: Vivek Natarajan <[email protected]>
---
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/mlme.c | 41 ++++++++++++++++++++++++++++++++++++++++-
net/mac80211/scan.c | 2 +-
net/mac80211/wext.c | 13 +++++++++----
4 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f3eec98..2038498 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -986,6 +986,9 @@ u64 ieee80211_mandatory_rates(struct ieee80211_local *local,
void ieee80211_dynamic_ps_enable_work(struct work_struct *work);
void ieee80211_dynamic_ps_disable_work(struct work_struct *work);
void ieee80211_dynamic_ps_timer(unsigned long data);
+void ieee80211_send_nullfunc(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ int powersave);
void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
enum queue_stop_reason reason);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b23e62b..ca22718 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -568,6 +568,30 @@ static void ieee80211_sta_wmm_params(struct ieee80211_local *local,
}
}
+static bool check_tim(struct ieee802_11_elems *elems, u16 aid, bool *is_mc)
+{
+ u8 mask;
+ u8 index, indexn1, indexn2;
+ struct ieee80211_tim_ie *tim = (struct ieee80211_tim_ie *) elems->tim;
+
+ aid &= 0x3fff;
+ index = aid / 8;
+ mask = 1 << (aid & 7);
+
+ if (tim->bitmap_ctrl & 0x01)
+ *is_mc = true;
+
+ indexn1 = tim->bitmap_ctrl & 0xfe;
+ indexn2 = elems->tim_len + indexn1 - 4;
+
+ if (index < indexn1 || index > indexn2)
+ return false;
+
+ index -= indexn1;
+
+ return !!(tim->virtual_map[index] & mask);
+}
+
static u32 ieee80211_handle_bss_capability(struct ieee80211_sub_if_data *sdata,
u16 capab, bool erp_valid, u8 erp)
{
@@ -752,6 +776,7 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
mod_timer(&local->dynamic_ps_timer, jiffies +
msecs_to_jiffies(local->dynamic_ps_timeout));
else {
+ ieee80211_send_nullfunc(local, sdata, 1);
conf->flags |= IEEE80211_CONF_PS;
ieee80211_hw_config(local,
IEEE80211_CONF_CHANGE_PS);
@@ -1729,7 +1754,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
struct ieee802_11_elems elems;
struct ieee80211_local *local = sdata->local;
u32 changed = 0;
- bool erp_valid;
+ bool erp_valid, directed_tim, is_mc = false;
u8 erp_value = 0;
/* Process beacon from the current BSS */
@@ -1752,6 +1777,18 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
ieee80211_sta_wmm_params(local, ifsta, elems.wmm_param,
elems.wmm_param_len);
+ if (!(local->hw.flags & IEEE80211_HW_NO_STACK_DYNAMIC_PS)) {
+ directed_tim = check_tim(&elems, ifsta->aid, &is_mc);
+
+ if (directed_tim || is_mc) {
+ if (local->hw.conf.flags && IEEE80211_CONF_PS) {
+ local->hw.conf.flags &= ~IEEE80211_CONF_PS;
+ ieee80211_hw_config(local,
+ IEEE80211_CONF_CHANGE_PS);
+ ieee80211_send_nullfunc(local, sdata, 0);
+ }
+ }
+ }
if (elems.erp_info && elems.erp_info_len >= 1) {
erp_valid = true;
@@ -2651,10 +2688,12 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
struct ieee80211_local *local =
container_of(work, struct ieee80211_local,
dynamic_ps_enable_work);
+ struct ieee80211_sub_if_data *sdata = local->scan_sdata;
if (local->hw.conf.flags & IEEE80211_CONF_PS)
return;
+ ieee80211_send_nullfunc(local, sdata, 1);
local->hw.conf.flags |= IEEE80211_CONF_PS;
ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index f5c7c33..a2caeed 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -395,7 +395,7 @@ ieee80211_scan_rx(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb,
return RX_QUEUED;
}
-static void ieee80211_send_nullfunc(struct ieee80211_local *local,
+void ieee80211_send_nullfunc(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata,
int powersave)
{
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index 673c5d7..6470614 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -871,12 +871,17 @@ set:
mod_timer(&local->dynamic_ps_timer, jiffies +
msecs_to_jiffies(local->dynamic_ps_timeout));
else {
- if (local->powersave)
+ if (local->powersave) {
+ ieee80211_send_nullfunc(local, sdata, 1);
conf->flags |= IEEE80211_CONF_PS;
- else
+ ret = ieee80211_hw_config(local,
+ IEEE80211_CONF_CHANGE_PS);
+ } else {
conf->flags &= ~IEEE80211_CONF_PS;
- ret = ieee80211_hw_config(local,
- IEEE80211_CONF_CHANGE_PS);
+ ret = ieee80211_hw_config(local,
+ IEEE80211_CONF_CHANGE_PS);
+ ieee80211_send_nullfunc(local, sdata, 0);
+ }
}
}
--
1.6.0.1
On Mon, Dec 22, 2008 at 10:42 PM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2008-12-22 at 20:44 -0800, Vivek Natarajan wrote:
>> This patch enables mac80211 to send a null frame and also to
>> check for tim in the beacon if power save is enabled.
>>
>> Signed-off-by: Vivek Natarajan <[email protected]>
>
> Looks good to me, to be sure, this is all dependent on !NO_STACK_PS,
> right?
Yes, it all depends on that flag. I will send these RFCs as patches.
Thanks,
Vivek.
On Tue, Dec 2, 2008 at 2:58 PM, Vivek Natarajan <[email protected]> wrote:
> Tomas Winkler <[email protected]> wrote:
>> On Mon, Dec 1, 2008 at 6:17 PM, Kalle Valo <[email protected]> wrote:
>>> Johannes Berg <[email protected]> writes:
>>>
>>>> On Mon, 2008-12-01 at 17:17 -0800, Vivek Natarajan wrote:
>>>>> This patch is based on Kalle's initial RFC patches on dynamic power save.
>>>>> Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null
>>>>> frame, it is appropriate to do it from mac80211.
>>>>> This patch enables mac80211 to send a null frame and also to
>>>>> check for tim in the beacon if power save is enabled.
>>>>
>>>> One thing I forgot, do we need a special iwlwifi flag to turn this off?
>>>> Or can iwlwifi be programmed to let the host handle this (*), which
>>>> might be beneficial for interoperability and uniformity purposes?
>>>
>>> Having the null frame handling in firmware is faster and we might get
>>> some power savings from that. But is the saving observable, that's an
>>> another question :)
>>
>> This patch is not much relevant in actual saving , since this is the
>> waking up part, you are powering the whole nic up anyway.
>
> I agree but with PM bit on-off, this is what one can do to reduce some power
> consumption although not significant. If more significant savings is needed,
> we may need to implement PS-POLL or the APSD mechanism. But this
> is a good start to power save in mac80211.
I was more referring to Kalle comment whether more power is saved when
host or firmware sends the null packet,
then to actual implementation of PM.
PM be done through either toggling PM bit or PS-POLL. I'm not sure
which one is better. In PS-POLL the station has more timing control
but there is additional overhead since for each packet PS-POLL frame
has to be issued.
Simplified explanation to APSD would be this as APSD is fine grained
PM per AC category, if I'm not confusing with something else :)
>
>> The firmware handling of power save is more relevant in going to sleep
>> phase and sustaining where decision about sending the null frame is
>> faster then from host. The NIC is powered down in stages up to state
>> where only slow clock is ticking so this is rather time sensitive
>
> AFAIK, only iwlwifi has this firmware support and we need mac80211's
> support in this for ath9k/ath5k,stlc45xx and b43. I believe a powerful
> 2GHz host processor can manage these timings as a Windows driver
> for these vendor cards seems to support power save without firmware support.
I think I think what you meant is that only iwlwifi issues null
packets from the firmware but I believe that all other NICs supports
PM in HW/firmware a.k.a shutting down some parts of the HW.
So to be good also to all NICs null packet shell not be sent for PM
for iwlwifi driver. In good case it's just redundant in worst it can
screw up the PM state machine. I suggest to add flag that indicates
whether null packet is required or not.
Thanks
Tomas
Vivek Natarajan <[email protected]> writes:
> This patch is based on Kalle's initial RFC patches on dynamic power save.
> Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null
> frame, it is appropriate to do it from mac80211.
Yes, I agree. Actually the way I implemented this in stlc45xx was to
use PS-Poll. In future, it would be nice to have it supported as well,
but we can add it later.
> + directed_tim = check_tim(&elems, ifsta->aid, &is_mc);
> +
> + if (directed_tim || is_mc) {
> + if (local->hw.conf.flags && IEEE80211_CONF_PS) {
> + local->hw.conf.flags &= ~IEEE80211_CONF_PS;
> + ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
> + ieee80211_send_nullfunc(local, sdata, 0);
> + }
> + }
Maybe I have been working too much with slow embedded hardware, but can
ath5k really wake up for the beacon, send the beacon to the host,
mac80211 read the beacon and then wakeup the hardware to listen for
multicast frames following the beacon? The multicast frames are sent
immeaditely after the dtim beacon, so this should happen really quick.
But most probably I'm just missing something here.
In stlc45xx the firmware listens automatically for the multicast
beacons following dtim beacons and mac80211 doesn't have to do
anything for them.
> - ret = ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
> + ret = ieee80211_hw_config(local,
> + IEEE80211_CONF_CHANGE_PS);
> + ieee80211_send_nullfunc(local, sdata, 0);
Maybe we should have a seperate function for enabling and disabling
power save mode. So that we don't forget sending of the null frame.
--
Kalle Valo
On Mon, 2008-12-22 at 20:44 -0800, Vivek Natarajan wrote:
> This patch enables mac80211 to send a null frame and also to
> check for tim in the beacon if power save is enabled.
>
> Signed-off-by: Vivek Natarajan <[email protected]>
Looks good to me, to be sure, this is all dependent on !NO_STACK_PS,
right?
johannes
> ---
> net/mac80211/ieee80211_i.h | 3 +++
> net/mac80211/mlme.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> net/mac80211/scan.c | 2 +-
> net/mac80211/wext.c | 13 +++++++++----
> 4 files changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index f3eec98..2038498 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -986,6 +986,9 @@ u64 ieee80211_mandatory_rates(struct ieee80211_local *local,
> void ieee80211_dynamic_ps_enable_work(struct work_struct *work);
> void ieee80211_dynamic_ps_disable_work(struct work_struct *work);
> void ieee80211_dynamic_ps_timer(unsigned long data);
> +void ieee80211_send_nullfunc(struct ieee80211_local *local,
> + struct ieee80211_sub_if_data *sdata,
> + int powersave);
>
> void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
> enum queue_stop_reason reason);
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index b23e62b..ca22718 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -568,6 +568,30 @@ static void ieee80211_sta_wmm_params(struct ieee80211_local *local,
> }
> }
>
> +static bool check_tim(struct ieee802_11_elems *elems, u16 aid, bool *is_mc)
> +{
> + u8 mask;
> + u8 index, indexn1, indexn2;
> + struct ieee80211_tim_ie *tim = (struct ieee80211_tim_ie *) elems->tim;
> +
> + aid &= 0x3fff;
> + index = aid / 8;
> + mask = 1 << (aid & 7);
> +
> + if (tim->bitmap_ctrl & 0x01)
> + *is_mc = true;
> +
> + indexn1 = tim->bitmap_ctrl & 0xfe;
> + indexn2 = elems->tim_len + indexn1 - 4;
> +
> + if (index < indexn1 || index > indexn2)
> + return false;
> +
> + index -= indexn1;
> +
> + return !!(tim->virtual_map[index] & mask);
> +}
> +
> static u32 ieee80211_handle_bss_capability(struct ieee80211_sub_if_data *sdata,
> u16 capab, bool erp_valid, u8 erp)
> {
> @@ -752,6 +776,7 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
> mod_timer(&local->dynamic_ps_timer, jiffies +
> msecs_to_jiffies(local->dynamic_ps_timeout));
> else {
> + ieee80211_send_nullfunc(local, sdata, 1);
> conf->flags |= IEEE80211_CONF_PS;
> ieee80211_hw_config(local,
> IEEE80211_CONF_CHANGE_PS);
> @@ -1729,7 +1754,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
> struct ieee802_11_elems elems;
> struct ieee80211_local *local = sdata->local;
> u32 changed = 0;
> - bool erp_valid;
> + bool erp_valid, directed_tim, is_mc = false;
> u8 erp_value = 0;
>
> /* Process beacon from the current BSS */
> @@ -1752,6 +1777,18 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
> ieee80211_sta_wmm_params(local, ifsta, elems.wmm_param,
> elems.wmm_param_len);
>
> + if (!(local->hw.flags & IEEE80211_HW_NO_STACK_DYNAMIC_PS)) {
> + directed_tim = check_tim(&elems, ifsta->aid, &is_mc);
> +
> + if (directed_tim || is_mc) {
> + if (local->hw.conf.flags && IEEE80211_CONF_PS) {
> + local->hw.conf.flags &= ~IEEE80211_CONF_PS;
> + ieee80211_hw_config(local,
> + IEEE80211_CONF_CHANGE_PS);
> + ieee80211_send_nullfunc(local, sdata, 0);
> + }
> + }
> + }
>
> if (elems.erp_info && elems.erp_info_len >= 1) {
> erp_valid = true;
> @@ -2651,10 +2688,12 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
> struct ieee80211_local *local =
> container_of(work, struct ieee80211_local,
> dynamic_ps_enable_work);
> + struct ieee80211_sub_if_data *sdata = local->scan_sdata;
>
> if (local->hw.conf.flags & IEEE80211_CONF_PS)
> return;
>
> + ieee80211_send_nullfunc(local, sdata, 1);
> local->hw.conf.flags |= IEEE80211_CONF_PS;
>
> ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
> diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
> index f5c7c33..a2caeed 100644
> --- a/net/mac80211/scan.c
> +++ b/net/mac80211/scan.c
> @@ -395,7 +395,7 @@ ieee80211_scan_rx(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb,
> return RX_QUEUED;
> }
>
> -static void ieee80211_send_nullfunc(struct ieee80211_local *local,
> +void ieee80211_send_nullfunc(struct ieee80211_local *local,
> struct ieee80211_sub_if_data *sdata,
> int powersave)
> {
> diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
> index 673c5d7..6470614 100644
> --- a/net/mac80211/wext.c
> +++ b/net/mac80211/wext.c
> @@ -871,12 +871,17 @@ set:
> mod_timer(&local->dynamic_ps_timer, jiffies +
> msecs_to_jiffies(local->dynamic_ps_timeout));
> else {
> - if (local->powersave)
> + if (local->powersave) {
> + ieee80211_send_nullfunc(local, sdata, 1);
> conf->flags |= IEEE80211_CONF_PS;
> - else
> + ret = ieee80211_hw_config(local,
> + IEEE80211_CONF_CHANGE_PS);
> + } else {
> conf->flags &= ~IEEE80211_CONF_PS;
> - ret = ieee80211_hw_config(local,
> - IEEE80211_CONF_CHANGE_PS);
> + ret = ieee80211_hw_config(local,
> + IEEE80211_CONF_CHANGE_PS);
> + ieee80211_send_nullfunc(local, sdata, 0);
> + }
> }
> }
>
Vivek Natarajan <[email protected]> writes:
> This patch enables mac80211 to send a null frame and also to
> check for tim in the beacon if power save is enabled.
I would like to see a separate hw flag for disabling this feature. Not
all drivers need this, for example iwlwifi.
> diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
> index 673c5d7..6470614 100644
> --- a/net/mac80211/wext.c
> +++ b/net/mac80211/wext.c
> @@ -871,12 +871,17 @@ set:
> mod_timer(&local->dynamic_ps_timer, jiffies +
> msecs_to_jiffies(local->dynamic_ps_timeout));
> else {
> - if (local->powersave)
> + if (local->powersave) {
> + ieee80211_send_nullfunc(local, sdata, 1);
> conf->flags |= IEEE80211_CONF_PS;
> - else
> + ret = ieee80211_hw_config(local,
> + IEEE80211_CONF_CHANGE_PS);
> + } else {
> conf->flags &= ~IEEE80211_CONF_PS;
> - ret = ieee80211_hw_config(local,
> - IEEE80211_CONF_CHANGE_PS);
> + ret = ieee80211_hw_config(local,
> + IEEE80211_CONF_CHANGE_PS);
> + ieee80211_send_nullfunc(local, sdata, 0);
Maybe add a helper function for checking the new flag I proposed and
sending the null frame?
--
Kalle Valo
Tomas Winkler <[email protected]> wrote:
> On Mon, Dec 1, 2008 at 6:17 PM, Kalle Valo <[email protected]> wrote:
>> Johannes Berg <[email protected]> writes:
>>
>>> On Mon, 2008-12-01 at 17:17 -0800, Vivek Natarajan wrote:
>>>> This patch is based on Kalle's initial RFC patches on dynamic power save.
>>>> Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null
>>>> frame, it is appropriate to do it from mac80211.
>>>> This patch enables mac80211 to send a null frame and also to
>>>> check for tim in the beacon if power save is enabled.
>>>
>>> One thing I forgot, do we need a special iwlwifi flag to turn this off?
>>> Or can iwlwifi be programmed to let the host handle this (*), which
>>> might be beneficial for interoperability and uniformity purposes?
>>
>> Having the null frame handling in firmware is faster and we might get
>> some power savings from that. But is the saving observable, that's an
>> another question :)
>
> This patch is not much relevant in actual saving , since this is the
> waking up part, you are powering the whole nic up anyway.
I agree but with PM bit on-off, this is what one can do to reduce some power
consumption although not significant. If more significant savings is needed,
we may need to implement PS-POLL or the APSD mechanism. But this
is a good start to power save in mac80211.
> The firmware handling of power save is more relevant in going to sleep
> phase and sustaining where decision about sending the null frame is
> faster then from host. The NIC is powered down in stages up to state
> where only slow clock is ticking so this is rather time sensitive
AFAIK, only iwlwifi has this firmware support and we need mac80211's
support in this for ath9k/ath5k,stlc45xx and b43. I believe a powerful
2GHz host processor can manage these timings as a Windows driver
for these vendor cards seems to support power save without firmware support.
--
Vivek.
On Mon, Dec 1, 2008 at 6:17 PM, Kalle Valo <[email protected]> wrote:
> Johannes Berg <[email protected]> writes:
>
>> On Mon, 2008-12-01 at 17:17 -0800, Vivek Natarajan wrote:
>>> This patch is based on Kalle's initial RFC patches on dynamic power save.
>>> Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null
>>> frame, it is appropriate to do it from mac80211.
>>> This patch enables mac80211 to send a null frame and also to
>>> check for tim in the beacon if power save is enabled.
>>
>> One thing I forgot, do we need a special iwlwifi flag to turn this off?
>> Or can iwlwifi be programmed to let the host handle this (*), which
>> might be beneficial for interoperability and uniformity purposes?
>
> Having the null frame handling in firmware is faster and we might get
> some power savings from that. But is the saving observable, that's an
> another question :)
This patch is not much relevant in actual saving , since this is the
waking up part, you are powering the whole nic up anyway.But sending 2
null packets instead of one doesn't really help power save either.
If you want numbers you can measure power consumption on power lines
of the chip, simple loop and power probe with scope will do the the
trick.
The firmware handling of power save is more relevant in going to sleep
phase and sustaining where decision about sending the null frame is
faster then from host. The NIC is powered down in stages up to state
where only slow clock is ticking so this is rather time sensitive
Hope this helps
Tomas