| Message ID | 1550813554-11581-2-git-send-email-tamizhr@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | cfg80211/mac80211: Add support for TID specific configuration | expand |
Hello Tamizh, > Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org> > --- > include/net/cfg80211.h | 35 +++++++++++++++ > include/uapi/linux/nl80211.h | 51 ++++++++++++++++++++++ > net/wireless/nl80211.c | 102 +++++++++++++++++++++++++++++++++++++++++++ > net/wireless/rdev-ops.h | 11 +++++ > net/wireless/trace.h | 18 ++++++++ > 5 files changed, 217 insertions(+) ... > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > index 82d5e1e..352eb4a2 100644 > --- a/net/wireless/nl80211.c > +++ b/net/wireless/nl80211.c > @@ -280,6 +280,13 @@ static int validate_ie_attr(const struct nlattr *attr, > NLA_POLICY_NESTED_ARRAY(nl80211_psmr_peer_attr_policy), > }; > > +static const struct nla_policy > +nl80211_attr_tid_config_policy[NL80211_ATTR_TID_CONFIG_MAX + 1] = { > + [NL80211_ATTR_TID_CONFIG_TID] = NLA_POLICY_MAX(NLA_U8, 7), Such a policy permits configuration of per-TID settings, either for per-STA or for all the STAs. However it is not possible to perform configuration for all TIDs at once, e.g. passing -1 value to driver. Maybe simplify policy and use .type = NLA_U8 ? Sanity check, if needed, can be performed by driver or even by firmware. > + [NL80211_ATTR_TID_CONFIG_NOACK] = > + NLA_POLICY_MAX(NLA_U8, NL80211_TID_CONFIG_DISABLE), > +}; > + > const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { > [NL80211_ATTR_WIPHY] = { .type = NLA_U32 }, > [NL80211_ATTR_WIPHY_NAME] = { .type = NLA_NUL_STRING, > @@ -541,6 +548,8 @@ static int validate_ie_attr(const struct nlattr *attr, > [NL80211_ATTR_PEER_MEASUREMENTS] = > NLA_POLICY_NESTED(nl80211_pmsr_attr_policy), > [NL80211_ATTR_AIRTIME_WEIGHT] = NLA_POLICY_MIN(NLA_U16, 1), > + [NL80211_ATTR_TID_CONFIG] = > + NLA_POLICY_NESTED(nl80211_attr_tid_config_policy), > }; ... > +static int nl80211_set_tid_config(struct sk_buff *skb, > + struct genl_info *info) > +{ > + struct cfg80211_registered_device *rdev = info->user_ptr[0]; > + struct nlattr *attrs[NL80211_ATTR_TID_CONFIG_MAX + 1]; > + struct net_device *dev = info->user_ptr[1]; > + struct ieee80211_tid_config *tid_conf; > + struct nlattr *tid; > + int conf_idx = 0, rem_conf; > + u32 num_conf = 0, size_of_conf; > + int ret = -EINVAL; > + > + if (!info->attrs[NL80211_ATTR_TID_CONFIG]) > + return -EINVAL; > + > + if (!rdev->ops->set_tid_config) > + return -EOPNOTSUPP; > + > + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG], > + rem_conf) > + num_conf++; > + > + size_of_conf = sizeof(struct ieee80211_tid_config) + > + num_conf * sizeof(struct ieee80211_tid_cfg); > + > + tid_conf = kzalloc(size_of_conf, GFP_KERNEL); > + if (!tid_conf) > + return -ENOMEM; > + > + tid_conf->n_tid_conf = num_conf; > + > + if (info->attrs[NL80211_ATTR_MAC]) > + tid_conf->peer = nla_data(info->attrs[NL80211_ATTR_MAC]); > + else > + tid_conf->peer = NULL; > + > + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG], > + rem_conf) { > + ret = nla_parse_nested(attrs, NL80211_ATTR_TID_CONFIG_MAX, tid, > + NULL, NULL); > + > + if (ret) > + return ret; > + > + if (!attrs[NL80211_ATTR_TID_CONFIG_TID]) > + return -EINVAL; > + > + ret = parse_tid_conf(rdev, attrs, &tid_conf->tid_conf[conf_idx], > + tid_conf->peer); > + if (ret) > + goto bad_tid_conf; > + > + conf_idx++; > + } > + > + return rdev_set_tid_config(rdev, dev, tid_conf); What is the ownership rule for tid_conf ? In other words, who is responsible for freeing this structure ? Unless I am missing something, in the suggested patches there is no kfree for this structure and all the nested structures (see Tx bitrate patch), neither in cfg80211/mac80211 nor in ath10k. As far as I understand, we have two options here. One option is to explicitly specify that ownership is passed to the caller, similar to nl80211_set_reg. Another option is to release memory in this function after driver makes copies of all necessary fields, e.g. see nl80211_set_mac_acl. > + > +bad_tid_conf: > + kfree(tid_conf); > + return ret; > +} Regards, Sergey
Hi Sergey, > >> Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org> >> --- >> include/net/cfg80211.h | 35 +++++++++++++++ >> include/uapi/linux/nl80211.h | 51 ++++++++++++++++++++++ >> net/wireless/nl80211.c | 102 >> +++++++++++++++++++++++++++++++++++++++++++ >> net/wireless/rdev-ops.h | 11 +++++ >> net/wireless/trace.h | 18 ++++++++ >> 5 files changed, 217 insertions(+) > > ... > >> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c >> index 82d5e1e..352eb4a2 100644 >> --- a/net/wireless/nl80211.c >> +++ b/net/wireless/nl80211.c >> @@ -280,6 +280,13 @@ static int validate_ie_attr(const struct nlattr >> *attr, >> >> NLA_POLICY_NESTED_ARRAY(nl80211_psmr_peer_attr_policy), >> }; >> >> +static const struct nla_policy >> +nl80211_attr_tid_config_policy[NL80211_ATTR_TID_CONFIG_MAX + 1] = { >> + [NL80211_ATTR_TID_CONFIG_TID] = NLA_POLICY_MAX(NLA_U8, 7), > > Such a policy permits configuration of per-TID settings, either > for per-STA or for all the STAs. However it is not possible to > perform configuration for all TIDs at once, e.g. passing -1 value > to driver. > > Maybe simplify policy and use .type = NLA_U8 ? > > Sanity check, if needed, can be performed by driver or even by > firmware. > Sure, we can have like that. And do you feel driver should advertise support to perform configuration for all TIDs(like accepting tid -1) ? >> + [NL80211_ATTR_TID_CONFIG_NOACK] = >> + NLA_POLICY_MAX(NLA_U8, >> NL80211_TID_CONFIG_DISABLE), >> +}; >> + >> const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { >> [NL80211_ATTR_WIPHY] = { .type = NLA_U32 }, >> [NL80211_ATTR_WIPHY_NAME] = { .type = NLA_NUL_STRING, >> @@ -541,6 +548,8 @@ static int validate_ie_attr(const struct nlattr >> *attr, >> [NL80211_ATTR_PEER_MEASUREMENTS] = >> NLA_POLICY_NESTED(nl80211_pmsr_attr_policy), >> [NL80211_ATTR_AIRTIME_WEIGHT] = NLA_POLICY_MIN(NLA_U16, 1), >> + [NL80211_ATTR_TID_CONFIG] = >> + >> NLA_POLICY_NESTED(nl80211_attr_tid_config_policy), >> }; > > ... > >> +static int nl80211_set_tid_config(struct sk_buff *skb, >> + struct genl_info *info) >> +{ >> + struct cfg80211_registered_device *rdev = info->user_ptr[0]; >> + struct nlattr *attrs[NL80211_ATTR_TID_CONFIG_MAX + 1]; >> + struct net_device *dev = info->user_ptr[1]; >> + struct ieee80211_tid_config *tid_conf; >> + struct nlattr *tid; >> + int conf_idx = 0, rem_conf; >> + u32 num_conf = 0, size_of_conf; >> + int ret = -EINVAL; >> + >> + if (!info->attrs[NL80211_ATTR_TID_CONFIG]) >> + return -EINVAL; >> + >> + if (!rdev->ops->set_tid_config) >> + return -EOPNOTSUPP; >> + >> + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG], >> + rem_conf) >> + num_conf++; >> + >> + size_of_conf = sizeof(struct ieee80211_tid_config) + >> + num_conf * sizeof(struct ieee80211_tid_cfg); >> + >> + tid_conf = kzalloc(size_of_conf, GFP_KERNEL); >> + if (!tid_conf) >> + return -ENOMEM; >> + >> + tid_conf->n_tid_conf = num_conf; >> + >> + if (info->attrs[NL80211_ATTR_MAC]) >> + tid_conf->peer = >> nla_data(info->attrs[NL80211_ATTR_MAC]); >> + else >> + tid_conf->peer = NULL; >> + >> + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG], >> + rem_conf) { >> + ret = nla_parse_nested(attrs, >> NL80211_ATTR_TID_CONFIG_MAX, tid, >> + NULL, NULL); >> + >> + if (ret) >> + return ret; >> + >> + if (!attrs[NL80211_ATTR_TID_CONFIG_TID]) >> + return -EINVAL; >> + >> + ret = parse_tid_conf(rdev, attrs, >> &tid_conf->tid_conf[conf_idx], >> + tid_conf->peer); >> + if (ret) >> + goto bad_tid_conf; >> + >> + conf_idx++; >> + } >> + >> + return rdev_set_tid_config(rdev, dev, tid_conf); > > What is the ownership rule for tid_conf ? In other words, who is > responsible for freeing this structure ? > > Unless I am missing something, in the suggested patches there is no > kfree for this structure and all the nested structures (see Tx bitrate > patch), neither in cfg80211/mac80211 nor in ath10k. > > As far as I understand, we have two options here. One option is to > explicitly specify that ownership is passed to the caller, similar > to nl80211_set_reg. Another option is to release memory in this > function after driver makes copies of all necessary fields, > e.g. see nl80211_set_mac_acl. > Thanks for pointing out, will free it in this function(cfg80211) itself. I will make the change and send next patchset. >> + >> +bad_tid_conf: >> + kfree(tid_conf); >> + return ret; >> +} Thanks, Tamizh.
Hi Tamizh, > Hi Sergey, > > > > > Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org> > > > --- > > > include/net/cfg80211.h | 35 +++++++++++++++ > > > include/uapi/linux/nl80211.h | 51 ++++++++++++++++++++++ > > > net/wireless/nl80211.c | 102 > > > +++++++++++++++++++++++++++++++++++++++++++ > > > net/wireless/rdev-ops.h | 11 +++++ > > > net/wireless/trace.h | 18 ++++++++ > > > 5 files changed, 217 insertions(+) > > > > ... > > > > > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > > > index 82d5e1e..352eb4a2 100644 > > > --- a/net/wireless/nl80211.c > > > +++ b/net/wireless/nl80211.c > > > @@ -280,6 +280,13 @@ static int validate_ie_attr(const struct nlattr > > > *attr, > > > > > > NLA_POLICY_NESTED_ARRAY(nl80211_psmr_peer_attr_policy), > > > }; > > > > > > +static const struct nla_policy > > > +nl80211_attr_tid_config_policy[NL80211_ATTR_TID_CONFIG_MAX + 1] = { > > > + [NL80211_ATTR_TID_CONFIG_TID] = NLA_POLICY_MAX(NLA_U8, 7), > > > > Such a policy permits configuration of per-TID settings, either > > for per-STA or for all the STAs. However it is not possible to > > perform configuration for all TIDs at once, e.g. passing -1 value > > to driver. > > > > Maybe simplify policy and use .type = NLA_U8 ? > > > > Sanity check, if needed, can be performed by driver or even by > > firmware. > > > Sure, we can have like that. And do you feel driver should advertise > support to perform configuration for all TIDs(like accepting tid -1) ? Do you think this additional level of granularity is needed ? IIUC if driver/firmware supports per TID feature configuration, then can handle all-TIDs configuration as well. Anyway, attempt for global feature configuration can be rejected on driver or firmware level if needed. > > > + [NL80211_ATTR_TID_CONFIG_NOACK] = > > > + NLA_POLICY_MAX(NLA_U8, > > > NL80211_TID_CONFIG_DISABLE), > > > +}; > > > + > > > const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { > > > [NL80211_ATTR_WIPHY] = { .type = NLA_U32 }, > > > [NL80211_ATTR_WIPHY_NAME] = { .type = NLA_NUL_STRING, > > > @@ -541,6 +548,8 @@ static int validate_ie_attr(const struct nlattr > > > *attr, > > > [NL80211_ATTR_PEER_MEASUREMENTS] = > > > NLA_POLICY_NESTED(nl80211_pmsr_attr_policy), > > > [NL80211_ATTR_AIRTIME_WEIGHT] = NLA_POLICY_MIN(NLA_U16, 1), > > > + [NL80211_ATTR_TID_CONFIG] = > > > + > > > NLA_POLICY_NESTED(nl80211_attr_tid_config_policy), > > > }; > > > > ... > > > > > +static int nl80211_set_tid_config(struct sk_buff *skb, > > > + struct genl_info *info) > > > +{ > > > + struct cfg80211_registered_device *rdev = info->user_ptr[0]; > > > + struct nlattr *attrs[NL80211_ATTR_TID_CONFIG_MAX + 1]; > > > + struct net_device *dev = info->user_ptr[1]; > > > + struct ieee80211_tid_config *tid_conf; > > > + struct nlattr *tid; > > > + int conf_idx = 0, rem_conf; > > > + u32 num_conf = 0, size_of_conf; > > > + int ret = -EINVAL; > > > + > > > + if (!info->attrs[NL80211_ATTR_TID_CONFIG]) > > > + return -EINVAL; > > > + > > > + if (!rdev->ops->set_tid_config) > > > + return -EOPNOTSUPP; > > > + > > > + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG], > > > + rem_conf) > > > + num_conf++; > > > + > > > + size_of_conf = sizeof(struct ieee80211_tid_config) + > > > + num_conf * sizeof(struct ieee80211_tid_cfg); > > > + > > > + tid_conf = kzalloc(size_of_conf, GFP_KERNEL); > > > + if (!tid_conf) > > > + return -ENOMEM; > > > + > > > + tid_conf->n_tid_conf = num_conf; > > > + > > > + if (info->attrs[NL80211_ATTR_MAC]) > > > + tid_conf->peer = > > > nla_data(info->attrs[NL80211_ATTR_MAC]); > > > + else > > > + tid_conf->peer = NULL; > > > + > > > + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG], > > > + rem_conf) { > > > + ret = nla_parse_nested(attrs, > > > NL80211_ATTR_TID_CONFIG_MAX, tid, > > > + NULL, NULL); > > > + > > > + if (ret) > > > + return ret; > > > + > > > + if (!attrs[NL80211_ATTR_TID_CONFIG_TID]) > > > + return -EINVAL; > > > + > > > + ret = parse_tid_conf(rdev, attrs, > > > &tid_conf->tid_conf[conf_idx], > > > + tid_conf->peer); > > > + if (ret) > > > + goto bad_tid_conf; > > > + > > > + conf_idx++; > > > + } > > > + > > > + return rdev_set_tid_config(rdev, dev, tid_conf); > > > > What is the ownership rule for tid_conf ? In other words, who is > > responsible for freeing this structure ? > > > > Unless I am missing something, in the suggested patches there is no > > kfree for this structure and all the nested structures (see Tx bitrate > > patch), neither in cfg80211/mac80211 nor in ath10k. > > > > As far as I understand, we have two options here. One option is to > > explicitly specify that ownership is passed to the caller, similar > > to nl80211_set_reg. Another option is to release memory in this > > function after driver makes copies of all necessary fields, > > e.g. see nl80211_set_mac_acl. > > > Thanks for pointing out, will free it in this function(cfg80211) itself. > I will make the change and send next patchset. Great! Thanks, Sergey
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index f81677f..07eb2de 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -509,6 +509,35 @@ struct cfg80211_chan_def { u32 center_freq2; }; +enum ieee80211_tid_conf_mask { + IEEE80211_TID_CONF_NOACK = BIT(0), +}; + +/** + * struct ieee80211_tid_cfg - TID specific configuration + * @tid: TID number + * @tid_conf_mask: bitmap indicating which parameter changed + * see %enum ieee80211_tid_conf_mask + * @noack: noack configuration value for the TID + */ +struct ieee80211_tid_cfg { + u8 tid; + enum ieee80211_tid_conf_mask tid_conf_mask; + u8 noack; +}; + +/** + * struct ieee80211_tid_config - TID configuration + * @peer: Station's MAC address + * @n_tid_conf: Number of TID specific configurations to be applied + * @tid_conf: Configuration change info + */ +struct ieee80211_tid_config { + const u8 *peer; + u32 n_tid_conf; + struct ieee80211_tid_cfg tid_conf[]; +}; + /** * cfg80211_get_chandef_type - return old channel type from chandef * @chandef: the channel definition @@ -3436,6 +3465,10 @@ struct cfg80211_pmsr_request { * Statistics should be cumulative, currently no way to reset is provided. * @start_pmsr: start peer measurement (e.g. FTM) * @abort_pmsr: abort peer measurement + * @set_tid_config: TID specific configuration. Apply this configuration for + * all the connected stations in the BSS if peer is NULL. Otherwise + * apply this configuration to the specific station. + * This callback may sleep. */ struct cfg80211_ops { int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow); @@ -3750,6 +3783,8 @@ struct cfg80211_ops { struct cfg80211_pmsr_request *request); void (*abort_pmsr)(struct wiphy *wiphy, struct wireless_dev *wdev, struct cfg80211_pmsr_request *request); + int (*set_tid_config)(struct wiphy *wiphy, struct net_device *dev, + struct ieee80211_tid_config *tid_conf); }; /* diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index dd4f86e..c901a48 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -1065,6 +1065,10 @@ * indicated by %NL80211_ATTR_WIPHY_FREQ and other attributes * determining the width and type. * + * @NL80211_CMD_SET_TID_CONFIG: Data frame TID specific configuration + * is passed through this command using %NL80211_ATTR_TID_CONFIG + * nested attributes. + * * @NL80211_CMD_MAX: highest used command number * @__NL80211_CMD_AFTER_LAST: internal use */ @@ -1285,6 +1289,8 @@ enum nl80211_commands { NL80211_CMD_NOTIFY_RADAR, + NL80211_CMD_SET_TID_CONFIG, + /* add new commands above here */ /* used to define NL80211_CMD_MAX below */ @@ -2308,6 +2314,9 @@ enum nl80211_commands { * @NL80211_ATTR_AIRTIME_WEIGHT: Station's weight when scheduled by the airtime * scheduler. * + * @NL80211_ATTR_TID_CONFIG: TID specific configuration in a + * nested attribute with %NL80211_ATTR_TID_* sub-attributes. + * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use @@ -2759,6 +2768,8 @@ enum nl80211_attrs { NL80211_ATTR_AIRTIME_WEIGHT, + NL80211_ATTR_TID_CONFIG, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, @@ -4543,6 +4554,39 @@ enum nl80211_tx_power_setting { NL80211_TX_POWER_FIXED, }; +enum nl80211_tid_config { + NL80211_TID_CONFIG_DEFAULT, + NL80211_TID_CONFIG_ENABLE, + NL80211_TID_CONFIG_DISABLE, +}; + +/* enum nl80211_attr_tid_config - TID specific configuration. + * @NL80211_ATTR_TID_CONFIG_TID: a TID value (u8 attribute). + * @NL80211_ATTR_TID_CONFIG_NOACK: Configure ack policy for the TID. + * specified in %NL80211_ATTR_TID_CONFIG_TID. see %enum nl80211_tid_config. + * Its type is u8, if the peer MAC address is passed in %NL80211_ATTR_MAC, + * then the noack configuration is applied to the data frame for the tid + * to that connected station. This configuration is valid only for STA's + * current connection. i.e. the configuration will be reset to default when + * the station connects back after disconnection/roaming. + * when user-space does not include %NL80211_ATTR_MAC, then this + * configuration should be treated as per-netdev configuration. + * This configuration will be cleared when the interface goes down and on + * the disconnection from a BSS. Driver supporting this feature should + * advertise NL80211_EXT_FEATURE_PER_TID_NOACK_CONFIG and + * NL80211_EXT_FEATURE_PER_STA_NOACK_CONFIG for supporting per sta + * configuration. + */ +enum nl80211_attr_tid_config { + __NL80211_ATTR_TID_INVALID, + NL80211_ATTR_TID_CONFIG_TID, + NL80211_ATTR_TID_CONFIG_NOACK, + + /* keep last */ + __NL80211_ATTR_TID_CONFIG_AFTER_LAST, + NL80211_ATTR_TID_CONFIG_MAX = __NL80211_ATTR_TID_CONFIG_AFTER_LAST - 1 +}; + /** * enum nl80211_packet_pattern_attr - packet pattern attribute * @__NL80211_PKTPAT_INVALID: invalid number for nested attribute @@ -5343,6 +5387,11 @@ enum nl80211_feature_flags { * @NL80211_EXT_FEATURE_AP_PMKSA_CACHING: Driver/device supports PMKSA caching * (set/del PMKSA operations) in AP mode. * + * @NL80211_EXT_FEATURE_PER_TID_NOACK_CONFIG: Driver supports per TID NoAck + * policy functionality. + * @NL80211_EXT_FEATURE_PER_STA_NOACK_CONFIG: Driver supports STA specific NoAck + * policy functionality. + * * @NUM_NL80211_EXT_FEATURES: number of extended features. * @MAX_NL80211_EXT_FEATURES: highest extended feature index. */ @@ -5384,6 +5433,8 @@ enum nl80211_ext_feature_index { NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER, NL80211_EXT_FEATURE_AIRTIME_FAIRNESS, NL80211_EXT_FEATURE_AP_PMKSA_CACHING, + NL80211_EXT_FEATURE_PER_TID_NOACK_CONFIG, + NL80211_EXT_FEATURE_PER_STA_NOACK_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 82d5e1e..352eb4a2 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -280,6 +280,13 @@ static int validate_ie_attr(const struct nlattr *attr, NLA_POLICY_NESTED_ARRAY(nl80211_psmr_peer_attr_policy), }; +static const struct nla_policy +nl80211_attr_tid_config_policy[NL80211_ATTR_TID_CONFIG_MAX + 1] = { + [NL80211_ATTR_TID_CONFIG_TID] = NLA_POLICY_MAX(NLA_U8, 7), + [NL80211_ATTR_TID_CONFIG_NOACK] = + NLA_POLICY_MAX(NLA_U8, NL80211_TID_CONFIG_DISABLE), +}; + const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { [NL80211_ATTR_WIPHY] = { .type = NLA_U32 }, [NL80211_ATTR_WIPHY_NAME] = { .type = NLA_NUL_STRING, @@ -541,6 +548,8 @@ static int validate_ie_attr(const struct nlattr *attr, [NL80211_ATTR_PEER_MEASUREMENTS] = NLA_POLICY_NESTED(nl80211_pmsr_attr_policy), [NL80211_ATTR_AIRTIME_WEIGHT] = NLA_POLICY_MIN(NLA_U16, 1), + [NL80211_ATTR_TID_CONFIG] = + NLA_POLICY_NESTED(nl80211_attr_tid_config_policy), }; /* policy for the key attributes */ @@ -13241,6 +13250,91 @@ static int nl80211_get_ftm_responder_stats(struct sk_buff *skb, return -ENOBUFS; } +static int parse_tid_conf(struct cfg80211_registered_device *rdev, + struct nlattr *attrs[], + struct ieee80211_tid_cfg *tid_conf, + const u8 *peer) +{ + tid_conf->tid = nla_get_u8(attrs[NL80211_ATTR_TID_CONFIG_TID]); + if (attrs[NL80211_ATTR_TID_CONFIG_NOACK]) { + if (!wiphy_ext_feature_isset(&rdev->wiphy, + NL80211_EXT_FEATURE_PER_TID_NOACK_CONFIG)) + return -ENOTSUPP; + + if (peer && !wiphy_ext_feature_isset(&rdev->wiphy, + NL80211_EXT_FEATURE_PER_STA_NOACK_CONFIG)) + return -ENOTSUPP; + + tid_conf->tid_conf_mask |= IEEE80211_TID_CONF_NOACK; + tid_conf->noack = + nla_get_u8(attrs[NL80211_ATTR_TID_CONFIG_NOACK]); + } + + return 0; +} + +static int nl80211_set_tid_config(struct sk_buff *skb, + struct genl_info *info) +{ + struct cfg80211_registered_device *rdev = info->user_ptr[0]; + struct nlattr *attrs[NL80211_ATTR_TID_CONFIG_MAX + 1]; + struct net_device *dev = info->user_ptr[1]; + struct ieee80211_tid_config *tid_conf; + struct nlattr *tid; + int conf_idx = 0, rem_conf; + u32 num_conf = 0, size_of_conf; + int ret = -EINVAL; + + if (!info->attrs[NL80211_ATTR_TID_CONFIG]) + return -EINVAL; + + if (!rdev->ops->set_tid_config) + return -EOPNOTSUPP; + + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG], + rem_conf) + num_conf++; + + size_of_conf = sizeof(struct ieee80211_tid_config) + + num_conf * sizeof(struct ieee80211_tid_cfg); + + tid_conf = kzalloc(size_of_conf, GFP_KERNEL); + if (!tid_conf) + return -ENOMEM; + + tid_conf->n_tid_conf = num_conf; + + if (info->attrs[NL80211_ATTR_MAC]) + tid_conf->peer = nla_data(info->attrs[NL80211_ATTR_MAC]); + else + tid_conf->peer = NULL; + + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG], + rem_conf) { + ret = nla_parse_nested(attrs, NL80211_ATTR_TID_CONFIG_MAX, tid, + NULL, NULL); + + if (ret) + return ret; + + if (!attrs[NL80211_ATTR_TID_CONFIG_TID]) + return -EINVAL; + + ret = parse_tid_conf(rdev, attrs, &tid_conf->tid_conf[conf_idx], + tid_conf->peer); + if (ret) + goto bad_tid_conf; + + conf_idx++; + } + + return rdev_set_tid_config(rdev, dev, tid_conf); + +bad_tid_conf: + kfree(tid_conf); + return ret; +} + #define NL80211_FLAG_NEED_WIPHY 0x01 #define NL80211_FLAG_NEED_NETDEV 0x02 #define NL80211_FLAG_NEED_RTNL 0x04 @@ -14175,6 +14269,14 @@ static void nl80211_post_doit(const struct genl_ops *ops, struct sk_buff *skb, .internal_flags = NL80211_FLAG_NEED_NETDEV_UP | NL80211_FLAG_NEED_RTNL, }, + { + .cmd = NL80211_CMD_SET_TID_CONFIG, + .doit = nl80211_set_tid_config, + .policy = nl80211_policy, + .flags = GENL_UNS_ADMIN_PERM, + .internal_flags = NL80211_FLAG_NEED_NETDEV | + NL80211_FLAG_NEED_RTNL, + }, }; static struct genl_family nl80211_fam __ro_after_init = { diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h index 5cb48d1..59e1905 100644 --- a/net/wireless/rdev-ops.h +++ b/net/wireless/rdev-ops.h @@ -1272,4 +1272,15 @@ static inline int rdev_del_pmk(struct cfg80211_registered_device *rdev, trace_rdev_return_void(&rdev->wiphy); } +static inline int rdev_set_tid_config(struct cfg80211_registered_device *rdev, + struct net_device *dev, + struct ieee80211_tid_config *tid_conf) +{ + int ret; + + trace_rdev_set_tid_config(&rdev->wiphy, dev, tid_conf); + ret = rdev->ops->set_tid_config(&rdev->wiphy, dev, tid_conf); + trace_rdev_return_int(&rdev->wiphy, ret); + return ret; +} #endif /* __CFG80211_RDEV_OPS */ diff --git a/net/wireless/trace.h b/net/wireless/trace.h index 44b2ce1..186de1f 100644 --- a/net/wireless/trace.h +++ b/net/wireless/trace.h @@ -3362,6 +3362,24 @@ WIPHY_PR_ARG, WDEV_PR_ARG, (unsigned long long)__entry->cookie) ); + +TRACE_EVENT(rdev_set_tid_config, + TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, + struct ieee80211_tid_config *tid_conf), + TP_ARGS(wiphy, netdev, tid_conf), + TP_STRUCT__entry( + WIPHY_ENTRY + NETDEV_ENTRY + MAC_ENTRY(peer) + ), + TP_fast_assign( + WIPHY_ASSIGN; + NETDEV_ASSIGN; + MAC_ASSIGN(peer, tid_conf->peer); + ), + TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", peer: " MAC_PR_FMT, + WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(peer)) +); #endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */ #undef TRACE_INCLUDE_PATH
Add a new NL command, NL80211_CMD_SET_TID_CONFIG to support data TID specific configuration. This per TID configurations are passed in NL80211_ATTR_TID_CONFIG which is a nested attribute. This patch adds support to configure per TID noack policy through NL80211_ATTR_TID_CONFIG_NOACK attribute. Data TID value for this configuration will be passed through NL80211_ATTR_TID_CONFIG_TID attribute. When the user-space wants this configuration peer specific rather than being applied for all the connected stations, MAC address of the peer can be passed in NL80211_ATTR_MAC attribute. This patch introduced enum ieee80211_tid_conf_mask to notify the driver that which configuration modified. Driver supporting data TID specific noack policy configuration should be advertise through NL80211_EXT_FEATURE_PER_TID_NOACK_CONFIG and supporting per STA data TID noack policy configuration should be advertise through NL80211_EXT_FEATURE_PER_STA_NOACK_CONFIG Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org> --- include/net/cfg80211.h | 35 +++++++++++++++ include/uapi/linux/nl80211.h | 51 ++++++++++++++++++++++ net/wireless/nl80211.c | 102 +++++++++++++++++++++++++++++++++++++++++++ net/wireless/rdev-ops.h | 11 +++++ net/wireless/trace.h | 18 ++++++++ 5 files changed, 217 insertions(+)