From d372c393e57897dab5f33eca3f064b6d57ae1dd3 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 23 Jan 2018 20:09:06 +0100 Subject: TBF: make UL/DL state internal * add functions/macros for setting TBF's UL/DL state * add functions for checking TBF's UL/DL state * move pre-free check into separate function N. B: this should not be confused with TBF-UL or TBF-DL state. Change-Id: Idcbf5775d17b1247f2ed01788f9b0788ce66e871 Related: OS#1539 --- src/bts.cpp | 14 ++++++------ src/gprs_rlcmac_sched.cpp | 19 +++++++---------- src/tbf.cpp | 22 ++++--------------- src/tbf.h | 54 +++++++++++++++++++++++++++++++++++++++++++++-- src/tbf_dl.cpp | 2 +- tests/tbf/TbfTest.cpp | 10 ++++----- 6 files changed, 77 insertions(+), 44 deletions(-) diff --git a/src/bts.cpp b/src/bts.cpp index 873af737..f614c1a3 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -1022,11 +1022,11 @@ void gprs_rlcmac_pdch::rcv_control_ack(Packet_Control_Acknowledgement_t *packet, tbf_free(tbf); return; } - if (tbf->dl_ass_state == GPRS_RLCMAC_DL_ASS_WAIT_ACK) { + 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->dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE; + TBF_SET_ASS_STATE_DL(tbf, GPRS_RLCMAC_DL_ASS_NONE); new_tbf = tbf->ms() ? tbf->ms()->dl_tbf() : NULL; if (!new_tbf) { @@ -1054,11 +1054,11 @@ void gprs_rlcmac_pdch::rcv_control_ack(Packet_Control_Acknowledgement_t *packet, tbf_assign_control_ts(new_tbf); return; } - if (tbf->ul_ass_state == GPRS_RLCMAC_UL_ASS_WAIT_ACK) { + 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->ul_ass_state = GPRS_RLCMAC_UL_ASS_NONE; + TBF_SET_ASS_STATE_UL(tbf, GPRS_RLCMAC_UL_ASS_NONE); new_tbf = tbf->ms() ? tbf->ms()->ul_tbf() : NULL; if (!new_tbf) { @@ -1156,10 +1156,10 @@ static inline void sched_ul_ass_or_rej(BTS *bts, gprs_rlcmac_bts *bts_data, stru /* schedule uplink assignment or reject */ if (ul_tbf) { LOGP(DRLCMAC, LOGL_DEBUG, "MS requests UL TBF in ack message, so we provide one:\n"); - tbf->ul_ass_state = GPRS_RLCMAC_UL_ASS_SEND_ASS; + TBF_SET_ASS_STATE_UL(tbf, GPRS_RLCMAC_UL_ASS_SEND_ASS); } else { LOGP(DRLCMAC, LOGL_DEBUG, "MS requests UL TBF in ack message, so we packet access reject:\n"); - tbf->ul_ass_state = GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ; + TBF_SET_ASS_STATE_UL(tbf, GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ); } } @@ -1403,7 +1403,7 @@ void gprs_rlcmac_pdch::rcv_resource_request(Packet_Resource_Request_t *request, ul_tbf->control_ts = ts_no; /* schedule uplink assignment */ - ul_tbf->ul_ass_state = GPRS_RLCMAC_UL_ASS_SEND_ASS; + TBF_SET_ASS_STATE_UL(ul_tbf, GPRS_RLCMAC_UL_ASS_SEND_ASS); /* get capabilities */ if (ul_tbf->ms()) diff --git a/src/gprs_rlcmac_sched.cpp b/src/gprs_rlcmac_sched.cpp index 89250321..a87217f7 100644 --- a/src/gprs_rlcmac_sched.cpp +++ b/src/gprs_rlcmac_sched.cpp @@ -54,11 +54,10 @@ static uint32_t sched_poll(BTS *bts, *poll_tbf = ul_tbf; if (ul_tbf->ul_ack_state == GPRS_RLCMAC_UL_ACK_SEND_ACK) *ul_ack_tbf = ul_tbf; - if (ul_tbf->dl_ass_state == GPRS_RLCMAC_DL_ASS_SEND_ASS) + if (ul_tbf->dl_ass_state_is(GPRS_RLCMAC_DL_ASS_SEND_ASS)) *dl_ass_tbf = ul_tbf; - if (ul_tbf->ul_ass_state == GPRS_RLCMAC_UL_ASS_SEND_ASS - || ul_tbf->ul_ass_state == - GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ) + if (ul_tbf->ul_ass_state_is(GPRS_RLCMAC_UL_ASS_SEND_ASS) + || ul_tbf->ul_ass_state_is(GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ)) *ul_ass_tbf = ul_tbf; /* FIXME: Is this supposed to be fair? The last TBF for each wins? Maybe use llist_add_tail and skip once we have all states? */ @@ -73,10 +72,10 @@ states? */ if (dl_tbf->poll_state == GPRS_RLCMAC_POLL_SCHED && dl_tbf->poll_fn == poll_fn) *poll_tbf = dl_tbf; - if (dl_tbf->dl_ass_state == GPRS_RLCMAC_DL_ASS_SEND_ASS) + if (dl_tbf->dl_ass_state_is(GPRS_RLCMAC_DL_ASS_SEND_ASS)) *dl_ass_tbf = dl_tbf; - if (dl_tbf->ul_ass_state == GPRS_RLCMAC_UL_ASS_SEND_ASS - || dl_tbf->ul_ass_state == GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ) + if (dl_tbf->ul_ass_state_is(GPRS_RLCMAC_UL_ASS_SEND_ASS) + || dl_tbf->ul_ass_state_is(GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ)) *ul_ass_tbf = dl_tbf; } @@ -139,13 +138,11 @@ static struct msgb *sched_select_ctrl_msg( * because they may kill the TBF when the CONTROL ACK is * received, thus preventing the others from being processed. */ - if (tbf == ul_ass_tbf && tbf->ul_ass_state == - GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ) + if (tbf == ul_ass_tbf && tbf->ul_ass_state_is(GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ)) msg = ul_ass_tbf->create_packet_access_reject(); else if (tbf == ul_ass_tbf && tbf->direction == GPRS_RLCMAC_DL_TBF) - if (tbf->ul_ass_state == - GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ) + if (tbf->ul_ass_state_is(GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ)) msg = ul_ass_tbf->create_packet_access_reject(); else msg = ul_ass_tbf->create_ul_ass(fn, ts); diff --git a/src/tbf.cpp b/src/tbf.cpp index 699f960f..0cb54bc8 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -165,8 +165,6 @@ gprs_rlcmac_tbf::gprs_rlcmac_tbf(BTS *bts_, gprs_rlcmac_tbf_direction dir) : first_ts(0), first_common_ts(0), control_ts(0xff), - dl_ass_state(GPRS_RLCMAC_DL_ASS_NONE), - ul_ass_state(GPRS_RLCMAC_UL_ASS_NONE), ul_ack_state(GPRS_RLCMAC_UL_ACK_NONE), poll_state(GPRS_RLCMAC_POLL_NONE), poll_fn(0), @@ -185,6 +183,8 @@ gprs_rlcmac_tbf::gprs_rlcmac_tbf(BTS *bts_, gprs_rlcmac_tbf_direction dir) : m_ta(GSM48_TA_INVALID), m_ms_class(0), state(GPRS_RLCMAC_NULL), + dl_ass_state(GPRS_RLCMAC_DL_ASS_NONE), + ul_ass_state(GPRS_RLCMAC_UL_ASS_NONE), m_list(this), m_ms_list(this), m_egprs_enabled(false) @@ -452,21 +452,7 @@ void tbf_free(struct gprs_rlcmac_tbf *tbf) } LOGPTBF(tbf, LOGL_INFO, "free\n"); - if (tbf->ul_ass_state != GPRS_RLCMAC_UL_ASS_NONE) - LOGPTBF(tbf, LOGL_ERROR, "Software error: Pending uplink " - "assignment in state %s. This may not happen, because the " - "assignment message never gets transmitted. Please " - "be sure not to free in this state. PLEASE FIX!\n", - get_value_string(gprs_rlcmac_tbf_ul_ass_state_names, - tbf->ul_ass_state)); - if (tbf->dl_ass_state != GPRS_RLCMAC_DL_ASS_NONE) - LOGPTBF(tbf, LOGL_ERROR, "Software error: Pending downlink " - "assignment in state %s. This may not happen, because the " - "assignment message never gets transmitted. Please " - "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->check_pending_ass(); tbf->stop_timers("freeing TBF"); /* TODO: Could/Should generate bssgp_tx_llc_discarded */ tbf_unlink_pdch(tbf); @@ -1513,7 +1499,7 @@ struct gprs_rlcmac_ul_tbf *handle_tbf_reject(struct gprs_rlcmac_bts *bts, ul_tbf->set_ms(ms); ul_tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF); - ul_tbf->ul_ass_state = GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ; + TBF_SET_ASS_STATE_UL(ul_tbf, GPRS_RLCMAC_UL_ASS_SEND_ASS_REJ); ul_tbf->control_ts = ts; ul_tbf->trx = trx; ul_tbf->m_ctrs = rate_ctr_group_alloc(ul_tbf, &tbf_ctrg_desc, 0); diff --git a/src/tbf.h b/src/tbf.h index 244ddd4c..2d519458 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -181,6 +181,8 @@ enum tbf_timers { #define T_START(tbf, t, sec, usec, r, f) tbf->t_start(t, sec, usec, r, f, __FILE__, __LINE__) #define TBF_SET_STATE(t, st) do { t->set_state(st, __FILE__, __LINE__); } while(0) +#define TBF_SET_ASS_STATE_DL(t, st) do { t->set_ass_state_dl(st, __FILE__, __LINE__); } while(0) +#define TBF_SET_ASS_STATE_UL(t, st) do { t->set_ass_state_ul(st, __FILE__, __LINE__); } while(0) #define TBF_SET_ASS_ON(t, fl, chk) do { t->set_assigned_on(fl, chk, __FILE__, __LINE__); } while(0) struct gprs_rlcmac_tbf { @@ -191,7 +193,12 @@ struct gprs_rlcmac_tbf { bool state_is(enum gprs_rlcmac_tbf_state rhs) const; bool state_is_not(enum gprs_rlcmac_tbf_state rhs) const; + bool dl_ass_state_is(enum gprs_rlcmac_tbf_dl_ass_state rhs) const; + bool ul_ass_state_is(enum gprs_rlcmac_tbf_ul_ass_state rhs) const; void set_state(enum gprs_rlcmac_tbf_state new_state, const char *file, int line); + void set_ass_state_dl(enum gprs_rlcmac_tbf_dl_ass_state new_state, const char *file, int line); + void set_ass_state_ul(enum gprs_rlcmac_tbf_ul_ass_state new_state, const char *file, int line); + void check_pending_ass(); bool check_n_clear(uint8_t state_flag); void set_assigned_on(uint8_t state_flag, bool check_ccch, const char *file, int line); const char *state_name() const; @@ -272,8 +279,6 @@ struct gprs_rlcmac_tbf { gprs_llc m_llc; - enum gprs_rlcmac_tbf_dl_ass_state dl_ass_state; - enum gprs_rlcmac_tbf_ul_ass_state ul_ass_state; enum gprs_rlcmac_tbf_ul_ack_state ul_ack_state; enum gprs_rlcmac_tbf_poll_state poll_state; @@ -336,6 +341,8 @@ protected: private: enum gprs_rlcmac_tbf_state state; + enum gprs_rlcmac_tbf_dl_ass_state dl_ass_state; + enum gprs_rlcmac_tbf_ul_ass_state ul_ass_state; LListHead m_list; LListHead m_ms_list; bool m_egprs_enabled; @@ -368,6 +375,16 @@ inline bool gprs_rlcmac_tbf::state_is(enum gprs_rlcmac_tbf_state rhs) const return state == rhs; } +inline bool gprs_rlcmac_tbf::dl_ass_state_is(enum gprs_rlcmac_tbf_dl_ass_state rhs) const +{ + return dl_ass_state == rhs; +} + +inline bool gprs_rlcmac_tbf::ul_ass_state_is(enum gprs_rlcmac_tbf_ul_ass_state rhs) const +{ + return ul_ass_state == rhs; +} + inline bool gprs_rlcmac_tbf::state_is_not(enum gprs_rlcmac_tbf_state rhs) const { return state != rhs; @@ -399,6 +416,39 @@ inline void gprs_rlcmac_tbf::set_state(enum gprs_rlcmac_tbf_state new_state, con state = new_state; } +inline void gprs_rlcmac_tbf::set_ass_state_dl(enum gprs_rlcmac_tbf_dl_ass_state new_state, const char *file, int line) +{ + LOGPSRC(DTBF, LOGL_DEBUG, file, line, "%s changes DL ASS state from %s to %s\n", + tbf_name(this), + get_value_string(gprs_rlcmac_tbf_dl_ass_state_names, dl_ass_state), + get_value_string(gprs_rlcmac_tbf_dl_ass_state_names, new_state)); + dl_ass_state = new_state; +} + +inline void gprs_rlcmac_tbf::set_ass_state_ul(enum gprs_rlcmac_tbf_ul_ass_state new_state, const char *file, int line) +{ + LOGPSRC(DTBF, LOGL_DEBUG, file, line, "%s changes UL ASS state from %s to %s\n", + tbf_name(this), + get_value_string(gprs_rlcmac_tbf_ul_ass_state_names, ul_ass_state), + get_value_string(gprs_rlcmac_tbf_ul_ass_state_names, new_state)); + ul_ass_state = new_state; +} + +inline void gprs_rlcmac_tbf::check_pending_ass() +{ + if (ul_ass_state != GPRS_RLCMAC_UL_ASS_NONE) + LOGPTBF(this, LOGL_ERROR, "FIXME: Software error: Pending uplink assignment in state %s. " + "This may not happen, because the assignment message never gets transmitted. " + "Please be sure not to free in this state. PLEASE FIX!\n", + get_value_string(gprs_rlcmac_tbf_ul_ass_state_names, ul_ass_state)); + + if (dl_ass_state != GPRS_RLCMAC_DL_ASS_NONE) + LOGPTBF(this, LOGL_ERROR, "FIXME: Software error: Pending downlink assignment in state %s. " + "This may not happen, because the assignment message never gets transmitted. " + "Please be sure not to free in this state. PLEASE FIX!\n", + get_value_string(gprs_rlcmac_tbf_dl_ass_state_names, dl_ass_state)); +} + inline bool gprs_rlcmac_tbf::check_n_clear(uint8_t state_flag) { if ((state_flags & (1 << state_flag))) { diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index 9a0bf78c..fdbbd166 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -483,7 +483,7 @@ void gprs_rlcmac_dl_tbf::trigger_ass(struct gprs_rlcmac_tbf *old_tbf) /* check for downlink tbf: */ if (old_tbf) { LOGPTBFDL(this, LOGL_DEBUG, "Send dowlink assignment on PACCH, because %s exists\n", old_tbf->name()); - old_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_SEND_ASS; + TBF_SET_ASS_STATE_DL(old_tbf, GPRS_RLCMAC_DL_ASS_SEND_ASS); old_tbf->was_releasing = old_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE); /* change state */ diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp index 9e21c73e..968f9eb4 100644 --- a/tests/tbf/TbfTest.cpp +++ b/tests/tbf/TbfTest.cpp @@ -175,7 +175,7 @@ static gprs_rlcmac_dl_tbf *create_dl_tbf(BTS *the_bts, uint8_t ms_class, check_tbf(dl_tbf); /* "Establish" the DL TBF */ - dl_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_SEND_ASS; + TBF_SET_ASS_STATE_DL(dl_tbf, GPRS_RLCMAC_DL_ASS_SEND_ASS); TBF_SET_STATE(dl_tbf, GPRS_RLCMAC_FLOW); dl_tbf->m_wait_confirm = 0; check_tbf(dl_tbf); @@ -276,7 +276,7 @@ static void test_tbf_final_ack(enum test_tbf_final_ack_mode test_mode) OSMO_ASSERT(new_tbf != dl_tbf); OSMO_ASSERT(new_tbf->tfi() == 1); check_tbf(dl_tbf); - dl_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE; + TBF_SET_ASS_STATE_DL(dl_tbf, GPRS_RLCMAC_DL_ASS_NONE); if (test_mode == TEST_MODE_REVERSE_FREE) { GprsMs::Guard guard(ms); tbf_free(new_tbf); @@ -365,7 +365,7 @@ static void test_tbf_delayed_release() /* Clean up and ensure tbfs are in the correct state */ OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE)); - dl_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE; + TBF_SET_ASS_STATE_DL(dl_tbf, GPRS_RLCMAC_DL_ASS_NONE); check_tbf(dl_tbf); tbf_free(dl_tbf); printf("=== end %s ===\n", __func__); @@ -2699,7 +2699,7 @@ static void establish_and_use_egprs_dl_tbf(BTS *the_bts, int mcs) /* Clean up and ensure tbfs are in the correct state */ OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE)); - dl_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE; + TBF_SET_ASS_STATE_DL(dl_tbf, GPRS_RLCMAC_DL_ASS_NONE); check_tbf(dl_tbf); tbf_free(dl_tbf); } @@ -2748,7 +2748,7 @@ static void tbf_cleanup(gprs_rlcmac_dl_tbf *dl_tbf) /* Clean up and ensure tbfs are in the correct state */ OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE)); - dl_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE; + TBF_SET_ASS_STATE_DL(dl_tbf, GPRS_RLCMAC_DL_ASS_NONE); check_tbf(dl_tbf); tbf_free(dl_tbf); -- cgit v1.2.3