pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33280 )

Change subject: Move call to bts_snd_dl_ass() from tbf_dl.cpp to tbf_dl_fsm.c ......................................................................

Move call to bts_snd_dl_ass() from tbf_dl.cpp to tbf_dl_fsm.c

It makes more sense to have it there, where it is transmitted with more control depending on current state. Furthermore, it's counterpart/continuation TBF_EV_ASSIGN_PCUIF_CNF logic is already there, so it makes sense to have the whole logic together.

Change-Id: I8af39d3087ccf197321f0c71cb29b67adbe1f36e --- M src/tbf_dl.cpp M src/tbf_dl_fsm.c 2 files changed, 26 insertions(+), 10 deletions(-)

Approvals: Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve laforge: Looks good to me, approved

diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index 17d2934..40442c3 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -485,9 +485,6 @@

/* change state */ osmo_fsm_inst_dispatch(tbf->state_fi, TBF_EV_ASSIGN_ADD_CCCH, NULL); - - /* send immediate assignment */ - bts_snd_dl_ass(ms->bts, tbf); }

int dl_tbf_upgrade_to_multislot(struct gprs_rlcmac_dl_tbf *dl_tbf) diff --git a/src/tbf_dl_fsm.c b/src/tbf_dl_fsm.c index a977507..02e1ff5 100644 --- a/src/tbf_dl_fsm.c +++ b/src/tbf_dl_fsm.c @@ -104,6 +104,7 @@ static void st_assign_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state) { struct tbf_dl_fsm_ctx *ctx = (struct tbf_dl_fsm_ctx *)fi->priv; + struct GprsMs *ms = tbf_ms(ctx->tbf); unsigned long val; unsigned int sec, micro;

@@ -125,20 +126,29 @@ sec, micro); osmo_timer_schedule(&fi->timer, sec, micro); } else { - /* GPRS_RLCMAC_FLAG_CCCH is set, so here we submitted an DL Ass + /* GPRS_RLCMAC_FLAG_CCCH is set, so here we submit a DL Ass * through PCUIF on CCCH */ + OSMO_ASSERT(ctx->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH)); + /* Send CCCH (PCH) Immediate Assignment over PCUIF: */ + bts_snd_dl_ass(ms->bts, ctx->dl_tbf); } }

static void st_assign(struct osmo_fsm_inst *fi, uint32_t event, void *data) { struct tbf_dl_fsm_ctx *ctx = (struct tbf_dl_fsm_ctx *)fi->priv; + struct GprsMs *ms; unsigned long val; unsigned int sec, micro;

switch (event) { case TBF_EV_ASSIGN_ADD_CCCH: + /* Note: This code path is not really used nowadays, since ADD_CCCH is + * only dispatched during dl_tbf allocation (st=NEW) */ + ms = tbf_ms(ctx->tbf); mod_ass_type(ctx, GPRS_RLCMAC_FLAG_CCCH, true); + /* Re-send CCCH (PCH) Immediate Assignment over PCUIF: */ + bts_snd_dl_ass(ms->bts, ctx->dl_tbf); break; case TBF_EV_ASSIGN_ADD_PACCH: mod_ass_type(ctx, GPRS_RLCMAC_FLAG_PACCH, true); @@ -210,13 +220,8 @@ * from continuing and start assignment on CCCH again */ if ((ctx->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH)) && !dl_tbf_first_dl_ack_rcvd(ctx->dl_tbf)) { - struct GprsMs *ms = tbf_ms(ctx->tbf); - LOGPTBFDL(ctx->dl_tbf, LOGL_DEBUG, - "Re-send downlink assignment on PCH (IMSI=%s)\n", - ms_imsi_is_valid(ms) ? ms_imsi(ms) : ""); + LOGPTBFDL(ctx->dl_tbf, LOGL_DEBUG, "Retransmit ImmAss[PktDlAss] on PCH\n"); tbf_dl_fsm_state_chg(fi, TBF_ST_ASSIGN); - /* send immediate assignment */ - bts_snd_dl_ass(ms->bts, ctx->dl_tbf); } break; case TBF_EV_LAST_DL_DATA_SENT: