pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/33900 ) Change subject: rlcmac: recalculate CV as needed when already in Countdown procedure ...................................................................... rlcmac: recalculate CV as needed when already in Countdown procedure Getting out of contention resolution means we may have to update our calculated CV state because we are no longer sending TLLI. Same happens if a new tx CS is provided by the network, since different block size means different CV. In this commit only code paths for the state where already in Countdown Procedure are added. If TBF has to enter Countdown Procedure due the above mentioned changes, it will do so using regular path where a new RLC block is created. Related specs: TS 44.060 9.3.1 Related: OS#6018 Change-Id: I6ca88c005060ba1302d46717e45b0d9731d86d8d --- M include/osmocom/gprs/rlcmac/tbf_ul.h M src/rlcmac/tbf_ul.c M src/rlcmac/tbf_ul_fsm.c 3 files changed, 83 insertions(+), 16 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/include/osmocom/gprs/rlcmac/tbf_ul.h b/include/osmocom/gprs/rlcmac/tbf_ul.h index 660cf5a..fe9db64 100644 --- a/include/osmocom/gprs/rlcmac/tbf_ul.h +++ b/include/osmocom/gprs/rlcmac/tbf_ul.h @@ -52,6 +52,8 @@ struct gprs_rlcmac_ul_tbf *gprs_rlcmac_ul_tbf_alloc(struct gprs_rlcmac_entity *gre); void gprs_rlcmac_ul_tbf_free(struct gprs_rlcmac_ul_tbf *ul_tbf); +void gprs_rlcmac_ul_tbf_countdown_proc_update_cv(struct gprs_rlcmac_ul_tbf *ul_tbf); + bool gprs_rlcmac_ul_tbf_in_contention_resolution(const struct gprs_rlcmac_ul_tbf *ul_tbf); unsigned int gprs_rlcmac_ul_tbf_n3104_max(const struct gprs_rlcmac_ul_tbf *ul_tbf); bool gprs_rlcmac_ul_tbf_have_data(const struct gprs_rlcmac_ul_tbf *ul_tbf); diff --git a/src/rlcmac/tbf_ul.c b/src/rlcmac/tbf_ul.c index 0fa98ae..369be55 100644 --- a/src/rlcmac/tbf_ul.c +++ b/src/rlcmac/tbf_ul.c @@ -243,7 +243,13 @@ gprs_rlcmac_mcs_name(ul_tbf->tx_cs), gprs_rlcmac_mcs_name(tx_cs)); ul_tbf->tx_cs = tx_cs; - /* TODO: recalculate countdown_state, have to look a TS 44.060 specs on what to do exactly. */ + /* TS 44.060 9.3.1.2: If in Countdown Procedure state, CV needs to be + * recalculated since CS change means also block size change and hence + * the new CV != old CV (new CV may be greater or lesser than old CV). + * This means CV can go back to 15, but still be in Countdown Procedure, + * aka no new enqueued LLC data in the MS is to be transmitted until the + * current TBF finishes. */ + gprs_rlcmac_ul_tbf_countdown_proc_update_cv(ul_tbf); } int gprs_rlcmac_ul_tbf_handle_pkt_ul_ack_nack(struct gprs_rlcmac_ul_tbf *ul_tbf, @@ -517,7 +523,7 @@ static uint8_t gprs_rlcmac_ul_tbf_calculate_cv(const struct gprs_rlcmac_ul_tbf *ul_tbf) { struct blk_count_state st; - const struct gprs_rlcmac_llc_queue *q = ul_tbf->tbf.gre->llc_queue; + const struct gprs_rlcmac_llc_queue *q = gprs_rlcmac_ul_tbf_llc_queue(ul_tbf); unsigned int i, j; unsigned x; @@ -572,11 +578,22 @@ ul_tbf->tbf.gre->llc_queue = gprs_rlcmac_llc_queue_alloc(ul_tbf->tbf.gre); } -static void gprs_rlcmac_ul_tbf_check_countdown_proc(struct gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_rlcmac_rts_block_ind *bi) +/* Check if UL TBF needs to enter Countdown Procedure everytime a new RLC/MAC block is to be transmitted */ +static void gprs_rlcmac_ul_tbf_countdown_proc_check_enter(struct gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_rlcmac_rts_block_ind *bi) { - if (ul_tbf->countdown_proc.active) + if (ul_tbf->countdown_proc.active) { + /* This may happen if TBF entered Countdown Procedure state but + * later on due to CS change the CV incremented to more than BS_CV_MAX. + * In this case we cannot simply decrement the CV each time a + * new block is transmitted, but we rather need to keep + * calculating it here: + */ + if (ul_tbf->countdown_proc.cv == 15) + ul_tbf->countdown_proc.cv = gprs_rlcmac_ul_tbf_calculate_cv(ul_tbf); return; + } + /* Not (yet) in Countdown Procedure, check if we need to enter into it */ ul_tbf->countdown_proc.cv = gprs_rlcmac_ul_tbf_calculate_cv(ul_tbf); if (ul_tbf->countdown_proc.cv < 15) { if (gprs_rlcmac_ul_tbf_shall_keep_open(ul_tbf, bi)) { @@ -590,6 +607,18 @@ } } +/* Recalculate CV once in Countdown Procedure if conditions change (called by): + * - If contention resolution succeeds + * - If tx CS requested by network changes + */ +void gprs_rlcmac_ul_tbf_countdown_proc_update_cv(struct gprs_rlcmac_ul_tbf *ul_tbf) +{ + + if (!ul_tbf->countdown_proc.active) + return; + ul_tbf->countdown_proc.cv = gprs_rlcmac_ul_tbf_calculate_cv(ul_tbf); +} + static int create_new_bsn(struct gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_rlcmac_rts_block_ind *bi, enum gprs_rlcmac_coding_scheme cs) { const uint16_t bsn = gprs_rlcmac_rlc_ul_window_v_s(ul_tbf->ulw); @@ -600,7 +629,7 @@ int write_offset = 0; enum gpr_rlcmac_append_result ar; - gprs_rlcmac_ul_tbf_check_countdown_proc(ul_tbf, bi); + gprs_rlcmac_ul_tbf_countdown_proc_check_enter(ul_tbf, bi); if (!ul_tbf->llc_tx_msg || msgb_length(ul_tbf->llc_tx_msg) == 0) gprs_rlcmac_ul_tbf_schedule_next_llc_frame(ul_tbf); @@ -628,10 +657,17 @@ rdbi->data_len = block_data_len; rdbi->ti = gprs_rlcmac_ul_tbf_in_contention_resolution(ul_tbf); - rdbi->cv = ul_tbf->countdown_proc.cv--; + rdbi->cv = ul_tbf->countdown_proc.cv; rdbi->bsn = bsn; /* Block Sequence Number */ rdbi->e = 1; /* Extension bit, maybe set later (1: no extension) */ + /* Once we enter countdown procedure, simply decrement the counter to + * avoid recalculating all the time. */ + if (ul_tbf->countdown_proc.cv < 15) + ul_tbf->countdown_proc.cv--; + /* else: It will be updated in next call to + gprs_rlcmac_ul_tbf_countdown_proc_check_enter() above */ + if (rdbi->ti) { /* Append TLLI: */ if (gprs_rlcmac_mcs_is_gprs(cs)) diff --git a/src/rlcmac/tbf_ul_fsm.c b/src/rlcmac/tbf_ul_fsm.c index 2265aa4..7639d83 100644 --- a/src/rlcmac/tbf_ul_fsm.c +++ b/src/rlcmac/tbf_ul_fsm.c @@ -127,6 +127,20 @@ } } +static void _contention_resolution_succeeded(struct gprs_rlcmac_tbf_ul_fsm_ctx *ctx) +{ + LOGPFSML(ctx->fi, LOGL_INFO, "Contention resolution succeeded, stop T3166\n"); + OSMO_ASSERT(ctx->ul_tbf->ul_ass_fsm.ass_type == GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE); + OSMO_ASSERT(ctx->fi->T == 3166); + osmo_timer_del(&ctx->fi->timer); + ctx->fi->T = 0; + + /* TS 44.060 9.3.1.2: If in Countdown Procedure state, CV needs to be recalculated + * since TBF is no longer transmitting TLLI in the block, hence 4 more bytes per + * block are available. This means the new CV <= old CV. */ + gprs_rlcmac_ul_tbf_countdown_proc_update_cv(ctx->ul_tbf); +} + /* This one is triggered when packet access procedure fails, which can happen * either in WAIT_IMM_ASS (ImmAss timeout), FLOW (T3164) or FINISHED (T3164, T3166) */ static void st_new_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state) @@ -195,11 +209,7 @@ break; case GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK: if (gprs_rlcmac_ul_tbf_in_contention_resolution(ctx->ul_tbf)) { - LOGPFSML(ctx->fi, LOGL_INFO, "Contention resolution succeeded, stop T3166\n"); - OSMO_ASSERT(ctx->ul_tbf->ul_ass_fsm.ass_type == GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE); - OSMO_ASSERT(fi->T == 3166); - osmo_timer_del(&fi->timer); - fi->T = 0; + _contention_resolution_succeeded(ctx); } /* It's impossible we receive a correct final_ack here, since we didn't * sent last data (FSM would be in FINISHED state then) */ @@ -224,11 +234,7 @@ case GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK: ctx_ul_ack_nack = (struct tbf_ul_ass_ev_rx_ul_ack_nack *)data; if (gprs_rlcmac_ul_tbf_in_contention_resolution(ctx->ul_tbf)) { - LOGPFSML(ctx->fi, LOGL_INFO, "Contention resolution succeeded, stop T3166\n"); - OSMO_ASSERT(ctx->ul_tbf->ul_ass_fsm.ass_type == GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE); - OSMO_ASSERT(fi->T == 3166); - osmo_timer_del(&fi->timer); - fi->T = 0; + _contention_resolution_succeeded(ctx); } else if (fi->T == 3182 && osmo_timer_pending(&fi->timer)) { /* 9.3.3.3.2 "Upon reception of a PACKET UPLINK ACK/NACK message for this TBF * the mobile station shall stop timer T3182 for the TBF". -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/33900 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I6ca88c005060ba1302d46717e45b0d9731d86d8d Gerrit-Change-Number: 33900 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith@sysmocom.de> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-MessageType: merged