diff mbox series

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

Message ID 20250325180041.238676-7-prestwoj@gmail.com (mailing list archive)
State New
Headers show
Series [v3,01/10] 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 25, 2025, 6 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 3 groups:

Optimal - Not roam blacklisted, and above the roaming threshold
Above Threshold - May be blacklisted, and above the roaming threshold
Below Threshold - BSS is below the roaming threshold
---
 src/station.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 84 insertions(+), 4 deletions(-)

Comments

James Prestwood March 26, 2025, 5:49 p.m. UTC | #1 
On 3/25/25 11:00 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 3 groups:
>
> Optimal - Not roam blacklisted, and above the roaming threshold
> Above Threshold - May be blacklisted, and above the roaming threshold
> Below Threshold - BSS is below the roaming threshold
> ---
>   src/station.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 84 insertions(+), 4 deletions(-)
>
> diff --git a/src/station.c b/src/station.c
> index e2ed78f3..e3c7c189 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -155,6 +155,57 @@ 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 nieve) approach here we can use the RoamThreshod[5G]
> + * and sort BSS's into "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.
> + *
> + * Since we have several call sites needing to check/compare these groupings
> + * a bitmask can be used to describe the grouping:
> + *
> + * Each group will have 2 bits. The lower order bit signifies the BSS is in the
> + * group, but also blacklisted. The higher order bit signifies the BSS is in
> + * the group, but not blacklisted. The bitmask between two BSS's can then be
> + * compared directly similar to rank.
> + */
> +
> +#define UNDER_THRESHOLD_BIT 1
> +#define ABOVE_THRESHOLD_BIT 3
> +
> +static uint32_t evaluate_bss_group(const uint8_t *addr, uint32_t freq,
> +					int16_t signal_strength)
> +{
> +	int threshold;
> +	int signal = signal_strength / 100;
> +	bool roam_blacklist;
> +	uint32_t mask = 0;
> +
> +	if (blacklist_contains_bss(addr, BLACKLIST_REASON_CONNECT_FAILED))
> +		return mask;
> +
> +	roam_blacklist = blacklist_contains_bss(addr,
> +					BLACKLIST_REASON_ROAM_REQUESTED);
> +
> +	if (freq > 4000)
> +		netdev_get_low_signal_thresholds(NULL, &threshold);
> +	else
> +		netdev_get_low_signal_thresholds(&threshold, NULL);
> +
> +	if (signal >= threshold)
> +		set_bit(&mask, ABOVE_THRESHOLD_BIT - roam_blacklist);
> +	else
> +		set_bit(&mask, UNDER_THRESHOLD_BIT - roam_blacklist);
> +
> +	return mask;
> +}
> +
>   /*
>    * 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
> @@ -164,11 +215,13 @@ struct roam_bss {
>   	uint8_t addr[6];
>   	uint16_t rank;
>   	int32_t signal_strength;
> +	uint32_t group;
>   	bool ft_failed: 1;
>   };
>   
>   static struct roam_bss *roam_bss_from_scan_bss(const struct scan_bss *bss,
> -						uint16_t rank)
> +						uint16_t rank,
> +						uint32_t group)
>   {
>   	struct roam_bss *rbss = l_new(struct roam_bss, 1);
>   
> @@ -176,6 +229,7 @@ static struct roam_bss *roam_bss_from_scan_bss(const struct scan_bss *bss,
>   	rbss->rank = rank;
>   	rbss->signal_strength = bss->signal_strength;
>   	rbss->ft_failed = false;
> +	rbss->group = group;
>   
>   	return rbss;
>   }
> @@ -184,6 +238,12 @@ static int roam_bss_rank_compare(const void *a, const void *b, void *user_data)
>   {
>   	const struct roam_bss *new_bss = a, *bss = b;
>   
> +	if (bss->group > new_bss->group)
> +		return 1;
> +	else if (bss->group < new_bss->group)
> +		return -1;
> +
> +	/* BSS's both have the same group, sort by rank */
>   	if (bss->rank == new_bss->rank)
>   		return (bss->signal_strength >
>   					new_bss->signal_strength) ? 1 : -1;
> @@ -2805,6 +2865,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;
> +	uint32_t cur_bss_group = 0;
>   	double cur_bss_rank = 0.0;
>   	static const double RANK_FT_FACTOR = 1.3;
>   	uint16_t mdid;
> @@ -2835,6 +2896,9 @@ 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;
> +		cur_bss_group = evaluate_bss_group(current_bss->addr,
> +						current_bss->frequency,
> +						current_bss->signal_strength);
This is what caused the CI to fail on testPSK-roam. Instead I should 
have done the group evaluation on the "bss" pointer, not "current_bss". 
This will take into account the most recent scan, rather than the stale 
object we have from past scans.
>   
>   		if (hs->mde && bss->mde_present && l_get_le16(bss->mde) == mdid)
>   			cur_bss_rank *= RANK_FT_FACTOR;
> @@ -2859,6 +2923,9 @@ 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 = evaluate_bss_group(bss->addr,
> +							bss->frequency,
> +							bss->signal_strength);
>   
>   		station_print_scan_bss(bss);
>   
> @@ -2889,7 +2956,15 @@ 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)
> +		/*
> +		 * First check the group:
> +		 *   - If worse, disregard BSS candidate
> +		 *   - If better, keep BSS candidate
> +		 *   - If equal, compare based on rank
> +		 */
> +		if (group < cur_bss_group)
> +			goto next;
> +		else if (group == cur_bss_group && rank <= cur_bss_rank)
>   			goto next;
>   
>   		/*
> @@ -2898,7 +2973,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, rank, group);
>   
>   		l_queue_insert(station->roam_bss_list, rbss,
>   				roam_bss_rank_compare, NULL);
> @@ -3268,6 +3343,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,
> @@ -5344,7 +5423,8 @@ static bool station_force_roam_scan_notify(int err, struct l_queue *bss_list,
>   	/* The various roam routines expect this to be set from scanning */
>   	station->preparing_roam = true;
>   	l_queue_push_tail(station->roam_bss_list,
> -				roam_bss_from_scan_bss(target, target->rank));
> +				roam_bss_from_scan_bss(target, target->rank,
> +				0xffff));
>   
>   	station_update_roam_bss(station, target);
>
diff mbox series

Patch

diff --git a/src/station.c b/src/station.c
index e2ed78f3..e3c7c189 100644
--- a/src/station.c
+++ b/src/station.c
@@ -155,6 +155,57 @@  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 nieve) approach here we can use the RoamThreshod[5G]
+ * and sort BSS's into "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.
+ *
+ * Since we have several call sites needing to check/compare these groupings
+ * a bitmask can be used to describe the grouping:
+ *
+ * Each group will have 2 bits. The lower order bit signifies the BSS is in the
+ * group, but also blacklisted. The higher order bit signifies the BSS is in
+ * the group, but not blacklisted. The bitmask between two BSS's can then be
+ * compared directly similar to rank.
+ */
+
+#define UNDER_THRESHOLD_BIT 1
+#define ABOVE_THRESHOLD_BIT 3
+
+static uint32_t evaluate_bss_group(const uint8_t *addr, uint32_t freq,
+					int16_t signal_strength)
+{
+	int threshold;
+	int signal = signal_strength / 100;
+	bool roam_blacklist;
+	uint32_t mask = 0;
+
+	if (blacklist_contains_bss(addr, BLACKLIST_REASON_CONNECT_FAILED))
+		return mask;
+
+	roam_blacklist = blacklist_contains_bss(addr,
+					BLACKLIST_REASON_ROAM_REQUESTED);
+
+	if (freq > 4000)
+		netdev_get_low_signal_thresholds(NULL, &threshold);
+	else
+		netdev_get_low_signal_thresholds(&threshold, NULL);
+
+	if (signal >= threshold)
+		set_bit(&mask, ABOVE_THRESHOLD_BIT - roam_blacklist);
+	else
+		set_bit(&mask, UNDER_THRESHOLD_BIT - roam_blacklist);
+
+	return mask;
+}
+
 /*
  * 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
@@ -164,11 +215,13 @@  struct roam_bss {
 	uint8_t addr[6];
 	uint16_t rank;
 	int32_t signal_strength;
+	uint32_t group;
 	bool ft_failed: 1;
 };
 
 static struct roam_bss *roam_bss_from_scan_bss(const struct scan_bss *bss,
-						uint16_t rank)
+						uint16_t rank,
+						uint32_t group)
 {
 	struct roam_bss *rbss = l_new(struct roam_bss, 1);
 
@@ -176,6 +229,7 @@  static struct roam_bss *roam_bss_from_scan_bss(const struct scan_bss *bss,
 	rbss->rank = rank;
 	rbss->signal_strength = bss->signal_strength;
 	rbss->ft_failed = false;
+	rbss->group = group;
 
 	return rbss;
 }
@@ -184,6 +238,12 @@  static int roam_bss_rank_compare(const void *a, const void *b, void *user_data)
 {
 	const struct roam_bss *new_bss = a, *bss = b;
 
+	if (bss->group > new_bss->group)
+		return 1;
+	else if (bss->group < new_bss->group)
+		return -1;
+
+	/* BSS's both have the same group, sort by rank */
 	if (bss->rank == new_bss->rank)
 		return (bss->signal_strength >
 					new_bss->signal_strength) ? 1 : -1;
@@ -2805,6 +2865,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;
+	uint32_t cur_bss_group = 0;
 	double cur_bss_rank = 0.0;
 	static const double RANK_FT_FACTOR = 1.3;
 	uint16_t mdid;
@@ -2835,6 +2896,9 @@  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;
+		cur_bss_group = evaluate_bss_group(current_bss->addr,
+						current_bss->frequency,
+						current_bss->signal_strength);
 
 		if (hs->mde && bss->mde_present && l_get_le16(bss->mde) == mdid)
 			cur_bss_rank *= RANK_FT_FACTOR;
@@ -2859,6 +2923,9 @@  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 = evaluate_bss_group(bss->addr,
+							bss->frequency,
+							bss->signal_strength);
 
 		station_print_scan_bss(bss);
 
@@ -2889,7 +2956,15 @@  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)
+		/*
+		 * First check the group:
+		 *   - If worse, disregard BSS candidate
+		 *   - If better, keep BSS candidate
+		 *   - If equal, compare based on rank
+		 */
+		if (group < cur_bss_group)
+			goto next;
+		else if (group == cur_bss_group && rank <= cur_bss_rank)
 			goto next;
 
 		/*
@@ -2898,7 +2973,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, rank, group);
 
 		l_queue_insert(station->roam_bss_list, rbss,
 				roam_bss_rank_compare, NULL);
@@ -3268,6 +3343,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,
@@ -5344,7 +5423,8 @@  static bool station_force_roam_scan_notify(int err, struct l_queue *bss_list,
 	/* The various roam routines expect this to be set from scanning */
 	station->preparing_roam = true;
 	l_queue_push_tail(station->roam_bss_list,
-				roam_bss_from_scan_bss(target, target->rank));
+				roam_bss_from_scan_bss(target, target->rank,
+				0xffff));
 
 	station_update_roam_bss(station, target);