2015-01-06 17:41:13

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCHv4] mac80211: enable TPC through mac80211 stack

Control per packet Transmit Power Control (TPC) in lower drivers
according to TX power settings configured by the user. In particular TPC is
enabled if value passed in enum nl80211_tx_power_setting is
NL80211_TX_POWER_AUTOMATIC (no limit from userspace) or
NL80211_TX_POWER_LIMITED (allow using less than specified from userspace),
whereas TPC is disabled if nl80211_tx_power_setting is set to
NL80211_TX_POWER_FIXED (use value configured from userspace)

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
include/net/mac80211.h | 8 ++++++++
net/mac80211/cfg.c | 16 +++++++++++++---
net/mac80211/chan.c | 4 ++--
net/mac80211/ieee80211_i.h | 3 ++-
net/mac80211/iface.c | 5 +++--
5 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 4913c00..e47fdbc 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -376,6 +376,13 @@ enum ieee80211_rssi_event {
* @ssid_len: Length of SSID given in @ssid.
* @hidden_ssid: The SSID of the current vif is hidden. Only valid in AP-mode.
* @txpower: TX power in dBm
+ * @txpower_type: TX power adjustment used to control per packet Transmit
+ * Power Control (TPC) in lower driver for the current vif. In particular
+ * TPC is enabled if value passed in %txpower_type is
+ * NL80211_TX_POWER_AUTOMATIC (no limit from userspace) or
+ * NL80211_TX_POWER_LIMITED (allow using less than specified from
+ * userspace), whereas TPC is disabled if %txpower_type is set to
+ * NL80211_TX_POWER_FIXED (use value configured from userspace)
* @p2p_noa_attr: P2P NoA attribute for P2P powersave
*/
struct ieee80211_bss_conf {
@@ -411,6 +418,7 @@ struct ieee80211_bss_conf {
size_t ssid_len;
bool hidden_ssid;
int txpower;
+ enum nl80211_tx_power_setting txpower_type;
struct ieee80211_p2p_noa_attr p2p_noa_attr;
};

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1696658..9c7e1ee 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2103,6 +2103,7 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
{
struct ieee80211_local *local = wiphy_priv(wiphy);
struct ieee80211_sub_if_data *sdata;
+ bool update_txp_type = false;

if (wdev) {
sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
@@ -2119,7 +2120,12 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
break;
}

- ieee80211_recalc_txpower(sdata);
+ if (type != sdata->vif.bss_conf.txpower_type) {
+ update_txp_type = true;
+ sdata->vif.bss_conf.txpower_type = type;
+ }
+
+ ieee80211_recalc_txpower(sdata, update_txp_type);

return 0;
}
@@ -2137,10 +2143,14 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
}

mutex_lock(&local->iflist_mtx);
- list_for_each_entry(sdata, &local->interfaces, list)
+ list_for_each_entry(sdata, &local->interfaces, list) {
sdata->user_power_level = local->user_power_level;
+ if (type != sdata->vif.bss_conf.txpower_type)
+ update_txp_type = true;
+ sdata->vif.bss_conf.txpower_type = type;
+ }
list_for_each_entry(sdata, &local->interfaces, list)
- ieee80211_recalc_txpower(sdata);
+ ieee80211_recalc_txpower(sdata, update_txp_type);
mutex_unlock(&local->iflist_mtx);

return 0;
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 5d6dae9..a881370 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -628,7 +628,7 @@ out:
}

if (new_ctx && ieee80211_chanctx_num_assigned(local, new_ctx) > 0) {
- ieee80211_recalc_txpower(sdata);
+ ieee80211_recalc_txpower(sdata, false);
ieee80211_recalc_chanctx_min_def(local, new_ctx);
}

@@ -1356,7 +1356,7 @@ static int ieee80211_vif_use_reserved_switch(struct ieee80211_local *local)
ieee80211_bss_info_change_notify(sdata,
changed);

- ieee80211_recalc_txpower(sdata);
+ ieee80211_recalc_txpower(sdata, false);
}

ieee80211_recalc_chanctx_chantype(local, ctx);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4f45cab..486dbc5d 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1623,7 +1623,8 @@ int ieee80211_add_virtual_monitor(struct ieee80211_local *local);
void ieee80211_del_virtual_monitor(struct ieee80211_local *local);

bool __ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata);
-void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata);
+void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata,
+ bool update_bss);

static inline bool ieee80211_sdata_running(struct ieee80211_sub_if_data *sdata)
{
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 4173553..bc885e2 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -73,9 +73,10 @@ bool __ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata)
return false;
}

-void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata)
+void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata,
+ bool update_bss)
{
- if (__ieee80211_recalc_txpower(sdata))
+ if (__ieee80211_recalc_txpower(sdata) || update_bss)
ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_TXPOWER);
}

--
2.1.0



2015-01-08 07:48:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: enable TPC through mac80211 stack

On Wed, 2015-01-07 at 15:39 +0100, Lorenzo Bianconi wrote:

> > Does it really make sense to ask the driver to do "automatic"? After
> > all, there are other limits in mac80211 (like 11h from the AP and
> > regulatory) that need to be considered. So perhaps if userspace says
> > "automatic", the driver should still see only "limited"?
> >
>
> IMHO "automatic" means lower driver has to take into account just
> channel/regulatory constraints (that is ath9k behavior), since the
> user/mac80211 does not cap TX power, whereas "limited" means the
> user/mac80211 has set a limit for emitted power, so that configuration
> has to be taken into account.

Well, mac80211 always gives you a max TX power value though - and if
it's only the maximum regulatory permitted on that channel. So there's
always a valid upper bound - unlike the userspace API where "automatic"
means that you have no idea of an upper bound. Inside the kernel you
always have an upper bound, so "automatic" doesn't make that much sense?

johannes


2015-01-08 11:07:12

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: enable TPC through mac80211 stack

> On Wed, 2015-01-07 at 15:39 +0100, Lorenzo Bianconi wrote:
>
>> > Does it really make sense to ask the driver to do "automatic"? After
>> > all, there are other limits in mac80211 (like 11h from the AP and
>> > regulatory) that need to be considered. So perhaps if userspace says
>> > "automatic", the driver should still see only "limited"?
>> >
>>
>> IMHO "automatic" means lower driver has to take into account just
>> channel/regulatory constraints (that is ath9k behavior), since the
>> user/mac80211 does not cap TX power, whereas "limited" means the
>> user/mac80211 has set a limit for emitted power, so that configuration
>> has to be taken into account.
>
> Well, mac80211 always gives you a max TX power value though - and if
> it's only the maximum regulatory permitted on that channel. So there's
> always a valid upper bound - unlike the userspace API where "automatic"
> means that you have no idea of an upper bound. Inside the kernel you
> always have an upper bound, so "automatic" doesn't make that much sense?
>

Sounds good, now I have clearer vision :)
I will overwrite bss_conf nl80211_tx_power_setting value in
ieee80211_set_tx_power() when txpower type is set to
NL80211_TX_POWER_AUTOMATIC from userspace. Moreover should we set
nl80211_tx_power_setting to NL80211_TX_POWER_LIMITED in
ieee80211_handle_pwr_constr() when the station receives a power
constraint in beacon frames? What do you think?

> johannes
>

Regards,
Lorenzo

--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

2015-01-07 14:39:44

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: enable TPC through mac80211 stack

> On Tue, 2015-01-06 at 18:41 +0100, Lorenzo Bianconi wrote:
>
>> + * @txpower_type: TX power adjustment used to control per packet Transmit
>> + * Power Control (TPC) in lower driver for the current vif. In particular
>> + * TPC is enabled if value passed in %txpower_type is
>> + * NL80211_TX_POWER_AUTOMATIC (no limit from userspace) or
>> + * NL80211_TX_POWER_LIMITED (allow using less than specified from
>> + * userspace), whereas TPC is disabled if %txpower_type is set to
>> + * NL80211_TX_POWER_FIXED (use value configured from userspace)
>
> I was going to apply this, but now I have second thoughts.
>
> Does it really make sense to ask the driver to do "automatic"? After
> all, there are other limits in mac80211 (like 11h from the AP and
> regulatory) that need to be considered. So perhaps if userspace says
> "automatic", the driver should still see only "limited"?
>

IMHO "automatic" means lower driver has to take into account just
channel/regulatory constraints (that is ath9k behavior), since the
user/mac80211 does not cap TX power, whereas "limited" means the
user/mac80211 has set a limit for emitted power, so that configuration
has to be taken into account. Maybe we have just left to set
txpower_type to NL80211_TX_POWER_LIMITED for the current vif when TX
power is limited by the AP (802.11h). What do you think? Are there any
other cases when TX power is limited by mac80211 (excluding 802.11h
and when TX power is limited by the user)?

> johannes
>

Regards,
Lorenzo

--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

2015-01-14 08:29:44

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: enable TPC through mac80211 stack

Sorry ...

On Thu, 2015-01-08 at 12:07 +0100, Lorenzo Bianconi wrote:

> Sounds good, now I have clearer vision :)
> I will overwrite bss_conf nl80211_tx_power_setting value in
> ieee80211_set_tx_power() when txpower type is set to
> NL80211_TX_POWER_AUTOMATIC from userspace. Moreover should we set
> nl80211_tx_power_setting to NL80211_TX_POWER_LIMITED in
> ieee80211_handle_pwr_constr() when the station receives a power
> constraint in beacon frames? What do you think?

Isn't it basically always only valid to have LIMITED and FIXED? I don't
see much value in fixed really though, but if the user requested
that ...

johannes


2015-01-07 14:12:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: enable TPC through mac80211 stack

On Tue, 2015-01-06 at 18:41 +0100, Lorenzo Bianconi wrote:

> + * @txpower_type: TX power adjustment used to control per packet Transmit
> + * Power Control (TPC) in lower driver for the current vif. In particular
> + * TPC is enabled if value passed in %txpower_type is
> + * NL80211_TX_POWER_AUTOMATIC (no limit from userspace) or
> + * NL80211_TX_POWER_LIMITED (allow using less than specified from
> + * userspace), whereas TPC is disabled if %txpower_type is set to
> + * NL80211_TX_POWER_FIXED (use value configured from userspace)

I was going to apply this, but now I have second thoughts.

Does it really make sense to ask the driver to do "automatic"? After
all, there are other limits in mac80211 (like 11h from the AP and
regulatory) that need to be considered. So perhaps if userspace says
"automatic", the driver should still see only "limited"?

johannes