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.orgHello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/5367 to look at the new patch set (#2). 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. 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(+), 32 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/67/5367/2 diff --git a/src/bts.cpp b/src/bts.cpp index 867658e..12890f6 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -1031,7 +1031,7 @@ 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, " @@ -1042,12 +1042,9 @@ new_tbf->set_state(GPRS_RLCMAC_FLOW); /* stop pending assignment timer */ new_tbf->stop_timer("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 +1065,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 dc0777f..8a59478 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); tbf->t_start(T3169, bts->t3169, 0, "allocation (UL-TBF)", true); tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF); OSMO_ASSERT(tbf->ms()); @@ -1537,8 +1536,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 4489695..1d555d4 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -175,6 +175,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; @@ -372,12 +374,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() @@ -455,7 +478,7 @@ int rcvd_dl_ack(uint8_t 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 b043989..49cda98 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -494,9 +494,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 */ tbf_timer_start(this, 0, Tassign_pacch, "assignment (PACCH)"); @@ -506,8 +504,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()); @@ -635,18 +632,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; } @@ -857,7 +848,7 @@ if (is_final) t_start(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 8e4e57e..43589f5 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: newpatchset Gerrit-Change-Id: Ic4560280c72f91700f2e19c6c7f6658dc29625c2 Gerrit-PatchSet: 2 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