Re: [PATCH v5 1/4] scan: parse the scan start time

Re: [PATCH v5 1/4] scan: parse the scan start time

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

Hi James,

On 11/21/19 2:08 PM, James Prestwood wrote:
> The kernel sends NL80211_ATTR_SCAN_START_TIME_TSF with CMD_TRIGGER and
> RRM requires this value for beacon measurement reports.
> 
> The start time is parsed during CMD_TRIGGER and set into the scan request.
> A getter was added to obtain this time value for an already triggered
> scan.
> 
> After making the change, the SCAN_ABORTED case was cleaned up a bit to
> remove the local scan_request usage in favor of the one used for all the
> other cases.
> ---
>   src/scan.c | 41 ++++++++++++++++++++++++++++++++++-------
>   src/scan.h |  2 ++
>   2 files changed, 36 insertions(+), 7 deletions(-)
> 

I went ahead and took the first 3 in this series.  The last one has some 
missing L_CPU_TO_LE* calls.

Regards,
-Denis

[PATCH v5 1/4] scan: parse the scan start time

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 3928 bytes --]

The kernel sends NL80211_ATTR_SCAN_START_TIME_TSF with CMD_TRIGGER and
RRM requires this value for beacon measurement reports.

The start time is parsed during CMD_TRIGGER and set into the scan request.
A getter was added to obtain this time value for an already triggered
scan.

After making the change, the SCAN_ABORTED case was cleaned up a bit to
remove the local scan_request usage in favor of the one used for all the
other cases.
---
 src/scan.c | 41 ++++++++++++++++++++++++++++++++++-------
 src/scan.h |  2 ++
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/src/scan.c b/src/scan.c
index e007ce5d..e2d73601 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -75,6 +75,8 @@ struct scan_request {
 	scan_destroy_func_t destroy;
 	bool passive:1; /* Active or Passive scan? */
 	struct l_queue *cmds;
+	/* The time the current scan was started. Reported in TRIGGER_SCAN */
+	uint64_t start_time_tsf;
 };
 
 struct scan_context {
@@ -778,6 +780,25 @@ bool scan_periodic_stop(uint64_t wdev_id)
 	return true;
 }
 
+uint64_t scan_get_triggered_time(uint64_t wdev_id, uint32_t id)
+{
+	struct scan_context *sc;
+	struct scan_request *sr;
+
+	sc = l_queue_find(scan_contexts, scan_context_match, &wdev_id);
+	if (!sc)
+		return 0;
+
+	if (!sc->triggered)
+		return 0;
+
+	sr = l_queue_find(sc->requests, scan_request_match, L_UINT_TO_PTR(id));
+	if (!sr)
+		return 0;
+
+	return sr->start_time_tsf;
+}
+
 static void scan_periodic_timeout(struct l_timeout *timeout, void *user_data)
 {
 	struct scan_context *sc = user_data;
@@ -1439,6 +1460,8 @@ static void scan_notify(struct l_genl_msg *msg, void *user_data)
 	uint32_t wiphy_id;
 	struct scan_context *sc;
 	bool active_scan = false;
+	uint64_t start_time_tsf = 0;
+	struct scan_request *sr;
 
 	cmd = l_genl_msg_get_command(msg);
 
@@ -1461,15 +1484,22 @@ static void scan_notify(struct l_genl_msg *msg, void *user_data)
 		case NL80211_ATTR_SCAN_SSIDS:
 			active_scan = true;
 			break;
+		case NL80211_ATTR_SCAN_START_TIME_TSF:
+			if (len != sizeof(uint64_t))
+				return;
+
+			start_time_tsf = l_get_u64(data);
+			break;
 		}
 	}
 
+	sr = l_queue_peek_head(sc->requests);
+
 	switch (cmd) {
 	case NL80211_CMD_NEW_SCAN_RESULTS:
 	{
 		struct l_genl_msg *scan_msg;
 		struct scan_results *results;
-		struct scan_request *sr = l_queue_peek_head(sc->requests);
 		bool send_next = false;
 		bool get_results = false;
 
@@ -1543,12 +1573,11 @@ static void scan_notify(struct l_genl_msg *msg, void *user_data)
 		else
 			sc->state = SCAN_STATE_PASSIVE;
 
+		sr->start_time_tsf = start_time_tsf;
+
 		break;
 
 	case NL80211_CMD_SCAN_ABORTED:
-	{
-		struct scan_request *sr = l_queue_peek_head(sc->requests);
-
 		if (sc->state == SCAN_STATE_NOT_RUNNING)
 			break;
 
@@ -1557,8 +1586,7 @@ static void scan_notify(struct l_genl_msg *msg, void *user_data)
 		if (sc->triggered) {
 			sc->triggered = false;
 
-			scan_finished(sc, -ECANCELED, NULL,
-					l_queue_peek_head(sc->requests));
+			scan_finished(sc, -ECANCELED, NULL, sr);
 		} else if (sr && !sc->start_cmd_id && !sc->get_scan_cmd_id) {
 			/*
 			 * If this was an external scan that got aborted
@@ -1572,7 +1600,6 @@ static void scan_notify(struct l_genl_msg *msg, void *user_data)
 
 		break;
 	}
-	}
 }
 
 uint8_t scan_freq_to_channel(uint32_t freq, enum scan_band *out_band)
diff --git a/src/scan.h b/src/scan.h
index 8fc2aa56..b6c4e12d 100644
--- a/src/scan.h
+++ b/src/scan.h
@@ -124,6 +124,8 @@ void scan_periodic_start(uint64_t wdev_id, scan_trigger_func_t trigger,
 				scan_notify_func_t func, void *userdata);
 bool scan_periodic_stop(uint64_t wdev_id);
 
+uint64_t scan_get_triggered_time(uint64_t wdev_id, uint32_t id);
+
 void scan_bss_free(struct scan_bss *bss);
 int scan_bss_rank_compare(const void *a, const void *b, void *user);
 
-- 
2.17.1

[PATCH v5 2/4] rrm: include actual scan start time in report

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]

---
 src/rrm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/rrm.c b/src/rrm.c
index 2b0a22fb..ca30a7f7 100644
--- a/src/rrm.c
+++ b/src/rrm.c
@@ -109,6 +109,7 @@ struct rrm_beacon_req_info {
 	char ssid[33];		/* Request filtered by SSID */
 	bool has_ssid;
 	uint32_t scan_id;
+	uint64_t scan_start_time;
 };
 
 /* Per-netdev state */
@@ -249,8 +250,7 @@ static size_t build_report_for_bss(struct rrm_beacon_req_info *beacon,
 
 	*to++ = beacon->oper_class;
 	*to++ = scan_freq_to_channel(bss->frequency, NULL);
-	/* skip start time */
-	memset(to, 0, 8);
+	l_put_le64(beacon->scan_start_time, to);
 	to += 8;
 	l_put_le16(beacon->duration, to);
 	to += 2;
@@ -390,11 +390,18 @@ static bool rrm_scan_results(int err, struct l_queue *bss_list, void *userdata)
 static void rrm_scan_triggered(int err, void *userdata)
 {
 	struct rrm_state *rrm = userdata;
+	struct rrm_beacon_req_info *beacon = l_container_of(rrm->pending,
+						struct rrm_beacon_req_info,
+						info);
 
 	if (err < 0) {
 		l_error("Could not start RRM scan");
 		rrm_reject_measurement_request(rrm, REPORT_REJECT_INCAPABLE);
+		return;
 	}
+
+	beacon->scan_start_time = scan_get_triggered_time(rrm->wdev_id,
+							beacon->scan_id);
 }
 
 static void rrm_handle_beacon_scan(struct rrm_state *rrm,
-- 
2.17.1

[PATCH v5 3/4] rrm: fix non-ascii character in comment

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

---
 src/rrm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rrm.c b/src/rrm.c
index ca30a7f7..660565eb 100644
--- a/src/rrm.c
+++ b/src/rrm.c
@@ -274,7 +274,7 @@ static size_t build_report_for_bss(struct rrm_beacon_req_info *beacon,
 	 * 802.11 9.4.2.22.7 Beacon report
 	 *
 	 * "The Parent TSF field contains the lower 4 octets of the measuring
-	 *  STA’s TSF timer value"
+	 *  STA's TSF timer value"
 	 */
 	l_put_le32((uint32_t)(bss->parent_tsf & 0xffffffff), to);
 	to += 4;
-- 
2.17.1

[PATCH v5 4/4] rrm: add packed struct for beacon reports

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]

build_report_for_bss was refactored to use this packed structure rather
than l_put_* APIs.
---
 src/rrm.c | 66 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 25 deletions(-)

diff --git a/src/rrm.c b/src/rrm.c
index 660565eb..3b600f09 100644
--- a/src/rrm.c
+++ b/src/rrm.c
@@ -122,6 +122,21 @@ struct rrm_state {
 	uint64_t last_request;
 };
 
+/* 802.11, Section 9.4.2.22.7 */
+struct rrm_beacon_report {
+	uint8_t oper_class;
+	uint8_t channel;
+	__le64 scan_start_time;
+	__le16 duration;
+	uint8_t frame_info;
+	uint8_t rcpi;
+	uint8_t rsni;
+	uint8_t bssid[6];
+	uint8_t antenna_id;
+	__le32 parent_tsf;
+	uint8_t subelements[0];
+} __attribute__ ((packed));
+
 static struct l_queue *states;
 static struct l_genl_family *nl80211;
 static uint32_t netdev_watch;
@@ -225,6 +240,17 @@ static void rrm_build_measurement_report(struct rrm_request_info *info,
 		memcpy(to, report, report_len);
 }
 
+/* 802.11 Table 9-154 */
+static uint8_t mdb_to_rcpi(int32_t mdb)
+{
+	if (mdb <= 10950)
+		return 0;
+	else if (mdb >= -10950 && mdb < 0)
+		return ((uint8_t)((2 * (mdb + 11000)) / 100));
+	else
+		return 220;
+}
+
 /*
  * 802.11-2016 11.11.9.1 Beacon report
  *
@@ -246,38 +272,28 @@ static size_t build_report_for_bss(struct rrm_beacon_req_info *beacon,
 					struct scan_bss *bss,
 					uint8_t *to)
 {
-	uint8_t *start = to;
-
-	*to++ = beacon->oper_class;
-	*to++ = scan_freq_to_channel(bss->frequency, NULL);
-	l_put_le64(beacon->scan_start_time, to);
-	to += 8;
-	l_put_le16(beacon->duration, to);
-	to += 2;
-	*to++ = rrm_phy_type(bss);
-
-	/* 802.11 Table 9-154 - RCPI values */
-	if (bss->signal_strength < -10950)
-		*to++ = 0;
-	else if (bss->signal_strength >= -10950 && bss->signal_strength < 0)
-		*to++ = (uint8_t)((2 * (bss->signal_strength + 11000)) / 100);
-	else
-		*to++ = 220;
+	struct rrm_beacon_report *report = (struct rrm_beacon_report *) to;
+
+	report->oper_class = beacon->oper_class;
+	report->channel = scan_freq_to_channel(bss->frequency, NULL);
+	report->scan_start_time = beacon->scan_start_time;
+	report->duration = beacon->duration;
+	report->frame_info = rrm_phy_type(bss);
+	report->rcpi = mdb_to_rcpi(bss->signal_strength);
 
 	/* RSNI not available (could get this from GET_SURVEY) */
-	*to++ = 255;
-	memcpy(to, bss->addr, 6);
-	to += 6;
-	/* Antenna identifier unknown */
-	*to++ = 0;
+	report->rsni = 255;
+	memcpy(report->bssid, bss->addr, 6);
+
+	report->antenna_id = 0;
+
 	/*
 	 * 802.11 9.4.2.22.7 Beacon report
 	 *
 	 * "The Parent TSF field contains the lower 4 octets of the measuring
 	 *  STA's TSF timer value"
 	 */
-	l_put_le32((uint32_t)(bss->parent_tsf & 0xffffffff), to);
-	to += 4;
+	report->parent_tsf = (uint32_t)bss->parent_tsf;
 
 	/*
 	 * TODO: Support optional subelements
@@ -285,7 +301,7 @@ static size_t build_report_for_bss(struct rrm_beacon_req_info *beacon,
 	 * (see "TODO: Support Reported Frame Body..." below)
 	 */
 
-	return to - start;
+	return sizeof(struct rrm_beacon_report);
 }
 
 static bool bss_in_request_range(struct rrm_beacon_req_info *beacon,
-- 
2.17.1