Re: [RFC PATCHv3 1/2] mac80211: Determine dynamic PS timeout based on ps-qos network latency

[Juuso Oikarinen <juuso.oikarinen@nokia.com>]

On Mon, 2010-04-19 at 16:42 +0200, ext Johannes Berg wrote:
> On Fri, 2010-04-16 at 12:14 +0300, Juuso Oikarinen wrote:
> > Determine the dynamic PS timeout based on the configured ps-qos network
> > latency. For backwards wext compatibility, allow the dynamic PS timeout
> > configured by the cfg80211 to overrule the automatically determined value.
> 
> This seems OK, but I fear that you'll write applications setting the
> pm_qos network latency just to affect this parameter?
> 

Well you have to see where I'm coming from - I must come up with a way
to tune the dynamic ps timeout value from user-space in a way that is
agreeable with others, and that is somewhat future-proof.

That said, obviously the network latency should be tuned as, well, the
expected network latency. In this phase though, there are no other
parameters affected by the network latency, so the result is quite
obvious - your fear will realise itself ;)

> > Signed-off-by: Juuso Oikarinen <juuso.oikarinen@nokia.com>
> > ---
> >  include/net/mac80211.h |    5 ++++-
> >  net/mac80211/cfg.c     |    4 ++--
> >  net/mac80211/main.c    |    2 ++
> >  net/mac80211/mlme.c    |   14 ++++++++++++++
> >  4 files changed, 22 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> > index dcf3c5f..243e4ab 100644
> > --- a/include/net/mac80211.h
> > +++ b/include/net/mac80211.h
> > @@ -661,6 +661,9 @@ enum ieee80211_smps_mode {
> >   * @dynamic_ps_timeout: The dynamic powersave timeout (in ms), see the
> >   *	powersave documentation below. This variable is valid only when
> >   *	the CONF_PS flag is set.
> > + * @dynamic_ps_forced_timeout: The dynamic powersave timeout (in ms) configured
> > + *	by cfg80211 (essentially, wext) If set, this value overrules the value
> > + *	chosen by mac80211 based on ps qos network latency.
> >   *
> >   * @power_level: requested transmit power (in dBm)
> >   *
> > @@ -680,7 +683,7 @@ enum ieee80211_smps_mode {
> >   */
> >  struct ieee80211_conf {
> >  	u32 flags;
> > -	int power_level, dynamic_ps_timeout;
> > +	int power_level, dynamic_ps_timeout, dynamic_ps_forced_timeout;
> >  	int max_sleep_period;
> >  
> >  	u16 listen_interval;
> > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> > index 7dd7cda..9a1a91c 100644
> > --- a/net/mac80211/cfg.c
> > +++ b/net/mac80211/cfg.c
> > @@ -1388,11 +1388,11 @@ static int ieee80211_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
> >  		return -EOPNOTSUPP;
> >  
> >  	if (enabled == sdata->u.mgd.powersave &&
> > -	    timeout == conf->dynamic_ps_timeout)
> > +	    timeout == conf->dynamic_ps_forced_timeout)
> >  		return 0;
> >  
> >  	sdata->u.mgd.powersave = enabled;
> > -	conf->dynamic_ps_timeout = timeout;
> > +	conf->dynamic_ps_forced_timeout = timeout;
> >  
> >  	/* no change, but if automatic follow powersave */
> >  	mutex_lock(&sdata->u.mgd.mtx);
> > diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> > index 4afe851..ebcca0e 100644
> > --- a/net/mac80211/main.c
> > +++ b/net/mac80211/main.c
> > @@ -569,6 +569,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
> >  
> >  	local->hw.conf.listen_interval = local->hw.max_listen_interval;
> >  
> > +	local->hw.conf.dynamic_ps_forced_timeout = -1;
> > +
> >  	result = sta_info_start(local);
> >  	if (result < 0)
> >  		goto fail_sta_info;
> > diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> > index 35d8502..6402997 100644
> > --- a/net/mac80211/mlme.c
> > +++ b/net/mac80211/mlme.c
> > @@ -476,6 +476,7 @@ void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
> >  {
> >  	struct ieee80211_sub_if_data *sdata, *found = NULL;
> >  	int count = 0;
> > +	int timeout;
> >  
> >  	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS)) {
> >  		local->ps_sdata = NULL;
> > @@ -509,6 +510,19 @@ void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
> >  		beaconint_us = ieee80211_tu_to_usec(
> >  					found->vif.bss_conf.beacon_int);
> >  
> > +		timeout = local->hw.conf.dynamic_ps_forced_timeout;
> > +		if (timeout < 0) {
> > +			if (latency <= 50000)
> > +				timeout = 300;
> > +			else if (latency <= 2000000000)
> > +				timeout = 100;
> > +			else if (latency <= 2100000000)
> > +				timeout = 50;
> > +			else
> > +				timeout = 0;
> 
> Is it even possible to set it larger than 2000 seconds?

Good question. I have not yet had the opportunity to test this, so I
can't tell. You could be right. ;)

-Juuso

> johannes
>