On Tue, 2023-02-07 at 08:54 +0000, Jaewan Kim wrote:
> PMSR (a.k.a. peer measurement) is generalized measurement between two
> devices with Wi-Fi support. And currently FTM (a.k.a. fine time measurement
> or flight time measurement) is the one and only measurement.
>
> This change allows mac80211_hwsim to report PMSR result. The PMSR result
[...]
> This change adds HWSIM_CMD_REPORT_PMSR, HWSIM_ATTR_PMSR_RESULT. When
[...]
> To help receive the PMSR result detail, this CL also adds nla policies to
Generally you don't really need to talk about "this change" (or "this
CL"), just say it:
Add the necessarily functionality to allow hwsim to report PMSR
results. [...]
To do that, add new hwsim netlink attributes ..._PMSR and
..._PMSR_RESULT.
(and I'd just remove the language about policies, that's obvious anyway)
> + if (tb[NL80211_PMSR_PEER_ATTR_RESP]) {
> + ret = mac80211_hwsim_parse_pmsr_resp(tb[NL80211_PMSR_PEER_ATTR_RESP], result, info);
I didn't comment about long lines again, but this one stands out :)
> @@ -5758,7 +6118,7 @@ static struct genl_family hwsim_genl_family __ro_after_init = {
> .module = THIS_MODULE,
> .small_ops = hwsim_ops,
> .n_small_ops = ARRAY_SIZE(hwsim_ops),
> - .resv_start_op = HWSIM_CMD_DEL_MAC_ADDR + 1,
> + .resv_start_op = __HWSIM_CMD_MAX,
No. This was intentional that way. If this causes your new userspace
trouble, please adjust it. Clearly, it's new userspace implementation -
that's why the resv_start_op is set that way, to force stricter
validation on new ops that userspace cannot have implemented since they
didn't exist at the time we set this.
johannes
