From 9e47e02f2a6052504b9b9810c960059f53fdc579 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 27 Sep 2024 10:15:34 +0200 Subject: [PATCH] Bluetooth: Controller: Fix multiple Extended Adv chain reception Fix assertion when enabling simultaneous multiple Extended Advertising chain reception that is enabled by increasing supported auxiliary scan contexts. ULL sets the association of aux context to scan and sync context, and LLL resets the association; this is safer compared to earlier implementation where ULL did both the association and reset which caused aux context memory leak. Supported auxiliary scan contexts can be increased using CONFIG_BT_CTLR_SCAN_AUX_SET value. Signed-off-by: Vinayak Kariappa Chettimada --- .../controller/ll_sw/nordic/lll/lll_scan.c | 5 ++++ .../ll_sw/nordic/lll/lll_scan_aux.c | 15 ++++++++++ .../controller/ll_sw/nordic/lll/lll_sync.c | 5 ++++ .../bluetooth/controller/ll_sw/ull_scan_aux.c | 30 ++++++++----------- 4 files changed, 37 insertions(+), 18 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c index fc85e67441e2b2..37bc040b88378f 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c @@ -1549,6 +1549,11 @@ static int isr_rx_scan_report(struct lll_scan *lll, uint8_t devmatch_ok, { struct node_rx_ftr *ftr; + /* Reset Scan context association with any Aux context as a new + * extended advertising chain is being setup for reception here. + */ + lll->lll_aux = NULL; + ftr = &(node_rx->rx_ftr); ftr->param = lll; ftr->ticks_anchor = radio_tmr_start_get(); diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c index 2424ab8a653a13..8f913558aead71 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c @@ -1241,17 +1241,32 @@ static int isr_rx_pdu(struct lll_scan *lll, struct lll_scan_aux *lll_aux, ftr = &(node_rx->rx_ftr); if (lll_aux) { + /* Auxiliary context was used in ULL scheduling in the + * reception of this current PDU. + */ ftr->param = lll_aux; ftr->scan_rsp = lll_aux->state; /* Further auxiliary PDU reception will be chain PDUs */ lll_aux->is_chain_sched = 1U; + + /* Reset auxiliary context association with scan context + * as ULL scheduling has been used and may switch to + * using LLL scheduling if the next auxiliary PDU in + * chain is below the threshold to use ULL scheduling. + */ + lll->lll_aux = NULL; + } else if (lll->lll_aux) { + /* Auxiliary context was allocated to Scan context in + * LLL scheduling in the reception of this current PDU. + */ ftr->param = lll; ftr->scan_rsp = lll->lll_aux->state; /* Further auxiliary PDU reception will be chain PDUs */ lll->lll_aux->is_chain_sched = 1U; + } else { /* Return -ECHILD, as ULL execution has not yet assigned * an aux context. This can happen only under LLL diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c index 63b6e9a53b5d3f..f74b5fa145d17d 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c @@ -805,6 +805,11 @@ static int isr_rx(struct lll_sync *lll, uint8_t node_type, uint8_t crc_ok, * again a node_rx for periodic report incomplete. */ if (node_type != NODE_RX_TYPE_EXT_AUX_REPORT) { + /* Reset Sync context association with any Aux context + * as a new chain is being setup for reception here. + */ + lll->lll_aux = NULL; + node_rx = ull_pdu_rx_alloc_peek(4); } else { node_rx = ull_pdu_rx_alloc_peek(3); diff --git a/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c b/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c index 4b609017524524..17aa91f07bf1a2 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c +++ b/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c @@ -645,6 +645,9 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) */ if (ftr->aux_lll_sched) { if (IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC) && sync_lll) { + /* Associate Sync context with the Aux context so that + * it can continue reception in LLL scheduling. + */ sync_lll->lll_aux = lll_aux; /* AUX_ADV_IND/AUX_CHAIN_IND PDU reception is being @@ -661,8 +664,8 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) */ LL_ASSERT(!lll->lll_aux || (lll->lll_aux == lll_aux)); - /* scan context get the aux context so that it can - * continue reception in LLL scheduling. + /* Associate Scan context with the Aux context so that + * it can continue reception in LLL scheduling. */ lll->lll_aux = lll_aux; @@ -688,11 +691,6 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) if (unlikely(scan->is_stop)) { goto ull_scan_aux_rx_flush; } - - /* Remove auxiliary context association with scan context so - * that LLL can differentiate it to being ULL scheduling. - */ - lll->lll_aux = NULL; } else { struct ll_sync_set *sync_set; @@ -705,7 +703,13 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_pdu *rx) goto ull_scan_aux_rx_flush; } - /* Associate the auxiliary context with sync context */ + /* Associate the auxiliary context with sync context, we do this + * for ULL scheduling also in constrast to how extended + * advertising only associates when LLL scheduling is used. + * Each Periodic Advertising chain is received by unique sync + * context, hence LLL and ULL scheduling is always associated + * with same unique sync context. + */ sync_lll->lll_aux = lll_aux; /* Backup the node rx to be dispatch on successfully ULL @@ -1280,19 +1284,9 @@ static void flush(void *param) scan = HDR_LLL2ULL(lll); scan = ull_scan_is_valid_get(scan); if (!IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC) || scan) { - lll->lll_aux = NULL; #if defined(CONFIG_BT_CTLR_JIT_SCHEDULING) lll->scan_aux_score = aux->lll.hdr.score; #endif /* CONFIG_BT_CTLR_JIT_SCHEDULING */ - } else { - struct lll_sync *sync_lll; - struct ll_sync_set *sync; - - sync_lll = aux->parent; - sync = HDR_LLL2ULL(sync_lll); - - LL_ASSERT(sync->is_stop || sync_lll->lll_aux); - sync_lll->lll_aux = NULL; } aux_release(aux);