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.
Signed-off-by: Vivek Natarajan <[email protected]>
---
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/mlme.c | 40 ++++++++++++++++++++++++++++++++++++++--
net/mac80211/scan.c | 2 +-
net/mac80211/wext.c | 14 ++++++++++----
4 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ac4779a..d9e3169 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -979,6 +979,9 @@ u64 ieee80211_mandatory_rates(struct ieee80211_local *local,
void ieee80211_ps_enable_work(struct work_struct *work);
void ieee80211_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);
#ifdef CONFIG_MAC80211_NOINLINE
#define debug_noinline noinline
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 437a180..dbc1577 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);
+
+ indexn1 = tim->bitmap_ctrl & 0xfe;
+ indexn2 = elems->tim_len + indexn1 - 4;
+
+ if (index < indexn1 || index > indexn2)
+ return false;
+
+ index -= indexn1;
+
+ if (tim->bitmap_ctrl & 0x01)
+ *is_mc = true;
+
+ return (bool)(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,
msecs_to_jiffies(local->dynamic_ps_timeout));
else {
conf->flags |= IEEE80211_CONF_PS;
+ ieee80211_send_nullfunc(local, sdata, 1);
ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
}
}
@@ -1711,7 +1736,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 */
@@ -1734,6 +1759,15 @@ 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);
+ 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;
@@ -2616,13 +2650,15 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)
void ieee80211_ps_enable_work(struct work_struct *work)
{
- struct ieee80211_local *local = container_of(work,
+ struct ieee80211_local *local = container_of(work,
struct ieee80211_local,
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 9e74e9f..254fb4e 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -985,12 +985,18 @@ set:
local->powersave = ps;
if (ifsta->flags && IEEE80211_STA_ASSOCIATED) {
- 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);
+ }
}
return ret;
}
--
1.6.0.1
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?
johannes
(*) it would need to implement a beacon filter that passes up only
modified beacons (where modified doesn't take into account the TIM IE
except for the own AID/bcast bits), and turns off the null function
stuff in firmware
Vivek Natarajan <[email protected]> writes:
>> One thing I forgot, do we need a special iwlwifi flag to turn this off?
Or a flag to turn it on? Either of them are fine, I guess.
> Kalle added a flag 'IEEE80211_HW_NO_DYNAMIC_PS' in v3 RFC.
> May be, it can be used for iwlwifi to use its own power save and
> null frames as it does now instead of using from mac80211.
> We can check this flag and then change CONF_PS wherever it is used.
I think we should have a separate hw flag for null frames.
--
Kalle Valo
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.
Nice! Looks pretty much good to me, but could use some documentation
(Kalle will hopefully add some too). For instance, a casual observer
might wonder how the hardware can be saving power when the host software
has to parse beacons -- obviously things will only fall into place once
we support beacon miss offload.
> +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);
> +
> + indexn1 = tim->bitmap_ctrl & 0xfe;
> + indexn2 = elems->tim_len + indexn1 - 4;
> +
> + if (index < indexn1 || index > indexn2)
> + return false;
> +
> + index -= indexn1;
> +
> + if (tim->bitmap_ctrl & 0x01)
> + *is_mc = true;
Shouldn't you update *is_mc before the above return false? And also set
it to false in the other case, I think.
> + return (bool)(tim->virtual_map[index] & mask);
I think
return !!(... & mask);
would be more appropriate.
> @@ -1734,6 +1759,15 @@ 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);
>
> + 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);
> + }
> + }
How will we get back into PS mode after
> @@ -2616,13 +2650,15 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)
>
> void ieee80211_ps_enable_work(struct work_struct *work)
> {
> - struct ieee80211_local *local = container_of(work,
> + struct ieee80211_local *local = container_of(work,
> struct ieee80211_local,
> ps_enable_work);
Huh, what's the change here?
> diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
> index 9e74e9f..254fb4e 100644
> --- a/net/mac80211/wext.c
> +++ b/net/mac80211/wext.c
> @@ -985,12 +985,18 @@ set:
> local->powersave = ps;
>
> if (ifsta->flags && IEEE80211_STA_ASSOCIATED) {
> - 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 {
The indentation here looks a bit odd, please put the closing brace
before the else
johannes
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.
>
> Nice! Looks pretty much good to me, but could use some documentation
> (Kalle will hopefully add some too).
I'm adding some documentation in my next round of patches, but we
definitely need more documentation.
> For instance, a casual observer might wonder how the hardware can be
> saving power when the host software has to parse beacons --
Even without beacon filtering and if there is no traffic, we can turn
off the radios for the time between beacons. For example, with beacon
interval 100 ms and dtim 1 the radios might be turned on for only 10
ms per dtim period. So 90% of the time the radios would be turned off
and we would save power. Of course this is very much hardware
specific, but just to give an idea.
> obviously things will only fall into place once we support beacon
> miss offload.
Beacon filtering improves cpu power consumption.
--
Kalle Valo
On Mon, Dec 01, 2008 at 03:53:51AM -0800, Johannes Berg wrote:
> 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.
>
> Nice! Looks pretty much good to me, but could use some documentation
> (Kalle will hopefully add some too). For instance, a casual observer
> might wonder how the hardware can be saving power when the host software
> has to parse beacons -- obviously things will only fall into place once
> we support beacon miss offload.
BTW does broadcom hw support beacon miss stuff? We need to consider
carefully how we add beacon miss support because if we want to rely on
it *and* also disable say the beacon filter we will be missing out on
DFS channel switch announcements and HT channel badwidth changes from
the AP.
Luis
Johannes Berg <[email protected]> wrote:
>> + if (index < indexn1 || index > indexn2)
>> + return false;
>> +
>> + index -= indexn1;
>> +
>> + if (tim->bitmap_ctrl & 0x01)
>> + *is_mc = true;
>
> Shouldn't you update *is_mc before the above return false? And also set
> it to false in the other case, I think.
Yes, you are right.Thanks.
>> + return (bool)(tim->virtual_map[index] & mask);
>
> I think
> return !!(... & mask);
> would be more appropriate.
Thanks. I will modify it.
>> @@ -1734,6 +1759,15 @@ 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);
>>
>> + 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);
>> + }
>> + }
>
> How will we get back into PS mode after
Once we send a null frame with pm bit off and CONF_PS changed,
the tx_idle timer kicks in and we once again move to sleep state
after the timeout. That piece of code is already there in Kalle's patch.
> One thing I forgot, do we need a special iwlwifi flag to turn this off?
Kalle added a flag 'IEEE80211_HW_NO_DYNAMIC_PS' in v3 RFC.
May be, it can be used for iwlwifi to use its own power save and
null frames as it does now instead of using from mac80211.
We can check this flag and then change CONF_PS wherever it is used.
Thanks,
Vivek.
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 :)
--
Kalle Valo