Skip to content

Bluetooth: Controller: Fix regression in scan aux release #85705

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 17, 2025
Merged

Bluetooth: Controller: Fix regression in scan aux release #85705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6508c87
Bluetooth: Controller: Fix regression in scan aux release
cvinayak Feb 13, 2025
d282b1f
Bluetooth: Controller: Fix redundant reset of sync aux association
cvinayak Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,9 @@ static void isr_done_cleanup(void *param)
lll->is_stop = 1U;
}

/* LLL scheduled auxiliary PDU reception is_abort on duration expire or
* aborted in the unreserved time space.
*/
if (lll->is_aux_sched) {
struct node_rx_pdu *node_rx2;

Expand Down
14 changes: 5 additions & 9 deletions subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,9 +635,6 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param)
lll = prepare_param->param;
lll->skip_prepare += (lll->lazy_prepare + 1U);

/* Reset Sync context association with any Aux context as the chain reception is aborted. */
lll->lll_aux = NULL;

/* Extra done event, to check sync lost */
e = ull_event_done_extra_get();
LL_ASSERT(e);
Expand Down Expand Up @@ -817,11 +814,6 @@ 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);
Expand Down Expand Up @@ -1189,7 +1181,11 @@ static void isr_rx_done_cleanup(struct lll_sync *lll, uint8_t crc_ok, bool sync_
{
struct event_done_extra *e;

/* Reset Sync context association with any Aux context as the chain reception is done. */
/* Reset Sync context association with any Aux context as the chain reception is done.
* By code inspection there should not be a race that ULL execution context assigns lll_aux
* that would be reset here, because either we are here not receiving a chain PDU or the
* lll_aux has been set in the node rx type NODE_RX_TYPE_EXT_AUX_RELEASE before we are here.
*/
lll->lll_aux = NULL;

/* Calculate and place the drift information in done event */
Expand Down
34 changes: 33 additions & 1 deletion subsys/bluetooth/controller/ll_sw/ull_scan_aux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,10 @@ void ull_scan_aux_release(memq_link_t *link, struct node_rx_pdu *rx)
param_ull = HDR_LLL2ULL(rx->rx_ftr.param);

if (ull_scan_is_valid_get(param_ull)) {
/* Release aux context when LLL scheduled auxiliary PDU
* reception is_abort on duration expire or aborted in the
* unreserved time space.
*/
struct lll_scan *lll;

/* Mark for buffer for release */
Expand All @@ -1134,8 +1138,21 @@ void ull_scan_aux_release(memq_link_t *link, struct node_rx_pdu *rx)
lll = rx->rx_ftr.param;
lll_aux = rx->rx_ftr.lll_aux;

/* Under race condition when LLL scheduling a reception of
* auxiliary PDU, a scan aux context may be assigned late and
* the node rx releasing the aux context will not have it.
* Release the scan aux context assigned in the scan context.
*/
if (!lll_aux) {
lll_aux = lll->lll_aux;
}

} else if (!IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC) ||
ull_scan_aux_is_valid_get(param_ull)) {
/* Release aux context when ULL scheduled auxiliary PDU
* reception is aborted.
*/

/* Mark for buffer for release */
rx->hdr.type = NODE_RX_TYPE_RELEASE;

Expand All @@ -1150,9 +1167,21 @@ void ull_scan_aux_release(memq_link_t *link, struct node_rx_pdu *rx)
/* reset data len total */
sync->data_len = 0U;

/* Release aux context in case of chain PDU reception, otherwise
* lll_aux is NULL.
*/
lll = rx->rx_ftr.param;
lll_aux = rx->rx_ftr.lll_aux;

/* Under race condition when LLL scheduling a reception of
* auxiliary PDU, a scan aux context may be assigned late and
* the node rx releasing the aux context will not have it.
* Release the scan aux context assigned in the sync context.
*/
if (!lll_aux) {
lll_aux = lll->lll_aux;
}

/* Change node type so HCI can dispatch report for truncated
* data properly.
*/
Expand Down Expand Up @@ -1183,13 +1212,16 @@ void ull_scan_aux_release(memq_link_t *link, struct node_rx_pdu *rx)
scan = ull_scan_is_valid_get(scan);
if (scan) {
is_stop = scan->is_stop;
} else {
} else if (IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC)) {
struct lll_sync *sync_lll;
struct ll_sync_set *sync;

sync_lll = (void *)lll;
sync = HDR_LLL2ULL(sync_lll);
is_stop = sync->is_stop;
} else {
LL_ASSERT(0);
return;
}

if (!is_stop) {
Expand Down
Loading