diff mbox series

[2/7] knownnetworks: add known_network_add_connected_frequency

Message ID 20231220131200.267489-3-prestwoj@gmail.com (mailing list archive)
State New
Headers show
Series Reduce and optimize quick/roam scan frequencies | expand

Checks

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

Commit Message

James Prestwood Dec. 20, 2023, 1:11 p.m. UTC
This should be called when BSS is connected/roamed to which will
insert the frequency ahead of entries which were only seen in scan
results.

Providing this and the 'seen' variant sets up the frequency list
into two adjacent sections:
  [Most recently used]...[Most recently seen]

Doing this will allow scanning to be more optimized and limit the
number of frequencies while prefering BSS's that have been connected
to prior.

Note that this list format is not synced to the file system.
Frequencies will be synced in the order of this list but not
containing the 'connected' bit. When IWD restarts this information
will be lost, but the list is still sorted on disk in a better state
than it was prior.
---
 src/knownnetworks.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
 src/knownnetworks.h |  3 +++
 2 files changed, 61 insertions(+), 1 deletion(-)

Comments

Denis Kenzior Dec. 22, 2023, 10:13 p.m. UTC | #1
Hi James,

On 12/20/23 07:11, James Prestwood wrote:
> This should be called when BSS is connected/roamed to which will
> insert the frequency ahead of entries which were only seen in scan
> results.
> 
> Providing this and the 'seen' variant sets up the frequency list
> into two adjacent sections:
>    [Most recently used]...[Most recently seen]
> 

So why not just keep two lists?

> Doing this will allow scanning to be more optimized and limit the
> number of frequencies while prefering BSS's that have been connected
> to prior.
> 
> Note that this list format is not synced to the file system.
> Frequencies will be synced in the order of this list but not
> containing the 'connected' bit. When IWD restarts this information
> will be lost, but the list is still sorted on disk in a better state
> than it was prior.
> ---
>   src/knownnetworks.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
>   src/knownnetworks.h |  3 +++
>   2 files changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/src/knownnetworks.c b/src/knownnetworks.c
> index eac6c66b..e44109fd 100644
> --- a/src/knownnetworks.c
> +++ b/src/knownnetworks.c
> @@ -562,15 +562,70 @@ static bool known_frequency_match(const void *a, const void *b)
>   	return known_freq->frequency == *frequency;
>   }
>   
> +static int known_frequency_compare(const void *a, const void *b,
> +					void *user_data)
> +{
> +	const struct known_frequency *ka = a;
> +
> +	/*
> +	 * Only meant to be used to insert 'seen' frequencies. Any existing
> +	 * entry that has 'connected' set should preceed an entry that was

precede?

> +	 * only seen.
> +	 */
> +	if (ka->connected)
> +		return -1; > +
> +	/*
> +	 * Otherwise, insert immediately after the last 'connected' entry,
> +	 * i.e. the most recently seen
> +	 */
> +	return 1;
> +}
> +
>   /*
>    * Adds a frequency to the 'known' set of frequencies that this network
> - * operates on.  The list is sorted according to most-recently seen
> + * operates on. Frequencies added here will follow frequencies which have been
> + * connected to and inserted according to most-recently seen
>    */
>   int known_network_add_seen_frequency(struct network_info *info,
>   					uint32_t frequency)
>   {
>   	struct known_frequency *known_freq;
>   
> +	if (!info->known_frequencies)
> +		info->known_frequencies = l_queue_new();
> +

I'm not thrilled about the implementation...

> +	known_freq = l_queue_find(info->known_frequencies,
> +					known_frequency_match, &frequency);

You walk the entire queue once, then walk it again to remove the known_freq 
object, then walk it one more time for the insert.

> +	/*
> +	 * If no match, create a new one.
> +	 * If connected to frequency before leave that entry in place
> +	 */
> +	if (!known_freq) {
> +		known_freq = l_new(struct known_frequency, 1);
> +		known_freq->frequency = frequency;
> +	} else if (known_freq->connected)
> +		return 0;

So 'seeing' a frequency that has been connected to before doesn't change its 
sort order?

> +
> +	l_queue_remove(info->known_frequencies, known_freq);
> +
> +	/* insert the entry immediately after the last 'connected' entry */
> +	l_queue_insert(info->known_frequencies, known_freq,
> +			known_frequency_compare, NULL);
> +
> +	return 0;
> +}
> +
> +/*
> + * Adds a frequency to the known set of frequencies that this network operates
> + * on. Frequencies added here are assumed to be most recently used and inserted
> + * at the head of the queue.
> + */
> +int known_network_add_connected_frequency(struct network_info *info,
> +						uint32_t frequency)
> +{
> +	struct known_frequency *known_freq;
> +
>   	if (!info->known_frequencies)
>   		info->known_frequencies = l_queue_new();
>   
> @@ -581,6 +636,8 @@ int known_network_add_seen_frequency(struct network_info *info,
>   		known_freq->frequency = frequency;
>   	}
>   
> +	known_freq->connected = true;
> +
>   	l_queue_push_head(info->known_frequencies, known_freq);
>   
>   	return 0;

Overall I'm getting the feeling that the linked-list is the wrong data structure 
for all this.  If the intent is to maintain a bounded shortlist of frequencies 
to scan for a given network, then that's what we should do instead.

Regards,
-Denis
James Prestwood Jan. 2, 2024, 1:18 p.m. UTC | #2
Hi Denis,
>
> Overall I'm getting the feeling that the linked-list is the wrong data 
> structure for all this.  If the intent is to maintain a bounded 
> shortlist of frequencies to scan for a given network, then that's what 
> we should do instead.

Yeah I agree, this is a bit complicated. I think a much simpler approach 
would be to limit the known frequency list to some maximum size (e.g. 5) 
and store frequencies based on the BSS rank rather than seen/connected.

Thanks,

James

>
> Regards,
> -Denis
diff mbox series

Patch

diff --git a/src/knownnetworks.c b/src/knownnetworks.c
index eac6c66b..e44109fd 100644
--- a/src/knownnetworks.c
+++ b/src/knownnetworks.c
@@ -562,15 +562,70 @@  static bool known_frequency_match(const void *a, const void *b)
 	return known_freq->frequency == *frequency;
 }
 
+static int known_frequency_compare(const void *a, const void *b,
+					void *user_data)
+{
+	const struct known_frequency *ka = a;
+
+	/*
+	 * Only meant to be used to insert 'seen' frequencies. Any existing
+	 * entry that has 'connected' set should preceed an entry that was
+	 * only seen.
+	 */
+	if (ka->connected)
+		return -1;
+
+	/*
+	 * Otherwise, insert immediately after the last 'connected' entry,
+	 * i.e. the most recently seen
+	 */
+	return 1;
+}
+
 /*
  * Adds a frequency to the 'known' set of frequencies that this network
- * operates on.  The list is sorted according to most-recently seen
+ * operates on. Frequencies added here will follow frequencies which have been
+ * connected to and inserted according to most-recently seen
  */
 int known_network_add_seen_frequency(struct network_info *info,
 					uint32_t frequency)
 {
 	struct known_frequency *known_freq;
 
+	if (!info->known_frequencies)
+		info->known_frequencies = l_queue_new();
+
+	known_freq = l_queue_find(info->known_frequencies,
+					known_frequency_match, &frequency);
+	/*
+	 * If no match, create a new one.
+	 * If connected to frequency before leave that entry in place
+	 */
+	if (!known_freq) {
+		known_freq = l_new(struct known_frequency, 1);
+		known_freq->frequency = frequency;
+	} else if (known_freq->connected)
+		return 0;
+
+	l_queue_remove(info->known_frequencies, known_freq);
+
+	/* insert the entry immediately after the last 'connected' entry */
+	l_queue_insert(info->known_frequencies, known_freq,
+			known_frequency_compare, NULL);
+
+	return 0;
+}
+
+/*
+ * Adds a frequency to the known set of frequencies that this network operates
+ * on. Frequencies added here are assumed to be most recently used and inserted
+ * at the head of the queue.
+ */
+int known_network_add_connected_frequency(struct network_info *info,
+						uint32_t frequency)
+{
+	struct known_frequency *known_freq;
+
 	if (!info->known_frequencies)
 		info->known_frequencies = l_queue_new();
 
@@ -581,6 +636,8 @@  int known_network_add_seen_frequency(struct network_info *info,
 		known_freq->frequency = frequency;
 	}
 
+	known_freq->connected = true;
+
 	l_queue_push_head(info->known_frequencies, known_freq);
 
 	return 0;
diff --git a/src/knownnetworks.h b/src/knownnetworks.h
index d404b161..644e713f 100644
--- a/src/knownnetworks.h
+++ b/src/knownnetworks.h
@@ -96,6 +96,7 @@  typedef void (*known_networks_destroy_func_t)(void *user_data);
 
 struct known_frequency {
 	uint32_t frequency;
+	bool connected : 1;
 };
 
 void __network_config_parse(const struct l_settings *settings,
@@ -116,6 +117,8 @@  struct scan_freq_set *known_networks_get_recent_frequencies(
 						uint8_t num_networks_tosearch);
 int known_network_add_seen_frequency(struct network_info *info,
 					uint32_t frequency);
+int known_network_add_connected_frequency(struct network_info *info,
+					uint32_t frequency);
 void known_network_frequency_sync(struct network_info *info);
 
 uint32_t known_networks_watch_add(known_networks_watch_func_t func,