Merge branch 'idf/ble_audio_misc_fixes' into 'master'

fix(ble_audio): Miscellaneous fixes for running coordinated set

See merge request espressif/esp-idf!51932
This commit is contained in:
Island
2026-08-21 16:36:02 +08:00
7 changed files with 178 additions and 71 deletions

View File

@@ -12,8 +12,6 @@
#include "bluedroid/hci.h"
#if USE_DIRECT_HCI
/* hci/hci_layer.h pulls in osi/osi.h which defines a 2-arg CONCAT(a, b)
* macro that conflicts with the variadic CONCAT(...) from Zephyr's
* <zephyr/sys/util.h>. hci.c does not use CONCAT itself, so drop the
@@ -29,6 +27,8 @@
LOG_MODULE_REGISTER(ISO_BHCI, CONFIG_BT_ISO_LOG_LEVEL);
#if USE_DIRECT_HCI
/* Adapter-private sync cmd path. Decouples sync HCI cmds from BTU's global
* ble_sync_info entirely: each cmd carries its own command_complete_cb (this
* file's direct_hci_complete_cb) and the caller waits on direct_hci_sem,
@@ -117,10 +117,15 @@ static void direct_hci_complete_cb(BT_HDR *response, void *context)
k_sem_give(&direct_hci_sem);
}
/* Dropped-wakeup recovery in send_sync below. SLICE is well above the ~5 ms
* round trip so a healthy command never kicks; the first kick already recovers,
* and the leftover budget is one final wait, keeping the total K_SEM_SHORT. */
#define DIRECT_HCI_KICK_SLICE (50 / portTICK_PERIOD_MS)
/* Dropped-wakeup recovery in send_sync below, and the only one that works for
* our own command: it recovers precisely because the sem wait deschedules
* iso_task, which is what lets the poster holding POSTING run and clear it (a
* re-post issued back-to-back cannot — see common/task.h). SLICE stays above
* the ~5 ms round trip so a healthy command never kicks, but far below the old
* 50 ms: iso_task is the sole consumer of the ISO RX queue, and every
* millisecond spent here backs up SDUs. The first kick already recovers, and
* the leftover budget is one final wait, keeping the total K_SEM_SHORT. */
#define DIRECT_HCI_KICK_SLICE (10 / portTICK_PERIOD_MS)
#define DIRECT_HCI_KICK_MAX 3
#define DIRECT_HCI_WAIT_REST (K_SEM_SHORT - DIRECT_HCI_KICK_MAX * DIRECT_HCI_KICK_SLICE)
@@ -168,6 +173,10 @@ tBTM_STATUS bt_le_bluedroid_hci_send_sync(uint16_t opcode,
hci_layer_get_interface()->transmit_command(p, direct_hci_complete_cb,
NULL, NULL);
/* Close the POSTING window transmit_command() just opened; empty by
* default. */
bt_le_bluedroid_hci_drain_downstream();
for (kicks = 0; kicks < DIRECT_HCI_KICK_MAX; kicks++) {
if (k_sem_take_poll(&direct_hci_sem, DIRECT_HCI_KICK_SLICE) == 0) {
break;
@@ -175,38 +184,14 @@ tBTM_STATUS bt_le_bluedroid_hci_send_sync(uint16_t opcode,
LOG_WRN("[B]DirectHciKick[0x%04x][%u]", opcode, kicks + 1);
/* Re-post the downstream event: our own wakeup may have been dropped.
*
* transmit_command() does not write the command from this task. It
* appends to hci_host_env.command_queue and wakes the hciT worker via
* hci_downstream_data_post(), whose return value it discards.
* osi_thread_post_event() refuses to post while OSI_EVENT_FLAG_POSTING
* is set, and that flag is held by whichever task is mid-post from the
* moment it sets the flag until it is rescheduled: osi_thread_post()
* ends in osi_sem_give(work_sem), which immediately yields to the
* higher-priority hciT, so POSTING stays set across the whole handler
* run. A second producer landing in that window is rejected:
*
* BTU set QUEUED|POSTING; osi_thread_post() -> sem give --.
* hciT clear QUEUED; handler: send the ACL, command_queue <-'
* still empty -> break; block again
* iso_task (same prio as BTU, round-robin) transmit_command():
* enqueue cmd ok, post -> QUEUED clear but POSTING set
* -> rejected, no wakeup, return value discarded
* BTU resumes, clears POSTING (too late)
*
* The command then sits in command_queue with nothing scheduled to
* drain it. It never reached commands_pending_response either, so
* Bluedroid's own COMMAND_PENDING_TIMEOUT never arms and only this
* task's sem timeout notices. That is expensive here: the caller is the
* ISO task, sole consumer of the ISO RX queue, so a full K_SEM_SHORT
* stall drops every SDU received during it.
*
* One slice later the POSTING holder has long been rescheduled, so this
* post lands and hciT drains the queued command. Bluedroid-only: NimBLE
* writes the command inline from the caller's task (ble_hs_hci_cmd_tx),
* so it has no wakeup to lose. Drop this loop once the POSTING gate in
* osi_event_can_post_locked() is fixed (regression in 0564b09e86f). */
/* Still no response: this command sits in command_queue with nothing
* scheduled to drain it — its post was rejected inside another poster's
* POSTING window, and it never reached commands_pending_response either,
* so COMMAND_PENDING_TIMEOUT never armed. The wait above descheduled us,
* so that poster has since cleared POSTING; this post lands and hciT
* drains the queue. Bluedroid-only: NimBLE writes the command inline
* from the caller's task (ble_hs_hci_cmd_tx), so it has no wakeup to
* lose. */
hci_downstream_data_post(OSI_THREAD_MAX_TIMEOUT);
}
@@ -249,3 +234,27 @@ void bt_le_bluedroid_hci_deinit(void)
}
#endif /* USE_DIRECT_HCI */
/* Re-post the downstream event after one of our HCI commands, to flush a packet
* whose own post was rejected inside the window that command opened.
*
* Both the command queue and the ACL data queue are drained by one shared
* osi_event (hci_host_env.downstream_data_ready). osi_thread_post_event() holds
* OSI_EVENT_FLAG_POSTING from before its osi_thread_post() — which yields to the
* higher-priority hciT — until the poster is rescheduled, and
* osi_event_can_post_locked() rejects any post landing in that window. Both
* victims are silent: transmit_downward() and host_send_pkt_available_cb()
* discard the return value, and a command rejected this way never reaches
* commands_pending_response, so COMMAND_PENDING_TIMEOUT never arms. The stranded
* packet then waits for the next successful post from anyone — measured at
* 56.6 s on a peripheral holding a CSIS set lock, where the lock's 60 s timeout
* notification happened to be that next post.
*
* Empty by default: ISO_TASK_PRIO above BTU closes the window instead, and this
* re-post cannot help once it does. See common/task.h. */
void bt_le_bluedroid_hci_drain_downstream(void)
{
#if ISO_HCI_DRAIN_DOWNSTREAM
hci_downstream_data_post(OSI_THREAD_MAX_TIMEOUT);
#endif /* ISO_HCI_DRAIN_DOWNSTREAM */
}

View File

@@ -26,6 +26,12 @@ int bt_le_bluedroid_hci_init(void);
void bt_le_bluedroid_hci_deinit(void);
/* Re-post Bluedroid's downstream-data event; call right after any HCI command
* this adapter sends from the iso task. Empty unless ISO_HCI_DRAIN_DOWNSTREAM
* is enabled — see hci.c for the mechanism and common/task.h for why it is off
* by default. */
void bt_le_bluedroid_hci_drain_downstream(void);
/* Send a sync HCI command, bypassing BTM and ble_sync_info. `cmd_params`
* is the cmd parameter payload (without opcode/length preamble).
*

View File

@@ -29,6 +29,26 @@ LOG_MODULE_REGISTER(ISO_BISO, CONFIG_BT_ISO_LOG_LEVEL);
* path (bt_le_bluedroid_hci_send_sync) nor the fire-and-forget BTM_*
* calls need bt_le_host_lock; single-task affinity is the serialization.
*
* The bt_le_bluedroid_hci_drain_downstream() call after each fire-and-forget
* BTM_* command below compiles away by default: what actually keeps a
* concurrent BTU ACL post from being rejected inside our POSTING window is
* ISO_TASK_PRIO sitting above BTU. See common/task.h for both.
*
* TODO:
* The opposite direction is uncovered for these commands. If BTU's own POSTING
* window rejects the post one of them makes, it strands in command_queue until
* the next successful post from anyone — unbounded when the link happens to go
* quiet. send_sync survives this via its kick loop, which works only because
* the sem wait deschedules us so BTU can clear POSTING; fire-and-forget has no
* response to wait on, so it has no equivalent. Raising ISO_TASK_PRIO above BTU
* makes us win that race more often, so the odds went up.
*
* Not observed yet. Each call site below names the event that goes missing when
* it happens. The fix is an explicit yield (vTaskDelay(1)) after the BTM call,
* so BTU can clear POSTING, then a re-post via
* bt_le_bluedroid_hci_drain_downstream() — add it only once this shows up, and
* only at the site that showed it.
*
* iso_sem is the legacy sync-response mechanism for the USE_DIRECT_HCI=0
* fallback only (set_cig_params / read_tx_sync): iso_evt_rx fills the
* file-scope tx_sync / set_cig_params buffers on BTU, then gives the
@@ -385,8 +405,10 @@ static int hci_cmd_create_cis(struct net_buf *buf, struct net_buf **rsp)
}
/* No direct_hci variant: HCI Create CIS returns Command_Status;
* outcome arrives via BTM_BLE_ISO_CIS_ESTABLISHED_EVT. */
* outcome arrives via BTM_BLE_ISO_CIS_ESTABLISHED_EVT.
* A lost post (file-top TODO) means that event never arrives. */
status = BTM_BleCreateCis(cis_count, (void *)cis_params);
bt_le_bluedroid_hci_drain_downstream();
net_buf_unref(buf);
free(cis_params);
@@ -423,8 +445,10 @@ static int hci_cmd_accept_cis_req(struct net_buf *buf, struct net_buf **rsp)
cis_handle = sys_get_le16(buf->data + 3);
/* No direct_hci variant: HCI Accept CIS Request returns Command_Status;
* outcome arrives via BTM_BLE_ISO_CIS_ESTABLISHED_EVT. */
* outcome arrives via BTM_BLE_ISO_CIS_ESTABLISHED_EVT.
* A lost post (file-top TODO) means that event never arrives. */
status = BTM_BleAcceptCisReq(cis_handle);
bt_le_bluedroid_hci_drain_downstream();
net_buf_unref(buf);
@@ -483,7 +507,8 @@ static int hci_cmd_create_big(struct net_buf *buf, struct net_buf **rsp)
bst_code = buf->data + 18;
/* No direct_hci variant: HCI Create BIG returns Command_Status;
* outcome arrives via BTM_BLE_ISO_BIG_CREATE_COMPLETE_EVT. */
* outcome arrives via BTM_BLE_ISO_BIG_CREATE_COMPLETE_EVT.
* A lost post (file-top TODO) means that event never arrives. */
status = BTM_BleBigCreate(big_handle,
adv_handle,
num_bis,
@@ -496,6 +521,7 @@ static int hci_cmd_create_big(struct net_buf *buf, struct net_buf **rsp)
framing,
encryption,
bst_code);
bt_le_bluedroid_hci_drain_downstream();
net_buf_unref(buf);
@@ -542,7 +568,8 @@ static int hci_cmd_create_big_test(struct net_buf *buf, struct net_buf **rsp)
bst_code = buf->data + 23;
/* No direct_hci variant: HCI Create BIG Test returns Command_Status;
* outcome arrives via BTM_BLE_ISO_BIG_CREATE_COMPLETE_EVT. */
* outcome arrives via BTM_BLE_ISO_BIG_CREATE_COMPLETE_EVT.
* A lost post (file-top TODO) means that event never arrives. */
status = BTM_BleBigCreateTest(big_handle,
adv_handle,
num_bis,
@@ -559,6 +586,7 @@ static int hci_cmd_create_big_test(struct net_buf *buf, struct net_buf **rsp)
pto,
encryption,
bst_code);
bt_le_bluedroid_hci_drain_downstream();
net_buf_unref(buf);
@@ -577,8 +605,10 @@ static int hci_cmd_terminate_big(struct net_buf *buf, struct net_buf **rsp)
reason = buf->data[4];
/* No direct_hci variant: HCI Terminate BIG returns Command_Status;
* outcome arrives via BTM_BLE_ISO_BIG_TERMINATE_COMPLETE_EVT. */
* outcome arrives via BTM_BLE_ISO_BIG_TERMINATE_COMPLETE_EVT.
* A lost post (file-top TODO) means that event never arrives. */
status = BTM_BleBigTerminate(big_handle, reason);
bt_le_bluedroid_hci_drain_downstream();
net_buf_unref(buf);
@@ -609,7 +639,8 @@ static int hci_cmd_big_create_sync(struct net_buf *buf, struct net_buf **rsp)
bis = buf->data + 27;
/* No direct_hci variant: HCI BIG Create Sync returns Command_Status;
* outcome arrives via BTM_BLE_ISO_BIG_SYNC_ESTABLISHED_EVT. */
* outcome arrives via BTM_BLE_ISO_BIG_SYNC_ESTABLISHED_EVT.
* A lost post (file-top TODO) means that event never arrives. */
status = BTM_BleBigSyncCreate(big_handle,
sync_handle,
encryption,
@@ -618,6 +649,7 @@ static int hci_cmd_big_create_sync(struct net_buf *buf, struct net_buf **rsp)
sync_timeout,
num_bis,
bis);
bt_le_bluedroid_hci_drain_downstream();
net_buf_unref(buf);
@@ -1241,8 +1273,10 @@ int bt_le_bluedroid_iso_disconnect(uint16_t conn_handle, uint8_t reason)
LOG_DBG("[B]IsoDisconn[0x%03x][%02x]", conn_handle, reason);
/* No direct_hci variant: HCI Disconnect returns Command_Status;
* outcome arrives via BTM_BLE_ISO_CIS_DISCONNECTED_EVT. */
* outcome arrives via BTM_BLE_ISO_CIS_DISCONNECTED_EVT.
* A lost post (file-top TODO) means that event never arrives. */
status = BTM_BleDisconCis(conn_handle, reason);
bt_le_bluedroid_hci_drain_downstream();
if (status != BTM_SUCCESS) {
LOG_ERR("[B]IsoDisconnFail[0x%03x][%02x]", conn_handle, status);

View File

@@ -22,30 +22,52 @@ extern "C" {
#if CONFIG_BT_BLUEDROID_ENABLED
#if CONFIG_BT_BLUEDROID_PINNED_TO_CORE
#define ISO_TASK_CORE CONFIG_BT_BLUEDROID_PINNED_TO_CORE
#define ISO_TASK_CORE CONFIG_BT_BLUEDROID_PINNED_TO_CORE
#else /* CONFIG_BT_BLUEDROID_PINNED_TO_CORE */
#define ISO_TASK_CORE (0)
#define ISO_TASK_CORE (0)
#endif /* CONFIG_BT_BLUEDROID_PINNED_TO_CORE */
#else /* CONFIG_BT_BLUEDROID_ENABLED */
#if CONFIG_BT_NIMBLE_PINNED_TO_CORE
#define ISO_TASK_CORE CONFIG_BT_NIMBLE_PINNED_TO_CORE
#define ISO_TASK_CORE CONFIG_BT_NIMBLE_PINNED_TO_CORE
#else /* CONFIG_BT_NIMBLE_PINNED_TO_CORE */
#define ISO_TASK_CORE (0)
#define ISO_TASK_CORE (0)
#endif /* CONFIG_BT_NIMBLE_PINNED_TO_CORE */
#endif /* CONFIG_BT_BLUEDROID_ENABLED */
#define ISO_TASK_STACK_SIZE 4096
#define ISO_TASK_NAME "iso_task"
#define ISO_TASK_STACK_SIZE 4096
#define ISO_TASK_NAME "iso_task"
/* Ref:
* - Bluedroid BTC task: configMAX_PRIORITIES - 6
* - Bluedroid BTU task: configMAX_PRIORITIES - 5
* - NimBLE Host task: configMAX_PRIORITIES - 4
* - Bluedroid hci_layer task: configMAX_PRIORITIES - 3
* - NimBLE Host task: configMAX_PRIORITIES - 4
* - Bluedroid BTU task: configMAX_PRIORITIES - 5
* - Bluedroid BTC task: configMAX_PRIORITIES - 6
*
* MUST stay strictly above BTU. Both tasks post to Bluedroid's single shared
* downstream event, and osi_thread_post_event() keeps OSI_EVENT_FLAG_POSTING
* set across its osi_thread_post() — which yields to the higher-priority
* hci_layer task — until the poster is rescheduled. Any post landing in that
* window is rejected, and the ACL path discards the result, stranding the
* packet until someone else posts (measured at 56.6 s). At equal priority the
* round-robin lets BTU run inside our window and lose a notification that way;
* one rung above BTU, BTU cannot be scheduled until we have cleared POSTING,
* so it can never be the victim. Same core is a precondition — see
* ISO_TASK_CORE above. The reverse (BTU's window rejecting our command) stays
* possible and is covered by the kick loop in adapter/bluedroid/hci.c.
*/
#if CONFIG_BT_BLUEDROID_ENABLED
#define ISO_TASK_PRIO (configMAX_PRIORITIES - 5)
#else
#define ISO_TASK_PRIO (configMAX_PRIORITIES - 4)
#endif
#define ISO_TASK_PRIO (configMAX_PRIORITIES - 4)
/* The other, weaker mitigation for the same lost-wakeup race: re-post the
* shared downstream event after every HCI command we send, hoping to flush a
* packet whose own post was rejected inside our POSTING window.
*
* Off because the priority above supersedes it, and once that holds it cannot
* help in either direction: BTU never runs inside our window, so there is no
* stranded ACL packet to flush; and if BTU's window rejected our command, BTU
* is not scheduled between our two posts either, so the re-post is rejected
* with it. Measured ineffective on hardware before the priority change too.
* Enable only together with lowering ISO_TASK_PRIO back to BTU's level.
* Mechanism: adapter/bluedroid/hci.c. */
#define ISO_HCI_DRAIN_DOWNSTREAM 0
/* Ordered by priority tier, highest first. The numeric value is not used for
* prioritization (that comes from which queue bt_le_iso_task_post routes to);

View File

@@ -28,11 +28,6 @@ static esp_bt_status_t scan_op_status;
#define WAIT_API(_call) EXAMPLE_WAIT_API_CHECK(_call, scan_sem, portMAX_DELAY, scan_op_status)
/* Cached peer address. Bluedroid's pairing and disconnect APIs key off
* bd_addr rather than conn_handle, so we stash the addr at conn_create
* time and reuse it in pairing_start / security_failed_recover. */
static esp_bd_addr_t peer_bda;
static esp_ble_ext_scan_params_t ext_scan_params = {
.own_addr_type = BLE_ADDR_TYPE_PUBLIC,
.filter_policy = BLE_SCAN_FILTER_ALLOW_ALL,
@@ -138,6 +133,9 @@ int conn_create(uint8_t addr_type, const uint8_t addr[6])
esp_gatt_if_t gattc_if;
esp_err_t err;
esp_bd_addr_t peer_bda;
/* Local copy: the Bluedroid APIs below take a non-const esp_bd_addr_t. */
memcpy(peer_bda, addr, sizeof(peer_bda));
err = esp_ble_gap_prefer_ext_connect_params_set(
@@ -165,8 +163,14 @@ int conn_create(uint8_t addr_type, const uint8_t addr[6])
int pairing_start(uint16_t conn_handle)
{
(void)conn_handle;
return esp_ble_set_encryption(peer_bda, ESP_BLE_SEC_ENCRYPT_NO_MITM);
const uint8_t *addr = set_member_addr(conn_handle);
if (addr == NULL) {
ESP_LOGE(TAG, "No address for handle %u; not starting security", conn_handle);
return ESP_ERR_INVALID_STATE;
}
return esp_ble_set_encryption((uint8_t *)addr, ESP_BLE_SEC_ENCRYPT_NO_MITM);
}
int exchange_mtu(uint16_t conn_handle)
@@ -181,13 +185,20 @@ int exchange_mtu(uint16_t conn_handle)
void security_failed_recover(uint16_t conn_handle, uint8_t status)
{
(void)conn_handle;
const uint8_t *addr = set_member_addr(conn_handle);
if (addr == NULL) {
ESP_LOGE(TAG, "Security change failed on handle %u, status %u; member already gone",
conn_handle, status);
return;
}
/* Asymmetric bond state: we still hold an LTK for this peer but it
* cleared its side, so encrypt-with-cached-key times out. Drop the bond
* and tear down the link; the next reconnect runs fresh pairing. */
ESP_LOGE(TAG, "Security change failed, status %u, clearing local bond and reconnecting", status);
ESP_LOGE(TAG, "Security change failed on handle %u, status %u, clearing local bond and reconnecting",
conn_handle, status);
esp_ble_remove_bond_device(peer_bda);
esp_ble_gap_disconnect(peer_bda);
esp_ble_remove_bond_device((uint8_t *)addr);
esp_ble_gap_disconnect((uint8_t *)addr);
}

View File

@@ -27,6 +27,12 @@ struct set_member {
static struct set_member members[TMAP_CEN_PEER_COUNT];
static size_t member_count;
/* One connect at a time. members[] only learns a peer at acl_connect, and
* ext_scan_stop() does not discard reports already queued for us, so without
* this a second report starts an overlapping conn_create that the stack
* rejects ("L2CAP - LE - cannot start new connection at conn st: 1"). */
static bool conn_pending;
/* Lock state: pending until the callback, held until stream setup completes. */
static bool lock_pending;
static bool set_locked;
@@ -41,6 +47,17 @@ uint16_t set_member_handle(size_t index)
return (index < member_count) ? members[index].handle : CONN_HANDLE_INIT;
}
const uint8_t *set_member_addr(uint16_t conn_handle)
{
for (size_t i = 0; i < member_count; i++) {
if (members[i].handle == conn_handle) {
return members[i].dst;
}
}
return NULL;
}
static struct set_member *member_by_handle(uint16_t handle)
{
for (size_t i = 0; i < member_count; i++) {
@@ -196,7 +213,7 @@ static void ext_scan_recv(esp_ble_audio_gap_app_event_t *event)
struct adv_match match = {0};
int err;
if (member_count >= ARRAY_SIZE(members)) {
if (member_count >= ARRAY_SIZE(members) || conn_pending) {
return;
}
@@ -235,15 +252,21 @@ static void ext_scan_recv(esp_ble_audio_gap_app_event_t *event)
if (err) {
ESP_LOGE(TAG, "Failed to create conn, err %d", err);
ext_scan_start();
return;
}
conn_pending = true;
}
static void acl_connect(esp_ble_audio_gap_app_event_t *event)
{
int err;
conn_pending = false;
if (event->acl_connect.status) {
ESP_LOGE(TAG, "Connection failed, status %d", event->acl_connect.status);
ext_scan_start();
return;
}

View File

@@ -93,6 +93,8 @@ size_t set_member_count(void);
uint16_t set_member_handle(size_t index);
const uint8_t *set_member_addr(uint16_t conn_handle);
void set_member_discovered(uint16_t conn_handle, int err);
void set_lock_complete(int err);