From ca396950326faa5e90bfc1c111144abc26e20395 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Wed, 16 Mar 2022 10:39:08 -0700 Subject: [PATCH 01/19] Workflow's for syncing with upstream, build, unit test, and test-runner --- .github/workflows/ci.yml | 236 +++++++++++++++++++++++++++ .github/workflows/pw-to-pr-email.txt | 16 ++ .github/workflows/pw-to-pr.json | 14 ++ .github/workflows/schedule_work.yml | 43 +++++ 4 files changed, 309 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/pw-to-pr-email.txt create mode 100644 .github/workflows/pw-to-pr.json create mode 100644 .github/workflows/schedule_work.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..8e140ad8c --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,236 @@ +name: IWD CI + +# +# The basic flow of the CI is as follows: +# +# 1. Get all inputs, or default values, and set as 'setup' job output +# 2. Find any cached binaries (hostapd, wpa_supplicant, kernel etc) +# 3. Checkout all dependent repositories +# 4. Tar all local files. This is an unfortunate requirement since github jobs +# cannot share local files. Since there are multiple CI's acting on the same +# set of repositories it makes more sense to retain these and re-download +# them for each CI job. +# 5. Run each CI, currently 'main' and 'musl'. +# * 'main' is the default IWD CI which runs all the build steps as well +# as test-runner +# * 'musl' uses an alpine docker image to test the build on musl-libc +# +# Both CI's use the 'iwd-ci-v2' repo which calls into 'ci-docker'. The +# 'ci-docker' action essentially re-implements the native Github docker +# action but allows arbitrary options to be passed in (e.g. privileged or +# mounting non-standard directories) +# + +on: + pull_request: + workflow_dispatch: + inputs: + tests: + description: Tests to run (comma separated, no spaces) + default: all + kernel: + description: Kernel version + default: '5.16' + hostapd_version: + description: Hostapd and wpa_supplicant version + default: '2_10' + ell_ref: + description: ELL reference + default: refs/heads/workflow + + repository_dispatch: + types: [ell-dispatch] + +jobs: + setup: + runs-on: ubuntu-22.04 + outputs: + tests: ${{ steps.inputs.outputs.tests }} + kernel: ${{ steps.inputs.outputs.kernel }} + hostapd_version: ${{ steps.inputs.outputs.hostapd_version }} + ell_ref: ${{ steps.inputs.outputs.ell_ref }} + repository: ${{ steps.inputs.outputs.repository }} + ref_branch: ${{ steps.inputs.outputs.ref_branch }} + steps: + # + # This makes CI inputs consistent depending on how the CI was invoked: + # * pull_request trigger won't have any inputs, so these need to be set + # to default values. + # * workflow_dispatch sets all inputs from the user input + # * repository_dispatch sets all inputs based on the JSON payload of + # the request. + # + - name: Setup Inputs + id: inputs + run: | + if [ ${{ github.event_name }} == 'workflow_dispatch' ] + then + TESTS=${{ github.event.inputs.tests }} + KERNEL=${{ github.event.inputs.kernel }} + HOSTAPD_VERSION=${{ github.event.inputs.hostapd_version }} + ELL_REF=${{ github.event.inputs.ell_ref }} + REF="$GITHUB_REF" + REPO="$GITHUB_REPOSITORY" + elif [ ${{ github.event_name }} == 'repository_dispatch' ] + then + TESTS=all + KERNEL=5.16 + HOSTAPD_VERSION=2_10 + ELL_REF=${{ github.event.client_payload.ref }} + REF=$ELL_REF + REPO=${{ github.event.client_payload.repo }} + else + TESTS=all + KERNEL=5.16 + HOSTAPD_VERSION=2_10 + ELL_REF="refs/heads/workflow" + REF="$GITHUB_REF" + REPO="$GITHUB_REPOSITORY" + fi + + # + # Now that the inputs are sorted, set the output of this step to these + # values so future jobs can refer to them. + # + echo ::set-output name=tests::$TESTS + echo ::set-output name=kernel::$KERNEL + echo ::set-output name=hostapd_version::$HOSTAPD_VERSION + echo ::set-output name=ell_ref::$ELL_REF + echo ::set-output name=repository::$REPO + echo ::set-output name=ref_branch::$REF + + - name: Cache UML Kernel + id: cache-uml-kernel + uses: actions/cache@v3 + with: + path: ${{ github.workspace }}/cache/um-linux-${{ steps.inputs.outputs.kernel }} + key: um-linux-${{ steps.inputs.outputs.kernel }}_ubuntu22 + + - name: Cache Hostapd + id: cache-hostapd + uses: actions/cache@v3 + with: + path: | + ${{ github.workspace }}/cache/hostapd_${{ steps.inputs.outputs.hostapd_version }} + ${{ github.workspace }}/cache/hostapd_cli_${{ steps.inputs.outputs.hostapd_version }} + key: hostapd_${{ steps.inputs.outputs.hostapd_version }}_ssl3 + + - name: Cache WpaSupplicant + id: cache-wpas + uses: actions/cache@v3 + with: + path: | + ${{ github.workspace }}/cache/wpa_supplicant_${{ steps.inputs.outputs.hostapd_version }} + ${{ github.workspace }}/cache/wpa_cli_${{ steps.inputs.outputs.hostapd_version }} + key: wpa_supplicant_${{ steps.inputs.outputs.hostapd_version }}_ssl3 + + - name: Checkout IWD + uses: actions/checkout@v3 + with: + path: iwd + repository: IWDTestBot/iwd + token: ${{ secrets.ACTION_TOKEN }} + + - name: Checkout ELL + uses: actions/checkout@v3 + with: + path: ell + repository: IWDTestBot/ell + ref: ${{ steps.inputs.outputs.ell_ref }} + + - name: Checkout CiBase + uses: actions/checkout@v3 + with: + repository: IWDTestBot/cibase + path: cibase + + - name: Checkout CI + uses: actions/checkout@v3 + with: + repository: IWDTestBot/iwd-ci-v2 + path: iwd-ci + + - name: Tar files + run: | + tar -cvf archive.tar \ + ${{ github.workspace }}/cache/um-linux-${{ steps.inputs.outputs.kernel }} \ + ${{ github.workspace }}/cache/hostapd_${{ steps.inputs.outputs.hostapd_version }} \ + ${{ github.workspace }}/cache/hostapd_cli_${{ steps.inputs.outputs.hostapd_version }} \ + ${{ github.workspace }}/cache/wpa_supplicant_${{ steps.inputs.outputs.hostapd_version }} \ + ${{ github.workspace }}/cache/wpa_cli_${{ steps.inputs.outputs.hostapd_version }} \ + iwd \ + ell \ + cibase \ + iwd-ci \ + cache + + - name: Upload artifacts + uses: actions/upload-artifact@v3 + with: + name: iwd-artifacts + path: | + archive.tar + + iwd-alpine-ci: + runs-on: ubuntu-22.04 + needs: setup + steps: + - name: Download artifacts + uses: actions/download-artifact@v3 + with: + name: iwd-artifacts + + - name: Untar + run: tar -xf archive.tar + + - name: Modprobe pkcs8_key_parser + run: | + sudo modprobe pkcs8_key_parser + + - name: Alpine CI + uses: IWDTestBot/iwd-ci-v2@master + with: + ref_branch: ${{ needs.setup.outputs.ref_branch }} + repository: ${{ needs.setup.outputs.repository }} + github_token: ${{ secrets.ACTION_TOKEN }} + email_token: ${{ secrets.EMAIL_TOKEN }} + patchwork_token: ${{ secrets.PATCHWORK_TOKEN }} + ci: musl + + iwd-ci: + runs-on: ubuntu-22.04 + needs: setup + steps: + - name: Download artifacts + uses: actions/download-artifact@v3 + with: + name: iwd-artifacts + + - name: Untar + run: tar -xf archive.tar + + - name: Modprobe pkcs8_key_parser + run: | + sudo modprobe pkcs8_key_parser + echo ${{ needs.setup.outputs.ref_branch }} + echo ${{ needs.setup.outputs.repository }} + + - name: Run CI + uses: IWDTestBot/iwd-ci-v2@master + with: + ref_branch: ${{ needs.setup.outputs.ref_branch }} + repository: ${{ needs.setup.outputs.repository }} + tests: ${{ needs.setup.outputs.tests }} + kernel: ${{ needs.setup.outputs.kernel }} + hostapd_version: ${{ needs.setup.outputs.hostapd_version }} + github_token: ${{ secrets.ACTION_TOKEN }} + email_token: ${{ secrets.EMAIL_TOKEN }} + patchwork_token: ${{ secrets.PATCHWORK_TOKEN }} + ci: main + + - name: Upload Logs + if: always() + uses: actions/upload-artifact@v3 + with: + name: test-runner-logs + path: ${{ github.workspace }}/log diff --git a/.github/workflows/pw-to-pr-email.txt b/.github/workflows/pw-to-pr-email.txt new file mode 100644 index 000000000..0ad6d7659 --- /dev/null +++ b/.github/workflows/pw-to-pr-email.txt @@ -0,0 +1,16 @@ +This is an automated email and please do not reply to this email. + +Dear Submitter, + +Thank you for submitting the patches to the IWD mailing list. +While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository. + +----- Output ----- +{} + +Please resolve the issue and submit the patches again. + + +--- +Regards, +IWDTestBot diff --git a/.github/workflows/pw-to-pr.json b/.github/workflows/pw-to-pr.json new file mode 100644 index 000000000..b4491413c --- /dev/null +++ b/.github/workflows/pw-to-pr.json @@ -0,0 +1,14 @@ +{ + "email": { + "enable": true, + "server": "smtp.gmail.com", + "port": 587, + "user": "iwd.ci.bot@gmail.com", + "starttls": true, + "default-to": "prestwoj@gmail.com", + "only-maintainers": false, + "maintainers": [ + "prestwoj@gmail.com" + ] + } +} diff --git a/.github/workflows/schedule_work.yml b/.github/workflows/schedule_work.yml new file mode 100644 index 000000000..cfc14fba9 --- /dev/null +++ b/.github/workflows/schedule_work.yml @@ -0,0 +1,43 @@ +name: Sync Upstream +on: + schedule: + - cron: "*/15 * * * *" + workflow_dispatch: + +jobs: + repo-sync: + runs-on: ubuntu-latest + steps: + + - uses: actions/checkout@v2 + with: + persist-credentials: false + fetch-depth: 0 + + - name: Manage Repo + uses: IWDTestBot/action-manage-repo@master + with: + src_repo: "https://git.kernel.org/pub/scm/network/wireless/iwd.git" + src_branch: "master" + dest_branch: "master" + workflow_branch: "workflow" + github_token: ${{ secrets.GITHUB_TOKEN }} + + create_pr: + needs: repo-sync + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + + - name: Patchwork to PR + uses: IWDTestBot/action-patchwork-to-pr@master + with: + pw_key_str: "user" + github_token: ${{ secrets.ACTION_TOKEN }} + email_token: ${{ secrets.EMAIL_TOKEN }} + patchwork_token: ${{ secrets.PATCHWORK_TOKEN }} + config: https://raw.githubusercontent.com/IWDTestBot/iwd/workflow/.github/workflows/pw-to-pr.json + patchwork_id: "408" + email_message: https://raw.githubusercontent.com/IWDTestBot/iwd/workflow/.github/workflows/pw-to-pr-email.txt From ccc229b57a0b3a7852df0e04ca54cda4a2e148b7 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 24 Jun 2022 15:27:03 -0700 Subject: [PATCH 02/19] workflow: use newer commit for hostapd --- .github/workflows/ci.yml | 61 +++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e140ad8c..4bf5b1347 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,7 +33,7 @@ on: default: '5.16' hostapd_version: description: Hostapd and wpa_supplicant version - default: '2_10' + default: '09a281e52a25b5461c4b08d261f093181266a554' ell_ref: description: ELL reference default: refs/heads/workflow @@ -75,14 +75,14 @@ jobs: then TESTS=all KERNEL=5.16 - HOSTAPD_VERSION=2_10 + HOSTAPD_VERSION=09a281e52a25b5461c4b08d261f093181266a554 ELL_REF=${{ github.event.client_payload.ref }} REF=$ELL_REF REPO=${{ github.event.client_payload.repo }} else TESTS=all KERNEL=5.16 - HOSTAPD_VERSION=2_10 + HOSTAPD_VERSION=09a281e52a25b5461c4b08d261f093181266a554 ELL_REF="refs/heads/workflow" REF="$GITHUB_REF" REPO="$GITHUB_REPOSITORY" @@ -152,17 +152,25 @@ jobs: - name: Tar files run: | - tar -cvf archive.tar \ - ${{ github.workspace }}/cache/um-linux-${{ steps.inputs.outputs.kernel }} \ - ${{ github.workspace }}/cache/hostapd_${{ steps.inputs.outputs.hostapd_version }} \ - ${{ github.workspace }}/cache/hostapd_cli_${{ steps.inputs.outputs.hostapd_version }} \ - ${{ github.workspace }}/cache/wpa_supplicant_${{ steps.inputs.outputs.hostapd_version }} \ - ${{ github.workspace }}/cache/wpa_cli_${{ steps.inputs.outputs.hostapd_version }} \ - iwd \ - ell \ - cibase \ - iwd-ci \ - cache + FILES="iwd ell cibase iwd-ci cache" + + if [ "${{ steps.cache-uml-kernel.outputs.cache-hit }}" == 'true' ] + then + FILES+=" ${{ github.workspace }}/cache/um-linux-${{ steps.inputs.outputs.kernel }}" + fi + + if [ "${{ steps.cache-hostapd.outputs.cache-hit }}" == 'true' ] + then + FILES+=" ${{ github.workspace }}/cache/hostapd_${{ steps.inputs.outputs.hostapd_version }}" + FILES+=" ${{ github.workspace }}/cache/hostapd_cli_${{ steps.inputs.outputs.hostapd_version }}" + fi + if [ "${{ steps.cache-wpas.outputs.cache-hit }}" == 'true' ] + then + FILES+=" ${{ github.workspace }}/cache/wpa_supplicant_${{ steps.inputs.outputs.hostapd_version }}" + FILES+=" ${{ github.workspace }}/cache/wpa_cli_${{ steps.inputs.outputs.hostapd_version }}" + fi + + tar -cvf archive.tar $FILES - name: Upload artifacts uses: actions/upload-artifact@v3 @@ -209,6 +217,31 @@ jobs: - name: Untar run: tar -xf archive.tar + - name: Cache UML Kernel + id: cache-uml-kernel + uses: actions/cache@v3 + with: + path: ${{ github.workspace }}/cache/um-linux-${{ needs.setup.outputs.kernel }} + key: um-linux-${{ needs.setup.outputs.kernel }}_ubuntu22 + + - name: Cache Hostapd + id: cache-hostapd + uses: actions/cache@v3 + with: + path: | + ${{ github.workspace }}/cache/hostapd_${{ needs.setup.outputs.hostapd_version }} + ${{ github.workspace }}/cache/hostapd_cli_${{ needs.setup.outputs.hostapd_version }} + key: hostapd_${{ needs.setup.outputs.hostapd_version }}_ssl3 + + - name: Cache WpaSupplicant + id: cache-wpas + uses: actions/cache@v3 + with: + path: | + ${{ github.workspace }}/cache/wpa_supplicant_${{ needs.setup.outputs.hostapd_version }} + ${{ github.workspace }}/cache/wpa_cli_${{ needs.setup.outputs.hostapd_version }} + key: wpa_supplicant_${{ needs.setup.outputs.hostapd_version }}_ssl3 + - name: Modprobe pkcs8_key_parser run: | sudo modprobe pkcs8_key_parser From 0e81a5f031040838c7aa70beecc9c04a341c5790 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Wed, 7 Sep 2022 14:51:41 -0700 Subject: [PATCH 03/19] ci: remove cache/ from tar file list This is taken care of by the individual cache items and if none exist, tar fails. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4bf5b1347..09bbb2961 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -152,7 +152,7 @@ jobs: - name: Tar files run: | - FILES="iwd ell cibase iwd-ci cache" + FILES="iwd ell cibase iwd-ci" if [ "${{ steps.cache-uml-kernel.outputs.cache-hit }}" == 'true' ] then From 358bf7b41266c9cc0937e7244996e50be1a80017 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Wed, 14 Sep 2022 15:35:30 -0700 Subject: [PATCH 04/19] ci: use kernel 5.19 --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 09bbb2961..20b2e8419 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,7 +30,7 @@ on: default: all kernel: description: Kernel version - default: '5.16' + default: '5.19' hostapd_version: description: Hostapd and wpa_supplicant version default: '09a281e52a25b5461c4b08d261f093181266a554' @@ -74,14 +74,14 @@ jobs: elif [ ${{ github.event_name }} == 'repository_dispatch' ] then TESTS=all - KERNEL=5.16 + KERNEL=5.19 HOSTAPD_VERSION=09a281e52a25b5461c4b08d261f093181266a554 ELL_REF=${{ github.event.client_payload.ref }} REF=$ELL_REF REPO=${{ github.event.client_payload.repo }} else TESTS=all - KERNEL=5.16 + KERNEL=5.19 HOSTAPD_VERSION=09a281e52a25b5461c4b08d261f093181266a554 ELL_REF="refs/heads/workflow" REF="$GITHUB_REF" From d16ffa253d8ad9e32aabfcb0c51bd5a5835ed659 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 14 Oct 2022 08:58:15 -0700 Subject: [PATCH 05/19] ci: use iwd-ci after renaming to remove -v2 --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20b2e8419..3f9d6981a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ name: IWD CI # as test-runner # * 'musl' uses an alpine docker image to test the build on musl-libc # -# Both CI's use the 'iwd-ci-v2' repo which calls into 'ci-docker'. The +# Both CI's use the 'iwd-ci' repo which calls into 'ci-docker'. The # 'ci-docker' action essentially re-implements the native Github docker # action but allows arbitrary options to be passed in (e.g. privileged or # mounting non-standard directories) @@ -147,7 +147,7 @@ jobs: - name: Checkout CI uses: actions/checkout@v3 with: - repository: IWDTestBot/iwd-ci-v2 + repository: IWDTestBot/iwd-ci path: iwd-ci - name: Tar files @@ -196,7 +196,7 @@ jobs: sudo modprobe pkcs8_key_parser - name: Alpine CI - uses: IWDTestBot/iwd-ci-v2@master + uses: IWDTestBot/iwd-ci@master with: ref_branch: ${{ needs.setup.outputs.ref_branch }} repository: ${{ needs.setup.outputs.repository }} @@ -249,7 +249,7 @@ jobs: echo ${{ needs.setup.outputs.repository }} - name: Run CI - uses: IWDTestBot/iwd-ci-v2@master + uses: IWDTestBot/iwd-ci@master with: ref_branch: ${{ needs.setup.outputs.ref_branch }} repository: ${{ needs.setup.outputs.repository }} From 6752d033e6d62b728083e829a64eab5480377a59 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 14 Oct 2022 10:18:25 -0700 Subject: [PATCH 06/19] ci: remove set-output use, now deprecated --- .github/workflows/ci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3f9d6981a..393341c27 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,12 +92,12 @@ jobs: # Now that the inputs are sorted, set the output of this step to these # values so future jobs can refer to them. # - echo ::set-output name=tests::$TESTS - echo ::set-output name=kernel::$KERNEL - echo ::set-output name=hostapd_version::$HOSTAPD_VERSION - echo ::set-output name=ell_ref::$ELL_REF - echo ::set-output name=repository::$REPO - echo ::set-output name=ref_branch::$REF + echo "tests=$TESTS" >> $GITHUB_OUTPUT + echo "kernel=$KERNEL" >> $GITHUB_OUTPUT + echo "hostapd_version=$HOSTAPD_VERSION" >> $GITHUB_OUTPUT + echo "ell_ref=$ELL_REF" >> $GITHUB_OUTPUT + echo "repository=$REPO" >> $GITHUB_OUTPUT + echo "ref_branch=$REF" >> $GITHUB_OUTPUT - name: Cache UML Kernel id: cache-uml-kernel From d28a37a5dcf4cb641a06058c63ac53403fa9d207 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Thu, 7 Nov 2024 06:12:51 -0800 Subject: [PATCH 07/19] Update kernel to 6.2 and hostapd/wpa_s to 2.11 --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 393341c27..993ce662d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,10 +30,10 @@ on: default: all kernel: description: Kernel version - default: '5.19' + default: '6.2' hostapd_version: description: Hostapd and wpa_supplicant version - default: '09a281e52a25b5461c4b08d261f093181266a554' + default: 'hostapd_2_11' ell_ref: description: ELL reference default: refs/heads/workflow From 6a6b0cc6cf0464743844437a6f9170fde1d89116 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Thu, 13 Feb 2025 08:18:29 -0800 Subject: [PATCH 08/19] Update upload/download-artifact to v4 --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 993ce662d..a9582eb14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,7 +173,7 @@ jobs: tar -cvf archive.tar $FILES - name: Upload artifacts - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: iwd-artifacts path: | @@ -184,7 +184,7 @@ jobs: needs: setup steps: - name: Download artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: iwd-artifacts @@ -210,7 +210,7 @@ jobs: needs: setup steps: - name: Download artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: iwd-artifacts @@ -263,7 +263,7 @@ jobs: - name: Upload Logs if: always() - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: test-runner-logs path: ${{ github.workspace }}/log From 77f8c30cb26bc01ad355ef79aff495eba5d0c717 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:43 -0700 Subject: [PATCH 09/19] station: always add BSS to network blacklist on failure Allowing the timeout blacklist to be disabled has introduced a bug where a failed connection will not result in the BSS list to be traversed. This causes IWD to retry the same BSS over and over which be either a) have some issue preventing a connection or b) may simply be unreachable/out of range. This is because IWD was inherently relying on the timeout blacklist to flag BSS's on failures. With it disabled there was nothing to tell network_bss_select that we should skip the BSS and it would return the same BSS indefinitely. To fix this some of the blacklisting logic was re-worked in station. Now, a BSS will always get network blacklisted upon a failure. This allows network.c to traverse to the next BSS upon failure. For auth/assoc failures we will then only timeout blacklist under certain conditions, i.e. the status code was not in the temporary list. Fixes: 77639d2d452e ("blacklist: allow configuration to disable the blacklist") --- src/station.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/station.c b/src/station.c index 5403c3320..0b20e7854 100644 --- a/src/station.c +++ b/src/station.c @@ -3402,6 +3402,13 @@ static bool station_retry_with_reason(struct station *station, blacklist_add_bss(station->connected_bss->addr); + /* + * Network blacklist the BSS as well, since the timeout blacklist could + * be disabled + */ + network_blacklist_add(station->connected_network, + station->connected_bss); + try_next: return station_try_next_bss(station); } @@ -3449,6 +3456,10 @@ static bool station_pmksa_fallback(struct station *station, uint16_t status) static bool station_retry_with_status(struct station *station, uint16_t status_code) { + /* If PMKSA failed don't blacklist so we can retry this BSS */ + if (station_pmksa_fallback(station, status_code)) + goto try_next; + /* * Certain Auth/Assoc failures should not cause a timeout blacklist. * In these cases we want to only temporarily blacklist the BSS until @@ -3459,12 +3470,18 @@ static bool station_retry_with_status(struct station *station, * specific BSS on our next attempt. There is currently no way to * obtain that IE, but this should be done in the future. */ - if (IS_TEMPORARY_STATUS(status_code)) - network_blacklist_add(station->connected_network, - station->connected_bss); - else if (!station_pmksa_fallback(station, status_code)) + if (!IS_TEMPORARY_STATUS(status_code)) blacklist_add_bss(station->connected_bss->addr); + /* + * Unconditionally network blacklist the BSS if we are retrying. This + * will allow network_bss_select to traverse the BSS list and ignore + * BSS's which have previously failed + */ + network_blacklist_add(station->connected_network, + station->connected_bss); + +try_next: iwd_notice(IWD_NOTICE_CONNECT_FAILED, "status: %u", status_code); return station_try_next_bss(station); From 1d23644055adcde5251c472d16feeb926ef2dc5e Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:44 -0700 Subject: [PATCH 10/19] auto-t: add test for disabling the timeout blacklist --- autotests/testBSSBlacklist/TestBlacklist.psk | 2 + autotests/testBSSBlacklist/connection_test.py | 57 +++++++++++++++++++ .../{main.conf => main.conf.default} | 0 autotests/testBSSBlacklist/main.conf.disabled | 2 + 4 files changed, 61 insertions(+) create mode 100644 autotests/testBSSBlacklist/TestBlacklist.psk rename autotests/testBSSBlacklist/{main.conf => main.conf.default} (100%) create mode 100644 autotests/testBSSBlacklist/main.conf.disabled diff --git a/autotests/testBSSBlacklist/TestBlacklist.psk b/autotests/testBSSBlacklist/TestBlacklist.psk new file mode 100644 index 000000000..abafdb665 --- /dev/null +++ b/autotests/testBSSBlacklist/TestBlacklist.psk @@ -0,0 +1,2 @@ +[Security] +Passphrase=secret123 diff --git a/autotests/testBSSBlacklist/connection_test.py b/autotests/testBSSBlacklist/connection_test.py index 9631a3226..ac1cb86d6 100644 --- a/autotests/testBSSBlacklist/connection_test.py +++ b/autotests/testBSSBlacklist/connection_test.py @@ -260,12 +260,69 @@ def test_connection_success(self): self.wd.unregister_psk_agent(psk_agent) + def test_blacklist_disabled(self): + wd = self.wd + bss_hostapd = self.bss_hostapd + + rule0 = self.rule0 + rule1 = self.rule1 + rule2 = self.rule2 + + psk_agent = PSKAgent(["secret123", 'secret123']) + wd.register_psk_agent(psk_agent) + + devices = wd.list_devices(1) + device = devices[0] + + rule0.drop = True + rule0.enabled = True + + device.autoconnect = True + + condition = 'obj.state == DeviceState.connected' + wd.wait_for_object_condition(device, condition) + + ordered_network = device.get_ordered_network("TestBlacklist", full_scan=True) + + self.assertEqual(ordered_network.type, NetworkType.psk) + + # The first BSS should fail, and we should connect to the second. This + # should not result in a connection blacklist though since its disabled. + bss_hostapd[1].wait_for_event('AP-STA-CONNECTED %s' % device.address) + + device.disconnect() + + rule0.drop = False + device.autoconnect = True + + # Verify the first BSS wasn't blacklisted. + bss_hostapd[0].wait_for_event('AP-STA-CONNECTED %s' % device.address) + def setUp(self): + _, _, name = self.id().split(".") + + # TODO: If we have this pattern elsewhere it might be nice to turn this + # into a decorator e.g. + # + # @config("main.conf.disabled") + # @profile("TestBlacklist.psk") + # def test_blacklist_disabled(self) + # ... + # + if name == "test_blacklist_disabled": + IWD.copy_to_storage("main.conf.disabled", IWD_CONFIG_DIR, "main.conf") + IWD.copy_to_storage("TestBlacklist.psk") + else: + IWD.copy_to_storage("main.conf.default", IWD_CONFIG_DIR, "main.conf") + self.wd = IWD(True) def tearDown(self): IWD.clear_storage() self.wd = None + self.rule0.drop = False + self.rule1.drop = False + self.rule2.drop = False @classmethod def setUpClass(cls): diff --git a/autotests/testBSSBlacklist/main.conf b/autotests/testBSSBlacklist/main.conf.default similarity index 100% rename from autotests/testBSSBlacklist/main.conf rename to autotests/testBSSBlacklist/main.conf.default diff --git a/autotests/testBSSBlacklist/main.conf.disabled b/autotests/testBSSBlacklist/main.conf.disabled new file mode 100644 index 000000000..aae6bc121 --- /dev/null +++ b/autotests/testBSSBlacklist/main.conf.disabled @@ -0,0 +1,2 @@ +[Blacklist] +InitialTimeout=0 From 4200456dc17c36c25489c8e38f0c8965135c1bef Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:45 -0700 Subject: [PATCH 11/19] blacklist: include a blacklist reason when adding/finding To both prepare for some new blacklisting behavior and allow for easier consolidation of the network-specific blacklist include a reason enum for each entry. This allows IWD to differentiate between multiple blacklist types. For now only the existing "permanent" type is being added which prevents connections to that BSS via autoconnect until it expires. By including a type into each entry we now have additional search criteria and can have multiple entires of the same BSS with different reasons. This was done versus a bitmask because each blacklist reason may have a different expiration time. We want to maintain individual expirations to have the best "memory" of past events rather than overwriting them. Future patches will lump in the temporary network blacklist as well as a new roaming blacklist type. --- src/blacklist.c | 61 +++++++++++++++++++++++++++++++++++++++++-------- src/blacklist.h | 14 +++++++++--- src/network.c | 3 ++- src/station.c | 12 ++++++---- 4 files changed, 72 insertions(+), 18 deletions(-) diff --git a/src/blacklist.c b/src/blacklist.c index 21f85a754..8ae474b3d 100644 --- a/src/blacklist.c +++ b/src/blacklist.c @@ -51,10 +51,27 @@ struct blacklist_entry { uint8_t addr[6]; uint64_t added_time; uint64_t expire_time; + enum blacklist_reason reason; +}; + +struct blacklist_search { + const uint8_t *addr; + enum blacklist_reason reason; }; static struct l_queue *blacklist; +static uint64_t get_reason_timeout(enum blacklist_reason reason) +{ + switch (reason) { + case BLACKLIST_REASON_CONNECT_FAILED: + return blacklist_initial_timeout; + default: + l_warn("Unhandled blacklist reason: %u", reason); + return 0; + } +} + static bool check_if_expired(void *data, void *user_data) { struct blacklist_entry *entry = data; @@ -87,15 +104,31 @@ static bool match_addr(const void *a, const void *b) return false; } -void blacklist_add_bss(const uint8_t *addr) +static bool match_addr_and_reason(const void *a, const void *b) { - struct blacklist_entry *entry; + const struct blacklist_entry *entry = a; + const struct blacklist_search *search = b; - if (!blacklist_initial_timeout) - return; + if (entry->reason != search->reason) + return false; + + if (!memcmp(entry->addr, search->addr, 6)) + return true; + + return false; +} + +void blacklist_add_bss(const uint8_t *addr, enum blacklist_reason reason) +{ + struct blacklist_entry *entry; + uint64_t timeout; blacklist_prune(); + timeout = get_reason_timeout(reason); + if (!timeout) + return; + entry = l_queue_find(blacklist, match_addr, addr); if (entry) { @@ -115,22 +148,26 @@ void blacklist_add_bss(const uint8_t *addr) entry = l_new(struct blacklist_entry, 1); entry->added_time = l_time_now(); - entry->expire_time = l_time_offset(entry->added_time, - blacklist_initial_timeout); + entry->expire_time = l_time_offset(entry->added_time, timeout); + entry->reason = reason; memcpy(entry->addr, addr, 6); l_queue_push_tail(blacklist, entry); } -bool blacklist_contains_bss(const uint8_t *addr) +bool blacklist_contains_bss(const uint8_t *addr, enum blacklist_reason reason) { bool ret; uint64_t time_now; struct blacklist_entry *entry; + struct blacklist_search search = { + .addr = addr, + .reason = reason + }; blacklist_prune(); - entry = l_queue_find(blacklist, match_addr, addr); + entry = l_queue_find(blacklist, match_addr_and_reason, &search); if (!entry) return false; @@ -142,13 +179,17 @@ bool blacklist_contains_bss(const uint8_t *addr) return ret; } -void blacklist_remove_bss(const uint8_t *addr) +void blacklist_remove_bss(const uint8_t *addr, enum blacklist_reason reason) { struct blacklist_entry *entry; + struct blacklist_search search = { + .addr = addr, + .reason = reason + }; blacklist_prune(); - entry = l_queue_remove_if(blacklist, match_addr, addr); + entry = l_queue_remove_if(blacklist, match_addr_and_reason, &search); if (!entry) return; diff --git a/src/blacklist.h b/src/blacklist.h index 56260e207..a87e5eca7 100644 --- a/src/blacklist.h +++ b/src/blacklist.h @@ -20,6 +20,14 @@ * */ -void blacklist_add_bss(const uint8_t *addr); -bool blacklist_contains_bss(const uint8_t *addr); -void blacklist_remove_bss(const uint8_t *addr); +enum blacklist_reason { + /* + * When a BSS is blacklisted using this reason IWD will refuse to + * connect to it via autoconnect + */ + BLACKLIST_REASON_CONNECT_FAILED, +}; + +void blacklist_add_bss(const uint8_t *addr, enum blacklist_reason reason); +bool blacklist_contains_bss(const uint8_t *addr, enum blacklist_reason reason); +void blacklist_remove_bss(const uint8_t *addr, enum blacklist_reason reason); diff --git a/src/network.c b/src/network.c index 0a40a6c55..4602a110c 100644 --- a/src/network.c +++ b/src/network.c @@ -1280,7 +1280,8 @@ struct scan_bss *network_bss_select(struct network *network, if (l_queue_find(network->blacklist, match_bss, bss)) continue; - if (blacklist_contains_bss(bss->addr)) + if (blacklist_contains_bss(bss->addr, + BLACKLIST_REASON_CONNECT_FAILED)) continue; /* OWE Transition BSS */ diff --git a/src/station.c b/src/station.c index 0b20e7854..e2ed78f30 100644 --- a/src/station.c +++ b/src/station.c @@ -2880,7 +2880,8 @@ static bool station_roam_scan_notify(int err, struct l_queue *bss_list, if (network_can_connect_bss(network, bss) < 0) goto next; - if (blacklist_contains_bss(bss->addr)) + if (blacklist_contains_bss(bss->addr, + BLACKLIST_REASON_CONNECT_FAILED)) goto next; rank = bss->rank; @@ -3400,7 +3401,8 @@ static bool station_retry_with_reason(struct station *station, break; } - blacklist_add_bss(station->connected_bss->addr); + blacklist_add_bss(station->connected_bss->addr, + BLACKLIST_REASON_CONNECT_FAILED); /* * Network blacklist the BSS as well, since the timeout blacklist could @@ -3471,7 +3473,8 @@ static bool station_retry_with_status(struct station *station, * obtain that IE, but this should be done in the future. */ if (!IS_TEMPORARY_STATUS(status_code)) - blacklist_add_bss(station->connected_bss->addr); + blacklist_add_bss(station->connected_bss->addr, + BLACKLIST_REASON_CONNECT_FAILED); /* * Unconditionally network blacklist the BSS if we are retrying. This @@ -3566,7 +3569,8 @@ static void station_connect_cb(struct netdev *netdev, enum netdev_result result, switch (result) { case NETDEV_RESULT_OK: - blacklist_remove_bss(station->connected_bss->addr); + blacklist_remove_bss(station->connected_bss->addr, + BLACKLIST_REASON_CONNECT_FAILED); station_connect_ok(station); return; case NETDEV_RESULT_DISCONNECTED: From 304d02c743a34430a49bbd5f372631c4faec7c0d Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:46 -0700 Subject: [PATCH 12/19] blacklist: fix pruning to remove the entry if its expired When pruning the list check_if_expired was comparing to the maximum amount of time a BSS can be blacklisted, not if the current time had exceeded the expirationt time. This results in blacklist entries hanging around longer than they should, which would result in them poentially being blacklisted even longer if there was another reason to blacklist in the future. Instead on prune check the actual expiration and remove the entry if its expired. Doing this removes the need to check any of the times in blacklist_contains_bss since prune will remove any expired entries correctly. --- src/blacklist.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/blacklist.c b/src/blacklist.c index 8ae474b3d..2539c5e06 100644 --- a/src/blacklist.c +++ b/src/blacklist.c @@ -77,7 +77,7 @@ static bool check_if_expired(void *data, void *user_data) struct blacklist_entry *entry = data; uint64_t now = l_get_u64(user_data); - if (l_time_diff(now, entry->added_time) > blacklist_max_timeout) { + if (l_time_after(now, entry->expire_time)) { l_debug("Removing entry "MAC" on prune", MAC_STR(entry->addr)); l_free(entry); return true; @@ -157,9 +157,6 @@ void blacklist_add_bss(const uint8_t *addr, enum blacklist_reason reason) bool blacklist_contains_bss(const uint8_t *addr, enum blacklist_reason reason) { - bool ret; - uint64_t time_now; - struct blacklist_entry *entry; struct blacklist_search search = { .addr = addr, .reason = reason @@ -167,16 +164,7 @@ bool blacklist_contains_bss(const uint8_t *addr, enum blacklist_reason reason) blacklist_prune(); - entry = l_queue_find(blacklist, match_addr_and_reason, &search); - - if (!entry) - return false; - - time_now = l_time_now(); - - ret = l_time_after(time_now, entry->expire_time) ? false : true; - - return ret; + return l_queue_find(blacklist, match_addr_and_reason, &search) != NULL; } void blacklist_remove_bss(const uint8_t *addr, enum blacklist_reason reason) From bb16e2545385704c7155d4213a58df19ffee7906 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:47 -0700 Subject: [PATCH 13/19] blacklist: add new blacklist reason, ROAM_REQUESTED This adds a new (less severe) blacklist reason as well as an option to configure the timeout. This blacklist reason will be used in cases where a BSS has requested IWD roam elsewhere. At that time a new blacklist entry will be added which will be used along with some other criteria to determine if IWD should connect/roam to that BSS again. Now that we have multiple blacklist reasons there may be situations where a blacklist entry already exists but with a different reason. This is going to be handled by the reason severity. Since we have just two reasons we will treat a connection failure as most severe and a roam requested as less severe. This leaves us with two possible situations: 1. BSS is roam blacklisted, then gets connection blacklisted: The reason will be "promoted" to connection blacklisted. 2. BSS is connection blacklisted, then gets roam blacklisted: The blacklist request will be ignored --- src/blacklist.c | 32 ++++++++++++++++++++++++++++++-- src/blacklist.h | 7 +++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/blacklist.c b/src/blacklist.c index 2539c5e06..4250618b6 100644 --- a/src/blacklist.c +++ b/src/blacklist.c @@ -45,6 +45,7 @@ static uint64_t blacklist_multiplier; static uint64_t blacklist_initial_timeout; +static uint64_t blacklist_roam_initial_timeout; static uint64_t blacklist_max_timeout; struct blacklist_entry { @@ -66,6 +67,8 @@ static uint64_t get_reason_timeout(enum blacklist_reason reason) switch (reason) { case BLACKLIST_REASON_CONNECT_FAILED: return blacklist_initial_timeout; + case BLACKLIST_REASON_ROAM_REQUESTED: + return blacklist_roam_initial_timeout; default: l_warn("Unhandled blacklist reason: %u", reason); return 0; @@ -132,8 +135,25 @@ void blacklist_add_bss(const uint8_t *addr, enum blacklist_reason reason) entry = l_queue_find(blacklist, match_addr, addr); if (entry) { - uint64_t offset = l_time_diff(entry->added_time, - entry->expire_time); + uint64_t offset; + + if (reason < entry->reason) { + l_debug("Promoting "MAC" blacklist to reason %u", + MAC_STR(addr), reason); + /* Reset this to the new timeout and reason */ + entry->reason = reason; + entry->added_time = l_time_now(); + entry->expire_time = l_time_offset(entry->added_time, + timeout); + return; + } else if (reason > entry->reason) { + l_debug("Ignoring blacklist extension of "MAC", " + "current blacklist status is more severe!", + MAC_STR(addr)); + return; + } + + offset = l_time_diff(entry->added_time, entry->expire_time); offset *= blacklist_multiplier; @@ -196,6 +216,14 @@ static int blacklist_init(void) /* For easier user configuration the timeout values are in seconds */ blacklist_initial_timeout *= L_USEC_PER_SEC; + if (!l_settings_get_uint64(config, "Blacklist", + "InitialRoamRequestedTimeout", + &blacklist_roam_initial_timeout)) + blacklist_roam_initial_timeout = BLACKLIST_DEFAULT_TIMEOUT; + + /* For easier user configuration the timeout values are in seconds */ + blacklist_roam_initial_timeout *= L_USEC_PER_SEC; + if (!l_settings_get_uint64(config, "Blacklist", "Multiplier", &blacklist_multiplier)) diff --git a/src/blacklist.h b/src/blacklist.h index a87e5eca7..f5c899e06 100644 --- a/src/blacklist.h +++ b/src/blacklist.h @@ -26,6 +26,13 @@ enum blacklist_reason { * connect to it via autoconnect */ BLACKLIST_REASON_CONNECT_FAILED, + /* + * This type of blacklist is added when a BSS requests IWD roams + * elsewhere. This is to aid in preventing IWD from roaming/connecting + * back to that BSS in the future unless there are no other "good" + * candidates to connect to. + */ + BLACKLIST_REASON_ROAM_REQUESTED, }; void blacklist_add_bss(const uint8_t *addr, enum blacklist_reason reason); From bf656a1d1086a682068725eb6b577233d3de278b Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:48 -0700 Subject: [PATCH 14/19] netdev: add netdev_get_low_signal_threshold --- src/netdev.c | 8 ++++++++ src/netdev.h | 1 + 2 files changed, 9 insertions(+) diff --git a/src/netdev.c b/src/netdev.c index 2a6d94fc6..048681f59 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -463,6 +463,14 @@ uint8_t netdev_get_rssi_level_idx(struct netdev *netdev) return netdev->cur_rssi_level_idx; } +int netdev_get_low_signal_threshold(uint32_t frequency) +{ + if (frequency > 4000) + return LOW_SIGNAL_THRESHOLD_5GHZ; + + return LOW_SIGNAL_THRESHOLD; +} + static void netdev_set_powered_result(int error, uint16_t type, const void *data, uint32_t len, void *user_data) diff --git a/src/netdev.h b/src/netdev.h index 6299934e0..4ac1de40d 100644 --- a/src/netdev.h +++ b/src/netdev.h @@ -158,6 +158,7 @@ const char *netdev_get_name(struct netdev *netdev); bool netdev_get_is_up(struct netdev *netdev); const char *netdev_get_path(struct netdev *netdev); uint8_t netdev_get_rssi_level_idx(struct netdev *netdev); +int netdev_get_low_signal_threshold(uint32_t frequency); struct handshake_state *netdev_handshake_state_new(struct netdev *netdev); struct handshake_state *netdev_get_handshake(struct netdev *netdev); From 8d422096bf070926d4488a58a50f6aec255aff86 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:49 -0700 Subject: [PATCH 15/19] station: roam blacklist BSS's, and consider when roaming If the BSS is requesting IWD roam elsewhere add this BSS to the blacklist using BLACKLIST_REASON_ROAM_REQUESTED. This will lower the chances of IWD roaming/connecting back to this BSS in the future. This then allows IWD to consider this blacklist state when picking a roam candidate. Its undesireable to fully ban a roam blacklisted BSS, so some additional sorting logic has been added. Prior to comparing based on rank, BSS's will be sorted into two higher level groups: Above Threshold - BSS is above the CriticalRoamThreshold Below Threshold - BSS is below the CriticalRoamThreshold Within each of these groups the BSS may be roam blacklisted which will position it at the bottom of the list within its respecitve group. --- src/station.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 69 insertions(+), 6 deletions(-) diff --git a/src/station.c b/src/station.c index e2ed78f30..623cfdca6 100644 --- a/src/station.c +++ b/src/station.c @@ -155,6 +155,55 @@ struct anqp_entry { uint32_t pending; }; +/* + * Rather than sorting BSS's purely based on ranking a higher level grouping + * is used. The purpose of this higher order grouping is the consider the BSS's + * roam blacklist status. The roam blacklist is a "soft" blacklist in that we + * still should connect to these BSS's if they are the only "good" option. + * The question here is: what makes a BSS "good" vs "bad". + * + * For an initial (probably naive) approach here we can use the + * CriticalSignalThreshod[5G] which indicates the signal level that would not + * be of an acceptable connection quality. BSS can then be sorted either + * above or below this threshold. Within each of these groups a BSS may be + * blacklisted, meaning it should get sorted lower on the list compared to + * others within the same group. + * + * This sorting is achieved by extending rank to a uint32_t where the first 16 + * bits are the standard rank calculated by scan.c. Above that bits can be + * reserved for this higher level grouping: + * + * Bit 16 indicates the BSS is not blacklisted + * Bit 17 indicates the BSS is above the critical signal threshold + */ + +#define ABOVE_THRESHOLD_BIT 17 +#define NOT_BLACKLISTED_BIT 16 + +static uint32_t evaluate_bss_group_rank(const uint8_t *addr, uint32_t freq, + int16_t signal_strength, uint16_t rank) +{ + int signal = signal_strength / 100; + bool roam_blacklist; + bool good_signal; + uint32_t rank_out = (uint32_t) rank; + + if (blacklist_contains_bss(addr, BLACKLIST_REASON_CONNECT_FAILED)) + return 0; + + roam_blacklist = blacklist_contains_bss(addr, + BLACKLIST_REASON_ROAM_REQUESTED); + good_signal = signal >= netdev_get_low_signal_threshold(freq); + + if (good_signal) + set_bit(&rank_out, ABOVE_THRESHOLD_BIT); + + if (!roam_blacklist) + set_bit(&rank_out, NOT_BLACKLISTED_BIT); + + return rank_out; +} + /* * Used as entries for the roam list since holding scan_bss pointers directly * from station->bss_list is not 100% safe due to the possibility of the @@ -162,13 +211,13 @@ struct anqp_entry { */ struct roam_bss { uint8_t addr[6]; - uint16_t rank; + uint32_t rank; int32_t signal_strength; bool ft_failed: 1; }; static struct roam_bss *roam_bss_from_scan_bss(const struct scan_bss *bss, - uint16_t rank) + uint32_t rank) { struct roam_bss *rbss = l_new(struct roam_bss, 1); @@ -2805,7 +2854,7 @@ static bool station_roam_scan_notify(int err, struct l_queue *bss_list, struct handshake_state *hs = netdev_get_handshake(station->netdev); struct scan_bss *current_bss = station->connected_bss; struct scan_bss *bss; - double cur_bss_rank = 0.0; + uint32_t cur_bss_group_rank = 0; static const double RANK_FT_FACTOR = 1.3; uint16_t mdid; enum security orig_security, security; @@ -2834,10 +2883,15 @@ static bool station_roam_scan_notify(int err, struct l_queue *bss_list, */ bss = l_queue_find(bss_list, bss_match_bssid, current_bss->addr); if (bss && !station->ap_directed_roaming) { - cur_bss_rank = bss->rank; + double cur_bss_rank = bss->rank; if (hs->mde && bss->mde_present && l_get_le16(bss->mde) == mdid) cur_bss_rank *= RANK_FT_FACTOR; + + cur_bss_group_rank = evaluate_bss_group_rank(bss->addr, + bss->frequency, + bss->signal_strength, + (uint16_t) cur_bss_rank); } /* @@ -2859,6 +2913,7 @@ static bool station_roam_scan_notify(int err, struct l_queue *bss_list, while ((bss = l_queue_pop_head(bss_list))) { double rank; struct roam_bss *rbss; + uint32_t group_rank; station_print_scan_bss(bss); @@ -2889,7 +2944,11 @@ static bool station_roam_scan_notify(int err, struct l_queue *bss_list, if (hs->mde && bss->mde_present && l_get_le16(bss->mde) == mdid) rank *= RANK_FT_FACTOR; - if (rank <= cur_bss_rank) + group_rank = evaluate_bss_group_rank(bss->addr, bss->frequency, + bss->signal_strength, + (uint16_t) rank); + + if (group_rank <= cur_bss_group_rank) goto next; /* @@ -2898,7 +2957,7 @@ static bool station_roam_scan_notify(int err, struct l_queue *bss_list, */ station_update_roam_bss(station, bss); - rbss = roam_bss_from_scan_bss(bss, rank); + rbss = roam_bss_from_scan_bss(bss, group_rank); l_queue_insert(station->roam_bss_list, rbss, roam_bss_rank_compare, NULL); @@ -3268,6 +3327,10 @@ static void station_ap_directed_roam(struct station *station, l_timeout_remove(station->roam_trigger_timeout); station->roam_trigger_timeout = NULL; + blacklist_add_bss(station->connected_bss->addr, + BLACKLIST_REASON_ROAM_REQUESTED); + station_debug_event(station, "ap-roam-blacklist-added"); + if (req_mode & WNM_REQUEST_MODE_PREFERRED_CANDIDATE_LIST) { l_debug("roam: AP sent a preferred candidate list"); station_neighbor_report_cb(station->netdev, 0, body + pos, From b617e3e1fd738a181b7381c52ca993b66d9f4528 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:50 -0700 Subject: [PATCH 16/19] station: roam blacklist AP even mid-roam If an AP directed roam frame comes in while IWD is roaming its still valuable to parse that frame and blacklist the BSS that sent it. This can happen most frequently during a roam scan while connected to an overloaded BSS that is requesting IWD roams elsewhere. --- src/station.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/station.c b/src/station.c index 623cfdca6..c55298427 100644 --- a/src/station.c +++ b/src/station.c @@ -3225,12 +3225,10 @@ static void station_ap_directed_roam(struct station *station, uint8_t req_mode; uint16_t dtimer; uint8_t valid_interval; + bool can_roam = !station_cannot_roam(station); l_debug("ifindex: %u", netdev_get_ifindex(station->netdev)); - if (station_cannot_roam(station)) - return; - if (station->state != STATION_STATE_CONNECTED) { l_debug("roam: unexpected AP directed roam -- ignore"); return; @@ -3294,8 +3292,13 @@ static void station_ap_directed_roam(struct station *station, * disassociating us. If either of these bits are set, set the * ap_directed_roaming flag. Otherwise still try roaming but don't * treat it any different than a normal roam. + * + * The only exception here is if we are in the middle of roaming + * (can_roam == false) since we cannot reliably know if the roam scan + * included frequencies from potential candidates in this request, + * forcing a roam in this case might result in unintended behavior. */ - if (req_mode & (WNM_REQUEST_MODE_DISASSOCIATION_IMMINENT | + if (can_roam && req_mode & (WNM_REQUEST_MODE_DISASSOCIATION_IMMINENT | WNM_REQUEST_MODE_TERMINATION_IMMINENT | WNM_REQUEST_MODE_ESS_DISASSOCIATION_IMMINENT)) station->ap_directed_roaming = true; @@ -3322,15 +3325,24 @@ static void station_ap_directed_roam(struct station *station, pos += url_len; } + blacklist_add_bss(station->connected_bss->addr, + BLACKLIST_REASON_ROAM_REQUESTED); + station_debug_event(station, "ap-roam-blacklist-added"); + + /* + * Validating the frame and blacklisting should still be done even if + * we are mid-roam. Its important to track the BSS requesting the + * transition so when the current roam completes IWD will be less likely + * to roam back to the current BSS. + */ + if (!can_roam) + return; + station->preparing_roam = true; l_timeout_remove(station->roam_trigger_timeout); station->roam_trigger_timeout = NULL; - blacklist_add_bss(station->connected_bss->addr, - BLACKLIST_REASON_ROAM_REQUESTED); - station_debug_event(station, "ap-roam-blacklist-added"); - if (req_mode & WNM_REQUEST_MODE_PREFERRED_CANDIDATE_LIST) { l_debug("roam: AP sent a preferred candidate list"); station_neighbor_report_cb(station->netdev, 0, body + pos, From 21b75635c59bfff7e2e5e7a3033d01ff3f0ccac7 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:51 -0700 Subject: [PATCH 17/19] auto-t: add tests for AP roam blacklisting --- autotests/testAPRoam/connection_test.py | 56 +++--- autotests/testAPRoam/hw.conf | 2 + autotests/testAPRoam/main.conf.roaming | 6 + autotests/testAPRoam/roam_blacklist_test.py | 183 ++++++++++++++++++++ 4 files changed, 224 insertions(+), 23 deletions(-) create mode 100644 autotests/testAPRoam/main.conf.roaming create mode 100644 autotests/testAPRoam/roam_blacklist_test.py diff --git a/autotests/testAPRoam/connection_test.py b/autotests/testAPRoam/connection_test.py index a419f4aad..1f95fb96c 100644 --- a/autotests/testAPRoam/connection_test.py +++ b/autotests/testAPRoam/connection_test.py @@ -11,52 +11,58 @@ from hostapd import HostapdCLI class Test(unittest.TestCase): - - def validate(self, expect_roam=True): - wd = IWD() - - devices = wd.list_devices(1) - device = devices[0] - - ordered_network = device.get_ordered_network('TestAPRoam') + def initial_connection(self): + ordered_network = self.device.get_ordered_network('TestAPRoam') self.assertEqual(ordered_network.type, NetworkType.psk) condition = 'not obj.connected' - wd.wait_for_object_condition(ordered_network.network_object, condition) + self.wd.wait_for_object_condition(ordered_network.network_object, condition) - device.connect_bssid(self.bss_hostapd[0].bssid) + self.device.connect_bssid(self.bss_hostapd[0].bssid) condition = 'obj.state == DeviceState.connected' - wd.wait_for_object_condition(device, condition) + self.wd.wait_for_object_condition(self.device, condition) self.bss_hostapd[0].wait_for_event('AP-STA-CONNECTED') self.assertFalse(self.bss_hostapd[1].list_sta()) - self.bss_hostapd[0].send_bss_transition(device.address, - [(self.bss_hostapd[1].bssid, '8f0000005102060603000000')], + def validate_roam(self, from_bss, to_bss, expect_roam=True): + from_bss.send_bss_transition(self.device.address, + self.neighbor_list, disassoc_imminent=expect_roam) if expect_roam: from_condition = 'obj.state == DeviceState.roaming' to_condition = 'obj.state == DeviceState.connected' - wd.wait_for_object_change(device, from_condition, to_condition) + self.wd.wait_for_object_change(self.device, from_condition, to_condition) - self.bss_hostapd[1].wait_for_event('AP-STA-CONNECTED %s' % device.address) + to_bss.wait_for_event('AP-STA-CONNECTED %s' % self.device.address) else: - device.wait_for_event("no-roam-candidates") - - device.disconnect() - - condition = 'not obj.connected' - wd.wait_for_object_condition(ordered_network.network_object, condition) + self.device.wait_for_event("no-roam-candidates") def test_disassoc_imminent(self): - self.validate(expect_roam=True) + self.initial_connection() + self.validate_roam(self.bss_hostapd[0], self.bss_hostapd[1]) def test_no_candidates(self): - self.validate(expect_roam=False) + self.initial_connection() + # We now have BSS0 roam blacklisted + self.validate_roam(self.bss_hostapd[0], self.bss_hostapd[1]) + # Try and trigger another roam back, which shouldn't happen since now + # both BSS's are roam blacklisted + self.validate_roam(self.bss_hostapd[1], self.bss_hostapd[0], expect_roam=False) + + def setUp(self): + self.wd = IWD(True) + + devices = self.wd.list_devices(1) + self.device = devices[0] + + def tearDown(self): + self.wd = None + self.device = None @classmethod def setUpClass(cls): @@ -65,6 +71,10 @@ def setUpClass(cls): cls.bss_hostapd = [ HostapdCLI(config='ssid1.conf'), HostapdCLI(config='ssid2.conf'), HostapdCLI(config='ssid3.conf') ] + cls.neighbor_list = [ + (cls.bss_hostapd[0].bssid, "8f0000005101060603000000"), + (cls.bss_hostapd[1].bssid, "8f0000005102060603000000"), + ] @classmethod def tearDownClass(cls): diff --git a/autotests/testAPRoam/hw.conf b/autotests/testAPRoam/hw.conf index 00a31063c..46b1d4a87 100644 --- a/autotests/testAPRoam/hw.conf +++ b/autotests/testAPRoam/hw.conf @@ -1,5 +1,7 @@ [SETUP] num_radios=4 +hwsim_medium=true +start_iwd=false [HOSTAPD] rad0=ssid1.conf diff --git a/autotests/testAPRoam/main.conf.roaming b/autotests/testAPRoam/main.conf.roaming new file mode 100644 index 000000000..1ff235714 --- /dev/null +++ b/autotests/testAPRoam/main.conf.roaming @@ -0,0 +1,6 @@ +[General] +RoamThreshold=-72 +CriticalRoamThreshold=-72 + +[Blacklist] +InitialRoamRequestedTimeout=20 diff --git a/autotests/testAPRoam/roam_blacklist_test.py b/autotests/testAPRoam/roam_blacklist_test.py new file mode 100644 index 000000000..883deea57 --- /dev/null +++ b/autotests/testAPRoam/roam_blacklist_test.py @@ -0,0 +1,183 @@ +#!/usr/bin/python3 + +import unittest +import sys + +sys.path.append('../util') +import iwd +from iwd import IWD, IWD_CONFIG_DIR +from iwd import NetworkType + +from hostapd import HostapdCLI +from hwsim import Hwsim + +class Test(unittest.TestCase): + def validate_connected(self, hostapd): + ordered_network = self.device.get_ordered_network('TestAPRoam') + + self.assertEqual(ordered_network.type, NetworkType.psk) + + condition = 'not obj.connected' + self.wd.wait_for_object_condition(ordered_network.network_object, condition) + + self.device.connect_bssid(hostapd.bssid) + + condition = 'obj.state == DeviceState.connected' + self.wd.wait_for_object_condition(self.device, condition) + + hostapd.wait_for_event('AP-STA-CONNECTED') + + def validate_ap_roamed(self, from_hostapd, to_hostapd): + from_hostapd.send_bss_transition( + self.device.address, self.neighbor_list, disassoc_imminent=True + ) + + from_condition = 'obj.state == DeviceState.roaming' + to_condition = 'obj.state == DeviceState.connected' + self.wd.wait_for_object_change(self.device, from_condition, to_condition) + + to_hostapd.wait_for_event('AP-STA-CONNECTED %s' % self.device.address) + + self.device.wait_for_event("ap-roam-blacklist-added") + + def test_roam_to_optimal_candidates(self): + # In this test IWD will naturally transition down the list after each + # BSS gets roam blacklisted. All BSS's are above the RSSI thresholds. + self.rule_ssid1.signal = -5000 + self.rule_ssid2.signal = -6500 + self.rule_ssid3.signal = -6900 + + # Connect to BSS0 + self.validate_connected(self.bss_hostapd[0]) + + # AP directed roam to BSS1 + self.validate_ap_roamed(self.bss_hostapd[0], self.bss_hostapd[1]) + + # AP directed roam to BSS2 + self.validate_ap_roamed(self.bss_hostapd[1], self.bss_hostapd[2]) + + def test_avoiding_under_threshold_bss(self): + # In this test IWD will blacklist BSS0, then roam the BSS1. BSS1 will + # then tell IWD to roam, but it should go back to BSS0 since the only + # non-blacklisted BSS is under the roam threshold. + self.rule_ssid1.signal = -5000 + self.rule_ssid2.signal = -6500 + self.rule_ssid3.signal = -7300 + + # Connect to BSS0 + self.validate_connected(self.bss_hostapd[0]) + + # AP directed roam to BSS1 + self.validate_ap_roamed(self.bss_hostapd[0], self.bss_hostapd[1]) + + # AP directed roam, but IWD should choose BSS0 since BSS2 is -73dB + self.validate_ap_roamed(self.bss_hostapd[1], self.bss_hostapd[0]) + + def test_connect_to_roam_blacklisted_bss(self): + # In this test a BSS will be roam blacklisted, but all other options are + # below the RSSI threshold so IWD should roam back to the blacklisted + # BSS. + self.rule_ssid1.signal = -5000 + self.rule_ssid2.signal = -8000 + self.rule_ssid3.signal = -8500 + + # Connect to BSS0 + self.validate_connected(self.bss_hostapd[0]) + + # AP directed roam, should connect to BSS1 as its the next best + self.validate_ap_roamed(self.bss_hostapd[0], self.bss_hostapd[1]) + + # Connected to BSS1, but the signal is bad, so IWD should try to roam + # again. BSS0 is still blacklisted, but its the only reasonable option + # since both BSS1 and BSS2 are below the set RSSI threshold (-72dB) + + from_condition = 'obj.state == DeviceState.roaming' + to_condition = 'obj.state == DeviceState.connected' + self.wd.wait_for_object_change(self.device, from_condition, to_condition) + + # IWD should have connected to BSS0, even though its roam blacklisted + self.bss_hostapd[0].wait_for_event('AP-STA-CONNECTED %s' % self.device.address) + + def test_blacklist_during_roam_scan(self): + # Tests that an AP roam request mid-roam results in the AP still being + # blacklisted even though the request itself doesn't directly trigger + # a roam. + self.rule_ssid1.signal = -7300 + self.rule_ssid2.signal = -7500 + self.rule_ssid3.signal = -8500 + + # Connect to BSS0 under the roam threshold so IWD will immediately try + # roaming elsewhere + self.validate_connected(self.bss_hostapd[0]) + + self.device.wait_for_event("roam-scan-triggered") + + self.bss_hostapd[0].send_bss_transition( + self.device.address, self.neighbor_list, disassoc_imminent=True + ) + self.device.wait_for_event("ap-roam-blacklist-added") + + # BSS0 should have gotten blacklisted even though IWD was mid-roam, + # causing IWD to choose BSS1 when it gets is results. + + from_condition = 'obj.state == DeviceState.roaming' + to_condition = 'obj.state == DeviceState.connected' + self.wd.wait_for_object_change(self.device, from_condition, to_condition) + + self.bss_hostapd[1].wait_for_event('AP-STA-CONNECTED %s' % self.device.address) + + def setUp(self): + self.wd = IWD(True) + + devices = self.wd.list_devices(1) + self.device = devices[0] + + + def tearDown(self): + self.wd = None + self.device = None + + + @classmethod + def setUpClass(cls): + IWD.copy_to_storage("main.conf.roaming", IWD_CONFIG_DIR, "main.conf") + IWD.copy_to_storage('TestAPRoam.psk') + hwsim = Hwsim() + + cls.bss_hostapd = [ HostapdCLI(config='ssid1.conf'), + HostapdCLI(config='ssid2.conf'), + HostapdCLI(config='ssid3.conf') ] + HostapdCLI.group_neighbors(*cls.bss_hostapd) + + rad0 = hwsim.get_radio('rad0') + rad1 = hwsim.get_radio('rad1') + rad2 = hwsim.get_radio('rad2') + + cls.neighbor_list = [ + (cls.bss_hostapd[0].bssid, "8f0000005101060603000000"), + (cls.bss_hostapd[1].bssid, "8f0000005102060603000000"), + (cls.bss_hostapd[2].bssid, "8f0000005103060603000000"), + ] + + + cls.rule_ssid1 = hwsim.rules.create() + cls.rule_ssid1.source = rad0.addresses[0] + cls.rule_ssid1.bidirectional = True + cls.rule_ssid1.enabled = True + + cls.rule_ssid2 = hwsim.rules.create() + cls.rule_ssid2.source = rad1.addresses[0] + cls.rule_ssid2.bidirectional = True + cls.rule_ssid2.enabled = True + + cls.rule_ssid3 = hwsim.rules.create() + cls.rule_ssid3.source = rad2.addresses[0] + cls.rule_ssid3.bidirectional = True + cls.rule_ssid3.enabled = True + + @classmethod + def tearDownClass(cls): + IWD.clear_storage() + +if __name__ == '__main__': + unittest.main(exit=True) From 1deb30fd7b988d915936a93b29cfdee110680261 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:52 -0700 Subject: [PATCH 18/19] doc: document InitialRoamRequestedTimeout --- src/iwd.config.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/iwd.config.rst b/src/iwd.config.rst index 895a1012e..9761e8fde 100644 --- a/src/iwd.config.rst +++ b/src/iwd.config.rst @@ -292,6 +292,12 @@ control how long a misbehaved BSS spends on the blacklist. The initial time that a BSS spends on the blacklist. Setting this to zero will disable blacklisting functionality in IWD. + * - InitialRoamRequestedTimeout + - Values: uint64 value in seconds (default: **30**) + + The initial time that a BSS will be marked after a BSS requests a roam. + This is to aid in avoiding roaming back to BSS's which are likely + overloaded. Setting this to zero will disabled this form of blacklisting. * - Multiplier - Values: unsigned int value greater than zero, in seconds (default: **30**) From 991acce578ffb2fddccf989d3d0ce5f5d480f05b Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Fri, 28 Mar 2025 07:42:53 -0700 Subject: [PATCH 19/19] netdev: fix invalid read after netdev_free The netdev frame watches got cleaned up upon the interface going down which works if the interface is simply being toggled but when IWD shuts down it first shuts down the interface, then immediately frees netdev. If a watched frame arrives immediately after that before the interface shutdown callback it will reference netdev, which has been freed. Fix this by clearing out the frame watches in netdev_free. ==147== Invalid read of size 8 ==147== at 0x408ADB: netdev_neighbor_report_frame_event (netdev.c:4772) ==147== by 0x467C75: frame_watch_unicast_notify (frame-xchg.c:234) ==147== by 0x4E28F8: __notifylist_notify (notifylist.c:91) ==147== by 0x4E2D37: l_notifylist_notify_matches (notifylist.c:204) ==147== by 0x4A1388: process_unicast (genl.c:844) ==147== by 0x4A1388: received_data (genl.c:972) ==147== by 0x49D82F: io_callback (io.c:105) ==147== by 0x49C93C: l_main_iterate (main.c:461) ==147== by 0x49CA0B: l_main_run (main.c:508) ==147== by 0x49CA0B: l_main_run (main.c:490) ==147== by 0x49CC3F: l_main_run_with_signal (main.c:630) ==147== by 0x4049EC: main (main.c:614) --- src/netdev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/netdev.c b/src/netdev.c index 048681f59..b81a475f4 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -1109,6 +1109,7 @@ static void netdev_free(void *data) l_timeout_remove(netdev->rssi_poll_timeout); scan_wdev_remove(netdev->wdev_id); + frame_watch_wdev_remove(netdev->wdev_id); watchlist_destroy(&netdev->station_watches);