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 <[email protected]>

Author: cvinayak

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

@@ -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;
 

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

@@ -635,7 +635,13 @@ 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. */
+	/* Reset Sync context association with any Aux context as the chain reception is aborted.
+	 * There is no race with ULL execution context assigning lll_aux as this is prepare being
+	 * aborted and no assignment would be done yet before the superior PDU is received.
+	 * TODO: This code here is redundant as isr_rx_done_cleanup() should ensure it is reset
+	 *       on every periodic sync chain done. Remove it if there is no regression running
+	 *       conformance tests.
+	 */
 	lll->lll_aux = NULL;
 
 	/* 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,
 		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.
+			 * TODO: This code here is redundant as
+			 *       isr_rx_done_cleanup() should ensure it is reset
+			 *       on every periodic sync chain done. Remove it if
+			 *       there is no regression running conformance
+			 *       tests.
 			 */
 			lll->lll_aux = NULL;
 
@@ -1189,7 +1200,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 */

subsys/bluetooth/controller/ll_sw/ull_scan_aux.c

@@ -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 */
@@ -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;
 
@@ -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.
 		 */
@@ -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) {