Skip to content

Commit

Permalink
bluetooth: id: fix adv sets with same id use different RPA
Browse files Browse the repository at this point in the history
The fix is to check if any of the adv set's rpa expired
callback returns false, then all adv sets continues with
the old RPA.

Note: Fix is applicable only to adv sets which shares the
same id.

Signed-off-by: Nithin Ramesh Myliattil <[email protected]>
  • Loading branch information
niym-ot committed Mar 5, 2024
1 parent 3203bd6 commit 6a52b14
Show file tree
Hide file tree
Showing 13 changed files with 489 additions and 44 deletions.
55 changes: 45 additions & 10 deletions subsys/bluetooth/host/id.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,23 +198,31 @@ int bt_id_set_adv_random_addr(struct bt_le_ext_adv *adv,
return 0;
}

static void adv_rpa_expired(struct bt_le_ext_adv *adv)
static void adv_rpa_expired(struct bt_le_ext_adv *adv, void *data)
{
bool rpa_invalid = true;

#if defined(CONFIG_BT_EXT_ADV) && defined(CONFIG_BT_PRIVACY)
/* Notify the user about the RPA timeout and set the RPA validity. */
if (atomic_test_bit(adv->flags, BT_ADV_RPA_VALID) &&
adv->cb && adv->cb->rpa_expired) {
rpa_invalid = adv->cb->rpa_expired(adv);
}
#endif
if (rpa_invalid) {
atomic_clear_bit(adv->flags, BT_ADV_RPA_VALID);

#if defined(CONFIG_BT_RPA_SHARING)
bt_addr_copy(&bt_dev.rpa[adv->id], BT_ADDR_NONE);
#endif
if (IS_ENABLED(CONFIG_BT_RPA_SHARING)) {

if (adv->id >= bt_dev.id_count) {
return;
}
bool *rpa_invalid_set_ptr = data;

if (!rpa_invalid) {
rpa_invalid_set_ptr[adv->id] = false;
}
} else {
if (rpa_invalid) {
atomic_clear_bit(adv->flags, BT_ADV_RPA_VALID);
}
}
}

Expand All @@ -225,9 +233,26 @@ static void adv_rpa_invalidate(struct bt_le_ext_adv *adv, void *data)
*/
if (!atomic_test_bit(adv->flags, BT_ADV_LIMITED) &&
!atomic_test_bit(adv->flags, BT_ADV_USE_IDENTITY)) {
adv_rpa_expired(adv);
adv_rpa_expired(adv, data);
}
}

#if defined(CONFIG_BT_RPA_SHARING)
static void adv_rpa_clear_data(struct bt_le_ext_adv *adv, void *data)
{
if (adv->id >= bt_dev.id_count) {
return;
}
bool *rpa_invalid_set_ptr = data;

if (rpa_invalid_set_ptr[adv->id]) {
atomic_clear_bit(adv->flags, BT_ADV_RPA_VALID);
bt_addr_copy(&bt_dev.rpa[adv->id], BT_ADDR_NONE);
} else {
LOG_WRN("Adv sets rpa expired cb with id %d returns false\n", adv->id);
}
}
#endif

static void le_rpa_invalidate(void)
{
Expand All @@ -238,7 +263,17 @@ static void le_rpa_invalidate(void)
}

if (IS_ENABLED(CONFIG_BT_BROADCASTER)) {
bt_le_ext_adv_foreach(adv_rpa_invalidate, NULL);

if (bt_dev.id_count == 0) {
return;
}
bool rpa_expired_data[bt_dev.id_count];

bt_le_ext_adv_foreach(adv_rpa_invalidate, &rpa_expired_data);
#if defined(CONFIG_BT_RPA_SHARING)
/* rpa_expired data collected. now clear data based on data collected. */
bt_le_ext_adv_foreach(adv_rpa_clear_data, &rpa_expired_data);
#endif
}
}

Expand Down Expand Up @@ -769,7 +804,7 @@ bool bt_id_adv_random_addr_check(const struct bt_le_adv_param *param)

void bt_id_adv_limited_stopped(struct bt_le_ext_adv *adv)
{
adv_rpa_expired(adv);
adv_rpa_expired(adv, NULL);
}

#if defined(CONFIG_BT_SMP)
Expand Down
1 change: 1 addition & 0 deletions tests/bsim/bluetooth/host/compile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ app=tests/bsim/bluetooth/host/misc/unregister_conn_cb compile

app=tests/bsim/bluetooth/host/privacy/central compile
app=tests/bsim/bluetooth/host/privacy/peripheral compile
app=tests/bsim/bluetooth/host/privacy/peripheral conf_file=prj_rpa_expired.conf compile
app=tests/bsim/bluetooth/host/privacy/peripheral conf_file=prj_rpa_sharing.conf compile
app=tests/bsim/bluetooth/host/privacy/device compile
app=tests/bsim/bluetooth/host/privacy/legacy compile
Expand Down
8 changes: 6 additions & 2 deletions tests/bsim/bluetooth/host/privacy/peripheral/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ project(bsim_test_rpa_peripheral)

target_sources(app PRIVATE
src/bs_bt_utils.c
src/tester.c
src/tester_rpa_rotation.c
src/tester_rpa_expired.c
src/main.c
src/dut.c
src/main_rpa_rotation.c
src/main_rpa_expired.c
src/dut_rpa_rotation.c
src/dut_rpa_expired.c
)

zephyr_include_directories(
Expand Down
25 changes: 25 additions & 0 deletions tests/bsim/bluetooth/host/privacy/peripheral/prj_rpa_expired.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
CONFIG_BT=y
CONFIG_BT_PERIPHERAL=y
CONFIG_BT_CENTRAL=y
CONFIG_BT_SMP=y
CONFIG_ASSERT=y

CONFIG_BT_EXT_ADV=y
CONFIG_BT_PRIVACY=y
CONFIG_BT_RPA_TIMEOUT=10
CONFIG_BT_EXT_ADV_MAX_ADV_SET=4
CONFIG_BT_CTLR_ADVANCED_FEATURES=y
CONFIG_BT_CTLR_ADV_DATA_BUF_MAX=4
CONFIG_BT_ID_MAX=3

CONFIG_FLASH=y
CONFIG_FLASH_PAGE_LAYOUT=y
CONFIG_FLASH_MAP=y
CONFIG_NVS=y
CONFIG_SETTINGS=y
CONFIG_BT_SETTINGS=y
# Increased stack due to settings API usage
CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=2048

# Enable the RPA sharing mode
CONFIG_BT_RPA_SHARING=y
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ CONFIG_FLASH_MAP=y
CONFIG_NVS=y
CONFIG_SETTINGS=y
CONFIG_BT_SETTINGS=y

# Increased stack due to settings API usage
CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=2048

Expand Down
178 changes: 178 additions & 0 deletions tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_expired.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
/*
* Copyright (c) 2023 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: Apache-2.0
*/

#include "bs_bt_utils.h"

#include <stdint.h>
#include <string.h>

#include <zephyr/bluetooth/addr.h>
#include <zephyr/bluetooth/bluetooth.h>
#include <zephyr/bluetooth/conn.h>
#include <zephyr/toolchain.h>
#include <zephyr/settings/settings.h>

#include "common/bt_str.h"

#define ID_1 1
#define ID_2 2

#define ADV_SET_INDEX_1 0x00
#define ADV_SET_INDEX_2 0x01
#define ADV_SET_INDEX_3 0x02
#define ADV_SET_INDEX_4 0x03

static struct bt_le_ext_adv *adv_set[CONFIG_BT_EXT_ADV_MAX_ADV_SET];

static const struct bt_data ad_id[] = {
BT_DATA_BYTES(BT_DATA_MANUFACTURER_DATA, ADV_SET_INDEX_1),
BT_DATA_BYTES(BT_DATA_MANUFACTURER_DATA, ADV_SET_INDEX_2),
BT_DATA_BYTES(BT_DATA_MANUFACTURER_DATA, ADV_SET_INDEX_3),
BT_DATA_BYTES(BT_DATA_MANUFACTURER_DATA, ADV_SET_INDEX_4),
};

bool rpa_expired_cb_returns_true(struct bt_le_ext_adv *adv)
{
/* Return true to rotate the current RPA */
int err;
struct bt_le_ext_adv_info info;

err = bt_le_ext_adv_get_info(adv, &info);
if (err) {
return false;
}
printk("%s advertiser[%d] RPA %s\n", __func__, info.id, bt_addr_le_str(info.addr));

return true;
}

bool rpa_expired_cb_returns_false(struct bt_le_ext_adv *adv)
{
/* Return false not to rotate the current RPA */
int err;
struct bt_le_ext_adv_info info;

err = bt_le_ext_adv_get_info(adv, &info);
if (err) {
return false;
}
printk("%s advertiser[%d] RPA %s\n", __func__, info.id, bt_addr_le_str(info.addr));

return false;
}

static void create_adv(struct bt_le_ext_adv **adv, int id, bool expired_return)
{
int err;
struct bt_le_adv_param params;
static struct bt_le_ext_adv_cb cb_adv[] = {
{.rpa_expired = rpa_expired_cb_returns_true},
{.rpa_expired = rpa_expired_cb_returns_false}
};

memset(&params, 0, sizeof(struct bt_le_adv_param));

params.options |= BT_LE_ADV_OPT_EXT_ADV;
params.id = id;
params.sid = 0;
params.interval_min = BT_GAP_ADV_FAST_INT_MIN_1;
params.interval_max = BT_GAP_ADV_FAST_INT_MAX_1;

err = bt_le_ext_adv_create(&params, expired_return ? &cb_adv[0] : &cb_adv[1], adv);
if (err) {
FAIL("Failed to create advertiser (%d)\n", err);
}
}

void start_rpa_advertising(void)
{
int err;
size_t bt_id_count;

/* Enable bluetooth */
err = bt_enable(NULL);
if (err) {
FAIL("Failed to enable bluetooth (err %d\n)", err);
}

err = settings_load();
if (err) {
FAIL("Failed to enable settings (err %d\n)", err);
}

bt_id_get(NULL, &bt_id_count);

if (bt_id_count == 1) {
int id_a;
int id_b;

printk("No extra identity found in settings, creating new ones...\n");

id_a = bt_id_create(NULL, NULL);
if (id_a != ID_1) {
FAIL("bt_id_create id_a failed (err %d)\n", id_a);
}

id_b = bt_id_create(NULL, NULL);
if (id_b != ID_2) {
FAIL("bt_id_create id_b failed (err %d)\n", id_b);
}
} else {
printk("Extra identities loaded from settings\n");
}

bt_id_get(NULL, &bt_id_count);
if (bt_id_count != CONFIG_BT_ID_MAX) {
FAIL("bt_id_get returned incorrect number of identities %u\n", bt_id_count);
}

for (int i = 0; i < CONFIG_BT_EXT_ADV_MAX_ADV_SET; i++) {
/* Create first 2 advertising sets with one id and for both sets,rpa_expied_cb
* returns true.
* Create remaining 2 sets with different id and last adv set's rpa_expired cb
* returns false.
*
* So for first two adv sets with same id new rpa's will be generated every
* rotation and for last two adv sets with same id, rpa will continue with
* only one rpa through out the rotations since one of the adv set returned
* false.
*/
switch (i) {
case ADV_SET_INDEX_1:
case ADV_SET_INDEX_2:
create_adv(&adv_set[i], ID_1, true);
break;
case ADV_SET_INDEX_3:
create_adv(&adv_set[i], ID_2, true);
break;
case ADV_SET_INDEX_4:
create_adv(&adv_set[i], ID_2, false);
break;
default:
printk("Shouldn't be here\n");
break;
}

/* Set extended advertising data */
err = bt_le_ext_adv_set_data(adv_set[i], &ad_id[i], 1, NULL, 0);
if (err) {
FAIL("Failed to set advertising data for set %d (err %d)\n", i, err);
}

err = bt_le_ext_adv_start(adv_set[i], BT_LE_EXT_ADV_START_DEFAULT);
if (err) {
FAIL("Failed to start advertising (err %d)\n", err);
}
}
}

void dut_rpa_expired_procedure(void)
{
start_rpa_advertising();

/* Nothing to do */
PASS("PASS\n");
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#define ADV_SET_INDEX_ONE 0x00
#define ADV_SET_INDEX_TWO 0x01
#define ADV_SET_INDEX_THREE 0x02
#define ADV_SET_INDEX_FOUR 0x03

static struct bt_le_ext_adv *adv_set[CONFIG_BT_EXT_ADV_MAX_ADV_SET];

Expand Down Expand Up @@ -110,11 +111,10 @@ void start_advertising(void)

for (int i = 0; i < CONFIG_BT_EXT_ADV_MAX_ADV_SET; i++) {

if (i != ADV_SET_INDEX_THREE) {
/* Create advertising set 1 and 2 with same id */
/* Create first 2 advertising sets with one id and remaining with another id */
if (i < 2) {
create_adv(&adv_set[i], ID_A_INDEX);
} else {
/* Create advertising set 3 with different id */
create_adv(&adv_set[i], ID_B_INDEX);
}

Expand Down
29 changes: 6 additions & 23 deletions tests/bsim/bluetooth/host/privacy/peripheral/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,15 @@
#include "bs_bt_utils.h"
#include "bstests.h"

void tester_procedure(void);
void dut_procedure(void);
extern struct bst_test_list *test_main_rpa_rotation_install(struct bst_test_list *tests);
extern struct bst_test_list *test_main_rpa_expired_install(struct bst_test_list *tests);

static const struct bst_test_instance test_to_add[] = {
{
.test_id = "central",
.test_post_init_f = test_init,
.test_tick_f = test_tick,
.test_main_f = tester_procedure,
},
{
.test_id = "peripheral",
.test_post_init_f = test_init,
.test_tick_f = test_tick,
.test_main_f = dut_procedure,
},
BSTEST_END_MARKER,
bst_test_install_t test_installers[] = {
test_main_rpa_rotation_install,
test_main_rpa_expired_install,
NULL,
};

static struct bst_test_list *install(struct bst_test_list *tests)
{
return bst_add_tests(tests, test_to_add);
};

bst_test_install_t test_installers[] = {install, NULL};

int main(void)
{
bst_main();
Expand Down
Loading

0 comments on commit 6a52b14

Please sign in to comment.