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/2858 to look at the new patch set (#2). Cleanup TBF state handling * use OSMO_BIT_* macros for manipulating TBF state flags to improve code readability and make state change tracking easier * move copy-pasted code into inline function Change-Id: Icca5ce3273799e0165adcc062db8a4b71dd9fb04 Related: OS#1524 --- 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, 53 insertions(+), 56 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/58/2858/2 diff --git a/src/bts.cpp b/src/bts.cpp index 1d27284..3bc02a2 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -651,7 +651,7 @@ } else { tbf->set_ta(ta); tbf->set_state(GPRS_RLCMAC_FLOW); - tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_CCCH); + OSMO_BIT_SET(tbf->state_flags, GPRS_RLCMAC_FLAG_CCCH); tbf_timer_start(tbf, 3169, m_bts.t3169, 0); LOGP(DRLCMAC, LOGL_DEBUG, "%s [UPLINK] START\n", tbf_name(tbf)); @@ -788,8 +788,8 @@ /* change state */ dl_tbf->set_state(GPRS_RLCMAC_ASSIGN); - if (!(dl_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) - dl_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH); + if (!OSMO_BIT_GET(dl_tbf->state_flags, GPRS_RLCMAC_FLAG_CCCH)) + OSMO_BIT_SET(dl_tbf->state_flags, GPRS_RLCMAC_FLAG_PACCH); /* start timer */ tbf_timer_start(dl_tbf, 0, Tassign_pacch); } else { @@ -797,7 +797,7 @@ dl_tbf->was_releasing = dl_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE); /* change state */ dl_tbf->set_state(GPRS_RLCMAC_ASSIGN); - dl_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_CCCH); + OSMO_BIT_SET(dl_tbf->state_flags, GPRS_RLCMAC_FLAG_CCCH); /* send immediate assignment */ dl_tbf->bts->snd_dl_ass(dl_tbf, 0, dl_tbf->imsi()); dl_tbf->m_wait_confirm = 1; @@ -1007,10 +1007,8 @@ if (tbf->ul_ack_state == GPRS_RLCMAC_UL_ACK_WAIT_ACK) { LOGP(DRLCMAC, LOGL_DEBUG, "TBF: [UPLINK] END %s\n", tbf_name(tbf)); tbf->ul_ack_state = GPRS_RLCMAC_UL_ACK_NONE; - if ((tbf->state_flags & - (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK))) { - tbf->state_flags &= - ~(1 << GPRS_RLCMAC_FLAG_TO_UL_ACK); + if (OSMO_BIT_GET(tbf->state_flags, GPRS_RLCMAC_FLAG_TO_UL_ACK)) { + OSMO_BIT_CLEAR(tbf->state_flags, GPRS_RLCMAC_FLAG_TO_UL_ACK); LOGP(DRLCMAC, LOGL_NOTICE, "Recovered uplink " "ack for UL %s\n", tbf_name(tbf)); } @@ -1033,22 +1031,20 @@ tbf->direction == new_tbf->direction) tbf_free(tbf); - if ((new_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) { + if (OSMO_BIT_GET(new_tbf->state_flags, GPRS_RLCMAC_FLAG_CCCH)) { /* We now know that the PACCH really existed */ LOGP(DRLCMAC, LOGL_INFO, "The TBF has been confirmed on the PACCH, " "changed type from CCCH to PACCH for %s\n", tbf_name(new_tbf)); - new_tbf->state_flags &= ~(1 << GPRS_RLCMAC_FLAG_CCCH); - new_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH); + OSMO_BIT_CLEAR(new_tbf->state_flags, GPRS_RLCMAC_FLAG_CCCH); + OSMO_BIT_SET(new_tbf->state_flags, GPRS_RLCMAC_FLAG_PACCH); } new_tbf->set_state(GPRS_RLCMAC_FLOW); /* stop pending assignment timer */ new_tbf->stop_timer(); - if ((new_tbf->state_flags & - (1 << GPRS_RLCMAC_FLAG_TO_DL_ASS))) { - new_tbf->state_flags &= - ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ASS); + if (OSMO_BIT_GET(new_tbf->state_flags, GPRS_RLCMAC_FLAG_TO_DL_ASS)) { + OSMO_BIT_CLEAR(new_tbf->state_flags, GPRS_RLCMAC_FLAG_TO_DL_ASS); LOGP(DRLCMAC, LOGL_NOTICE, "Recovered downlink " "assignment for %s\n", tbf_name(new_tbf)); } @@ -1072,10 +1068,8 @@ 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 (OSMO_BIT_GET(new_tbf->state_flags, GPRS_RLCMAC_FLAG_TO_UL_ASS)) { + OSMO_BIT_CLEAR(new_tbf->state_flags, GPRS_RLCMAC_FLAG_TO_UL_ASS); LOGP(DRLCMAC, LOGL_NOTICE, "Recovered uplink " "assignment for UL %s\n", tbf_name(new_tbf)); } @@ -1150,6 +1144,15 @@ } } +static inline void try_dl_ack_recovery(struct gprs_rlcmac_dl_tbf *tbf) +{ + OSMO_BIT_SET(tbf->state_flags, GPRS_RLCMAC_FLAG_DL_ACK); + if (OSMO_BIT_GET(tbf->state_flags, GPRS_RLCMAC_FLAG_TO_DL_ACK)) { + OSMO_BIT_CLEAR(tbf->state_flags, GPRS_RLCMAC_FLAG_TO_DL_ACK); + LOGP(DRLCMAC, LOGL_NOTICE, "Recovered downlink ack for %s\n", tbf_name(tbf)); + } +} + void gprs_rlcmac_pdch::rcv_control_dl_ack_nack(Packet_Downlink_Ack_Nack_t *ack_nack, uint32_t fn) { int8_t tfi = 0; /* must be signed */ @@ -1175,12 +1178,9 @@ "wrong TFI=%d, ignoring!\n", tfi); return; } - tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_DL_ACK); - if ((tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK))) { - tbf->state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); - LOGP(DRLCMAC, LOGL_NOTICE, "Recovered downlink ack " - "for %s\n", tbf_name(tbf)); - } + + try_dl_ack_recovery(tbf); + /* reset N3105 */ tbf->n3105 = 0; tbf->stop_t3191(); @@ -1263,12 +1263,9 @@ "wrong TFI=%d, ignoring!\n", tfi); return; } - tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_DL_ACK); - if ((tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK))) { - tbf->state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); - LOGP(DRLCMAC, LOGL_NOTICE, "Recovered EGPRS downlink ack " - "for %s\n", tbf_name(tbf)); - } + + try_dl_ack_recovery(tbf); + /* reset N3105 */ tbf->n3105 = 0; tbf->stop_t3191(); diff --git a/src/tbf.cpp b/src/tbf.cpp index 48e8289..5a38c49 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -393,7 +393,7 @@ } tbf->m_contention_resolution_done = 1; tbf->set_state(GPRS_RLCMAC_ASSIGN); - tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH); + OSMO_BIT_SET(tbf->state_flags, GPRS_RLCMAC_FLAG_PACCH); tbf_timer_start(tbf, 3169, bts->t3169, 0); tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF); OSMO_ASSERT(tbf->ms()); @@ -613,11 +613,11 @@ poll_state = GPRS_RLCMAC_POLL_NONE; if (ul_ack_state == GPRS_RLCMAC_UL_ACK_WAIT_ACK) { - if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK))) { + if (!OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_TO_UL_ACK)) { LOGP(DRLCMAC, LOGL_NOTICE, "- Timeout for polling " "PACKET CONTROL ACK for PACKET UPLINK ACK\n"); rlcmac_diag(); - state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK); + OSMO_BIT_SET(state_flags, GPRS_RLCMAC_FLAG_TO_UL_ACK); } ul_ack_state = GPRS_RLCMAC_UL_ACK_NONE; bts->rlc_ack_timedout(); @@ -638,12 +638,12 @@ } } else if (ul_ass_state == GPRS_RLCMAC_UL_ASS_WAIT_ACK) { - if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ASS))) { + if (!OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_TO_UL_ASS)) { LOGP(DRLCMAC, LOGL_NOTICE, "- Timeout for polling " "PACKET CONTROL ACK for PACKET UPLINK " "ASSIGNMENT.\n"); rlcmac_diag(); - state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ASS); + OSMO_BIT_SET(state_flags, GPRS_RLCMAC_FLAG_TO_UL_ASS); } ul_ass_state = GPRS_RLCMAC_UL_ASS_NONE; n3105++; @@ -660,12 +660,12 @@ /* reschedule UL assignment */ ul_ass_state = GPRS_RLCMAC_UL_ASS_SEND_ASS; } else if (dl_ass_state == GPRS_RLCMAC_DL_ASS_WAIT_ACK) { - if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ASS))) { + if (!OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_TO_DL_ASS)) { LOGP(DRLCMAC, LOGL_NOTICE, "- Timeout for polling " "PACKET CONTROL ACK for PACKET DOWNLINK " "ASSIGNMENT.\n"); rlcmac_diag(); - state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_DL_ASS); + OSMO_BIT_SET(state_flags, GPRS_RLCMAC_FLAG_TO_DL_ASS); } dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE; n3105++; @@ -684,11 +684,11 @@ } else if (direction == GPRS_RLCMAC_DL_TBF) { gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(this); - if (!(dl_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK))) { + if (!OSMO_BIT_GET(dl_tbf->state_flags, GPRS_RLCMAC_FLAG_TO_DL_ACK)) { LOGP(DRLCMAC, LOGL_NOTICE, "- Timeout for polling " "PACKET DOWNLINK ACK.\n"); dl_tbf->rlcmac_diag(); - dl_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); + OSMO_BIT_SET(dl_tbf->state_flags, GPRS_RLCMAC_FLAG_TO_DL_ACK); } dl_tbf->n3105++; if (dl_tbf->state_is(GPRS_RLCMAC_RELEASING)) @@ -706,8 +706,8 @@ return; } /* resend IMM.ASS on CCCH on timeout */ - if ((dl_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH)) - && !(dl_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_DL_ACK))) { + if (OSMO_BIT_GET(dl_tbf->state_flags, GPRS_RLCMAC_FLAG_CCCH) + && !OSMO_BIT_GET(dl_tbf->state_flags, GPRS_RLCMAC_FLAG_DL_ACK)) { LOGP(DRLCMAC, LOGL_DEBUG, "Re-send dowlink assignment " "for %s on PCH (IMSI=%s)\n", tbf_name(dl_tbf), @@ -961,7 +961,7 @@ switch (T) { case 0: /* assignment */ - if ((state_flags & (1 << GPRS_RLCMAC_FLAG_PACCH))) { + if (OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_PACCH)) { if (state_is(GPRS_RLCMAC_ASSIGN)) { LOGP(DRLCMAC, LOGL_NOTICE, "%s releasing due to " "PACCH assignment timeout.\n", tbf_name(this)); @@ -971,7 +971,7 @@ LOGP(DRLCMAC, LOGL_ERROR, "Error: %s is not " "in assign state\n", tbf_name(this)); } - if ((state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) { + if (OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_CCCH)) { gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(this); dl_tbf->m_wait_confirm = 0; if (dl_tbf->state_is(GPRS_RLCMAC_ASSIGN)) { @@ -1022,15 +1022,15 @@ int gprs_rlcmac_tbf::rlcmac_diag() { - if ((state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) + if (OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_CCCH)) LOGP(DRLCMAC, LOGL_NOTICE, "- Assignment was on CCCH\n"); - if ((state_flags & (1 << GPRS_RLCMAC_FLAG_PACCH))) + if (OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_PACCH)) LOGP(DRLCMAC, LOGL_NOTICE, "- Assignment was on PACCH\n"); - if ((state_flags & (1 << GPRS_RLCMAC_FLAG_UL_DATA))) + if (OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_UL_DATA)) LOGP(DRLCMAC, LOGL_NOTICE, "- Uplink data was received\n"); else if (direction == GPRS_RLCMAC_UL_TBF) LOGP(DRLCMAC, LOGL_NOTICE, "- No uplink data received yet\n"); - if ((state_flags & (1 << GPRS_RLCMAC_FLAG_DL_ACK))) + if (OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_DL_ACK)) LOGP(DRLCMAC, LOGL_NOTICE, "- Downlink ACK was received\n"); else if (direction == GPRS_RLCMAC_DL_TBF) LOGP(DRLCMAC, LOGL_NOTICE, "- No downlink ACK received yet\n"); @@ -1427,7 +1427,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); + OSMO_BIT_SET(ul_tbf->state_flags, GPRS_RLCMAC_FLAG_PACCH); 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 09e3122..46c43a2 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -377,7 +377,7 @@ return state > GPRS_RLCMAC_ASSIGN || (direction == GPRS_RLCMAC_DL_TBF && state == GPRS_RLCMAC_ASSIGN && - (state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))); + OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_CCCH)); } inline uint8_t gprs_rlcmac_tbf::tfi() const diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index 24c6385..0efa893 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -777,7 +777,7 @@ if (m_last_dl_poll_fn < 0) m_last_dl_poll_fn = fn; - need_poll = state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); + need_poll = OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_TO_DL_ACK); /* poll after POLL_ACK_AFTER_FRAMES frames, or when final block is tx. */ @@ -808,7 +808,7 @@ tbf_timer_start(this, 3191, bts_data()->t3191, 0); /* Clear poll timeout flag */ - state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); + OSMO_BIT_CLEAR(state_flags, GPRS_RLCMAC_FLAG_TO_DL_ACK); /* Clear request flag */ m_dl_ack_requested = false; @@ -1080,7 +1080,7 @@ /* keep to flags */ state_flags &= GPRS_RLCMAC_FLAG_TO_MASK; - state_flags &= ~(1 << GPRS_RLCMAC_FLAG_CCCH); + OSMO_BIT_CLEAR(state_flags, GPRS_RLCMAC_FLAG_CCCH); return 0; } @@ -1108,7 +1108,7 @@ /* keep to flags */ state_flags &= GPRS_RLCMAC_FLAG_TO_MASK; - state_flags &= ~(1 << GPRS_RLCMAC_FLAG_CCCH); + OSMO_BIT_CLEAR(state_flags, GPRS_RLCMAC_FLAG_CCCH); return 0; } @@ -1159,7 +1159,7 @@ if (poll_state != GPRS_RLCMAC_POLL_NONE) return false; - return state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK) || + return OSMO_BIT_GET(state_flags, GPRS_RLCMAC_FLAG_TO_DL_ACK) || m_tx_counter >= POLL_ACK_AFTER_FRAMES || m_dl_ack_requested; } diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp index 1eee41a..7c9f2f4 100644 --- a/src/tbf_ul.cpp +++ b/src/tbf_ul.cpp @@ -156,7 +156,7 @@ const uint16_t ws = m_window.ws(); - this->state_flags |= (1 << GPRS_RLCMAC_FLAG_UL_DATA); + OSMO_BIT_SET(this->state_flags, GPRS_RLCMAC_FLAG_UL_DATA); LOGP(DRLCMACUL, LOGL_DEBUG, "UL DATA TFI=%d received (V(Q)=%d .. " "V(R)=%d)\n", rlc->tfi, this->m_window.v_q(), -- To view, visit https://gerrit.osmocom.org/2858 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icca5ce3273799e0165adcc062db8a4b71dd9fb04 Gerrit-PatchSet: 2 Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Owner: Max <msuraev at sysmocom.de> Gerrit-Reviewer: Jenkins Builder