[PATCH] osmo-pcu[master]: TBF: implement independent T31xx timers

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Max gerrit-no-reply at lists.osmocom.org
Thu Dec 7 14:07:56 UTC 2017


Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/5158

to look at the new patch set (#3).

TBF: implement independent T31xx timers

Previously TBF got single timer so the pending timer was automatically
cancelled when new one was scheduled. Let's make it more robust by
implementing independent T31 xx timers from 3GPP TS 44.060 §13.2 with
corresponding start/stop functions and counters.

The semantics of the timers is preserved as before: pending timers are
restarted unconditionally. It might be neecessary to change this later on
after spec review.

N. B. T0: used for assign/reject timeouts, have to be properly
attributed and documented first.

The test output tweaking is required due to T3169 change.

Change-Id: I0305873ca47534f53441247217881da59625e1f7
Related: OS#2407
---
M src/bts.cpp
M src/tbf.cpp
M src/tbf.h
M src/tbf_dl.cpp
M src/tbf_ul.cpp
M tests/tbf/TbfTest.cpp
M tests/tbf/TbfTest.err
7 files changed, 171 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/58/5158/3

diff --git a/src/bts.cpp b/src/bts.cpp
index 341c9d4..9a559f9 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -681,7 +681,7 @@
 			tbf->set_ta(ta);
 			tbf->set_state(GPRS_RLCMAC_FLOW);
 			tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_CCCH);
-			tbf_timer_start(tbf, 3169, m_bts.t3169, 0, "RACH (new UL-TBF)");
+			tbf->t_start(T3169, m_bts.t3169, 0, "RACH (new UL-TBF)", true);
 			LOGP(DRLCMAC, LOGL_DEBUG, "%s [UPLINK] START\n",
 					tbf_name(tbf));
 			LOGP(DRLCMAC, LOGL_DEBUG, "%s RX: [PCU <- BTS] RACH "
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 7b609c8..8bd0d29 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -32,6 +32,7 @@
 
 extern "C" {
 #include <osmocom/core/msgb.h>
+#include <osmocom/core/utils.h>
 #include <osmocom/core/talloc.h>
 #include <osmocom/core/stats.h>
 }
@@ -56,6 +57,14 @@
 	OSMO_VALUE_STRING(GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ),
 	OSMO_VALUE_STRING(GPRS_RLCMAC_UL_ASS_WAIT_ACK),
  	{ 0, NULL }
+};
+
+static const struct value_string tbf_timers_names[] = {
+	OSMO_VALUE_STRING(T3169),
+	OSMO_VALUE_STRING(T3191),
+	OSMO_VALUE_STRING(T3193),
+	OSMO_VALUE_STRING(T3195),
+	{ 0, NULL }
 };
 
 static const struct rate_ctr_desc tbf_ctr_description[] = {
@@ -183,6 +192,7 @@
 	 * Just set them to 0 like talloc_zero did */
 	memset(&pdch, 0, sizeof(pdch));
 	memset(&timer, 0, sizeof(timer));
+	memset(&T31, 0, sizeof(T31));
 	memset(&m_rlc, 0, sizeof(m_rlc));
 	memset(&gsm_timer, 0, sizeof(gsm_timer));
 
@@ -394,7 +404,7 @@
 	tbf->m_contention_resolution_done = 1;
 	tbf->set_state(GPRS_RLCMAC_ASSIGN);
 	tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
-	tbf_timer_start(tbf, 3169, bts->t3169, 0, "allocation (UL-TBF)");
+	tbf->t_start(T3169, bts->t3169, 0, "allocation (UL-TBF)", true);
 	tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
 	OSMO_ASSERT(tbf->ms());
 
@@ -465,6 +475,7 @@
 		     get_value_string(gprs_rlcmac_tbf_dl_ass_state_names,
 				      tbf->dl_ass_state));
 	tbf->stop_timer("freeing TBF");
+	tbf->stop_timers("freeing TBF");
 	/* TODO: Could/Should generate  bssgp_tx_llc_discarded */
 	tbf_unlink_pdch(tbf);
 	llist_del(&tbf->list());
@@ -550,9 +561,24 @@
 	osmo_timer_schedule(&tbf->timer, seconds, microseconds);
 }
 
-void gprs_rlcmac_tbf::stop_t3191()
+void gprs_rlcmac_tbf::t_stop(enum tbf_timers t, const char *reason)
 {
-	return stop_timer("T3191");
+	if (t >= T_MAX) {
+		LOGP(DRLCMAC, LOGL_ERROR, "%s attempting to stop unknown timer %s [%s]\n",
+		     tbf_name(this), get_value_string(tbf_timers_names, t), reason);
+	}
+
+	if (osmo_timer_pending(&T31[t])) {
+		LOGP(DRLCMAC, LOGL_DEBUG, "%s stopping timer %s [%s]\n",
+		     tbf_name(this), get_value_string(tbf_timers_names, t), reason);
+		osmo_timer_del(&T31[t]);
+	}
+}
+
+void gprs_rlcmac_tbf::stop_timers(const char *reason) {
+	uint8_t i;
+	for (i = 0; i < T_MAX; i++)
+		t_stop((enum tbf_timers)i, reason);
 }
 
 void gprs_rlcmac_tbf::stop_timer(const char *reason)
@@ -562,6 +588,61 @@
 		     tbf_name(this), T, reason);
 		osmo_timer_del(&timer);
 	}
+}
+
+static inline void tbf_timeout_free(struct gprs_rlcmac_tbf *tbf, enum tbf_timers t, bool run_diag)
+{
+	LOGP(DRLCMAC, LOGL_NOTICE, "%s %s timeout expired, freeing TBF\n",
+	     tbf_name(tbf), get_value_string(tbf_timers_names, t));
+
+	if (run_diag)
+		tbf->rlcmac_diag();
+
+	tbf_free(tbf);
+}
+
+#define T_CBACK(t, diag) static void cb_##t(void *_tbf) { tbf_timeout_free((struct gprs_rlcmac_tbf *)_tbf, t, diag); }
+
+T_CBACK(T3169, true)
+T_CBACK(T3191, true)
+T_CBACK(T3193, false)
+T_CBACK(T3195, true)
+
+void gprs_rlcmac_tbf::t_start(enum tbf_timers t, uint32_t sec, uint32_t microsec, const char *reason, bool force)
+{
+	if (t >= T_MAX) {
+		LOGP(DRLCMAC, LOGL_ERROR, "%s attempting to start unknown timer %s [%s]\n",
+		     tbf_name(this), get_value_string(tbf_timers_names, t), reason);
+	}
+
+	if (!force && osmo_timer_pending(&T31[t]))
+		return;
+
+	LOGP(DRLCMAC, LOGL_DEBUG, "%s %sstarting timer %s [%s] with %u sec. %u microsec.\n",
+	     tbf_name(this), osmo_timer_pending(&T31[t]) ? "re" : "",
+	     get_value_string(tbf_timers_names, t), reason, sec, microsec);
+
+	T31[t].data = this;
+
+	switch(t) {
+	case T3169:
+		T31[t].cb = cb_T3169;
+		break;
+	case T3191:
+		T31[t].cb = cb_T3191;
+		break;
+	case T3193:
+		T31[t].cb = cb_T3193;
+		break;
+	case T3195:
+		T31[t].cb = cb_T3195;
+		break;
+	default:
+		LOGP(DRLCMAC, LOGL_ERROR, "%s attempting to set callback for unknown timer %s [%s]\n",
+		     tbf_name(this), get_value_string(tbf_timers_names, t), reason);
+	}
+
+	osmo_timer_schedule(&T31[t], sec, microsec);
 }
 
 int gprs_rlcmac_tbf::check_polling(uint32_t fn, uint8_t ts,
@@ -663,7 +744,7 @@
 				     "- N3103 exceeded\n");
 				bts->pkt_ul_ack_nack_poll_failed();
 				ul_tbf->set_state(GPRS_RLCMAC_RELEASING);
-				tbf_timer_start(ul_tbf, 3169, ul_tbf->bts->bts_data()->t3169, 0, "MAX N3103 reached");
+				ul_tbf->t_start(T3169, ul_tbf->bts->bts_data()->t3169, 0, "MAX N3103 reached", false);
 				return;
 			}
 			/* reschedule UL ack */
@@ -685,7 +766,7 @@
 		if (n3105 == bts_data()->n3105) {
 			LOGP(DRLCMAC, LOGL_NOTICE, "- N3105 exceeded\n");
 			set_state(GPRS_RLCMAC_RELEASING);
-			tbf_timer_start(this, 3195, bts_data()->t3195, 0, "MAX N3105 reached");
+			t_start(T3195, bts_data()->t3195, 0, "MAX N3105 reached", true);
 			bts->rlc_ass_failed();
 			bts->pua_poll_failed();
 			return;
@@ -707,7 +788,7 @@
 		if (n3105 == bts->bts_data()->n3105) {
 			LOGP(DRLCMAC, LOGL_NOTICE, "- N3105 exceeded\n");
 			set_state(GPRS_RLCMAC_RELEASING);
-			tbf_timer_start(this, 3195, bts_data()->t3195, 0, "MAX N3105 reached");
+			t_start(T3195, bts_data()->t3195, 0, "MAX N3105 reached", true);
 			bts->rlc_ass_failed();
 			bts->pda_poll_failed();
 			return;
@@ -733,7 +814,7 @@
 		if (dl_tbf->n3105 == dl_tbf->bts->bts_data()->n3105) {
 			LOGP(DRLCMAC, LOGL_NOTICE, "- N3105 exceeded\n");
 			dl_tbf->set_state(GPRS_RLCMAC_RELEASING);
-			tbf_timer_start(dl_tbf, 3195, dl_tbf->bts_data()->t3195, 0, "MAX N3105 reached");
+			dl_tbf->t_start(T3195, dl_tbf->bts_data()->t3195, 0, "MAX N3105 reached", true);
 			bts->pkt_dl_ack_nack_poll_failed();
 			bts->rlc_ack_failed();
 			return;
@@ -1004,6 +1085,7 @@
 	tbf->handle_timeout();
 }
 
+/* FIXME: remove this once the switch over to t_start*()/t_stop*() is complete */
 void gprs_rlcmac_tbf::handle_timeout()
 {
 	LOGP(DRLCMAC, LOGL_DEBUG, "%s timer %u expired.\n",
@@ -1011,64 +1093,51 @@
 
 	num_T_exp++;
 
-	switch (T) {
-	case 0: /* assignment */
-		if ((state_flags & (1 << GPRS_RLCMAC_FLAG_PACCH))) {
-			if (state_is(GPRS_RLCMAC_ASSIGN)) {
-				LOGP(DRLCMAC, LOGL_NOTICE, "%s releasing due to "
-					"PACCH assignment timeout.\n", tbf_name(this));
-				tbf_free(this);
-				return;
-			} else
-				LOGP(DRLCMAC, LOGL_ERROR, "Error: %s is not "
-					"in assign state\n", tbf_name(this));
-		}
-		if ((state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) {
-			gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(this);
-			dl_tbf->m_wait_confirm = 0;
-			if (dl_tbf->state_is(GPRS_RLCMAC_ASSIGN)) {
-				tbf_assign_control_ts(dl_tbf);
-
-				if (!dl_tbf->upgrade_to_multislot) {
-					/* change state to FLOW, so scheduler
-					 * will start transmission */
-					dl_tbf->set_state(GPRS_RLCMAC_FLOW);
-					break;
-				}
-
-				/* This tbf can be upgraded to use multiple DL
-				 * timeslots and now that there is already one
-				 * slot assigned send another DL assignment via
-				 * PDCH. */
-
-				/* keep to flags */
-				dl_tbf->state_flags &= GPRS_RLCMAC_FLAG_TO_MASK;
-
-				dl_tbf->update();
-
-				dl_tbf->trigger_ass(dl_tbf);
-			} else
-				LOGP(DRLCMAC, LOGL_NOTICE, "%s Continue flow after "
-					"IMM.ASS confirm\n", tbf_name(dl_tbf));
-		}
-		break;
-	case 3169:
-	case 3191:
-	case 3195:
-		LOGP(DRLCMAC, LOGL_NOTICE, "%s T%d timeout during "
-			"transsmission\n", tbf_name(this), T);
-		rlcmac_diag();
-		/* fall through */
-	case 3193:
-		LOGP(DRLCMAC, LOGL_DEBUG,
-			"%s will be freed due to timeout\n", tbf_name(this));
-		/* free TBF */
-		tbf_free(this);
+	if (T) {
+		LOGP(DRLCMAC, LOGL_ERROR, "%s timer expired in unknown mode: %u\n",
+		     tbf_name(this), T);
 		return;
-		break;
-	default:
-		LOGP(DRLCMAC, LOGL_ERROR,
-			"%s timer expired in unknown mode: %u\n", tbf_name(this), T);
+	}
+
+	/* assignment */
+	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_PACCH))) {
+		if (state_is(GPRS_RLCMAC_ASSIGN)) {
+			LOGP(DRLCMAC, LOGL_NOTICE, "%s releasing due to PACCH assignment timeout.\n",
+			     tbf_name(this));
+			tbf_free(this);
+			return;
+		} else
+			LOGP(DRLCMAC, LOGL_ERROR, "Error: %s is not in assign state\n",
+			     tbf_name(this));
+	}
+
+	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) {
+		gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(this);
+		dl_tbf->m_wait_confirm = 0;
+		if (dl_tbf->state_is(GPRS_RLCMAC_ASSIGN)) {
+			tbf_assign_control_ts(dl_tbf);
+
+			if (!dl_tbf->upgrade_to_multislot) {
+				/* change state to FLOW, so scheduler
+				 * will start transmission */
+				dl_tbf->set_state(GPRS_RLCMAC_FLOW);
+				return;
+			}
+
+			/* This tbf can be upgraded to use multiple DL
+			 * timeslots and now that there is already one
+			 * slot assigned send another DL assignment via
+			 * PDCH. */
+
+			/* keep to flags */
+			dl_tbf->state_flags &= GPRS_RLCMAC_FLAG_TO_MASK;
+
+			dl_tbf->update();
+
+			dl_tbf->trigger_ass(dl_tbf);
+		} else
+			LOGP(DRLCMAC, LOGL_NOTICE, "%s Continue flow after IMM.ASS confirm\n",
+			     tbf_name(dl_tbf));
 	}
 }
 
diff --git a/src/tbf.h b/src/tbf.h
index 80249df..cdb4858 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -136,6 +136,24 @@
         TBF_CTR_EGPRS_UL_MCS9,
 };
 
+enum tbf_timers {
+	/* Wait for reuse of USF and TFI(s) after the MS uplink assignment for this TBF is invalid. */
+	T3169,
+
+	/* Wait for reuse of TFI(s) after sending of the last RLC Data Block on this TBF.
+	   Wait for reuse of TFI(s) after sending the PACKET TBF RELEASE for an MBMS radio bearer. */
+	T3191,
+
+	/* Wait for reuse of TFI(s) after reception of the final PACKET DOWNLINK ACK/NACK from the
+	   MS for this TBF. */
+	T3193,
+
+	/* Wait for reuse of TFI(s) when there is no response from the MS
+	   (radio failure or cell change) for this TBF/MBMS radio bearer. */
+	T3195,
+	T_MAX
+};
+
 #define GPRS_RLCMAC_FLAG_CCCH		0 /* assignment on CCCH */
 #define GPRS_RLCMAC_FLAG_PACCH		1 /* assignment on PACCH */
 #define GPRS_RLCMAC_FLAG_UL_DATA	2 /* uplink data received */
@@ -175,7 +193,9 @@
 	int update();
 	void handle_timeout();
 	void stop_timer(const char *reason);
-	void stop_t3191();
+	void stop_timers(const char *reason);
+	void t_stop(enum tbf_timers t, const char *reason);
+	void t_start(enum tbf_timers t, uint32_t sec, uint32_t microsec, const char *reason, bool force);
 	int establish_dl_tbf_on_pacch();
 
 	int check_polling(uint32_t fn, uint8_t ts,
@@ -248,6 +268,8 @@
 	struct osmo_timer_list	timer;
 	unsigned int T; /* Txxxx number */
 	unsigned int num_T_exp; /* number of consecutive T expirations */
+
+	struct osmo_timer_list T31[T_MAX]; /* T31xx timers */
 	
 	struct osmo_gsm_timer_list	gsm_timer;
 	unsigned int fT; /* fTxxxx number */
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 73708f5..eb4334c 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -489,6 +489,7 @@
 {
 	/* stop pending timer */
 	stop_timer("assignment (DL-TBF)");
+	stop_timers("assignment (DL-TBF)");
 
 	/* check for downlink tbf:  */
 	if (old_tbf) {
@@ -658,7 +659,7 @@
 
 	/* reset N3105 */
 	n3105 = 0;
-	stop_t3191();
+	t_stop(T3191, "ACK/NACK received");
 	poll_state = GPRS_RLCMAC_POLL_NONE;
 
 	return ack_recovered;
@@ -861,7 +862,7 @@
 			m_tx_counter = 0;
 			/* start timer whenever we send the final block */
 			if (is_final)
-				tbf_timer_start(this, 3191, bts_data()->t3191, 0, "final block (DL-TBF)");
+				t_start(T3191, bts_data()->t3191, 0, "final block (DL-TBF)", true);
 
 			clear_poll_timeout_flag();
 
@@ -1124,9 +1125,8 @@
 	set_state(GPRS_RLCMAC_WAIT_RELEASE);
 
 	/* start T3193 */
-	tbf_timer_start(this, 3193,
-		bts_data()->t3193_msec / 1000,
-			(bts_data()->t3193_msec % 1000) * 1000, "release (DL-TBF)");
+	t_start(T3193, bts_data()->t3193_msec / 1000, (bts_data()->t3193_msec % 1000) * 1000,
+		  "release (DL-TBF)", true);
 
 	/* reset rlc states */
 	m_tx_counter = 0;
diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index 0bbb817..c2cedc7 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -193,7 +193,7 @@
 	unsigned int block_idx;
 
 	/* restart T3169 */
-	tbf_timer_start(this, 3169, bts_data()->t3169, 0, "acked (data)");
+	t_start(T3169, bts_data()->t3169, 0, "acked (data)", true);
 
 	/* Increment RX-counter */
 	this->m_rx_counter++;
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index d4b51fe..18d8444 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -51,7 +51,7 @@
 {
 	OSMO_ASSERT(tbf);
 	if (tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE))
-		OSMO_ASSERT(tbf->T == 3191 || tbf->T == 3193);
+		OSMO_ASSERT(osmo_timer_pending(&tbf->T31[T3191]) || osmo_timer_pending(&tbf->T31[T3193]));
 	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 f534c54..f09f732 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -1897,8 +1897,15 @@
 TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=FINISHED) IMSI 0011223344: moving DL TBF to new MS object
 Detaching TBF from MS object, TLLI = 0xf1223344, TBF = TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=FINISHED)
 Attaching TBF to MS object, TLLI = 0xf5667788, TBF = TBF(TFI=0 TLLI=0xf5667788 DIR=DL STATE=FINISHED)
+UL RSSI of TLLI=0xf1223344: 31 dBm
+TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW) free
+TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW) stopping timer T3169 [freeing TBF]
+PDCH(TS 7, TRX 0): Detaching TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW), 1 TBFs, USFs = 02, TFIs = 00000002.
+Detaching TBF from MS object, TLLI = 0xf1223344, TBF = TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW)
+********** TBF ends here **********
 Modifying MS object, TLLI = 0xf5667788, IMSI '' -> '0011223344'
 Clearing MS object, TLLI: 0xf1223344, IMSI: '0011223344'
+Destroying MS object, TLLI = 0x00000000
 TBF(TFI=0 TLLI=0xf5667788 DIR=DL STATE=FINISHED) append
 Modifying MS object, TLLI: 0xf5667788 confirmed
 New MS: TLLI = 0xf5667788, TA = 7, IMSI = 0011223344, LLC = 1

-- 
To view, visit https://gerrit.osmocom.org/5158
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0305873ca47534f53441247217881da59625e1f7
Gerrit-PatchSet: 3
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list