| Message ID | 20230207085400.2232544-2-jaewan@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | mac80211_hwsim: Add PMSR support | expand |
On Tue, 2023-02-07 at 08:53 +0000, Jaewan Kim wrote: > PMSR (a.k.a. peer measurement) is generalized measurement between two > Wi-Fi devices. And currently FTM (a.k.a. fine time measurement or flight > time measurement) is the one and only measurement. FTM is measured by > RTT (a.k.a. round trip time) of packets between two Wi-Fi devices. > > This change allows mac80211_hwsim to be configured with PMSR capability. > The capability is mandatory to accept incoming PMSR request because > nl80211_pmsr_start() ignores incoming the request without the PMSR > capability. > > This change adds HWSIM_ATTR_PMSR_SUPPORT, which is used to set PMSR > capability when creating a new radio. To send extra details, > HWSIM_ATTR_PMSR_SUPPORT can have nested PMSR capability attributes defined > in the nl80211.h. Data format is the same as cfg80211_pmsr_capabilities. > > If HWSIM_ATTR_PMSR_SUPPORT is specified, mac80211_hwsim builds > cfg80211_pmsr_capabilities and sets wiphy.pmsr_capa. I feel kind of bad for even still commenting on v7 already ... :) Sorry I didn't pay much attention to this before. > +static const struct nla_policy > +hwsim_ftm_capa_policy[NL80211_PMSR_FTM_CAPA_ATTR_MAX + 1] = { This feels a bit iffy to have here, but I guess it's better that defining new attributes for all this over and over again. > + [NL80211_PMSR_FTM_CAPA_ATTR_ASAP] = { .type = NLA_FLAG }, > + [NL80211_PMSR_FTM_CAPA_ATTR_NON_ASAP] = { .type = NLA_FLAG }, > + [NL80211_PMSR_FTM_CAPA_ATTR_REQ_LCI] = { .type = NLA_FLAG }, > + [NL80211_PMSR_FTM_CAPA_ATTR_REQ_CIVICLOC] = { .type = NLA_FLAG }, > + [NL80211_PMSR_FTM_CAPA_ATTR_PREAMBLES] = { .type = NLA_U32 }, > + [NL80211_PMSR_FTM_CAPA_ATTR_BANDWIDTHS] = { .type = NLA_U32 }, > + [NL80211_PMSR_FTM_CAPA_ATTR_MAX_BURSTS_EXPONENT] = NLA_POLICY_MAX(NLA_U8, 15), > + [NL80211_PMSR_FTM_CAPA_ATTR_MAX_FTMS_PER_BURST] = NLA_POLICY_MAX(NLA_U8, 31), Could add some line-breaks where it's easy to stay below 80 columns. Not a hard rule, but still reads nicer. > + if (param->pmsr_capa) > + ret = cfg80211_send_pmsr_capa(param->pmsr_capa, skb); I'm not a fan of exporting this function to drivers - it feels odd. It's also not really needed, since once the new radio exists the user can query it through cfg80211. I'd just remove this part, along with the changes in include/ and net/ > @@ -4445,6 +4481,8 @@ static int mac80211_hwsim_new_radio(struct genl_info *info, > NL80211_EXT_FEATURE_MULTICAST_REGISTRATIONS); > wiphy_ext_feature_set(hw->wiphy, > NL80211_EXT_FEATURE_BEACON_RATE_LEGACY); > + wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER); > + > no need for the extra blank line. > +static int parse_ftm_capa(const struct nlattr *ftm_capa, struct cfg80211_pmsr_capabilities *out, > + struct genl_info *info) That line also got really long, unnecessarily. > +{ > + struct nlattr *tb[NL80211_PMSR_FTM_CAPA_ATTR_MAX + 1]; > + int ret = nla_parse_nested(tb, NL80211_PMSR_FTM_CAPA_ATTR_MAX, > + ftm_capa, hwsim_ftm_capa_policy, NULL); should have a blank line here I guess. > +static int parse_pmsr_capa(const struct nlattr *pmsr_capa, struct cfg80211_pmsr_capabilities *out, > + struct genl_info *info) > +{ > + struct nlattr *tb[NL80211_PMSR_ATTR_MAX + 1]; > + struct nlattr *nla; > + int size; > + int ret = nla_parse_nested(tb, NL80211_PMSR_ATTR_MAX, pmsr_capa, > + hwsim_pmsr_capa_policy, NULL); > + if (ret) { same here for both of those comments > > + if (info->attrs[HWSIM_ATTR_PMSR_SUPPORT]) { > + struct cfg80211_pmsr_capabilities *pmsr_capa = > + kmalloc(sizeof(struct cfg80211_pmsr_capabilities), GFP_KERNEL); sizeof(*pmsr_capa), also makes that line a lot shorter > + * @HWSIM_ATTR_PMSR_SUPPORT: claim peer measurement support This should probably explain that it's nested, and what should be inside of it. johannes
On Thu, Feb 16, 2023 at 3:01 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Tue, 2023-02-07 at 08:53 +0000, Jaewan Kim wrote: > > PMSR (a.k.a. peer measurement) is generalized measurement between two > > Wi-Fi devices. And currently FTM (a.k.a. fine time measurement or flight > > time measurement) is the one and only measurement. FTM is measured by > > RTT (a.k.a. round trip time) of packets between two Wi-Fi devices. > > > > This change allows mac80211_hwsim to be configured with PMSR capability. > > The capability is mandatory to accept incoming PMSR request because > > nl80211_pmsr_start() ignores incoming the request without the PMSR > > capability. > > > > This change adds HWSIM_ATTR_PMSR_SUPPORT, which is used to set PMSR > > capability when creating a new radio. To send extra details, > > HWSIM_ATTR_PMSR_SUPPORT can have nested PMSR capability attributes defined > > in the nl80211.h. Data format is the same as cfg80211_pmsr_capabilities. > > > > If HWSIM_ATTR_PMSR_SUPPORT is specified, mac80211_hwsim builds > > cfg80211_pmsr_capabilities and sets wiphy.pmsr_capa. > > I feel kind of bad for even still commenting on v7 already ... :) > > Sorry I didn't pay much attention to this before. > > > > +static const struct nla_policy > > +hwsim_ftm_capa_policy[NL80211_PMSR_FTM_CAPA_ATTR_MAX + 1] = { > > This feels a bit iffy to have here, but I guess it's better that > defining new attributes for all this over and over again. I'm sorry but could you rephrase what you expect here? Are you suggesting to define new sets of HWSIM_PMSR_* enums instead of using existing enums NL80211_PMSR_*? > > > + [NL80211_PMSR_FTM_CAPA_ATTR_ASAP] = { .type = NLA_FLAG }, > > + [NL80211_PMSR_FTM_CAPA_ATTR_NON_ASAP] = { .type = NLA_FLAG }, > > + [NL80211_PMSR_FTM_CAPA_ATTR_REQ_LCI] = { .type = NLA_FLAG }, > > + [NL80211_PMSR_FTM_CAPA_ATTR_REQ_CIVICLOC] = { .type = NLA_FLAG }, > > + [NL80211_PMSR_FTM_CAPA_ATTR_PREAMBLES] = { .type = NLA_U32 }, > > + [NL80211_PMSR_FTM_CAPA_ATTR_BANDWIDTHS] = { .type = NLA_U32 }, > > + [NL80211_PMSR_FTM_CAPA_ATTR_MAX_BURSTS_EXPONENT] = NLA_POLICY_MAX(NLA_U8, 15), > > + [NL80211_PMSR_FTM_CAPA_ATTR_MAX_FTMS_PER_BURST] = NLA_POLICY_MAX(NLA_U8, 31), > > Could add some line-breaks where it's easy to stay below 80 columns. Not > a hard rule, but still reads nicer. > > > + if (param->pmsr_capa) > > + ret = cfg80211_send_pmsr_capa(param->pmsr_capa, skb); > > I'm not a fan of exporting this function to drivers - it feels odd. It's > also not really needed, since once the new radio exists the user can > query it through cfg80211. I'd just remove this part, along with the > changes in include/ and net/ > > > @@ -4445,6 +4481,8 @@ static int mac80211_hwsim_new_radio(struct genl_info *info, > > NL80211_EXT_FEATURE_MULTICAST_REGISTRATIONS); > > wiphy_ext_feature_set(hw->wiphy, > > NL80211_EXT_FEATURE_BEACON_RATE_LEGACY); > > + wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER); > > + > > > > no need for the extra blank line. > > > +static int parse_ftm_capa(const struct nlattr *ftm_capa, struct cfg80211_pmsr_capabilities *out, > > + struct genl_info *info) > > That line also got really long, unnecessarily. > > > +{ > > + struct nlattr *tb[NL80211_PMSR_FTM_CAPA_ATTR_MAX + 1]; > > + int ret = nla_parse_nested(tb, NL80211_PMSR_FTM_CAPA_ATTR_MAX, > > + ftm_capa, hwsim_ftm_capa_policy, NULL); > > should have a blank line here I guess. > > > +static int parse_pmsr_capa(const struct nlattr *pmsr_capa, struct cfg80211_pmsr_capabilities *out, > > + struct genl_info *info) > > +{ > > + struct nlattr *tb[NL80211_PMSR_ATTR_MAX + 1]; > > + struct nlattr *nla; > > + int size; > > + int ret = nla_parse_nested(tb, NL80211_PMSR_ATTR_MAX, pmsr_capa, > > + hwsim_pmsr_capa_policy, NULL); > > + if (ret) { > > same here for both of those comments > > > > > + if (info->attrs[HWSIM_ATTR_PMSR_SUPPORT]) { > > + struct cfg80211_pmsr_capabilities *pmsr_capa = > > + kmalloc(sizeof(struct cfg80211_pmsr_capabilities), GFP_KERNEL); > > sizeof(*pmsr_capa), also makes that line a lot shorter > > > + * @HWSIM_ATTR_PMSR_SUPPORT: claim peer measurement support > > This should probably explain that it's nested, and what should be inside > of it. > > johannes Dear Johannes Berg, First of all, thank you for the review. I left a question for clarification. I'll send another patchset when I address your feedback correctly. BTW, can I expect you to review my changes for further patchsets? I sometimes get conflicting opinions (e.g. line limits) so it would be a great help if you take a look at my changes. -- Jaewan Kim (김재완) | Software Engineer in Google Korea | jaewan@google.com | +82-10-2781-5078
On Fri, Feb 17, 2023 at 02:11:38PM +0900, Jaewan Kim wrote: > BTW, can I expect you to review my changes for further patchsets? > I sometimes get conflicting opinions (e.g. line limits) Sorry, I was the one that said "you can use 100 columns", if that's not ok in the networking subsystem yet, that was my fault as it's been that way in other parts of the kernel tree for a while. > so it would be a great help if you take a look at my changes. Why not help out and start reviewing other people's changes? To only ask for others to do work for you isn't the easiest way to get that work done :) thanks, greg k-h
On Fri, 2023-02-17 at 08:43 +0100, Greg KH wrote: > On Fri, Feb 17, 2023 at 02:11:38PM +0900, Jaewan Kim wrote: > > BTW, can I expect you to review my changes for further patchsets? > > I sometimes get conflicting opinions (e.g. line limits) > > Sorry, I was the one that said "you can use 100 columns", if that's not > ok in the networking subsystem yet, that was my fault as it's been that > way in other parts of the kernel tree for a while. > Hah. Maybe that's my mistake then, I was still at "use 80 columns where it's simple, and more if it would look worse" ... I don't really mind the longer lines that much personally :) johannes
On Fri, 2023-02-17 at 14:11 +0900, Jaewan Kim wrote: > > > > > +static const struct nla_policy > > > +hwsim_ftm_capa_policy[NL80211_PMSR_FTM_CAPA_ATTR_MAX + 1] = { > > > > This feels a bit iffy to have here, but I guess it's better that > > defining new attributes for all this over and over again. > > I'm sorry but could you rephrase what you expect here? > Are you suggesting to define new sets of HWSIM_PMSR_* enums > instead of using existing enums NL80211_PMSR_*? No, I was just drive-by commenting on this. Given all the options this feels like it's probably the best one :-) > BTW, can I expect you to review my changes for further patchsets? > I sometimes get conflicting opinions (e.g. line limits) Sorry about that. See my other mail. I'm happy to accept it as it is. > so it would be a great help if you take a look at my changes. > I'll be the one applying the patches, so yes. johannes
On Fri, Feb 17, 2023 at 10:13:08AM +0100, Johannes Berg wrote: > On Fri, 2023-02-17 at 08:43 +0100, Greg KH wrote: > > On Fri, Feb 17, 2023 at 02:11:38PM +0900, Jaewan Kim wrote: > > > BTW, can I expect you to review my changes for further patchsets? > > > I sometimes get conflicting opinions (e.g. line limits) > > > > Sorry, I was the one that said "you can use 100 columns", if that's not > > ok in the networking subsystem yet, that was my fault as it's been that > > way in other parts of the kernel tree for a while. > > > > Hah. Maybe that's my mistake then, I was still at "use 80 columns where > it's simple, and more if it would look worse" ... It was changed back in 2020: bdc48fa11e46 ("checkpatch/coding-style: deprecate 80-column warning") seems to take a while to propagate out to all the subsystems :)
On Fri, 2023-02-17 at 10:31 +0100, Greg KH wrote: > On Fri, Feb 17, 2023 at 10:13:08AM +0100, Johannes Berg wrote: > > On Fri, 2023-02-17 at 08:43 +0100, Greg KH wrote: > > > On Fri, Feb 17, 2023 at 02:11:38PM +0900, Jaewan Kim wrote: > > > > BTW, can I expect you to review my changes for further patchsets? > > > > I sometimes get conflicting opinions (e.g. line limits) > > > > > > Sorry, I was the one that said "you can use 100 columns", if that's not > > > ok in the networking subsystem yet, that was my fault as it's been that > > > way in other parts of the kernel tree for a while. > > > > > > > Hah. Maybe that's my mistake then, I was still at "use 80 columns where > > it's simple, and more if it would look worse" ... > > It was changed back in 2020: > bdc48fa11e46 ("checkpatch/coding-style: deprecate 80-column warning") > > seems to take a while to propagate out to all the subsystems :) Ah no, I was aware of that, but I guess we interpret this bit differently: +Statements longer than 80 columns should be broken into sensible chunks, +unless exceeding 80 columns significantly increases readability and does +not hide information. Here, I would've said something like: + if (request->request_lci && nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_REQUEST_LCI)) + return -ENOBUFS; can indeed "be broken into sensible chunks, unless ..." Just like this one already did: + if (request->request_civicloc && + nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_REQUEST_CIVICLOC)) + return -ENOBUFS; Personally I think the latter is easier to read because scanning the long line for the logical break at "&&" is harder for me, but YMMV. johannes
On Fri, Feb 17, 2023 at 10:34:32AM +0100, Johannes Berg wrote: > On Fri, 2023-02-17 at 10:31 +0100, Greg KH wrote: > > On Fri, Feb 17, 2023 at 10:13:08AM +0100, Johannes Berg wrote: > > > On Fri, 2023-02-17 at 08:43 +0100, Greg KH wrote: > > > > On Fri, Feb 17, 2023 at 02:11:38PM +0900, Jaewan Kim wrote: > > > > > BTW, can I expect you to review my changes for further patchsets? > > > > > I sometimes get conflicting opinions (e.g. line limits) > > > > > > > > Sorry, I was the one that said "you can use 100 columns", if that's not > > > > ok in the networking subsystem yet, that was my fault as it's been that > > > > way in other parts of the kernel tree for a while. > > > > > > > > > > Hah. Maybe that's my mistake then, I was still at "use 80 columns where > > > it's simple, and more if it would look worse" ... > > > > It was changed back in 2020: > > bdc48fa11e46 ("checkpatch/coding-style: deprecate 80-column warning") > > > > seems to take a while to propagate out to all the subsystems :) > > Ah no, I was aware of that, but I guess we interpret this bit > differently: > > +Statements longer than 80 columns should be broken into sensible chunks, > +unless exceeding 80 columns significantly increases readability and does > +not hide information. > > > Here, I would've said something like: > > + if (request->request_lci && nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_REQUEST_LCI)) > + return -ENOBUFS; > > can indeed "be broken into sensible chunks, unless ..." > > Just like this one already did: > > + if (request->request_civicloc && > + nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_REQUEST_CIVICLOC)) > + return -ENOBUFS; > > > Personally I think the latter is easier to read because scanning the > long line for the logical break at "&&" is harder for me, but YMMV. I think the latter is also better, so all is good :)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c index c57c8903b7c0..eca559e63d27 100644 --- a/drivers/net/wireless/mac80211_hwsim.c +++ b/drivers/net/wireless/mac80211_hwsim.c @@ -719,6 +719,9 @@ struct mac80211_hwsim_data { /* RSSI in rx status of the receiver */ int rx_rssi; + /* only used when pmsr capability is supplied */ + struct cfg80211_pmsr_capabilities pmsr_capa; + struct mac80211_hwsim_link_data link_data[IEEE80211_MLD_MAX_NUM_LINKS]; }; @@ -760,6 +763,34 @@ static const struct genl_multicast_group hwsim_mcgrps[] = { /* MAC80211_HWSIM netlink policy */ +static const struct nla_policy +hwsim_ftm_capa_policy[NL80211_PMSR_FTM_CAPA_ATTR_MAX + 1] = { + [NL80211_PMSR_FTM_CAPA_ATTR_ASAP] = { .type = NLA_FLAG }, + [NL80211_PMSR_FTM_CAPA_ATTR_NON_ASAP] = { .type = NLA_FLAG }, + [NL80211_PMSR_FTM_CAPA_ATTR_REQ_LCI] = { .type = NLA_FLAG }, + [NL80211_PMSR_FTM_CAPA_ATTR_REQ_CIVICLOC] = { .type = NLA_FLAG }, + [NL80211_PMSR_FTM_CAPA_ATTR_PREAMBLES] = { .type = NLA_U32 }, + [NL80211_PMSR_FTM_CAPA_ATTR_BANDWIDTHS] = { .type = NLA_U32 }, + [NL80211_PMSR_FTM_CAPA_ATTR_MAX_BURSTS_EXPONENT] = NLA_POLICY_MAX(NLA_U8, 15), + [NL80211_PMSR_FTM_CAPA_ATTR_MAX_FTMS_PER_BURST] = NLA_POLICY_MAX(NLA_U8, 31), + [NL80211_PMSR_FTM_CAPA_ATTR_TRIGGER_BASED] = { .type = NLA_FLAG }, + [NL80211_PMSR_FTM_CAPA_ATTR_NON_TRIGGER_BASED] = { .type = NLA_FLAG }, +}; + +static const struct nla_policy +hwsim_pmsr_capa_type_policy[NL80211_PMSR_TYPE_MAX + 1] = { + [NL80211_PMSR_TYPE_FTM] = NLA_POLICY_NESTED(hwsim_ftm_capa_policy), +}; + +static const struct nla_policy +hwsim_pmsr_capa_policy[NL80211_PMSR_ATTR_MAX + 1] = { + [NL80211_PMSR_ATTR_MAX_PEERS] = { .type = NLA_U32 }, + [NL80211_PMSR_ATTR_REPORT_AP_TSF] = { .type = NLA_FLAG }, + [NL80211_PMSR_ATTR_RANDOMIZE_MAC_ADDR] = { .type = NLA_FLAG }, + [NL80211_PMSR_ATTR_TYPE_CAPA] = NLA_POLICY_NESTED(hwsim_pmsr_capa_type_policy), + [NL80211_PMSR_ATTR_PEERS] = { .type = NLA_REJECT }, // only for request. +}; + static const struct nla_policy hwsim_genl_policy[HWSIM_ATTR_MAX + 1] = { [HWSIM_ATTR_ADDR_RECEIVER] = NLA_POLICY_ETH_ADDR_COMPAT, [HWSIM_ATTR_ADDR_TRANSMITTER] = NLA_POLICY_ETH_ADDR_COMPAT, @@ -788,6 +819,7 @@ static const struct nla_policy hwsim_genl_policy[HWSIM_ATTR_MAX + 1] = { [HWSIM_ATTR_IFTYPE_SUPPORT] = { .type = NLA_U32 }, [HWSIM_ATTR_CIPHER_SUPPORT] = { .type = NLA_BINARY }, [HWSIM_ATTR_MLO_SUPPORT] = { .type = NLA_FLAG }, + [HWSIM_ATTR_PMSR_SUPPORT] = NLA_POLICY_NESTED(hwsim_pmsr_capa_policy), }; #if IS_REACHABLE(CONFIG_VIRTIO) @@ -3186,6 +3218,7 @@ struct hwsim_new_radio_params { u32 *ciphers; u8 n_ciphers; bool mlo; + const struct cfg80211_pmsr_capabilities *pmsr_capa; }; static void hwsim_mcast_config_msg(struct sk_buff *mcast_skb, @@ -3260,7 +3293,10 @@ static int append_radio_msg(struct sk_buff *skb, int id, return ret; } - return 0; + if (param->pmsr_capa) + ret = cfg80211_send_pmsr_capa(param->pmsr_capa, skb); + + return ret; } static void hwsim_mcast_new_radio(int id, struct genl_info *info, @@ -4445,6 +4481,8 @@ static int mac80211_hwsim_new_radio(struct genl_info *info, NL80211_EXT_FEATURE_MULTICAST_REGISTRATIONS); wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_BEACON_RATE_LEGACY); + wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER); + hw->wiphy->interface_modes = param->iftypes; @@ -4606,6 +4644,11 @@ static int mac80211_hwsim_new_radio(struct genl_info *info, data->debugfs, data, &hwsim_simulate_radar); + if (param->pmsr_capa) { + data->pmsr_capa = *param->pmsr_capa; + hw->wiphy->pmsr_capa = &data->pmsr_capa; + } + spin_lock_bh(&hwsim_radio_lock); err = rhashtable_insert_fast(&hwsim_radios_rht, &data->rht, hwsim_rht_params); @@ -4715,6 +4758,7 @@ static int mac80211_hwsim_get_radio(struct sk_buff *skb, param.regd = data->regd; param.channels = data->channels; param.hwname = wiphy_name(data->hw->wiphy); + param.pmsr_capa = &data->pmsr_capa; res = append_radio_msg(skb, data->idx, ¶m); if (res < 0) @@ -5053,6 +5097,74 @@ static bool hwsim_known_ciphers(const u32 *ciphers, int n_ciphers) return true; } +static int parse_ftm_capa(const struct nlattr *ftm_capa, struct cfg80211_pmsr_capabilities *out, + struct genl_info *info) +{ + struct nlattr *tb[NL80211_PMSR_FTM_CAPA_ATTR_MAX + 1]; + int ret = nla_parse_nested(tb, NL80211_PMSR_FTM_CAPA_ATTR_MAX, + ftm_capa, hwsim_ftm_capa_policy, NULL); + if (ret) { + NL_SET_ERR_MSG_ATTR(info->extack, ftm_capa, "malformed FTM capability"); + return -EINVAL; + } + + out->ftm.supported = 1; + if (tb[NL80211_PMSR_FTM_CAPA_ATTR_PREAMBLES]) + out->ftm.preambles = nla_get_u32(tb[NL80211_PMSR_FTM_CAPA_ATTR_PREAMBLES]); + if (tb[NL80211_PMSR_FTM_CAPA_ATTR_BANDWIDTHS]) + out->ftm.bandwidths = nla_get_u32(tb[NL80211_PMSR_FTM_CAPA_ATTR_BANDWIDTHS]); + if (tb[NL80211_PMSR_FTM_CAPA_ATTR_MAX_BURSTS_EXPONENT]) + out->ftm.max_bursts_exponent = + nla_get_u8(tb[NL80211_PMSR_FTM_CAPA_ATTR_MAX_BURSTS_EXPONENT]); + if (tb[NL80211_PMSR_FTM_CAPA_ATTR_MAX_FTMS_PER_BURST]) + out->ftm.max_ftms_per_burst = + nla_get_u8(tb[NL80211_PMSR_FTM_CAPA_ATTR_MAX_FTMS_PER_BURST]); + out->ftm.asap = !!tb[NL80211_PMSR_FTM_CAPA_ATTR_ASAP]; + out->ftm.non_asap = !!tb[NL80211_PMSR_FTM_CAPA_ATTR_NON_ASAP]; + out->ftm.request_lci = !!tb[NL80211_PMSR_FTM_CAPA_ATTR_REQ_LCI]; + out->ftm.request_civicloc = !!tb[NL80211_PMSR_FTM_CAPA_ATTR_REQ_CIVICLOC]; + out->ftm.trigger_based = !!tb[NL80211_PMSR_FTM_CAPA_ATTR_TRIGGER_BASED]; + out->ftm.non_trigger_based = !!tb[NL80211_PMSR_FTM_CAPA_ATTR_NON_TRIGGER_BASED]; + + return 0; +} + +static int parse_pmsr_capa(const struct nlattr *pmsr_capa, struct cfg80211_pmsr_capabilities *out, + struct genl_info *info) +{ + struct nlattr *tb[NL80211_PMSR_ATTR_MAX + 1]; + struct nlattr *nla; + int size; + int ret = nla_parse_nested(tb, NL80211_PMSR_ATTR_MAX, pmsr_capa, + hwsim_pmsr_capa_policy, NULL); + if (ret) { + NL_SET_ERR_MSG_ATTR(info->extack, pmsr_capa, "malformed PMSR capability"); + return -EINVAL; + } + + if (tb[NL80211_PMSR_ATTR_MAX_PEERS]) + out->max_peers = nla_get_u32(tb[NL80211_PMSR_ATTR_MAX_PEERS]); + out->report_ap_tsf = !!tb[NL80211_PMSR_ATTR_REPORT_AP_TSF]; + out->randomize_mac_addr = !!tb[NL80211_PMSR_ATTR_RANDOMIZE_MAC_ADDR]; + + if (!tb[NL80211_PMSR_ATTR_TYPE_CAPA]) { + NL_SET_ERR_MSG_ATTR(info->extack, tb[NL80211_PMSR_ATTR_TYPE_CAPA], + "malformed PMSR type"); + return -EINVAL; + } + + nla_for_each_nested(nla, tb[NL80211_PMSR_ATTR_TYPE_CAPA], size) { + switch (nla_type(nla)) { + case NL80211_PMSR_TYPE_FTM: + parse_ftm_capa(nla, out, info); + break; + default: + WARN_ON(1); + } + } + return 0; +} + static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info) { struct hwsim_new_radio_params param = { 0 }; @@ -5173,8 +5285,24 @@ static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info) param.hwname = hwname; } + if (info->attrs[HWSIM_ATTR_PMSR_SUPPORT]) { + struct cfg80211_pmsr_capabilities *pmsr_capa = + kmalloc(sizeof(struct cfg80211_pmsr_capabilities), GFP_KERNEL); + if (!pmsr_capa) { + ret = -ENOMEM; + goto out_free; + } + ret = parse_pmsr_capa(info->attrs[HWSIM_ATTR_PMSR_SUPPORT], pmsr_capa, info); + if (ret) + goto out_free; + param.pmsr_capa = pmsr_capa; + } + ret = mac80211_hwsim_new_radio(info, ¶m); + +out_free: kfree(hwname); + kfree(param.pmsr_capa); return ret; } diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h index 527799b2de0f..81cd02d2555c 100644 --- a/drivers/net/wireless/mac80211_hwsim.h +++ b/drivers/net/wireless/mac80211_hwsim.h @@ -142,6 +142,7 @@ enum { * @HWSIM_ATTR_CIPHER_SUPPORT: u32 array of supported cipher types * @HWSIM_ATTR_MLO_SUPPORT: claim MLO support (exact parameters TBD) for * the new radio + * @HWSIM_ATTR_PMSR_SUPPORT: claim peer measurement support * @__HWSIM_ATTR_MAX: enum limit */ @@ -173,6 +174,7 @@ enum { HWSIM_ATTR_IFTYPE_SUPPORT, HWSIM_ATTR_CIPHER_SUPPORT, HWSIM_ATTR_MLO_SUPPORT, + HWSIM_ATTR_PMSR_SUPPORT, __HWSIM_ATTR_MAX, }; #define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1) diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 03d4f4deadae..33f775b0f0b0 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -8751,6 +8751,16 @@ void cfg80211_pmsr_complete(struct wireless_dev *wdev, struct cfg80211_pmsr_request *req, gfp_t gfp); +/** + * cfg80211_send_pmsr_capa - send the pmsr capabilities. + * @cap: peer measurement capabilities + * @skb: The skb to send pmsr capa + * + * Send the peer measurement capabilities to skb. + */ +int cfg80211_send_pmsr_capa(const struct cfg80211_pmsr_capabilities *cap, + struct sk_buff *msg); + /** * cfg80211_iftype_allowed - check whether the interface can be allowed * @wiphy: the wiphy diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 33a82ecab9d5..ae1924210663 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -2152,10 +2152,9 @@ nl80211_send_pmsr_ftm_capa(const struct cfg80211_pmsr_capabilities *cap, return 0; } -static int nl80211_send_pmsr_capa(struct cfg80211_registered_device *rdev, - struct sk_buff *msg) +int cfg80211_send_pmsr_capa(const struct cfg80211_pmsr_capabilities *cap, + struct sk_buff *msg) { - const struct cfg80211_pmsr_capabilities *cap = rdev->wiphy.pmsr_capa; struct nlattr *pmsr, *caps; if (!cap) @@ -2193,6 +2192,13 @@ static int nl80211_send_pmsr_capa(struct cfg80211_registered_device *rdev, return 0; } +EXPORT_SYMBOL_GPL(cfg80211_send_pmsr_capa); + +static int nl80211_send_pmsr_capa(struct cfg80211_registered_device *rdev, + struct sk_buff *msg) +{ + return cfg80211_send_pmsr_capa(rdev->wiphy.pmsr_capa, msg); +} static int nl80211_put_iftype_akm_suites(struct cfg80211_registered_device *rdev,
PMSR (a.k.a. peer measurement) is generalized measurement between two Wi-Fi devices. And currently FTM (a.k.a. fine time measurement or flight time measurement) is the one and only measurement. FTM is measured by RTT (a.k.a. round trip time) of packets between two Wi-Fi devices. This change allows mac80211_hwsim to be configured with PMSR capability. The capability is mandatory to accept incoming PMSR request because nl80211_pmsr_start() ignores incoming the request without the PMSR capability. This change adds HWSIM_ATTR_PMSR_SUPPORT, which is used to set PMSR capability when creating a new radio. To send extra details, HWSIM_ATTR_PMSR_SUPPORT can have nested PMSR capability attributes defined in the nl80211.h. Data format is the same as cfg80211_pmsr_capabilities. If HWSIM_ATTR_PMSR_SUPPORT is specified, mac80211_hwsim builds cfg80211_pmsr_capabilities and sets wiphy.pmsr_capa. Signed-off-by: Jaewan Kim <jaewan@google.com> --- V6 -> V7: Added terms definitions. Removed pr_*() uses. V5 -> V6: Added per change patch history. V4 -> V5: Fixed style for commit messages. V3 -> V4: Added change details for new attribute, and fixed memory leak. V1 -> V3: Initial commit (includes resends). --- drivers/net/wireless/mac80211_hwsim.c | 130 +++++++++++++++++++++++++- drivers/net/wireless/mac80211_hwsim.h | 2 + include/net/cfg80211.h | 10 ++ net/wireless/nl80211.c | 12 ++- 4 files changed, 150 insertions(+), 4 deletions(-)