[PATCH] osmo-pcu[master]: TBF: unify timer handling

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
Fri Dec 8 12:07:41 UTC 2017


Review at  https://gerrit.osmocom.org/5234

TBF: unify timer handling

Use generic timer handling infrastracture to handle assignment/reject
internal timer as well. Rename timer array accordingly.

Change-Id: I63fb7e6f0695383a83472c836a381a055f64690b
---
M src/bts.cpp
M src/tbf.cpp
M src/tbf.h
M src/tbf_dl.cpp
M tests/tbf/TbfTest.cpp
5 files changed, 41 insertions(+), 79 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/34/5234/1

diff --git a/src/bts.cpp b/src/bts.cpp
index 29b743c..d01b234 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -548,7 +548,7 @@
 	LOGP(DRLCMAC, LOGL_DEBUG, "Got IMM.ASS confirm for TLLI=%08x\n", tlli);
 
 	if (dl_tbf->m_wait_confirm)
-		tbf_timer_start(dl_tbf, 0, Tassign_agch, "assignment (AGCH)");
+		dl_tbf->t_start(T0, Tassign_agch, "assignment (AGCH)", true);
 
 	return 0;
 }
@@ -1040,7 +1040,7 @@
 		}
 		new_tbf->set_state(GPRS_RLCMAC_FLOW);
 		/* stop pending assignment timer */
-		new_tbf->stop_timer("control acked (DL-TBF)");
+		new_tbf->t_stop(T0, "control acked (DL-TBF)");
 		if ((new_tbf->state_flags &
 			(1 << GPRS_RLCMAC_FLAG_TO_DL_ASS))) {
 			new_tbf->state_flags &=
diff --git a/src/tbf.cpp b/src/tbf.cpp
index f2b8055..35d93a5 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -60,6 +60,7 @@
 };
 
 static const struct value_string tbf_timers_names[] = {
+	OSMO_VALUE_STRING(T0),
 	OSMO_VALUE_STRING(T3169),
 	OSMO_VALUE_STRING(T3191),
 	OSMO_VALUE_STRING(T3193),
@@ -170,7 +171,6 @@
 	poll_fn(0),
 	poll_ts(0),
 	n3105(0),
-	T(0),
 	fT(0),
 	num_fT_exp(0),
 	state(GPRS_RLCMAC_NULL),
@@ -191,8 +191,7 @@
 	/* The classes of these members do not have proper constructors yet.
 	 * 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(&T, 0, sizeof(T));
 	memset(&m_rlc, 0, sizeof(m_rlc));
 	memset(&gsm_timer, 0, sizeof(gsm_timer));
 
@@ -334,7 +333,7 @@
 
 	/* Clean up the old MS object */
 	/* TODO: Use timer? */
-	if (old_ms->ul_tbf() && !old_ms->ul_tbf()->timers_pending()) {
+	if (old_ms->ul_tbf() && !old_ms->ul_tbf()->timers_pending(T_MAX)) {
 		if (old_ms->ul_tbf() == this) {
 			LOGP(DRLCMAC, LOGL_ERROR,
 				"%s is referred by the old MS "
@@ -345,7 +344,7 @@
 			tbf_free(old_ms->ul_tbf());
 		}
 	}
-	if (old_ms->dl_tbf() && !old_ms->dl_tbf()->timers_pending()) {
+	if (old_ms->dl_tbf() && !old_ms->dl_tbf()->timers_pending(T_MAX)) {
 		if (old_ms->dl_tbf() == this) {
 			LOGP(DRLCMAC, LOGL_ERROR,
 				"%s is referred by the old MS "
@@ -474,7 +473,7 @@
 		     tbf_name(tbf),
 		     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);
@@ -539,27 +538,6 @@
 	"RELEASING",
 };
 
-void tbf_timer_start(struct gprs_rlcmac_tbf *tbf, unsigned int T,
-		     unsigned int seconds, unsigned int microseconds, const char *reason)
-{
-	LOGPC(DRLCMAC, (T != tbf->T) ? LOGL_ERROR : LOGL_DEBUG,
-	      "%s %sstarting timer T%u [%s] with %u sec. %u microsec.",
-	      tbf_name(tbf), osmo_timer_pending(&tbf->timer) ? "re" : "", T, reason, seconds, microseconds);
-
-	if (T != tbf->T && osmo_timer_pending(&tbf->timer))
-		LOGPC(DRLCMAC, LOGL_ERROR, " while old timer T%u pending", tbf->T);
-
-	LOGPC(DRLCMAC, (T != tbf->T) ? LOGL_ERROR : LOGL_DEBUG, "\n");
-
-	tbf->T = T;
-
-	/* Tunning timers can be safely re-scheduled. */
-	tbf->timer.data = tbf;
-	tbf->timer.cb = &tbf_timer_cb;
-
-	osmo_timer_schedule(&tbf->timer, seconds, microseconds);
-}
-
 void gprs_rlcmac_tbf::t_stop(enum tbf_timers t, const char *reason)
 {
 	if (t >= T_MAX) {
@@ -567,20 +545,24 @@
 		     tbf_name(this), get_value_string(tbf_timers_names, t), reason);
 	}
 
-	if (osmo_timer_pending(&T31[t])) {
+	if (osmo_timer_pending(&T[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]);
+		osmo_timer_del(&T[t]);
 	}
 }
 
 /* check if any of T31xx timers are pending */
-bool gprs_rlcmac_tbf::timers_pending()
+bool gprs_rlcmac_tbf::timers_pending(enum tbf_timers t)
 {
 	uint8_t i;
 
+	if (t != T_MAX)
+		return osmo_timer_pending(&T[t]);
+
+	/* we don't start with T0 because it's internal timer which requires special handling */
 	for (i = T3169; i < T_MAX; i++)
-		if (osmo_timer_pending(&T31[i]))
+		if (osmo_timer_pending(&T[i]))
 			return true;
 
 	return false;
@@ -589,17 +571,9 @@
 void gprs_rlcmac_tbf::stop_timers(const char *reason)
 {
 	uint8_t i;
-	for (i = 0; i < T_MAX; i++)
+	/* we start with T0 because timer reset does not require any special handling */
+	for (i = T0; i < T_MAX; i++)
 		t_stop((enum tbf_timers)i, reason);
-}
-
-void gprs_rlcmac_tbf::stop_timer(const char *reason)
-{
-	if (osmo_timer_pending(&timer)) {
-		LOGP(DRLCMAC, LOGL_DEBUG, "%s stopping timer T%u [%s]\n",
-		     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)
@@ -627,34 +601,37 @@
 		     tbf_name(this), get_value_string(tbf_timers_names, t), reason);
 	}
 
-	if (!force && osmo_timer_pending(&T31[t]))
+	if (!force && osmo_timer_pending(&T[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" : "",
+	     tbf_name(this), osmo_timer_pending(&T[t]) ? "re" : "",
 	     get_value_string(tbf_timers_names, t), reason, sec, microsec);
 
-	T31[t].data = this;
+	T[t].data = this;
 
 	switch(t) {
+	case T0:
+		T[t].cb = tbf_timer_cb;
+		break;
 	case T3169:
-		T31[t].cb = cb_T3169;
+		T[t].cb = cb_T3169;
 		break;
 	case T3191:
-		T31[t].cb = cb_T3191;
+		T[t].cb = cb_T3191;
 		break;
 	case T3193:
-		T31[t].cb = cb_T3193;
+		T[t].cb = cb_T3193;
 		break;
 	case T3195:
-		T31[t].cb = cb_T3195;
+		T[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);
+	osmo_timer_schedule(&T[t], sec, microsec);
 }
 
 int gprs_rlcmac_tbf::check_polling(uint32_t fn, uint8_t ts,
@@ -1105,17 +1082,9 @@
 	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",
-		tbf_name(this), T);
-
-	if (T) {
-		LOGP(DRLCMAC, LOGL_ERROR, "%s timer expired in unknown mode: %u\n",
-		     tbf_name(this), T);
-		return;
-	}
+	LOGP(DRLCMAC, LOGL_DEBUG, "%s timer 0 expired.\n", tbf_name(this));
 
 	/* assignment */
 	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_PACCH))) {
@@ -1280,7 +1249,7 @@
 		new_dl_tbf->set_state(GPRS_RLCMAC_FLOW);
 		tbf_assign_control_ts(new_dl_tbf);
 		/* stop pending assignment timer */
-		new_dl_tbf->stop_timer("assignment (DL-TBF)");
+		new_dl_tbf->t_stop(T0, "assignment (DL-TBF)");
 
 	}
 
@@ -1310,7 +1279,7 @@
 
 	/* Start Tmr only if it is UL TBF */
 	if (direction == GPRS_RLCMAC_UL_TBF)
-		tbf_timer_start(this, 0, Treject_pacch, "reject (PACCH)");
+		t_start(T0, Treject_pacch, "reject (PACCH)", true);
 
 	return msg;
 
diff --git a/src/tbf.h b/src/tbf.h
index 05a4313..2e8a2ba 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -137,6 +137,9 @@
 };
 
 enum tbf_timers {
+	/* internal assign/reject timer */
+	T0,
+
 	/* Wait for reuse of USF and TFI(s) after the MS uplink assignment for this TBF is invalid. */
 	T3169,
 
@@ -192,9 +195,8 @@
 
 	int update();
 	void handle_timeout();
-	void stop_timer(const char *reason);
 	void stop_timers(const char *reason);
-	bool timers_pending();
+	bool timers_pending(enum tbf_timers t);
 	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();
@@ -265,11 +267,6 @@
 	gprs_rlc m_rlc;
 	
 	uint8_t n3105;	/* N3105 counter */
-
-	struct osmo_timer_list	timer;
-	unsigned int T; /* Txxxx number */
-
-	struct osmo_timer_list T31[T_MAX]; /* T31xx timers */
 	
 	struct osmo_gsm_timer_list	gsm_timer;
 	unsigned int fT; /* fTxxxx number */
@@ -329,7 +326,7 @@
 	LListHead<gprs_rlcmac_tbf> m_list;
 	LListHead<gprs_rlcmac_tbf> m_ms_list;
 	bool m_egprs_enabled;
-
+	struct osmo_timer_list T[T_MAX];
 	mutable char m_name_buf[60];
 };
 
@@ -352,9 +349,6 @@
 	GprsMs *ms, uint32_t tlli, uint8_t trx_no, uint8_t ts_no);
 
 int tbf_assign_control_ts(struct gprs_rlcmac_tbf *tbf);
-
-void tbf_timer_start(struct gprs_rlcmac_tbf *tbf, unsigned int T,
-		     unsigned int seconds, unsigned int microseconds, const char *reason);
 
 inline bool gprs_rlcmac_tbf::state_is(enum gprs_rlcmac_tbf_state rhs) const
 {
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 9dba2e3..87a0dce 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -219,9 +219,9 @@
 			}
 			/* Clean up the old MS object */
 			/* TODO: Put this into a separate function, use timer? */
-			if (ms_old->ul_tbf() && !ms_old->ul_tbf()->timers_pending())
+			if (ms_old->ul_tbf() && !ms_old->ul_tbf()->timers_pending(T_MAX))
 				tbf_free(ms_old->ul_tbf());
-			if (ms_old->dl_tbf() && !ms_old->dl_tbf()->timers_pending())
+			if (ms_old->dl_tbf() && !ms_old->dl_tbf()->timers_pending(T_MAX))
 				tbf_free(ms_old->dl_tbf());
 
 			ms->merge_old_ms(ms_old);
@@ -488,7 +488,6 @@
 void gprs_rlcmac_dl_tbf::trigger_ass(struct gprs_rlcmac_tbf *old_tbf)
 {
 	/* stop pending timer */
-	stop_timer("assignment (DL-TBF)");
 	stop_timers("assignment (DL-TBF)");
 
 	/* check for downlink tbf:  */
@@ -503,7 +502,7 @@
 			state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
 
 		/* start timer */
-		tbf_timer_start(this, 0, Tassign_pacch, "assignment (PACCH)");
+		t_start(T0, Tassign_pacch, "assignment (PACCH)", true);
 	} else {
 		LOGP(DRLCMACDL, LOGL_DEBUG, "Send dowlink assignment for %s on PCH, no TBF exist (IMSI=%s)\n",
 		     tbf_name(this), imsi());
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index b466d1b..18bbc76 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -51,9 +51,9 @@
 {
 	OSMO_ASSERT(tbf);
 	if (tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE))
-		OSMO_ASSERT(osmo_timer_pending(&tbf->T31[T3191]) || osmo_timer_pending(&tbf->T31[T3193]));
+		OSMO_ASSERT(tbf->timers_pending(T3191) || tbf->timers_pending(T3193));
 	if (tbf->state_is(GPRS_RLCMAC_RELEASING))
-		OSMO_ASSERT(tbf->timers_pending());
+		OSMO_ASSERT(tbf->timers_pending(T_MAX));
 }
 
 static void test_tbf_base()

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I63fb7e6f0695383a83472c836a381a055f64690b
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>



More information about the gerrit-log mailing list