Differentiate between T3192 and T3193pespin/wip

T3192 on the MS side (boadcasted by the network on SI13) is used for the
MS to stay monitoring the PDCH of a DL TBF after sending PKT DL ACK/NACK
for the last DL data block (FinalAck=1).
T3193 is the counterpart in the network, defined as >T3192, which is
used to make sure the TFI is no longer used by the MS and hence can be
reused/re-assigned again.

Hence, we want to differentiate between those 2 timers, since the first
one gives us information on how late can we still tx PktDlAss on PACCH
on that DL TBF, while the later only provides info on when to free the
DL TBF in order to reuse the TFI.

Change-Id: I7421342461bf159d945651037e6fe690f88c6fae

5 files changed, 90 insertions, 23 deletions

diff --git a/src/gprs_ms.c b/src/gprs_ms.c
index 0b062c1c..dda2f290 100644
--- a/src/gprs_ms.c
+++ b/src/gprs_ms.c
@@ -1368,20 +1368,31 @@ int ms_append_llc_dl_data(struct GprsMs *ms, uint16_t pdu_delay_csec, const uint
 
 	dl_tbf = ms_dl_tbf(ms);
 	if (dl_tbf) {
-		if (tbf_state(dl_tbf_as_tbf_const(dl_tbf)) == TBF_ST_WAIT_RELEASE) {
-			LOGPTBFDL(dl_tbf, LOGL_DEBUG, "in WAIT RELEASE state (T3193), so reuse TBF\n");
+		switch (tbf_state(dl_tbf_as_tbf_const(dl_tbf))) {
+		case TBF_ST_WAIT_RELEASE:
+			LOGPTBFDL(dl_tbf, LOGL_DEBUG, "in WAIT RELEASE state (T3192), so reuse TBF\n");
 			rc = ms_new_dl_tbf_assigned_on_pacch(ms, dl_tbf_as_tbf(dl_tbf));
+			return rc;
+		case TBF_ST_WAIT_REUSE_TFI:
+			/* According to DL TBF state it should be back to CCCH, let's check UL TBF to have more information. */
+			break;
+		case TBF_ST_RELEASING:
+			/* Something went wrong (T3195), delay for later. */
+		default:
+			/* DL TBF in working status (do nothing)*/
+			return 0;
 		}
-	} else {
-		/* Check if we can create a DL TBF to start sending the enqueued
-		 * data. Otherwise it will be triggered later when it is reachable
-		 * again. */
-		if (ms_is_reachable_for_dl_ass(ms)) {
-			if (ms_ul_tbf(ms))
-				rc = ms_new_dl_tbf_assigned_on_pacch(ms, ul_tbf_as_tbf(ms_ul_tbf(ms)));
-			else
-				rc = ms_new_dl_tbf_assigned_on_pch(ms);
-		}
+
+	}
+
+	/* Check if we can create a DL TBF to start sending the enqueued
+	 * data. Otherwise it will be triggered later when it is reachable
+	 * again. */
+	if (ms_is_reachable_for_dl_ass(ms)) {
+		if (ms_ul_tbf(ms))
+			rc = ms_new_dl_tbf_assigned_on_pacch(ms, ul_tbf_as_tbf(ms_ul_tbf(ms)));
+		else
+			rc = ms_new_dl_tbf_assigned_on_pch(ms);
 	}
 	return rc;
 }
diff --git a/src/pdch.cpp b/src/pdch.cpp
index 8a2ad96e..7dfd3c29 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -429,8 +429,9 @@ void gprs_rlcmac_pdch::rcv_control_ack(Packet_Control_Acknowledgement_t *packet,
 				"TBF is gone TLLI=0x%08x\n", tlli);
 			return;
 		}
-		if (tbf->state_is(TBF_ST_WAIT_RELEASE) &&
-				tbf->direction == new_tbf->direction)
+		if ((tbf->state_is(TBF_ST_WAIT_RELEASE) ||
+		     tbf->state_is(TBF_ST_WAIT_REUSE_TFI)) &&
+		    tbf->direction == new_tbf->direction)
 			tbf_free(tbf);
 
 		osmo_fsm_inst_dispatch(new_tbf->state_fi, TBF_EV_ASSIGN_ACK_PACCH, NULL);
diff --git a/src/tbf_dl_fsm.c b/src/tbf_dl_fsm.c
index ebcd8b8a..2877e218 100644
--- a/src/tbf_dl_fsm.c
+++ b/src/tbf_dl_fsm.c
@@ -34,7 +34,8 @@ static const struct osmo_tdef_state_timeout tbf_dl_fsm_timeouts[32] = {
 	[TBF_ST_ASSIGN] = {},
 	[TBF_ST_FLOW] = {},
 	[TBF_ST_FINISHED] = {},
-	[TBF_ST_WAIT_RELEASE] = { .T = 3193 },
+	[TBF_ST_WAIT_RELEASE] = { .T = 3192 },
+	[TBF_ST_WAIT_REUSE_TFI] = { /* .T = 3193 set manually onenter subtracting T3192 */ },
 	[TBF_ST_RELEASING] = { .T = 3195 },
 };
 
@@ -243,8 +244,9 @@ static void st_finished(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 		break;
 	case TBF_EV_FINAL_ACK_RECVD:
 		/* We received Final Ack (DL ACK/NACK) from MS. move to
-		 * WAIT_RELEASE, we wait there for release or re-use the TBF in
-		 * case we receive more DL data to tx */
+		 * WAIT_RELEASE, where MS stays monitoring PDCH over T3192 span,
+		 * where we can use this DL TBF to assign a new one in case we
+		 * receive more DL data to Tx */
 		tbf_dl_fsm_state_chg(fi, TBF_ST_WAIT_RELEASE);
 		break;
 	case TBF_EV_MAX_N3105:
@@ -259,6 +261,14 @@ static void st_wait_release_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_sta
 {
 	struct tbf_dl_fsm_ctx *ctx = (struct tbf_dl_fsm_ctx *)fi->priv;
 
+	/* T3192 is running on the MS and has also been armed by this FSM now.
+	 * During that time, it is possible to reach the MS over PACCH to assign
+	 * new DL TBF.
+	 * Upon T3192 expiration, FSM wil transition to TBF_ST_WAIT_REUSE_TFI
+	 * for some more time (T3193 - T3192) imtil internally freeing the TBF
+	 * object, at which time the resources can be reused.
+	 */
+
 	mod_ass_type(ctx, GPRS_RLCMAC_FLAG_CCCH, false);
 }
 
@@ -266,10 +276,41 @@ static void st_wait_release(struct osmo_fsm_inst *fi, uint32_t event, void *data
 {
 	switch (event) {
 	case TBF_EV_FINAL_ACK_RECVD:
-		/* ignore, duplicate ACK, we already know about since we are in WAIT_RELEASE */
+		/* ignore, duplicate ACK, we already know about since we left ST_FINISHED */
 		break;
-	case TBF_EV_MAX_N3105:
-		tbf_dl_fsm_state_chg(fi, TBF_ST_RELEASING);
+	default:
+		OSMO_ASSERT(0);
+	}
+}
+
+static void st_wait_reuse_tfi_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
+{
+	struct tbf_dl_fsm_ctx *ctx = (struct tbf_dl_fsm_ctx *)fi->priv;
+	struct GprsMs *ms = tbf_ms(ctx->tbf);
+	unsigned long t3192, t3193, res;
+
+	/* T3192 has expired, so the MS is not listening on that PACCH anymore.
+	 * Still, wait until remaining of T3193 expiration (>T3192) to internally
+	 * free the TBF, at which point the TFI and other allocated resources
+	 * will be freed and can then be reused.
+	 */
+
+	t3192 = osmo_tdef_get(ms->bts->T_defs_bts, 3192, OSMO_TDEF_MS, -1);
+	t3193 = osmo_tdef_get(ms->bts->T_defs_bts, 3193, OSMO_TDEF_MS, -1);
+	/* As per spec T3193 shall be greater than T3192, but let's be safe against wrong configs: */
+	res = (t3193 >= t3192) ? (t3193 - t3192) : 0;
+	fi->T = 3193;
+	LOGPTBF(ctx->tbf, LOGL_DEBUG, "Waiting %lu sec. %lu microsec (T3193 - T3192) [REUSE TFI]\n",
+		res / 1000, (res % 1000) * 1000);
+	osmo_timer_schedule(&fi->timer, res / 1000, (res % 1000) * 1000);
+}
+
+static void st_wait_reuse_tfi(struct osmo_fsm_inst *fi, uint32_t event, void *data)
+{
+	/* Simply wait for T3193 timeout, it will tbf_free() */
+	switch (event) {
+	case TBF_EV_FINAL_ACK_RECVD:
+		/* ignore, duplicate ACK, we already know about since we left ST_FINISHED */
 		break;
 	default:
 		OSMO_ASSERT(0);
@@ -334,6 +375,9 @@ static int tbf_dl_fsm_timer_cb(struct osmo_fsm_inst *fi)
 	case -2002:
 		handle_timeout_X2002(fi);
 		break;
+	case 3192:
+		tbf_dl_fsm_state_chg(fi, TBF_ST_WAIT_REUSE_TFI);
+		break;
 	case -2001:
 		LOGPTBFDL(ctx->dl_tbf, LOGL_NOTICE, "releasing due to PACCH assignment timeout.\n");
 		/* fall-through */
@@ -401,11 +445,21 @@ static struct osmo_fsm_state tbf_dl_fsm_states[] = {
 			X(TBF_EV_FINAL_ACK_RECVD) |
 			X(TBF_EV_MAX_N3105),
 		.out_state_mask =
+			X(TBF_ST_WAIT_REUSE_TFI) |
 			X(TBF_ST_RELEASING),
 		.name = "WAIT_RELEASE",
 		.action = st_wait_release,
 		.onenter = st_wait_release_on_enter,
 	},
+	[TBF_ST_WAIT_REUSE_TFI] = {
+		.in_event_mask =
+			X(TBF_EV_FINAL_ACK_RECVD),
+		.out_state_mask =
+			X(TBF_ST_RELEASING),
+		.name = "WAIT_REUSE_TFI",
+		.action = st_wait_reuse_tfi,
+		.onenter = st_wait_reuse_tfi_on_enter,
+	},
 	[TBF_ST_RELEASING] = {
 		.in_event_mask =
 			X(TBF_EV_DL_ACKNACK_MISS),
diff --git a/src/tbf_fsm.h b/src/tbf_fsm.h
index 999b3c0e..fd6ffa6f 100644
--- a/src/tbf_fsm.h
+++ b/src/tbf_fsm.h
@@ -52,7 +52,8 @@ enum tbf_fsm_states {
 	TBF_ST_ASSIGN,	/* wait for downlink assignment */
 	TBF_ST_FLOW,	/* RLC/MAC flow, resource needed */
 	TBF_ST_FINISHED,	/* flow finished, wait for release */
-	TBF_ST_WAIT_RELEASE,/* DL TBF: wait for release or restart */
+	TBF_ST_WAIT_RELEASE,	/* DL TBF: wait for release or restart */
+	TBF_ST_WAIT_REUSE_TFI,	/* DL TBF: wait to reuse TFI after last PKT ACK/NACK */
 	TBF_ST_RELEASING,	/* releasing, wait to free TFI/USF */
 };
 
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index a78e2044..83bf6db0 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -295,7 +295,7 @@ DL_TBF(DL:TFI-0-0-0:G){FINISHED}: Received Event FINAL_ACK_RECVD
 DL_TBF(DL:TFI-0-0-0:G){FINISHED}: state_chg to WAIT_RELEASE
 TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} unset ass. type CCCH [prev CCCH:1, PACCH:0]
 MS(TLLI-0xffeeddcc:TA-0:MSCLS-45-0:DL) appending 200 bytes to DL LLC queue
-TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} in WAIT RELEASE state (T3193), so reuse TBF
+TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} in WAIT RELEASE state (T3192), so reuse TBF
 [DL] algo A <multi> (suggested TRX: 0): Alloc start
 - Skipping TS 0, because not enabled
 - Skipping TS 1, because not enabled
@@ -573,7 +573,7 @@ DL_TBF(DL:TFI-0-0-0:G){FINISHED}: Received Event FINAL_ACK_RECVD
 DL_TBF(DL:TFI-0-0-0:G){FINISHED}: state_chg to WAIT_RELEASE
 TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} unset ass. type CCCH [prev CCCH:1, PACCH:0]
 MS(TLLI-0xffeeddcc:TA-0:MSCLS-45-0:DL) appending 200 bytes to DL LLC queue
-TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} in WAIT RELEASE state (T3193), so reuse TBF
+TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} in WAIT RELEASE state (T3192), so reuse TBF
 [DL] algo A <multi> (suggested TRX: 0): Alloc start
 - Skipping TS 0, because not enabled
 - Skipping TS 1, because not enabled