diff mbox series

[v4,07/11] station: roam blacklist BSS's, and consider when roaming

Message ID 20250328144253.421425-7-prestwoj@gmail.com (mailing list archive)
State New
Headers show
Series [v4,01/11] station: always add BSS to network blacklist on failure | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-ci-gitlint success GitLint

Commit Message

James Prestwood March 28, 2025, 2:42 p.m. UTC 
If the BSS is requesting IWD roam elsewhere add this BSS to the
blacklist using BLACKLIST_REASON_ROAM_REQUESTED. This will lower
the chances of IWD roaming/connecting back to this BSS in the
future.

This then allows IWD to consider this blacklist state when picking
a roam candidate. Its undesireable to fully ban a roam blacklisted
BSS, so some additional sorting logic has been added. Prior to
comparing based on rank, BSS's will be sorted into two higher level
groups:

Above Threshold - BSS is above the CriticalRoamThreshold
Below Threshold - BSS is below the CriticalRoamThreshold

Within each of these groups the BSS may be roam blacklisted which
will position it at the bottom of the list within its respecitve
group.
---
 src/station.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 69 insertions(+), 6 deletions(-)

Comments

Denis Kenzior April 1, 2025, 4:08 p.m. UTC | #1 
Hi James,

On 3/28/25 9:42 AM, James Prestwood wrote:
> If the BSS is requesting IWD roam elsewhere add this BSS to the
> blacklist using BLACKLIST_REASON_ROAM_REQUESTED. This will lower
> the chances of IWD roaming/connecting back to this BSS in the
> future.
> 
> This then allows IWD to consider this blacklist state when picking
> a roam candidate. Its undesireable to fully ban a roam blacklisted
> BSS, so some additional sorting logic has been added. Prior to
> comparing based on rank, BSS's will be sorted into two higher level
> groups:
> 
> Above Threshold - BSS is above the CriticalRoamThreshold
> Below Threshold - BSS is below the CriticalRoamThreshold

As discussed, I amended this to read RoamThreshold since that is what patch 6 is 
returning.

> 
> Within each of these groups the BSS may be roam blacklisted which
> will position it at the bottom of the list within its respecitve
> group.
> ---
>   src/station.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 69 insertions(+), 6 deletions(-)
> 

<snip>

> + *
> + * For an initial (probably naive) approach here we can use the
> + * CriticalSignalThreshod[5G] which indicates the signal level that would not

ditto here.

> + * be of an acceptable connection quality. BSS can then be sorted either
> + * above or below this threshold. Within each of these groups a BSS may be
> + * blacklisted, meaning it should get sorted lower on the list compared to
> + * others within the same group.
> + *

All applied, thanks.

Regards,
-Denis
diff mbox series

Patch

diff --git a/src/station.c b/src/station.c
index e2ed78f3..623cfdca 100644
--- a/src/station.c
+++ b/src/station.c
@@ -155,6 +155,55 @@  struct anqp_entry {
 	uint32_t pending;
 };
 
+/*
+ * Rather than sorting BSS's purely based on ranking a higher level grouping
+ * is used. The purpose of this higher order grouping is the consider the BSS's
+ * roam blacklist status. The roam blacklist is a "soft" blacklist in that we
+ * still should connect to these BSS's if they are the only "good" option.
+ * The question here is: what makes a BSS "good" vs "bad".
+ *
+ * For an initial (probably naive) approach here we can use the
+ * CriticalSignalThreshod[5G] which indicates the signal level that would not
+ * be of an acceptable connection quality. BSS can then be sorted either
+ * above or below this threshold. Within each of these groups a BSS may be
+ * blacklisted, meaning it should get sorted lower on the list compared to
+ * others within the same group.
+ *
+ * This sorting is achieved by extending rank to a uint32_t where the first 16
+ * bits are the standard rank calculated by scan.c. Above that bits can be
+ * reserved for this higher level grouping:
+ *
+ * Bit 16 indicates the BSS is not blacklisted
+ * Bit 17 indicates the BSS is above the critical signal threshold
+ */
+
+#define ABOVE_THRESHOLD_BIT 17
+#define NOT_BLACKLISTED_BIT 16
+
+static uint32_t evaluate_bss_group_rank(const uint8_t *addr, uint32_t freq,
+					int16_t signal_strength, uint16_t rank)
+{
+	int signal = signal_strength / 100;
+	bool roam_blacklist;
+	bool good_signal;
+	uint32_t rank_out = (uint32_t) rank;
+
+	if (blacklist_contains_bss(addr, BLACKLIST_REASON_CONNECT_FAILED))
+		return 0;
+
+	roam_blacklist = blacklist_contains_bss(addr,
+					BLACKLIST_REASON_ROAM_REQUESTED);
+	good_signal = signal >= netdev_get_low_signal_threshold(freq);
+
+	if (good_signal)
+		set_bit(&rank_out, ABOVE_THRESHOLD_BIT);
+
+	if (!roam_blacklist)
+		set_bit(&rank_out, NOT_BLACKLISTED_BIT);
+
+	return rank_out;
+}
+
 /*
  * Used as entries for the roam list since holding scan_bss pointers directly
  * from station->bss_list is not 100% safe due to the possibility of the
@@ -162,13 +211,13 @@  struct anqp_entry {
  */
 struct roam_bss {
 	uint8_t addr[6];
-	uint16_t rank;
+	uint32_t rank;
 	int32_t signal_strength;
 	bool ft_failed: 1;
 };
 
 static struct roam_bss *roam_bss_from_scan_bss(const struct scan_bss *bss,
-						uint16_t rank)
+						uint32_t rank)
 {
 	struct roam_bss *rbss = l_new(struct roam_bss, 1);
 
@@ -2805,7 +2854,7 @@  static bool station_roam_scan_notify(int err, struct l_queue *bss_list,
 	struct handshake_state *hs = netdev_get_handshake(station->netdev);
 	struct scan_bss *current_bss = station->connected_bss;
 	struct scan_bss *bss;
-	double cur_bss_rank = 0.0;
+	uint32_t cur_bss_group_rank = 0;
 	static const double RANK_FT_FACTOR = 1.3;
 	uint16_t mdid;
 	enum security orig_security, security;
@@ -2834,10 +2883,15 @@  static bool station_roam_scan_notify(int err, struct l_queue *bss_list,
 	 */
 	bss = l_queue_find(bss_list, bss_match_bssid, current_bss->addr);
 	if (bss && !station->ap_directed_roaming) {
-		cur_bss_rank = bss->rank;
+		double cur_bss_rank = bss->rank;
 
 		if (hs->mde && bss->mde_present && l_get_le16(bss->mde) == mdid)
 			cur_bss_rank *= RANK_FT_FACTOR;
+
+		cur_bss_group_rank = evaluate_bss_group_rank(bss->addr,
+						bss->frequency,
+						bss->signal_strength,
+						(uint16_t) cur_bss_rank);
 	}
 
 	/*
@@ -2859,6 +2913,7 @@  static bool station_roam_scan_notify(int err, struct l_queue *bss_list,
 	while ((bss = l_queue_pop_head(bss_list))) {
 		double rank;
 		struct roam_bss *rbss;
+		uint32_t group_rank;
 
 		station_print_scan_bss(bss);
 
@@ -2889,7 +2944,11 @@  static bool station_roam_scan_notify(int err, struct l_queue *bss_list,
 		if (hs->mde && bss->mde_present && l_get_le16(bss->mde) == mdid)
 			rank *= RANK_FT_FACTOR;
 
-		if (rank <= cur_bss_rank)
+		group_rank = evaluate_bss_group_rank(bss->addr, bss->frequency,
+						bss->signal_strength,
+						(uint16_t) rank);
+
+		if (group_rank <= cur_bss_group_rank)
 			goto next;
 
 		/*
@@ -2898,7 +2957,7 @@  static bool station_roam_scan_notify(int err, struct l_queue *bss_list,
 		 */
 		station_update_roam_bss(station, bss);
 
-		rbss = roam_bss_from_scan_bss(bss, rank);
+		rbss = roam_bss_from_scan_bss(bss, group_rank);
 
 		l_queue_insert(station->roam_bss_list, rbss,
 				roam_bss_rank_compare, NULL);
@@ -3268,6 +3327,10 @@  static void station_ap_directed_roam(struct station *station,
 	l_timeout_remove(station->roam_trigger_timeout);
 	station->roam_trigger_timeout = NULL;
 
+	blacklist_add_bss(station->connected_bss->addr,
+				BLACKLIST_REASON_ROAM_REQUESTED);
+	station_debug_event(station, "ap-roam-blacklist-added");
+
 	if (req_mode & WNM_REQUEST_MODE_PREFERRED_CANDIDATE_LIST) {
 		l_debug("roam: AP sent a preferred candidate list");
 		station_neighbor_report_cb(station->netdev, 0, body + pos,