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

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

[Alvin Šipraga]

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

---
 src/scan.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/src/scan.c b/src/scan.c
index 5548914a12de..c3763ee41173 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -151,6 +151,22 @@ static bool scan_context_match(const void *a, const void *b)
 	return sc->wdev_id == *wdev_id;
 }
 
+static bool scan_context_match_retry(const void *a, const void *b)
+{
+	const struct scan_context *sc = a;
+	const struct scan_request *sr;
+	const uint32_t *wiphy_id = b;
+
+	if (wiphy_get_id(sc->wiphy) != *wiphy_id)
+		return false;
+
+	sr = l_queue_peek_head(sc->requests);
+	if (!sr)
+		return false;
+
+	return wiphy_radio_work_is_running(sc->wiphy, sr->work.id);
+}
+
 static bool scan_request_match(const void *a, const void *b)
 {
 	const struct scan_request *sr = a;
@@ -231,6 +247,7 @@ static void scan_request_triggered(struct l_genl_msg *msg, void *userdata)
 	if (err < 0) {
 		/* Scan in progress, assume another scan is running */
 		if (err == -EBUSY) {
+			l_debug("XXX busy");
 			sc->state = SCAN_STATE_PASSIVE;
 			return;
 		}
@@ -2044,6 +2061,29 @@ static void scan_parse_result_frequencies(struct l_genl_msg *msg,
 	}
 }
 
+static void scan_retry_pending(uint32_t wiphy_id)
+{
+	struct scan_context *sc = l_queue_find(scan_contexts,
+					       scan_context_match_retry,
+					       &wiphy_id);
+	struct scan_request *sr;
+
+	l_debug("");
+
+	if (!sc)
+		return;
+
+	sr = l_queue_peek_head(sc->requests);
+
+	if (L_WARN_ON(!sr))
+		return;
+
+	l_debug("XXX retry pending request");
+
+	sc->state = SCAN_STATE_NOT_RUNNING;
+	start_next_scan_request(&sr->work);
+}
+
 static void scan_notify(struct l_genl_msg *msg, void *user_data)
 {
 	struct l_genl_attr attr;
@@ -2065,8 +2105,13 @@ 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 (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: [RFC PATCH] scan: retry scan based on scan done events per wiphy, not wdev

[Denis Kenzior]

Hi Alvin,

On 11/17/22 11:29, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> ---
>   src/scan.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/src/scan.c b/src/scan.c
> index 5548914a12de..c3763ee41173 100644
> --- a/src/scan.c
> +++ b/src/scan.c

<snip>

> @@ -231,6 +247,7 @@ static void scan_request_triggered(struct l_genl_msg *msg, void *userdata)
>   	if (err < 0) {
>   		/* Scan in progress, assume another scan is running */
>   		if (err == -EBUSY) {
> +			l_debug("XXX busy");

This might need a nicer message

>   			sc->state = SCAN_STATE_PASSIVE;
>   			return;
>   		}
> @@ -2044,6 +2061,29 @@ static void scan_parse_result_frequencies(struct l_genl_msg *msg,
>   	}
>   }
>   
> +static void scan_retry_pending(uint32_t wiphy_id)
> +{
> +	struct scan_context *sc = l_queue_find(scan_contexts,
> +					       scan_context_match_retry,
> +					       &wiphy_id);
> +	struct scan_request *sr;
> +
> +	l_debug("");
> +
> +	if (!sc)
> +		return;
> +
> +	sr = l_queue_peek_head(sc->requests);
> +
> +	if (L_WARN_ON(!sr))
> +		return;

Also you could simplify out the need for this L_WARN_ON by using 
l_queue_get_entries() style for loop on the scan_context(s) instead of using 
l_queue_find.  But that's just me nitpicking.

> +
> +	l_debug("XXX retry pending request");
> +
> +	sc->state = SCAN_STATE_NOT_RUNNING;
> +	start_next_scan_request(&sr->work);
> +}
> +

Otherwise this looks fine to me.  Does it work?

Regards,
-Denis

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

[Alvin Šipraga]

Hi Denis,

On Thu, Nov 17, 2022 at 11:49:02AM -0600, Denis Kenzior wrote:
> Hi Alvin,
> 
> On 11/17/22 11:29, Alvin Šipraga wrote:
> > From: Alvin Šipraga <alsi@bang-olufsen.dk>
> > 
> > ---
> >   src/scan.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 46 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/scan.c b/src/scan.c
> > index 5548914a12de..c3763ee41173 100644
> > --- a/src/scan.c
> > +++ b/src/scan.c
> 
> <snip>
> 
> > @@ -231,6 +247,7 @@ static void scan_request_triggered(struct l_genl_msg *msg, void *userdata)
> >   	if (err < 0) {
> >   		/* Scan in progress, assume another scan is running */
> >   		if (err == -EBUSY) {
> > +			l_debug("XXX busy");
> 
> This might need a nicer message

Sure, it was just an RFC patch. I'll send a proper patch w/ commit
message later :)

> 
> >   			sc->state = SCAN_STATE_PASSIVE;
> >   			return;
> >   		}
> > @@ -2044,6 +2061,29 @@ static void scan_parse_result_frequencies(struct l_genl_msg *msg,
> >   	}
> >   }
> > +static void scan_retry_pending(uint32_t wiphy_id)
> > +{
> > +	struct scan_context *sc = l_queue_find(scan_contexts,
> > +					       scan_context_match_retry,
> > +					       &wiphy_id);
> > +	struct scan_request *sr;
> > +
> > +	l_debug("");
> > +
> > +	if (!sc)
> > +		return;
> > +
> > +	sr = l_queue_peek_head(sc->requests);
> > +
> > +	if (L_WARN_ON(!sr))
> > +		return;
> 
> Also you could simplify out the need for this L_WARN_ON by using
> l_queue_get_entries() style for loop on the scan_context(s) instead of using
> l_queue_find.  But that's just me nitpicking.

Alright, I'll try and improve it before sending again.

> 
> > +
> > +	l_debug("XXX retry pending request");
> > +
> > +	sc->state = SCAN_STATE_NOT_RUNNING;
> > +	start_next_scan_request(&sr->work);
> > +}
> > +
> 
> Otherwise this looks fine to me.  Does it work?

Yes!

Thanks for the quick review!

Kind regards,
Alvin