From 3d36a229e8ef5e2ef579ad5652d355fbdf54e1d8 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Thu, 30 Jan 2025 09:44:55 +0100 Subject: [PATCH] Bluetooth: Controller: Fix device address in Periodic Adv Sync Fix incorrect device address reported in the LE Periodic Advertising Sync Established event when using Periodic Advertiser List. During Extended Scanning there can be an ADV_EXT_IND PDU received between currently being received ADV_EXT_IND PDU and AUX_ADV_IND PDU; if the one received between has an address match then incorrectly the Periodic Synchronization was established to the device whos AUX_ADV_IND PDU is being received. Fix by storing the auxiliary context that has the address match and compare with it when matching the SID in SyncInfo of AUX_ADV_IND PDU being received prior to creating the synchronization. Signed-off-by: Vinayak Kariappa Chettimada (cherry picked from commit 83e2ec3c9b17f410adc5d960b0b386f6212d75ed) --- .../bluetooth/controller/ll_sw/ull_scan_aux.c | 87 ++++++++++++++++--- .../controller/ll_sw/ull_scan_types.h | 5 ++ subsys/bluetooth/controller/ll_sw/ull_sync.c | 19 ++-- .../controller/ll_sw/ull_sync_internal.h | 2 +- 4 files changed, 94 insertions(+), 19 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c b/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c index b572f1d0d92..ce9c027c2b9 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c +++ b/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c @@ -440,19 +440,30 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) ptr = h->data; +#if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) + bool is_aux_addr_match = false; +#endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ + if (h->adv_addr) { #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) /* Check if Periodic Advertising Synchronization to be created */ if (sync && (scan->periodic.state != LL_SYNC_STATE_CREATED)) { - /* Check address and update internal state */ #if defined(CONFIG_BT_CTLR_PRIVACY) - ull_sync_setup_addr_check(sync, scan, pdu->tx_addr, ptr, - ftr->rl_idx); + uint8_t rl_idx = ftr->rl_idx; #else /* !CONFIG_BT_CTLR_PRIVACY */ - ull_sync_setup_addr_check(sync, scan, pdu->tx_addr, ptr, 0U); + uint8_t rl_idx = 0U; #endif /* !CONFIG_BT_CTLR_PRIVACY */ + /* Check address and update internal state */ + is_aux_addr_match = + ull_sync_setup_addr_check(sync, scan->periodic.filter_policy, + pdu->tx_addr, ptr, rl_idx); + if (is_aux_addr_match) { + scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + } else { + scan->periodic.state = LL_SYNC_STATE_IDLE; + } } #endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ @@ -485,14 +496,21 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) si = (void *)ptr; ptr += sizeof(*si); +#if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) /* Check if Periodic Advertising Synchronization to be created. * Setup synchronization if address and SID match in the * Periodic Advertiser List or with the explicitly supplied. + * + * is_aux_addr_match, device address in auxiliary channel PDU; + * scan->periodic.param has not been assigned yet. + * Otherwise, address was in primary channel PDU and we are now + * checking SID (in SyncInfo) in auxiliary channel PDU. */ - if (IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC) && aux && sync && adi && + if (sync && aux && (is_aux_addr_match || (scan->periodic.param == aux)) && adi && ull_sync_setup_sid_match(sync, scan, PDU_ADV_ADI_SID_GET(adi))) { ull_sync_setup(scan, aux->lll.phy, rx, si); } +#endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ } if (h->tx_pwr) { @@ -688,6 +706,15 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) #endif /* CONFIG_BT_CTLR_JIT_SCHEDULING */ #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) + /* Store the aux context that has Periodic Advertising + * Synchronization address match. + */ + if (sync && (scan->periodic.state == LL_SYNC_STATE_ADDR_MATCH)) { + scan->periodic.param = aux; + } + + /* Store the node rx allocated for incomplete report, if needed. + */ aux->rx_incomplete = rx_incomplete; rx_incomplete = NULL; #endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ @@ -912,6 +939,7 @@ ull_scan_aux_rx_flush: #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) if (sync && (scan->periodic.state != LL_SYNC_STATE_CREATED)) { scan->periodic.state = LL_SYNC_STATE_IDLE; + scan->periodic.param = NULL; } #endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ @@ -1514,7 +1542,16 @@ static void ticker_op_cb(uint32_t status, void *param) } #else /* CONFIG_BT_CTLR_SCAN_AUX_USE_CHAINS */ - +/* NOTE: BT_CTLR_SCAN_AUX_USE_CHAINS is alternative new design with less RAM + * usage for supporting Extended Scanning of simultaneous interleaved + * Extended Advertising chains. + * + * TODO: As the previous design has Bluetooth Qualified Design Listing by + * Nordic Semiconductor ASA, both implementation are present in this file, + * and default builds use CONFIG_BT_CTLR_SCAN_AUX_USE_CHAINS=n. Remove old + * implementation when we have a new Bluetooth Qualified Design Listing + * with the new Extended Scanning and Periodic Sync implementation. + */ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) { struct node_rx_pdu *rx_incomplete; @@ -1774,19 +1811,30 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) ptr = h->data; +#if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) + bool is_aux_addr_match = false; +#endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ + if (h->adv_addr) { #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) /* Check if Periodic Advertising Synchronization to be created */ if (sync && (scan->periodic.state != LL_SYNC_STATE_CREATED)) { - /* Check address and update internal state */ #if defined(CONFIG_BT_CTLR_PRIVACY) - ull_sync_setup_addr_check(sync, scan, pdu->tx_addr, ptr, - ftr->rl_idx); + uint8_t rl_idx = ftr->rl_idx; #else /* !CONFIG_BT_CTLR_PRIVACY */ - ull_sync_setup_addr_check(sync, scan, pdu->tx_addr, ptr, 0U); + uint8_t rl_idx = 0U; #endif /* !CONFIG_BT_CTLR_PRIVACY */ + /* Check address and update internal state */ + is_aux_addr_match = + ull_sync_setup_addr_check(sync, scan->periodic.filter_policy, + pdu->tx_addr, ptr, rl_idx); + if (is_aux_addr_match) { + scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + } else { + scan->periodic.state = LL_SYNC_STATE_IDLE; + } } #endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ @@ -1819,14 +1867,21 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) si = (void *)ptr; ptr += sizeof(*si); +#if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) /* Check if Periodic Advertising Synchronization to be created. * Setup synchronization if address and SID match in the * Periodic Advertiser List or with the explicitly supplied. + * + * is_aux_addr_match, device address in auxiliary channel PDU; + * scan->periodic.param has not been assigned yet. + * Otherwise, address was in primary channel PDU and we are now + * checking SID (in SyncInfo) in auxiliary channel PDU. */ - if (IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC) && chain && sync && adi && - ull_sync_setup_sid_match(sync, scan, PDU_ADV_ADI_SID_GET(adi))) { + if (sync && chain && (is_aux_addr_match || (scan->periodic.param == chain)) && + adi && ull_sync_setup_sid_match(sync, scan, PDU_ADV_ADI_SID_GET(adi))) { ull_sync_setup(scan, chain->lll.phy, rx, si); } +#endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ } if (h->tx_pwr) { @@ -2008,6 +2063,13 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) #endif /* CONFIG_BT_CTLR_JIT_SCHEDULING */ #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) + /* Store the chain context that has Periodic Advertising + * Synchronization address match. + */ + if (sync && (scan->periodic.state == LL_SYNC_STATE_ADDR_MATCH)) { + scan->periodic.param = chain; + } + if (sync_lll) { struct ll_sync_set *sync_set = HDR_LLL2ULL(sync_lll); @@ -2149,6 +2211,7 @@ ull_scan_aux_rx_flush: #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) if (sync && (scan->periodic.state != LL_SYNC_STATE_CREATED)) { scan->periodic.state = LL_SYNC_STATE_IDLE; + scan->periodic.param = NULL; } #endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_scan_types.h b/subsys/bluetooth/controller/ll_sw/ull_scan_types.h index c9d2c073f8f..f4458825a5a 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_scan_types.h +++ b/subsys/bluetooth/controller/ll_sw/ull_scan_types.h @@ -31,6 +31,11 @@ struct ll_scan_set { * cancelling sync create, hence the volatile keyword. */ struct ll_sync_set *volatile sync; + + /* Non-NULL when Periodic Advertising Synchronisation address + * matched. + */ + void *param; } periodic; #endif }; diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync.c b/subsys/bluetooth/controller/ll_sw/ull_sync.c index 63e9e7ef57c..9094366716f 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_sync.c +++ b/subsys/bluetooth/controller/ll_sw/ull_sync.c @@ -152,11 +152,13 @@ uint8_t ll_sync_create(uint8_t options, uint8_t sid, uint8_t adv_addr_type, scan->periodic.cancelled = 0U; scan->periodic.state = LL_SYNC_STATE_IDLE; + scan->periodic.param = NULL; scan->periodic.filter_policy = options & BT_HCI_LE_PER_ADV_CREATE_SYNC_FP_USE_LIST; if (IS_ENABLED(CONFIG_BT_CTLR_PHY_CODED)) { scan_coded->periodic.cancelled = 0U; scan_coded->periodic.state = LL_SYNC_STATE_IDLE; + scan_coded->periodic.param = NULL; scan_coded->periodic.filter_policy = scan->periodic.filter_policy; } @@ -880,12 +882,12 @@ void ull_sync_release(struct ll_sync_set *sync) mem_release(sync, &sync_free); } -void ull_sync_setup_addr_check(struct ll_sync_set *sync, struct ll_scan_set *scan, +bool ull_sync_setup_addr_check(struct ll_sync_set *sync, uint8_t filter_policy, uint8_t addr_type, uint8_t *addr, uint8_t rl_idx) { /* Check if Periodic Advertiser list to be used */ if (IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC_ADV_LIST) && - scan->periodic.filter_policy) { + filter_policy) { /* Check in Periodic Advertiser List */ if (ull_filter_ull_pal_addr_match(addr_type, addr)) { /* Remember the address, to check with @@ -896,7 +898,7 @@ void ull_sync_setup_addr_check(struct ll_sync_set *sync, struct ll_scan_set *sca BDADDR_SIZE); /* Address matched */ - scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + return true; /* Check in Resolving List */ } else if (IS_ENABLED(CONFIG_BT_CTLR_PRIVACY) && @@ -911,14 +913,14 @@ void ull_sync_setup_addr_check(struct ll_sync_set *sync, struct ll_scan_set *sca sync->peer_addr_resolved = 1U; /* Address matched */ - scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + return true; } /* Check with explicitly supplied address */ } else if ((addr_type == sync->peer_id_addr_type) && !memcmp(addr, sync->peer_id_addr, BDADDR_SIZE)) { /* Address matched */ - scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + return true; /* Check identity address with explicitly supplied address */ } else if (IS_ENABLED(CONFIG_BT_CTLR_PRIVACY) && @@ -930,9 +932,11 @@ void ull_sync_setup_addr_check(struct ll_sync_set *sync, struct ll_scan_set *sca sync->peer_addr_resolved = 1U; /* Identity address matched */ - scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + return true; } } + + return false; } bool ull_sync_setup_sid_match(struct ll_sync_set *sync, struct ll_scan_set *scan, uint8_t sid) @@ -1060,6 +1064,7 @@ void ull_sync_setup(struct ll_scan_set *scan, uint8_t phy, /* Set the state to sync create */ scan->periodic.state = LL_SYNC_STATE_CREATED; + scan->periodic.param = NULL; if (IS_ENABLED(CONFIG_BT_CTLR_PHY_CODED)) { struct ll_scan_set *scan_1m; @@ -1069,8 +1074,10 @@ void ull_sync_setup(struct ll_scan_set *scan, uint8_t phy, scan_coded = ull_scan_set_get(SCAN_HANDLE_PHY_CODED); scan_coded->periodic.state = LL_SYNC_STATE_CREATED; + scan_coded->periodic.param = NULL; } else { scan_1m->periodic.state = LL_SYNC_STATE_CREATED; + scan_1m->periodic.param = NULL; } } diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync_internal.h b/subsys/bluetooth/controller/ll_sw/ull_sync_internal.h index 79f2f3d67de..e8111a783c9 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_sync_internal.h +++ b/subsys/bluetooth/controller/ll_sw/ull_sync_internal.h @@ -9,7 +9,7 @@ int ull_sync_reset(void); uint16_t ull_sync_handle_get(struct ll_sync_set *sync); struct ll_sync_set *ull_sync_is_enabled_get(uint16_t handle); void ull_sync_release(struct ll_sync_set *sync); -void ull_sync_setup_addr_check(struct ll_sync_set *sync, struct ll_scan_set *scan, +bool ull_sync_setup_addr_check(struct ll_sync_set *sync, uint8_t filter_policy, uint8_t addr_type, uint8_t *addr, uint8_t rl_idx); bool ull_sync_setup_sid_match(struct ll_sync_set *sync, struct ll_scan_set *scan, uint8_t sid); void ull_sync_create_from_sync_transfer(uint16_t conn_handle, uint16_t service_data,