This patch adds support to set transmit power setting type and transmit
power level attributes to NL80211_CMD_SET_STATION in order to facilitate
adjusting the transmit power level of a station associated to the AP.
The added attributes allow selection of automatic and limited transmit
power level, with the level defined in mBm format.
Signed-off-by: Ashok Raj Nagarajan <[email protected]>
---
v2:
- refactor the shared code
- "keep what you had" using STATION_PARAM_APPLY_* BIT
- feature flag to let the user know if this feature is supported or not (Johannes)
include/net/cfg80211.h | 6 ++++++
include/uapi/linux/nl80211.h | 15 +++++++++++++
net/wireless/nl80211.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 71 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 814be4b..c989fbd 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -808,6 +808,7 @@ enum station_parameters_apply_mask {
STATION_PARAM_APPLY_UAPSD = BIT(0),
STATION_PARAM_APPLY_CAPABILITY = BIT(1),
STATION_PARAM_APPLY_PLINK_STATE = BIT(2),
+ STATION_PARAM_APPLY_STA_TXPOWER = BIT(3),
};
/**
@@ -849,6 +850,10 @@ enum station_parameters_apply_mask {
* @opmode_notif: operating mode field from Operating Mode Notification
* @opmode_notif_used: information if operating mode field is used
* @support_p2p_ps: information if station supports P2P PS mechanism
+ * @txpwr: tx power (in mBm) to be used for sending data traffic. If tx power
+ * is not provided, the default per-interface tx power setting will be
+ * overriding. Driver should be picking up the lowest tx power, either tx
+ * power per-interface or per-station.
*/
struct station_parameters {
const u8 *supported_rates;
@@ -876,6 +881,7 @@ struct station_parameters {
u8 opmode_notif;
bool opmode_notif_used;
int support_p2p_ps;
+ unsigned int txpwr;
};
/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6b76e3b..de2f72c 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1980,6 +1980,15 @@ enum nl80211_commands {
* @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
* used in various commands/events for specifying the BSSID.
*
+ * @NL80211_ATTR_STA_TX_POWER_SETTING: Transmit power setting type (u32) for
+ * station associated with the AP. See &enum nl80211_tx_power_setting for
+ * possible values.
+ * @NL80211_ATTR_STA_TX_POWER: Transmit power level (u32) in mBm units. This
+ * allows to set Tx power for a station. If this attribute is not included,
+ * the default per-interface tx power setting will be overriding. Driver
+ * should be picking up the lowest tx power, either tx power per-interface
+ * or per-station.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2386,6 +2395,9 @@ enum nl80211_attrs {
NL80211_ATTR_BSSID,
+ NL80211_ATTR_STA_TX_POWER_SETTING,
+ NL80211_ATTR_STA_TX_POWER,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -4697,6 +4709,8 @@ enum nl80211_feature_flags {
* configuration (AP/mesh) with VHT rates.
* @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial Link Setup
* with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
+ * @NL80211_EXT_FEATURE_STA_TX_PWR: This driver supports controlling tx power
+ * to a station.
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4712,6 +4726,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_BEACON_RATE_HT,
NL80211_EXT_FEATURE_BEACON_RATE_VHT,
NL80211_EXT_FEATURE_FILS_STA,
+ NL80211_EXT_FEATURE_STA_TX_PWR,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ef5eff93..ace87de 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -246,6 +246,8 @@ enum nl80211_multicast_groups {
[NL80211_ATTR_STA_SUPPORTED_RATES] = { .type = NLA_BINARY,
.len = NL80211_MAX_SUPP_RATES },
[NL80211_ATTR_STA_PLINK_ACTION] = { .type = NLA_U8 },
+ [NL80211_ATTR_STA_TX_POWER_SETTING] = { .type = NLA_U32 },
+ [NL80211_ATTR_STA_TX_POWER] = { .type = NLA_U32 },
[NL80211_ATTR_STA_VLAN] = { .type = NLA_U32 },
[NL80211_ATTR_MNTR_FLAGS] = { /* NLA_NESTED can't be empty */ },
[NL80211_ATTR_MESH_ID] = { .type = NLA_BINARY,
@@ -4548,6 +4550,8 @@ int cfg80211_check_station_change(struct wiphy *wiphy,
return -EINVAL;
if (params->sta_modify_mask & STATION_PARAM_APPLY_CAPABILITY)
return -EINVAL;
+ if (params->sta_modify_mask & STATION_PARAM_APPLY_STA_TXPOWER)
+ return -EINVAL;
if (params->supported_rates)
return -EINVAL;
if (params->ext_capab || params->ht_capa || params->vht_capa)
@@ -4755,6 +4759,40 @@ static int nl80211_set_station_tdls(struct genl_info *info,
return nl80211_parse_sta_wme(info, params);
}
+static int nl80211_parse_sta_txpower_setting(struct genl_info *info,
+ struct station_parameters *params)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ enum nl80211_tx_power_setting type;
+ int idx;
+
+ if (!rdev->ops->set_tx_power ||
+ !wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_STA_TX_PWR))
+ return -EOPNOTSUPP;
+
+ idx = NL80211_ATTR_STA_TX_POWER_SETTING;
+ type = nla_get_u32(info->attrs[idx]);
+
+ if (!info->attrs[NL80211_ATTR_STA_TX_POWER] &&
+ (type != NL80211_TX_POWER_AUTOMATIC))
+ return -EINVAL;
+
+ if (type != NL80211_TX_POWER_AUTOMATIC) {
+ if (type == NL80211_TX_POWER_LIMITED) {
+ idx = NL80211_ATTR_STA_TX_POWER;
+ params->txpwr = nla_get_u32(info->attrs[idx]);
+ params->sta_modify_mask |=
+ STATION_PARAM_APPLY_STA_TXPOWER;
+ } else {
+ return -EINVAL;
+ }
+ } else {
+ params->txpwr = 0;
+ }
+ return 0;
+}
+
static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
{
struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -4854,6 +4892,12 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
params.local_pm = pm;
}
+ if (info->attrs[NL80211_ATTR_STA_TX_POWER_SETTING]) {
+ err = nl80211_parse_sta_txpower_setting(info, ¶ms);
+ if (err)
+ return err;
+ }
+
/* Include parameters for TDLS peer (will check later) */
err = nl80211_set_station_tdls(info, ¶ms);
if (err)
@@ -4981,6 +5025,12 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
}
+ if (info->attrs[NL80211_ATTR_STA_TX_POWER_SETTING]) {
+ err = nl80211_parse_sta_txpower_setting(info, ¶ms);
+ if (err)
+ return err;
+ }
+
err = nl80211_parse_sta_channel_info(info, ¶ms);
if (err)
return err;
--
1.9.1
On 01/31/2017 10:41 AM, Ashok Raj Nagarajan wrote:
> This patch adds support to set transmit power setting type and transmit
> power level attributes to NL80211_CMD_SET_STATION in order to facilitate
> adjusting the transmit power level of a station associated to the AP.
>
> The added attributes allow selection of automatic and limited transmit
> power level, with the level defined in mBm format.
>
> Signed-off-by: Ashok Raj Nagarajan <[email protected]>
> ---
> v2:
> - refactor the shared code
> - "keep what you had" using STATION_PARAM_APPLY_* BIT
> - feature flag to let the user know if this feature is supported or not (Johannes)
>
> include/net/cfg80211.h | 6 ++++++
> include/uapi/linux/nl80211.h | 15 +++++++++++++
> net/wireless/nl80211.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 71 insertions(+)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 814be4b..c989fbd 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -808,6 +808,7 @@ enum station_parameters_apply_mask {
> STATION_PARAM_APPLY_UAPSD = BIT(0),
> STATION_PARAM_APPLY_CAPABILITY = BIT(1),
> STATION_PARAM_APPLY_PLINK_STATE = BIT(2),
> + STATION_PARAM_APPLY_STA_TXPOWER = BIT(3),
> };
>
> /**
> @@ -849,6 +850,10 @@ enum station_parameters_apply_mask {
> * @opmode_notif: operating mode field from Operating Mode Notification
> * @opmode_notif_used: information if operating mode field is used
> * @support_p2p_ps: information if station supports P2P PS mechanism
> + * @txpwr: tx power (in mBm) to be used for sending data traffic. If tx power
> + * is not provided, the default per-interface tx power setting will be
> + * overriding. Driver should be picking up the lowest tx power, either tx
> + * power per-interface or per-station.
> */
> struct station_parameters {
> const u8 *supported_rates;
> @@ -876,6 +881,7 @@ struct station_parameters {
> u8 opmode_notif;
> bool opmode_notif_used;
> int support_p2p_ps;
> + unsigned int txpwr;
> };
>
> /**
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 6b76e3b..de2f72c 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1980,6 +1980,15 @@ enum nl80211_commands {
> * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
> * used in various commands/events for specifying the BSSID.
> *
> + * @NL80211_ATTR_STA_TX_POWER_SETTING: Transmit power setting type (u32) for
> + * station associated with the AP. See &enum nl80211_tx_power_setting for
> + * possible values.
> + * @NL80211_ATTR_STA_TX_POWER: Transmit power level (u32) in mBm units. This
> + * allows to set Tx power for a station. If this attribute is not included,
> + * the default per-interface tx power setting will be overriding. Driver
> + * should be picking up the lowest tx power, either tx power per-interface
> + * or per-station.
> + *
> * @NUM_NL80211_ATTR: total number of nl80211_attrs available
> * @NL80211_ATTR_MAX: highest attribute number currently defined
> * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2386,6 +2395,9 @@ enum nl80211_attrs {
>
> NL80211_ATTR_BSSID,
>
> + NL80211_ATTR_STA_TX_POWER_SETTING,
> + NL80211_ATTR_STA_TX_POWER,
> +
> /* add attributes here, update the policy in nl80211.c */
>
> __NL80211_ATTR_AFTER_LAST,
> @@ -4697,6 +4709,8 @@ enum nl80211_feature_flags {
> * configuration (AP/mesh) with VHT rates.
> * @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial Link Setup
> * with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
> + * @NL80211_EXT_FEATURE_STA_TX_PWR: This driver supports controlling tx power
> + * to a station.
> *
> * @NUM_NL80211_EXT_FEATURES: number of extended features.
> * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
> @@ -4712,6 +4726,7 @@ enum nl80211_ext_feature_index {
> NL80211_EXT_FEATURE_BEACON_RATE_HT,
> NL80211_EXT_FEATURE_BEACON_RATE_VHT,
> NL80211_EXT_FEATURE_FILS_STA,
> + NL80211_EXT_FEATURE_STA_TX_PWR,
>
> /* add new features before the definition below */
> NUM_NL80211_EXT_FEATURES,
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index ef5eff93..ace87de 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -246,6 +246,8 @@ enum nl80211_multicast_groups {
> [NL80211_ATTR_STA_SUPPORTED_RATES] = { .type = NLA_BINARY,
> .len = NL80211_MAX_SUPP_RATES },
> [NL80211_ATTR_STA_PLINK_ACTION] = { .type = NLA_U8 },
> + [NL80211_ATTR_STA_TX_POWER_SETTING] = { .type = NLA_U32 },
> + [NL80211_ATTR_STA_TX_POWER] = { .type = NLA_U32 },
> [NL80211_ATTR_STA_VLAN] = { .type = NLA_U32 },
> [NL80211_ATTR_MNTR_FLAGS] = { /* NLA_NESTED can't be empty */ },
> [NL80211_ATTR_MESH_ID] = { .type = NLA_BINARY,
> @@ -4548,6 +4550,8 @@ int cfg80211_check_station_change(struct wiphy *wiphy,
> return -EINVAL;
> if (params->sta_modify_mask & STATION_PARAM_APPLY_CAPABILITY)
> return -EINVAL;
> + if (params->sta_modify_mask & STATION_PARAM_APPLY_STA_TXPOWER)
> + return -EINVAL;
Does this mean that we cannot change the tx-power after the station is associated?
Seems like that would be a good limitation to remove if possible!
> if (params->supported_rates)
> return -EINVAL;
> if (params->ext_capab || params->ht_capa || params->vht_capa)
> @@ -4755,6 +4759,40 @@ static int nl80211_set_station_tdls(struct genl_info *info,
> return nl80211_parse_sta_wme(info, params);
> }
>
> +static int nl80211_parse_sta_txpower_setting(struct genl_info *info,
> + struct station_parameters *params)
> +{
> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
> + enum nl80211_tx_power_setting type;
> + int idx;
> +
> + if (!rdev->ops->set_tx_power ||
> + !wiphy_ext_feature_isset(&rdev->wiphy,
> + NL80211_EXT_FEATURE_STA_TX_PWR))
> + return -EOPNOTSUPP;
Maybe always let a user set to default value even if the driver does not
support setting specific values?
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 01/31/2017 10:41 AM, Ashok Raj Nagarajan wrote:
> This patch introduce a new driver callback drv_sta_set_txpwr. This API will
> copy the transmit power value passed from user space and call the driver
> callback to set the tx power for the station.
>
> Signed-off-by: Ashok Raj Nagarajan <[email protected]>
> ---
> include/net/mac80211.h | 6 ++++++
> net/mac80211/cfg.c | 7 +++++++
> net/mac80211/driver-ops.c | 21 +++++++++++++++++++++
> net/mac80211/driver-ops.h | 5 +++++
> net/mac80211/trace.h | 27 +++++++++++++++++++++++++++
> 5 files changed, 66 insertions(+)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 5345d35..e059d5a 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1777,6 +1777,8 @@ struct ieee80211_sta_rates {
> * This is defined by the spec (IEEE 802.11-2012 section 8.3.2.2 NOTE 2).
> * @support_p2p_ps: indicates whether the STA supports P2P PS mechanism or not.
> * @max_rc_amsdu_len: Maximum A-MSDU size in bytes recommended by rate control.
> + * @txpwr: indicates the tx power, in dBm, to be used when sending data frames
> + * to the STA. Value of 0 means, automatic (default) tx power.
Atheros NICs use 1/2 dBm increments internally, so maybe pass down mBm to the driver so you don't
loose the granularity? (Other NICs may potentially have even finer control.)
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
This patch introduce a new driver callback drv_sta_set_txpwr. This API will
copy the transmit power value passed from user space and call the driver
callback to set the tx power for the station.
Signed-off-by: Ashok Raj Nagarajan <[email protected]>
---
include/net/mac80211.h | 6 ++++++
net/mac80211/cfg.c | 7 +++++++
net/mac80211/driver-ops.c | 21 +++++++++++++++++++++
net/mac80211/driver-ops.h | 5 +++++
net/mac80211/trace.h | 27 +++++++++++++++++++++++++++
5 files changed, 66 insertions(+)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 5345d35..e059d5a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1777,6 +1777,8 @@ struct ieee80211_sta_rates {
* This is defined by the spec (IEEE 802.11-2012 section 8.3.2.2 NOTE 2).
* @support_p2p_ps: indicates whether the STA supports P2P PS mechanism or not.
* @max_rc_amsdu_len: Maximum A-MSDU size in bytes recommended by rate control.
+ * @txpwr: indicates the tx power, in dBm, to be used when sending data frames
+ * to the STA. Value of 0 means, automatic (default) tx power.
* @txq: per-TID data TX queues (if driver uses the TXQ abstraction)
*/
struct ieee80211_sta {
@@ -1800,6 +1802,7 @@ struct ieee80211_sta {
u16 max_amsdu_len;
bool support_p2p_ps;
u16 max_rc_amsdu_len;
+ u8 txpwr;
struct ieee80211_txq *txq[IEEE80211_NUM_TIDS];
@@ -3545,6 +3548,9 @@ struct ieee80211_ops {
#endif
void (*sta_notify)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
enum sta_notify_cmd, struct ieee80211_sta *sta);
+ int (*sta_set_txpwr)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta);
int (*sta_state)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
struct ieee80211_sta *sta,
enum ieee80211_sta_state old_state,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index e91e503..f84ada0 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1346,6 +1346,13 @@ static int sta_apply_parameters(struct ieee80211_local *local,
if (params->listen_interval >= 0)
sta->listen_interval = params->listen_interval;
+ if (params->txpwr >= 0) {
+ sta->sta.txpwr = MBM_TO_DBM(params->txpwr);
+ ret = drv_sta_set_txpwr(local, sdata, sta);
+ if (ret)
+ return ret;
+ }
+
if (params->supported_rates) {
ieee80211_parse_bitrates(&sdata->vif.bss_conf.chandef,
sband, params->supported_rates,
diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index bb886e7..839c002 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -138,6 +138,27 @@ int drv_sta_state(struct ieee80211_local *local,
return ret;
}
+__must_check
+int drv_sta_set_txpwr(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct sta_info *sta)
+{
+ int ret = -EOPNOTSUPP;
+
+ might_sleep();
+
+ sdata = get_bss_sdata(sdata);
+ if (!check_sdata_in_driver(sdata))
+ return -EIO;
+
+ trace_drv_sta_set_txpwr(local, sdata, &sta->sta);
+ if (local->ops->sta_set_txpwr)
+ ret = local->ops->sta_set_txpwr(&local->hw, &sdata->vif,
+ &sta->sta);
+ trace_drv_return_int(local, ret);
+ return ret;
+}
+
void drv_sta_rc_update(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata,
struct ieee80211_sta *sta, u32 changed)
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 09f77e4..2b13c39 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -526,6 +526,11 @@ int drv_sta_state(struct ieee80211_local *local,
enum ieee80211_sta_state old_state,
enum ieee80211_sta_state new_state);
+__must_check
+int drv_sta_set_txpwr(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct sta_info *sta);
+
void drv_sta_rc_update(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata,
struct ieee80211_sta *sta, u32 changed);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 92a47af..a264261 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -823,6 +823,33 @@
)
);
+TRACE_EVENT(drv_sta_set_txpwr,
+ TP_PROTO(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_sta *sta),
+
+ TP_ARGS(local, sdata, sta),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ VIF_ENTRY
+ STA_ENTRY
+ __field(u8, txpwr)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ VIF_ASSIGN;
+ STA_ASSIGN;
+ __entry->txpwr = sta->txpwr;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT VIF_PR_FMT STA_PR_FMT " txpwr: %d",
+ LOCAL_PR_ARG, VIF_PR_ARG, STA_PR_ARG, __entry->txpwr
+ )
+);
+
TRACE_EVENT(drv_sta_rc_update,
TP_PROTO(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata,
--
1.9.1
On 2017-02-01 23:06, Ben Greear wrote:
> On 02/01/2017 09:27 AM, Ashok Raj Nagarajan wrote:
>
>>>> +static int nl80211_parse_sta_txpower_setting(struct genl_info
>>>> *info,
>>>> + struct station_parameters *params)
>>>> +{
>>>> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
>>>> + enum nl80211_tx_power_setting type;
>>>> + int idx;
>>>> +
>>>> + if (!rdev->ops->set_tx_power ||
>>>> + !wiphy_ext_feature_isset(&rdev->wiphy,
>>>> + NL80211_EXT_FEATURE_STA_TX_PWR))
>>>> + return -EOPNOTSUPP;
>>>
>>> Maybe always let a user set to default value even if the driver does
>>> not
>>> support setting specific values?
>>>
>>
>> IMHO, having some default value in place of non-valid values may not
>> be the right way.
>
> If a user or user-space script wants to always be able to initialize
> things to default
> values, it would be nice if it did not have to pay specific attention
> to whether the
> NIC supports STA_TX_PWR feature or not. Since a NIC that does not
> support this feature will always
> have sta TX power set to default by definition, that is why I think
> you should let
> the call succeed in that case.
>
I think it would be better/easier to handle the error cases in the
user-space scripts than at the driver here, no? NIC that doesn't support
this feature will set the tx power to the station depending on the rate
algorithm in place. So the same NIC and same station will have different
txpower depending on the environment? On the other hand, how do we
decide what constant (?) default value to be sent to userspace?
Thanks,
Ashok
> Thanks,
> Ben
On 2017-02-01 00:36, Ben Greear wrote:
> On 01/31/2017 10:41 AM, Ashok Raj Nagarajan wrote:
>> This patch adds support to set transmit power setting type and
>> transmit
>> power level attributes to NL80211_CMD_SET_STATION in order to
>> facilitate
>> adjusting the transmit power level of a station associated to the AP.
>>
>> The added attributes allow selection of automatic and limited transmit
>> power level, with the level defined in mBm format.
>>
>> Signed-off-by: Ashok Raj Nagarajan <[email protected]>
>> ---
>> v2:
>> - refactor the shared code
>> - "keep what you had" using STATION_PARAM_APPLY_* BIT
>> - feature flag to let the user know if this feature is supported or
>> not (Johannes)
>>
>> include/net/cfg80211.h | 6 ++++++
>> include/uapi/linux/nl80211.h | 15 +++++++++++++
>> net/wireless/nl80211.c | 50
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 71 insertions(+)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index 814be4b..c989fbd 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -808,6 +808,7 @@ enum station_parameters_apply_mask {
>> STATION_PARAM_APPLY_UAPSD = BIT(0),
>> STATION_PARAM_APPLY_CAPABILITY = BIT(1),
>> STATION_PARAM_APPLY_PLINK_STATE = BIT(2),
>> + STATION_PARAM_APPLY_STA_TXPOWER = BIT(3),
>> };
>>
>> /**
>> @@ -849,6 +850,10 @@ enum station_parameters_apply_mask {
>> * @opmode_notif: operating mode field from Operating Mode
>> Notification
>> * @opmode_notif_used: information if operating mode field is used
>> * @support_p2p_ps: information if station supports P2P PS mechanism
>> + * @txpwr: tx power (in mBm) to be used for sending data traffic. If
>> tx power
>> + * is not provided, the default per-interface tx power setting will
>> be
>> + * overriding. Driver should be picking up the lowest tx power,
>> either tx
>> + * power per-interface or per-station.
>> */
>> struct station_parameters {
>> const u8 *supported_rates;
>> @@ -876,6 +881,7 @@ struct station_parameters {
>> u8 opmode_notif;
>> bool opmode_notif_used;
>> int support_p2p_ps;
>> + unsigned int txpwr;
>> };
>>
>> /**
>> diff --git a/include/uapi/linux/nl80211.h
>> b/include/uapi/linux/nl80211.h
>> index 6b76e3b..de2f72c 100644
>> --- a/include/uapi/linux/nl80211.h
>> +++ b/include/uapi/linux/nl80211.h
>> @@ -1980,6 +1980,15 @@ enum nl80211_commands {
>> * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that
>> %NL80211_ATTR_MAC is also
>> * used in various commands/events for specifying the BSSID.
>> *
>> + * @NL80211_ATTR_STA_TX_POWER_SETTING: Transmit power setting type
>> (u32) for
>> + * station associated with the AP. See &enum nl80211_tx_power_setting
>> for
>> + * possible values.
>> + * @NL80211_ATTR_STA_TX_POWER: Transmit power level (u32) in mBm
>> units. This
>> + * allows to set Tx power for a station. If this attribute is not
>> included,
>> + * the default per-interface tx power setting will be overriding.
>> Driver
>> + * should be picking up the lowest tx power, either tx power
>> per-interface
>> + * or per-station.
>> + *
>> * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>> * @NL80211_ATTR_MAX: highest attribute number currently defined
>> * @__NL80211_ATTR_AFTER_LAST: internal use
>> @@ -2386,6 +2395,9 @@ enum nl80211_attrs {
>>
>> NL80211_ATTR_BSSID,
>>
>> + NL80211_ATTR_STA_TX_POWER_SETTING,
>> + NL80211_ATTR_STA_TX_POWER,
>> +
>> /* add attributes here, update the policy in nl80211.c */
>>
>> __NL80211_ATTR_AFTER_LAST,
>> @@ -4697,6 +4709,8 @@ enum nl80211_feature_flags {
>> * configuration (AP/mesh) with VHT rates.
>> * @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial
>> Link Setup
>> * with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
>> + * @NL80211_EXT_FEATURE_STA_TX_PWR: This driver supports controlling
>> tx power
>> + * to a station.
>> *
>> * @NUM_NL80211_EXT_FEATURES: number of extended features.
>> * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
>> @@ -4712,6 +4726,7 @@ enum nl80211_ext_feature_index {
>> NL80211_EXT_FEATURE_BEACON_RATE_HT,
>> NL80211_EXT_FEATURE_BEACON_RATE_VHT,
>> NL80211_EXT_FEATURE_FILS_STA,
>> + NL80211_EXT_FEATURE_STA_TX_PWR,
>>
>> /* add new features before the definition below */
>> NUM_NL80211_EXT_FEATURES,
>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>> index ef5eff93..ace87de 100644
>> --- a/net/wireless/nl80211.c
>> +++ b/net/wireless/nl80211.c
>> @@ -246,6 +246,8 @@ enum nl80211_multicast_groups {
>> [NL80211_ATTR_STA_SUPPORTED_RATES] = { .type = NLA_BINARY,
>> .len = NL80211_MAX_SUPP_RATES },
>> [NL80211_ATTR_STA_PLINK_ACTION] = { .type = NLA_U8 },
>> + [NL80211_ATTR_STA_TX_POWER_SETTING] = { .type = NLA_U32 },
>> + [NL80211_ATTR_STA_TX_POWER] = { .type = NLA_U32 },
>> [NL80211_ATTR_STA_VLAN] = { .type = NLA_U32 },
>> [NL80211_ATTR_MNTR_FLAGS] = { /* NLA_NESTED can't be empty */ },
>> [NL80211_ATTR_MESH_ID] = { .type = NLA_BINARY,
>> @@ -4548,6 +4550,8 @@ int cfg80211_check_station_change(struct wiphy
>> *wiphy,
>> return -EINVAL;
>> if (params->sta_modify_mask & STATION_PARAM_APPLY_CAPABILITY)
>> return -EINVAL;
>> + if (params->sta_modify_mask & STATION_PARAM_APPLY_STA_TXPOWER)
>> + return -EINVAL;
>
> Does this mean that we cannot change the tx-power after the station is
> associated?
>
> Seems like that would be a good limitation to remove if possible!
>
>
Ben, that is a good catch! We don't want to have that limitation and
would remove in the next version.
>
>> if (params->supported_rates)
>> return -EINVAL;
>> if (params->ext_capab || params->ht_capa || params->vht_capa)
>> @@ -4755,6 +4759,40 @@ static int nl80211_set_station_tdls(struct
>> genl_info *info,
>> return nl80211_parse_sta_wme(info, params);
>> }
>>
>> +static int nl80211_parse_sta_txpower_setting(struct genl_info *info,
>> + struct station_parameters *params)
>> +{
>> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
>> + enum nl80211_tx_power_setting type;
>> + int idx;
>> +
>> + if (!rdev->ops->set_tx_power ||
>> + !wiphy_ext_feature_isset(&rdev->wiphy,
>> + NL80211_EXT_FEATURE_STA_TX_PWR))
>> + return -EOPNOTSUPP;
>
> Maybe always let a user set to default value even if the driver does
> not
> support setting specific values?
>
IMHO, having some default value in place of non-valid values may not be
the right way.
Thanks,
Ashok
> Thanks,
> Ben
On 2017-02-01 00:30, Ben Greear wrote:
> On 01/31/2017 10:41 AM, Ashok Raj Nagarajan wrote:
>> This patch introduce a new driver callback drv_sta_set_txpwr. This API
>> will
>> copy the transmit power value passed from user space and call the
>> driver
>> callback to set the tx power for the station.
>>
>> Signed-off-by: Ashok Raj Nagarajan <[email protected]>
>> ---
>> include/net/mac80211.h | 6 ++++++
>> net/mac80211/cfg.c | 7 +++++++
>> net/mac80211/driver-ops.c | 21 +++++++++++++++++++++
>> net/mac80211/driver-ops.h | 5 +++++
>> net/mac80211/trace.h | 27 +++++++++++++++++++++++++++
>> 5 files changed, 66 insertions(+)
>>
>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>> index 5345d35..e059d5a 100644
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -1777,6 +1777,8 @@ struct ieee80211_sta_rates {
>> * This is defined by the spec (IEEE 802.11-2012 section 8.3.2.2 NOTE
>> 2).
>> * @support_p2p_ps: indicates whether the STA supports P2P PS
>> mechanism or not.
>> * @max_rc_amsdu_len: Maximum A-MSDU size in bytes recommended by
>> rate control.
>> + * @txpwr: indicates the tx power, in dBm, to be used when sending
>> data frames
>> + * to the STA. Value of 0 means, automatic (default) tx power.
>
> Atheros NICs use 1/2 dBm increments internally, so maybe pass down mBm
> to the driver so you don't
> loose the granularity? (Other NICs may potentially have even finer
> control.)
>
ath10k firmware expects values to be in dBm and the calculations
regarding 1/2 dBm is taken care in firmware.
Thanks,
Ashok
> Thanks,
> Ben
On 02/01/2017 09:27 AM, Ashok Raj Nagarajan wrote:
>>> +static int nl80211_parse_sta_txpower_setting(struct genl_info *info,
>>> + struct station_parameters *params)
>>> +{
>>> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
>>> + enum nl80211_tx_power_setting type;
>>> + int idx;
>>> +
>>> + if (!rdev->ops->set_tx_power ||
>>> + !wiphy_ext_feature_isset(&rdev->wiphy,
>>> + NL80211_EXT_FEATURE_STA_TX_PWR))
>>> + return -EOPNOTSUPP;
>>
>> Maybe always let a user set to default value even if the driver does not
>> support setting specific values?
>>
>
> IMHO, having some default value in place of non-valid values may not be the right way.
If a user or user-space script wants to always be able to initialize things to default
values, it would be nice if it did not have to pay specific attention to whether the
NIC supports STA_TX_PWR feature or not. Since a NIC that does not support this feature will always
have sta TX power set to default by definition, that is why I think you should let
the call succeed in that case.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
> You use value '0' to mean set to default values, as far as I can
> tell.
I think you're confusing the internal API and the userspace API - at a
userspace level you have to set NL80211_ATTR_STA_TX_POWER_SETTING to
NL80211_TX_POWER_AUTOMATIC to revert back to defaults, no?
For perfect backwards compatibility we should ignore it if not
supported, but that doesn't really make sense - I think we should
reject it and handle errors elsewhere in the not supported case.
johannes
On 02/01/2017 09:29 AM, Ashok Raj Nagarajan wrote:
> On 2017-02-01 00:30, Ben Greear wrote:
>> On 01/31/2017 10:41 AM, Ashok Raj Nagarajan wrote:
>>> This patch introduce a new driver callback drv_sta_set_txpwr. This API will
>>> copy the transmit power value passed from user space and call the driver
>>> callback to set the tx power for the station.
>>>
>>> Signed-off-by: Ashok Raj Nagarajan <[email protected]>
>>> ---
>>> include/net/mac80211.h | 6 ++++++
>>> net/mac80211/cfg.c | 7 +++++++
>>> net/mac80211/driver-ops.c | 21 +++++++++++++++++++++
>>> net/mac80211/driver-ops.h | 5 +++++
>>> net/mac80211/trace.h | 27 +++++++++++++++++++++++++++
>>> 5 files changed, 66 insertions(+)
>>>
>>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>>> index 5345d35..e059d5a 100644
>>> --- a/include/net/mac80211.h
>>> +++ b/include/net/mac80211.h
>>> @@ -1777,6 +1777,8 @@ struct ieee80211_sta_rates {
>>> * This is defined by the spec (IEEE 802.11-2012 section 8.3.2.2 NOTE 2).
>>> * @support_p2p_ps: indicates whether the STA supports P2P PS mechanism or not.
>>> * @max_rc_amsdu_len: Maximum A-MSDU size in bytes recommended by rate control.
>>> + * @txpwr: indicates the tx power, in dBm, to be used when sending data frames
>>> + * to the STA. Value of 0 means, automatic (default) tx power.
>>
>> Atheros NICs use 1/2 dBm increments internally, so maybe pass down mBm
>> to the driver so you don't
>> loose the granularity? (Other NICs may potentially have even finer control.)
>>
>
> ath10k firmware expects values to be in dBm and the calculations regarding 1/2 dBm is taken care in firmware.
Other drivers may want more precision, so I still think you should store this in mBm instead
of throwing away precision in the API.
A new and improved ath10k firmware may support 1/2 dBm units directly, for instance.
Thanks,
Ben
>
> Thanks,
> Ashok
>> Thanks,
>> Ben
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 2017-02-01 23:02, Ben Greear wrote:
> On 02/01/2017 09:29 AM, Ashok Raj Nagarajan wrote:
>> On 2017-02-01 00:30, Ben Greear wrote:
>>> On 01/31/2017 10:41 AM, Ashok Raj Nagarajan wrote:
>>>> This patch introduce a new driver callback drv_sta_set_txpwr. This
>>>> API will
>>>> copy the transmit power value passed from user space and call the
>>>> driver
>>>> callback to set the tx power for the station.
>>>>
>>>> Signed-off-by: Ashok Raj Nagarajan <[email protected]>
>>>> ---
>>>> include/net/mac80211.h | 6 ++++++
>>>> net/mac80211/cfg.c | 7 +++++++
>>>> net/mac80211/driver-ops.c | 21 +++++++++++++++++++++
>>>> net/mac80211/driver-ops.h | 5 +++++
>>>> net/mac80211/trace.h | 27 +++++++++++++++++++++++++++
>>>> 5 files changed, 66 insertions(+)
>>>>
>>>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>>>> index 5345d35..e059d5a 100644
>>>> --- a/include/net/mac80211.h
>>>> +++ b/include/net/mac80211.h
>>>> @@ -1777,6 +1777,8 @@ struct ieee80211_sta_rates {
>>>> * This is defined by the spec (IEEE 802.11-2012 section 8.3.2.2
>>>> NOTE 2).
>>>> * @support_p2p_ps: indicates whether the STA supports P2P PS
>>>> mechanism or not.
>>>> * @max_rc_amsdu_len: Maximum A-MSDU size in bytes recommended by
>>>> rate control.
>>>> + * @txpwr: indicates the tx power, in dBm, to be used when sending
>>>> data frames
>>>> + * to the STA. Value of 0 means, automatic (default) tx power.
>>>
>>> Atheros NICs use 1/2 dBm increments internally, so maybe pass down
>>> mBm
>>> to the driver so you don't
>>> loose the granularity? (Other NICs may potentially have even finer
>>> control.)
>>>
>>
>> ath10k firmware expects values to be in dBm and the calculations
>> regarding 1/2 dBm is taken care in firmware.
>
> Other drivers may want more precision, so I still think you should
> store this in mBm instead
> of throwing away precision in the API.
>
Okay point taken! Instead of having the conversion happening now in API,
I will push it down to the ath10k driver.
> A new and improved ath10k firmware may support 1/2 dBm units directly,
> for instance.
>
> Thanks,
> Ben
>
>>
>> Thanks,
>> Ashok
>>> Thanks,
>>> Ben
>>
On 02/01/2017 09:57 AM, Ashok Raj Nagarajan wrote:
> On 2017-02-01 23:06, Ben Greear wrote:
>> On 02/01/2017 09:27 AM, Ashok Raj Nagarajan wrote:
>>
>>>>> +static int nl80211_parse_sta_txpower_setting(struct genl_info *info,
>>>>> + struct station_parameters *params)
>>>>> +{
>>>>> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
>>>>> + enum nl80211_tx_power_setting type;
>>>>> + int idx;
>>>>> +
>>>>> + if (!rdev->ops->set_tx_power ||
>>>>> + !wiphy_ext_feature_isset(&rdev->wiphy,
>>>>> + NL80211_EXT_FEATURE_STA_TX_PWR))
>>>>> + return -EOPNOTSUPP;
>>>>
>>>> Maybe always let a user set to default value even if the driver does not
>>>> support setting specific values?
>>>>
>>>
>>> IMHO, having some default value in place of non-valid values may not be the right way.
>>
>> If a user or user-space script wants to always be able to initialize
>> things to default
>> values, it would be nice if it did not have to pay specific attention
>> to whether the
>> NIC supports STA_TX_PWR feature or not. Since a NIC that does not
>> support this feature will always
>> have sta TX power set to default by definition, that is why I think
>> you should let
>> the call succeed in that case.
>>
>
> I think it would be better/easier to handle the error cases in the user-space scripts than at the driver here, no? NIC that doesn't support this feature will
> set the tx power to the station depending on the rate algorithm in place. So the same NIC and same station will have different txpower depending on the
> environment? On the other hand, how do we decide what constant (?) default value to be sent to userspace?
You use value '0' to mean set to default values, as far as I can tell.
So, always let a user set the value to 0, regardless of whether STA_RX_PWR feature exists or not.
If you are querying for a value to show user-space, return '0' in this case.
If user tries to set the value to non-zero, then it should fail with EOPNOTSUPP in case STA_TX_PWR
feature does not exist.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com