pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/29914 ) Change subject: Refactor and clarify tbf->triger_ass() code ...................................................................... Refactor and clarify tbf->triger_ass() code Split the function into 2 functions, one for assignment on PACCH and one for assignment on PCH. This makes code calling this API far more clearer on what is the exact aim when assigning the TBF. Change-Id: Ic92867e55337b0bd6b5bfc97f13b7982eedb1cb7 --- M src/gprs_ms.c M src/tbf.cpp M src/tbf.h M src/tbf_dl.cpp M src/tbf_dl.h M src/tbf_fsm.c 6 files changed, 43 insertions(+), 28 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/src/gprs_ms.c b/src/gprs_ms.c index 5733a7c..c072b0a 100644 --- a/src/gprs_ms.c +++ b/src/gprs_ms.c @@ -1085,7 +1085,10 @@ LOGPTBFDL(dl_tbf, LOGL_DEBUG, "[DOWNLINK] START\n"); /* Trigger the assignment now. */ - tbf_dl_trigger_ass(dl_tbf, ul_tbf_as_tbf(ul_tbf)); + if (ul_tbf) + dl_tbf_trigger_ass_on_pacch(dl_tbf, ul_tbf_as_tbf(ul_tbf)); + else + dl_tbf_trigger_ass_on_pch(dl_tbf); return 0; } diff --git a/src/tbf.cpp b/src/tbf.cpp index d115b82..4bf4ce8 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -703,7 +703,7 @@ } LOGPTBF(this, LOGL_DEBUG, "Trigger downlink assignment on PACCH\n"); - new_tbf->trigger_ass(this); + dl_tbf_trigger_ass_on_pacch(new_tbf, this); return 0; } @@ -960,3 +960,8 @@ { return tbf->trx; } + +void tbf_stop_timers(struct gprs_rlcmac_tbf *tbf, const char *reason) +{ + tbf->stop_timers(reason); +} \ No newline at end of file diff --git a/src/tbf.h b/src/tbf.h index d69ae5a..09de188 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -150,6 +150,7 @@ int tbf_update(struct gprs_rlcmac_tbf *tbf); int tbf_establish_dl_tbf_on_pacch(struct gprs_rlcmac_tbf *tbf); struct gprs_rlcmac_trx *tbf_get_trx(struct gprs_rlcmac_tbf *tbf); +void tbf_stop_timers(struct gprs_rlcmac_tbf *tbf, const char *reason); #ifdef __cplusplus } #endif diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index 0936e1c..856dae6 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -418,29 +418,41 @@ return create_dl_acked_block(fn, ts, bsn, bsn2); } -/* depending on the current TBF, we assign on PACCH or AGCH */ -void gprs_rlcmac_dl_tbf::trigger_ass(struct gprs_rlcmac_tbf *old_tbf) +/* old_tbf (UL TBF or DL TBF) will send a Pkt Dl Ass on PACCH to assign tbf. + * Note: It is possible that "tbf == old_tbf" if the TBF is being updated. This can + * happen when we first assign over PCH (only single slot is possible) and we want + * to upgrade the DL-TBF to be multislot. See code calling tbf_update() for more + * information. + */ +void dl_tbf_trigger_ass_on_pacch(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf) +{ + OSMO_ASSERT(tbf); + OSMO_ASSERT(old_tbf); + /* stop pending timer */ + tbf_stop_timers(tbf, "DL assignment (PACCH)"); + + LOGPTBFDL(tbf, LOGL_DEBUG, "Send downlink assignment on PACCH, because %s exists\n", old_tbf->name()); + osmo_fsm_inst_dispatch(old_tbf->dl_ass_fsm.fi, TBF_DL_ASS_EV_SCHED_ASS, NULL); + + /* change state */ + osmo_fsm_inst_dispatch(tbf->state_fsm.fi, TBF_EV_ASSIGN_ADD_PACCH, NULL); + +} + +void dl_tbf_trigger_ass_on_pch(struct gprs_rlcmac_dl_tbf *tbf) { /* stop pending timer */ - stop_timers("assignment (DL-TBF)"); + struct GprsMs *ms = tbf_ms(tbf); - /* check for downlink tbf: */ - if (old_tbf) { - LOGPTBFDL(this, LOGL_DEBUG, "Send downlink assignment on PACCH, because %s exists\n", old_tbf->name()); - osmo_fsm_inst_dispatch(old_tbf->dl_ass_fsm.fi, TBF_DL_ASS_EV_SCHED_ASS, NULL); + tbf_stop_timers(tbf, "DL assignment (PCH)"); - /* change state */ - osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_ASSIGN_ADD_PACCH, NULL); - } else { - LOGPTBFDL(this, LOGL_DEBUG, "Send downlink assignment on PCH, no TBF exist (IMSI=%s)\n", - imsi()); + LOGPTBFDL(tbf, LOGL_DEBUG, "Send downlink assignment on PCH, no TBF exist (IMSI=%s)\n", ms_imsi(ms)); - /* change state */ - osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_ASSIGN_ADD_CCCH, NULL); + /* change state */ + osmo_fsm_inst_dispatch(tbf->state_fsm.fi, TBF_EV_ASSIGN_ADD_CCCH, NULL); - /* send immediate assignment */ - bts_snd_dl_ass(bts, this); - } + /* send immediate assignment */ + bts_snd_dl_ass(ms->bts, tbf); } void gprs_rlcmac_dl_tbf::schedule_next_frame() @@ -1272,8 +1284,3 @@ else return NULL; } - -void tbf_dl_trigger_ass(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf) -{ - return tbf->trigger_ass(old_tbf); -} diff --git a/src/tbf_dl.h b/src/tbf_dl.h index e0daf57..ba5d582 100644 --- a/src/tbf_dl.h +++ b/src/tbf_dl.h @@ -41,7 +41,6 @@ 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, enum mcs_kind req_mcs_kind = EGPRS); - void trigger_ass(struct gprs_rlcmac_tbf *old_tbf); void request_dl_ack(); bool need_poll_for_dl_ack_nack() const; @@ -138,7 +137,8 @@ const uint8_t egprs_ms_class, const uint16_t delay_csec, const uint8_t *data, const uint16_t len); -void tbf_dl_trigger_ass(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf); +void dl_tbf_trigger_ass_on_pacch(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf); +void dl_tbf_trigger_ass_on_pch(struct gprs_rlcmac_dl_tbf *tbf); void tbf_dl_request_dl_ack(struct gprs_rlcmac_dl_tbf *tbf); static inline struct gprs_rlcmac_tbf *dl_tbf_as_tbf(struct gprs_rlcmac_dl_tbf *dl_tbf) diff --git a/src/tbf_fsm.c b/src/tbf_fsm.c index 25253fd..eeccd6b 100644 --- a/src/tbf_fsm.c +++ b/src/tbf_fsm.c @@ -387,8 +387,7 @@ ctx->state_flags &= GPRS_RLCMAC_FLAG_TO_MASK; tbf_update(ctx->tbf); - - tbf_dl_trigger_ass(dl_tbf, ctx->tbf); + dl_tbf_trigger_ass_on_pacch(dl_tbf, ctx->tbf); } else LOGPTBF(ctx->tbf, LOGL_NOTICE, "Continue flow after IMM.ASS confirm\n"); } -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/29914 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ic92867e55337b0bd6b5bfc97f13b7982eedb1cb7 Gerrit-Change-Number: 29914 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged