Skip to content

Commit 49642ef

Browse files
cvinayakfabiobaltieri
authored andcommitted
Bluetooth: Controller: Fix regression in scan aux release
Fix regression in scan aux release now that aux context is retrieved from the node rx when supporting multiple chain reception. Relates to commit a806592 ("Bluetooth: Controller: Fix to release aux context stored in node rx"). Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
1 parent aec991e commit 49642ef

File tree

3 files changed

+53
-3
lines changed

3 files changed

+53
-3
lines changed

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,9 @@ static void isr_done_cleanup(void *param)
10891089
lll->is_stop = 1U;
10901090
}
10911091

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

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,13 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param)
635635
lll = prepare_param->param;
636636
lll->skip_prepare += (lll->lazy_prepare + 1U);
637637

638-
/* Reset Sync context association with any Aux context as the chain reception is aborted. */
638+
/* Reset Sync context association with any Aux context as the chain reception is aborted.
639+
* There is no race with ULL execution context assigning lll_aux as this is prepare being
640+
* aborted and no assignment would be done yet before the superior PDU is received.
641+
* TODO: This code here is redundant as isr_rx_done_cleanup() should ensure it is reset
642+
* on every periodic sync chain done. Remove it if there is no regression running
643+
* conformance tests.
644+
*/
639645
lll->lll_aux = NULL;
640646

641647
/* Extra done event, to check sync lost */
@@ -819,6 +825,11 @@ static int isr_rx(struct lll_sync *lll, uint8_t node_type, uint8_t crc_ok,
819825
if (node_type != NODE_RX_TYPE_EXT_AUX_REPORT) {
820826
/* Reset Sync context association with any Aux context
821827
* as a new chain is being setup for reception here.
828+
* TODO: This code here is redundant as
829+
* isr_rx_done_cleanup() should ensure it is reset
830+
* on every periodic sync chain done. Remove it if
831+
* there is no regression running conformance
832+
* tests.
822833
*/
823834
lll->lll_aux = NULL;
824835

@@ -1189,7 +1200,11 @@ static void isr_rx_done_cleanup(struct lll_sync *lll, uint8_t crc_ok, bool sync_
11891200
{
11901201
struct event_done_extra *e;
11911202

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

11951210
/* Calculate and place the drift information in done event */

subsys/bluetooth/controller/ll_sw/ull_scan_aux.c

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1126,6 +1126,10 @@ void ull_scan_aux_release(memq_link_t *link, struct node_rx_pdu *rx)
11261126
param_ull = HDR_LLL2ULL(rx->rx_ftr.param);
11271127

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

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

1141+
/* Under race condition when LLL scheduling a reception of
1142+
* auxiliary PDU, a scan aux context may be assigned late and
1143+
* the node rx releasing the aux context will not have it.
1144+
* Release the scan aux context assigned in the scan context.
1145+
*/
1146+
if (!lll_aux) {
1147+
lll_aux = lll->lll_aux;
1148+
}
1149+
11371150
} else if (!IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC) ||
11381151
ull_scan_aux_is_valid_get(param_ull)) {
1152+
/* Release aux context when ULL scheduled auxiliary PDU
1153+
* reception is aborted.
1154+
*/
1155+
11391156
/* Mark for buffer for release */
11401157
rx->hdr.type = NODE_RX_TYPE_RELEASE;
11411158

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

1170+
/* Release aux context in case of chain PDU reception, otherwise
1171+
* lll_aux is NULL.
1172+
*/
11531173
lll = rx->rx_ftr.param;
11541174
lll_aux = rx->rx_ftr.lll_aux;
11551175

1176+
/* Under race condition when LLL scheduling a reception of
1177+
* auxiliary PDU, a scan aux context may be assigned late and
1178+
* the node rx releasing the aux context will not have it.
1179+
* Release the scan aux context assigned in the sync context.
1180+
*/
1181+
if (!lll_aux) {
1182+
lll_aux = lll->lll_aux;
1183+
}
1184+
11561185
/* Change node type so HCI can dispatch report for truncated
11571186
* data properly.
11581187
*/
@@ -1183,13 +1212,16 @@ void ull_scan_aux_release(memq_link_t *link, struct node_rx_pdu *rx)
11831212
scan = ull_scan_is_valid_get(scan);
11841213
if (scan) {
11851214
is_stop = scan->is_stop;
1186-
} else {
1215+
} else if (IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC)) {
11871216
struct lll_sync *sync_lll;
11881217
struct ll_sync_set *sync;
11891218

11901219
sync_lll = (void *)lll;
11911220
sync = HDR_LLL2ULL(sync_lll);
11921221
is_stop = sync->is_stop;
1222+
} else {
1223+
LL_ASSERT(0);
1224+
return;
11931225
}
11941226

11951227
if (!is_stop) {

0 commit comments

Comments
 (0)