From 528820dbe13d47de6f92e22993b6704f5e61b2e4 Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Mon, 26 Oct 2020 12:40:11 +0100 Subject: tbf: Clean up gprs_rlcmac_dl_tbf::handle() Avoid passing tons of params to internal helper function tbf_nel_dl_assignment() in order to either fetch again the ms object or create a new one. Let's instead create the ms function earlier if needed and fill it with all the discovered information prior to calling the helper function. This provides cleaner code and also better log output. This way we also avoid trying to fill the MS twice and unneeded getter+setter for TA. tbf::imsi(): There' always an ms, so simply forward call to ms()->imsi(). We can also get rid of assign_imsi, since the modified code is the only place where it's used and there's already some code in place there to update the MS. We instead merge it with set_imsi and keep the duplication code to catch possible bugs from callers. Move merge_and_clear_ms from tbf class to GprsMS, where it really belongs. Change-Id: Id18098bac3cff26fc4a8d2f419e21641a1f4c83b --- src/tbf_dl.cpp | 40 +++++++++------------------------------- 1 file changed, 9 insertions(+), 31 deletions(-) (limited to 'src/tbf_dl.cpp') diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index b41c161c..7d1f85c0 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -251,27 +251,15 @@ int gprs_rlcmac_dl_tbf::append_data(const uint8_t ms_class, return 0; } -static int tbf_new_dl_assignment(struct gprs_rlcmac_bts *bts, - const char *imsi, - const uint32_t tlli, const uint32_t tlli_old, - const uint8_t ms_class, - const uint8_t egprs_ms_class, - struct gprs_rlcmac_dl_tbf **tbf) +static int tbf_new_dl_assignment(struct gprs_rlcmac_bts *bts, GprsMs *ms, + struct gprs_rlcmac_dl_tbf **tbf) { bool ss; int8_t use_trx; - uint8_t ta = GSM48_TA_INVALID; struct gprs_rlcmac_ul_tbf *ul_tbf = NULL, *old_ul_tbf; struct gprs_rlcmac_dl_tbf *dl_tbf = NULL; - GprsMs *ms; - - /* check for uplink data, so we copy our informations */ - ms = bts->bts->ms_store().get_ms(tlli, tlli_old, imsi); - if (!ms) - ms = bts->bts->ms_alloc(ms_class, egprs_ms_class); /* ms class updated later */ ul_tbf = ms->ul_tbf(); - ta = ms->ta(); if (ul_tbf && ul_tbf->m_contention_resolution_done && !ul_tbf->m_final_ack_sent) { @@ -293,14 +281,9 @@ static int tbf_new_dl_assignment(struct gprs_rlcmac_bts *bts, LOGP(DTBF, LOGL_NOTICE, "No PDCH resource\n"); return -EBUSY; } - dl_tbf->update_ms(tlli, GPRS_RLCMAC_DL_TBF); - dl_tbf->ms()->set_ta(ta); LOGPTBFDL(dl_tbf, LOGL_DEBUG, "[DOWNLINK] START\n"); - /* Store IMSI for later look-up and PCH retransmission */ - dl_tbf->assign_imsi(imsi); - /* trigger downlink assignment and set state to ASSIGN. * we don't use old_downlink, so the possible uplink is used * to trigger downlink assignment. if there is no uplink, @@ -354,28 +337,23 @@ int gprs_rlcmac_dl_tbf::handle(struct gprs_rlcmac_bts *bts, /* Move the DL TBF to the new MS */ dl_tbf->set_ms(ms); } - /* Clean up the old MS object */ - /* TODO: Put this into a separate function, use timer? */ - if (ms_old->ul_tbf() && !ms_old->ul_tbf()->timers_pending(T_MAX)) - tbf_free(ms_old->ul_tbf()); - if (ms_old->dl_tbf() && !ms_old->dl_tbf()->timers_pending(T_MAX)) - tbf_free(ms_old->dl_tbf()); - - ms->merge_old_ms(ms_old); + ms->merge_and_clear_ms(ms_old); } } + if (!ms) + ms = bts->bts->ms_alloc(ms_class, egprs_ms_class); + ms->set_imsi(imsi); + ms->confirm_tlli(tlli); + if (!dl_tbf) { - rc = tbf_new_dl_assignment(bts, imsi, tlli, tlli_old, - ms_class, egprs_ms_class, &dl_tbf); + rc = tbf_new_dl_assignment(bts, ms, &dl_tbf); if (rc < 0) return rc; } /* TODO: ms_class vs. egprs_ms_class is not handled here */ rc = dl_tbf->append_data(ms_class, delay_csec, data, len); - dl_tbf->update_ms(tlli, GPRS_RLCMAC_DL_TBF); - dl_tbf->assign_imsi(imsi); return rc; } -- cgit v1.2.3