-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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: Mesh: Allow to suspend mesh from bt_mesh_send_cb callbacks #68735
Merged
aescolar
merged 4 commits into
zephyrproject-rtos:main
from
PavelVPV:adv_suspend_test_async
Feb 26, 2024
Merged
Bluetooth: Mesh: Allow to suspend mesh from bt_mesh_send_cb callbacks #68735
aescolar
merged 4 commits into
zephyrproject-rtos:main
from
PavelVPV:adv_suspend_test_async
Feb 26, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit allows to suspend the mesh stack from `bt_mesh_send_cb` callbacks by removing the deadlock caused by `k_work_flush` in the extended advertiser. In case of the extended advertiser there are 2 cases: - when the `bt_mesh_adv_disable` is called from any of `bt_mesh_send_cb` callbacks which are called from the advertiser work item, or - when it is called from any other context. When it is called from `bt_mesh_send_cb` callbacks, since these callbacks are called from the delayable work which is running on the system workqueue, the advertiser can check the current context and its work state. If the function is called from the advertiser work, it can disable the advertising set straight away because all ble host APIs have already been called in `adv_start` function. Before sending anything else, the advertiser checks the `instance` value in `adv_start` function, which is also reset to NULL in `bt_mesh_adv_disable` call, and aborts all next advertisements. The `ADV_FLAG_SUSPENDING` tells the advertiser work to abort processing while `bt_mesh_adv_disable` function didn't finish stopping advertising set. This can happen if the work has been already scheduled and the schedler ran it while sleeping inside the `bt_le_ext_adv_stop` or `bt_le_ext_adv_disable` functions. When `bt_mesh_adv_disable` is called from any other context or from the system workqueue but not from the advertiser work, then `k_work_flush` can be called safely as it won't cause any deadlocks. The `adv_sent` function is inside the `bt_mesh_adv_disable` function to schedule the advertiser work (`send_pending_adv`) and abort all pending advertisements that have been already added to the pool. In case of the legacy advertiser, if the `bt_mesh_adv_disable` is called form the advertiser thread (this happens when it is called from `bt_mesh_send_cb.start` or `bt_mesh_send_cb.end` callbacks), then `k_thread_join` returns `-EDEADLK`. But the `enabled` flag is set to false and the thread will abort the current advertisement and the pending advertisements. Signed-off-by: Pavel Vasilyev <[email protected]>
Now, when the deadlock is removed from `bt_mesh_adv_disable` function, the advertiser can be disabled from the `bt_mesh_send_cb` callbacks. Signed-off-by: Pavel Vasilyev <[email protected]>
zephyrbot
requested review from
akredalen,
alxelax,
Andrewpini,
jhedberg and
LingaoM
February 8, 2024 10:53
This reverts commit c1bbd48. Signed-off-by: Pavel Vasilyev <[email protected]>
The stack manages to suspend the advertiser before it finishes transmitting the Outbound PDU Report message to confirm the transmission of a Provisioning PDU. The test requires the server to become unresponsive when the Provisioning PDU is sent to the unprovisioned device to test timeout of the provisioning protocol. Signed-off-by: Pavel Vasilyev <[email protected]>
PavelVPV
force-pushed
the
adv_suspend_test_async
branch
from
February 8, 2024 11:52
e19ba93
to
a78ad4e
Compare
alxelax
approved these changes
Feb 9, 2024
akredalen
approved these changes
Feb 13, 2024
jhedberg
approved these changes
Feb 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR allows to suspend the mesh stack from
bt_mesh_send_cb
callback by removing the deadlock caused byk_work_flush
.In case of the extended advertiser there are 2 cases:
bt_mesh_adv_disable
is called from any ofbt_mesh_send_cb
callbacks which are called from the advertiser work item, orWhen it is called from
bt_mesh_send_cb
callbacks, since these callbacks are called from the delayable work which is running on the system workqueue, the advertiser can check the current context and its work state. If the function is called from the advertiser work, it can disable the advertising set straight away because all ble host APIs have already been called inadv_start
function. Before sending anything else, the advertiser checks theinstance
value inadv_start
function, which is also reset to NULL inbt_mesh_adv_disable
call, and aborts all next advertisements. TheADV_FLAG_SUSPENDING
tells the advertiser work to abort processing whilebt_mesh_adv_disable
function didn't finish stopping advertising set. This can happen if the work has been already scheduled and the schedler ran it while sleeping inside thebt_le_ext_adv_stop
orbt_le_ext_adv_disable
functions.When
bt_mesh_adv_disable
is called from any other context or from the system workqueue but not from the advertiser work, thenk_work_flush
can be called safely as it won't cause any deadlocks.The
adv_sent
function is inside thebt_mesh_adv_disable
function to schedule the advertiser work (send_pending_adv
) and abort all pending advertisements that have been already added to the pool.In case of the legacy advertiser, if the
bt_mesh_adv_disable
is called form the advertiser thread (this happens when it is called frombt_mesh_send_cb.start
orbt_mesh_send_cb.end
callbacks), thenk_thread_join
returns-EDEADLK
. But theenabled
flag is set to false and the thread will abort the current advertisement and the pending advertisements.