On Thu, Mar 02, 2023 at 04:03:06PM +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.
>
> Add necessary functionality to allow 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.
>
> In detail, add new mac80211_hwsim attribute HWSIM_ATTR_PMSR_SUPPORT.
> HWSIM_ATTR_PMSR_SUPPORT is used to set PMSR capability when creating a new
> radio. To send extra capability 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@xxxxxxxxxx>
Thanks for your patch, a few comments below.
> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
> index 4cc4eaf80b14..79476d55c1ca 100644
> --- a/drivers/net/wireless/mac80211_hwsim.c
> +++ b/drivers/net/wireless/mac80211_hwsim.c
...
> @@ -3186,6 +3218,7 @@ struct hwsim_new_radio_params {
> u32 *ciphers;
> u8 n_ciphers;
> bool mlo;
> + const struct cfg80211_pmsr_capabilities *pmsr_capa;
nit: not related to this patch,
but there are lots of holes in hwsim_new_radio_params.
And, I think that all fields, other than the new pmsr_capa field,
could fit into one cacheline on x86_64.
I'm unsure if it is worth cleaning up or not.
> };
>
> static void hwsim_mcast_config_msg(struct sk_buff *mcast_skb,
> @@ -3260,7 +3293,7 @@ static int append_radio_msg(struct sk_buff *skb, int id,
> return ret;
> }
>
> - return 0;
> + return ret;
This change seems unrelated to the rest of the patch.
> }
>
> static void hwsim_mcast_new_radio(int id, struct genl_info *info,
...
> +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);
WARN_ON doesn't seem right here. I suspect that the following is more fitting.
NL_SET_ERR_MSG_ATTR(...);
return -EINVAL;
> + }
> + }
> + return 0;
> +}
> +
...
