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

3 files changed, 60 insertions, 16 deletions

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 {
 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 @@ static void gprs_rlcmac_ul_tbf_update_tx_cs(struct gprs_rlcmac_ul_tbf *ul_tbf, e
 		  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 @@ _* return 0: check blk_count_to_x(st) */
 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 @@ static void gprs_rlcmac_ul_tbf_steal_llc_queue_from_gre(struct gprs_rlcmac_ul_tb
 	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 @@ static void gprs_rlcmac_ul_tbf_check_countdown_proc(struct gprs_rlcmac_ul_tbf *u
 	}
 }
 
+/* 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 @@ static int create_new_bsn(struct gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_r
 	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 @@ static int create_new_bsn(struct gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_r
 	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 arm_T3182_if_needed(struct gprs_rlcmac_tbf_ul_fsm_ctx *ctx)
 	}
 }
 
+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 @@ static void st_flow(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 		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 @@ static void st_finished(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 	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".