Harald Welte has submitted this change and it was merged.

Change subject: TBF: unify timer handling
......................................................................


TBF: unify timer handling

Use generic timer handling infrastracture to handle assignment/reject
internal timer. Rename timer array accordingly. Use defines with
explicit second/microsecond values to make it more readable.

Change-Id: I63fb7e6f0695383a83472c836a381a055f64690b
---
M src/bts.cpp
M src/tbf.cpp
M src/tbf.h
M src/tbf_dl.cpp
4 files changed, 32 insertions(+), 68 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/bts.cpp b/src/bts.cpp
index 3655567..d0ba768 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -554,7 +554,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, 0, T_ASS_AGCH_USEC, "assignment (AGCH)", 
true);
 
        return 0;
 }
@@ -1041,7 +1041,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 dc0777f..ea27597 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));
 
@@ -472,7 +471,7 @@
                        "be sure not to free in this state. PLEASE FIX!\n",
                     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);
@@ -538,27 +537,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) {
@@ -579,10 +557,11 @@
        uint8_t i;
 
        if (t != T_MAX)
-               return osmo_timer_pending(&T31[t]);
+               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;
@@ -591,17 +570,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)) {
-               LOGPTBF(this, LOGL_DEBUG, "stopping timer T%u [%s]\n",
-                       T, reason);
-               osmo_timer_del(&timer);
-       }
 }
 
 static inline void tbf_timeout_free(struct gprs_rlcmac_tbf *tbf, enum 
tbf_timers t, bool run_diag)
@@ -629,33 +600,36 @@
                        get_value_string(tbf_timers_names, t), reason);
        }
 
-       if (!force && osmo_timer_pending(&T31[t]))
+       if (!force && osmo_timer_pending(&T[t]))
                return;
 
        LOGPTBF(this, LOGL_DEBUG, "%sstarting timer %s [%s] with %u sec. %u 
microsec.\n",
-               osmo_timer_pending(&T31[t]) ? "re" : "", 
get_value_string(tbf_timers_names, t), reason, sec, microsec);
+               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:
                LOGPTBF(this, LOGL_ERROR, "attempting to set callback for 
unknown timer %s [%s]\n",
                        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,
@@ -1102,12 +1076,7 @@
 
 void gprs_rlcmac_tbf::handle_timeout()
 {
-       LOGPTBF(this, LOGL_DEBUG, "timer %u expired.\n", T);
-
-       if (T) {
-               LOGPTBF(this, LOGL_ERROR, "%s timer expired in unknown mode: 
%u\n", T);
-               return;
-       }
+       LOGPTBF(this, LOGL_DEBUG, "timer 0 expired.\n");
 
        /* assignment */
        if ((state_flags & (1 << GPRS_RLCMAC_FLAG_PACCH))) {
@@ -1267,7 +1236,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)");
 
        }
 
@@ -1297,7 +1266,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, 0, T_REJ_PACCH_USEC, "reject (PACCH)", true);
 
        return msg;
 
diff --git a/src/tbf.h b/src/tbf.h
index 4489695..88f5d6a 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -39,9 +39,9 @@
  * TBF instance
  */
 
-#define Tassign_agch 0,200000  /* waiting after IMM.ASS confirm */
-#define Tassign_pacch 2,0      /* timeout for pacch assigment */
-#define Treject_pacch 0,2000   /* timeout for tbf reject for PRR*/
+#define T_ASS_AGCH_USEC 200000 /* waiting after IMM.ASS confirm */
+#define T_ASS_PACCH_SEC 2      /* timeout for pacch assigment */
+#define T_REJ_PACCH_USEC 2000  /* timeout for tbf reject for PRR*/
 
 enum gprs_rlcmac_tbf_state {
        GPRS_RLCMAC_NULL = 0,   /* new created TBF */
@@ -139,6 +139,9 @@
 #define LOGPTBFDL(tbf, level, fmt, args...) LOGP(DRLCMACDL, level, "%s " fmt, 
tbf_name(tbf), ## args)
 
 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,
 
@@ -194,7 +197,6 @@
 
        int update();
        void handle_timeout();
-       void stop_timer(const char *reason);
        void stop_timers(const char *reason);
        bool timers_pending(enum tbf_timers t);
        void t_stop(enum tbf_timers t, const char *reason);
@@ -266,9 +268,6 @@
        gprs_rlc m_rlc;
        
        uint8_t n3105;  /* N3105 counter */
-
-       struct osmo_timer_list  timer;
-       unsigned int T; /* Txxxx number */
        
        struct osmo_gsm_timer_list      gsm_timer;
        unsigned int fT; /* fTxxxx number */
@@ -328,7 +327,7 @@
        LListHead<gprs_rlcmac_tbf> m_list;
        LListHead<gprs_rlcmac_tbf> m_ms_list;
        bool m_egprs_enabled;
-       struct osmo_timer_list T31[T_MAX];
+       struct osmo_timer_list T[T_MAX];
        mutable char m_name_buf[60];
 };
 
@@ -351,9 +350,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 b043989..33eb75b 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -484,7 +484,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:  */
@@ -499,7 +498,7 @@
                        state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
 
                /* start timer */
-               tbf_timer_start(this, 0, Tassign_pacch, "assignment (PACCH)");
+               t_start(T0, T_ASS_PACCH_SEC, 0, "assignment (PACCH)", true);
        } else {
                LOGPTBFDL(this, LOGL_DEBUG, "Send dowlink assignment on PCH, no 
TBF exist (IMSI=%s)\n",
                          imsi());

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I63fb7e6f0695383a83472c836a381a055f64690b
Gerrit-PatchSet: 5
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder

Reply via email to