[PATCH 1/6] scan: rework BSS ranking

[PATCH 1/6] scan: rework BSS ranking

[James Prestwood]

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

It was observed that IWD's ranking for BSS's did not always
end up with the fastest being chosen. This was due to IWD's
heavy weight on signal strength. This is a decent way of ranking
but even better is calculating a theoretical data rate which
was also done and factored in. The problem is the data rate
factor was always outdone by the signal strength.

Intead remove signal strength entirely as this is already taken
into account with the data rate calculation.

There were a few other factors removed which will be added back
when ranking *networks* rather than BSS's. WPA version (or open)
was removed as well as the privacy capability. These values really
should not differ between BSS's in the same SSID and as such
should be used for network ranking instead.
---
 src/scan.c | 33 +++++----------------------------
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/src/scan.c b/src/scan.c
index a37e1714..afcd6c1d 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -1326,40 +1326,13 @@ static struct scan_bss *scan_parse_result(struct l_genl_msg *msg,
 
 static void scan_bss_compute_rank(struct scan_bss *bss)
 {
-	static const double RANK_RSNE_FACTOR = 1.2;
-	static const double RANK_WPA_FACTOR = 1.0;
-	static const double RANK_OPEN_FACTOR = 0.5;
-	static const double RANK_NO_PRIVACY_FACTOR = 0.5;
 	static const double RANK_HIGH_UTILIZATION_FACTOR = 0.8;
 	static const double RANK_LOW_UTILIZATION_FACTOR = 1.2;
 	static const double RANK_MIN_SUPPORTED_RATE_FACTOR = 0.6;
 	static const double RANK_MAX_SUPPORTED_RATE_FACTOR = 1.3;
-	double rank;
+	double rank = 10000;
 	uint32_t irank;
 
-	/*
-	 * Signal strength is in mBm (100 * dBm) and is negative.
-	 * WiFi range is -0 to -100 dBm
-	 */
-
-	/* Heavily slanted towards signal strength */
-	rank = 10000 + bss->signal_strength;
-
-	/*
-	 * Prefer RSNE first, WPA second.  Open networks are much less
-	 * desirable.
-	 */
-	if (bss->rsne)
-		rank *= RANK_RSNE_FACTOR;
-	else if (bss->wpa)
-		rank *= RANK_WPA_FACTOR;
-	else
-		rank *= RANK_OPEN_FACTOR;
-
-	/* We prefer networks with CAP PRIVACY */
-	if (!(bss->capability & IE_BSS_CAP_PRIVACY))
-		rank *= RANK_NO_PRIVACY_FACTOR;
-
 	/* Prefer 5G networks over 2.4G */
 	if (bss->frequency > 4000)
 		rank *= RANK_5G_FACTOR;
@@ -1373,6 +1346,10 @@ static void scan_bss_compute_rank(struct scan_bss *bss)
 	if (bss->has_sup_rates || bss->ext_supp_rates_ie) {
 		uint64_t data_rate;
 
+		/*
+		 * Signal strength is in mBm (100 * dBm) and is negative.
+		 * WiFi range is -0 to -100 dBm
+		 */
 		if (ie_parse_data_rates(bss->has_sup_rates ?
 					bss->supp_rates_ie : NULL,
 					bss->ext_supp_rates_ie,
-- 
2.31.1

[PATCH 2/6] network: use WPA version and privacy for ranking

[James Prestwood]

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

These ranking factors were moved out of scan.c and into
network.c as they are more relevant for network ranking
than BSS ranking.
---
 src/network.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/network.c b/src/network.c
index 5bd57777..feedf99f 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1446,6 +1446,10 @@ int network_rank_compare(const void *a, const void *b, void *user)
 
 void network_rank_update(struct network *network, bool connected)
 {
+	static const double RANK_RSNE_FACTOR = 1.2;
+	static const double RANK_WPA_FACTOR = 1.0;
+	static const double RANK_OPEN_FACTOR = 0.5;
+	static const double RANK_NO_PRIVACY_FACTOR = 0.5;
 	/*
 	 * Theoretically there may be difference between the BSS selection
 	 * here and in network_bss_select but those should be rare cases.
@@ -1485,6 +1489,21 @@ void network_rank_update(struct network *network, bool connected)
 		network->rank = rankmod_table[n] * best_bss->rank + USHRT_MAX;
 	} else
 		network->rank = best_bss->rank;
+
+	/*
+	 * Prefer RSNE first, WPA second.  Open networks are much less
+	 * desirable.
+	 */
+	if (best_bss->rsne)
+		network->rank *= RANK_RSNE_FACTOR;
+	else if (best_bss->wpa)
+		network->rank *= RANK_WPA_FACTOR;
+	else
+		network->rank *= RANK_OPEN_FACTOR;
+
+	/* We prefer networks with CAP PRIVACY */
+	if (!(best_bss->capability & IE_BSS_CAP_PRIVACY))
+		network->rank *= RANK_NO_PRIVACY_FACTOR;
 }
 
 static void network_unset_hotspot(struct network *network, void *user_data)
-- 
2.31.1

[PATCH 3/6] station: autoconnect based on network, not BSS

[James Prestwood]

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

Prior to the BSS blacklist a BSS based autoconnect list made
the most sense, but now station actually retries all BSS's upon
failure. This means that for each BSS in the autoconnect list
every other BSS under that SSID will be attempted to connect to
if there is a failure. Essentially this is a network based
autoconnect list, just an indirect way of doing it.

Intead the autoconnect list can be purely network based, using
the network rank for sorting. This avoids the need for a special
autoconnect_entry struct as well as ensures the last connected
network is chosen first (simply based on existing network ranking
logic).
---
 src/station.c | 66 +++++++++++----------------------------------------
 1 file changed, 14 insertions(+), 52 deletions(-)

diff --git a/src/station.c b/src/station.c
index 479f81f5..aa9a70aa 100644
--- a/src/station.c
+++ b/src/station.c
@@ -152,12 +152,6 @@ static bool station_is_autoconnecting(struct station *station)
 			station->state == STATION_STATE_AUTOCONNECT_QUICK;
 }
 
-struct autoconnect_entry {
-	uint16_t rank;
-	struct network *network;
-	struct scan_bss *bss;
-};
-
 static void station_property_set_scanning(struct station *station,
 								bool scanning)
 {
@@ -176,25 +170,23 @@ static void station_enter_state(struct station *station,
 
 static void station_autoconnect_next(struct station *station)
 {
-	struct autoconnect_entry *entry;
+	struct network *network;
 	int r;
 
-	while ((entry = l_queue_pop_head(station->autoconnect_list))) {
-		const char *ssid = network_get_ssid(entry->network);
+	while ((network = l_queue_pop_head(station->autoconnect_list))) {
+		const char *ssid = network_get_ssid(network);
+		struct scan_bss *bss = network_bss_select(network, false);
 
 		l_debug("Considering autoconnecting to BSS '%s' with SSID: %s,"
 			" freq: %u, rank: %u, strength: %i",
-			util_address_to_string(entry->bss->addr), ssid,
-			entry->bss->frequency, entry->rank,
-			entry->bss->signal_strength);
+			util_address_to_string(bss->addr), ssid,
+			bss->frequency, bss->rank,
+			bss->signal_strength);
 
-		if (blacklist_contains_bss(entry->bss->addr)) {
-			l_free(entry);
+		if (blacklist_contains_bss(bss->addr))
 			continue;
-		}
 
-		r = network_autoconnect(entry->network, entry->bss);
-		l_free(entry);
+		r = network_autoconnect(network, bss);
 
 		if (!r) {
 			station_enter_state(station, STATION_STATE_CONNECTING);
@@ -212,33 +204,6 @@ static void station_autoconnect_next(struct station *station)
 	}
 }
 
-static int autoconnect_rank_compare(const void *a, const void *b, void *user)
-{
-	const struct autoconnect_entry *new_ae = a;
-	const struct autoconnect_entry *ae = b;
-
-	return (ae->rank > new_ae->rank) ? 1 : -1;
-}
-
-static void station_add_autoconnect_bss(struct station *station,
-					struct network *network,
-					struct scan_bss *bss)
-{
-	double rankmod;
-	struct autoconnect_entry *entry;
-
-	/* See if network is autoconnectable (is a known network) */
-	if (!network_rankmod(network, &rankmod))
-		return;
-
-	entry = l_new(struct autoconnect_entry, 1);
-	entry->network = network;
-	entry->bss = bss;
-	entry->rank = bss->rank * rankmod;
-	l_queue_insert(station->autoconnect_list, entry,
-				autoconnect_rank_compare, NULL);
-}
-
 static void bss_free(void *data)
 {
 	struct scan_bss *bss = data;
@@ -457,12 +422,9 @@ static bool match_nai_realms(const struct network_info *info, void *user_data)
 static void network_add_foreach(struct network *network, void *user_data)
 {
 	struct station *station = user_data;
-	struct scan_bss *bss = network_bss_select(network, false);
-
-	if (!bss)
-		return;
 
-	station_add_autoconnect_bss(station, network, bss);
+	l_queue_insert(station->autoconnect_list, network,
+				network_rank_compare, NULL);
 }
 
 static bool match_pending(const void *a, const void *b)
@@ -553,7 +515,7 @@ request_done:
 	/* Notify all watchers now that every ANQP request has finished */
 	l_queue_foreach_remove(station->anqp_pending, anqp_entry_foreach, NULL);
 
-	l_queue_destroy(station->autoconnect_list, l_free);
+	l_queue_destroy(station->autoconnect_list, NULL);
 	station->autoconnect_list = l_queue_new();
 
 	if (station_is_autoconnecting(station)) {
@@ -662,7 +624,7 @@ void station_set_scan_results(struct station *station,
 
 	l_queue_clear(station->hidden_bss_list_sorted, NULL);
 
-	l_queue_destroy(station->autoconnect_list, l_free);
+	l_queue_destroy(station->autoconnect_list, NULL);
 	station->autoconnect_list = l_queue_new();
 
 	station_bss_list_remove_expired_bsses(station, freqs);
@@ -3600,7 +3562,7 @@ static void station_free(struct station *station)
 	l_hashmap_destroy(station->networks, network_free);
 	l_queue_destroy(station->bss_list, bss_free);
 	l_queue_destroy(station->hidden_bss_list_sorted, NULL);
-	l_queue_destroy(station->autoconnect_list, l_free);
+	l_queue_destroy(station->autoconnect_list, NULL);
 
 	watchlist_destroy(&station->state_watches);
 
-- 
2.31.1

[PATCH 4/6] netdev: introduce [General].RoamThreshold5G

[James Prestwood]

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

This value sets the roaming threshold on 5GHz networks. The
threshold has been separated from 2.4GHz because in many cases
5GHz can perform much better at low RSSI than 2.4GHz.

In addition the BSS ranking logic was re-worked and now 5GHz is
much more preferred, even at low RSSI. This means we need a
lower floor for RSSI before roaming, otherwise IWD would end
up roaming immediately after connecting due to low RSSI CQM
events.
---
 src/netdev.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index 4fbe813a..9e7fc821 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -891,6 +891,7 @@ struct netdev *netdev_find(int ifindex)
 
 /* Threshold RSSI for roaming to trigger, configurable in main.conf */
 static int LOW_SIGNAL_THRESHOLD;
+static int LOW_SIGNAL_THRESHOLD_5GHZ;
 
 static void netdev_cqm_event_rssi_threshold(struct netdev *netdev,
 						uint32_t rssi_event)
@@ -925,6 +926,8 @@ static void netdev_cqm_event_rssi_value(struct netdev *netdev, int rssi_val)
 {
 	bool new_rssi_low;
 	uint8_t prev_rssi_level_idx = netdev->cur_rssi_level_idx;
+	int threshold = netdev->frequency > 4000 ? LOW_SIGNAL_THRESHOLD_5GHZ :
+						LOW_SIGNAL_THRESHOLD;
 
 	if (!netdev->connected)
 		return;
@@ -939,7 +942,7 @@ static void netdev_cqm_event_rssi_value(struct netdev *netdev, int rssi_val)
 	if (!netdev->event_filter)
 		return;
 
-	new_rssi_low = rssi_val < LOW_SIGNAL_THRESHOLD;
+	new_rssi_low = rssi_val < threshold;
 	if (netdev->cur_rssi_low != new_rssi_low) {
 		int event = new_rssi_low ?
 			NETDEV_EVENT_RSSI_THRESHOLD_LOW :
@@ -4806,9 +4809,11 @@ static struct l_genl_msg *netdev_build_cmd_cqm_rssi_update(
 	uint32_t hyst = 5;
 	int thold_count;
 	int32_t thold_list[levels_num + 2];
+	int threshold = netdev->frequency > 4000 ? LOW_SIGNAL_THRESHOLD_5GHZ :
+						LOW_SIGNAL_THRESHOLD;
 
 	if (levels_num == 0) {
-		thold_list[0] = LOW_SIGNAL_THRESHOLD;
+		thold_list[0] = threshold;
 		thold_count = 1;
 	} else {
 		/*
@@ -4827,13 +4832,12 @@ static struct l_genl_msg *netdev_build_cmd_cqm_rssi_update(
 			if (i && thold_list[thold_count - 1] >= val)
 				return NULL;
 
-			if (val >= LOW_SIGNAL_THRESHOLD && !low_sig_added) {
-				thold_list[thold_count++] =
-					LOW_SIGNAL_THRESHOLD;
+			if (val >= threshold && !low_sig_added) {
+				thold_list[thold_count++] = threshold;
 				low_sig_added = true;
 
 				/* Duplicate values are not allowed */
-				if (val == LOW_SIGNAL_THRESHOLD)
+				if (val == threshold)
 					continue;
 			}
 
@@ -5833,6 +5837,10 @@ static int netdev_init(void)
 					&LOW_SIGNAL_THRESHOLD))
 		LOW_SIGNAL_THRESHOLD = -70;
 
+	if (!l_settings_get_int(settings, "General", "RoamThreshold5G",
+					&LOW_SIGNAL_THRESHOLD_5GHZ))
+		LOW_SIGNAL_THRESHOLD_5GHZ = -76;
+
 	if (!l_settings_get_bool(settings, "General", "ControlPortOverNL80211",
 					&pae_over_nl80211))
 		pae_over_nl80211 = true;
-- 
2.31.1

[PATCH 5/6] doc: document [General].RoamThreshold5G

[James Prestwood]

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

---
 src/iwd.config.rst | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 478bd616..8e3662d4 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -131,7 +131,14 @@ The group ``[General]`` contains general settings.
    * - RoamThreshold
      - Value: rssi dBm value, from -100 to 1, default: **-70**
 
-       This can be used to control how aggressively **iwd** roams.
+       This can be used to control how aggressively **iwd** roams from 2.4Ghz
+       networks.
+
+   * - RoamThreshold5G
+     - Value: rssi dBm value, from -100 to 1, default: **-76**
+
+       This value can be used to control how aggressively **iwd** roams from
+       5GHz networks.
 
    * - RoamRetryInterval
      - Value: unsigned int value in seconds (default: **60**)
-- 
2.31.1

[PATCH 6/6] test-runner: remove stale file after test

[James Prestwood]

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

test-runner will print out if files were left behind after a
test which lets the developer know something was not cleaned
up. But in this case test-runner should also remove these files
so they are not left, and printed, for each subsequent test.
---
 tools/test-runner | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/test-runner b/tools/test-runner
index dd4cbf82..892e63ff 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -1186,6 +1186,7 @@ def post_test(ctx, to_copy):
 	allowed = ['phonesim.conf', 'certs', 'secrets', 'iwd']
 	for f in [f for f in os.listdir('/tmp') if f not in allowed]:
 		dbg("File %s was not cleaned up!" % f)
+		os.remove('/tmp/' + f)
 
 	if ctx.args.valgrind:
 		with open('/tmp/valgrind.log', 'r') as f:
-- 
2.31.1