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

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

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

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(-)

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);
 
-- 
2.37.3

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

[Denis Kenzior]

Hi Alvin,

On 11/17/22 15:11, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> 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