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.orgMax has submitted this change and it was merged.
Change subject: TBF: make UL/DL state internal
......................................................................
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
---
M src/bts.cpp
M src/gprs_rlcmac_sched.cpp
M src/tbf.cpp
M src/tbf.h
M src/tbf_dl.cpp
M tests/tbf/TbfTest.cpp
6 files changed, 77 insertions(+), 44 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/bts.cpp b/src/bts.cpp
index 873af73..f614c1a 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -1022,11 +1022,11 @@
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 @@
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 @@
/* 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 @@
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 8925032..a87217f 100644
--- a/src/gprs_rlcmac_sched.cpp
+++ b/src/gprs_rlcmac_sched.cpp
@@ -54,11 +54,10 @@
*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 @@
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 @@
* 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 699f960..0cb54bc 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -165,8 +165,6 @@
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 @@
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 @@
}
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 @@
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 244ddd4..2d51945 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -181,6 +181,8 @@
#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 @@
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 @@
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 @@
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<gprs_rlcmac_tbf> m_list;
LListHead<gprs_rlcmac_tbf> m_ms_list;
bool m_egprs_enabled;
@@ -368,6 +375,16 @@
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 @@
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 9a0bf78..fdbbd16 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -483,7 +483,7 @@
/* 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 9e21c73..968f9eb 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -175,7 +175,7 @@
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 @@
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 @@
/* 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 @@
/* 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 @@
/* 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);
--
To view, visit https://gerrit.osmocom.org/5862
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idcbf5775d17b1247f2ed01788f9b0788ce66e871
Gerrit-PatchSet: 3
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>