| Message ID | 20240126202243.91947-1-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/4] network: add network_update_known_frequencies | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-alpine-ci-fetch | success | Fetch PR |
| prestwoj/iwd-ci-fetch | success | Fetch PR |
| prestwoj/iwd-ci-gitlint | success | GitLint |
| prestwoj/iwd-alpine-ci-makedistcheck | success | Make Distcheck |
| prestwoj/iwd-ci-build | success | Build - Configure |
| prestwoj/iwd-alpine-ci-build | success | Build - Configure |
| prestwoj/iwd-ci-makecheckvalgrind | success | Make Check w/Valgrind |
| prestwoj/iwd-ci-clang | success | clang PASS |
| prestwoj/iwd-ci-makecheck | success | Make Check |
| prestwoj/iwd-alpine-ci-makecheckvalgrind | success | Make Check w/Valgrind |
| prestwoj/iwd-alpine-ci-makecheck | success | Make Check |
| prestwoj/iwd-alpine-ci-incremental_build | success | Incremental Build with patches |
| prestwoj/iwd-ci-incremental_build | success | Incremental Build with patches |
| prestwoj/iwd-ci-makedistcheck | success | Make Distcheck |
| prestwoj/iwd-ci-testrunner | success | test-runner PASS |
Hi James, On 1/26/24 14:22, James Prestwood wrote: > In order to support an ordered list of known frequencies the list > should be in order of last seen BSS frequencies with the highest > ranked ones first. To accomplish this without adding a lot of > complexity the frequencies can be pushed into the list as long as > they are pushed in reverse rank order (lowest rank first, highest > last). This ensures that very high ranked BSS's will always get > superseded by subsequent scans if not seen. > > This adds a new network API to update the known frequency list > based on the current newtork->bss_list. This assumes that station > always wipes the BSS list on scans and populates with only fresh Sort of. Strictly speaking it takes the frequencies scanned into account. So for example if we scan only certain 2.4GHz channels (say through DBus Scan API), then only BSSes within that frequency set will be updated. However, such scans are initiated one-after-the-other, so it is probably okay in this case. One thing you might have to be careful of is limited scans due to roaming logic. But I think this series is good enough for now. > BSS entries. After the scan this API can be called and it will > reverse the list, then add each frequency. > --- > src/network.c | 40 ++++++++++++++++++++++++++++------------ > src/network.h | 2 ++ > 2 files changed, 30 insertions(+), 12 deletions(-) > > v4: > * Added an API to do the insertion once all the BSS's have been added > to the network object. > * I'm also ok adding something to ELL like: > l_queue_new_reverse(list); Given the sizes of our queues, it might be faster to reverse it twice than re-allocate / free one. Might even be a good candidate for l_newa() if you can keep a known bound on the number of entries. > > The current reverse is in-place, so I cant really use it unless I > wanted to reverse twice to get it back to decending order. > <snip> > bool network_bss_add(struct network *network, struct scan_bss *bss) > { > if (!l_queue_insert(network->bss_list, bss, scan_bss_rank_compare, > NULL)) > return false; > > - if (network->info) > - known_network_add_frequency(network->info, bss->frequency); > - > /* Done if BSS is not HS20 or we already have network_info set */ > if (!bss->hs20_capable) > return true; This part looked like it belong with patch 2, so I moved it there. All patches applied, thanks. Regards, -Denis
Hi Denis, On 1/29/24 6:54 PM, Denis Kenzior wrote: > Hi James, > > On 1/26/24 14:22, James Prestwood wrote: >> In order to support an ordered list of known frequencies the list >> should be in order of last seen BSS frequencies with the highest >> ranked ones first. To accomplish this without adding a lot of >> complexity the frequencies can be pushed into the list as long as >> they are pushed in reverse rank order (lowest rank first, highest >> last). This ensures that very high ranked BSS's will always get >> superseded by subsequent scans if not seen. >> >> This adds a new network API to update the known frequency list >> based on the current newtork->bss_list. This assumes that station >> always wipes the BSS list on scans and populates with only fresh > > Sort of. Strictly speaking it takes the frequencies scanned into > account. So for example if we scan only certain 2.4GHz channels (say > through DBus Scan API), then only BSSes within that frequency set will > be updated. However, such scans are initiated one-after-the-other, so > it is probably okay in this case. Yeah we are technically re-adding the same frequencies for each successive scan. > > One thing you might have to be careful of is limited scans due to > roaming logic. But I think this series is good enough for now. Yep this is why I didn't add anything in the roaming results just yet. That and neighbor reports will require a bit more thought. I think for roam scans/NR's we might be able to get away with doing the same thing, but limited only to the connected network. In theory a roam scan should still yield more updated frequencies than past scans, so we probably could shove those to the head of the queue in the same fashion. > >> BSS entries. After the scan this API can be called and it will >> reverse the list, then add each frequency. >> --- >> src/network.c | 40 ++++++++++++++++++++++++++++------------ >> src/network.h | 2 ++ >> 2 files changed, 30 insertions(+), 12 deletions(-) >> >> v4: >> * Added an API to do the insertion once all the BSS's have been added >> to the network object. >> * I'm also ok adding something to ELL like: >> l_queue_new_reverse(list); > > Given the sizes of our queues, it might be faster to reverse it twice > than re-allocate / free one. Might even be a good candidate for > l_newa() if you can keep a known bound on the number of entries. > >> >> The current reverse is in-place, so I cant really use it unless I >> wanted to reverse twice to get it back to decending order. >> > > <snip> > >> bool network_bss_add(struct network *network, struct scan_bss *bss) >> { >> if (!l_queue_insert(network->bss_list, bss, scan_bss_rank_compare, >> NULL)) >> return false; >> - if (network->info) >> - known_network_add_frequency(network->info, bss->frequency); >> - >> /* Done if BSS is not HS20 or we already have network_info set */ >> if (!bss->hs20_capable) >> return true; > > This part looked like it belong with patch 2, so I moved it there. > > All patches applied, thanks. > > Regards, > -Denis >
diff --git a/src/network.c b/src/network.c index 4723334e..287e2be0 100644 --- a/src/network.c +++ b/src/network.c @@ -802,21 +802,13 @@ const struct network_info *network_get_info(const struct network *network) return network->info; } -static void add_known_frequency(void *data, void *user_data) -{ - struct scan_bss *bss = data; - struct network_info *info = user_data; - - known_network_add_frequency(info, bss->frequency); -} - void network_set_info(struct network *network, struct network_info *info) { if (info) { network->info = info; network->info->seen_count++; - l_queue_foreach(network->bss_list, add_known_frequency, info); + network_update_known_frequencies(network); } else { network->info->seen_count--; network->info = NULL; @@ -1087,15 +1079,39 @@ static bool match_hotspot_network(const struct network_info *info, return true; } +bool network_update_known_frequencies(struct network *network) +{ + const struct l_queue_entry *e; + struct l_queue *reversed; + + if (!network->info) + return false; + + reversed = l_queue_new(); + + for (e = l_queue_get_entries(network->bss_list); e; e = e->next) { + struct scan_bss *bss = e->data; + + l_queue_push_head(reversed, bss); + } + + for (e = l_queue_get_entries(reversed); e; e = e->next) { + struct scan_bss *bss = e->data; + + known_network_add_frequency(network->info, bss->frequency); + } + + l_queue_destroy(reversed, NULL); + + return true; +} + bool network_bss_add(struct network *network, struct scan_bss *bss) { if (!l_queue_insert(network->bss_list, bss, scan_bss_rank_compare, NULL)) return false; - if (network->info) - known_network_add_frequency(network->info, bss->frequency); - /* Done if BSS is not HS20 or we already have network_info set */ if (!bss->hs20_capable) return true; diff --git a/src/network.h b/src/network.h index f29649f7..ea619f3f 100644 --- a/src/network.h +++ b/src/network.h @@ -61,6 +61,8 @@ void network_set_info(struct network *network, struct network_info *info); void network_set_force_default_owe_group(struct network *network); bool network_get_force_default_owe_group(struct network *network); +bool network_update_known_frequencies(struct network *network); + int network_can_connect_bss(struct network *network, const struct scan_bss *bss); int network_autoconnect(struct network *network, struct scan_bss *bss);