2020-01-13 13:12:24

by Tamizh chelvam

[permalink] [raw]
Subject: [PATCHv9 5/6] nl80211: Add support to configure TID specific txrate configuration

From: Tamizh chelvam <[email protected]>

This patch adds support to configure per TID txrate configuration
configuration through the NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE
and NL80211_TID_CONFIG_ATTR_TX_RATE
attribute. TX bitrate mask values passed
in NL80211_ATTR_TX_RATES attribute and NL80211_TID_CONFIG_ATTR_TX_RATES
attribute will have types of the TX rate should be applied. This uses
nl80211_parse_tx_bitrate_mask to validate and calculate the bitrate
mask.

Signed-off-by: Tamizh chelvam <[email protected]>
---
include/net/cfg80211.h | 5 +++
include/uapi/linux/nl80211.h | 24 +++++++++++++
net/wireless/nl80211.c | 76 ++++++++++++++++++++++++++++++++----------
3 files changed, 88 insertions(+), 17 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 62085a6..ec913ac 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -608,6 +608,7 @@ enum ieee80211_tid_conf_mask {
BIT(NL80211_TID_CONFIG_ATTR_AMPDU_CTRL),
IEEE80211_TID_CONF_RTSCTS =
BIT(NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL),
+ IEEE80211_TID_CONF_TX_BITRATE = BIT(NL80211_TID_CONFIG_ATTR_TX_RATE),
};

/**
@@ -622,6 +623,8 @@ enum ieee80211_tid_conf_mask {
* @retry_short: retry count value
* @ampdu: Enable/Disable aggregation
* @rtscts: Enable/Disable RTS/CTS
+ * @txrate_type: TX bitrate mask type
+ * @mask: bitrate to be applied for the TID
*/
struct ieee80211_tid_cfg {
bool config_override;
@@ -632,6 +635,8 @@ struct ieee80211_tid_cfg {
int retry_short;
u8 ampdu;
u8 rtscts;
+ enum nl80211_tx_rate_setting txrate_type;
+ struct cfg80211_bitrate_mask *mask;
};

/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c4ac7d7..b9e79c3 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4736,6 +4736,18 @@ enum nl80211_tx_power_setting {
};

/**
+ * enum nl80211_tx_rate_setting - TX rate configuration type
+ * @NL80211_TX_RATE_AUTOMATIC: automatically determine TX rate
+ * @NL80211_TX_RATE_LIMITED: limit the TX rate by the TX rate parameter
+ * @NL80211_TX_RATE_FIXED: fix TX rate to the TX rate parameter
+ */
+enum nl80211_tx_rate_setting {
+ NL80211_TX_RATE_AUTOMATIC,
+ NL80211_TX_RATE_LIMITED,
+ NL80211_TX_RATE_FIXED,
+};
+
+/**
* enum nl80211_tid_config - TID config state
* @NL80211_TID_CONFIG_ENABLE: Enable config for the TID
* @NL80211_TID_CONFIG_DISABLE: Disable config for the TID
@@ -4771,6 +4783,10 @@ enum nl80211_tid_config {
* specified in %NL80211_TID_CONFIG_ATTR_TIDS. Its type is u8,
* @NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL: Enable/Disable RTS_CTS for the TID
* specified in %%NL80211_TID_CONFIG_ATTR_TIDS. It is u8 type.
+ * @NL80211_TID_CONFIG_ATTR_TX_RATE: Data frame TX rate mask should be applied
+ * with the parameters passed through %NL80211_ATTR_TX_RATES. This
+ * configuration is per TID, TID is specified with
+ * %NL80211_TID_CONFIG_ATTR_TIDS.
*/
enum nl80211_tid_config_attr {
__NL80211_TID_CONFIG_ATTR_INVALID,
@@ -4781,6 +4797,8 @@ enum nl80211_tid_config_attr {
NL80211_TID_CONFIG_ATTR_RETRY_LONG,
NL80211_TID_CONFIG_ATTR_AMPDU_CTRL,
NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL,
+ NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE,
+ NL80211_TID_CONFIG_ATTR_TX_RATE,

/* keep last */
__NL80211_TID_CONFIG_ATTR_AFTER_LAST,
@@ -5621,6 +5639,10 @@ enum nl80211_feature_flags {
* RTS_CTS control(enable/disable).
* @NL80211_EXT_FEATURE_PER_STA_RTSCTS_CTRL_CONFIG: Driver supports STA specific
* RTS_CTS control(enable/disable).
+ * @NL80211_EXT_FEATURE_PER_TID_TX_RATE_CONFIG: Driver supports TID specific
+ * TX bitrate configuration.
+ * @NL80211_EXT_FEATURE_PER_STA_TX_RATE_CONFIG: Driver supports STA specific
+ * TX bitrate configuration.
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -5678,6 +5700,8 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_PER_STA_AMPDU_CTRL_CONFIG,
NL80211_EXT_FEATURE_PER_TID_RTSCTS_CTRL_CONFIG,
NL80211_EXT_FEATURE_PER_STA_RTSCTS_CTRL_CONFIG,
+ NL80211_EXT_FEATURE_PER_TID_TX_RATE_CONFIG,
+ NL80211_EXT_FEATURE_PER_STA_TX_RATE_CONFIG,

/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8388dbf..2ae8f6f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -321,6 +321,18 @@ static int validate_ie_attr(const struct nlattr *attr,
NLA_POLICY_RANGE(NLA_U8, 1, 20),
};

+static const struct nla_policy nl80211_txattr_policy[NL80211_TXRATE_MAX + 1] = {
+ [NL80211_TXRATE_LEGACY] = { .type = NLA_BINARY,
+ .len = NL80211_MAX_SUPP_RATES },
+ [NL80211_TXRATE_HT] = { .type = NLA_BINARY,
+ .len = NL80211_MAX_SUPP_HT_RATES },
+ [NL80211_TXRATE_VHT] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = sizeof(struct nl80211_txrate_vht),
+ },
+ [NL80211_TXRATE_GI] = { .type = NLA_U8 },
+};
+
static const struct nla_policy
nl80211_tid_config_attr_policy[NL80211_TID_CONFIG_ATTR_MAX + 1] = {
[NL80211_TID_CONFIG_ATTR_OVERRIDE] = { .type = NLA_FLAG },
@@ -333,6 +345,10 @@ static int validate_ie_attr(const struct nlattr *attr,
NLA_POLICY_MAX(NLA_U8, NL80211_TID_CONFIG_DISABLE),
[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL] =
NLA_POLICY_MAX(NLA_U8, NL80211_TID_CONFIG_DISABLE),
+ [NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE] =
+ NLA_POLICY_MAX(NLA_U8, NL80211_TX_RATE_FIXED),
+ [NL80211_TID_CONFIG_ATTR_TX_RATE] =
+ NLA_POLICY_NESTED(nl80211_txattr_policy),
};

const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
@@ -4245,19 +4261,9 @@ static bool vht_set_mcs_mask(struct ieee80211_supported_band *sband,
return true;
}

-static const struct nla_policy nl80211_txattr_policy[NL80211_TXRATE_MAX + 1] = {
- [NL80211_TXRATE_LEGACY] = { .type = NLA_BINARY,
- .len = NL80211_MAX_SUPP_RATES },
- [NL80211_TXRATE_HT] = { .type = NLA_BINARY,
- .len = NL80211_MAX_SUPP_HT_RATES },
- [NL80211_TXRATE_VHT] = {
- .type = NLA_EXACT_LEN_WARN,
- .len = sizeof(struct nl80211_txrate_vht),
- },
- [NL80211_TXRATE_GI] = { .type = NLA_U8 },
-};
-
static int nl80211_parse_tx_bitrate_mask(struct genl_info *info,
+ struct nlattr *attrs[],
+ enum nl80211_attrs attr,
struct cfg80211_bitrate_mask *mask)
{
struct nlattr *tb[NL80211_TXRATE_MAX + 1];
@@ -4288,14 +4294,14 @@ static int nl80211_parse_tx_bitrate_mask(struct genl_info *info,
}

/* if no rates are given set it back to the defaults */
- if (!info->attrs[NL80211_ATTR_TX_RATES])
+ if (!attrs[attr])
goto out;

/* The nested attribute uses enum nl80211_band as the index. This maps
* directly to the enum nl80211_band values used in cfg80211.
*/
BUILD_BUG_ON(NL80211_MAX_SUPP_HT_RATES > IEEE80211_HT_MCS_MASK_LEN * 8);
- nla_for_each_nested(tx_rates, info->attrs[NL80211_ATTR_TX_RATES], rem) {
+ nla_for_each_nested(tx_rates, attrs[attr], rem) {
enum nl80211_band band = nla_type(tx_rates);
int err;

@@ -4771,7 +4777,9 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;

if (info->attrs[NL80211_ATTR_TX_RATES]) {
- err = nl80211_parse_tx_bitrate_mask(info, &params.beacon_rate);
+ err = nl80211_parse_tx_bitrate_mask(info, info->attrs,
+ NL80211_ATTR_TX_RATES,
+ &params.beacon_rate);
if (err)
return err;

@@ -10521,7 +10529,8 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
if (!rdev->ops->set_bitrate_mask)
return -EOPNOTSUPP;

- err = nl80211_parse_tx_bitrate_mask(info, &mask);
+ err = nl80211_parse_tx_bitrate_mask(info, info->attrs,
+ NL80211_ATTR_TX_RATES, &mask);
if (err)
return err;

@@ -11116,7 +11125,9 @@ static int nl80211_join_mesh(struct sk_buff *skb, struct genl_info *info)
}

if (info->attrs[NL80211_ATTR_TX_RATES]) {
- err = nl80211_parse_tx_bitrate_mask(info, &setup.beacon_rate);
+ err = nl80211_parse_tx_bitrate_mask(info, info->attrs,
+ NL80211_ATTR_TX_RATES,
+ &setup.beacon_rate);
if (err)
return err;

@@ -13936,6 +13947,37 @@ static int parse_tid_conf(struct cfg80211_registered_device *rdev,
nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL]);
}

+ if (attrs[NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE]) {
+ int idx;
+ enum nl80211_attrs attr;
+
+ err = nl80211_check_tid_config_support(rdev, extack, peer,
+ attrs, tid_conf,
+ TX_RATE);
+ if (err)
+ return err;
+ idx = NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE;
+ tid_conf->txrate_type = nla_get_u8(attrs[idx]);
+ if (tid_conf->txrate_type != NL80211_TX_RATE_AUTOMATIC) {
+ tid_conf->mask =
+ kzalloc(sizeof(struct cfg80211_bitrate_mask),
+ GFP_KERNEL);
+ if (!tid_conf->mask)
+ return -ENOMEM;
+
+ attr =
+ (enum nl80211_attrs)NL80211_TID_CONFIG_ATTR_TX_RATE;
+ err = nl80211_parse_tx_bitrate_mask(info, attrs, attr,
+ tid_conf->mask);
+ if (err) {
+ kfree(tid_conf->mask);
+ return err;
+ }
+ } else {
+ tid_conf->mask = NULL;
+ }
+ }
+
return 0;
}

--
1.7.9.5


2020-01-14 12:34:16

by Sergey Matyukevich

[permalink] [raw]
Subject: Re: [PATCHv9 5/6] nl80211: Add support to configure TID specific txrate configuration

> This patch adds support to configure per TID txrate configuration
> configuration through the NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE
> and NL80211_TID_CONFIG_ATTR_TX_RATE
> attribute. TX bitrate mask values passed
> in NL80211_ATTR_TX_RATES attribute and NL80211_TID_CONFIG_ATTR_TX_RATES
> attribute will have types of the TX rate should be applied. This uses
> nl80211_parse_tx_bitrate_mask to validate and calculate the bitrate
> mask.
>
> Signed-off-by: Tamizh chelvam <[email protected]>
> ---
> include/net/cfg80211.h | 5 +++
> include/uapi/linux/nl80211.h | 24 +++++++++++++
> net/wireless/nl80211.c | 76 ++++++++++++++++++++++++++++++++----------
> 3 files changed, 88 insertions(+), 17 deletions(-)

...

> @@ -13936,6 +13947,37 @@ static int parse_tid_conf(struct cfg80211_registered_device *rdev,
> nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL]);
> }
>
> + if (attrs[NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE]) {
> + int idx;
> + enum nl80211_attrs attr;
> +
> + err = nl80211_check_tid_config_support(rdev, extack, peer,
> + attrs, tid_conf,
> + TX_RATE);
> + if (err)
> + return err;
> + idx = NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE;
> + tid_conf->txrate_type = nla_get_u8(attrs[idx]);
> + if (tid_conf->txrate_type != NL80211_TX_RATE_AUTOMATIC) {
> + tid_conf->mask =
> + kzalloc(sizeof(struct cfg80211_bitrate_mask),
> + GFP_KERNEL);
> + if (!tid_conf->mask)
> + return -ENOMEM;
> +
> + attr =
> + (enum nl80211_attrs)NL80211_TID_CONFIG_ATTR_TX_RATE;
> + err = nl80211_parse_tx_bitrate_mask(info, attrs, attr,
> + tid_conf->mask);
> + if (err) {
> + kfree(tid_conf->mask);
> + return err;
> + }

IIUC we have to free all the allocated tid_conf->mask entries in the end of
nl80211_set_tid_config, right before tid_config is freed. Alternatively,
struct ieee80211_tid_cfg can be modified to keep cfg80211_bitrate_mask
value rather than pointer.

> + } else {
> + tid_conf->mask = NULL;
> + }
> + }
> +
> return 0;
> }

Regards,
Sergey

2020-01-20 07:51:45

by Tamizh chelvam

[permalink] [raw]
Subject: Re: [PATCHv9 5/6] nl80211: Add support to configure TID specific txrate configuration

On 2020-01-14 18:01, Sergey Matyukevich wrote:
>> This patch adds support to configure per TID txrate configuration
>> configuration through the NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE
>> and NL80211_TID_CONFIG_ATTR_TX_RATE
>> attribute. TX bitrate mask values passed
>> in NL80211_ATTR_TX_RATES attribute and
>> NL80211_TID_CONFIG_ATTR_TX_RATES
>> attribute will have types of the TX rate should be applied. This uses
>> nl80211_parse_tx_bitrate_mask to validate and calculate the bitrate
>> mask.
>>
>> Signed-off-by: Tamizh chelvam <[email protected]>
>> ---
>> include/net/cfg80211.h | 5 +++
>> include/uapi/linux/nl80211.h | 24 +++++++++++++
>> net/wireless/nl80211.c | 76
>> ++++++++++++++++++++++++++++++++----------
>> 3 files changed, 88 insertions(+), 17 deletions(-)
>
> ...
>
>> @@ -13936,6 +13947,37 @@ static int parse_tid_conf(struct
>> cfg80211_registered_device *rdev,
>> nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL]);
>> }
>>
>> + if (attrs[NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE]) {
>> + int idx;
>> + enum nl80211_attrs attr;
>> +
>> + err = nl80211_check_tid_config_support(rdev, extack, peer,
>> + attrs, tid_conf,
>> + TX_RATE);
>> + if (err)
>> + return err;
>> + idx = NL80211_TID_CONFIG_ATTR_TX_RATE_TYPE;
>> + tid_conf->txrate_type = nla_get_u8(attrs[idx]);
>> + if (tid_conf->txrate_type != NL80211_TX_RATE_AUTOMATIC) {
>> + tid_conf->mask =
>> + kzalloc(sizeof(struct cfg80211_bitrate_mask),
>> + GFP_KERNEL);
>> + if (!tid_conf->mask)
>> + return -ENOMEM;
>> +
>> + attr =
>> + (enum nl80211_attrs)NL80211_TID_CONFIG_ATTR_TX_RATE;
>> + err = nl80211_parse_tx_bitrate_mask(info, attrs, attr,
>> + tid_conf->mask);
>> + if (err) {
>> + kfree(tid_conf->mask);
>> + return err;
>> + }
>
> IIUC we have to free all the allocated tid_conf->mask entries in the
> end of
> nl80211_set_tid_config, right before tid_config is freed.
Yeah, this needs to be take care by the driver, since it will be sent
with multiple
configuration. I have added that in the comment in next patchset.
> Alternatively,struct ieee80211_tid_cfg can be modified to keep
> cfg80211_bitrate_mask
> value rather than pointer.
I have just reused the nl80211_parse_tx_bitrate_mask, so I feel using
the similar approach
should be good.

>
>> + } else {
>> + tid_conf->mask = NULL;
>> + }
>> + }
>> +
>> return 0;
>> }
>
Thanks,
Tamizh.