| Message ID | 20200618050427.5891-2-alokad@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | Add FILS discovery support | expand |
On Wed, 2020-06-17 at 22:04 -0700, Aloka Dixit wrote: > + * @NL80211_FILS_DISCOVERY_TMPL: Optional FILS discovery template. > + * It has contents of IEEE Std 802.11ai-2016 9.6.8.36 FILS discovery frame > + * (Figure 9-687a). Is that "It has (contents of ... FILS discovery frame) ..." or "It has contents of (... FILS discovery frame) ..."? I mean, is that with or without headers? The wording doesn't seem entirely clear to me. OTOH, if it's with headers, how could it be optional? In fact, either way, how is it optional? > +static int nl80211_parse_fils_discovery(struct nlattr *attrs, > + struct cfg80211_ap_settings *params) > +{ > + struct nlattr *tmpl; > + struct nlattr *tb[NL80211_FILS_DISCOVERY_MAX + 1]; > + int ret; > + struct cfg80211_fils_discovery *fd = ¶ms->fils_discovery; > + > + ret = nla_parse_nested(tb, NL80211_FILS_DISCOVERY_MAX, attrs, > + fils_discovery_policy, NULL); > + if (ret) > + return ret; > + > + if (!tb[NL80211_FILS_DISCOVERY_INT_MIN] || > + !tb[NL80211_FILS_DISCOVERY_INT_MAX]) > + return -EINVAL; > + > + fd->min_interval = nla_get_u32(tb[NL80211_FILS_DISCOVERY_INT_MIN]); > + fd->max_interval = nla_get_u32(tb[NL80211_FILS_DISCOVERY_INT_MAX]); > + > + tmpl = tb[NL80211_FILS_DISCOVERY_TMPL]; > + if (tmpl) { > + fd->tmpl = nla_data(tmpl); > + fd->tmpl_len = nla_len(tmpl); And if it's with headers, it should have some kind of minimum length too? You've only put a maximum length into the policy. (Which reminds me I wanted to have an NLA_POLICY_RANGE(NLA_BINARY, min, max) but haven't done that yet ...) johannes
On 2020-07-30 07:43, Johannes Berg wrote: > On Wed, 2020-06-17 at 22:04 -0700, Aloka Dixit wrote: >> + * @NL80211_FILS_DISCOVERY_TMPL: Optional FILS discovery template. >> + * It has contents of IEEE Std 802.11ai-2016 9.6.8.36 FILS discovery >> frame >> + * (Figure 9-687a). > > Is that > > "It has (contents of ... FILS discovery frame) ..." > > or > > "It has contents of (... FILS discovery frame) ..."? > > I mean, is that with or without headers? The wording doesn't seem > entirely clear to me. > > OTOH, if it's with headers, how could it be optional? In fact, either > way, how is it optional? > Template has management frame headers as well. Will change the wording accordingly. I made the template optional because FILS discovery may or may not be offloaded to FW. Another way would be to make it mandatory here. >> +static int nl80211_parse_fils_discovery(struct nlattr *attrs, >> + struct cfg80211_ap_settings *params) >> +{ >> + struct nlattr *tmpl; >> + struct nlattr *tb[NL80211_FILS_DISCOVERY_MAX + 1]; >> + int ret; >> + struct cfg80211_fils_discovery *fd = ¶ms->fils_discovery; >> + >> + ret = nla_parse_nested(tb, NL80211_FILS_DISCOVERY_MAX, attrs, >> + fils_discovery_policy, NULL); >> + if (ret) >> + return ret; >> + >> + if (!tb[NL80211_FILS_DISCOVERY_INT_MIN] || >> + !tb[NL80211_FILS_DISCOVERY_INT_MAX]) >> + return -EINVAL; >> + >> + fd->min_interval = nla_get_u32(tb[NL80211_FILS_DISCOVERY_INT_MIN]); >> + fd->max_interval = nla_get_u32(tb[NL80211_FILS_DISCOVERY_INT_MAX]); >> + >> + tmpl = tb[NL80211_FILS_DISCOVERY_TMPL]; >> + if (tmpl) { >> + fd->tmpl = nla_data(tmpl); >> + fd->tmpl_len = nla_len(tmpl); > > And if it's with headers, it should have some kind of minimum length > too? You've only put a maximum length into the policy. > > (Which reminds me I wanted to have an NLA_POLICY_RANGE(NLA_BINARY, min, > max) but haven't done that yet ...) > Yeah, I looked through existing examples for NLA_BINARY, those provide only the higher bound for length. But I can modify it to range once that is added. > johannes
On Thu, 2020-07-30 at 14:17 -0700, Aloka Dixit wrote: > > OTOH, if it's with headers, how could it be optional? In fact, either > > way, how is it optional? > > > > Template has management frame headers as well. Will change the wording > accordingly. OK. > I made the template optional because FILS discovery may or may not be > offloaded to FW. But how would anyone know? Try without it, and then try again if that fails? Would it fail? I mean, you also said it was required at least for 6 GHz, so wouldn't userspace be better off always giving it - and then we should probably make it mandatory so it doesn't fall into the trap? However - and here that's my ignorance speaking - can it really be offloaded? I mean, is everything in there completely determined by the beacon already, and so you have no choice in how to build it? Or how does that work? > Yeah, I looked through existing examples for NLA_BINARY, those provide > only the higher bound for length. Yeah, no way to do anything else right now. But you should have a lower bound in the code, I think. > But I can modify it to range once that is added. Later maybe :) johannes
On 2020-07-30 14:22, Johannes Berg wrote: > On Thu, 2020-07-30 at 14:17 -0700, Aloka Dixit wrote: > >> > OTOH, if it's with headers, how could it be optional? In fact, either >> > way, how is it optional? >> > >> >> Template has management frame headers as well. Will change the wording >> accordingly. > > OK. > >> I made the template optional because FILS discovery may or may not be >> offloaded to FW. > > But how would anyone know? Try without it, and then try again if that > fails? Would it fail? I mean, you also said it was required at least > for > 6 GHz, so wouldn't userspace be better off always giving it - and then > we should probably make it mandatory so it doesn't fall into the trap? > If the template is not provided, FW keeps sending event to get it. But as my ath11k driver code is limited to 6GHz, it already throws error if template not provided. Yes, in general it will be better to make it mandatory, I will do it in next version. > However - and here that's my ignorance speaking - can it really be > offloaded? I mean, is everything in there completely determined by the > beacon already, and so you have no choice in how to build it? Or how > does that work? > Yes, the frame parameters are fixed except for the timestamp which FW is expected to fill. >> Yeah, I looked through existing examples for NLA_BINARY, those provide >> only the higher bound for length. > > Yeah, no way to do anything else right now. But you should have a lower > bound in the code, I think. > Okay. >> But I can modify it to range once that is added. > > Later maybe :) > > johannes
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index fc7e8807838d..64d47463155a 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -1047,6 +1047,23 @@ struct cfg80211_acl_data { struct mac_address mac_addrs[]; }; +/** + * struct cfg80211_fils_discovery - FILS discovery parameters from + * IEEE Std 802.11ai-2016, Annex C.3 MIB detail. + * + * @min_interval: Minimum packet interval in TUs (0 - 10000) + * @max_interval: Maximum packet interval in TUs (0 - 10000) + * @tmpl_len: Template length + * @tmpl: Template data from IEEE Std 802.11ai-2016 9.6.8.36 FILS discovery + * frame (Figure 9-687a). + */ +struct cfg80211_fils_discovery { + u32 min_interval; + u32 max_interval; + size_t tmpl_len; + const u8 *tmpl; +}; + /** * enum cfg80211_ap_settings_flags - AP settings flags * @@ -1094,6 +1111,7 @@ enum cfg80211_ap_settings_flags { * @he_obss_pd: OBSS Packet Detection settings * @he_bss_color: BSS Color settings * @he_oper: HE operation IE (or %NULL if HE isn't enabled) + * @fils_discovery: FILS discovery transmission parameters */ struct cfg80211_ap_settings { struct cfg80211_chan_def chandef; @@ -1124,6 +1142,7 @@ struct cfg80211_ap_settings { u32 flags; struct ieee80211_he_obss_pd he_obss_pd; struct cfg80211_he_bss_color he_bss_color; + struct cfg80211_fils_discovery fils_discovery; }; /** diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 4e6339ab1fce..adab79c0f907 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -2505,6 +2505,10 @@ enum nl80211_commands { * @NL80211_ATTR_HE_6GHZ_CAPABILITY: HE 6 GHz Band Capability element (from * association request when used with NL80211_CMD_NEW_STATION). * + * @NL80211_ATTR_FILS_DISCOVERY: Optional parameter to configure FILS + * discovery. It is a nested attribute, see + * &enum nl80211_fils_discovery_attributes. + * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use @@ -2987,6 +2991,8 @@ enum nl80211_attrs { NL80211_ATTR_HE_6GHZ_CAPABILITY, + NL80211_ATTR_FILS_DISCOVERY, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, @@ -6922,4 +6928,34 @@ enum nl80211_iftype_akm_attributes { NL80211_IFTYPE_AKM_ATTR_MAX = __NL80211_IFTYPE_AKM_ATTR_LAST - 1, }; +/** + * enum nl80211_fils_discovery_attributes - FILS discovery configuration + * from IEEE Std 802.11ai-2016, Annex C.3 MIB detail. + * + * @__NL80211_FILS_DISCOVERY_INVALID: Invalid + * + * @NL80211_FILS_DISCOVERY_INT_MIN: Minimum packet interval (u32, TU). + * Allowed range: 0..10000 (TU = Time Unit) + * @NL80211_FILS_DISCOVERY_INT_MAX: Maximum packet interval (u32, TU). + * Allowed range: 0..10000 (TU = Time Unit) + * @NL80211_FILS_DISCOVERY_TMPL: Optional FILS discovery template. + * It has contents of IEEE Std 802.11ai-2016 9.6.8.36 FILS discovery frame + * (Figure 9-687a). + * It may include 6GHz specific data specified in IEEE P802.11ax/D6.0, + * 9.6.7.36 FILS Discovery frame format. + * + * @__NL80211_FILS_DISCOVERY_LAST: Internal + * @NL80211_FILS_DISCOVERY_MAX: highest attribute + */ +enum nl80211_fils_discovery_attributes { + __NL80211_FILS_DISCOVERY_INVALID, + + NL80211_FILS_DISCOVERY_INT_MIN, + NL80211_FILS_DISCOVERY_INT_MAX, + NL80211_FILS_DISCOVERY_TMPL, + + /* keep last */ + __NL80211_FILS_DISCOVERY_LAST, + NL80211_FILS_DISCOVERY_MAX = __NL80211_FILS_DISCOVERY_LAST - 1 +}; #endif /* __LINUX_NL80211_H */ diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 263ae395ad44..89f787dc2421 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -360,6 +360,14 @@ nl80211_tid_config_attr_policy[NL80211_TID_CONFIG_ATTR_MAX + 1] = { NLA_POLICY_NESTED(nl80211_txattr_policy), }; +static const struct nla_policy +fils_discovery_policy[NL80211_FILS_DISCOVERY_MAX + 1] = { + [NL80211_FILS_DISCOVERY_INT_MIN] = NLA_POLICY_MAX(NLA_U32, 10000), + [NL80211_FILS_DISCOVERY_INT_MAX] = NLA_POLICY_MAX(NLA_U32, 10000), + [NL80211_FILS_DISCOVERY_TMPL] = { .type = NLA_BINARY, + .len = IEEE80211_MAX_DATA_LEN } +}; + static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { [0] = { .strict_start_type = NL80211_ATTR_HE_OBSS_PD }, [NL80211_ATTR_WIPHY] = { .type = NLA_U32 }, @@ -658,6 +666,8 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { .type = NLA_EXACT_LEN, .len = sizeof(struct ieee80211_he_6ghz_capa), }, + [NL80211_ATTR_FILS_DISCOVERY] = + NLA_POLICY_NESTED(fils_discovery_policy), }; /* policy for the key attributes */ @@ -4721,6 +4731,35 @@ static int nl80211_parse_he_bss_color(struct nlattr *attrs, return 0; } +static int nl80211_parse_fils_discovery(struct nlattr *attrs, + struct cfg80211_ap_settings *params) +{ + struct nlattr *tmpl; + struct nlattr *tb[NL80211_FILS_DISCOVERY_MAX + 1]; + int ret; + struct cfg80211_fils_discovery *fd = ¶ms->fils_discovery; + + ret = nla_parse_nested(tb, NL80211_FILS_DISCOVERY_MAX, attrs, + fils_discovery_policy, NULL); + if (ret) + return ret; + + if (!tb[NL80211_FILS_DISCOVERY_INT_MIN] || + !tb[NL80211_FILS_DISCOVERY_INT_MAX]) + return -EINVAL; + + fd->min_interval = nla_get_u32(tb[NL80211_FILS_DISCOVERY_INT_MIN]); + fd->max_interval = nla_get_u32(tb[NL80211_FILS_DISCOVERY_INT_MAX]); + + tmpl = tb[NL80211_FILS_DISCOVERY_TMPL]; + if (tmpl) { + fd->tmpl = nla_data(tmpl); + fd->tmpl_len = nla_len(tmpl); + } + + return 0; +} + static void nl80211_check_ap_rate_selectors(struct cfg80211_ap_settings *params, const u8 *rates) { @@ -5027,6 +5066,13 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info) return err; } + if (info->attrs[NL80211_ATTR_FILS_DISCOVERY]) { + err = nl80211_parse_fils_discovery(info->attrs[NL80211_ATTR_FILS_DISCOVERY], + ¶ms); + if (err) + return err; + } + nl80211_calculate_ap_params(¶ms); if (info->attrs[NL80211_ATTR_EXTERNAL_AUTH_SUPPORT])
FILS discovery attribute, NL80211_ATTR_FILS_DISCOVERY, is nested which supports following parameters (IEEE Std 802.11ai-2016, Annex C.3 MIB detail): (1) NL80211_FILS_DISCOVERY_INT_MIN - Minimum packet interval (2) NL80211_FILS_DISCOVERY_INT_MAX - Maximum packet interval (3) NL0211_FILS_DISCOVERY_TMPL - Template data Signed-off-by: Aloka Dixit <alokad@codeaurora.org> --- v4: Removed attributes unrelated to FILS discovery - NL80211_FILS_DISCOVERY_BCN_RESP_WIN, NL80211_FILS_DISCOVERY_OMIT_REPLICATE_PROBE_RESP and member named enabled in struct cfg80211_fils_discovery. include/net/cfg80211.h | 19 +++++++++++++++ include/uapi/linux/nl80211.h | 36 ++++++++++++++++++++++++++++ net/wireless/nl80211.c | 46 ++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+)