Harald Welte has submitted this change and it was merged.

Change subject: TBF: make network counters internal
......................................................................


TBF: make network counters internal

* store N310* counters in shared array similar to corresponding timers
* add functions to increment/reset counters

This avoids direct access to TBF counters from PDCH.

Change-Id: I8ffff9c7186f74bde7e6ac5f6e98f0b3e4c35274
Related: OS#1539
---
M src/pdch.cpp
M src/tbf.cpp
M src/tbf.h
M src/tbf_dl.cpp
M src/tbf_ul.cpp
5 files changed, 88 insertions(+), 35 deletions(-)

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



diff --git a/src/pdch.cpp b/src/pdch.cpp
index ee9df31..22a1605 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -290,7 +290,7 @@
        }
 
        /* Reset N3101 counter: */
-       tbf->m_n3101 = 0;
+       tbf->n_reset(N3101);
 
        tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
 
@@ -310,7 +310,7 @@
        if (tbf->dl_ass_state_is(GPRS_RLCMAC_DL_ASS_WAIT_ACK)) {
                LOGPTBF(tbf, LOGL_DEBUG, "[UPLINK] DOWNLINK ASSIGNED\n");
                /* reset N3105 */
-               tbf->n3105 = 0;
+               tbf->n_reset(N3105);
                TBF_SET_ASS_STATE_DL(tbf, GPRS_RLCMAC_DL_ASS_NONE);
 
                new_tbf = tbf->ms() ? tbf->ms()->dl_tbf() : NULL;
@@ -342,7 +342,7 @@
        if (tbf->ul_ass_state_is(GPRS_RLCMAC_UL_ASS_WAIT_ACK)) {
                LOGPTBF(tbf, LOGL_DEBUG, "[DOWNLINK] UPLINK ASSIGNED\n");
                /* reset N3105 */
-               tbf->n3105 = 0;
+               tbf->n_reset(N3105);
                TBF_SET_ASS_STATE_UL(tbf, GPRS_RLCMAC_UL_ASS_NONE);
 
                new_tbf = tbf->ms() ? tbf->ms()->ul_tbf() : NULL;
@@ -399,7 +399,7 @@
        }
 
        /* Reset N3101 counter: */
-       tbf->m_n3101 = 0;
+       tbf->n_reset(N3101);
 
        if (tbf->handle_ack_nack())
                LOGPTBF(tbf, LOGL_NOTICE, "Recovered downlink ack\n");
@@ -466,7 +466,7 @@
        }
 
        /* Reset N3101 counter: */
-       tbf->m_n3101 = 0;
+       tbf->n_reset(N3101);
 
        if (tbf->handle_ack_nack())
                LOGPTBF(tbf, LOGL_NOTICE, "Recovered EGPRS downlink ack\n");
@@ -638,7 +638,7 @@
                        "RX: [PCU <- BTS] FIXME: Packet resource request\n");
 
                /* Reset N3101 counter: */
-               dl_tbf->m_n3101 = 0;
+               dl_tbf->n_reset(N3101);
        } else {
                struct gprs_rlcmac_ul_tbf *ul_tbf;
                int8_t tfi = request->ID.u.Global_TFI.u.UPLINK_TFI;
@@ -651,7 +651,7 @@
                        "RX: [PCU <- BTS] FIXME: Packet resource request\n");
 
                /* Reset N3101 counter: */
-               ul_tbf->m_n3101 = 0;
+               ul_tbf->n_reset(N3101);
        }
 }
 
@@ -806,7 +806,7 @@
        }
 
        /* Reset N3101 counter: */
-       tbf->m_n3101 = 0;
+       tbf->n_reset(N3101);
 
        return tbf->rcv_data_block_acknowledged(&rlc_dec, data, meas);
 }
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 72b39ec..7036ea1 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -80,6 +80,13 @@
        { 0, NULL }
 };
 
+static const struct value_string tbf_counters_names[] = {
+       OSMO_VALUE_STRING(N3101),
+       OSMO_VALUE_STRING(N3103),
+       OSMO_VALUE_STRING(N3105),
+       { 0, NULL }
+};
+
 static const struct value_string tbf_timers_names[] = {
        OSMO_VALUE_STRING(T0),
        OSMO_VALUE_STRING(T3169),
@@ -187,13 +194,11 @@
        control_ts(0xff),
        poll_fn(0),
        poll_ts(0),
-       n3105(0),
        fT(0),
        num_fT_exp(0),
        was_releasing(0),
        upgrade_to_multislot(0),
        bts(bts_),
-       m_n3101(0),
        m_tfi(0),
        m_created_ts(0),
        m_ctrs(NULL),
@@ -213,6 +218,7 @@
         * Just set them to 0 like talloc_zero did */
        memset(&pdch, 0, sizeof(pdch));
        memset(&T, 0, sizeof(T));
+       memset(&N, 0, sizeof(N));
        memset(&m_rlc, 0, sizeof(m_rlc));
        memset(&gsm_timer, 0, sizeof(gsm_timer));
 
@@ -546,6 +552,55 @@
        "RELEASING",
 };
 
+void gprs_rlcmac_tbf::n_reset(enum tbf_counters n)
+{
+       if (n >= N_MAX) {
+               LOGPTBF(this, LOGL_ERROR, "attempting to reset unknown counter 
%s\n",
+                       get_value_string(tbf_counters_names, n));
+               return;
+       }
+
+       N[n] = 0;
+}
+
+/* Increment counter and check for MAX value (return true if we hit it) */
+bool gprs_rlcmac_tbf::n_inc(enum tbf_counters n)
+{
+       uint8_t chk;
+
+       if (n >= N_MAX) {
+               LOGPTBF(this, LOGL_ERROR, "attempting to increment unknown 
counter %s\n",
+                       get_value_string(tbf_counters_names, n));
+               return true;
+       }
+
+       N[n]++;
+
+       switch(n) {
+       case N3101:
+               chk = bts->bts_data()->n3101;
+               break;
+       case N3103:
+               chk = bts->bts_data()->n3103;
+               break;
+       case N3105:
+               chk = bts->bts_data()->n3105;
+               break;
+       default:
+               LOGPTBF(this, LOGL_ERROR, "unhandled counter %s\n",
+                       get_value_string(tbf_counters_names, n));
+               return true;
+       }
+
+       if (N[n] == chk) {
+               LOGPTBF(this, LOGL_NOTICE, "%s exceeded MAX (%u)\n",
+                       get_value_string(tbf_counters_names, n), chk);
+               return true;
+       }
+
+       return false;
+}
+
 void gprs_rlcmac_tbf::t_stop(enum tbf_timers t, const char *reason)
 {
        if (t >= T_MAX) {
@@ -728,9 +783,7 @@
 
        poll_state = GPRS_RLCMAC_POLL_NONE;
 
-       m_n3101++;
-       if (m_n3101 == bts->bts_data()->n3101) {
-               LOGPTBF(this, LOGL_NOTICE, "N3101 exceeded MAX (%u)\n", 
bts->bts_data()->n3101);
+       if (n_inc(N3101)) {
                TBF_SET_STATE(this, GPRS_RLCMAC_RELEASING);
                T_START(this, T3169, bts->bts_data()->t3169, 0, "MAX N3101 
reached", false);
                return;
@@ -744,9 +797,7 @@
                bts->rlc_ack_timedout();
                bts->pkt_ul_ack_nack_poll_timedout();
                if (state_is(GPRS_RLCMAC_FINISHED)) {
-                       ul_tbf->m_n3103++;
-                       if (ul_tbf->m_n3103 == ul_tbf->bts->bts_data()->n3103) {
-                               LOGPTBF(this, LOGL_NOTICE, "N3103 exceeded\n");
+                       if (ul_tbf->n_inc(N3103)) {
                                bts->pkt_ul_ack_nack_poll_failed();
                                TBF_SET_STATE(ul_tbf, GPRS_RLCMAC_RELEASING);
                                T_START(ul_tbf, T3169, 
ul_tbf->bts->bts_data()->t3169, 0, "MAX N3103 reached", false);
@@ -764,11 +815,9 @@
                        state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ASS);
                }
                ul_ass_state = GPRS_RLCMAC_UL_ASS_NONE;
-               n3105++;
                bts->rlc_ass_timedout();
                bts->pua_poll_timedout();
-               if (n3105 == bts_data()->n3105) {
-                       LOGPTBF(this, LOGL_NOTICE, "N3105 exceeded\n");
+               if (n_inc(N3105)) {
                        TBF_SET_STATE(this, GPRS_RLCMAC_RELEASING);
                        T_START(this, T3195, bts_data()->t3195, 0, "MAX N3105 
reached", true);
                        bts->rlc_ass_failed();
@@ -785,11 +834,9 @@
                        state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_DL_ASS);
                }
                dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE;
-               n3105++;
                bts->rlc_ass_timedout();
                bts->pda_poll_timedout();
-               if (n3105 == bts->bts_data()->n3105) {
-                       LOGPTBF(this, LOGL_NOTICE, "N3105 exceeded\n");
+               if (n_inc(N3105)) {
                        TBF_SET_STATE(this, GPRS_RLCMAC_RELEASING);
                        T_START(this, T3195, bts_data()->t3195, 0, "MAX N3105 
reached", true);
                        bts->rlc_ass_failed();
@@ -807,15 +854,15 @@
                        dl_tbf->rlcmac_diag();
                        dl_tbf->state_flags |= (1 << 
GPRS_RLCMAC_FLAG_TO_DL_ACK);
                }
-               dl_tbf->n3105++;
+
                if (dl_tbf->state_is(GPRS_RLCMAC_RELEASING))
                        bts->rlc_rel_timedout();
                else {
                        bts->rlc_ack_timedout();
                        bts->pkt_dl_ack_nack_poll_timedout();
                }
-               if (dl_tbf->n3105 == dl_tbf->bts->bts_data()->n3105) {
-                       LOGPTBF(this, LOGL_NOTICE, "N3105 exceeded\n");
+
+               if (dl_tbf->n_inc(N3105)) {
                        TBF_SET_STATE(dl_tbf, GPRS_RLCMAC_RELEASING);
                        T_START(dl_tbf, T3195, dl_tbf->bts_data()->t3195, 0, 
"MAX N3105 reached", true);
                        bts->pkt_dl_ack_nack_poll_failed();
@@ -884,7 +931,6 @@
 gprs_rlcmac_ul_tbf::gprs_rlcmac_ul_tbf(BTS *bts_) :
        gprs_rlcmac_tbf(bts_, GPRS_RLCMAC_UL_TBF),
        m_rx_counter(0),
-       m_n3103(0),
        m_contention_resolution_done(0),
        m_final_ack_sent(0),
        m_ul_gprs_ctrs(NULL),
diff --git a/src/tbf.h b/src/tbf.h
index 239b8fd..803b294 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -108,7 +108,7 @@
        DL_PRIO_CONTROL,   /* a control block needs to be sent */
 };
 
-enum tbf_counters {
+enum tbf_rlc_counters {
        TBF_CTR_RLC_NACKED,
 };
 
@@ -175,6 +175,14 @@
        T_MAX
 };
 
+enum tbf_counters { /* TBF counters from 3GPP TS 44.060 ยง13.4 */
+       /* counters are reset when: */
+       N3101, /* received a valid data block from mobile station in a block 
assigned for this USF */
+       N3103, /* transmitting the final PACKET UPLINK ACK/NACK message */
+       N3105, /* after sending a RRBP field in the downlink RLC data block, 
receives a valid RLC/MAC control message */
+       N_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 */
@@ -233,6 +241,9 @@
        uint8_t tsc() const;
 
        int rlcmac_diag();
+
+       bool n_inc(enum tbf_counters n);
+       void n_reset(enum tbf_counters n);
 
        int update();
        void handle_timeout();
@@ -301,9 +312,7 @@
        uint8_t poll_ts; /* TS to poll */
 
        gprs_rlc m_rlc;
-       
-       uint8_t n3105;  /* N3105 counter */
-       
+
        struct osmo_gsm_timer_list      gsm_timer;
        unsigned int fT; /* fTxxxx number */
        unsigned int num_fT_exp; /* number of consecutive fT expirations */
@@ -325,8 +334,6 @@
 
        /* store the BTS this TBF belongs to */
        BTS *bts;
-
-       uint8_t m_n3101; /* N3101 counter */
 
        /*
         * private fields. We can't make it private as it is breaking the
@@ -364,6 +371,7 @@
        LListHead<gprs_rlcmac_tbf> m_ms_list;
        bool m_egprs_enabled;
        struct osmo_timer_list T[T_MAX];
+       uint8_t N[N_MAX];
        mutable char m_name_buf[60];
 };
 
@@ -751,7 +759,6 @@
         * variables are in both (dl and ul) structs and not outside union.
         */
        int32_t m_rx_counter; /* count all received blocks */
-       uint8_t m_n3103;        /* N3103 counter */
        uint8_t m_usf[8];       /* list USFs per PDCH (timeslot) */
        uint8_t m_contention_resolution_done; /* set after done */
        uint8_t m_final_ack_sent; /* set if we sent final ack */
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index c92bfdf..dd24963 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -643,7 +643,7 @@
        }
 
        /* reset N3105 */
-       n3105 = 0;
+       n_reset(N3105);
        t_stop(T3191, "ACK/NACK received");
        TBF_POLL_SCHED_UNSET(this);
 
diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index a4d3499..02f4ddb 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -333,7 +333,7 @@
                        LOGPTBFUL(this, LOGL_DEBUG, "Finished with UL TBF\n");
                        TBF_SET_STATE(this, GPRS_RLCMAC_FINISHED);
                        /* Reset N3103 counter. */
-                       this->m_n3103 = 0;
+                       this->n_reset(N3103);
                }
        }
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8ffff9c7186f74bde7e6ac5f6e98f0b3e4c35274
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msur...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder

Reply via email to