From 4cebbbccccfc21bcaf388bb4faf83f5d6a432fbc Mon Sep 17 00:00:00 2001 From: Andrzej Kaczmarek Date: Wed, 20 Mar 2024 02:19:12 +0100 Subject: [PATCH 1/3] nimble/ll: Reschedule preempted adv event if possible Currently we simply drop adv event if it was preempted, but instead we can try to reschedule it as longs as new start time does not exceed adv_delay limit. Note that we only reschedule if no PDU was sent in that event, otherwise we could not guarantee that aux is properly scheduled relative to exts. --- .../include/controller/ble_ll_adv.h | 2 +- nimble/controller/src/ble_ll_adv.c | 61 ++++++++++++------- nimble/controller/src/ble_ll_sched.c | 6 +- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/nimble/controller/include/controller/ble_ll_adv.h b/nimble/controller/include/controller/ble_ll_adv.h index b861d19dba..db35209dc1 100644 --- a/nimble/controller/include/controller/ble_ll_adv.h +++ b/nimble/controller/include/controller/ble_ll_adv.h @@ -168,7 +168,7 @@ int ble_ll_adv_can_chg_whitelist(void); * Called when an advertising event has been removed from the scheduler * without being run. */ -void ble_ll_adv_event_rmvd_from_sched(struct ble_ll_adv_sm *advsm); +void ble_ll_adv_preempted(struct ble_ll_adv_sm *advsm); /* * Called when a periodic event has been removed from the scheduler diff --git a/nimble/controller/src/ble_ll_adv.c b/nimble/controller/src/ble_ll_adv.c index 52606f93e6..8dc4444775 100644 --- a/nimble/controller/src/ble_ll_adv.c +++ b/nimble/controller/src/ble_ll_adv.c @@ -110,6 +110,7 @@ struct ble_ll_adv_sm uint8_t own_addr_type; uint8_t peer_addr_type; uint8_t adv_chan; + uint8_t retry_event; uint8_t adv_pdu_len; int8_t adv_rpa_index; int8_t tx_power; @@ -227,7 +228,7 @@ struct ble_ll_adv_sm struct ble_ll_adv_sm g_ble_ll_adv_sm[BLE_ADV_INSTANCES]; struct ble_ll_adv_sm *g_ble_ll_cur_adv_sm; -static void ble_ll_adv_drop_event(struct ble_ll_adv_sm *advsm); +static void ble_ll_adv_drop_event(struct ble_ll_adv_sm *advsm, bool preempted); static struct ble_ll_adv_sm * ble_ll_adv_sm_find_configured(uint8_t instance) @@ -1068,14 +1069,10 @@ ble_ll_adv_tx_done(void *arg) g_ble_ll_cur_adv_sm = NULL; } -/* - * Called when an advertising event has been removed from the scheduler - * without being run. - */ void -ble_ll_adv_event_rmvd_from_sched(struct ble_ll_adv_sm *advsm) +ble_ll_adv_preempted(struct ble_ll_adv_sm *advsm) { - ble_ll_adv_drop_event(advsm); + ble_ll_adv_drop_event(advsm, 1); } #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PERIODIC_ADV) @@ -4742,8 +4739,10 @@ ble_ll_adv_rx_isr_start(uint8_t pdu_type) } static void -ble_ll_adv_drop_event(struct ble_ll_adv_sm *advsm) +ble_ll_adv_drop_event(struct ble_ll_adv_sm *advsm, bool preempted) { + os_sr_t sr; + STATS_INC(ble_ll_stats, adv_drop_event); ble_ll_sched_rmv_elem(&advsm->adv_sch); @@ -4755,6 +4754,12 @@ ble_ll_adv_drop_event(struct ble_ll_adv_sm *advsm) advsm->aux_active = 0; #endif + if (preempted) { + OS_ENTER_CRITICAL(sr); + advsm->retry_event = !(advsm->flags & BLE_LL_ADV_SM_FLAG_ACTIVE_CHANSET_MASK); + OS_EXIT_CRITICAL(sr); + } + advsm->adv_chan = ble_ll_adv_final_chan(advsm); ble_ll_event_add(&advsm->adv_txdone_ev); } @@ -4779,7 +4784,7 @@ ble_ll_adv_reschedule_event(struct ble_ll_adv_sm *advsm) rc = ble_ll_sched_adv_reschedule(sch, max_delay_ticks); if (rc) { - ble_ll_adv_drop_event(advsm); + ble_ll_adv_drop_event(advsm, 0); return; } @@ -4845,25 +4850,35 @@ ble_ll_adv_done(struct ble_ll_adv_sm *advsm) final_adv_chan = ble_ll_adv_final_chan(advsm); if (advsm->adv_chan == final_adv_chan) { -#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) - if (advsm->events_max) { - advsm->events++; - } -#endif - ble_ll_scan_chk_resume(); /* This event is over. Set adv channel to first one */ advsm->adv_chan = ble_ll_adv_first_chan(advsm); - /* - * Calculate start time of next advertising event. NOTE: we do not - * add the random advDelay as the scheduling code will do that. - */ itvl = advsm->adv_itvl_usecs; tick_itvl = ble_ll_tmr_u2t(itvl); - advsm->adv_event_start_time += tick_itvl; - advsm->adv_pdu_start_time = advsm->adv_event_start_time; + + /* do not calculate new event time if current event should be retried; + * this happens if event was preempted, so we just try to schedule one + * more time with the same start time + */ + + if (!advsm->retry_event) { +#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) + if (advsm->events_max) { + advsm->events++; + } +#endif + + /* + * Calculate start time of next advertising event. NOTE: we do not + * add the random advDelay as the scheduling code will do that. + */ + advsm->adv_event_start_time += tick_itvl; + advsm->adv_pdu_start_time = advsm->adv_event_start_time; + } else { + advsm->retry_event = 0; + } /* * The scheduled time better be in the future! If it is not, we will @@ -4909,7 +4924,7 @@ ble_ll_adv_done(struct ble_ll_adv_sm *advsm) advsm->aux_active && LL_TMR_GT(advsm->adv_pdu_start_time, AUX_CURRENT(advsm)->start_time)) { - ble_ll_adv_drop_event(advsm); + ble_ll_adv_drop_event(advsm, 0); return; } #endif @@ -5009,7 +5024,7 @@ ble_ll_adv_sec_done(struct ble_ll_adv_sm *advsm) ble_ll_rfmgmt_release(); if (advsm->aux_dropped) { - ble_ll_adv_drop_event(advsm); + ble_ll_adv_drop_event(advsm, 0); return; } diff --git a/nimble/controller/src/ble_ll_sched.c b/nimble/controller/src/ble_ll_sched.c index bfec620d01..f2987dfdf0 100644 --- a/nimble/controller/src/ble_ll_sched.c +++ b/nimble/controller/src/ble_ll_sched.c @@ -151,9 +151,9 @@ ble_ll_sched_preempt(struct ble_ll_sched_item *sch, break; #endif #if MYNEWT_VAL(BLE_LL_ROLE_BROADCASTER) - case BLE_LL_SCHED_TYPE_ADV: - ble_ll_adv_event_rmvd_from_sched(entry->cb_arg); - break; + case BLE_LL_SCHED_TYPE_ADV: + ble_ll_adv_preempted(entry->cb_arg); + break; #endif #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) && MYNEWT_VAL(BLE_LL_ROLE_OBSERVER) case BLE_LL_SCHED_TYPE_SCAN_AUX: From 84c8912c942f0a961c1a022aeb80b0e7b9720171 Mon Sep 17 00:00:00 2001 From: Andrzej Kaczmarek Date: Wed, 20 Mar 2024 17:34:22 +0100 Subject: [PATCH 2/3] nimble/ll: Remove periodic adv min/max interval in sm We only need current interval, no need to store min and max (and min was never used anyway). --- nimble/controller/src/ble_ll_adv.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/nimble/controller/src/ble_ll_adv.c b/nimble/controller/src/ble_ll_adv.c index 8dc4444775..0ce13a6da8 100644 --- a/nimble/controller/src/ble_ll_adv.c +++ b/nimble/controller/src/ble_ll_adv.c @@ -159,8 +159,6 @@ struct ble_ll_adv_sm struct os_mbuf *periodic_new_data; uint32_t periodic_crcinit; /* only 3 bytes are used */ uint32_t periodic_access_addr; - uint16_t periodic_adv_itvl_min; - uint16_t periodic_adv_itvl_max; uint16_t periodic_adv_props; uint16_t periodic_channel_id; uint16_t periodic_event_cntr; @@ -171,8 +169,9 @@ struct ble_ll_adv_sm uint8_t periodic_sync_index : 1; uint8_t periodic_num_used_chans; uint8_t periodic_chanmap[BLE_LL_CHAN_MAP_LEN]; + uint16_t periodic_adv_itvl; uint32_t periodic_adv_itvl_ticks; - uint8_t periodic_adv_itvl_rem_usec; + uint8_t periodic_adv_itvl_rem_us; uint8_t periodic_adv_event_start_time_remainder; uint32_t periodic_adv_event_start_time; struct ble_ll_adv_sync periodic_sync[2]; @@ -665,7 +664,7 @@ ble_ll_adv_put_syncinfo(struct ble_ll_adv_sm *advsm, offset = ble_ll_tmr_t2u(anchor - AUX_CURRENT(advsm)->start_time); offset += advsm->periodic_adv_event_start_time_remainder; - offset += advsm->periodic_adv_itvl_rem_usec; + offset += advsm->periodic_adv_itvl_rem_us; } /* Sync Packet Offset (13 bits), Offset Units (1 bit), Offset Adjust (1 bit), @@ -696,7 +695,7 @@ ble_ll_adv_put_syncinfo(struct ble_ll_adv_sm *advsm, dptr[1] = ((offset >> 8) & 0x0000001f) | units; /* Interval (2 bytes) */ - put_le16(&dptr[2], advsm->periodic_adv_itvl_max); + put_le16(&dptr[2], advsm->periodic_adv_itvl); /* Channels Mask (37 bits) */ dptr[4] = advsm->periodic_chanmap[0]; @@ -2432,7 +2431,7 @@ ble_ll_adv_periodic_schedule_first(struct ble_ll_adv_sm *advsm, ble_ll_tmr_add_u(&advsm->periodic_adv_event_start_time, &advsm->periodic_adv_event_start_time_remainder, - advsm->periodic_adv_itvl_rem_usec); + advsm->periodic_adv_itvl_rem_us); sch->start_time = advsm->periodic_adv_event_start_time; sch->remainder = advsm->periodic_adv_event_start_time_remainder; @@ -2658,10 +2657,9 @@ ble_ll_adv_sm_start_periodic(struct ble_ll_adv_sm *advsm) (advsm->periodic_access_addr & 0x0000ffff); advsm->periodic_crcinit = ble_ll_rand() & 0xffffff; - usecs = (uint32_t)advsm->periodic_adv_itvl_max * BLE_LL_ADV_PERIODIC_ITVL; - + usecs = (uint32_t)advsm->periodic_adv_itvl * BLE_LL_ADV_PERIODIC_ITVL; advsm->periodic_adv_itvl_ticks = ble_ll_tmr_u2t_r(usecs, - &advsm->periodic_adv_itvl_rem_usec); + &advsm->periodic_adv_itvl_rem_us); /* There is no point in starting periodic advertising until next advertising * event since SyncInfo is needed for synchronization @@ -3975,8 +3973,7 @@ ble_ll_adv_periodic_set_param(const uint8_t *cmdbuf, uint8_t len) return BLE_ERR_PACKET_TOO_LONG; } - advsm->periodic_adv_itvl_min = adv_itvl_min; - advsm->periodic_adv_itvl_max = adv_itvl_max; + advsm->periodic_adv_itvl = adv_itvl_max; advsm->periodic_adv_props = props; ble_ll_adv_flags_set(advsm, BLE_LL_ADV_SM_FLAG_PERIODIC_CONFIGURED); @@ -4056,7 +4053,7 @@ ble_ll_adv_periodic_set_data(const uint8_t *cmdbuf, uint8_t len) */ if (!ble_ll_adv_periodic_check_data_itvl(payload_total_len, advsm->periodic_adv_props, - advsm->periodic_adv_itvl_max, + advsm->periodic_adv_itvl, advsm->sec_phy)) { return BLE_ERR_PACKET_TOO_LONG; } @@ -4149,7 +4146,7 @@ ble_ll_adv_periodic_enable(const uint8_t *cmdbuf, uint8_t len) */ if (!ble_ll_adv_periodic_check_data_itvl(SYNC_DATA_LEN(advsm), advsm->periodic_adv_props, - advsm->periodic_adv_itvl_max, + advsm->periodic_adv_itvl, advsm->sec_phy)) { return BLE_ERR_PACKET_TOO_LONG; } From eb5f48329735baabf6ff4ec4e4537e30f3d2be3b Mon Sep 17 00:00:00 2001 From: Andrzej Kaczmarek Date: Wed, 20 Mar 2024 17:35:01 +0100 Subject: [PATCH 3/3] nimble/ll: Fix sync packet offset in syncinfo We need to move periodic advertising event start time by full interval (not only ticks) as otherwise any calculation of start time in the future event will not be accurate. --- nimble/controller/src/ble_ll_adv.c | 31 ++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/nimble/controller/src/ble_ll_adv.c b/nimble/controller/src/ble_ll_adv.c index 0ce13a6da8..f194ec21c9 100644 --- a/nimble/controller/src/ble_ll_adv.c +++ b/nimble/controller/src/ble_ll_adv.c @@ -623,48 +623,51 @@ ble_ll_adv_put_syncinfo(struct ble_ll_adv_sm *advsm, uint8_t *dptr) { #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PERIODIC_ADV_SYNC_TRANSFER) - uint8_t anchor_usecs; uint16_t conn_cnt; #endif unsigned int event_cnt_off = 0; uint32_t offset = 0; - uint32_t anchor; + uint32_t itvl_us; + uint32_t anchor_ticks; + uint8_t anchor_rem_us; uint8_t units; if (connsm) { #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PERIODIC_ADV_SYNC_TRANSFER) - anchor = connsm->anchor_point; - anchor_usecs = connsm->anchor_point_usecs; + anchor_ticks = connsm->anchor_point; + anchor_rem_us = connsm->anchor_point_usecs; conn_cnt = connsm->event_cntr; /* get anchor for conn event that is before periodic_adv_event_start_time */ - while (LL_TMR_GT(anchor, advsm->periodic_adv_event_start_time)) { - ble_ll_conn_get_anchor(connsm, --conn_cnt, &anchor, &anchor_usecs); + while (LL_TMR_GT(anchor_ticks, advsm->periodic_adv_event_start_time)) { + ble_ll_conn_get_anchor(connsm, --conn_cnt, &anchor_ticks, &anchor_rem_us); } - offset = ble_ll_tmr_t2u(advsm->periodic_adv_event_start_time - anchor); - offset -= anchor_usecs; + offset = ble_ll_tmr_t2u(advsm->periodic_adv_event_start_time - anchor_ticks); + offset -= anchor_rem_us; offset += advsm->periodic_adv_event_start_time_remainder; /* connEventCount */ put_le16(conn_event_cnt, conn_cnt); #endif } else { - anchor = advsm->periodic_adv_event_start_time; + anchor_ticks = advsm->periodic_adv_event_start_time; + anchor_rem_us = advsm->periodic_adv_event_start_time_remainder; + itvl_us = advsm->periodic_adv_itvl * BLE_LL_ADV_PERIODIC_ITVL; /* Get periodic event that is past AUX start time (so that we always * provide valid offset if it is not too far in future). This can * happen if advertising event is interleaved with periodic advertising * event (when chaining). */ - while (LL_TMR_GT(AUX_CURRENT(advsm)->start_time, anchor)) { - anchor += advsm->periodic_adv_itvl_ticks; + while (LL_TMR_GEQ(AUX_CURRENT(advsm)->start_time, anchor_ticks)) { + ble_ll_tmr_add(&anchor_ticks, &anchor_rem_us, itvl_us); event_cnt_off++; } - offset = ble_ll_tmr_t2u(anchor - AUX_CURRENT(advsm)->start_time); - offset += advsm->periodic_adv_event_start_time_remainder; - offset += advsm->periodic_adv_itvl_rem_us; + /* We always schedule aux with 0 rem_us so no need to include it here */ + offset = ble_ll_tmr_t2u(anchor_ticks - AUX_CURRENT(advsm)->start_time); + offset += anchor_rem_us; } /* Sync Packet Offset (13 bits), Offset Units (1 bit), Offset Adjust (1 bit),