From ff80c0b926e42409ba3a69e7c0aa375ccc286803 Mon Sep 17 00:00:00 2001 From: Rubin Gerritsen Date: Mon, 13 May 2024 11:51:41 +0200 Subject: [PATCH] Bluetooth: Host: Fix connection establishment upon RPA timeout Before this commit, the following bugs were present: - When `CONFIG_BT_FILTER_ACCEPT_LIST` was set, connection establishment was cancelled upon RPA timeout. This required the application to restart the initiator every RPA timeout. - When `CONFIG_BT_FILTER_ACCEPT_LIST` was not set, the RPA was not updated while the initiator was running. This commit unifies the RPA timeout handling for both these cases. Upon RPA timeout the initiator is cancelled and restarted when the controller raises the LE Connection Complete event. The workqueue state is checked when restarting the initiator to prevent it being restarted when the timeout is hit. Corresponding test cases have been added to ensure that this feature works. Signed-off-by: Rubin Gerritsen --- subsys/bluetooth/host/hci_core.c | 36 +++++----- subsys/bluetooth/host/id.c | 1 - .../host/privacy/central/CMakeLists.txt | 6 ++ .../bluetooth/host/privacy/central/prj.conf | 5 ++ .../bluetooth/host/privacy/central/src/dut.c | 70 +++++++++++++++++++ .../bluetooth/host/privacy/central/src/main.c | 30 ++++++++ .../host/privacy/central/src/tester.c | 60 ++++++++++++++++ .../run_test_short_conn_timeout.sh | 26 +++++++ .../run_test_short_rpa_timeout.sh | 31 ++++++++ 9 files changed, 248 insertions(+), 17 deletions(-) create mode 100755 tests/bsim/bluetooth/host/privacy/central/test_scripts/run_test_short_conn_timeout.sh create mode 100755 tests/bsim/bluetooth/host/privacy/central/test_scripts/run_test_short_rpa_timeout.sh diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 3f0a32dd094..9a87a47f6db 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -1159,6 +1159,7 @@ static void conn_auto_initiate(struct bt_conn *conn) static void le_conn_complete_cancel(uint8_t err) { + int ret; struct bt_conn *conn; /* Handle create connection cancel. @@ -1172,27 +1173,30 @@ static void le_conn_complete_cancel(uint8_t err) return; } - conn->err = err; - - /* Handle cancellation of outgoing connection attempt. */ - if (!IS_ENABLED(CONFIG_BT_FILTER_ACCEPT_LIST)) { - /* We notify before checking autoconnect flag - * as application may choose to change it from - * callback. - */ - bt_conn_set_state(conn, BT_CONN_DISCONNECTED); - /* Check if device is marked for autoconnect. */ - if (atomic_test_bit(conn->flags, BT_CONN_AUTO_CONNECT)) { + if (atomic_test_bit(conn->flags, BT_CONN_AUTO_CONNECT)) { + if (!IS_ENABLED(CONFIG_BT_FILTER_ACCEPT_LIST)) { /* Restart passive scanner for device */ bt_conn_set_state(conn, BT_CONN_SCAN_BEFORE_INITIATING); + } else { + /* Restart FAL initiator after RPA timeout. */ + ret = bt_le_create_conn(conn); + if (ret) { + LOG_ERR("Failed to restart initiator"); + } } } else { - if (atomic_test_bit(conn->flags, BT_CONN_AUTO_CONNECT)) { - /* Restart FAL initiator after RPA timeout. */ - bt_le_create_conn(conn); - } else { - /* Create connection canceled by timeout */ + int busy_status = k_work_delayable_busy_get(&conn->deferred_work); + + if (!(busy_status & (K_WORK_QUEUED | K_WORK_DELAYED))) { + /* Connection initiation timeout triggered. */ + conn->err = err; bt_conn_set_state(conn, BT_CONN_DISCONNECTED); + } else { + /* Restart initiator after RPA timeout. */ + ret = bt_le_create_conn(conn); + if (ret) { + LOG_ERR("Failed to restart initiator"); + } } } diff --git a/subsys/bluetooth/host/id.c b/subsys/bluetooth/host/id.c index d6f23f2db64..38263e1cb2a 100644 --- a/subsys/bluetooth/host/id.c +++ b/subsys/bluetooth/host/id.c @@ -606,7 +606,6 @@ static void le_update_private_addr(void) } #endif if (IS_ENABLED(CONFIG_BT_CENTRAL) && - IS_ENABLED(CONFIG_BT_FILTER_ACCEPT_LIST) && atomic_test_bit(bt_dev.flags, BT_DEV_INITIATING)) { /* Canceled initiating procedure will be restarted by * connection complete event. diff --git a/tests/bsim/bluetooth/host/privacy/central/CMakeLists.txt b/tests/bsim/bluetooth/host/privacy/central/CMakeLists.txt index 75e1deb8f82..7ec3436c8f8 100644 --- a/tests/bsim/bluetooth/host/privacy/central/CMakeLists.txt +++ b/tests/bsim/bluetooth/host/privacy/central/CMakeLists.txt @@ -5,6 +5,12 @@ cmake_minimum_required(VERSION 3.20.0) find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) project(bsim_test_rpa_central) +add_subdirectory(${ZEPHYR_BASE}/tests/bluetooth/common/testlib testlib) +target_link_libraries(app PRIVATE testlib) + +add_subdirectory(${ZEPHYR_BASE}/tests/bsim/babblekit babblekit) +target_link_libraries(app PRIVATE babblekit) + target_sources(app PRIVATE src/bs_bt_utils.c src/tester.c diff --git a/tests/bsim/bluetooth/host/privacy/central/prj.conf b/tests/bsim/bluetooth/host/privacy/central/prj.conf index e3fa9de43d7..a4eea958b17 100644 --- a/tests/bsim/bluetooth/host/privacy/central/prj.conf +++ b/tests/bsim/bluetooth/host/privacy/central/prj.conf @@ -11,3 +11,8 @@ CONFIG_BT_RPA_TIMEOUT=10 CONFIG_BT_CTLR_ADV_SET=2 CONFIG_BT_CTLR_SCAN_REQ_NOTIFY=y CONFIG_BT_SCAN_WITH_IDENTITY=y + +# Enable the possibility to set the RPA such that the RPA +# refreshes before the connection attempt has completed +CONFIG_BT_RPA_TIMEOUT_DYNAMIC=y +CONFIG_BT_CREATE_CONN_TIMEOUT=10 diff --git a/tests/bsim/bluetooth/host/privacy/central/src/dut.c b/tests/bsim/bluetooth/host/privacy/central/src/dut.c index e80b9bc8f93..5a49b6e5097 100644 --- a/tests/bsim/bluetooth/host/privacy/central/src/dut.c +++ b/tests/bsim/bluetooth/host/privacy/central/src/dut.c @@ -12,6 +12,10 @@ #include #include +#include +#include +#include + void start_scanning(void) { int err; @@ -46,3 +50,69 @@ void dut_procedure(void) PASS("PASS\n"); } + +void dut_procedure_connect_short_rpa_timeout(void) +{ + backchannel_init(1); + + const uint16_t rpa_timeout_s = 1; + + int err; + bt_addr_le_t peer = {}; + struct bt_conn *conn = NULL; + + /* Enable bluetooth */ + err = bt_enable(NULL); + TEST_ASSERT(!err, "Failed to enable bluetooth (err %d)"); + + /* Central to use a short RPA timeout */ + err = bt_le_set_rpa_timeout(rpa_timeout_s); + TEST_ASSERT(!err, "Failed to set RPA timeout (err %d)", err); + + err = bt_testlib_scan_find_name(&peer, CONFIG_BT_DEVICE_NAME); + TEST_ASSERT(!err, "Failed to start scan (err %d)", err); + + /* Indicate to the peer device that we have found the advertiser. */ + backchannel_sync_send(); + + /* Create a connection using that address */ + err = bt_testlib_connect(&peer, &conn); + TEST_ASSERT(!err, "Failed to initiate connection (err %d)", err); + + PASS("PASS\n"); +} + +void dut_procedure_connect_timeout(void) +{ + const uint16_t rpa_timeout_s = 1; + + int err; + bt_addr_le_t peer = {.type = BT_ADDR_LE_RANDOM, .a = {.val = {1, 2, 3, 4, 5, 6}}}; + struct bt_conn *conn = NULL; + + /* Enable bluetooth */ + err = bt_enable(NULL); + TEST_ASSERT(!err, "Failed to enable bluetooth (err %d)", err); + + /* Central to use a short RPA timeout */ + err = bt_le_set_rpa_timeout(rpa_timeout_s); + TEST_ASSERT(!err, "Failed to set RPA timeout (err %d)", err); + + int64_t old_time = k_uptime_get(); + + /* Create a connection using that address */ + err = bt_testlib_connect(&peer, &conn); + TEST_ASSERT(err == BT_HCI_ERR_UNKNOWN_CONN_ID, + "Expected connection establishment to time out (err %d)", err); + + int64_t new_time = k_uptime_get(); + int64_t time_diff_ms = new_time - old_time; + int64_t expected_conn_timeout_ms = CONFIG_BT_CREATE_CONN_TIMEOUT * 1000; + int64_t diff_to_expected_ms = abs(time_diff_ms - expected_conn_timeout_ms); + + printk("Connection creation timed out after %d ms\n", time_diff_ms); + TEST_ASSERT(diff_to_expected_ms < 0.1 * expected_conn_timeout_ms, + "Connection timeout not within 10 \%% of expected timeout"); + + PASS("PASS\n"); +} diff --git a/tests/bsim/bluetooth/host/privacy/central/src/main.c b/tests/bsim/bluetooth/host/privacy/central/src/main.c index cb5f12f9d0f..a2f48b09ec8 100644 --- a/tests/bsim/bluetooth/host/privacy/central/src/main.c +++ b/tests/bsim/bluetooth/host/privacy/central/src/main.c @@ -8,21 +8,51 @@ #include "bstests.h" void tester_procedure(void); +void tester_procedure_periph_delayed_start_of_conn_adv(void); void dut_procedure(void); +void dut_procedure_connect_short_rpa_timeout(void); +void dut_procedure_connect_timeout(void); static const struct bst_test_instance test_to_add[] = { { .test_id = "central", + .test_descr = "Central performs active scanning using RPA", .test_post_init_f = test_init, .test_tick_f = test_tick, .test_main_f = dut_procedure, }, + { + .test_id = "central_connect_short_rpa_timeout", + .test_descr = "Central connects to a peripheral using a short RPA timeout", + .test_post_init_f = test_init, + .test_tick_f = test_tick, + .test_main_f = dut_procedure_connect_short_rpa_timeout, + }, + { + .test_id = "central_connect_fails_with_short_rpa_timeout", + .test_descr = "Central connects to a peripheral using a short RPA timeout" + " but expects connection establishment to time out.", + .test_post_init_f = test_init, + .test_tick_f = test_tick, + .test_main_f = dut_procedure_connect_timeout, + }, { .test_id = "peripheral", + .test_descr = "Performs scannable advertising, validates that the scanner" + " RPA address refreshes", .test_post_init_f = test_init, .test_tick_f = test_tick, .test_main_f = tester_procedure, }, + { + .test_id = "periph_delayed_start_of_conn_adv", + .test_descr = "Performs connectable advertising. " + "The advertiser is stopped for 10 seconds when instructed by the DUT" + " to allow it to run the initiator for longer than its RPA timeout.", + .test_post_init_f = test_init, + .test_tick_f = test_tick, + .test_main_f = tester_procedure_periph_delayed_start_of_conn_adv, + }, BSTEST_END_MARKER, }; diff --git a/tests/bsim/bluetooth/host/privacy/central/src/tester.c b/tests/bsim/bluetooth/host/privacy/central/src/tester.c index 1bbb8fc8de6..b1c6e9fd1bc 100644 --- a/tests/bsim/bluetooth/host/privacy/central/src/tester.c +++ b/tests/bsim/bluetooth/host/privacy/central/src/tester.c @@ -14,7 +14,10 @@ #include #include +#include + DEFINE_FLAG(flag_new_address); +DEFINE_FLAG(flag_connected); void scanned_cb(struct bt_le_ext_adv *adv, struct bt_le_ext_adv_scanned_info *info) { @@ -64,8 +67,14 @@ void scanned_cb(struct bt_le_ext_adv *adv, struct bt_le_ext_adv_scanned_info *in old_addr = new_addr; } +static void connected_cb(struct bt_le_ext_adv *adv, struct bt_le_ext_adv_connected_info *info) +{ + SET_FLAG(flag_connected); +} + static struct bt_le_ext_adv_cb adv_callbacks = { .scanned = scanned_cb, + .connected = connected_cb }; void start_advertising(void) @@ -125,3 +134,54 @@ void tester_procedure(void) PASS("PASS\n"); } + +void tester_procedure_periph_delayed_start_of_conn_adv(void) +{ + backchannel_init(0); + + int err; + struct bt_le_adv_param params = + BT_LE_ADV_PARAM_INIT(BT_LE_ADV_OPT_CONNECTABLE | BT_LE_ADV_OPT_USE_IDENTITY, + BT_GAP_ADV_FAST_INT_MIN_2, + BT_GAP_ADV_FAST_INT_MAX_2, NULL); + struct bt_data ad; + struct bt_le_ext_adv *adv; + + /* Enable bluetooth */ + err = bt_enable(NULL); + TEST_ASSERT(!err, "Failed to enable bluetooth (err %d)"); + + /* Advertiser to use a long RPA timeout */ + err = bt_le_set_rpa_timeout(100); + TEST_ASSERT(!err, "Failed to set RPA timeout (err %d)", err); + + err = bt_le_ext_adv_create(¶ms, &adv_callbacks, &adv); + TEST_ASSERT(!err, "Failed to create advertising set (err %d)", err); + + ad.type = BT_DATA_NAME_COMPLETE; + ad.data_len = strlen(CONFIG_BT_DEVICE_NAME); + ad.data = (const uint8_t *)CONFIG_BT_DEVICE_NAME; + + err = bt_le_ext_adv_set_data(adv, &ad, 1, NULL, 0); + TEST_ASSERT(!err, "Failed to set advertising data (err %d)", err); + + err = bt_le_ext_adv_start(adv, BT_LE_EXT_ADV_START_DEFAULT); + TEST_ASSERT(!err, "Failed to start advertiser (err %d)", err); + + backchannel_sync_wait(); + + err = bt_le_ext_adv_stop(adv); + TEST_ASSERT(!err, "Failed to stop advertiser (err %d)", err); + + /* Wait a few RPA cycles before restaring the advertiser to force RPA timeout + * on the DUT. + */ + k_sleep(K_SECONDS(7)); + + err = bt_le_ext_adv_start(adv, BT_LE_EXT_ADV_START_DEFAULT); + TEST_ASSERT(!err, "Failed to restart advertiser (err %d)", err); + + WAIT_FOR_FLAG(flag_connected); + + PASS("PASS\n"); +} diff --git a/tests/bsim/bluetooth/host/privacy/central/test_scripts/run_test_short_conn_timeout.sh b/tests/bsim/bluetooth/host/privacy/central/test_scripts/run_test_short_conn_timeout.sh new file mode 100755 index 00000000000..5e15e785178 --- /dev/null +++ b/tests/bsim/bluetooth/host/privacy/central/test_scripts/run_test_short_conn_timeout.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# Copyright 2024 Nordic Semiconductor ASA +# SPDX-License-Identifier: Apache-2.0 +set -eu + +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +EXECUTE_TIMEOUT=100 +verbosity_level=2 + +# Test that connection establishment times out when RPA +# timeout is shorter than the connection establishment timeout +simulation_id="test_central_connect_fails_with_short_rpa_timeout" + +central_exe="${BSIM_OUT_PATH}/bin/bs_${BOARD_TS}_$(guess_test_long_name)_prj_conf" + +cd ${BSIM_OUT_PATH}/bin + +Execute "$central_exe" \ + -v=${verbosity_level} -s=${simulation_id} -d=0 \ + -testid=central_connect_fails_with_short_rpa_timeout -RealEncryption=1 + +Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s=${simulation_id} \ + -D=1 -sim_length=60e6 $@ + +wait_for_background_jobs diff --git a/tests/bsim/bluetooth/host/privacy/central/test_scripts/run_test_short_rpa_timeout.sh b/tests/bsim/bluetooth/host/privacy/central/test_scripts/run_test_short_rpa_timeout.sh new file mode 100755 index 00000000000..0ed76b2f86a --- /dev/null +++ b/tests/bsim/bluetooth/host/privacy/central/test_scripts/run_test_short_rpa_timeout.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# Copyright 2024 Nordic Semiconductor ASA +# SPDX-License-Identifier: Apache-2.0 +set -eu + +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +EXECUTE_TIMEOUT=100 +verbosity_level=2 + +# Test connection establishment when the RPA times out before +# we expect the connection to be established +simulation_id="test_central_connect_short_rpa_timeout" + +central_exe="${BSIM_OUT_PATH}/bin/bs_${BOARD_TS}_$(guess_test_long_name)_prj_conf" +peripheral_exe="${central_exe}" + +cd ${BSIM_OUT_PATH}/bin + +Execute "$central_exe" \ + -v=${verbosity_level} -s=${simulation_id} -d=0 \ + -testid=central_connect_short_rpa_timeout -RealEncryption=1 + +Execute "$peripheral_exe" \ + -v=${verbosity_level} -s=${simulation_id} -d=1 \ + -testid=periph_delayed_start_of_conn_adv -RealEncryption=1 + +Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s=${simulation_id} \ + -D=2 -sim_length=60e6 $@ + +wait_for_background_jobs