Dear Kernel maintainers,
First of all, thank you for spending your precious time for reviewing
my changes, and also sorry for my mistakes in previous patchsets.
Let me propose series of CLs for adding PMSR support in the mac80211_hwsim.
PMSR (peer measurement) is generalized measurement between STAs,
and currently FTM (fine time measurement or flight time measurement)
is the one and only measurement.
FTM measures the RTT (round trip time) and FTM can be used to measure
distances between two STAs. RTT is often referred as 'measuring distance'
as well.
Kernel had already defined protocols for PMSR in the
include/uapi/linux/nl80211.h and relevant parsing/sending code are in the
net/wireless/pmsr.c, but they are only used in intel's iwlwifi driver.
CLs are tested with iw tool on Virtual Android device (a.k.a. Cuttlefish).
Hope this explains my CLs.
Many Thanks,
--
V7 -> V8: Separated CL for exporting nl80211_send_chandef
V6 -> V7: Split 'mac80211_hwsim: handle FTM requests with virtio'
with three pieces
V5 -> V6: Added per CL change history.
V4 -> V5: Fixed style
V3 -> V4: Added detailed explanation to cover letter and per CL commit
messages, includes explanation of PMSR and FTM.
Also fixed memory leak.
V1 -> V3: Initial commits (include resends)
Jaewan Kim (5):
mac80211_hwsim: add PMSR capability support
wifi: nl80211: make nl80211_send_chandef non-static
mac80211_hwsim: add PMSR request support via virtio
mac80211_hwsim: add PMSR abort support via virtio
mac80211_hwsim: add PMSR report support via virtio
drivers/net/wireless/mac80211_hwsim.c | 775 +++++++++++++++++++++++++-
drivers/net/wireless/mac80211_hwsim.h | 58 ++
include/net/cfg80211.h | 9 +
net/wireless/nl80211.c | 4 +-
4 files changed, 834 insertions(+), 12 deletions(-)
--
2.39.2.722.g9855ee24e9-goog
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 <[email protected]>
---
V7 -> V8: Stop sending pmsr_capa when adding new radio to limit
exporting cfg80211 function to driver.
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 | 128 +++++++++++++++++++++++++-
drivers/net/wireless/mac80211_hwsim.h | 3 +
2 files changed, 130 insertions(+), 1 deletion(-)
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
@@ -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,7 @@ static int append_radio_msg(struct sk_buff *skb, int id,
return ret;
}
- return 0;
+ return ret;
}
static void hwsim_mcast_new_radio(int id, struct genl_info *info,
@@ -4445,6 +4478,7 @@ 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 +4640,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 +4754,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 +5093,76 @@ 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 +5283,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(*pmsr_capa), 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..d10fa7f4853b 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -142,6 +142,8 @@ 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: nested attribute used with %HWSIM_CMD_CREATE_RADIO
+ * to provide peer measurement capabilities. (nl80211_peer_measurement_attrs)
* @__HWSIM_ATTR_MAX: enum limit
*/
@@ -173,6 +175,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)
--
2.39.2.722.g9855ee24e9-goog
Expose nl80211_send_chandef functionality for mac80211_hwsim or vendor
netlink can use it where needed.
Signed-off-by: Jaewan Kim <[email protected]>
---
V8: Initial commit (split from other change)
---
include/net/cfg80211.h | 9 +++++++++
net/wireless/nl80211.c | 4 ++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f115b2550309..bcce8e9e2aba 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -938,6 +938,15 @@ int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
const struct cfg80211_chan_def *chandef,
enum nl80211_iftype iftype);
+/**
+ * nl80211_send_chandef - sends the channel definition.
+ * @msg: the msg to send channel definition
+ * @chandef: the channel definition to check
+ *
+ * Returns: 0 if sent the channel definition to msg, < 0 on error
+ **/
+int nl80211_send_chandef(struct sk_buff *msg, const struct cfg80211_chan_def *chandef);
+
/**
* ieee80211_chanwidth_rate_flags - return rate flags for channel width
* @width: the channel width of the channel
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 112b4bb009c8..1fd9e6545225 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3756,8 +3756,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
return result;
}
-static int nl80211_send_chandef(struct sk_buff *msg,
- const struct cfg80211_chan_def *chandef)
+int nl80211_send_chandef(struct sk_buff *msg, const struct cfg80211_chan_def *chandef)
{
if (WARN_ON(!cfg80211_chandef_valid(chandef)))
return -EINVAL;
@@ -3788,6 +3787,7 @@ static int nl80211_send_chandef(struct sk_buff *msg,
return -ENOBUFS;
return 0;
}
+EXPORT_SYMBOL(nl80211_send_chandef);
static int nl80211_send_iface(struct sk_buff *msg, u32 portid, u32 seq, int flags,
struct cfg80211_registered_device *rdev,
--
2.39.2.722.g9855ee24e9-goog
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 functionalities for mac80211_hwsim to start PMSR request by
passthrough the request to wmediumd via virtio. mac80211_hwsim can't
measure RTT for real because mac80211_hwsim the software simulator and
packets are sent almost immediately for real. This change expect wmediumd
to have all the location information of devices, so passthrough requests
to wmediumd.
In detail, add new mac80211_hwsim command HWSIM_CMD_ABORT_PMSR. When
mac80211_hwsim receives the PMSR start request via
ieee80211_ops.start_pmsr, the received cfg80211_pmsr_request is resent to
the wmediumd with command HWSIM_CMD_START_PMSR and attribute
HWSIM_ATTR_PMSR_REQUEST. The attribute is formatted as the same way as
nl80211_pmsr_start() expects.
Signed-off-by: Jaewan Kim <[email protected]>
---
V7->V8: Export nl80211_send_chandef directly and instead of creating
wrapper.
V7: Initial commit (split from previously large patch)
---
drivers/net/wireless/mac80211_hwsim.c | 207 +++++++++++++++++++++++++-
drivers/net/wireless/mac80211_hwsim.h | 6 +
2 files changed, 212 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 79476d55c1ca..691b83140d57 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -721,6 +721,8 @@ struct mac80211_hwsim_data {
/* only used when pmsr capability is supplied */
struct cfg80211_pmsr_capabilities pmsr_capa;
+ struct cfg80211_pmsr_request *pmsr_request;
+ struct wireless_dev *pmsr_request_wdev;
struct mac80211_hwsim_link_data link_data[IEEE80211_MLD_MAX_NUM_LINKS];
};
@@ -3139,6 +3141,208 @@ static int mac80211_hwsim_change_sta_links(struct ieee80211_hw *hw,
return 0;
}
+static int mac80211_hwsim_send_pmsr_ftm_request_peer(struct sk_buff *msg,
+ struct cfg80211_pmsr_ftm_request_peer *request)
+{
+ struct nlattr *ftm;
+
+ if (!request->requested)
+ return -EINVAL;
+
+ ftm = nla_nest_start(msg, NL80211_PMSR_TYPE_FTM);
+ if (!ftm)
+ return -ENOBUFS;
+
+ if (nla_put_u32(msg, NL80211_PMSR_FTM_REQ_ATTR_PREAMBLE, request->preamble))
+ return -ENOBUFS;
+
+ if (nla_put_u16(msg, NL80211_PMSR_FTM_REQ_ATTR_BURST_PERIOD, request->burst_period))
+ return -ENOBUFS;
+
+ if (request->asap && nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_ASAP))
+ return -ENOBUFS;
+
+ if (request->request_lci && nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_REQUEST_LCI))
+ return -ENOBUFS;
+
+ if (request->request_civicloc &&
+ nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_REQUEST_CIVICLOC))
+ return -ENOBUFS;
+
+ if (request->trigger_based && nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_TRIGGER_BASED))
+ return -ENOBUFS;
+
+ if (request->non_trigger_based &&
+ nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_NON_TRIGGER_BASED))
+ return -ENOBUFS;
+
+ if (request->lmr_feedback && nla_put_flag(msg, NL80211_PMSR_FTM_REQ_ATTR_LMR_FEEDBACK))
+ return -ENOBUFS;
+
+ if (nla_put_u8(msg, NL80211_PMSR_FTM_REQ_ATTR_NUM_BURSTS_EXP, request->num_bursts_exp))
+ return -ENOBUFS;
+
+ if (nla_put_u8(msg, NL80211_PMSR_FTM_REQ_ATTR_BURST_DURATION, request->burst_duration))
+ return -ENOBUFS;
+
+ if (nla_put_u8(msg, NL80211_PMSR_FTM_REQ_ATTR_FTMS_PER_BURST, request->ftms_per_burst))
+ return -ENOBUFS;
+
+ if (nla_put_u8(msg, NL80211_PMSR_FTM_REQ_ATTR_NUM_FTMR_RETRIES, request->ftmr_retries))
+ return -ENOBUFS;
+
+ if (nla_put_u8(msg, NL80211_PMSR_FTM_REQ_ATTR_BURST_DURATION, request->burst_duration))
+ return -ENOBUFS;
+
+ if (nla_put_u8(msg, NL80211_PMSR_FTM_REQ_ATTR_BSS_COLOR, request->bss_color))
+ return -ENOBUFS;
+
+ nla_nest_end(msg, ftm);
+
+ return 0;
+}
+
+static int mac80211_hwsim_send_pmsr_request_peer(struct sk_buff *msg,
+ struct cfg80211_pmsr_request_peer *request)
+{
+ struct nlattr *peer, *chandef, *req, *data;
+ int err;
+
+ peer = nla_nest_start(msg, NL80211_PMSR_ATTR_PEERS);
+ if (!peer)
+ return -ENOBUFS;
+
+ if (nla_put(msg, NL80211_PMSR_PEER_ATTR_ADDR, ETH_ALEN,
+ request->addr))
+ return -ENOBUFS;
+
+ chandef = nla_nest_start(msg, NL80211_PMSR_PEER_ATTR_CHAN);
+ if (!chandef)
+ return -ENOBUFS;
+
+ err = nl80211_send_chandef(msg, &request->chandef);
+ if (err)
+ return err;
+
+ nla_nest_end(msg, chandef);
+
+ req = nla_nest_start(msg, NL80211_PMSR_PEER_ATTR_REQ);
+ if (request->report_ap_tsf && nla_put_flag(msg, NL80211_PMSR_REQ_ATTR_GET_AP_TSF))
+ return -ENOBUFS;
+
+ data = nla_nest_start(msg, NL80211_PMSR_REQ_ATTR_DATA);
+ if (!data)
+ return -ENOBUFS;
+
+ err = mac80211_hwsim_send_pmsr_ftm_request_peer(msg, &request->ftm);
+ if (err)
+ return err;
+
+ nla_nest_end(msg, data);
+ nla_nest_end(msg, req);
+ nla_nest_end(msg, peer);
+
+ return 0;
+}
+
+static int mac80211_hwsim_send_pmsr_request(struct sk_buff *msg,
+ struct cfg80211_pmsr_request *request)
+{
+ int err;
+ struct nlattr *pmsr = nla_nest_start(msg, NL80211_ATTR_PEER_MEASUREMENTS);
+
+ if (!pmsr)
+ return -ENOBUFS;
+
+ if (nla_put_u32(msg, NL80211_ATTR_TIMEOUT, request->timeout))
+ return -ENOBUFS;
+
+ if (!is_zero_ether_addr(request->mac_addr)) {
+ if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, request->mac_addr))
+ return -ENOBUFS;
+ if (nla_put(msg, NL80211_ATTR_MAC_MASK, ETH_ALEN, request->mac_addr_mask))
+ return -ENOBUFS;
+ }
+
+ for (int i = 0; i < request->n_peers; i++) {
+ err = mac80211_hwsim_send_pmsr_request_peer(msg, &request->peers[i]);
+ if (err)
+ return err;
+ }
+
+ nla_nest_end(msg, pmsr);
+
+ return 0;
+}
+
+static int mac80211_hwsim_start_pmsr(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct cfg80211_pmsr_request *request)
+{
+ struct mac80211_hwsim_data *data = hw->priv;
+ u32 _portid = READ_ONCE(data->wmediumd);
+ int err = 0;
+ struct sk_buff *skb = NULL;
+ void *msg_head;
+ struct nlattr *pmsr;
+
+ if (!_portid && !hwsim_virtio_enabled)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&data->mutex);
+
+ if (data->pmsr_request) {
+ err = -EBUSY;
+ goto out_err;
+ }
+
+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+
+ if (!skb) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+
+ msg_head = genlmsg_put(skb, 0, 0, &hwsim_genl_family, 0,
+ HWSIM_CMD_START_PMSR);
+
+ if (nla_put(skb, HWSIM_ATTR_ADDR_TRANSMITTER,
+ ETH_ALEN, data->addresses[1].addr)) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+
+ pmsr = nla_nest_start(skb, HWSIM_ATTR_PMSR_REQUEST);
+ if (!pmsr) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+
+ err = mac80211_hwsim_send_pmsr_request(skb, request);
+ if (err)
+ goto out_err;
+
+ nla_nest_end(skb, pmsr);
+
+ genlmsg_end(skb, msg_head);
+ if (hwsim_virtio_enabled)
+ hwsim_tx_virtio(data, skb);
+ else
+ hwsim_unicast_netgroup(data, skb, _portid);
+
+out_err:
+ if (err && skb)
+ nlmsg_free(skb);
+
+ if (!err) {
+ data->pmsr_request = request;
+ data->pmsr_request_wdev = ieee80211_vif_to_wdev(vif);
+ }
+
+ mutex_unlock(&data->mutex);
+ return err;
+}
+
#define HWSIM_COMMON_OPS \
.tx = mac80211_hwsim_tx, \
.wake_tx_queue = ieee80211_handle_wake_tx_queue, \
@@ -3161,7 +3365,8 @@ static int mac80211_hwsim_change_sta_links(struct ieee80211_hw *hw,
.flush = mac80211_hwsim_flush, \
.get_et_sset_count = mac80211_hwsim_get_et_sset_count, \
.get_et_stats = mac80211_hwsim_get_et_stats, \
- .get_et_strings = mac80211_hwsim_get_et_strings,
+ .get_et_strings = mac80211_hwsim_get_et_strings, \
+ .start_pmsr = mac80211_hwsim_start_pmsr, \
#define HWSIM_NON_MLO_OPS \
.sta_add = mac80211_hwsim_sta_add, \
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index d10fa7f4853b..98e586a56582 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -81,6 +81,8 @@ enum hwsim_tx_control_flags {
* to this receiver address for a given station.
* @HWSIM_CMD_DEL_MAC_ADDR: remove the MAC address again, the attributes
* are the same as to @HWSIM_CMD_ADD_MAC_ADDR.
+ * @HWSIM_CMD_START_PMSR: request to start peer measurement with the
+ * %HWSIM_ATTR_PMSR_REQUEST.
* @__HWSIM_CMD_MAX: enum limit
*/
enum {
@@ -93,6 +95,7 @@ enum {
HWSIM_CMD_GET_RADIO,
HWSIM_CMD_ADD_MAC_ADDR,
HWSIM_CMD_DEL_MAC_ADDR,
+ HWSIM_CMD_START_PMSR,
__HWSIM_CMD_MAX,
};
#define HWSIM_CMD_MAX (_HWSIM_CMD_MAX - 1)
@@ -144,6 +147,8 @@ enum {
* the new radio
* @HWSIM_ATTR_PMSR_SUPPORT: nested attribute used with %HWSIM_CMD_CREATE_RADIO
* to provide peer measurement capabilities. (nl80211_peer_measurement_attrs)
+ * @HWSIM_ATTR_PMSR_REQUEST: nested attribute used with %HWSIM_CMD_START_PMSR
+ * to provide details about peer measurement request (nl80211_peer_measurement_attrs)
* @__HWSIM_ATTR_MAX: enum limit
*/
@@ -176,6 +181,7 @@ enum {
HWSIM_ATTR_CIPHER_SUPPORT,
HWSIM_ATTR_MLO_SUPPORT,
HWSIM_ATTR_PMSR_SUPPORT,
+ HWSIM_ATTR_PMSR_REQUEST,
__HWSIM_ATTR_MAX,
};
#define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
--
2.39.2.722.g9855ee24e9-goog
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.
Add necessary functionalities for mac80211_hwsim to abort previous PMSR
request. The abortion request is sent to the wmedium where the PMSR request
is actually handled.
In detail, add new mac80211_hwsim command HWSIM_CMD_ABORT_PMSR. When
mac80211_hwsim receives the PMSR abortion request via
ieee80211_ops.abort_pmsr, the received cfg80211_pmsr_request is resent to
the wmediumd with command HWSIM_CMD_ABORT_PMSR and attribute
HWSIM_ATTR_PMSR_REQUEST. The attribute is formatted as the same way as
nl80211_pmsr_start() expects.
Signed-off-by: Jaewan Kim <[email protected]>
---
V7->V8: Rewrote commit msg
V7: Initial commit (split from previously large patch)
---
drivers/net/wireless/mac80211_hwsim.c | 61 +++++++++++++++++++++++++++
drivers/net/wireless/mac80211_hwsim.h | 2 +
2 files changed, 63 insertions(+)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 691b83140d57..0d92a7e51057 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3343,6 +3343,66 @@ static int mac80211_hwsim_start_pmsr(struct ieee80211_hw *hw,
return err;
}
+static void mac80211_hwsim_abort_pmsr(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct cfg80211_pmsr_request *request)
+{
+ struct mac80211_hwsim_data *data = hw->priv;
+ u32 _portid = READ_ONCE(data->wmediumd);
+ struct sk_buff *skb = NULL;
+ int err = 0;
+ void *msg_head;
+ struct nlattr *pmsr;
+
+ if (!_portid && !hwsim_virtio_enabled)
+ return;
+
+ mutex_lock(&data->mutex);
+
+ if (data->pmsr_request != request) {
+ err = -EINVAL;
+ goto out_err;
+ }
+
+ if (err)
+ return;
+
+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!skb)
+ return;
+
+ msg_head = genlmsg_put(skb, 0, 0, &hwsim_genl_family, 0, HWSIM_CMD_ABORT_PMSR);
+
+ if (nla_put(skb, HWSIM_ATTR_ADDR_TRANSMITTER, ETH_ALEN, data->addresses[1].addr))
+ goto out_err;
+
+ pmsr = nla_nest_start(skb, HWSIM_ATTR_PMSR_REQUEST);
+ if (!pmsr) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+
+ err = mac80211_hwsim_send_pmsr_request(skb, request);
+ if (err)
+ goto out_err;
+
+ err = nla_nest_end(skb, pmsr);
+ if (err)
+ goto out_err;
+
+ genlmsg_end(skb, msg_head);
+ if (hwsim_virtio_enabled)
+ hwsim_tx_virtio(data, skb);
+ else
+ hwsim_unicast_netgroup(data, skb, _portid);
+
+out_err:
+ if (err && skb)
+ nlmsg_free(skb);
+
+ mutex_unlock(&data->mutex);
+}
+
#define HWSIM_COMMON_OPS \
.tx = mac80211_hwsim_tx, \
.wake_tx_queue = ieee80211_handle_wake_tx_queue, \
@@ -3367,6 +3427,7 @@ static int mac80211_hwsim_start_pmsr(struct ieee80211_hw *hw,
.get_et_stats = mac80211_hwsim_get_et_stats, \
.get_et_strings = mac80211_hwsim_get_et_strings, \
.start_pmsr = mac80211_hwsim_start_pmsr, \
+ .abort_pmsr = mac80211_hwsim_abort_pmsr,
#define HWSIM_NON_MLO_OPS \
.sta_add = mac80211_hwsim_sta_add, \
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index 98e586a56582..383f3e39c911 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -83,6 +83,7 @@ enum hwsim_tx_control_flags {
* are the same as to @HWSIM_CMD_ADD_MAC_ADDR.
* @HWSIM_CMD_START_PMSR: request to start peer measurement with the
* %HWSIM_ATTR_PMSR_REQUEST.
+ * @HWSIM_CMD_ABORT_PMSR: abort previously sent peer measurement
* @__HWSIM_CMD_MAX: enum limit
*/
enum {
@@ -96,6 +97,7 @@ enum {
HWSIM_CMD_ADD_MAC_ADDR,
HWSIM_CMD_DEL_MAC_ADDR,
HWSIM_CMD_START_PMSR,
+ HWSIM_CMD_ABORT_PMSR,
__HWSIM_CMD_MAX,
};
#define HWSIM_CMD_MAX (_HWSIM_CMD_MAX - 1)
--
2.39.2.722.g9855ee24e9-goog
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.
Add the necessary functionality to allow mac80211_hwsim to report PMSR
result. The result would come from the wmediumd, where other Wi-Fi
devices' information are kept. mac80211_hwsim only need to deliver the
result to the userspace.
In detail, add new mac80211_hwsim attributes HWSIM_CMD_REPORT_PMSR, and
HWSIM_ATTR_PMSR_RESULT. When mac80211_hwsim receives the PMSR result with
command HWSIM_CMD_REPORT_PMSR and detail with attribute
HWSIM_ATTR_PMSR_RESULT, received data is parsed to cfg80211_pmsr_result and
resent to the userspace by cfg80211_pmsr_report().
To help receive the details of PMSR result, hwsim_rate_info_attributes is
added to receive rate_info without complex bitrate calculation. (i.e. send
rate_info without adding inverse of nl80211_put_sta_rate()).
Signed-off-by: Jaewan Kim <[email protected]>
---
V7->V8: Specify calculated last HWSIM_CMD for resv_start_op instead of
__HWSIM_CMD_MAX for adding new CMD more explicit.
V7: Initial commit (split from previously large patch)
---
drivers/net/wireless/mac80211_hwsim.c | 379 +++++++++++++++++++++++++-
drivers/net/wireless/mac80211_hwsim.h | 51 +++-
2 files changed, 420 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 0d92a7e51057..2a664b7e50d5 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -752,6 +752,11 @@ struct hwsim_radiotap_ack_hdr {
__le16 rt_chbitmask;
} __packed;
+static struct mac80211_hwsim_data *get_hwsim_data_ref_from_addr(const u8 *addr)
+{
+ return rhashtable_lookup_fast(&hwsim_radios_rht, addr, hwsim_rht_params);
+}
+
/* MAC80211_HWSIM netlink family */
static struct genl_family hwsim_genl_family;
@@ -765,6 +770,76 @@ static const struct genl_multicast_group hwsim_mcgrps[] = {
/* MAC80211_HWSIM netlink policy */
+static const struct nla_policy
+hwsim_rate_info_policy[HWSIM_RATE_INFO_ATTR_MAX + 1] = {
+ [HWSIM_RATE_INFO_ATTR_FLAGS] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_MCS] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_LEGACY] = { .type = NLA_U16 },
+ [HWSIM_RATE_INFO_ATTR_NSS] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_BW] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_HE_GI] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_HE_DCM] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_HE_RU_ALLOC] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_N_BOUNDED_CH] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_EHT_GI] = { .type = NLA_U8 },
+ [HWSIM_RATE_INFO_ATTR_EHT_RU_ALLOC] = { .type = NLA_U8 },
+};
+
+static const struct nla_policy
+hwsim_ftm_result_policy[NL80211_PMSR_FTM_RESP_ATTR_MAX + 1] = {
+ [NL80211_PMSR_FTM_RESP_ATTR_FAIL_REASON] = { .type = NLA_U32 },
+ [NL80211_PMSR_FTM_RESP_ATTR_BURST_INDEX] = { .type = NLA_U16 },
+ [NL80211_PMSR_FTM_RESP_ATTR_NUM_FTMR_ATTEMPTS] = { .type = NLA_U32 },
+ [NL80211_PMSR_FTM_RESP_ATTR_NUM_FTMR_SUCCESSES] = { .type = NLA_U32 },
+ [NL80211_PMSR_FTM_RESP_ATTR_BUSY_RETRY_TIME] = { .type = NLA_U8 },
+ [NL80211_PMSR_FTM_RESP_ATTR_NUM_BURSTS_EXP] = { .type = NLA_U8 },
+ [NL80211_PMSR_FTM_RESP_ATTR_BURST_DURATION] = { .type = NLA_U8 },
+ [NL80211_PMSR_FTM_RESP_ATTR_FTMS_PER_BURST] = { .type = NLA_U8 },
+ [NL80211_PMSR_FTM_RESP_ATTR_RSSI_AVG] = { .type = NLA_U32 },
+ [NL80211_PMSR_FTM_RESP_ATTR_RSSI_SPREAD] = { .type = NLA_U32 },
+ [NL80211_PMSR_FTM_RESP_ATTR_TX_RATE] = NLA_POLICY_NESTED(hwsim_rate_info_policy),
+ [NL80211_PMSR_FTM_RESP_ATTR_RX_RATE] = NLA_POLICY_NESTED(hwsim_rate_info_policy),
+ [NL80211_PMSR_FTM_RESP_ATTR_RTT_AVG] = { .type = NLA_U64 },
+ [NL80211_PMSR_FTM_RESP_ATTR_RTT_VARIANCE] = { .type = NLA_U64 },
+ [NL80211_PMSR_FTM_RESP_ATTR_RTT_SPREAD] = { .type = NLA_U64 },
+ [NL80211_PMSR_FTM_RESP_ATTR_DIST_AVG] = { .type = NLA_U64 },
+ [NL80211_PMSR_FTM_RESP_ATTR_DIST_VARIANCE] = { .type = NLA_U64 },
+ [NL80211_PMSR_FTM_RESP_ATTR_DIST_SPREAD] = { .type = NLA_U64 },
+ [NL80211_PMSR_FTM_RESP_ATTR_LCI] = { .type = NLA_STRING },
+ [NL80211_PMSR_FTM_RESP_ATTR_CIVICLOC] = { .type = NLA_STRING },
+};
+
+static const struct nla_policy
+hwsim_pmsr_resp_type_policy[NL80211_PMSR_TYPE_MAX + 1] = {
+ [NL80211_PMSR_TYPE_FTM] = NLA_POLICY_NESTED(hwsim_ftm_result_policy),
+};
+
+static const struct nla_policy
+hwsim_pmsr_resp_policy[NL80211_PMSR_RESP_ATTR_MAX + 1] = {
+ [NL80211_PMSR_RESP_ATTR_STATUS] = { .type = NLA_U32 },
+ [NL80211_PMSR_RESP_ATTR_HOST_TIME] = { .type = NLA_U64 },
+ [NL80211_PMSR_RESP_ATTR_AP_TSF] = { .type = NLA_U64 },
+ [NL80211_PMSR_RESP_ATTR_FINAL] = { .type = NLA_FLAG },
+ [NL80211_PMSR_RESP_ATTR_DATA] = NLA_POLICY_NESTED(hwsim_pmsr_resp_type_policy),
+};
+
+static const struct nla_policy
+hwsim_pmsr_peer_result_policy[NL80211_PMSR_PEER_ATTR_MAX + 1] = {
+ [NL80211_PMSR_PEER_ATTR_ADDR] = NLA_POLICY_ETH_ADDR_COMPAT,
+ [NL80211_PMSR_PEER_ATTR_CHAN] = { .type = NLA_REJECT },
+ [NL80211_PMSR_PEER_ATTR_REQ] = { .type = NLA_REJECT },
+ [NL80211_PMSR_PEER_ATTR_RESP] = NLA_POLICY_NESTED(hwsim_pmsr_resp_policy),
+};
+
+static const struct nla_policy
+hwsim_pmsr_peers_result_policy[NL80211_PMSR_ATTR_MAX + 1] = {
+ [NL80211_PMSR_ATTR_MAX_PEERS] = { .type = NLA_REJECT },
+ [NL80211_PMSR_ATTR_REPORT_AP_TSF] = { .type = NLA_REJECT },
+ [NL80211_PMSR_ATTR_RANDOMIZE_MAC_ADDR] = { .type = NLA_REJECT },
+ [NL80211_PMSR_ATTR_TYPE_CAPA] = { .type = NLA_REJECT },
+ [NL80211_PMSR_ATTR_PEERS] = NLA_POLICY_NESTED_ARRAY(hwsim_pmsr_peer_result_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 },
@@ -822,6 +897,7 @@ static const struct nla_policy hwsim_genl_policy[HWSIM_ATTR_MAX + 1] = {
[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),
+ [HWSIM_ATTR_PMSR_RESULT] = NLA_POLICY_NESTED(hwsim_pmsr_peers_result_policy),
};
#if IS_REACHABLE(CONFIG_VIRTIO)
@@ -3403,6 +3479,292 @@ static void mac80211_hwsim_abort_pmsr(struct ieee80211_hw *hw,
mutex_unlock(&data->mutex);
}
+static int mac80211_hwsim_parse_rate_info(struct nlattr *rateattr,
+ struct rate_info *rate_info,
+ struct genl_info *info)
+{
+ struct nlattr *tb[HWSIM_RATE_INFO_ATTR_MAX + 1];
+ int ret;
+
+ ret = nla_parse_nested(tb, HWSIM_RATE_INFO_ATTR_MAX,
+ rateattr, hwsim_rate_info_policy, info->extack);
+ if (ret)
+ return ret;
+
+ if (tb[HWSIM_RATE_INFO_ATTR_FLAGS])
+ rate_info->flags = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_FLAGS]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_MCS])
+ rate_info->mcs = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_MCS]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_LEGACY])
+ rate_info->legacy = nla_get_u16(tb[HWSIM_RATE_INFO_ATTR_LEGACY]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_NSS])
+ rate_info->nss = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_NSS]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_BW])
+ rate_info->bw = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_BW]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_HE_GI])
+ rate_info->he_gi = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_HE_GI]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_HE_DCM])
+ rate_info->he_dcm = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_HE_DCM]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_HE_RU_ALLOC])
+ rate_info->he_ru_alloc =
+ nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_HE_RU_ALLOC]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_N_BOUNDED_CH])
+ rate_info->n_bonded_ch = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_N_BOUNDED_CH]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_EHT_GI])
+ rate_info->eht_gi = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_EHT_GI]);
+
+ if (tb[HWSIM_RATE_INFO_ATTR_EHT_RU_ALLOC])
+ rate_info->eht_ru_alloc = nla_get_u8(tb[HWSIM_RATE_INFO_ATTR_EHT_RU_ALLOC]);
+
+ return 0;
+}
+
+static int mac80211_hwsim_parse_ftm_result(struct nlattr *ftm,
+ struct cfg80211_pmsr_ftm_result *result,
+ struct genl_info *info)
+{
+ struct nlattr *tb[NL80211_PMSR_FTM_RESP_ATTR_MAX + 1];
+ int ret;
+
+ ret = nla_parse_nested(tb, NL80211_PMSR_FTM_RESP_ATTR_MAX,
+ ftm, hwsim_ftm_result_policy, info->extack);
+ if (ret)
+ return ret;
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_FAIL_REASON])
+ result->failure_reason = nla_get_u32(tb[NL80211_PMSR_FTM_RESP_ATTR_FAIL_REASON]);
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_BURST_INDEX])
+ result->burst_index = nla_get_u16(tb[NL80211_PMSR_FTM_RESP_ATTR_BURST_INDEX]);
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_NUM_FTMR_ATTEMPTS]) {
+ result->num_ftmr_attempts_valid = 1;
+ result->num_ftmr_attempts =
+ nla_get_u32(tb[NL80211_PMSR_FTM_RESP_ATTR_NUM_FTMR_ATTEMPTS]);
+ }
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_NUM_FTMR_SUCCESSES]) {
+ result->num_ftmr_successes_valid = 1;
+ result->num_ftmr_successes =
+ nla_get_u32(tb[NL80211_PMSR_FTM_RESP_ATTR_NUM_FTMR_SUCCESSES]);
+ }
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_BUSY_RETRY_TIME])
+ result->busy_retry_time =
+ nla_get_u8(tb[NL80211_PMSR_FTM_RESP_ATTR_BUSY_RETRY_TIME]);
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_NUM_BURSTS_EXP])
+ result->num_bursts_exp = nla_get_u8(tb[NL80211_PMSR_FTM_RESP_ATTR_NUM_BURSTS_EXP]);
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_BURST_DURATION])
+ result->burst_duration = nla_get_u8(tb[NL80211_PMSR_FTM_RESP_ATTR_BURST_DURATION]);
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_FTMS_PER_BURST])
+ result->ftms_per_burst = nla_get_u8(tb[NL80211_PMSR_FTM_RESP_ATTR_FTMS_PER_BURST]);
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_RSSI_AVG]) {
+ result->rssi_avg_valid = 1;
+ result->rssi_avg = nla_get_s32(tb[NL80211_PMSR_FTM_RESP_ATTR_RSSI_AVG]);
+ }
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_RSSI_SPREAD]) {
+ result->rssi_spread_valid = 1;
+ result->rssi_spread =
+ nla_get_s32(tb[NL80211_PMSR_FTM_RESP_ATTR_RSSI_SPREAD]);
+ }
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_TX_RATE]) {
+ result->tx_rate_valid = 1;
+ ret = mac80211_hwsim_parse_rate_info(tb[NL80211_PMSR_FTM_RESP_ATTR_TX_RATE],
+ &result->tx_rate, info);
+ if (ret)
+ return ret;
+ }
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_RX_RATE]) {
+ result->rx_rate_valid = 1;
+ ret = mac80211_hwsim_parse_rate_info(tb[NL80211_PMSR_FTM_RESP_ATTR_RX_RATE],
+ &result->rx_rate, info);
+ if (ret)
+ return ret;
+ }
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_RTT_AVG]) {
+ result->rtt_avg_valid = 1;
+ result->rtt_avg =
+ nla_get_u64(tb[NL80211_PMSR_FTM_RESP_ATTR_RTT_AVG]);
+ }
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_RTT_VARIANCE]) {
+ result->rtt_variance_valid = 1;
+ result->rtt_variance =
+ nla_get_u64(tb[NL80211_PMSR_FTM_RESP_ATTR_RTT_VARIANCE]);
+ }
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_RTT_SPREAD]) {
+ result->rtt_spread_valid = 1;
+ result->rtt_spread =
+ nla_get_u64(tb[NL80211_PMSR_FTM_RESP_ATTR_RTT_SPREAD]);
+ }
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_DIST_AVG]) {
+ result->dist_avg_valid = 1;
+ result->dist_avg =
+ nla_get_u64(tb[NL80211_PMSR_FTM_RESP_ATTR_DIST_AVG]);
+ }
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_DIST_VARIANCE]) {
+ result->dist_variance_valid = 1;
+ result->dist_variance =
+ nla_get_u64(tb[NL80211_PMSR_FTM_RESP_ATTR_DIST_VARIANCE]);
+ }
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_DIST_SPREAD]) {
+ result->dist_spread_valid = 1;
+ result->dist_spread =
+ nla_get_u64(tb[NL80211_PMSR_FTM_RESP_ATTR_DIST_SPREAD]);
+ }
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_LCI]) {
+ result->lci = nla_data(tb[NL80211_PMSR_FTM_RESP_ATTR_LCI]);
+ result->lci_len = nla_len(tb[NL80211_PMSR_FTM_RESP_ATTR_LCI]);
+ }
+
+ if (tb[NL80211_PMSR_FTM_RESP_ATTR_CIVICLOC]) {
+ result->civicloc = nla_data(tb[NL80211_PMSR_FTM_RESP_ATTR_CIVICLOC]);
+ result->civicloc_len = nla_len(tb[NL80211_PMSR_FTM_RESP_ATTR_CIVICLOC]);
+ }
+
+ return 0;
+}
+
+static int mac80211_hwsim_parse_pmsr_resp(struct nlattr *resp,
+ struct cfg80211_pmsr_result *result,
+ struct genl_info *info)
+{
+ struct nlattr *tb[NL80211_PMSR_RESP_ATTR_MAX + 1];
+ struct nlattr *pmsr;
+ int rem;
+ int ret;
+
+ ret = nla_parse_nested(tb, NL80211_PMSR_RESP_ATTR_MAX, resp,
+ hwsim_pmsr_resp_policy, info->extack);
+
+ if (tb[NL80211_PMSR_RESP_ATTR_STATUS])
+ result->status = nla_get_u32(tb[NL80211_PMSR_RESP_ATTR_STATUS]);
+
+ if (tb[NL80211_PMSR_RESP_ATTR_HOST_TIME])
+ result->host_time = nla_get_u64(tb[NL80211_PMSR_RESP_ATTR_HOST_TIME]);
+
+ if (tb[NL80211_PMSR_RESP_ATTR_AP_TSF]) {
+ result->ap_tsf_valid = 1;
+ result->ap_tsf = nla_get_u64(tb[NL80211_PMSR_RESP_ATTR_AP_TSF]);
+ }
+
+ result->final = !!tb[NL80211_PMSR_RESP_ATTR_FINAL];
+
+ if (tb[NL80211_PMSR_RESP_ATTR_DATA]) {
+ nla_for_each_nested(pmsr, tb[NL80211_PMSR_RESP_ATTR_DATA], rem) {
+ switch (nla_type(pmsr)) {
+ case NL80211_PMSR_TYPE_FTM:
+ result->type = NL80211_PMSR_TYPE_FTM;
+ ret = mac80211_hwsim_parse_ftm_result(pmsr, &result->ftm, info);
+ if (ret)
+ return ret;
+ break;
+ default:
+ NL_SET_ERR_MSG_ATTR(info->extack, pmsr, "Unknown pmsr resp type");
+ return -EINVAL;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int mac80211_hwsim_parse_pmsr_result(struct nlattr *peer,
+ struct cfg80211_pmsr_result *result,
+ struct genl_info *info)
+{
+ struct nlattr *tb[NL80211_PMSR_PEER_ATTR_MAX + 1];
+ int ret;
+
+ if (!peer)
+ return -EINVAL;
+
+ ret = nla_parse_nested(tb, NL80211_PMSR_PEER_ATTR_MAX, peer,
+ hwsim_pmsr_peer_result_policy, info->extack);
+ if (ret)
+ return ret;
+
+ if (tb[NL80211_PMSR_PEER_ATTR_ADDR])
+ memcpy(result->addr, nla_data(tb[NL80211_PMSR_PEER_ATTR_ADDR]),
+ ETH_ALEN);
+
+ if (tb[NL80211_PMSR_PEER_ATTR_RESP]) {
+ ret = mac80211_hwsim_parse_pmsr_resp(tb[NL80211_PMSR_PEER_ATTR_RESP], result, info);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+};
+
+static int hwsim_pmsr_report_nl(struct sk_buff *msg, struct genl_info *info)
+{
+ struct nlattr *reqattr;
+ const u8 *src;
+ int err, rem;
+ struct nlattr *peers, *peer;
+ struct mac80211_hwsim_data *data;
+
+ src = nla_data(info->attrs[HWSIM_ATTR_ADDR_TRANSMITTER]);
+ data = get_hwsim_data_ref_from_addr(src);
+ if (!data)
+ return -EINVAL;
+
+ mutex_lock(&data->mutex);
+ if (!data->pmsr_request) {
+ err = -EINVAL;
+ goto out_err;
+ }
+
+ reqattr = info->attrs[HWSIM_ATTR_PMSR_RESULT];
+ if (!reqattr) {
+ err = -EINVAL;
+ goto out_err;
+ }
+
+ peers = nla_find_nested(reqattr, NL80211_PMSR_ATTR_PEERS);
+ if (!peers) {
+ err = -EINVAL;
+ goto out_err;
+ }
+
+ nla_for_each_nested(peer, peers, rem) {
+ struct cfg80211_pmsr_result result;
+
+ err = mac80211_hwsim_parse_pmsr_result(peer, &result, info);
+ if (err)
+ goto out_err;
+
+ cfg80211_pmsr_report(data->pmsr_request_wdev,
+ data->pmsr_request, &result, GFP_KERNEL);
+ }
+
+ cfg80211_pmsr_complete(data->pmsr_request_wdev, data->pmsr_request, GFP_KERNEL);
+
+out_err:
+ data->pmsr_request = NULL;
+ data->pmsr_request_wdev = NULL;
+
+ mutex_unlock(&data->mutex);
+ return err;
+}
+
#define HWSIM_COMMON_OPS \
.tx = mac80211_hwsim_tx, \
.wake_tx_queue = ieee80211_handle_wake_tx_queue, \
@@ -5072,13 +5434,6 @@ static void hwsim_mon_setup(struct net_device *dev)
eth_hw_addr_set(dev, addr);
}
-static struct mac80211_hwsim_data *get_hwsim_data_ref_from_addr(const u8 *addr)
-{
- return rhashtable_lookup_fast(&hwsim_radios_rht,
- addr,
- hwsim_rht_params);
-}
-
static void hwsim_register_wmediumd(struct net *net, u32 portid)
{
struct mac80211_hwsim_data *data;
@@ -5745,6 +6100,11 @@ static const struct genl_small_ops hwsim_ops[] = {
.doit = hwsim_get_radio_nl,
.dumpit = hwsim_dump_radio_nl,
},
+ {
+ .cmd = HWSIM_CMD_REPORT_PMSR,
+ .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+ .doit = hwsim_pmsr_report_nl,
+ },
};
static struct genl_family hwsim_genl_family __ro_after_init = {
@@ -5756,7 +6116,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_REPORT_PMSR + 1, // match with __HWSIM_CMD_MAX
.mcgrps = hwsim_mcgrps,
.n_mcgrps = ARRAY_SIZE(hwsim_mcgrps),
};
@@ -5925,6 +6285,9 @@ static int hwsim_virtio_handle_cmd(struct sk_buff *skb)
case HWSIM_CMD_TX_INFO_FRAME:
hwsim_tx_info_frame_received_nl(skb, &info);
break;
+ case HWSIM_CMD_REPORT_PMSR:
+ hwsim_pmsr_report_nl(skb, &info);
+ break;
default:
pr_err_ratelimited("hwsim: invalid cmd: %d\n", gnlh->cmd);
return -EPROTO;
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index 383f3e39c911..92126f02c58f 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -82,8 +82,8 @@ enum hwsim_tx_control_flags {
* @HWSIM_CMD_DEL_MAC_ADDR: remove the MAC address again, the attributes
* are the same as to @HWSIM_CMD_ADD_MAC_ADDR.
* @HWSIM_CMD_START_PMSR: request to start peer measurement with the
- * %HWSIM_ATTR_PMSR_REQUEST.
- * @HWSIM_CMD_ABORT_PMSR: abort previously sent peer measurement
+ * %HWSIM_ATTR_PMSR_REQUEST. Result will be sent back asynchronously
+ * with %HWSIM_CMD_REPORT_PMSR.
* @__HWSIM_CMD_MAX: enum limit
*/
enum {
@@ -98,6 +98,7 @@ enum {
HWSIM_CMD_DEL_MAC_ADDR,
HWSIM_CMD_START_PMSR,
HWSIM_CMD_ABORT_PMSR,
+ HWSIM_CMD_REPORT_PMSR,
__HWSIM_CMD_MAX,
};
#define HWSIM_CMD_MAX (_HWSIM_CMD_MAX - 1)
@@ -151,6 +152,8 @@ enum {
* to provide peer measurement capabilities. (nl80211_peer_measurement_attrs)
* @HWSIM_ATTR_PMSR_REQUEST: nested attribute used with %HWSIM_CMD_START_PMSR
* to provide details about peer measurement request (nl80211_peer_measurement_attrs)
+ * @HWSIM_ATTR_PMSR_RESULT: nested attributed used with %HWSIM_CMD_REPORT_PMSR
+ * to provide peer measurement result (nl80211_peer_measurement_attrs)
* @__HWSIM_ATTR_MAX: enum limit
*/
@@ -184,6 +187,7 @@ enum {
HWSIM_ATTR_MLO_SUPPORT,
HWSIM_ATTR_PMSR_SUPPORT,
HWSIM_ATTR_PMSR_REQUEST,
+ HWSIM_ATTR_PMSR_RESULT,
__HWSIM_ATTR_MAX,
};
#define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
@@ -288,4 +292,47 @@ enum {
HWSIM_VQ_RX,
HWSIM_NUM_VQS,
};
+
+/**
+ * enum hwsim_rate_info -- bitrate information.
+ *
+ * Information about a receiving or transmitting bitrate
+ * that can be mapped to struct rate_info
+ *
+ * @HWSIM_RATE_INFO_ATTR_FLAGS: bitflag of flags from &enum rate_info_flags
+ * @HWSIM_RATE_INFO_ATTR_MCS: mcs index if struct describes an HT/VHT/HE rate
+ * @HWSIM_RATE_INFO_ATTR_LEGACY: bitrate in 100kbit/s for 802.11abg
+ * @HWSIM_RATE_INFO_ATTR_NSS: number of streams (VHT & HE only)
+ * @HWSIM_RATE_INFO_ATTR_BW: bandwidth (from &enum rate_info_bw)
+ * @HWSIM_RATE_INFO_ATTR_HE_GI: HE guard interval (from &enum nl80211_he_gi)
+ * @HWSIM_RATE_INFO_ATTR_HE_DCM: HE DCM value
+ * @HWSIM_RATE_INFO_ATTR_HE_RU_ALLOC: HE RU allocation (from &enum nl80211_he_ru_alloc,
+ * only valid if bw is %RATE_INFO_BW_HE_RU)
+ * @HWSIM_RATE_INFO_ATTR_N_BOUNDED_CH: In case of EDMG the number of bonded channels (1-4)
+ * @HWSIM_RATE_INFO_ATTR_EHT_GI: EHT guard interval (from &enum nl80211_eht_gi)
+ * @HWSIM_RATE_INFO_ATTR_EHT_RU_ALLOC: EHT RU allocation (from &enum nl80211_eht_ru_alloc,
+ * only valid if bw is %RATE_INFO_BW_EHT_RU)
+ * @NUM_HWSIM_RATE_INFO_ATTRS: internal
+ * @HWSIM_RATE_INFO_ATTR_MAX: highest attribute number
+ */
+enum hwsim_rate_info_attributes {
+ __HWSIM_RATE_INFO_ATTR_INVALID,
+
+ HWSIM_RATE_INFO_ATTR_FLAGS,
+ HWSIM_RATE_INFO_ATTR_MCS,
+ HWSIM_RATE_INFO_ATTR_LEGACY,
+ HWSIM_RATE_INFO_ATTR_NSS,
+ HWSIM_RATE_INFO_ATTR_BW,
+ HWSIM_RATE_INFO_ATTR_HE_GI,
+ HWSIM_RATE_INFO_ATTR_HE_DCM,
+ HWSIM_RATE_INFO_ATTR_HE_RU_ALLOC,
+ HWSIM_RATE_INFO_ATTR_N_BOUNDED_CH,
+ HWSIM_RATE_INFO_ATTR_EHT_GI,
+ HWSIM_RATE_INFO_ATTR_EHT_RU_ALLOC,
+
+ /* keep last */
+ NUM_HWSIM_RATE_INFO_ATTRS,
+ HWSIM_RATE_INFO_ATTR_MAX = NUM_HWSIM_RATE_INFO_ATTRS - 1
+};
+
#endif /* __MAC80211_HWSIM_H */
--
2.39.2.722.g9855ee24e9-goog
On Thu, Mar 02, 2023 at 04:03:05PM +0000, Jaewan Kim wrote:
> Dear Kernel maintainers,
>
> First of all, thank you for spending your precious time for reviewing
> my changes, and also sorry for my mistakes in previous patchsets.
>
> Let me propose series of CLs for adding PMSR support in the mac80211_hwsim.
What is a "CL"?
Greg KH <[email protected]> writes:
> On Thu, Mar 02, 2023 at 04:03:05PM +0000, Jaewan Kim wrote:
>> Dear Kernel maintainers,
>>
>> First of all, thank you for spending your precious time for reviewing
>> my changes, and also sorry for my mistakes in previous patchsets.
>>
>> Let me propose series of CLs for adding PMSR support in the mac80211_hwsim.
>
> What is a "CL"?
Hehe, we are not the only ones asking for this:
https://stackoverflow.com/questions/25716920/what-does-cl-mean-in-a-commit-message-what-does-it-stand-for
Apparently this is Google terminology but in upstream we use "patch" and
"patchset". But the recommendation is to not say "in this patchset" or
"in this patch" in commit logs, everyone know they are patches anyway.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
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 <[email protected]>
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;
> +}
> +
...
>
> > @@ -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.
>
Probably not. It's just a temporary thing there, I don't think we even
have it for longer than temporarily on the stack.
johannes
On Thu, Mar 02, 2023 at 04:03:08PM +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 functionalities for mac80211_hwsim to start PMSR request by
> passthrough the request to wmediumd via virtio. mac80211_hwsim can't
> measure RTT for real because mac80211_hwsim the software simulator and
> packets are sent almost immediately for real. This change expect wmediumd
> to have all the location information of devices, so passthrough requests
> to wmediumd.
>
> In detail, add new mac80211_hwsim command HWSIM_CMD_ABORT_PMSR. When
> mac80211_hwsim receives the PMSR start request via
> ieee80211_ops.start_pmsr, the received cfg80211_pmsr_request is resent to
> the wmediumd with command HWSIM_CMD_START_PMSR and attribute
> HWSIM_ATTR_PMSR_REQUEST. The attribute is formatted as the same way as
> nl80211_pmsr_start() expects.
>
> Signed-off-by: Jaewan Kim <[email protected]>
> ---
> V7->V8: Export nl80211_send_chandef directly and instead of creating
> wrapper.
> V7: Initial commit (split from previously large patch)
> ---
> drivers/net/wireless/mac80211_hwsim.c | 207 +++++++++++++++++++++++++-
> drivers/net/wireless/mac80211_hwsim.h | 6 +
> 2 files changed, 212 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
> index 79476d55c1ca..691b83140d57 100644
> --- a/drivers/net/wireless/mac80211_hwsim.c
> +++ b/drivers/net/wireless/mac80211_hwsim.c
> @@ -721,6 +721,8 @@ struct mac80211_hwsim_data {
>
> /* only used when pmsr capability is supplied */
> struct cfg80211_pmsr_capabilities pmsr_capa;
> + struct cfg80211_pmsr_request *pmsr_request;
> + struct wireless_dev *pmsr_request_wdev;
>
> struct mac80211_hwsim_link_data link_data[IEEE80211_MLD_MAX_NUM_LINKS];
> };
> @@ -3139,6 +3141,208 @@ static int mac80211_hwsim_change_sta_links(struct ieee80211_hw *hw,
> return 0;
> }
>
> +static int mac80211_hwsim_send_pmsr_ftm_request_peer(struct sk_buff *msg,
> + struct cfg80211_pmsr_ftm_request_peer *request)
> +{
> + struct nlattr *ftm;
> +
> + if (!request->requested)
> + return -EINVAL;
> +
> + ftm = nla_nest_start(msg, NL80211_PMSR_TYPE_FTM);
> + if (!ftm)
> + return -ENOBUFS;
> +
> + if (nla_put_u32(msg, NL80211_PMSR_FTM_REQ_ATTR_PREAMBLE, request->preamble))
nit: I suspect that you need to invoke nla_nest_cancel() in
error paths to unwind nla_nest_start() calls.
> + return -ENOBUFS;
> +
...
> +static int mac80211_hwsim_send_pmsr_request(struct sk_buff *msg,
> + struct cfg80211_pmsr_request *request)
> +{
> + int err;
> + struct nlattr *pmsr = nla_nest_start(msg, NL80211_ATTR_PEER_MEASUREMENTS);
nit: reverse xmas tree - longest line to shortest - for local variable
declarations.
> +
> + if (!pmsr)
> + return -ENOBUFS;
> +
> + if (nla_put_u32(msg, NL80211_ATTR_TIMEOUT, request->timeout))
> + return -ENOBUFS;
> +
> + if (!is_zero_ether_addr(request->mac_addr)) {
> + if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, request->mac_addr))
> + return -ENOBUFS;
> + if (nla_put(msg, NL80211_ATTR_MAC_MASK, ETH_ALEN, request->mac_addr_mask))
> + return -ENOBUFS;
> + }
> +
> + for (int i = 0; i < request->n_peers; i++) {
nit: the scope of err can be reduced to this block.
> + err = mac80211_hwsim_send_pmsr_request_peer(msg, &request->peers[i]);
> + if (err)
> + return err;
> + }
> +
> + nla_nest_end(msg, pmsr);
> +
> + return 0;
> +}
On Thu, Mar 02, 2023 at 04:03:09PM +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.
>
> Add necessary functionalities for mac80211_hwsim to abort previous PMSR
> request. The abortion request is sent to the wmedium where the PMSR request
> is actually handled.
>
> In detail, add new mac80211_hwsim command HWSIM_CMD_ABORT_PMSR. When
> mac80211_hwsim receives the PMSR abortion request via
> ieee80211_ops.abort_pmsr, the received cfg80211_pmsr_request is resent to
> the wmediumd with command HWSIM_CMD_ABORT_PMSR and attribute
> HWSIM_ATTR_PMSR_REQUEST. The attribute is formatted as the same way as
> nl80211_pmsr_start() expects.
>
> Signed-off-by: Jaewan Kim <[email protected]>
> ---
> V7->V8: Rewrote commit msg
> V7: Initial commit (split from previously large patch)
> ---
> drivers/net/wireless/mac80211_hwsim.c | 61 +++++++++++++++++++++++++++
> drivers/net/wireless/mac80211_hwsim.h | 2 +
> 2 files changed, 63 insertions(+)
>
> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
> index 691b83140d57..0d92a7e51057 100644
> --- a/drivers/net/wireless/mac80211_hwsim.c
> +++ b/drivers/net/wireless/mac80211_hwsim.c
> @@ -3343,6 +3343,66 @@ static int mac80211_hwsim_start_pmsr(struct ieee80211_hw *hw,
> return err;
> }
>
> +static void mac80211_hwsim_abort_pmsr(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct cfg80211_pmsr_request *request)
> +{
> + struct mac80211_hwsim_data *data = hw->priv;
> + u32 _portid = READ_ONCE(data->wmediumd);
> + struct sk_buff *skb = NULL;
> + int err = 0;
> + void *msg_head;
> + struct nlattr *pmsr;
> +
> + if (!_portid && !hwsim_virtio_enabled)
> + return;
> +
> + mutex_lock(&data->mutex);
> +
> + if (data->pmsr_request != request) {
> + err = -EINVAL;
> + goto out_err;
> + }
> +
> + if (err)
> + return;
How can this occur?
And if it does, isn't the lock leaked?
> +
> + skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!skb)
> + return;
I think the mutex needs to be unlocked here in this error path.
> +
> + msg_head = genlmsg_put(skb, 0, 0, &hwsim_genl_family, 0, HWSIM_CMD_ABORT_PMSR);
> +
> + if (nla_put(skb, HWSIM_ATTR_ADDR_TRANSMITTER, ETH_ALEN, data->addresses[1].addr))
In the current scheme, I think err needs to be set here.
> + goto out_err;
> +
> + pmsr = nla_nest_start(skb, HWSIM_ATTR_PMSR_REQUEST);
> + if (!pmsr) {
> + err = -ENOMEM;
> + goto out_err;
> + }
> +
> + err = mac80211_hwsim_send_pmsr_request(skb, request);
> + if (err)
I think this error path needs to call nla_nest_cancel().
> + goto out_err;
> +
> + err = nla_nest_end(skb, pmsr);
> + if (err)
I don't think is an error condition.
> + goto out_err;
> +
> + genlmsg_end(skb, msg_head);
> + if (hwsim_virtio_enabled)
> + hwsim_tx_virtio(data, skb);
> + else
> + hwsim_unicast_netgroup(data, skb, _portid);
> +
> +out_err:
> + if (err && skb)
> + nlmsg_free(skb);
> +
> + mutex_unlock(&data->mutex);
I think it might be nicer to arrange this as:
goto out_unlock;
err_nest_cancel:
nla_nest_cancel(...);
err_free:
nlmsg_free(skb);
out_unlock:
mutex_unlock(&data->mutex);
}
> +}
> +
> #define HWSIM_COMMON_OPS \
> .tx = mac80211_hwsim_tx, \
> .wake_tx_queue = ieee80211_handle_wake_tx_queue, \
> @@ -3367,6 +3427,7 @@ static int mac80211_hwsim_start_pmsr(struct ieee80211_hw *hw,
> .get_et_stats = mac80211_hwsim_get_et_stats, \
> .get_et_strings = mac80211_hwsim_get_et_strings, \
> .start_pmsr = mac80211_hwsim_start_pmsr, \
> + .abort_pmsr = mac80211_hwsim_abort_pmsr,
>
> #define HWSIM_NON_MLO_OPS \
> .sta_add = mac80211_hwsim_sta_add, \
...
On Mon, Mar 06, 2023 at 05:58:37PM +0100, Johannes Berg wrote:
> >
> > > @@ -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.
> >
>
> Probably not. It's just a temporary thing there, I don't think we even
> have it for longer than temporarily on the stack.
Thanks, got it.
On Mon, Mar 06, 2023 at 06:12:02PM +0100, Simon Horman wrote:
> On Thu, Mar 02, 2023 at 04:03:08PM +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 functionalities for mac80211_hwsim to start PMSR request by
> > passthrough the request to wmediumd via virtio. mac80211_hwsim can't
> > measure RTT for real because mac80211_hwsim the software simulator and
> > packets are sent almost immediately for real. This change expect wmediumd
> > to have all the location information of devices, so passthrough requests
> > to wmediumd.
> >
> > In detail, add new mac80211_hwsim command HWSIM_CMD_ABORT_PMSR. When
> > mac80211_hwsim receives the PMSR start request via
> > ieee80211_ops.start_pmsr, the received cfg80211_pmsr_request is resent to
> > the wmediumd with command HWSIM_CMD_START_PMSR and attribute
> > HWSIM_ATTR_PMSR_REQUEST. The attribute is formatted as the same way as
> > nl80211_pmsr_start() expects.
> >
> > Signed-off-by: Jaewan Kim <[email protected]>
> > ---
> > V7->V8: Export nl80211_send_chandef directly and instead of creating
> > wrapper.
> > V7: Initial commit (split from previously large patch)
> > ---
> > drivers/net/wireless/mac80211_hwsim.c | 207 +++++++++++++++++++++++++-
> > drivers/net/wireless/mac80211_hwsim.h | 6 +
> > 2 files changed, 212 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
> > index 79476d55c1ca..691b83140d57 100644
> > --- a/drivers/net/wireless/mac80211_hwsim.c
> > +++ b/drivers/net/wireless/mac80211_hwsim.c
> > @@ -721,6 +721,8 @@ struct mac80211_hwsim_data {
> >
> > /* only used when pmsr capability is supplied */
> > struct cfg80211_pmsr_capabilities pmsr_capa;
> > + struct cfg80211_pmsr_request *pmsr_request;
> > + struct wireless_dev *pmsr_request_wdev;
> >
> > struct mac80211_hwsim_link_data link_data[IEEE80211_MLD_MAX_NUM_LINKS];
> > };
> > @@ -3139,6 +3141,208 @@ static int mac80211_hwsim_change_sta_links(struct ieee80211_hw *hw,
> > return 0;
> > }
> >
> > +static int mac80211_hwsim_send_pmsr_ftm_request_peer(struct sk_buff *msg,
> > + struct cfg80211_pmsr_ftm_request_peer *request)
> > +{
> > + struct nlattr *ftm;
> > +
> > + if (!request->requested)
> > + return -EINVAL;
> > +
> > + ftm = nla_nest_start(msg, NL80211_PMSR_TYPE_FTM);
> > + if (!ftm)
> > + return -ENOBUFS;
> > +
> > + if (nla_put_u32(msg, NL80211_PMSR_FTM_REQ_ATTR_PREAMBLE, request->preamble))
>
> nit: I suspect that you need to invoke nla_nest_cancel() in
> error paths to unwind nla_nest_start() calls.
>
> > + return -ENOBUFS;
> > +
>
> ...
>
> > +static int mac80211_hwsim_send_pmsr_request(struct sk_buff *msg,
> > + struct cfg80211_pmsr_request *request)
> > +{
> > + int err;
> > + struct nlattr *pmsr = nla_nest_start(msg, NL80211_ATTR_PEER_MEASUREMENTS);
> nit: reverse xmas tree - longest line to shortest - for local variable
> declarations.
Sorry, I meant to delete this one.
I don't think it is the practice in this driver.
>
> > +
> > + if (!pmsr)
> > + return -ENOBUFS;
> > +
> > + if (nla_put_u32(msg, NL80211_ATTR_TIMEOUT, request->timeout))
> > + return -ENOBUFS;
> > +
> > + if (!is_zero_ether_addr(request->mac_addr)) {
> > + if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, request->mac_addr))
> > + return -ENOBUFS;
> > + if (nla_put(msg, NL80211_ATTR_MAC_MASK, ETH_ALEN, request->mac_addr_mask))
> > + return -ENOBUFS;
> > + }
> > +
> > + for (int i = 0; i < request->n_peers; i++) {
>
> nit: the scope of err can be reduced to this block.
>
> > + err = mac80211_hwsim_send_pmsr_request_peer(msg, &request->peers[i]);
> > + if (err)
> > + return err;
> > + }
> > +
> > + nla_nest_end(msg, pmsr);
> > +
> > + return 0;
> > +}
>
On Mon, 2023-03-06 at 18:12 +0100, Simon Horman wrote:
>
> >
> > +static int mac80211_hwsim_send_pmsr_ftm_request_peer(struct sk_buff *msg,
> > + struct cfg80211_pmsr_ftm_request_peer *request)
> > +{
> > + struct nlattr *ftm;
> > +
> > + if (!request->requested)
> > + return -EINVAL;
> > +
> > + ftm = nla_nest_start(msg, NL80211_PMSR_TYPE_FTM);
> > + if (!ftm)
> > + return -ENOBUFS;
> > +
> > + if (nla_put_u32(msg, NL80211_PMSR_FTM_REQ_ATTR_PREAMBLE, request->preamble))
>
> nit: I suspect that you need to invoke nla_nest_cancel() in
> error paths to unwind nla_nest_start() calls.
The entire message is discarded if that happens, I think? Doesn't seem
all that necessary in that case.
johannes
On Mon, Mar 06, 2023 at 09:45:16PM +0100, Johannes Berg wrote:
> On Mon, 2023-03-06 at 18:12 +0100, Simon Horman wrote:
> >
> > >
> > > +static int mac80211_hwsim_send_pmsr_ftm_request_peer(struct sk_buff *msg,
> > > + struct cfg80211_pmsr_ftm_request_peer *request)
> > > +{
> > > + struct nlattr *ftm;
> > > +
> > > + if (!request->requested)
> > > + return -EINVAL;
> > > +
> > > + ftm = nla_nest_start(msg, NL80211_PMSR_TYPE_FTM);
> > > + if (!ftm)
> > > + return -ENOBUFS;
> > > +
> > > + if (nla_put_u32(msg, NL80211_PMSR_FTM_REQ_ATTR_PREAMBLE, request->preamble))
> >
> > nit: I suspect that you need to invoke nla_nest_cancel() in
> > error paths to unwind nla_nest_start() calls.
>
> The entire message is discarded if that happens, I think? Doesn't seem
> all that necessary in that case.
Thanks. I was wondering about that, but forgot to add it to my review message.
On Mon, Mar 06, 2023 at 06:37:14PM +0100, Simon Horman wrote:
> On Thu, Mar 02, 2023 at 04:03:09PM +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.
> >
> > Add necessary functionalities for mac80211_hwsim to abort previous PMSR
> > request. The abortion request is sent to the wmedium where the PMSR request
> > is actually handled.
> >
> > In detail, add new mac80211_hwsim command HWSIM_CMD_ABORT_PMSR. When
> > mac80211_hwsim receives the PMSR abortion request via
> > ieee80211_ops.abort_pmsr, the received cfg80211_pmsr_request is resent to
> > the wmediumd with command HWSIM_CMD_ABORT_PMSR and attribute
> > HWSIM_ATTR_PMSR_REQUEST. The attribute is formatted as the same way as
> > nl80211_pmsr_start() expects.
...
> > + goto out_err;
> > +
> > + pmsr = nla_nest_start(skb, HWSIM_ATTR_PMSR_REQUEST);
> > + if (!pmsr) {
> > + err = -ENOMEM;
> > + goto out_err;
> > + }
> > +
> > + err = mac80211_hwsim_send_pmsr_request(skb, request);
> > + if (err)
>
> I think this error path needs to call nla_nest_cancel().
As per Johannes's comment elsewhere,
I now realise this is not necessary as the skb is destroyed.
...
On Mon, Mar 6, 2023 at 4:55 PM Simon Horman <[email protected]> wrote:
>
> 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 <[email protected]>
>
> 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.
I asked to change this in the prior patch v3.
>
> > }
> >
> > 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;
>
Not using NL_SET_ERR_MSG_ATTR(...) is intended to follow the pattern
of net/wireless/pmsr.c,
where unknown type isn't considered as an error.
--
Jaewan Kim (김재완) | Software Engineer in Google Korea |
[email protected] | +82-10-2781-5078
On Wed, 2023-03-08 at 08:00 +0000, Jaewan Kim wrote:
> >
> > > +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;
> >
>
> Not using NL_SET_ERR_MSG_ATTR(...) is intended to follow the pattern
> of net/wireless/pmsr.c,
> where unknown type isn't considered as an error.
NL80211_PMSR_ATTR_TYPE_CAPA is normally NLA_REJECT (not sent by
userspace), you just use it here for the hwsim capabilities which makes
sense, but it feels better to just reject unknown types.
If you're thinking of actually using it we still have in pmsr.c this
code:
nla_for_each_nested(treq, req[NL80211_PMSR_REQ_ATTR_DATA], rem) {
switch (nla_type(treq)) {
case NL80211_PMSR_TYPE_FTM:
err = pmsr_parse_ftm(rdev, treq, out, info);
break;
default:
NL_SET_ERR_MSG_ATTR(info->extack, treq,
"unsupported measurement type");
err = -EINVAL;
}
johannes
Jaewan Kim <[email protected]> writes:
> Sorry about the inconvenience.
> I checked all patchset comments and also got internal reviews,
> but forgot to double check in the cover letter.
>
> Should I send another patchset just for cover-letter?
> Otherwise let me keep this as of now, unless I need to send another patchset.
Please don't use HTML, our lists drop all HTML mail.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Wed, Mar 8, 2023 at 5:07 PM Johannes Berg <[email protected]> wrote:
>
> On Wed, 2023-03-08 at 08:00 +0000, Jaewan Kim wrote:
> > >
> > > > +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;
> > >
> >
> > Not using NL_SET_ERR_MSG_ATTR(...) is intended to follow the pattern
> > of net/wireless/pmsr.c,
> > where unknown type isn't considered as an error.
>
> NL80211_PMSR_ATTR_TYPE_CAPA is normally NLA_REJECT (not sent by
> userspace), you just use it here for the hwsim capabilities which makes
> sense, but it feels better to just reject unknown types.
>
> If you're thinking of actually using it we still have in pmsr.c this
> code:
>
> nla_for_each_nested(treq, req[NL80211_PMSR_REQ_ATTR_DATA], rem) {
> switch (nla_type(treq)) {
> case NL80211_PMSR_TYPE_FTM:
> err = pmsr_parse_ftm(rdev, treq, out, info);
> break;
> default:
> NL_SET_ERR_MSG_ATTR(info->extack, treq,
> "unsupported measurement type");
> err = -EINVAL;
> }
>
>
> johannes
Done, and uploaded the next patchset for this. Thank you for the review.
--
Jaewan Kim (김재완) | Software Engineer in Google Korea |
[email protected] | +82-10-2781-5078
On Wed, Mar 8, 2023 at 7:05 PM Kalle Valo <[email protected]> wrote:
>
> Jaewan Kim <[email protected]> writes:
>
> > Sorry about the inconvenience.
> > I checked all patchset comments and also got internal reviews,
> > but forgot to double check in the cover letter.
> >
> > Should I send another patchset just for cover-letter?
> > Otherwise let me keep this as of now, unless I need to send another patchset.
>
> Please don't use HTML, our lists drop all HTML mail.
>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Sorry about any inconvenience caused by my mistake.
Uploaded a new patchset without 'CL', and also double checked 'plain
text mode' before replying.
--
Jaewan Kim (김재완) | Software Engineer in Google Korea |
[email protected] | +82-10-2781-5078