[PATCH v2 0/3] Improve roam scan strategy

[PATCH v2 0/3] Improve roam scan strategy

[Alexander Ganslandt]

I recommend looking at the diff in a side-by-side viewer, the patch
files are looking far more complicated than they actually are...

This is attempt #2 at improving roaming. This removes the previous
timestamp-based approach and instead uses a simple list to keep track of
which frequencies have been scanned in the current roam attempt.

The main idea is to make station_start_roam() the central point for all
roaming. When some function wants to trigger a roam, it calls
station_start_roam() that takes care of all scan logic.

In my own testing I've noticed a big improvement in connection stability
when running around a building. There might be edge-cases I've missed
though, as I don't have a setup to test all different functionality.

Happy to hear your thoughts on this approach and if it can be improved.

Thanks,
Alexander

---
Changes in v2:
- Removed code that tracked scan frequency timestamps
- Added code to track which frequencies have been scanned
- Link to v1: https://lore.kernel.org/r/20250415-roam-scan-improvements-v1-0-12827e086895@axis.com

---
Alexander Ganslandt (3):
      util: add scan_freq_set_size function
      station: improve scan_freqs_order channel subsets
      station: improve roam scan strategy

 src/station.c | 350 +++++++++++++++++++++++++++++++++-------------------------
 src/util.c    |  11 ++
 src/util.h    |   1 +
 3 files changed, 209 insertions(+), 153 deletions(-)
---
base-commit: 3760a49650b39edd623a85a2b9d51645b52d004a
change-id: 20250414-roam-scan-improvements-02116c7c4631

Best regards,
-- 
Alexander Ganslandt <alexander.ganslandt@axis.com>

[PATCH v2 1/3] util: add scan_freq_set_size function

[Alexander Ganslandt]

Return the number of freqs in a scan_freq_set, useful for knowing how
big the set is.
---
 src/util.c | 11 +++++++++++
 src/util.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/src/util.c b/src/util.c
index 65b97c8e..1e1dfc15 100644
--- a/src/util.c
+++ b/src/util.c
@@ -637,6 +637,17 @@ struct scan_freq_set *scan_freq_set_clone(const struct scan_freq_set *set,
 	return new;
 }
 
+uint32_t scan_freq_set_size(struct scan_freq_set *freqs)
+{
+	uint32_t size = 0;
+
+	size += __builtin_popcount(freqs->channels_2ghz);
+	size += l_uintset_size(freqs->channels_5ghz);
+	size += l_uintset_size(freqs->channels_6ghz);
+
+	return size;
+}
+
 /* First 64 entries calculated by 1 / pow(n, 0.3) for n >= 1 */
 static const double rankmod_table[] = {
 	1.0000000000, 0.8122523964, 0.7192230933, 0.6597539554,
diff --git a/src/util.h b/src/util.h
index 8aef2985..7c024c79 100644
--- a/src/util.h
+++ b/src/util.h
@@ -138,6 +138,7 @@ uint32_t *scan_freq_set_to_fixed_array(const struct scan_freq_set *set,
 					size_t *len_out);
 struct scan_freq_set *scan_freq_set_clone(const struct scan_freq_set *set,
 						uint32_t band_mask);
+uint32_t scan_freq_set_size(struct scan_freq_set *freqs);
 
 DEFINE_CLEANUP_FUNC(scan_freq_set_free);
 

-- 
2.39.5

[PATCH v2 2/3] station: improve scan_freqs_order channel subsets

[Alexander Ganslandt]

From: Alexander Ganslandt <alexanga@axis.com>

Splits the scan frequencies into more subsets that have been ordered
such that the more common frequencies appear first, and the more uncommon
frequencies last. Non-DFS channels are also added to the earlier
subsets to prioritize fast-scanning frequencies.

This approach allows iwd to scan the frequencies with the statistically
highest chance for BSSes first, resulting in shorter scan times until a
good BSS is found. In future patches this will also be used when
roaming.
---
 src/station.c | 122 ++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 89 insertions(+), 33 deletions(-)

diff --git a/src/station.c b/src/station.c
index f8069d89..c2e18fc9 100644
--- a/src/station.c
+++ b/src/station.c
@@ -125,7 +125,7 @@ struct station {
 	struct l_queue *roam_bss_list;
 
 	/* Frequencies split into subsets by priority */
-	struct scan_freq_set *scan_freqs_order[3];
+	struct scan_freq_set *scan_freqs_order[5];
 	unsigned int dbus_scan_subset_idx;
 
 	uint32_t wiphy_watch;
@@ -4500,8 +4500,7 @@ static bool station_dbus_scan_results(int err, struct l_queue *bss_list,
 		return false;
 	}
 
-	last_subset = next_idx >= L_ARRAY_SIZE(station->scan_freqs_order) ||
-		station->scan_freqs_order[next_idx] == NULL;
+	last_subset = next_idx >= L_ARRAY_SIZE(station->scan_freqs_order);
 	station->dbus_scan_subset_idx = next_idx;
 
 	station_set_scan_results(station, bss_list, freqs, false);
@@ -4516,6 +4515,15 @@ static bool station_dbus_scan_subset(struct station *station)
 {
 	unsigned int idx = station->dbus_scan_subset_idx;
 
+	/* Find the next non-empty subset */
+	while (idx < L_ARRAY_SIZE(station->scan_freqs_order) &&
+			scan_freq_set_isempty(station->scan_freqs_order[idx]))
+		idx++;
+	station->dbus_scan_subset_idx = idx;
+
+	if (idx >= L_ARRAY_SIZE(station->scan_freqs_order))
+		return false;
+
 	station->dbus_scan_id = station_scan_trigger(station,
 						station->scan_freqs_order[idx],
 						station_dbus_scan_triggered,
@@ -5061,40 +5069,91 @@ static void station_fill_scan_freq_subsets(struct station *station)
 				wiphy_get_supported_freqs(station->wiphy);
 	unsigned int subset_idx = 0;
 
-	/*
-	 * Scan the 2.4GHz "social channels" first, 5GHz second, if supported,
-	 * all other 2.4GHz channels last.  To be refined as needed.
-	 */
+	station->scan_freqs_order[subset_idx] = scan_freq_set_new();
+
+	/* Subset 0: 2.4GHz "social channels" and low 5GHz non-DFS channels */
 	if (allowed_bands & BAND_FREQ_2_4_GHZ) {
-		station->scan_freqs_order[subset_idx] = scan_freq_set_new();
+		/* Channels 1, 6, 11 */
 		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2412);
 		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2437);
 		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2462);
-		subset_idx++;
 	}
 
-	/*
-	 * TODO: It may might sense to split up 5 and 6ghz into separate subsets
-	 *       since the channel set is so large.
-	 */
-	if (allowed_bands & (BAND_FREQ_5_GHZ | BAND_FREQ_6_GHZ)) {
-		uint32_t mask = allowed_bands &
-					(BAND_FREQ_5_GHZ | BAND_FREQ_6_GHZ);
-		struct scan_freq_set *set = scan_freq_set_clone(supported,
-								mask);
-
-		/* 5/6ghz didn't add any frequencies */
-		if (scan_freq_set_isempty(set)) {
-			scan_freq_set_free(set);
-		} else
-			station->scan_freqs_order[subset_idx++] = set;
+	if (allowed_bands & BAND_FREQ_5_GHZ) {
+		/* Channels 32 - 48 */
+		for (int i = 5160; i <= 5240; i+=20) {
+			scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
+		}
 	}
 
-	/* Add remaining 2.4ghz channels to subset */
+	scan_freq_set_constrain(station->scan_freqs_order[subset_idx], supported);
+	station->scan_freqs_order[++subset_idx] = scan_freq_set_new();
+
+	/* Subset 1: Remaining common 2.4GHz channels and high 5GHz non-DFS channels */
 	if (allowed_bands & BAND_FREQ_2_4_GHZ) {
-		station->scan_freqs_order[subset_idx] = scan_freq_set_new();
-		scan_freq_set_foreach(supported, station_add_2_4ghz_freq,
-					station->scan_freqs_order[subset_idx]);
+		/* Channels 2 - 10, except 6 */
+		for (int i = 2417; i < 2462; i+=5) {
+			if (i != 2437)
+				scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
+		}
+	}
+
+	if (allowed_bands & BAND_FREQ_5_GHZ) {
+		/* Channels 149 - 177 */
+		for (int i = 5745; i <= 5885; i+=20) {
+			scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
+		}
+	}
+
+	scan_freq_set_constrain(station->scan_freqs_order[subset_idx], supported);
+	station->scan_freqs_order[++subset_idx] = scan_freq_set_new();
+
+	/* Subset 2: Uncommon 2.4GHz channels and 5GHz DFS channels */
+	if (allowed_bands & BAND_FREQ_2_4_GHZ) {
+		/* Channels 12 - 14 */
+		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2467);
+		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2472);
+		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2484);
+	}
+
+	if (allowed_bands & BAND_FREQ_5_GHZ) {
+		/* Channels 52 - 68 */
+		for (int i = 5260; i <= 5340; i+=20) {
+			scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
+		}
+
+		/* Channels 96 - 144 */
+		for (int i = 5480; i <= 5720; i+=20) {
+			scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
+		}
+	}
+
+	scan_freq_set_constrain(station->scan_freqs_order[subset_idx], supported);
+	station->scan_freqs_order[++subset_idx] = scan_freq_set_new();
+
+	/* Subset 3: 6GHz channels */
+	if (allowed_bands & BAND_FREQ_6_GHZ) {
+		struct scan_freq_set *set = scan_freq_set_clone(supported, BAND_FREQ_6_GHZ);
+
+		if (scan_freq_set_isempty(set))
+			scan_freq_set_free(set);
+		else
+			scan_freq_set_merge(station->scan_freqs_order[subset_idx], set);
+	}
+
+	scan_freq_set_constrain(station->scan_freqs_order[subset_idx], supported);
+	station->scan_freqs_order[++subset_idx] = scan_freq_set_clone(supported, allowed_bands);
+
+	/* All channels that are both supported and allowed should be in the subsets,
+	 * if this is not the case then some new channel has been added that we are
+	 * not tracking, put it in the last subset to make sure it's scanned */
+	for (int i = 0; i < 4; i++) {
+		scan_freq_set_subtract(station->scan_freqs_order[subset_idx],
+				station->scan_freqs_order[i]);
+	}
+
+	if (!scan_freq_set_isempty(station->scan_freqs_order[subset_idx])) {
+		l_warn("Final subset is not empty");
 	}
 
 	/*
@@ -5273,11 +5332,8 @@ static void station_free(struct station *station)
 
 	l_queue_destroy(station->anqp_pending, remove_anqp);
 
-	scan_freq_set_free(station->scan_freqs_order[0]);
-	scan_freq_set_free(station->scan_freqs_order[1]);
-
-	if (station->scan_freqs_order[2])
-		scan_freq_set_free(station->scan_freqs_order[2]);
+	for (uint8_t i = 0; i < L_ARRAY_SIZE(station->scan_freqs_order); i++)
+		scan_freq_set_free(station->scan_freqs_order[i]);
 
 	wiphy_state_watch_remove(station->wiphy, station->wiphy_watch);
 

-- 
2.39.5

Re: [PATCH v2 2/3] station: improve scan_freqs_order channel subsets

[James Prestwood]

Hi Alexander,

On 8/29/25 12:43 AM, Alexander Ganslandt wrote:
> From: Alexander Ganslandt <alexanga@axis.com>
>
> Splits the scan frequencies into more subsets that have been ordered
> such that the more common frequencies appear first, and the more uncommon
> frequencies last. Non-DFS channels are also added to the earlier
> subsets to prioritize fast-scanning frequencies.
>
> This approach allows iwd to scan the frequencies with the statistically
> highest chance for BSSes first, resulting in shorter scan times until a
> good BSS is found. In future patches this will also be used when
> roaming.
> ---
>   src/station.c | 122 ++++++++++++++++++++++++++++++++++++++++++----------------
>   1 file changed, 89 insertions(+), 33 deletions(-)
>
> diff --git a/src/station.c b/src/station.c
> index f8069d89..c2e18fc9 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -125,7 +125,7 @@ struct station {
>   	struct l_queue *roam_bss_list;
>   
>   	/* Frequencies split into subsets by priority */
> -	struct scan_freq_set *scan_freqs_order[3];
> +	struct scan_freq_set *scan_freqs_order[5];
>   	unsigned int dbus_scan_subset_idx;
>   
>   	uint32_t wiphy_watch;
> @@ -4500,8 +4500,7 @@ static bool station_dbus_scan_results(int err, struct l_queue *bss_list,
>   		return false;
>   	}
>   
> -	last_subset = next_idx >= L_ARRAY_SIZE(station->scan_freqs_order) ||
> -		station->scan_freqs_order[next_idx] == NULL;
> +	last_subset = next_idx >= L_ARRAY_SIZE(station->scan_freqs_order);
>   	station->dbus_scan_subset_idx = next_idx;
>   
>   	station_set_scan_results(station, bss_list, freqs, false);
> @@ -4516,6 +4515,15 @@ static bool station_dbus_scan_subset(struct station *station)
>   {
>   	unsigned int idx = station->dbus_scan_subset_idx;
>   
> +	/* Find the next non-empty subset */
> +	while (idx < L_ARRAY_SIZE(station->scan_freqs_order) &&
> +			scan_freq_set_isempty(station->scan_freqs_order[idx]))
> +		idx++;
> +	station->dbus_scan_subset_idx = idx;
> +
> +	if (idx >= L_ARRAY_SIZE(station->scan_freqs_order))
> +		return false;
> +
>   	station->dbus_scan_id = station_scan_trigger(station,
>   						station->scan_freqs_order[idx],
>   						station_dbus_scan_triggered,
> @@ -5061,40 +5069,91 @@ static void station_fill_scan_freq_subsets(struct station *station)
>   				wiphy_get_supported_freqs(station->wiphy);
>   	unsigned int subset_idx = 0;
>   
> -	/*
> -	 * Scan the 2.4GHz "social channels" first, 5GHz second, if supported,
> -	 * all other 2.4GHz channels last.  To be refined as needed.
> -	 */
> +	station->scan_freqs_order[subset_idx] = scan_freq_set_new();
> +
> +	/* Subset 0: 2.4GHz "social channels" and low 5GHz non-DFS channels */
>   	if (allowed_bands & BAND_FREQ_2_4_GHZ) {
> -		station->scan_freqs_order[subset_idx] = scan_freq_set_new();
> +		/* Channels 1, 6, 11 */
>   		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2412);
>   		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2437);
>   		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2462);
> -		subset_idx++;
>   	}
>   
> -	/*
> -	 * TODO: It may might sense to split up 5 and 6ghz into separate subsets
> -	 *       since the channel set is so large.
> -	 */
> -	if (allowed_bands & (BAND_FREQ_5_GHZ | BAND_FREQ_6_GHZ)) {
> -		uint32_t mask = allowed_bands &
> -					(BAND_FREQ_5_GHZ | BAND_FREQ_6_GHZ);
> -		struct scan_freq_set *set = scan_freq_set_clone(supported,
> -								mask);
> -
> -		/* 5/6ghz didn't add any frequencies */
> -		if (scan_freq_set_isempty(set)) {
> -			scan_freq_set_free(set);
> -		} else
> -			station->scan_freqs_order[subset_idx++] = set;
> +	if (allowed_bands & BAND_FREQ_5_GHZ) {
> +		/* Channels 32 - 48 */
> +		for (int i = 5160; i <= 5240; i+=20) {
> +			scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
> +		}
No need for the braces around this loop and others below.
>   	}
>   
> -	/* Add remaining 2.4ghz channels to subset */
> +	scan_freq_set_constrain(station->scan_freqs_order[subset_idx], supported);
> +	station->scan_freqs_order[++subset_idx] = scan_freq_set_new();
> +
> +	/* Subset 1: Remaining common 2.4GHz channels and high 5GHz non-DFS channels */
>   	if (allowed_bands & BAND_FREQ_2_4_GHZ) {
> -		station->scan_freqs_order[subset_idx] = scan_freq_set_new();
> -		scan_freq_set_foreach(supported, station_add_2_4ghz_freq,
> -					station->scan_freqs_order[subset_idx]);
> +		/* Channels 2 - 10, except 6 */
> +		for (int i = 2417; i < 2462; i+=5) {
> +			if (i != 2437)
> +				scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
> +		}
> +	}
> +
> +	if (allowed_bands & BAND_FREQ_5_GHZ) {
> +		/* Channels 149 - 177 */
> +		for (int i = 5745; i <= 5885; i+=20) {
> +			scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
> +		}
> +	}
> +
> +	scan_freq_set_constrain(station->scan_freqs_order[subset_idx], supported);
> +	station->scan_freqs_order[++subset_idx] = scan_freq_set_new();
> +
> +	/* Subset 2: Uncommon 2.4GHz channels and 5GHz DFS channels */
> +	if (allowed_bands & BAND_FREQ_2_4_GHZ) {
> +		/* Channels 12 - 14 */
> +		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2467);
> +		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2472);
> +		scan_freq_set_add(station->scan_freqs_order[subset_idx], 2484);
> +	}
> +
> +	if (allowed_bands & BAND_FREQ_5_GHZ) {
> +		/* Channels 52 - 68 */
> +		for (int i = 5260; i <= 5340; i+=20) {
> +			scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
> +		}
> +
> +		/* Channels 96 - 144 */
> +		for (int i = 5480; i <= 5720; i+=20) {
> +			scan_freq_set_add(station->scan_freqs_order[subset_idx], i);
> +		}
> +	}
> +
> +	scan_freq_set_constrain(station->scan_freqs_order[subset_idx], supported);
> +	station->scan_freqs_order[++subset_idx] = scan_freq_set_new();
> +
> +	/* Subset 3: 6GHz channels */
> +	if (allowed_bands & BAND_FREQ_6_GHZ) {
> +		struct scan_freq_set *set = scan_freq_set_clone(supported, BAND_FREQ_6_GHZ);
> +
> +		if (scan_freq_set_isempty(set))
> +			scan_freq_set_free(set);
> +		else
> +			scan_freq_set_merge(station->scan_freqs_order[subset_idx], set);
> +	}
> +
> +	scan_freq_set_constrain(station->scan_freqs_order[subset_idx], supported);
> +	station->scan_freqs_order[++subset_idx] = scan_freq_set_clone(supported, allowed_bands);
> +
> +	/* All channels that are both supported and allowed should be in the subsets,
> +	 * if this is not the case then some new channel has been added that we are
> +	 * not tracking, put it in the last subset to make sure it's scanned */
> +	for (int i = 0; i < 4; i++) {
> +		scan_freq_set_subtract(station->scan_freqs_order[subset_idx],
> +				station->scan_freqs_order[i]);
> +	}
> +
> +	if (!scan_freq_set_isempty(station->scan_freqs_order[subset_idx])) {
> +		l_warn("Final subset is not empty");
>   	}
>   
>   	/*
> @@ -5273,11 +5332,8 @@ static void station_free(struct station *station)
>   
>   	l_queue_destroy(station->anqp_pending, remove_anqp);
>   
> -	scan_freq_set_free(station->scan_freqs_order[0]);
> -	scan_freq_set_free(station->scan_freqs_order[1]);
> -
> -	if (station->scan_freqs_order[2])
> -		scan_freq_set_free(station->scan_freqs_order[2]);
> +	for (uint8_t i = 0; i < L_ARRAY_SIZE(station->scan_freqs_order); i++)
> +		scan_freq_set_free(station->scan_freqs_order[i]);
>   
>   	wiphy_state_watch_remove(station->wiphy, station->wiphy_watch);
>   
>

[PATCH v2 3/3] station: improve roam scan strategy

[Alexander Ganslandt]

From: Alexander Ganslandt <alexanga@axis.com>

When iwd decides to roam, it scans either neighbor freqs or known freqs
(if neighbors are not available). If it fails to roam after getting the
scan results, it scans ALL freqs. There's a high chance that both
neighbor and/or known scans fail to roam and we end up scanning all
freqs. This is very slow and if you're already moving away from the
current BSS, there's a high chance you will lose connection
completely before the scan is finished.

Instead of scanning all freqs at once, use the already-defined subsets
to optimize the scans. The subsets contain a handful of freqs each and
are ordered to increase the chance of finding a good BSS early. In order
to not scan the same freq multiple times, use a list (scanned_freqs) to
keep track of which freqs have been scanned in the current roam attempt.

When a roam scan is triggered, add the most prioritized freqs to the
list of freqs that should be scanned. The order of priority is:

1. Neighbor freqs
2. Known freqs
3. Subsets, starting with index 0 and incrementing if the subset is
   exhausted

A freq is only added to the scan if it has not yet been scanned in the
current roam attempt. An exception to this are neighbor freqs. They have
a higher chance of containing good BSSes, so they're scanned every 3rd
scan (defined by STATION_SCANS_BEFORE_NEIGHBOR_SCAN).

This approach results in more scans, but fewer freqs per scan, leading
to shorter delays between scan results. It also avoids scanning the same
freqs back-to-back, which is generally not very useful. In combination
with the subset ordering, this increases the chance of finding a good
BSS early and results in better roaming performance.
---
 src/station.c | 228 ++++++++++++++++++++++++++++------------------------------
 1 file changed, 108 insertions(+), 120 deletions(-)

diff --git a/src/station.c b/src/station.c
index c2e18fc9..0aef46ec 100644
--- a/src/station.c
+++ b/src/station.c
@@ -68,6 +68,8 @@
 
 #define STATION_RECENT_NETWORK_LIMIT	5
 #define STATION_RECENT_FREQS_LIMIT	5
+#define STATION_MAX_SCAN_FREQS	10
+#define STATION_SCANS_BEFORE_NEIGHBOR_SCAN	2
 
 static struct l_queue *station_list;
 static uint32_t netdev_watch;
@@ -123,6 +125,9 @@ struct station {
 	/* Set of frequencies to scan first when attempting a roam */
 	struct scan_freq_set *roam_freqs;
 	struct l_queue *roam_bss_list;
+	struct scan_freq_set *scan_freqs;
+	struct scan_freq_set *scanned_freqs;
+	uint8_t roam_scans_since_neighbor_scan;
 
 	/* Frequencies split into subsets by priority */
 	struct scan_freq_set *scan_freqs_order[5];
@@ -141,7 +146,6 @@ struct station {
 	struct handshake_state *hs;
 
 	bool preparing_roam : 1;
-	bool roam_scan_full : 1;
 	bool signal_low : 1;
 	bool ap_directed_roaming : 1;
 	bool scanning : 1;
@@ -1940,7 +1944,6 @@ static void station_roam_state_clear(struct station *station)
 	l_timeout_remove(station->roam_trigger_timeout);
 	station->roam_trigger_timeout = NULL;
 	station->preparing_roam = false;
-	station->roam_scan_full = false;
 	station->signal_low = false;
 	station->netconfig_after_roam = false;
 	station->last_roam_scan = 0;
@@ -2205,27 +2208,6 @@ static void parse_neighbor_report(struct station *station,
 	}
 }
 
-static void station_early_neighbor_report_cb(struct netdev *netdev, int err,
-						const uint8_t *reports,
-						size_t reports_len,
-						void *user_data)
-{
-	struct station *station = user_data;
-
-	if (err == -ENODEV)
-		return;
-
-	l_debug("ifindex: %u, error: %d(%s)",
-			netdev_get_ifindex(station->netdev),
-			err, err < 0 ? strerror(-err) : "");
-
-	if (!reports || err)
-		return;
-
-	parse_neighbor_report(station, reports, reports_len,
-				&station->roam_freqs);
-}
-
 static bool station_try_next_bss(struct station *station)
 {
 	struct scan_bss *next;
@@ -2360,9 +2342,32 @@ static bool netconfig_after_roam(struct station *station)
 	return true;
 }
 
+static void station_neighbor_report_cb(struct netdev *netdev, int err,
+					const uint8_t *reports,
+					size_t reports_len, void *user_data)
+{
+	struct station *station = user_data;
+
+	if (err == -ENODEV)
+		return;
+
+	l_debug("ifindex: %u, error: %d(%s)",
+			netdev_get_ifindex(station->netdev),
+			err, err < 0 ? strerror(-err) : "");
+
+	if (!reports || err) {
+		l_debug("no neighbor report results");
+		return;
+	}
+
+	parse_neighbor_report(station, reports, reports_len, &station->roam_freqs);
+}
+
 static void station_roamed(struct station *station)
 {
-	station->roam_scan_full = false;
+	scan_freq_set_free(station->scanned_freqs);
+	station->scanned_freqs = scan_freq_set_new();
+	station->roam_scans_since_neighbor_scan = STATION_SCANS_BEFORE_NEIGHBOR_SCAN;
 
 	/*
 	 * Schedule another roaming attempt in case the signal continues to
@@ -2382,7 +2387,7 @@ static void station_roamed(struct station *station)
 
 	if (station->connected_bss->cap_rm_neighbor_report) {
 		if (netdev_neighbor_report_req(station->netdev,
-					station_early_neighbor_report_cb) < 0)
+					station_neighbor_report_cb) < 0)
 			l_warn("Could not request neighbor report");
 	}
 
@@ -2404,8 +2409,10 @@ static void station_roam_retry(struct station *station)
 	 * time.
 	 */
 	station->preparing_roam = false;
-	station->roam_scan_full = false;
 	station->ap_directed_roaming = false;
+	scan_freq_set_free(station->scanned_freqs);
+	station->scanned_freqs = scan_freq_set_new();
+	station->roam_scans_since_neighbor_scan = STATION_SCANS_BEFORE_NEIGHBOR_SCAN;
 
 	if (station->roam_freqs) {
 		scan_freq_set_free(station->roam_freqs);
@@ -2416,6 +2423,8 @@ static void station_roam_retry(struct station *station)
 		station_roam_timeout_rearm(station, roam_retry_interval);
 }
 
+static void station_start_roam(struct station *station);
+
 static void station_roam_failed(struct station *station)
 {
 	l_debug("%u", netdev_get_ifindex(station->netdev));
@@ -2438,39 +2447,22 @@ static void station_roam_failed(struct station *station)
 	}
 
 	/*
-	 * We were told by the AP to roam, but failed.  Try ourselves or
-	 * wait for the AP to tell us to roam again
+	 * Keep trying to roam if the signal is still low, or we were told by the AP
+	 * to roam but failed.
 	 */
-	if (station->ap_directed_roaming) {
-		/*
-		 * The candidate list from the AP (or neighbor report) found
-		 * no BSS's. Force a full scan
-		 */
-		if (!station->roam_scan_full)
-			goto full_scan;
-
-		goto delayed_retry;
-	}
-
-	/*
-	 * If we tried a limited scan, failed and the signal is still low,
-	 * repeat with a full scan right away
-	 */
-	if (station->signal_low && !station->roam_scan_full) {
+	if (station->signal_low || station->ap_directed_roaming) {
 		/*
 		 * Since we're re-using roam_scan_id, explicitly cancel
 		 * the scan here, so that the destroy callback is not called
 		 * after the return of this function
 		 */
-full_scan:
 		scan_cancel(netdev_get_wdev_id(station->netdev),
 						station->roam_scan_id);
 
-		if (!station_roam_scan(station, NULL))
-			return;
+		station_start_roam(station);
+		return;
 	}
 
-delayed_retry:
 	station_roam_retry(station);
 }
 
@@ -3057,7 +3049,6 @@ static int station_roam_scan(struct station *station,
 	}
 
 	if (!freq_set) {
-		station->roam_scan_full = true;
 		params.freqs = allowed;
 		station_debug_event(station, "full-roam-scan");
 	} else
@@ -3080,71 +3071,68 @@ static int station_roam_scan(struct station *station,
 	return 0;
 }
 
-static int station_roam_scan_known_freqs(struct station *station)
+static void station_filter_roam_scan_freq(uint32_t freq, void *user_data)
 {
-	const struct network_info *info = network_get_info(
-						station->connected_network);
-	struct scan_freq_set *freqs = network_info_get_roam_frequencies(info,
-					station->connected_bss->frequency,
-					STATION_RECENT_FREQS_LIMIT);
-	int r = -ENODATA;
-
-	if (!freqs)
-		return r;
+	struct station *station = user_data;
 
-	if (!wiphy_constrain_freq_set(station->wiphy, freqs))
-		goto free_set;
+	if (scan_freq_set_size(station->scan_freqs) >= STATION_MAX_SCAN_FREQS)
+		return;
 
-	r = station_roam_scan(station, freqs);
+	/* Skip freq if already scanned */
+	if (scan_freq_set_contains(station->scanned_freqs, freq))
+		return;
 
-free_set:
-	scan_freq_set_free(freqs);
-	return r;
+	scan_freq_set_add(station->scan_freqs, freq);
+	scan_freq_set_add(station->scanned_freqs, freq);
 }
 
-static void station_neighbor_report_cb(struct netdev *netdev, int err,
-					const uint8_t *reports,
-					size_t reports_len, void *user_data)
+static void station_populate_roam_scan_freqs(struct station *station)
 {
-	struct station *station = user_data;
-	struct scan_freq_set *freq_set;
-	int r;
+	struct scan_freq_set *tmp;
 
-	if (err == -ENODEV)
-		return;
+	station->scan_freqs = scan_freq_set_new();
 
-	l_debug("ifindex: %u, error: %d(%s)",
-			netdev_get_ifindex(station->netdev),
-			err, err < 0 ? strerror(-err) : "");
+	/* Add current frequency, always scan this to get updated data for the
+	 * current BSS */
+	scan_freq_set_add(station->scan_freqs, station->connected_bss->frequency);
+	scan_freq_set_add(station->scanned_freqs, station->connected_bss->frequency);
 
-	/*
-	 * Check if we're still attempting to roam -- if dbus Disconnect
-	 * had been called in the meantime we just abort the attempt.
-	 */
-	if (!station->preparing_roam || err == -ENODEV)
+	/* Add neighbor frequencies */
+	if (station->roam_scans_since_neighbor_scan >=
+			STATION_SCANS_BEFORE_NEIGHBOR_SCAN && station->roam_freqs) {
+		station->roam_scans_since_neighbor_scan = 0;
+		scan_freq_set_merge(station->scan_freqs, station->roam_freqs);
+		scan_freq_set_merge(station->scanned_freqs, station->roam_freqs);
 		return;
+	}
+	station->roam_scans_since_neighbor_scan++;
 
-	if (!reports || err) {
-		r = station_roam_scan_known_freqs(station);
-
-		if (r == -ENODATA)
-			l_debug("no neighbor report results or known freqs");
-
-		if (r < 0)
-			station_roam_failed(station);
-
+	/* Add known frequencies */
+	const struct network_info *info = network_get_info(
+						station->connected_network);
+	tmp = network_info_get_roam_frequencies(info,
+					station->connected_bss->frequency,
+					STATION_RECENT_FREQS_LIMIT);
+	scan_freq_set_foreach(tmp, station_filter_roam_scan_freq, station);
+	scan_freq_set_free(tmp);
+	if (scan_freq_set_size(station->scan_freqs) >= STATION_MAX_SCAN_FREQS) {
 		return;
 	}
 
-	parse_neighbor_report(station, reports, reports_len, &freq_set);
-
-	r = station_roam_scan(station, freq_set);
-
-	if (freq_set)
-		scan_freq_set_free(freq_set);
+	/* Add frequencies based on the prioritized subsets */
+	for (uint8_t i = 0; i < L_ARRAY_SIZE(station->scan_freqs_order); i++) {
+		scan_freq_set_foreach(station->scan_freqs_order[i], station_filter_roam_scan_freq, station);
+		if (scan_freq_set_size(station->scan_freqs) >= STATION_MAX_SCAN_FREQS) {
+			return;
+		}
+	}
 
-	if (r < 0)
-		station_roam_failed(station);
+	if (scan_freq_set_size(station->scan_freqs) <= STATION_MAX_SCAN_FREQS) {
+		/* All freqs have been scanned after this, so empty the list of scanned
+		 * freqs to restart */
+		scan_freq_set_free(station->scanned_freqs);
+		station->scanned_freqs = scan_freq_set_new();
+	}
 }
 
 static void station_start_roam(struct station *station)
@@ -3154,34 +3142,26 @@ static void station_start_roam(struct station *station)
 	station->preparing_roam = true;
 
 	/*
-	 * If current BSS supports Neighbor Reports, narrow the scan down
-	 * to channels occupied by known neighbors in the ESS. If no neighbor
-	 * report was obtained upon connection, request one now. This isn't
-	 * 100% reliable as the neighbor lists are not required to be
-	 * complete or current.  It is likely still better than doing a
-	 * full scan.  10.11.10.1: "A neighbor report may not be exhaustive
-	 * either by choice, or due to the fact that there may be neighbor
-	 * APs not known to the AP."
+	 * If no neighbor report was obtained upon connection, request one now if BSS
+	 * supports it. This isn't 100% reliable as the neighbor lists are not
+	 * required to be complete or current.
+	 * 10.11.10.1: "A neighbor report may not be exhaustive either by choice, or
+	 * due to the fact that there may be neighbor APs not known to the AP."
+	 *
+	 * Continue roaming while waiting for the neighbor report, the neighbors will
+	 * be added to the roam scan when/if they're available.
 	 */
-	if (station->roam_freqs) {
-		if (station_roam_scan(station, station->roam_freqs) == 0) {
-			l_debug("Using cached neighbor report for roam");
-			return;
-		}
-	} else if (station->connected_bss->cap_rm_neighbor_report) {
+	if (!station->roam_freqs && station->connected_bss->cap_rm_neighbor_report) {
 		if (netdev_neighbor_report_req(station->netdev,
 					station_neighbor_report_cb) == 0) {
 			l_debug("Requesting neighbor report for roam");
-			return;
 		}
 	}
 
-	r = station_roam_scan_known_freqs(station);
-	if (r == -ENODATA)
-		l_debug("No neighbor report or known frequencies, roam failed");
-
-	if (r < 0)
-		station_roam_failed(station);
+	station_populate_roam_scan_freqs(station);
+	station_roam_scan(station, station->scan_freqs);
+	scan_freq_set_free(station->scan_freqs);
+	station->scan_freqs = NULL;
 }
 
 static bool station_cannot_roam(struct station *station)
@@ -3624,7 +3604,7 @@ static void station_connect_ok(struct station *station)
 	 */
 	if (station->connected_bss->cap_rm_neighbor_report) {
 		if (netdev_neighbor_report_req(station->netdev,
-					station_early_neighbor_report_cb) < 0)
+					station_neighbor_report_cb) < 0)
 			l_warn("Could not request neighbor report");
 	}
 
@@ -5246,6 +5226,9 @@ static struct station *station_create(struct netdev *netdev)
 	station->roam_bss_list = l_queue_new();
 	station->affinities = l_queue_new();
 
+	station->scanned_freqs = scan_freq_set_new();
+	station->roam_scans_since_neighbor_scan = STATION_SCANS_BEFORE_NEIGHBOR_SCAN;
+
 	return station;
 }
 
@@ -5344,6 +5327,11 @@ static void station_free(struct station *station)
 
 	l_queue_destroy(station->affinities, l_free);
 
+	scan_freq_set_free(station->scanned_freqs);
+
+	if (station->scan_freqs)
+		scan_freq_set_free(station->scan_freqs);
+
 	l_free(station);
 }
 

-- 
2.39.5

Re: [PATCH v2 3/3] station: improve roam scan strategy

[James Prestwood]

Hi,

On 8/29/25 12:43 AM, Alexander Ganslandt wrote:
> From: Alexander Ganslandt <alexanga@axis.com>
>
> When iwd decides to roam, it scans either neighbor freqs or known freqs
> (if neighbors are not available). If it fails to roam after getting the
> scan results, it scans ALL freqs. There's a high chance that both
> neighbor and/or known scans fail to roam and we end up scanning all
> freqs. This is very slow and if you're already moving away from the
> current BSS, there's a high chance you will lose connection
> completely before the scan is finished.
>
> Instead of scanning all freqs at once, use the already-defined subsets
> to optimize the scans. The subsets contain a handful of freqs each and
> are ordered to increase the chance of finding a good BSS early. In order
> to not scan the same freq multiple times, use a list (scanned_freqs) to
> keep track of which freqs have been scanned in the current roam attempt.
>
> When a roam scan is triggered, add the most prioritized freqs to the
> list of freqs that should be scanned. The order of priority is:
>
> 1. Neighbor freqs
> 2. Known freqs
> 3. Subsets, starting with index 0 and incrementing if the subset is
>     exhausted
>
> A freq is only added to the scan if it has not yet been scanned in the
> current roam attempt. An exception to this are neighbor freqs. They have
> a higher chance of containing good BSSes, so they're scanned every 3rd
> scan (defined by STATION_SCANS_BEFORE_NEIGHBOR_SCAN).
>
> This approach results in more scans, but fewer freqs per scan, leading
> to shorter delays between scan results. It also avoids scanning the same
> freqs back-to-back, which is generally not very useful. In combination
> with the subset ordering, this increases the chance of finding a good
> BSS early and results in better roaming performance.

Overall I agree with this approach, but I'm still looking at this patch 
in detail as its quite intrusive to the roaming logic. One thing to 
address right off the bat is the CI failure. Looks like there were some 
incremental patch issues (applying patches 1-by-1 and building in 
between). This prevented the autotests from running but I suspect we 
will need to alter the autotests a bit due to logic changes in this patch.

Thanks,

James

> ---
>   src/station.c | 228 ++++++++++++++++++++++++++++------------------------------
>   1 file changed, 108 insertions(+), 120 deletions(-)
>
> diff --git a/src/station.c b/src/station.c
> index c2e18fc9..0aef46ec 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -68,6 +68,8 @@
>   
>   #define STATION_RECENT_NETWORK_LIMIT	5
>   #define STATION_RECENT_FREQS_LIMIT	5
> +#define STATION_MAX_SCAN_FREQS	10
> +#define STATION_SCANS_BEFORE_NEIGHBOR_SCAN	2
>   
>   static struct l_queue *station_list;
>   static uint32_t netdev_watch;
> @@ -123,6 +125,9 @@ struct station {
>   	/* Set of frequencies to scan first when attempting a roam */
>   	struct scan_freq_set *roam_freqs;
>   	struct l_queue *roam_bss_list;
> +	struct scan_freq_set *scan_freqs;
> +	struct scan_freq_set *scanned_freqs;
> +	uint8_t roam_scans_since_neighbor_scan;
>   
>   	/* Frequencies split into subsets by priority */
>   	struct scan_freq_set *scan_freqs_order[5];
> @@ -141,7 +146,6 @@ struct station {
>   	struct handshake_state *hs;
>   
>   	bool preparing_roam : 1;
> -	bool roam_scan_full : 1;
>   	bool signal_low : 1;
>   	bool ap_directed_roaming : 1;
>   	bool scanning : 1;
> @@ -1940,7 +1944,6 @@ static void station_roam_state_clear(struct station *station)
>   	l_timeout_remove(station->roam_trigger_timeout);
>   	station->roam_trigger_timeout = NULL;
>   	station->preparing_roam = false;
> -	station->roam_scan_full = false;
>   	station->signal_low = false;
>   	station->netconfig_after_roam = false;
>   	station->last_roam_scan = 0;
> @@ -2205,27 +2208,6 @@ static void parse_neighbor_report(struct station *station,
>   	}
>   }
>   
> -static void station_early_neighbor_report_cb(struct netdev *netdev, int err,
> -						const uint8_t *reports,
> -						size_t reports_len,
> -						void *user_data)
> -{
> -	struct station *station = user_data;
> -
> -	if (err == -ENODEV)
> -		return;
> -
> -	l_debug("ifindex: %u, error: %d(%s)",
> -			netdev_get_ifindex(station->netdev),
> -			err, err < 0 ? strerror(-err) : "");
> -
> -	if (!reports || err)
> -		return;
> -
> -	parse_neighbor_report(station, reports, reports_len,
> -				&station->roam_freqs);
> -}
> -
>   static bool station_try_next_bss(struct station *station)
>   {
>   	struct scan_bss *next;
> @@ -2360,9 +2342,32 @@ static bool netconfig_after_roam(struct station *station)
>   	return true;
>   }
>   
> +static void station_neighbor_report_cb(struct netdev *netdev, int err,
> +					const uint8_t *reports,
> +					size_t reports_len, void *user_data)
> +{
> +	struct station *station = user_data;
> +
> +	if (err == -ENODEV)
> +		return;
> +
> +	l_debug("ifindex: %u, error: %d(%s)",
> +			netdev_get_ifindex(station->netdev),
> +			err, err < 0 ? strerror(-err) : "");
> +
> +	if (!reports || err) {
> +		l_debug("no neighbor report results");
> +		return;
> +	}
> +
> +	parse_neighbor_report(station, reports, reports_len, &station->roam_freqs);
> +}
> +
>   static void station_roamed(struct station *station)
>   {
> -	station->roam_scan_full = false;
> +	scan_freq_set_free(station->scanned_freqs);
> +	station->scanned_freqs = scan_freq_set_new();
> +	station->roam_scans_since_neighbor_scan = STATION_SCANS_BEFORE_NEIGHBOR_SCAN;
>   
>   	/*
>   	 * Schedule another roaming attempt in case the signal continues to
> @@ -2382,7 +2387,7 @@ static void station_roamed(struct station *station)
>   
>   	if (station->connected_bss->cap_rm_neighbor_report) {
>   		if (netdev_neighbor_report_req(station->netdev,
> -					station_early_neighbor_report_cb) < 0)
> +					station_neighbor_report_cb) < 0)
>   			l_warn("Could not request neighbor report");
>   	}
>   
> @@ -2404,8 +2409,10 @@ static void station_roam_retry(struct station *station)
>   	 * time.
>   	 */
>   	station->preparing_roam = false;
> -	station->roam_scan_full = false;
>   	station->ap_directed_roaming = false;
> +	scan_freq_set_free(station->scanned_freqs);
> +	station->scanned_freqs = scan_freq_set_new();
> +	station->roam_scans_since_neighbor_scan = STATION_SCANS_BEFORE_NEIGHBOR_SCAN;
>   
>   	if (station->roam_freqs) {
>   		scan_freq_set_free(station->roam_freqs);
> @@ -2416,6 +2423,8 @@ static void station_roam_retry(struct station *station)
>   		station_roam_timeout_rearm(station, roam_retry_interval);
>   }
>   
> +static void station_start_roam(struct station *station);
> +
>   static void station_roam_failed(struct station *station)
>   {
>   	l_debug("%u", netdev_get_ifindex(station->netdev));
> @@ -2438,39 +2447,22 @@ static void station_roam_failed(struct station *station)
>   	}
>   
>   	/*
> -	 * We were told by the AP to roam, but failed.  Try ourselves or
> -	 * wait for the AP to tell us to roam again
> +	 * Keep trying to roam if the signal is still low, or we were told by the AP
> +	 * to roam but failed.
>   	 */
> -	if (station->ap_directed_roaming) {
> -		/*
> -		 * The candidate list from the AP (or neighbor report) found
> -		 * no BSS's. Force a full scan
> -		 */
> -		if (!station->roam_scan_full)
> -			goto full_scan;
> -
> -		goto delayed_retry;
> -	}
> -
> -	/*
> -	 * If we tried a limited scan, failed and the signal is still low,
> -	 * repeat with a full scan right away
> -	 */
> -	if (station->signal_low && !station->roam_scan_full) {
> +	if (station->signal_low || station->ap_directed_roaming) {
>   		/*
>   		 * Since we're re-using roam_scan_id, explicitly cancel
>   		 * the scan here, so that the destroy callback is not called
>   		 * after the return of this function
>   		 */
> -full_scan:
>   		scan_cancel(netdev_get_wdev_id(station->netdev),
>   						station->roam_scan_id);
>   
> -		if (!station_roam_scan(station, NULL))
> -			return;
> +		station_start_roam(station);
> +		return;
>   	}
>   
> -delayed_retry:
>   	station_roam_retry(station);
>   }
>   
> @@ -3057,7 +3049,6 @@ static int station_roam_scan(struct station *station,
>   	}
>   
>   	if (!freq_set) {
> -		station->roam_scan_full = true;
>   		params.freqs = allowed;
>   		station_debug_event(station, "full-roam-scan");
>   	} else
> @@ -3080,71 +3071,68 @@ static int station_roam_scan(struct station *station,
>   	return 0;
>   }
>   
> -static int station_roam_scan_known_freqs(struct station *station)
> +static void station_filter_roam_scan_freq(uint32_t freq, void *user_data)
>   {
> -	const struct network_info *info = network_get_info(
> -						station->connected_network);
> -	struct scan_freq_set *freqs = network_info_get_roam_frequencies(info,
> -					station->connected_bss->frequency,
> -					STATION_RECENT_FREQS_LIMIT);
> -	int r = -ENODATA;
> -
> -	if (!freqs)
> -		return r;
> +	struct station *station = user_data;
>   
> -	if (!wiphy_constrain_freq_set(station->wiphy, freqs))
> -		goto free_set;
> +	if (scan_freq_set_size(station->scan_freqs) >= STATION_MAX_SCAN_FREQS)
> +		return;
>   
> -	r = station_roam_scan(station, freqs);
> +	/* Skip freq if already scanned */
> +	if (scan_freq_set_contains(station->scanned_freqs, freq))
> +		return;
>   
> -free_set:
> -	scan_freq_set_free(freqs);
> -	return r;
> +	scan_freq_set_add(station->scan_freqs, freq);
> +	scan_freq_set_add(station->scanned_freqs, freq);
>   }
>   
> -static void station_neighbor_report_cb(struct netdev *netdev, int err,
> -					const uint8_t *reports,
> -					size_t reports_len, void *user_data)
> +static void station_populate_roam_scan_freqs(struct station *station)
>   {
> -	struct station *station = user_data;
> -	struct scan_freq_set *freq_set;
> -	int r;
> +	struct scan_freq_set *tmp;
>   
> -	if (err == -ENODEV)
> -		return;
> +	station->scan_freqs = scan_freq_set_new();
>   
> -	l_debug("ifindex: %u, error: %d(%s)",
> -			netdev_get_ifindex(station->netdev),
> -			err, err < 0 ? strerror(-err) : "");
> +	/* Add current frequency, always scan this to get updated data for the
> +	 * current BSS */
> +	scan_freq_set_add(station->scan_freqs, station->connected_bss->frequency);
> +	scan_freq_set_add(station->scanned_freqs, station->connected_bss->frequency);
>   
> -	/*
> -	 * Check if we're still attempting to roam -- if dbus Disconnect
> -	 * had been called in the meantime we just abort the attempt.
> -	 */
> -	if (!station->preparing_roam || err == -ENODEV)
> +	/* Add neighbor frequencies */
> +	if (station->roam_scans_since_neighbor_scan >=
> +			STATION_SCANS_BEFORE_NEIGHBOR_SCAN && station->roam_freqs) {
> +		station->roam_scans_since_neighbor_scan = 0;
> +		scan_freq_set_merge(station->scan_freqs, station->roam_freqs);
> +		scan_freq_set_merge(station->scanned_freqs, station->roam_freqs);
>   		return;
> +	}
> +	station->roam_scans_since_neighbor_scan++;
>   
> -	if (!reports || err) {
> -		r = station_roam_scan_known_freqs(station);
> -
> -		if (r == -ENODATA)
> -			l_debug("no neighbor report results or known freqs");
> -
> -		if (r < 0)
> -			station_roam_failed(station);
> -
> +	/* Add known frequencies */
> +	const struct network_info *info = network_get_info(
> +						station->connected_network);
> +	tmp = network_info_get_roam_frequencies(info,
> +					station->connected_bss->frequency,
> +					STATION_RECENT_FREQS_LIMIT);
> +	scan_freq_set_foreach(tmp, station_filter_roam_scan_freq, station);
> +	scan_freq_set_free(tmp);
> +	if (scan_freq_set_size(station->scan_freqs) >= STATION_MAX_SCAN_FREQS) {
>   		return;
>   	}
>   
> -	parse_neighbor_report(station, reports, reports_len, &freq_set);
> -
> -	r = station_roam_scan(station, freq_set);
> -
> -	if (freq_set)
> -		scan_freq_set_free(freq_set);
> +	/* Add frequencies based on the prioritized subsets */
> +	for (uint8_t i = 0; i < L_ARRAY_SIZE(station->scan_freqs_order); i++) {
> +		scan_freq_set_foreach(station->scan_freqs_order[i], station_filter_roam_scan_freq, station);
> +		if (scan_freq_set_size(station->scan_freqs) >= STATION_MAX_SCAN_FREQS) {
> +			return;
> +		}
> +	}
>   
> -	if (r < 0)
> -		station_roam_failed(station);
> +	if (scan_freq_set_size(station->scan_freqs) <= STATION_MAX_SCAN_FREQS) {
> +		/* All freqs have been scanned after this, so empty the list of scanned
> +		 * freqs to restart */
> +		scan_freq_set_free(station->scanned_freqs);
> +		station->scanned_freqs = scan_freq_set_new();
> +	}
>   }
>   
>   static void station_start_roam(struct station *station)
> @@ -3154,34 +3142,26 @@ static void station_start_roam(struct station *station)
>   	station->preparing_roam = true;
>   
>   	/*
> -	 * If current BSS supports Neighbor Reports, narrow the scan down
> -	 * to channels occupied by known neighbors in the ESS. If no neighbor
> -	 * report was obtained upon connection, request one now. This isn't
> -	 * 100% reliable as the neighbor lists are not required to be
> -	 * complete or current.  It is likely still better than doing a
> -	 * full scan.  10.11.10.1: "A neighbor report may not be exhaustive
> -	 * either by choice, or due to the fact that there may be neighbor
> -	 * APs not known to the AP."
> +	 * If no neighbor report was obtained upon connection, request one now if BSS
> +	 * supports it. This isn't 100% reliable as the neighbor lists are not
> +	 * required to be complete or current.
> +	 * 10.11.10.1: "A neighbor report may not be exhaustive either by choice, or
> +	 * due to the fact that there may be neighbor APs not known to the AP."
> +	 *
> +	 * Continue roaming while waiting for the neighbor report, the neighbors will
> +	 * be added to the roam scan when/if they're available.
>   	 */
> -	if (station->roam_freqs) {
> -		if (station_roam_scan(station, station->roam_freqs) == 0) {
> -			l_debug("Using cached neighbor report for roam");
> -			return;
> -		}
> -	} else if (station->connected_bss->cap_rm_neighbor_report) {
> +	if (!station->roam_freqs && station->connected_bss->cap_rm_neighbor_report) {
>   		if (netdev_neighbor_report_req(station->netdev,
>   					station_neighbor_report_cb) == 0) {
>   			l_debug("Requesting neighbor report for roam");
> -			return;
>   		}
>   	}
>   
> -	r = station_roam_scan_known_freqs(station);
> -	if (r == -ENODATA)
> -		l_debug("No neighbor report or known frequencies, roam failed");
> -
> -	if (r < 0)
> -		station_roam_failed(station);
> +	station_populate_roam_scan_freqs(station);
> +	station_roam_scan(station, station->scan_freqs);
> +	scan_freq_set_free(station->scan_freqs);
> +	station->scan_freqs = NULL;
>   }
>   
>   static bool station_cannot_roam(struct station *station)
> @@ -3624,7 +3604,7 @@ static void station_connect_ok(struct station *station)
>   	 */
>   	if (station->connected_bss->cap_rm_neighbor_report) {
>   		if (netdev_neighbor_report_req(station->netdev,
> -					station_early_neighbor_report_cb) < 0)
> +					station_neighbor_report_cb) < 0)
>   			l_warn("Could not request neighbor report");
>   	}
>   
> @@ -5246,6 +5226,9 @@ static struct station *station_create(struct netdev *netdev)
>   	station->roam_bss_list = l_queue_new();
>   	station->affinities = l_queue_new();
>   
> +	station->scanned_freqs = scan_freq_set_new();
> +	station->roam_scans_since_neighbor_scan = STATION_SCANS_BEFORE_NEIGHBOR_SCAN;
> +
>   	return station;
>   }
>   
> @@ -5344,6 +5327,11 @@ static void station_free(struct station *station)
>   
>   	l_queue_destroy(station->affinities, l_free);
>   
> +	scan_freq_set_free(station->scanned_freqs);
> +
> +	if (station->scan_freqs)
> +		scan_freq_set_free(station->scan_freqs);
> +
>   	l_free(station);
>   }
>   
>

Re: [PATCH v2 3/3] station: improve roam scan strategy

[Alexander Ganslandt]

On 9/9/25 15:35, James Prestwood wrote:
> Overall I agree with this approach, but I'm still looking at this patch 
> in detail as its quite intrusive to the roaming logic. One thing to 
> address right off the bat is the CI failure. Looks like there were some 
> incremental patch issues (applying patches 1-by-1 and building in 
> between). This prevented the autotests from running but I suspect we 
> will need to alter the autotests a bit due to logic changes in this patch.
> 
> Thanks,
> 
> James

Hello James,

Thanks for the feedback! Yes, a lot of core roaming logic has changed, 
so it's good that we take our time to ensure it works well. I found it a 
bit tricky to follow all the paths where scans would be triggered so I 
tried to make all roads lead to station_start_roam(). If I succeeded 
with that in a good way is up for debate :)

I will push a fix for your comment and the CI issues soon!

Thanks again,
Alexander