apache / apache/mynewt-nimble

Host task priority and ble_hs_hci_cmd_tx()

Open
#2,106 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
893
Forks
512
Avg merge
13d 31m
Merged PRs (30d)
7

Description

*Preface: My project is using [esp-nimble](https://github.com/espressif/esp-nimble), which is mostly at [nimble_1_6_0_tag](https://github.com/apache/mynewt-nimble/releases/tag/nimble_1_6_0_tag). Relative to the current state of [master](https://github.com/apache/mynewt-nimble/tree/master/), there are differences in the call stack, but it looks like the issue I'm about to outline is still possible.*

Should it be assumed that the NimBLE stack host task (`nimble_port_run()`) always needs to be a higher priority than any task that would also be using the stack?

The situation I am seeing (references modified to try to align with current state of `master`):
1. The NimBLE host task is handling a `BLE_HCI_LE_SUBEV_ENH_CONN_COMPLETE` event:
* `ble_hs_hci_evt_le_enh_conn_complete()` ==>
* `ble_gap_rx_conn_complete()` ==>
* `ble_gap_rd_rem_sup_feat_tx()` ==>
* `ble_hs_hci_cmd_tx()` ==>
* Note that `ble_hs_hci_lock();` is taken as part of executing this function.
* `ble_hs_hci_wait_for_ack()` ==>
* `ble_npl_sem_pend()` currently waiting for the ACK packet from the controller.
* Task yields operation to other tasks.
2. A different task, with the same priority as the NimBLE host task, is picked up and is going to update some advertising data:
* `ble_gap_ext_adv_set_data()` ==>
* INTERRUPT! BLE controller task has an event (actually an ACK) for the host:
* `ble_transport_to_hs_evt_impl()` ==>
* `ble_hs_hci_rx_evt()` ==>
* `ble_hs_hci_rx_ack` ==>
* `ble_hs_hci_ack` gets set to a non-NULL value.
* `ble_npl_sem_release();`
* Interrupt eventually yields operation back to the previous task (the task about to update advertising data, not the host task, since they have the same priority).
* `ble_gap_ext_adv_set()` ==>
* `ble_hs_hci_cmd_tx()` ==>
* Debug assert! `ble_hs_hci_ack` is non-NULL.

Were the host task a higher priority, execution after the interrupt would return to it first since it was waiting on that semaphore. So I did just that; setting all my tasks to be one lower than the host task and I no longer have this issue. But, it didn't sit well with me that the explicit priority differences were required.

I know this is a debug assert, and could be turned off so it doesn't occur, but most of the debug asserts are related to unexpected/unexplainable conditions. I'm hoping I've given a sufficient example of a condition which does occur, but only when task priorities are the same.

Were the assert done after the call to `ble_hs_hci_lock();` then the above, should-be-allowable, condition would not throw the assert. The current task would try to take the lock, but the host task already, thus the current task yields execution. The host task could now be switched to since the ACK semaphore has been released and it would eventually release the lock and then eventually yield execution. I don't think there's a chance of deadly embrace with this change.

Contributor guide

No contributing guide indexed for this repository

Research direction

Trace the reported call paths from ble_hs_hci_cmd_tx(), ble_hs_hci_wait_for_ack(), and ble_npl_sem_pend(), then compare them with ble_hs_hci_lock() and ble_hs_hci_rx_ack() handling. Reproduce the equal-priority scheduling sequence and determine whether the assertion reflects a valid priority requirement or an ordering defect; done means the expected behavior and any needed test coverage are clear.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
embedded-iot, networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.