Skip to content

Commit fc14570

Browse files
cvinayakkartben
authored andcommitted
Bluetooth: Controller: Fix Central CIS offset for dissimilar intervals
Fix Central CIS offset calculation for dissimilar ACL and ISO intervals in use. Mayfly execution of `mfy_cig_offset_get()` could be after "LLL Prepare" or before depending on whether a previous radio event is being preempted or not, respectively; the `conn->lll.event_counter` may not be pre-incremented. This race condition is fixed by the fact that we use a constant instant delta value now. Dissimilar ACL and ISO intervals may lead to ACL overlapping or be too close to ISO event, causing preemption; under this case ACLs "LLL Prepare" would run after `mfy_cig_offset_get` causing incorrect calculation of CIS offset without the fix. Remove redundant `instant` member in `ll_conn_iso_stream` structure as a constant CIS Create instant delta is now used. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
1 parent 153be1b commit fc14570

File tree

2 files changed

+16
-23
lines changed

2 files changed

+16
-23
lines changed

subsys/bluetooth/controller/ll_sw/ull_central_iso.c

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@
6565
#define PHY_VALID_MASK (BT_HCI_ISO_PHY_VALID_MASK & ~BIT(2))
6666
#endif
6767

68+
/* CIS Create Procedure uses 3 PDU transmissions, and one connection interval to process the LLCP
69+
* requested, hence minimum relative instant not be less than 4. I.e. the CIS_REQ PDU will be
70+
* transmitted in the next ACL interval.
71+
* The +1 also helps with the fact that currently we do not have Central implementation to handle
72+
* event latencies at the instant. Refer to `ull_conn_iso_start()` implementation.
73+
*/
74+
#define CIS_CREATE_INSTANT_DELTA_MIN 4U
75+
6876
#if (CONFIG_BT_CTLR_CENTRAL_SPACING == 0)
6977
static void cig_offset_get(struct ll_conn_iso_stream *cis);
7078
static void mfy_cig_offset_get(void *param);
@@ -925,10 +933,10 @@ uint8_t ull_central_iso_setup(uint16_t cis_handle,
925933
cis->lll.prepared = 0U;
926934
#endif /* !CONFIG_BT_CTLR_JIT_SCHEDULING */
927935

928-
cis->central.instant = instant;
929936
#if defined(CONFIG_BT_CTLR_ISOAL_PSN_IGNORE)
930937
cis->pkt_seq_num = 0U;
931938
#endif /* CONFIG_BT_CTLR_ISOAL_PSN_IGNORE */
939+
932940
/* It is intentional to initialize to the 39 bit maximum value and rollover to 0 in the
933941
* prepare function, the event counter is pre-incremented in prepare function for the
934942
* current ISO event.
@@ -975,19 +983,11 @@ int ull_central_iso_cis_offset_get(uint16_t cis_handle,
975983

976984
conn = ll_conn_get(cis->lll.acl_handle);
977985

978-
/* NOTE: CIS Create Procedure uses 3 PDU transmissions, hence minimum relative instant not
979-
* be less than 3. As the CIS_REQ PDU will be transmitted in the next ACL interval,
980-
* add +1 to the instant. The +1 also helps with the fact that currently we do not
981-
* have Central implementation to handle event latencies at the instant. Refer to
982-
* `ull_conn_iso_start()` implementation.
983-
*
984-
* `ull_conn_llcp()` is called before `ull_ref_inc()` hence we do not need to use
985-
* `ull_conn_event_counter()`.
986+
/* `ull_conn_llcp()` (caller of this function) is called before `ull_ref_inc()` hence we do
987+
* not need to use `ull_conn_event_counter()`.
986988
*/
987-
cis->central.instant = conn->lll.event_counter + conn->lll.latency_prepare +
988-
conn->llcp.prep.lazy + 4U;
989-
990-
*conn_event_count = cis->central.instant;
989+
*conn_event_count = conn->lll.event_counter + conn->lll.latency_prepare +
990+
conn->llcp.prep.lazy + CIS_CREATE_INSTANT_DELTA_MIN;
991991

992992
/* Provide CIS offset range
993993
* CIS_Offset_Max < (connInterval - (CIG_Sync_Delay + T_MSS))
@@ -1090,18 +1090,18 @@ static void cis_offset_get(struct ll_conn_iso_stream *cis)
10901090
static void mfy_cis_offset_get(void *param)
10911091
{
10921092
uint32_t elapsed_acl_us, elapsed_cig_us;
1093-
uint16_t latency_acl, latency_cig;
10941093
struct ll_conn_iso_stream *cis;
10951094
struct ll_conn_iso_group *cig;
10961095
uint32_t cig_remainder_us;
10971096
uint32_t acl_remainder_us;
10981097
uint32_t cig_interval_us;
10991098
uint32_t offset_limit_us;
11001099
uint32_t ticks_to_expire;
1100+
uint32_t remainder = 0U;
11011101
uint32_t ticks_current;
11021102
uint32_t offset_min_us;
11031103
struct ll_conn *conn;
1104-
uint32_t remainder = 0U;
1104+
uint16_t latency_cig;
11051105
uint8_t ticker_id;
11061106
uint16_t lazy;
11071107
uint8_t retry;
@@ -1188,13 +1188,7 @@ static void mfy_cis_offset_get(void *param)
11881188
* and latency counts (typically 3) is low enough to avoid 32-bit
11891189
* overflow. Refer to ull_central_iso_cis_offset_get().
11901190
*/
1191-
/* FIXME: Mayfly execution of `mfy_cig_offset_get()` could be before "LLL Prepare" or after
1192-
* conn->lll.event_counter could have been pre-incremented.
1193-
* This race condition needs a fix.
1194-
*/
1195-
latency_acl = cis->central.instant - conn->lll.event_counter - conn->lll.latency_prepare -
1196-
conn->llcp.prep.lazy;
1197-
elapsed_acl_us = latency_acl * conn->lll.interval * CONN_INT_UNIT_US;
1191+
elapsed_acl_us = CIS_CREATE_INSTANT_DELTA_MIN * conn->lll.interval * CONN_INT_UNIT_US;
11981192

11991193
/* Calculate elapsed CIG intervals until the instant */
12001194
cig_interval_us = cig->iso_interval * ISO_INT_UNIT_US;

subsys/bluetooth/controller/ll_sw/ull_conn_iso_types.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ struct ll_conn_iso_stream {
3434
struct {
3535
uint8_t c_rtn;
3636
uint8_t p_rtn;
37-
uint16_t instant;
3837
} central;
3938
};
4039

0 commit comments

Comments
 (0)