Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth: Controller: Fix periodic advertising synchronization cancel and synchronization established synchronize #45673

Conversation

ppryga-nordic
Copy link
Collaborator

@ppryga-nordic ppryga-nordic commented May 16, 2022

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.

Fixes #40204.

Signed-off-by: Piotr Pryga [email protected]

cvinayak
cvinayak previously approved these changes May 16, 2022
Copy link
Contributor

@cvinayak cvinayak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job. LGTM.

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 <[email protected]>
@ppryga-nordic ppryga-nordic force-pushed the github-ble-fix-per-sync-canel-and-sync-establish-synchronization branch from d609666 to e65be58 Compare May 16, 2022 15:23
@carlescufi carlescufi closed this May 16, 2022
@carlescufi carlescufi deleted the github-ble-fix-per-sync-canel-and-sync-establish-synchronization branch May 16, 2022 15:28
@ppryga-nordic
Copy link
Collaborator Author

Closing PR because it was based on upstream branch, created by accident.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Controller area: Bluetooth bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
3 participants