diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..a9582eb14 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,269 @@ +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' 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: '6.2' + hostapd_version: + description: Hostapd and wpa_supplicant version + default: 'hostapd_2_11' + 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.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.19 + HOSTAPD_VERSION=09a281e52a25b5461c4b08d261f093181266a554 + 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 "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 + 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 + path: iwd-ci + + - name: Tar files + run: | + FILES="iwd ell cibase iwd-ci" + + 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@v4 + 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@v4 + 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@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@v4 + with: + name: iwd-artifacts + + - 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 + echo ${{ needs.setup.outputs.ref_branch }} + echo ${{ needs.setup.outputs.repository }} + + - name: Run CI + uses: IWDTestBot/iwd-ci@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@v4 + 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 diff --git a/autotests/testAPRoam/connection_test.py b/autotests/testAPRoam/connection_test.py index a419f4aad..9d17ca873 100644 --- a/autotests/testAPRoam/connection_test.py +++ b/autotests/testAPRoam/connection_test.py @@ -13,7 +13,7 @@ class Test(unittest.TestCase): def validate(self, expect_roam=True): - wd = IWD() + wd = IWD(True) devices = wd.list_devices(1) device = devices[0] 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..4bf07c292 --- /dev/null +++ b/autotests/testAPRoam/main.conf.roaming @@ -0,0 +1,6 @@ +[General] +RoamThreshold=-66 +OptimalSignalThreshold=-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..b9108d4e1 --- /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 = -6700 + self.rule_ssid2.signal = -7100 + 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) 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 diff --git a/src/blacklist.c b/src/blacklist.c index 21f85a754..e306195b7 100644 --- a/src/blacklist.c +++ b/src/blacklist.c @@ -45,22 +45,79 @@ 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 { 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 struct blacklist_entry *blacklist_entry_new(const uint8_t *addr, + enum blacklist_reason reason) +{ + struct blacklist_entry *entry; + uint64_t added; + uint64_t expires; + + switch (reason) { + case BLACKLIST_REASON_CONNECT_FAILED: + if (!blacklist_initial_timeout) + return NULL; + + added = l_time_now(); + expires = l_time_offset(added, blacklist_initial_timeout); + break; + case BLACKLIST_REASON_TRANSIENT_ERROR: + /* + * The temporary blacklist is a special case where entries are + * required to be removed manually. This type of blacklist is + * only used for an ongoing connection attempt to iterate BSS's + * and not retry until all have been exhausted. + */ + added = 0; + expires = 0; + break; + case BLACKLIST_REASON_ROAM_REQUESTED: + if (!blacklist_roam_initial_timeout) + return NULL; + + added = l_time_now(); + expires = l_time_offset(added, blacklist_roam_initial_timeout); + break; + default: + l_warn("Unhandled blacklist reason: %u", reason); + return NULL; + } + + entry = l_new(struct blacklist_entry, 1); + + entry->added_time = added; + entry->expire_time = expires; + entry->reason = reason; + memcpy(entry->addr, addr, 6); + + return entry; +} + 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 (entry->reason == BLACKLIST_REASON_TRANSIENT_ERROR) + return false; + + if (l_time_after(now, entry->expire_time)) { l_debug("Removing entry "MAC" on prune", MAC_STR(entry->addr)); l_free(entry); return true; @@ -79,24 +136,28 @@ static void blacklist_prune(void) static bool match_addr(const void *a, const void *b) { const struct blacklist_entry *entry = a; - const uint8_t *addr = b; + const struct blacklist_search *search = b; + + if (entry->reason != search->reason) + return false; - if (!memcmp(entry->addr, addr, 6)) + if (!memcmp(entry->addr, search->addr, 6)) return true; return false; } -void blacklist_add_bss(const uint8_t *addr) +void blacklist_add_bss(const uint8_t *addr, enum blacklist_reason reason) { struct blacklist_entry *entry; - - if (!blacklist_initial_timeout) - return; + 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, &search); if (entry) { uint64_t offset = l_time_diff(entry->added_time, @@ -112,43 +173,34 @@ void blacklist_add_bss(const uint8_t *addr) return; } - 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); - memcpy(entry->addr, addr, 6); - - l_queue_push_tail(blacklist, entry); + entry = blacklist_entry_new(addr, reason); + if (entry) + 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); - - 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, &search) != NULL; } -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, &search); if (!entry) return; @@ -167,6 +219,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 56260e207..8de5c1d7e 100644 --- a/src/blacklist.h +++ b/src/blacklist.h @@ -20,6 +20,33 @@ * */ -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, + /* + * Used to blacklist a BSS under certain failure conditions that don't + * warrant a full ban from connecting. This can include an invalid + * password, or an auth/assoc failure with a subset of status codes that + * indicate the BSS is overloaded or cannot accept new connections. + * + * This is used to mark the last BSS as having failed, and to continue + * iterating BSS's. Once the list has been exhausted or a connection has + * succeeded all blacklist entries with this reason code should be + * cleared. + */ + BLACKLIST_REASON_TRANSIENT_ERROR, + /* + * 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); +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/iwd.config.rst b/src/iwd.config.rst index 895a1012e..a2fea8b47 100644 --- a/src/iwd.config.rst +++ b/src/iwd.config.rst @@ -149,6 +149,21 @@ The group ``[General]`` contains general settings. This has the same effect as ``CriticalRoamThreshold``, but for the 5GHz band. + * - OptimalSignalThreshold + - Value: rssi dBm value, from -100 to -1, default: **0** (disabled) + + When set to a non-zero value, this enables BSS grouping which is a + sorts BSS's first based on this optimal signal threshold, then by rank. + In addition the BSS's blacklisting status is also taken into account. + IWD will order its connection preference first based on the following + groupings, then followed by rank: + + 1. "Optimal" is the most preferred group. The BSS is both above the + **OptimalSignalThreshold** and not roam blacklisted + 2. "Above Threshold" means the BSS is roam blacklisted, but still above + the **OptimalSignalThreshold** + 3. "Below Threshold" means the BSS is below **OptimalSignalThreshold** + * - RoamRetryInterval - Value: unsigned int value in seconds (default: **60**) @@ -292,6 +307,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**) diff --git a/src/network.c b/src/network.c index 0a40a6c55..be6641c05 100644 --- a/src/network.c +++ b/src/network.c @@ -78,7 +78,6 @@ struct network { struct l_queue *bss_list; struct l_settings *settings; struct l_queue *secrets; - struct l_queue *blacklist; /* temporary blacklist for BSS's */ uint8_t hessid[6]; char **nai_realms; uint8_t *rc_ie; @@ -168,6 +167,18 @@ static bool network_secret_check_cacheable(void *data, void *user_data) return false; } +static void remove_temporary_blacklist(void *user_data) +{ + struct scan_bss *bss = user_data; + + blacklist_remove_bss(bss->addr, BLACKLIST_REASON_TRANSIENT_ERROR); +} + +static void remove_blacklist_foreach(void *data, void *user_data) +{ + remove_temporary_blacklist(data); +} + void network_connected(struct network *network) { enum security security = network_get_security(network); @@ -198,7 +209,7 @@ void network_connected(struct network *network) l_queue_foreach_remove(network->secrets, network_secret_check_cacheable, network); - l_queue_clear(network->blacklist, NULL); + l_queue_foreach(network->bss_list, remove_blacklist_foreach, NULL); network->provisioning_hidden = false; } @@ -207,7 +218,7 @@ void network_disconnected(struct network *network) { network_settings_close(network); - l_queue_clear(network->blacklist, NULL); + l_queue_foreach(network->bss_list, remove_blacklist_foreach, NULL); if (network->provisioning_hidden) station_hide_network(network->station, network); @@ -254,7 +265,6 @@ struct network *network_create(struct station *station, const char *ssid, } network->bss_list = l_queue_new(); - network->blacklist = l_queue_new(); return network; } @@ -1197,11 +1207,6 @@ struct scan_bss *network_bss_find_by_addr(struct network *network, return l_queue_find(network->bss_list, match_addr, addr); } -static bool match_bss(const void *a, const void *b) -{ - return a == b; -} - struct erp_cache_entry *network_get_erp_cache(struct network *network) { struct erp_cache_entry *cache; @@ -1277,10 +1282,12 @@ struct scan_bss *network_bss_select(struct network *network, candidate = bss; /* check if temporarily blacklisted */ - if (l_queue_find(network->blacklist, match_bss, bss)) + if (blacklist_contains_bss(bss->addr, + BLACKLIST_REASON_TRANSIENT_ERROR)) continue; - if (blacklist_contains_bss(bss->addr)) + if (blacklist_contains_bss(bss->addr, + BLACKLIST_REASON_CONNECT_FAILED)) continue; /* OWE Transition BSS */ @@ -1783,11 +1790,6 @@ struct l_dbus_message *network_connect_new_hidden_network( return dbus_error_not_supported(message); } -void network_blacklist_add(struct network *network, struct scan_bss *bss) -{ - l_queue_push_head(network->blacklist, bss); -} - static bool network_property_get_name(struct l_dbus *dbus, struct l_dbus_message *message, struct l_dbus_message_builder *builder, @@ -1933,8 +1935,7 @@ void network_remove(struct network *network, int reason) if (network->info) network->info->seen_count -= 1; - l_queue_destroy(network->bss_list, NULL); - l_queue_destroy(network->blacklist, NULL); + l_queue_destroy(network->bss_list, remove_temporary_blacklist); if (network->nai_realms) l_strv_free(network->nai_realms); diff --git a/src/network.h b/src/network.h index 849051dd1..1e01de88e 100644 --- a/src/network.h +++ b/src/network.h @@ -95,8 +95,6 @@ struct l_dbus_message *network_connect_new_hidden_network( struct network *network, struct l_dbus_message *message); -void network_blacklist_add(struct network *network, struct scan_bss *bss); - struct erp_cache_entry *network_get_erp_cache(struct network *network); const struct l_queue_entry *network_bss_list_get_entries( diff --git a/src/scan.c b/src/scan.c index aeab65168..6c03e4086 100644 --- a/src/scan.c +++ b/src/scan.c @@ -51,6 +51,7 @@ #include "src/mpdu.h" #include "src/band.h" #include "src/scan.h" +#include "src/blacklist.h" /* User configurable options */ static double RANK_2G_FACTOR; @@ -58,6 +59,7 @@ static double RANK_5G_FACTOR; static double RANK_6G_FACTOR; static uint32_t RANK_HIGH_UTILIZATION; static uint32_t RANK_HIGH_STATION_COUNT; +static int RANK_OPTIMAL_SIGNAL_THRESHOLD; static uint32_t SCAN_MAX_INTERVAL; static uint32_t SCAN_INIT_INTERVAL; @@ -1910,15 +1912,41 @@ int scan_bss_get_security(const struct scan_bss *bss, enum security *security) return 0; } +/* + * Evaluate the BSS's grouping based on its signal strength and blacklist + * status. From best to worst the groupings are: + * + * Optimal: Not blacklisted in any form, and above the RSSI threshold + * Above Threshold: Above the RSSI threshold (may be roam blacklisted) + * Below Threshold: Below the RSSI threshold (may be roam blacklisted) + * Blacklisted: Permanently blacklisted + */ +enum scan_bss_group scan_bss_evaluate_group(const uint8_t *addr, + int16_t signal_strength) +{ + int rssi = signal_strength / 100; + + if (RANK_OPTIMAL_SIGNAL_THRESHOLD == 0) + return SCAN_BSS_GROUP_OPTIMAL; + + if (blacklist_contains_bss(addr, BLACKLIST_REASON_CONNECT_FAILED)) + return SCAN_BSS_GROUP_BLACKLISTED; + + if (!blacklist_contains_bss(addr, BLACKLIST_REASON_ROAM_REQUESTED) && + rssi >= RANK_OPTIMAL_SIGNAL_THRESHOLD) + return SCAN_BSS_GROUP_OPTIMAL; + + if (rssi >= RANK_OPTIMAL_SIGNAL_THRESHOLD) + return SCAN_BSS_GROUP_ABOVE_THRESHOLD; + + return SCAN_BSS_GROUP_UNDER_THRESHOLD; +} + int scan_bss_rank_compare(const void *a, const void *b, void *user_data) { const struct scan_bss *new_bss = a, *bss = b; - if (bss->rank == new_bss->rank) - return (bss->signal_strength > - new_bss->signal_strength) ? 1 : -1; - - return (bss->rank > new_bss->rank) ? 1 : -1; + return __scan_bss_rank_compare(new_bss, bss); } static bool scan_survey_get_snr(struct scan_results *results, @@ -2678,6 +2706,10 @@ static int scan_init(void) if (L_WARN_ON(RANK_HIGH_STATION_COUNT > 255)) RANK_HIGH_STATION_COUNT = 255; + if (!l_settings_get_int(config, "General", "OptimalSignalThreshold", + &RANK_OPTIMAL_SIGNAL_THRESHOLD)) + RANK_OPTIMAL_SIGNAL_THRESHOLD = 0; + return 0; } diff --git a/src/scan.h b/src/scan.h index 4c1ebc21d..4d06f29d0 100644 --- a/src/scan.h +++ b/src/scan.h @@ -45,6 +45,17 @@ enum scan_bss_frame_type { SCAN_BSS_BEACON, }; +/* + * Groupings for BSS's. These are assumed to be in order of preference where + * the last enum is the most preferred group to connect to. + */ +enum scan_bss_group { + SCAN_BSS_GROUP_BLACKLISTED, + SCAN_BSS_GROUP_UNDER_THRESHOLD, + SCAN_BSS_GROUP_ABOVE_THRESHOLD, + SCAN_BSS_GROUP_OPTIMAL, +}; + struct scan_bss { uint8_t addr[6]; uint32_t frequency; @@ -168,6 +179,36 @@ bool scan_get_firmware_scan(uint64_t wdev_id, scan_notify_func_t notify, void *userdata, scan_destroy_func_t destroy); void scan_bss_free(struct scan_bss *bss); + +enum scan_bss_group scan_bss_evaluate_group(const uint8_t *addr, + int16_t signal_strength); + +/* + * Macro to compare two scan_bss-like objects. This is to share code between + * station.c and scan.c since station uses "roam_bss" objects which is a subset + * of a scan_bss. + * - If the groups differ this is used as the comparison. + * - If the groups match, the BSS rank is used as the comparison + * - If the BSS ranks match, the signal strength is used as the comparison + */ +#define __scan_bss_rank_compare(a, b) ({ \ + int ret; \ + enum scan_bss_group a_group = scan_bss_evaluate_group( \ + (a)->addr, (a)->signal_strength); \ + enum scan_bss_group b_group = scan_bss_evaluate_group( \ + (b)->addr, (b)->signal_strength); \ + if (b_group > a_group) \ + ret = 1; \ + else if (b_group < a_group) \ + ret = -1; \ + else if ((b)->rank == (a)->rank) \ + ret = ((b)->signal_strength > \ + (a)->signal_strength) ? 1 : -1; \ + else \ + ret = ((b)->rank > (a)->rank) ? 1 : -1; \ + ret; \ +}) + int scan_bss_rank_compare(const void *a, const void *b, void *user); int scan_bss_get_rsn_info(const struct scan_bss *bss, struct ie_rsn_info *info); diff --git a/src/station.c b/src/station.c index 5403c3320..092d0b4d6 100644 --- a/src/station.c +++ b/src/station.c @@ -184,11 +184,7 @@ static int roam_bss_rank_compare(const void *a, const void *b, void *user_data) { const struct roam_bss *new_bss = a, *bss = b; - if (bss->rank == new_bss->rank) - return (bss->signal_strength > - new_bss->signal_strength) ? 1 : -1; - - return (bss->rank > new_bss->rank) ? 1 : -1; + return __scan_bss_rank_compare(new_bss, bss); } struct wiphy *station_get_wiphy(struct station *station) @@ -2805,6 +2801,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; + enum scan_bss_group cur_bss_group = SCAN_BSS_GROUP_BLACKLISTED; double cur_bss_rank = 0.0; static const double RANK_FT_FACTOR = 1.3; uint16_t mdid; @@ -2835,6 +2832,9 @@ 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; + cur_bss_group = scan_bss_evaluate_group( + current_bss->addr, + current_bss->signal_strength); if (hs->mde && bss->mde_present && l_get_le16(bss->mde) == mdid) cur_bss_rank *= RANK_FT_FACTOR; @@ -2859,6 +2859,9 @@ 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; + enum scan_bss_group group = scan_bss_evaluate_group( + bss->addr, + bss->signal_strength); station_print_scan_bss(bss); @@ -2880,7 +2883,7 @@ 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 (group == SCAN_BSS_GROUP_BLACKLISTED) goto next; rank = bss->rank; @@ -2888,7 +2891,15 @@ 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) + /* + * First check the group: + * - If worse, disregard BSS candidate + * - If better, keep BSS candidate + * - If equal, compare based on rank + */ + if (group < cur_bss_group) + goto next; + else if (group == cur_bss_group && rank <= cur_bss_rank) goto next; /* @@ -3165,12 +3176,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; @@ -3234,8 +3243,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; @@ -3262,6 +3276,19 @@ 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); @@ -3400,7 +3427,15 @@ 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 + * be disabled + */ + blacklist_add_bss(station->connected_bss->addr, + BLACKLIST_REASON_TRANSIENT_ERROR); try_next: return station_try_next_bss(station); @@ -3449,6 +3484,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 +3498,19 @@ 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)) - blacklist_add_bss(station->connected_bss->addr); + if (!IS_TEMPORARY_STATUS(status_code)) + blacklist_add_bss(station->connected_bss->addr, + BLACKLIST_REASON_CONNECT_FAILED); + /* + * 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 + */ + blacklist_add_bss(station->connected_bss->addr, + BLACKLIST_REASON_TRANSIENT_ERROR); + +try_next: iwd_notice(IWD_NOTICE_CONNECT_FAILED, "status: %u", status_code); return station_try_next_bss(station); @@ -3549,7 +3595,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: @@ -3558,8 +3605,8 @@ static void station_connect_cb(struct netdev *netdev, enum netdev_result result, iwd_notice(IWD_NOTICE_DISCONNECT_INFO, "reason: %u", reason); /* Disconnected while connecting */ - network_blacklist_add(station->connected_network, - station->connected_bss); + blacklist_add_bss(station->connected_bss->addr, + BLACKLIST_REASON_TRANSIENT_ERROR); if (station_try_next_bss(station)) return;