[PATCH 1/3] station: retry roaming unless notified of a high RSSI

[PATCH 1/3] station: retry roaming unless notified of a high RSSI

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

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

Following a successful roaming sequence, schedule another attempt unless
the driver has sent a high RSSI notification. This makes the behaviour
analogous to a failed roaming attempt where we remained connected to the
same BSS.

This makes iwd compatible with wireless drivers which do not necessarily
send out a duplicate low RSSI notification upon reassociation. Without
this change, iwd risks getting indefinitely stuck to a BSS with low
signal strength, even though a better BSS might later become available.

In the case of a high RSSI notification, the minimum roam time will also
be reset to zero. This preserves the original behaviour in the case
where a high RSSI notification is processed after station_roamed().
Doing so also gives a chance for faster roaming action in the following
example scenario:

    1. RSSI LOW
    2. schedule roam in 5 seconds
        (5 seconds pass)
    3. try roaming
    4. roaming fails, same BSS
    5. schedule roam in 60 seconds
        (20 seconds pass)
    6. RSSI HIGH
    7. cancel scheduled roam
        (20 seconds pass)
    8. RSSI LOW
    9. schedule roam in 5 seconds or 20 seconds?

By resetting the minimum roam time, we can avoid waiting 20 seconds when
the station may have moved considerably. And since the high/low RSSI
notifications are configured with a hysteresis, we should still be
protected against too frequent spurious roaming attempts.
---
 src/station.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/station.c b/src/station.c
index c65fc0cc..2e886a73 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1362,13 +1362,14 @@ static int station_roam_scan(struct station *station,
 
 static void station_roamed(struct station *station)
 {
+	station->roam_scan_full = false;
+
 	/*
-	 * New signal high/low notification should occur on the next
-	 * beacon from new AP.
+	 * Schedule another roaming attempt in case the signal continues to
+	 * remain low. A subsequent high signal notification will cancel it.
 	 */
-	station->signal_low = false;
-	station->roam_min_time.tv_sec = 0;
-	station->roam_scan_full = false;
+	if (station->signal_low)
+		station_roam_timeout_rearm(station, 60);
 
 	if (station->netconfig)
 		netconfig_reconfigure(station->netconfig);
@@ -2227,6 +2228,7 @@ static void station_ok_rssi(struct station *station)
 	station->roam_trigger_timeout = NULL;
 
 	station->signal_low = false;
+	station->roam_min_time.tv_sec = 0;
 }
 
 static void station_rssi_level_changed(struct station *station,
-- 
2.29.2

[PATCH 3/3] doc: describe RoamRetryInterval setting

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

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

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

diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 365494a4..0064dfd2 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -133,6 +133,13 @@ The group ``[General]`` contains general settings.
 
        This can be used to control how aggressively **iwd** roams.
 
+   * - RoamRetryInterval
+     - Value: unsigned int value in seconds (default: **60**)
+
+       Specifies how long **iwd** will wait before attempting to roam again if
+       the last roam attempt failed, or if the signal of the newly connected BSS
+       is still considered weak.
+
    * - ManagementFrameProtection
      - Values: 0, **1** or 2
 
-- 
2.29.2

[PATCH 2/3] station: add RoamRetryInterval setting

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

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

---
 src/station.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/station.c b/src/station.c
index 2e886a73..558ecf9a 100644
--- a/src/station.c
+++ b/src/station.c
@@ -29,6 +29,7 @@
 #include <errno.h>
 #include <time.h>
 #include <sys/time.h>
+#include <limits.h>
 #include <linux/if_ether.h>
 
 #include <ell/ell.h>
@@ -57,6 +58,7 @@
 static struct l_queue *station_list;
 static uint32_t netdev_watch;
 static uint32_t mfp_setting;
+static uint32_t roam_retry_interval;
 static bool anqp_disabled;
 static bool netconfig_enabled;
 static struct watchlist anqp_watches;
@@ -1369,7 +1371,7 @@ static void station_roamed(struct station *station)
 	 * remain low. A subsequent high signal notification will cancel it.
 	 */
 	if (station->signal_low)
-		station_roam_timeout_rearm(station, 60);
+		station_roam_timeout_rearm(station, roam_retry_interval);
 
 	if (station->netconfig)
 		netconfig_reconfigure(station->netconfig);
@@ -1416,7 +1418,7 @@ delayed_retry:
 	station->ap_directed_roaming = false;
 
 	if (station->signal_low)
-		station_roam_timeout_rearm(station, 60);
+		station_roam_timeout_rearm(station, roam_retry_interval);
 }
 
 static void station_netconfig_event_handler(enum netconfig_event event,
@@ -3646,6 +3648,14 @@ static int station_init(void)
 		mfp_setting = 1;
 	}
 
+	if (!l_settings_get_uint(iwd_get_config(), "General",
+				"RoamRetryInterval",
+				&roam_retry_interval))
+		roam_retry_interval = 60;
+
+	if (roam_retry_interval > INT_MAX)
+		roam_retry_interval = INT_MAX;
+
 	if (!l_settings_get_bool(iwd_get_config(), "General", "DisableANQP",
 				&anqp_disabled))
 		anqp_disabled = true;
-- 
2.29.2

Re: [PATCH 1/3] station: retry roaming unless notified of a high RSSI

[Denis Kenzior]

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

Hi Alvin,

On 1/22/21 7:37 AM, Alvin Šipraga wrote:
> Following a successful roaming sequence, schedule another attempt unless
> the driver has sent a high RSSI notification. This makes the behaviour
> analogous to a failed roaming attempt where we remained connected to the
> same BSS.
> 
> This makes iwd compatible with wireless drivers which do not necessarily
> send out a duplicate low RSSI notification upon reassociation. Without
> this change, iwd risks getting indefinitely stuck to a BSS with low
> signal strength, even though a better BSS might later become available.
> 
> In the case of a high RSSI notification, the minimum roam time will also
> be reset to zero. This preserves the original behaviour in the case
> where a high RSSI notification is processed after station_roamed().
> Doing so also gives a chance for faster roaming action in the following
> example scenario:
> 
>      1. RSSI LOW
>      2. schedule roam in 5 seconds
>          (5 seconds pass)
>      3. try roaming
>      4. roaming fails, same BSS
>      5. schedule roam in 60 seconds
>          (20 seconds pass)
>      6. RSSI HIGH
>      7. cancel scheduled roam
>          (20 seconds pass)
>      8. RSSI LOW
>      9. schedule roam in 5 seconds or 20 seconds?
> 
> By resetting the minimum roam time, we can avoid waiting 20 seconds when
> the station may have moved considerably. And since the high/low RSSI
> notifications are configured with a hysteresis, we should still be
> protected against too frequent spurious roaming attempts.
> ---
>   src/station.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 

All applied, thanks!

Any chance you would be willing (or at least attempt) to add an auto test for 
some of the above scenarios?

Regards,
-Denis

Re: [PATCH 1/3] station: retry roaming unless notified of a high RSSI

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

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

Hi Denis,

On 1/22/21 8:50 PM, Denis Kenzior wrote:
> Hi Alvin,
> 
> On 1/22/21 7:37 AM, Alvin Šipraga wrote:
>> Following a successful roaming sequence, schedule another attempt unless
>> the driver has sent a high RSSI notification. This makes the behaviour
>> analogous to a failed roaming attempt where we remained connected to the
>> same BSS.
>>
>> This makes iwd compatible with wireless drivers which do not necessarily
>> send out a duplicate low RSSI notification upon reassociation. Without
>> this change, iwd risks getting indefinitely stuck to a BSS with low
>> signal strength, even though a better BSS might later become available.
>>
>> In the case of a high RSSI notification, the minimum roam time will also
>> be reset to zero. This preserves the original behaviour in the case
>> where a high RSSI notification is processed after station_roamed().
>> Doing so also gives a chance for faster roaming action in the following
>> example scenario:
>>
>>      1. RSSI LOW
>>      2. schedule roam in 5 seconds
>>          (5 seconds pass)
>>      3. try roaming
>>      4. roaming fails, same BSS
>>      5. schedule roam in 60 seconds
>>          (20 seconds pass)
>>      6. RSSI HIGH
>>      7. cancel scheduled roam
>>          (20 seconds pass)
>>      8. RSSI LOW
>>      9. schedule roam in 5 seconds or 20 seconds?
>>
>> By resetting the minimum roam time, we can avoid waiting 20 seconds when
>> the station may have moved considerably. And since the high/low RSSI
>> notifications are configured with a hysteresis, we should still be
>> protected against too frequent spurious roaming attempts.
>> ---
>>   src/station.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
> 
> All applied, thanks!
> 
> Any chance you would be willing (or at least attempt) to add an auto 
> test for some of the above scenarios?

Sure, I'd be happy to. I haven't tried running the autotest suite 
before, so please have some patience with me as I try to figure things out.

Kind regards,
Alvin