Skip to content

Remove unnecessary ISO Rx/Tx ack serialization#16

Draft
cvinayak with Copilot wants to merge 4 commits into
mainfrom
copilot/add-iso-ack-mfifo
Draft

Remove unnecessary ISO Rx/Tx ack serialization#16
cvinayak with Copilot wants to merge 4 commits into
mainfrom
copilot/add-iso-ack-mfifo

Conversation

Copilot AI commented Jan 29, 2026

Copy link
Copy Markdown

ISO TX acknowledgments were being serialized with RX processing in rx_demux(), similar to ACL connections. This serialization is unnecessary for ISO—TX acks can be processed independently.

Changes

Removed ISO from rx_demux (ull.c)

  • Eliminated ISO ack peeking and processing from main RX demux loop
  • Removed rx_demux_iso_tx_ack() function

Added independent ISO TX ack handler (ull_iso.c)

  • New iso_tx_ack_demux() processes ISO TX acks in separate mayfly context
  • ull_iso_lll_ack_enqueue() now schedules independent processing after MFIFO enqueue
  • Helper functions made static (internal-only)

Cleaned up exports (ull_iso_internal.h)

  • Removed 4 functions no longer needed externally

Architecture

Before:

LLL → iso_ack MFIFO → rx_demux() (with RX serialization)

After:

LLL → iso_ack MFIFO → iso_tx_ack_demux() (independent)

ISO TX acks now follow the same independent processing pattern as ISO RX (iso_rx_demux()), eliminating unnecessary coupling between ISO RX and TX paths.

Net: -48 lines

Original prompt

Add iso_ack MFIFO for Context-Safe ISO TX Acknowledgment

Problem Statement

The current implementation in subsys/bluetooth/controller/ll_sw/ull_iso.c has architectural violations where LLL (Link Layer Low) context directly calls ULL (Upper Link Layer) functions:

  1. FIXME at lines 1487-1493: "ll_tx_ack_put is not LLL callable" - for CIS without vendor data path
  2. FIXME at lines 1499-1505: "ll_tx_ack_put is not LLL callable" - for BIS/ADV_ISO
  3. TODO at lines 1496-1497: "Can be unified with CIS and use ISOAL"

Current Unsafe Pattern

void ull_iso_lll_ack_enqueue(uint16_t handle, struct node_tx_iso *node_tx)
{
    // Called from LLL context
    ll_tx_ack_put(handle, (void *)node_tx);  // ❌ ULL function!
    ll_rx_sched();  // ❌ Context violation
}

Design Requirement

Implement an iso_ack MFIFO similar to the existing conn_ack MFIFO pattern used for ACL connections to provide context-safe handoff between LLL and ULL.

Reference: ACL TX Ack Design Pattern

File: subsys/bluetooth/controller/ll_sw/ull_conn.c

// MFIFO definition (lines 125-127)
static MFIFO_DEFINE(conn_ack, sizeof(struct lll_tx),
                    (CONFIG_BT_BUF_ACL_TX_COUNT + LLCP_TX_CTRL_BUF_COUNT));

// LLL enqueue (lines 1531-1541)
void ull_conn_lll_ack_enqueue(uint16_t handle, struct node_tx *tx)
{
    struct lll_tx *lll_tx;
    uint8_t idx;

    idx = MFIFO_ENQUEUE_GET(conn_ack, (void **)&lll_tx);
    LL_ASSERT_ERR(lll_tx);

    lll_tx->handle = handle;
    lll_tx->node = tx;

    MFIFO_ENQUEUE(conn_ack, idx);
}

// ULL dequeue (lines 1493-1507)
memq_link_t *ull_conn_ack_peek(uint8_t *ack_last, uint16_t *handle,
                                struct node_tx **tx)
{
    struct lll_tx *lll_tx;

    lll_tx = MFIFO_DEQUEUE_GET(conn_ack);
    if (!lll_tx) {
        return NULL;
    }

    *ack_last = mfifo_fifo_conn_ack.l;
    *handle = lll_tx->handle;
    *tx = lll_tx->node;

    return (*tx)->link;
}

Processing in ULL context (ull.c lines 2700-2760):

  • rx_demux() calls rx_demux_conn_tx_ack() in ULL context
  • Serializes TX acks with RX processing
  • Eventually calls ll_tx_ack_put() safely in ULL context

Implementation Design

Step 1: Define iso_ack MFIFO

File: subsys/bluetooth/controller/ll_sw/ull_iso.c

Add after existing includes and before function definitions:

#if defined(CONFIG_BT_CTLR_ADV_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO)
// MFIFO for ISO TX acknowledgments from LLL to ULL
static MFIFO_DEFINE(iso_ack, sizeof(struct lll_tx), 
                    BT_CTLR_ISO_TX_PDU_BUFFERS);
#endif

Step 2: Modify LLL Enqueue Function

Replace ull_iso_lll_ack_enqueue() (lines 1469-1508) to use MFIFO:

#if defined(CONFIG_BT_CTLR_ADV_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO)
void ull_iso_lll_ack_enqueue(uint16_t handle, struct node_tx_iso *node_tx)
{
    // Check if datapath exists and use ISOAL if available
    if (IS_ENABLED(CONFIG_BT_CTLR_CONN_ISO) && IS_CIS_HANDLE(handle)) {
        struct ll_conn_iso_stream *cis;
        struct ll_iso_datapath *dp;

        cis = ll_conn_iso_stream_get(handle);
        dp  = cis->hdr.datapath_in;

        if (dp) {
            // Use ISOAL for datapath case
            isoal_tx_pdu_release(dp->source_hdl, node_tx);
            return;
        }
#if defined(CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH)
        ll_data_path_tx_pdu_release(handle, node_tx);
        return;
#endif
    } else if (IS_ENABLED(CONFIG_BT_CTLR_ADV_ISO) && IS_ADV_ISO_HANDLE(handle)) {
        struct lll_adv_iso_stream *stream;
        struct ll_iso_datapath *dp;
        uint16_t stream_handle;

        stream_handle = LL_BIS_ADV_IDX_FROM_HANDLE(handle);
        stream = ull_adv_iso_stream_get(stream_handle);
        dp = stream ? stream->dp : NULL;

        if (dp) {
            // ✓ UNIFIED: Use ISOAL like CIS
            isoal_tx_pdu_release(dp->source_hdl, node_tx);
            return;
        }
#if defined(CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH)
        ll_data_path_tx_pdu_release(handle, node_tx);
        return;
#endif
    }

    // ✓ Context-safe fallback: Enqueue to iso_ack MFIFO
    struct lll_tx *lll_tx;
    uint8_t idx;

    idx = MFIFO_ENQUEUE_GET(iso_ack, (void **)&lll_tx);
    LL_ASSERT_ERR(lll_tx);

    lll_tx->handle = handle;
    lll_tx->node = (struct node_tx *)node_tx;

    MFIFO_ENQUEUE(iso_ack, idx);
}
#endif /* CONFIG_BT_CTLR_ADV_ISO || CONFIG_BT_CTLR_CONN_ISO */

Step 3: Add ULL Dequeue Functions

Add helper functions for ULL to process iso_ack MFIFO:

#if defined(CONFIG_BT_CTLR_ADV_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO)
/**
 * @brief Get last index of iso_ack MFIFO for serialization
 */
uint8_t ull_iso_ack_last_idx_get(void)
{
    return mfifo_fifo_iso_ack.l;
}

/**
 * @brief Peek at next ISO TX ack without dequeuing
 */
static memq_link_t *ull_iso_ack_peek(uint8_t *ack_last, uint16_t *handle,
                                     struct node_tx_iso **tx)
{
    struct lll_tx *lll_tx;

    lll_tx = MFIFO...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---Let Copilot coding agent [set things up for you](https://github.com/cvinayak/zephyr/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits January 29, 2026 16:21
Co-authored-by: cvinayak <6350656+cvinayak@users.noreply.github.com>
Co-authored-by: cvinayak <6350656+cvinayak@users.noreply.github.com>
Copilot AI changed the title [WIP] Add iso_ack MFIFO for context-safe ISO TX acknowledgment Add iso_ack MFIFO for context-safe ISO TX acknowledgment Jan 29, 2026
Copilot AI requested a review from cvinayak January 29, 2026 16:29
Co-authored-by: cvinayak <6350656+cvinayak@users.noreply.github.com>
Copilot AI changed the title Add iso_ack MFIFO for context-safe ISO TX acknowledgment Remove unnecessary ISO Rx/Tx ack serialization Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants