tbf: Always start T3193 when changing state to GPRS_RLCMAC_WAIT_RELEASE

Currently when receiving a PACKET DL ACK/NACK message with the Final
Ack Indicator bit set, the TBF's state is set to
GPRS_RLCMAC_WAIT_RELEASE but T3193 is only started when the LLC queue is
empty. Otherwise the reuse_tbf() method is called to establish a new
DL TBF. In that case, the timer is not started. This will leave the
current TBF without a timer so it is potentially not released later
on.

This is recognisable by sticky entries in the output of the
'show tbf all' command and possibly allocation failures if there are
too many of them.

This commit changes the code to always start T3193 to make sure, that
a timer is always active when the the state is set to
GPRS_RLCMAC_WAIT_RELEASE.

Note that TS 44.060, 9.3.2.6 requests to release the 'old' TBF
immediately in some cases, which is not implemented by this change.
This will lead to a longer reservation period of the TFI only, which
is safer than reassigning it too early.

Sponsored-by: On-Waves ehf

3 files changed, 16 insertions, 13 deletions

diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 4739a500..01e6add0 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -821,20 +821,16 @@ int gprs_rlcmac_dl_tbf::maybe_start_new_window()
 
 	set_state(GPRS_RLCMAC_WAIT_RELEASE);
 
+	/* start T3193 */
+	tbf_timer_start(this, 3193,
+		bts_data()->t3193_msec / 1000,
+		(bts_data()->t3193_msec % 1000) * 1000);
+
 	/* check for LLC PDU in the LLC Queue */
-	if (!have_data()) {
-		/* no message, start T3193, change state to RELEASE */
-		LOGP(DRLCMACDL, LOGL_DEBUG, "- No new message, so we release.\n");
-		/* start T3193 */
-		tbf_timer_start(this, 3193,
-			bts_data()->t3193_msec / 1000,
-			(bts_data()->t3193_msec % 1000) * 1000);
-
-		return 0;
-	}
+	if (have_data())
+		/* we have more data so we will re-use this tbf */
+		reuse_tbf();
 
-	/* we have more data so we will re-use this tbf */
-	reuse_tbf();
 	return 0;
 }
 
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index 39f57ad8..80bc8188 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -45,6 +45,10 @@ int16_t spoof_mnc = 0, spoof_mcc = 0;
 static void check_tbf(gprs_rlcmac_tbf *tbf)
 {
 	OSMO_ASSERT(tbf);
+	if (tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE))
+		OSMO_ASSERT(tbf->T == 3191 || tbf->T == 3193);
+	if (tbf->state_is(GPRS_RLCMAC_RELEASING))
+		OSMO_ASSERT(tbf->T != 0);
 }
 
 /*
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index ee57d638..099504f8 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -80,6 +80,7 @@ Sending data request: trx=0 ts=4 sapi=5 arfcn=0 fn=8 block=2 data=07 00 03 14 15
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) downlink acknowledge
 - Final ACK received.
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) changes state from FLOW to WAIT RELEASE
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=WAIT RELEASE) starting timer 3193.
 ********** TBF starts here **********
 Allocating DL TBF: MS_CLASS=45
 Slot Allocation (Algorithm A) for class 45
@@ -102,6 +103,7 @@ TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=NULL) changes state from NULL to ASSIGN
 TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) starting timer 0.
 DL packet loss of IMSI= / TLLI=0x00000000: 0%
 TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE) free
+TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE) stopping timer 3193.
 PDCH(TS 4, TRX 0): Detaching TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE), 1 TBFs, USFs = 00, TFIs = 00000002.
 ********** TBF ends here **********
 TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) free
@@ -155,6 +157,7 @@ Sending data request: trx=0 ts=4 sapi=5 arfcn=0 fn=8 block=2 data=07 00 03 14 15
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) downlink acknowledge
 - Final ACK received.
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) changes state from FLOW to WAIT RELEASE
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=WAIT RELEASE) starting timer 3193.
 ********** TBF starts here **********
 Allocating DL TBF: MS_CLASS=45
 Slot Allocation (Algorithm A) for class 45
@@ -182,6 +185,7 @@ Detaching TBF from MS object, TLLI = 0xffeeddcc, TBF = TBF(TFI=1 TLLI=0xffeeddcc
 ********** TBF ends here **********
 DL packet loss of IMSI= / TLLI=0x00000000: 0%
 TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE) free
+TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE) stopping timer 3193.
 PDCH(TS 4, TRX 0): Detaching TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE), 0 TBFs, USFs = 00, TFIs = 00000000.
 ********** TBF ends here **********
 Destroying MS object, TLLI = 0xffeeddcc
@@ -405,7 +409,6 @@ Sending data request: trx=0 ts=4 sapi=5 arfcn=0 fn=203 block=23 data=07 01 2c 4d
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FINISHED) downlink acknowledge
 - Final ACK received.
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FINISHED) changes state from FINISHED to WAIT RELEASE
-- No new message, so we release.
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=WAIT RELEASE) starting timer 3193.
 DL packet loss of IMSI= / TLLI=0xffeeddcc: 0%
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=WAIT RELEASE) free