[v2] scan: retry scan based on scan done events per wiphy, not wdev

Commit Message

Alvin Šipraga Nov. 17, 2022, 9:11 p.m. UTC

From: Alvin Šipraga <[e-mail]>

If a CMD_TRIGGER_SCAN request fails with -EBUSY, iwd currently assumes
that a scan is ongoing on the underlying wdev and will retry the same
command when that scan is complete. It gets notified of that completion
via the scan_notify() function, and kicks the scan logic to try again.

However, if there is another wdev on the same wiphy and that wdev has a
scan request in flight, the kernel will also return -EBUSY. In other
words, only one scan request per wiphy is permitted.

As an example, the brcmfmac driver can create an AP interface on the
same wiphy as the default station interface, and scans can be triggered
on that AP interface.

If -EBUSY is returned because another wdev is scanning, then iwd won't
know when it can retry the original trigger request because the relevant
netlink event will arrive on a different wdev. Indeed, if no scan
context exists for that other wdev, then scan_notify will return early
and the scan logic will stall indefinitely.

Instead, and in the event that no scan context matches, use it as a cue
to retry a pending scan request that happens to be destined for the same
wiphy.
---
 src/scan.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

Comments

Denis Kenzior Nov. 17, 2022, 10:51 p.m. UTC | #1

Hi Alvin,

On 11/17/22 15:11, Alvin Šipraga wrote:
> From: Alvin Šipraga <[e-mail]>
> 
> If a CMD_TRIGGER_SCAN request fails with -EBUSY, iwd currently assumes
> that a scan is ongoing on the underlying wdev and will retry the same
> command when that scan is complete. It gets notified of that completion
> via the scan_notify() function, and kicks the scan logic to try again.
> 
> However, if there is another wdev on the same wiphy and that wdev has a
> scan request in flight, the kernel will also return -EBUSY. In other
> words, only one scan request per wiphy is permitted.
> 
> As an example, the brcmfmac driver can create an AP interface on the
> same wiphy as the default station interface, and scans can be triggered
> on that AP interface.
> 
> If -EBUSY is returned because another wdev is scanning, then iwd won't
> know when it can retry the original trigger request because the relevant
> netlink event will arrive on a different wdev. Indeed, if no scan
> context exists for that other wdev, then scan_notify will return early
> and the scan logic will stall indefinitely.
> 
> Instead, and in the event that no scan context matches, use it as a cue
> to retry a pending scan request that happens to be destined for the same
> wiphy.
> ---
>   src/scan.c | 38 +++++++++++++++++++++++++++++++++++++-
>   1 file changed, 37 insertions(+), 1 deletion(-)
> 

Applied, thanks.

Regards,
-Denis

Patch

diff --git a/src/scan.c b/src/scan.c
index 5548914a12de..ef222f66409c 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -2044,6 +2044,33 @@  static void scan_parse_result_frequencies(struct l_genl_msg *msg,
 	}
 }
 
+static void scan_retry_pending(uint32_t wiphy_id)
+{
+	const struct l_queue_entry *entry;
+
+	l_debug("");
+
+	for (entry = l_queue_get_entries(scan_contexts); entry;
+						entry = entry->next) {
+		struct scan_context *sc = entry->data;
+		struct scan_request *sr = l_queue_peek_head(sc->requests);
+
+		if (wiphy_get_id(sc->wiphy) != wiphy_id)
+			continue;
+
+		if (!sr)
+			continue;
+
+		if (!wiphy_radio_work_is_running(sc->wiphy, sr->work.id))
+			continue;
+
+		sc->state = SCAN_STATE_NOT_RUNNING;
+		start_next_scan_request(&sr->work);
+
+		return;
+	}
+}
+
 static void scan_notify(struct l_genl_msg *msg, void *user_data)
 {
 	struct l_genl_attr attr;
@@ -2065,8 +2092,17 @@  static void scan_notify(struct l_genl_msg *msg, void *user_data)
 		return;
 
 	sc = l_queue_find(scan_contexts, scan_context_match, &wdev_id);
-	if (!sc)
+	if (!sc) {
+		/*
+		 * If the event is for an unmanaged device, retry pending scan
+		 * requests on the same wiphy.
+		 */
+		if (cmd == NL80211_CMD_NEW_SCAN_RESULTS ||
+		    cmd == NL80211_CMD_SCAN_ABORTED)
+			scan_retry_pending(wiphy_id);
+
 		return;
+	}
 
 	l_debug("Scan notification %s(%u)", nl80211cmd_to_string(cmd), cmd);