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/.
Harald Welte gerrit-no-reply at lists.osmocom.orgHarald Welte has submitted this change and it was merged. Change subject: TBF: cleanup state flag handling ...................................................................... TBF: cleanup state flag handling * introduce generic function to check whether particular flag was set for'a TBF and clear it if necessary. Use this instead of clear_poll_timeout_flag() * add function to explicitly set assignment and appropriate state flags Overall this makes the code easier to read and debug. Related: OS#1759 Change-Id: Ic4560280c72f91700f2e19c6c7f6658dc29625c2 --- M src/bts.cpp M src/tbf.cpp M src/tbf.h M src/tbf_dl.cpp M src/tbf_ul.cpp 5 files changed, 36 insertions(+), 33 deletions(-) Approvals: Stefan Sperling: Looks good to me, but someone else must approve Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/bts.cpp b/src/bts.cpp index 4bc792a..37a9c80 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -1031,23 +1031,19 @@ tbf->direction == new_tbf->direction) tbf_free(tbf); - if ((new_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) { + if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_CCCH)) { /* We now know that the PACCH really existed */ LOGPTBF(new_tbf, LOGL_INFO, "The TBF has been confirmed on the PACCH, " "changed type from CCCH to PACCH\n"); - new_tbf->state_flags &= ~(1 << GPRS_RLCMAC_FLAG_CCCH); new_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH); } new_tbf->set_state(GPRS_RLCMAC_FLOW); /* stop pending assignment timer */ 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 &= - ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ASS); + if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_TO_DL_ASS)) LOGPTBF(new_tbf, LOGL_NOTICE, "Recovered downlink assignment\n"); - } + tbf_assign_control_ts(new_tbf); return; } @@ -1068,12 +1064,9 @@ tbf_free(tbf); new_tbf->set_state(GPRS_RLCMAC_FLOW); - if ((new_tbf->state_flags & - (1 << GPRS_RLCMAC_FLAG_TO_UL_ASS))) { - new_tbf->state_flags &= - ~(1 << GPRS_RLCMAC_FLAG_TO_UL_ASS); + if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_TO_UL_ASS)) LOGPTBF(new_tbf, LOGL_NOTICE, "Recovered uplink assignment for UL\n"); - } + tbf_assign_control_ts(new_tbf); /* there might be LLC packets waiting in the queue, but the DL * TBF might have been released while the UL TBF has been diff --git a/src/tbf.cpp b/src/tbf.cpp index 9e3a8ef..6847e18 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -402,8 +402,7 @@ return NULL; } tbf->m_contention_resolution_done = 1; - tbf->set_state(GPRS_RLCMAC_ASSIGN); - tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH); + tbf->set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, false); T_START(tbf, T3169, bts->t3169, 0, "allocation (UL-TBF)", true); tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF); OSMO_ASSERT(tbf->ms()); @@ -1511,8 +1510,7 @@ llist_add(&ul_tbf->list(), &bts->bts->ul_tbfs()); ul_tbf->bts->tbf_ul_created(); - ul_tbf->set_state(GPRS_RLCMAC_ASSIGN); - ul_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH); + ul_tbf->set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, false); ul_tbf->set_ms(ms); ul_tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF); diff --git a/src/tbf.h b/src/tbf.h index 943ec92..6d7edbc 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -180,6 +180,8 @@ bool state_is(enum gprs_rlcmac_tbf_state rhs) const; bool state_is_not(enum gprs_rlcmac_tbf_state rhs) const; void set_state(enum gprs_rlcmac_tbf_state new_state); + bool check_n_clear(uint8_t state_flag); + void set_assigned_on(uint8_t state_flag, bool check_ccch); const char *state_name() const; const char *name() const; @@ -368,12 +370,33 @@ return tbf_state_name[state]; } +/* Set assignment state and corrsponding flags */ +inline void gprs_rlcmac_tbf::set_assigned_on(uint8_t state_flag, bool check_ccch) +{ + set_state(GPRS_RLCMAC_ASSIGN); + if (check_ccch) { + if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) + state_flags |= (1 << state_flag); + } else + state_flags |= (1 << state_flag); +} + inline void gprs_rlcmac_tbf::set_state(enum gprs_rlcmac_tbf_state new_state) { LOGP(DRLCMAC, LOGL_DEBUG, "%s changes state from %s to %s\n", tbf_name(this), tbf_state_name[state], tbf_state_name[new_state]); state = new_state; +} + +inline bool gprs_rlcmac_tbf::check_n_clear(uint8_t state_flag) +{ + if ((state_flags & (1 << state_flag))) { + state_flags &= ~(1 << state_flag); + return true; + } + + return false; } inline LListHead<gprs_rlcmac_tbf>& gprs_rlcmac_tbf::list() @@ -451,7 +474,7 @@ int rcvd_dl_ack(bool final_ack, unsigned first_bsn, struct bitvec *rbb); struct msgb *create_dl_acked_block(uint32_t fn, uint8_t ts); void trigger_ass(struct gprs_rlcmac_tbf *old_tbf); - void clear_poll_timeout_flag(); + bool handle_ack_nack(); void request_dl_ack(); bool need_control_ts() const; diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index b871bc3..e3b0a9d 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -493,9 +493,7 @@ old_tbf->was_releasing = old_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE); /* change state */ - set_state(GPRS_RLCMAC_ASSIGN); - if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) - state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH); + set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, true); /* start timer */ T_START(this, T0, T_ASS_PACCH_SEC, 0, "assignment (PACCH)", true); @@ -505,8 +503,7 @@ was_releasing = state_is(GPRS_RLCMAC_WAIT_RELEASE); /* change state */ - set_state(GPRS_RLCMAC_ASSIGN); - state_flags |= (1 << GPRS_RLCMAC_FLAG_CCCH); + set_assigned_on(GPRS_RLCMAC_FLAG_CCCH, false); /* send immediate assignment */ bts->snd_dl_ass(this, 0, imsi()); @@ -634,18 +631,12 @@ return bsn; } -void gprs_rlcmac_dl_tbf::clear_poll_timeout_flag() -{ - state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); -} - bool gprs_rlcmac_dl_tbf::handle_ack_nack() { bool ack_recovered = false; state_flags |= (1 << GPRS_RLCMAC_FLAG_DL_ACK); - if ((state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK))) { - clear_poll_timeout_flag(); + if (check_n_clear(GPRS_RLCMAC_FLAG_TO_DL_ACK)) { ack_recovered = true; } @@ -856,7 +847,7 @@ if (is_final) T_START(this, T3191, bts_data()->t3191, 0, "final block (DL-TBF)", true); - clear_poll_timeout_flag(); + state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); /* clear poll timeout flag */ /* Clear request flag */ m_dl_ack_requested = false; diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp index 45de7cd..6442b8f 100644 --- a/src/tbf_ul.cpp +++ b/src/tbf_ul.cpp @@ -96,10 +96,8 @@ bool gprs_rlcmac_ul_tbf::ctrl_ack_to_toggle() { - if ((state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK))) { - state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_UL_ACK); + if (check_n_clear(GPRS_RLCMAC_FLAG_TO_UL_ACK)) return true; /* GPRS_RLCMAC_FLAG_TO_UL_ACK was set, now cleared */ - } state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK); return false; /* GPRS_RLCMAC_FLAG_TO_UL_ACK was unset, now set */ -- To view, visit https://gerrit.osmocom.org/5367 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ic4560280c72f91700f2e19c6c7f6658dc29625c2 Gerrit-PatchSet: 6 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> Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>