bartoszek/zephyr
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bluetooth: Controller: Fix per sync cancel and sync establ synchronize

Browse Source 
Current implementation of ll_sync_create_cancel does not allow to stop
synchronization after ull_sync_setup is called. When that is done,
sync->timeout_reload is not zero and the ll_sync_create_cancel will
return BT_HCI_ERR_CMD_DISALLOWED. That means the Controller is able to
cancel periodic advertising synchronization only in period between
call to ll_sync_create and reception of AUX_ADV_IND that has SyncInfo
field.

The Controller should be able to cancell synchronization until first
AUX_SYNC_IND PDU is received and host notified about synchronization
established.

Complete information about synchronization status is provdied by two
ll_sync_set members: node_rx_sync_established and timeout_reload.
These two members of the structure were used in ll_sync_create_cancel
function to do a proper cancel and cleanup.
The node_rx_sync_established member was not cleared when sync was
established or expired. That was required to get a proper information
about synchronization state.

Besides that, to avoid race condition between ll_sync_create_cancel
and ull_sync_established_report, the latter function was extended
to check if cancel operation or sync lost has happened.

Signed-off-by: Piotr Pryga <piotr.pryga@nordicsemi.no>
This commit is contained in:
Piotr Pryga 2022-05-16 07:22:53 +02:00 committed by Carles Cufí
parent 80b2d7722d
commit bb7a67d5be
2 changed files with 69 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
subsys/bluetooth/controller/ll_sw/ull_sync.c
View File

@ -182,9 +182,14 @@ uint8_t ll_sync_create(uint8_t options, uint8_t sid, uint8_t adv_addr_type,
/* Initialize sync context */
node_rx->link = link_sync_estab;
sync->node_rx_sync_estab = node_rx;
sync->node_rx_lost.hdr.link = link_sync_lost;
/* Make sure that the node_rx_sync_establ hasn't got anything assigned. It is used to
* mark when sync establishment is in progress.
*/
LL_ASSERT(!sync->node_rx_sync_estab);
sync->node_rx_sync_estab = node_rx;
/* Reporting initially enabled/disabled */
sync->rx_enable =
!(options & BT_HCI_LE_PER_ADV_CREATE_SYNC_FP_REPORTS_DISABLED);
@ -301,16 +306,43 @@ uint8_t ll_sync_create_cancel(void **rx)
}
cpu_dmb();
sync = scan->periodic.sync;
if (!sync || sync->timeout_reload) {
/* FIXME: sync establishment in progress looking for first
* AUX_SYNC_IND. Cleanup by stopping ticker and disabling
* LLL events.
*/
if (!sync) {
return BT_HCI_ERR_CMD_DISALLOWED;
}
/* node_rx_sync_estab is assigned when Host calls create sync and cleared when sync is
* established. timeout_reload is set when sync is found and setup. It is non-zero until
* sync is terminated. Together they give information about current sync state:
* - node_rx_sync_estab == NULL && timeout_reload != 0 => sync is established
* - node_rx_sync_estab == NULL && timeout_reload == 0 => sync is terminated
* - node_rx_sync_estab != NULL && timeout_reload == 0 => sync is created
* - node_rx_sync_estab != NULL && timeout_reload != 0 => sync is waiting to be established
*/
if (!sync->node_rx_sync_estab) {
/* There is no sync to be cancelled */
return BT_HCI_ERR_CMD_DISALLOWED;
}
sync->is_stop = 1U;
cpu_dmb();
if (sync->timeout_reload != 0) {
uint16_t sync_handle = ull_sync_handle_get(sync);
LL_ASSERT(sync_handle <= UINT8_MAX);
/* Sync is not established yet, so stop sync ticker */
int err = ull_ticker_stop_with_mark(
TICKER_ID_SCAN_SYNC_BASE + (uint8_t)sync_handle, sync, &sync->lll);
LL_ASSERT(err == 0 || err == -EALREADY);
if (err != 0 && err != -EALREADY) {
return BT_HCI_ERR_CMD_DISALLOWED;
}
} /* else: sync was created but not yet setup, there is no sync ticker yet. */
/* It is safe to remove association with scanner as cancelled flag is
* set and sync has not been established.
* set, sync is_stop flag was set and sync has not been established.
*/
ull_sync_setup_reset(scan);
@ -327,6 +359,11 @@ uint8_t ll_sync_create_cancel(void **rx)
ll_rx_link_release(link_sync_estab);
ll_rx_release(node_rx);
/* Clear the node after release to mark the sync establish as being completed.
* In this case the completion reason is sync cancelled by Host.
*/
sync->node_rx_sync_estab = NULL;
node_rx = (void *)&sync->node_rx_lost;
node_rx->hdr.type = NODE_RX_TYPE_SYNC;
node_rx->hdr.handle = LLL_HANDLE_INVALID;
@ -792,6 +829,11 @@ void ull_sync_established_report(memq_link_t *link, struct node_rx_hdr *rx)
lll = ftr->param;
sync = HDR_LLL2ULL(lll);
/* Do nothing if sync is cancelled or lost. */
if (unlikely(sync->is_stop || !sync->timeout_reload)) {
return;
}
#if defined(CONFIG_BT_CTLR_SYNC_PERIODIC_CTE_TYPE_FILTERING)
enum sync_status sync_status;
@ -832,6 +874,11 @@ void ull_sync_established_report(memq_link_t *link, struct node_rx_hdr *rx)
rx_establ->hdr.type = NODE_RX_TYPE_SYNC;
rx_establ->hdr.handle = ull_sync_handle_get(sync);
se = (void *)rx_establ->pdu;
/* Clear the node to mark the sync establish as being completed.
* In this case the completion reason is sync being established.
*/
sync->node_rx_sync_estab = NULL;
#if defined(CONFIG_BT_CTLR_SYNC_PERIODIC_CTE_TYPE_FILTERING)
se->status = (ftr->sync_status == SYNC_STAT_TERM) ?
BT_HCI_ERR_UNSUPP_REMOTE_FEATURE :
@ -1242,6 +1289,11 @@ static void sync_expire(void *param)
rx->hdr.type = NODE_RX_TYPE_SYNC;
rx->hdr.handle = LLL_HANDLE_INVALID;
/* Clear the node to mark the sync establish as being completed.
* In this case the completion reason is sync expire.
*/
sync->node_rx_sync_estab = NULL;
/* NOTE: struct node_rx_sync_estab has uint8_t member following the
* struct node_rx_hdr to store the reason.
*/

12
subsys/bluetooth/controller/ll_sw/ull_sync_types.h
View File

@ -18,7 +18,12 @@ struct ll_sync_set {
uint16_t skip;
uint16_t timeout;
uint16_t volatile timeout_reload; /* Non-zero when sync established */
/* Non-zero when sync is setup. It can be in two sub-stated:
* - Waiting for first AUX_SYNC_IND, before sync established was notified to Host.
* If sync establishment is in progress node_rx_sync_estab is not NULL.
* - sync is already established, node_rx_sync_estab is NULL.
*/
uint16_t volatile timeout_reload;
uint16_t timeout_expire;
/* Member to store periodic advertising sync prepare.
@ -49,7 +54,7 @@ struct ll_sync_set {
uint8_t is_term:1;
#endif /* CONFIG_BT_CTLR_SYNC_PERIODIC_CTE_TYPE_FILTERING && !CONFIG_BT_CTLR_CTEINLINE_SUPPORT */
uint8_t is_stop:1; /* sync terminate requested */
uint8_t is_stop:1; /* sync terminate or cancel requested */
uint8_t sync_expire:3; /* countdown of 6 before fail to establish */
#if defined(CONFIG_BT_CTLR_CHECK_SAME_PEER_SYNC)
@ -68,6 +73,9 @@ struct ll_sync_set {
};
} node_rx_lost;
/* Not-Null when sync was setup and Controller is waiting for first AUX_SYNC_IND PDU.
* It means the sync was not estalished yet.
*/
struct node_rx_hdr *node_rx_sync_estab;
#if defined(CONFIG_BT_CTLR_SYNC_ISO)