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.orgHello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/5158
to look at the new patch set (#4).
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.
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
6 files changed, 183 insertions(+), 78 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/58/5158/4
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..240b5b5 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));
@@ -324,7 +334,7 @@
/* Clean up the old MS object */
/* TODO: Use timer? */
- if (old_ms->ul_tbf() && old_ms->ul_tbf()->T == 0) {
+ if (old_ms->ul_tbf() && !old_ms->ul_tbf()->timers_pending()) {
if (old_ms->ul_tbf() == this) {
LOGP(DRLCMAC, LOGL_ERROR,
"%s is referred by the old MS "
@@ -335,7 +345,7 @@
tbf_free(old_ms->ul_tbf());
}
}
- if (old_ms->dl_tbf() && old_ms->dl_tbf()->T == 0) {
+ if (old_ms->dl_tbf() && !old_ms->dl_tbf()->timers_pending()) {
if (old_ms->dl_tbf() == this) {
LOGP(DRLCMAC, LOGL_ERROR,
"%s is referred by the old MS "
@@ -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,37 @@
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]);
+ }
+}
+
+/* check if any of T31xx timers are pending */
+bool gprs_rlcmac_tbf::timers_pending()
+{
+ uint8_t i;
+
+ for (i = T3169; i < T_MAX; i++)
+ if (osmo_timer_pending(&T31[i]))
+ return true;
+
+ return false;
+}
+
+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 +601,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 +757,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 +779,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 +801,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 +827,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 +1098,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 +1106,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..2ac3455 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,10 @@
int update();
void handle_timeout();
void stop_timer(const char *reason);
- void stop_t3191();
+ void stop_timers(const char *reason);
+ bool timers_pending();
+ 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 +269,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..9dba2e3 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()->T == 0)
+ if (ms_old->ul_tbf() && !ms_old->ul_tbf()->timers_pending())
tbf_free(ms_old->ul_tbf());
- if (ms_old->dl_tbf() && ms_old->dl_tbf()->T == 0)
+ if (ms_old->dl_tbf() && !ms_old->dl_tbf()->timers_pending())
tbf_free(ms_old->dl_tbf());
ms->merge_old_ms(ms_old);
@@ -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..b466d1b 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(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);
+ OSMO_ASSERT(tbf->timers_pending());
}
static void test_tbf_base()
--
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: 4
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
Gerrit-Reviewer: Max <msuraev at sysmocom.de>