diff mbox series

rank: add option to normalize using defined rate

Message ID 20230921142302.77637-1-peda@bang-olufsen.dk (mailing list archive)
State New
Headers show
Series rank: add option to normalize using defined rate | expand

Checks

Context Check Description
tedd_an/pre-ci_am fail error: patch failed: src/scan.c:55 error: src/scan.c: patch does not apply hint: Use 'git am --show-current-patch' to see the failed patch

Commit Message

Pedro André Sept. 21, 2023, 2:23 p.m. UTC 
BSSID ranking relies on a quotient between the BSS data rate and
theoretical max data rate. This makes it so that when two or more BSSes
have the same SSID, the likelihood that higher frequency bands are
ranked first is high given their increased data throughput.
Such may not be ideal, for example, if one desires to connect to 2.4GHz
by default (if possible).

This tries to address such scenarios by introducing a new rank
configuration option where a necessary data rate (in bits/s) can be
provided. All BSSIDs that achieve or surpass this data rate will be
ranked identically before applying any other rank factors. If no value
is provided, the ranking process remains identical.
By using this configuration value in conjunction with already existing
ones, achieving a behaviour like preferring a certain band becomes more
accessible while guaranteeing that the first-ranked BSSID still complies
with minimal necessary data rate requirements.
---
 src/scan.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Comments

James Prestwood Sept. 21, 2023, 3:14 p.m. UTC | #1 
Hi Pedro,

On 9/21/23 7:23 AM, Pedro André wrote:
> BSSID ranking relies on a quotient between the BSS data rate and
> theoretical max data rate. This makes it so that when two or more BSSes
> have the same SSID, the likelihood that higher frequency bands are
> ranked first is high given their increased data throughput.
> Such may not be ideal, for example, if one desires to connect to 2.4GHz
> by default (if possible).
> 
> This tries to address such scenarios by introducing a new rank
> configuration option where a necessary data rate (in bits/s) can be
> provided. All BSSIDs that achieve or surpass this data rate will be
> ranked identically before applying any other rank factors. If no value
> is provided, the ranking process remains identical.
> By using this configuration value in conjunction with already existing
> ones, achieving a behaviour like preferring a certain band becomes more
> accessible while guaranteeing that the first-ranked BSSID still complies
> with minimal necessary data rate requirements.
> ---
>   src/scan.c | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/scan.c b/src/scan.c
> index 5a972efb..16d533e4 100644
> --- a/src/scan.c
> +++ b/src/scan.c
> @@ -55,6 +55,7 @@
>   /* User configurable options */
>   static double RANK_5G_FACTOR;
>   static double RANK_6G_FACTOR;
> +static double RANK_NECESSARY_RATE;
>   static uint32_t SCAN_MAX_INTERVAL;
>   static uint32_t SCAN_INIT_INTERVAL;
>   
> @@ -1625,14 +1626,15 @@ static void scan_bss_compute_rank(struct scan_bss *bss)
>   {
>   	static const double RANK_HIGH_UTILIZATION_FACTOR = 0.8;
>   	static const double RANK_LOW_UTILIZATION_FACTOR = 1.2;
> +	double data_rate;
>   	double rank;
>   	uint32_t irank;
> -	/*
> -	 * Maximum rate is 9607.8Mbps (HE)
> -	 */
> -	double max_rate = 9607800000;
>   
> -	rank = (double)bss->data_rate / max_rate * USHRT_MAX;
> +	data_rate = (double)bss->data_rate;
> +	if (data_rate > RANK_NECESSARY_RATE)
> +		data_rate = RANK_NECESSARY_RATE;
> +
> +	rank = data_rate / RANK_NECESSARY_RATE * USHRT_MAX;
>   
>   	/* Prefer 5G networks over 2.4G and 6G */
>   	if (bss->frequency >= 4900 && bss->frequency < 5900)
> @@ -2363,6 +2365,14 @@ static int scan_init(void)
>   					&RANK_6G_FACTOR))
>   		RANK_6G_FACTOR = 1.0;
>   
> +	/* Maximum rate is 9607.8Mbps (HE) */
> +	if (!l_settings_get_double(config, "Rank", "NecessaryRate",
> +				&RANK_NECESSARY_RATE))
> +		RANK_NECESSARY_RATE = 9607800000;
> +
> +	if (RANK_NECESSARY_RATE > 9607800000)
> +		RANK_NECESSARY_RATE = 9607800000;
> +
>   	if (!l_settings_get_uint(config, "Scan", "InitialPeriodicScanInterval",
>   					&SCAN_INIT_INTERVAL))
>   		SCAN_INIT_INTERVAL = 10;

So this just bypasses the data rate weight (for BSS's at or above the 
NecessaryRate), leaving only 5/6ghz rank modifiers, utilization, and 
signal strength (if ranks are equal).

I know its behind a non-default option, so I guess "use at your own 
risk", but... This significantly increases the pull of the rank 
modifiers and utilization (which was kinda your point in doing this). My 
concern is that you could run into a situation where IWD connects to a 
BSS with poor signal because utilization is lower than others. Since 
signal strength is only considered if the ranks are equal you could have 
the following scenario:

BSS A: utilization 192/256, RSSI: -55dbm
BSS B: utilization 60/256, RSSI: -88dbm

Correct me if I'm wrong, but assuming BSS A/B both met the NecessaryRate 
requirements IWD would choose BSS B to connect to since the only factor 
now is utilization. I do realize this only applies to very loaded APs 
(>= 192), but its something to think about.

This is more of a nit, but I wonder if we want to have the option in 
increments of Mbps rather than bytes. Just easier to read that way.

We would also need to add this to src/iwd.config.rst

Thanks,
James
Denis Kenzior Sept. 25, 2023, 2:51 p.m. UTC | #2 
Hi Pedro,

On 9/21/23 09:23, Pedro André wrote:
> BSSID ranking relies on a quotient between the BSS data rate and
> theoretical max data rate. This makes it so that when two or more BSSes
> have the same SSID, the likelihood that higher frequency bands are
> ranked first is high given their increased data throughput.
> Such may not be ideal, for example, if one desires to connect to 2.4GHz
> by default (if possible).

I'm curious why one would want to connect to 2.4 by default?

> 
> This tries to address such scenarios by introducing a new rank
> configuration option where a necessary data rate (in bits/s) can be
> provided. All BSSIDs that achieve or surpass this data rate will be
> ranked identically before applying any other rank factors. If no value
> is provided, the ranking process remains identical.

I can certainly see the use case for non-general purpose devices that have a 
specific bandwidth target.  I think we can benefit from something like this...

> By using this configuration value in conjunction with already existing
> ones, achieving a behaviour like preferring a certain band becomes more
> accessible while guaranteeing that the first-ranked BSSID still complies
> with minimal necessary data rate requirements.

However, I'm a bit worried about the details.  Given that you're only interested 
in BSSes that meet a certain bandwidth threshold, wouldn't iwd want to weight 
the RSSI more heavily?  I suppose one could rely on the 5G or 6G ranking 
factors, but is that going to be good enough?

> ---
>   src/scan.c | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/scan.c b/src/scan.c
> index 5a972efb..16d533e4 100644
> --- a/src/scan.c
> +++ b/src/scan.c
> @@ -55,6 +55,7 @@
>   /* User configurable options */
>   static double RANK_5G_FACTOR;
>   static double RANK_6G_FACTOR;
> +static double RANK_NECESSARY_RATE;

Why is this a double?  Might be easier to make this uint64_t instead, similar to 
what wiphy_estimate_data_rate produces.

>   static uint32_t SCAN_MAX_INTERVAL;
>   static uint32_t SCAN_INIT_INTERVAL;
>   
> @@ -1625,14 +1626,15 @@ static void scan_bss_compute_rank(struct scan_bss *bss)
>   {
>   	static const double RANK_HIGH_UTILIZATION_FACTOR = 0.8;
>   	static const double RANK_LOW_UTILIZATION_FACTOR = 1.2;
> +	double data_rate;
>   	double rank;
>   	uint32_t irank;
> -	/*
> -	 * Maximum rate is 9607.8Mbps (HE)
> -	 */
> -	double max_rate = 9607800000;
>   
> -	rank = (double)bss->data_rate / max_rate * USHRT_MAX;
> +	data_rate = (double)bss->data_rate;
> +	if (data_rate > RANK_NECESSARY_RATE)
> +		data_rate = RANK_NECESSARY_RATE;
> +
> +	rank = data_rate / RANK_NECESSARY_RATE * USHRT_MAX;
>   

If you pick a low enough necessary rate, the ranking will be always USHRT_MAX. 
Any factors applied will not affect the rank due to bss->rank being clamped to 
0..USHRT_MAX range.  This also creates some weirdness during FT, see my earlier 
comment in the 'Force use of 5GHz band' thread.

>   	/* Prefer 5G networks over 2.4G and 6G */
>   	if (bss->frequency >= 4900 && bss->frequency < 5900)

Regards,
-Denis
diff mbox series

Patch

diff --git a/src/scan.c b/src/scan.c
index 5a972efb..16d533e4 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -55,6 +55,7 @@ 
 /* User configurable options */
 static double RANK_5G_FACTOR;
 static double RANK_6G_FACTOR;
+static double RANK_NECESSARY_RATE;
 static uint32_t SCAN_MAX_INTERVAL;
 static uint32_t SCAN_INIT_INTERVAL;
 
@@ -1625,14 +1626,15 @@  static void scan_bss_compute_rank(struct scan_bss *bss)
 {
 	static const double RANK_HIGH_UTILIZATION_FACTOR = 0.8;
 	static const double RANK_LOW_UTILIZATION_FACTOR = 1.2;
+	double data_rate;
 	double rank;
 	uint32_t irank;
-	/*
-	 * Maximum rate is 9607.8Mbps (HE)
-	 */
-	double max_rate = 9607800000;
 
-	rank = (double)bss->data_rate / max_rate * USHRT_MAX;
+	data_rate = (double)bss->data_rate;
+	if (data_rate > RANK_NECESSARY_RATE)
+		data_rate = RANK_NECESSARY_RATE;
+
+	rank = data_rate / RANK_NECESSARY_RATE * USHRT_MAX;
 
 	/* Prefer 5G networks over 2.4G and 6G */
 	if (bss->frequency >= 4900 && bss->frequency < 5900)
@@ -2363,6 +2365,14 @@  static int scan_init(void)
 					&RANK_6G_FACTOR))
 		RANK_6G_FACTOR = 1.0;
 
+	/* Maximum rate is 9607.8Mbps (HE) */
+	if (!l_settings_get_double(config, "Rank", "NecessaryRate",
+				&RANK_NECESSARY_RATE))
+		RANK_NECESSARY_RATE = 9607800000;
+
+	if (RANK_NECESSARY_RATE > 9607800000)
+		RANK_NECESSARY_RATE = 9607800000;
+
 	if (!l_settings_get_uint(config, "Scan", "InitialPeriodicScanInterval",
 					&SCAN_INIT_INTERVAL))
 		SCAN_INIT_INTERVAL = 10;