Re: [PATCH 2/7] knownnetworks: add known_network_add_connected_frequency

Denis Kenzior
2023-12-22T22:13:43Z

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