[M] Change in libosmo-gprs[master]: rlcmac: Several fixes and improvements to have 2phase access working

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/34308?usp=email ) Change subject: rlcmac: Several fixes and improvements to have 2phase access working ...................................................................... rlcmac: Several fixes and improvements to have 2phase access working Some solutions are not meant to be final ones, but some small workarounds to have the whole thing running until the lower layers are fixed/improved. Related: OS#5500 Change-Id: I94bd0de6917b004cba73d2fffc7cf69b3b5c305d --- M src/rlcmac/sched.c M src/rlcmac/tbf_ul_ass_fsm.c M tests/rlcmac/rlcmac_prim_test.c M tests/rlcmac/rlcmac_prim_test.err M tests/rlcmac/rlcmac_prim_test.ok 5 files changed, 100 insertions(+), 13 deletions(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve diff --git a/src/rlcmac/sched.c b/src/rlcmac/sched.c index 8709f71..6af2b6a 100644 --- a/src/rlcmac/sched.c +++ b/src/rlcmac/sched.c @@ -222,7 +222,7 @@ if (tbfs->poll_ul_ass) { LOGRLCMAC(LOGL_DEBUG, "(ts=%u,fn=%u,usf=%u) Tx Pkt Control Ack (UL ASS poll)\n", bi->ts, bi->fn, bi->usf); - msg = gprs_rlcmac_gre_create_pkt_ctrl_ack(ul_tbf_as_tbf(tbfs->poll_ul_ass)->gre); + msg = gprs_rlcmac_tbf_ul_ass_create_rlcmac_msg(tbfs->poll_ul_ass, bi); if (msg) return msg; } diff --git a/src/rlcmac/tbf_ul_ass_fsm.c b/src/rlcmac/tbf_ul_ass_fsm.c index 8244f8e..0f0f9ee 100644 --- a/src/rlcmac/tbf_ul_ass_fsm.c +++ b/src/rlcmac/tbf_ul_ass_fsm.c @@ -30,6 +30,7 @@ #include <osmocom/gprs/rlcmac/types.h> #include <osmocom/gprs/rlcmac/tbf_ul_ass_fsm.h> #include <osmocom/gprs/rlcmac/tbf_ul.h> +#include <osmocom/gprs/rlcmac/tbf_dl.h> #include <osmocom/gprs/rlcmac/gre.h> #include <osmocom/gprs/rlcmac/sched.h> #include <osmocom/gprs/rlcmac/csn1_defs.h> @@ -392,9 +393,17 @@ /* If RRBP contains valid data, schedule a response (PKT CONTROL ACK or PKT RESOURCE REQ). */ if (d->dl_block->SP) { uint32_t poll_fn = rrbp2fn(d->fn, d->dl_block->RRBP); + uint32_t next_blk = fn_next_block(fn_next_block(poll_fn)); gprs_rlcmac_pdch_ulc_reserve(g_rlcmac_ctx->sched.ulc[d->ts_nr], poll_fn, GPRS_RLCMAC_PDCH_ULC_POLL_UL_ASS, ul_tbf_as_tbf(ctx->ul_tbf)); + /* We need to wait at least until sending the PKT CTRL + * ACK (in the old CTRL TS) before completing the + * assignment and using the new TS assignment. */ + if (!ctx->tbf_starting_time_exists && fn_cmp(ctx->tbf_starting_time, next_blk) < 0) { + ctx->tbf_starting_time_exists = true; + ctx->tbf_starting_time = next_blk; + } } if (ctx->tbf_starting_time_exists && @@ -409,10 +418,21 @@ } } +static void st_wait_tbf_starting_time2_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state) +{ + struct gprs_rlcmac_tbf_ul_ass_fsm_ctx *ctx = (struct gprs_rlcmac_tbf_ul_ass_fsm_ctx *)fi->priv; + + /* Configure lower layers to submit an RTS tick starting at tbf_starting_time + * and scheduler will send event GPRS_RLCMAC_TBF_UL_ASS_EV_TBF_STARTING_TIME to us. */ + gprs_rlcmac_ul_tbf_submit_configure_req(ctx->ul_tbf, &ctx->phase2_alloc, + ctx->tbf_starting_time_exists, ctx->tbf_starting_time); +} + static void st_wait_tbf_starting_time2(struct osmo_fsm_inst *fi, uint32_t event, void *data) { struct gprs_rlcmac_tbf_ul_ass_fsm_ctx *ctx = (struct gprs_rlcmac_tbf_ul_ass_fsm_ctx *)fi->priv; struct tbf_ul_ass_ev_rx_pkt_ul_ass_ctx *d; + struct tbf_ul_ass_ev_create_rlcmac_msg_ctx *data_ctx; int rc; switch (event) { @@ -425,9 +445,26 @@ /* If RRBP contains valid data, schedule a response (PKT CONTROL ACK or PKT RESOURCE REQ). */ if (d->dl_block->SP) { uint32_t poll_fn = rrbp2fn(d->fn, d->dl_block->RRBP); + uint32_t next_blk = fn_next_block(poll_fn); + /* FIXME: double fn_next_block() here is to delay + * release of old TS late enough so that the PKT CTRL ACK + * is transmitted. This is wrong since we basically lose + * the first TS in the new TBF, but otherwise lower layers + * may free the lchan while last burst of the last block is + * still not transmitted... IMHO lower layers need to be + * fixed to delay closing the lchan until all the + * blocks/bursts enqueued are transmitted... */ + next_blk = fn_next_block(next_blk); gprs_rlcmac_pdch_ulc_reserve(g_rlcmac_ctx->sched.ulc[d->ts_nr], poll_fn, GPRS_RLCMAC_PDCH_ULC_POLL_UL_ASS, ul_tbf_as_tbf(ctx->ul_tbf)); + /* We need to wait at least until sending the PKT CTRL + * ACK (in the old CTRL TS) before completing the + * assignment and using the new TS assignment. */ + if (!ctx->tbf_starting_time_exists && fn_cmp(ctx->tbf_starting_time, next_blk) < 0) { + ctx->tbf_starting_time_exists = true; + ctx->tbf_starting_time = next_blk; + } } if (ctx->tbf_starting_time_exists && @@ -440,6 +477,13 @@ case GPRS_RLCMAC_TBF_UL_ASS_EV_TBF_STARTING_TIME: tbf_ul_ass_fsm_state_chg(fi, GPRS_RLCMAC_TBF_UL_ASS_ST_COMPL); break; + case GPRS_RLCMAC_TBF_UL_ASS_EV_CREATE_RLCMAC_MSG: + data_ctx = (struct tbf_ul_ass_ev_create_rlcmac_msg_ctx *)data; + LOGPFSML(fi, LOGL_INFO, "TS=%u FN=%u Tx Pkt Ctrl Ack\n", data_ctx->ts, data_ctx->fn); + data_ctx->msg = gprs_rlcmac_gre_create_pkt_ctrl_ack(ul_tbf_as_tbf(ctx->ul_tbf)->gre); + if (!data_ctx->msg) + return; + break; default: OSMO_ASSERT(0); } @@ -517,12 +561,14 @@ [GPRS_RLCMAC_TBF_UL_ASS_ST_WAIT_TBF_STARTING_TIME2] = { .in_event_mask = X(GPRS_RLCMAC_TBF_UL_ASS_EV_RX_PKT_UL_ASS) | - X(GPRS_RLCMAC_TBF_UL_ASS_EV_TBF_STARTING_TIME), + X(GPRS_RLCMAC_TBF_UL_ASS_EV_TBF_STARTING_TIME) | + X(GPRS_RLCMAC_TBF_UL_ASS_EV_CREATE_RLCMAC_MSG), .out_state_mask = X(GPRS_RLCMAC_TBF_UL_ASS_ST_SCHED_PKT_RES_REQ) | X(GPRS_RLCMAC_TBF_UL_ASS_ST_WAIT_TBF_STARTING_TIME2) | X(GPRS_RLCMAC_TBF_UL_ASS_ST_COMPL), .name = "WAIT_TBF_STARTING_TIME2", + .onenter = st_wait_tbf_starting_time2_on_enter, .action = st_wait_tbf_starting_time2, }, [GPRS_RLCMAC_TBF_UL_ASS_ST_COMPL] = { @@ -638,6 +684,10 @@ { int rc; ul_tbf->ul_ass_fsm.dl_tbf = dl_tbf; + /* FIXME: Ideally this should only be the TS where the PKT UL ASS was received... */ + ul_tbf->ul_ass_fsm.phase1_alloc.num_ts = dl_tbf->cur_alloc.num_ts; + memcpy(&ul_tbf->ul_ass_fsm.phase1_alloc.ts[0], &dl_tbf->cur_alloc.ts[0], + sizeof(ul_tbf->ul_ass_fsm.phase1_alloc.ts)); rc = osmo_fsm_inst_dispatch(ul_tbf->ul_ass_fsm.fi, GPRS_RLCMAC_TBF_UL_ASS_EV_START_FROM_DL_TBF, NULL); @@ -666,11 +716,21 @@ { int res; - OSMO_ASSERT(gprs_rlcmac_tbf_ul_ass_waiting_tbf_starting_time(ul_tbf)); OSMO_ASSERT(ul_tbf->ul_ass_fsm.tbf_starting_time_exists); - OSMO_ASSERT(ul_tbf->ul_ass_fsm.phase1_alloc.num_ts > 0); - if (!ul_tbf->ul_ass_fsm.phase1_alloc.ts[ts_nr].allocated) - return; + switch (ul_tbf->ul_ass_fsm.fi->state) { + case GPRS_RLCMAC_TBF_UL_ASS_ST_WAIT_TBF_STARTING_TIME1: + OSMO_ASSERT(ul_tbf->ul_ass_fsm.phase1_alloc.num_ts > 0); + if (!ul_tbf->ul_ass_fsm.phase1_alloc.ts[ts_nr].allocated) + return; + break; + case GPRS_RLCMAC_TBF_UL_ASS_ST_WAIT_TBF_STARTING_TIME2: + OSMO_ASSERT(ul_tbf->ul_ass_fsm.phase2_alloc.num_ts > 0); + if (!ul_tbf->ul_ass_fsm.phase2_alloc.ts[ts_nr].allocated) + return; + break; + default: + OSMO_ASSERT(0); + } res = fn_cmp(fn, ul_tbf->ul_ass_fsm.tbf_starting_time); if (res < 0) {/* fn BEFORE tbf_starting_time */ LOGPTBFUL(ul_tbf, LOGL_DEBUG, "TS=%" PRIu8 " FN=%u Waiting for tbf_starting_time=%u\n", @@ -681,6 +741,7 @@ LOGPTBFUL(ul_tbf, LOGL_ERROR, "TS=%" PRIu8 " FN=%u Received late tick for tbf_starting_time=%u!\n", ts_nr, fn, ul_tbf->ul_ass_fsm.tbf_starting_time); /* fn == tbf_starting time */ + LOGPTBFUL(ul_tbf, LOGL_INFO, "TS=%" PRIu8 " FN=%u TBF_STARTING_TIME reached\n", fn, ts_nr); osmo_fsm_inst_dispatch(ul_tbf->ul_ass_fsm.fi, GPRS_RLCMAC_TBF_UL_ASS_EV_TBF_STARTING_TIME, NULL); } diff --git a/tests/rlcmac/rlcmac_prim_test.c b/tests/rlcmac/rlcmac_prim_test.c index 6e317b1..35def30 100644 --- a/tests/rlcmac/rlcmac_prim_test.c +++ b/tests/rlcmac/rlcmac_prim_test.c @@ -1136,6 +1136,9 @@ rts_fn = fn_next_block(rts_fn); ts_nr = 6; + /* FIXME: see extra fn_next_block() in libosmo-gprs-rlcmac st_wait_tbf_starting_time2() */ + rts_fn = fn_next_block(rts_fn); + /* Trigger transmission of LLC data (GMM Attach) (first part) */ rlcmac_prim = osmo_gprs_rlcmac_prim_alloc_l1ctl_pdch_rts_ind(ts_nr, rts_fn, usf_li[ts_nr]); rc = osmo_gprs_rlcmac_prim_lower_up(rlcmac_prim); diff --git a/tests/rlcmac/rlcmac_prim_test.err b/tests/rlcmac/rlcmac_prim_test.err index 0921266..95dc475 100644 --- a/tests/rlcmac/rlcmac_prim_test.err +++ b/tests/rlcmac/rlcmac_prim_test.err @@ -963,17 +963,25 @@ DLGLOBAL INFO TS=7 FN=26 Rx Pkt UL ASS DLGLOBAL INFO UL_TBF_ASS{WAIT_PKT_UL_ASS}: Received Event RX_PKT_UL_ASS DLGLOBAL DEBUG Register POLL (TS=7 FN=43, reason=UL_ASS) -DLGLOBAL INFO UL_TBF_ASS{WAIT_PKT_UL_ASS}: state_chg to COMPLETED +DLGLOBAL INFO UL_TBF_ASS{WAIT_PKT_UL_ASS}: state_chg to WAIT_TBF_STARTING_TIME2 +DLGLOBAL INFO TBF(UL:NR-0:TLLI-00000001) Send L1CTL-CFG_UL_TBF.req ul_tbf_nr=0 ul_slotmask=0xc0 tbf_starting_time(present=1 fn=52) +DLGLOBAL DEBUG Tx to lower layers: L1CTL-CFG_UL_TBF.request +DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_RTS.indication +DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00000001) TS=7 FN=43 Waiting for tbf_starting_time=52 +DLGLOBAL DEBUG (ts=7,fn=43,usf=2) Tx Pkt Control Ack (UL ASS poll) +DLGLOBAL INFO UL_TBF_ASS{WAIT_TBF_STARTING_TIME2}: Received Event CREATE_RLCMAC_MSG +DLGLOBAL INFO UL_TBF_ASS{WAIT_TBF_STARTING_TIME2}: TS=7 FN=43 Tx Pkt Ctrl Ack +DLGLOBAL DEBUG GRE(00000001) Tx Packet Control Ack +DLGLOBAL DEBUG Tx to lower layers: L1CTL-PDCH_DATA.request +DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_RTS.indication +DLGLOBAL INFO TBF(UL:NR-0:TLLI-00000001) TS=52 FN=6 TBF_STARTING_TIME reached +DLGLOBAL INFO UL_TBF_ASS{WAIT_TBF_STARTING_TIME2}: Received Event TBF_STARTING_TIME +DLGLOBAL INFO UL_TBF_ASS{WAIT_TBF_STARTING_TIME2}: state_chg to COMPLETED DLGLOBAL INFO UL_TBF{ASSIGN}: Received Event UL_ASS_COMPL DLGLOBAL INFO TBF(UL:NR-0:TLLI-00000001) Send L1CTL-CFG_UL_TBF.req ul_tbf_nr=0 ul_slotmask=0xc0 tbf_starting_time(present=0 fn=0) DLGLOBAL DEBUG Tx to lower layers: L1CTL-CFG_UL_TBF.request DLGLOBAL INFO UL_TBF{ASSIGN}: state_chg to FLOW DLGLOBAL INFO UL_TBF_ASS{COMPLETED}: state_chg to IDLE -DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_RTS.indication -DLGLOBAL DEBUG (ts=7,fn=43,usf=2) Tx Pkt Control Ack (UL ASS poll) -DLGLOBAL DEBUG GRE(00000001) Tx Packet Control Ack -DLGLOBAL DEBUG Tx to lower layers: L1CTL-PDCH_DATA.request -DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_RTS.indication DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00000001) Sending new block at BSN 0, CS=CS-2 DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00000001) Entering Countdown procedure CV=0 DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00000001) Dequeue next LLC (len=14) diff --git a/tests/rlcmac/rlcmac_prim_test.ok b/tests/rlcmac/rlcmac_prim_test.ok index d601794..0c66e24 100644 --- a/tests/rlcmac/rlcmac_prim_test.ok +++ b/tests/rlcmac/rlcmac_prim_test.ok @@ -204,8 +204,9 @@ test_rlcmac_prim_down_cb(): Rx L1CTL-PDCH_DATA.request fn=21 ts=7 data_len=23 data=[40 08 10 20 00 00 00 00 00 00 00 30 00 00 00 00 00 03 2b 2b 2b 2b 2b ] test_rlcmac_prim_down_cb(): Rx L1CTL-CFG_UL_TBF.request ul_tbf_nr=0 ul_slotmask=0xc0 test_rlcmac_prim_down_cb(): Rx L1CTL-PDCH_DATA.request fn=43 ts=7 data_len=23 data=[40 04 00 00 00 04 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b ] +test_rlcmac_prim_down_cb(): Rx L1CTL-CFG_UL_TBF.request ul_tbf_nr=0 ul_slotmask=0xc0 test_rlcmac_prim_up_cb(): Rx GMMRR-LLC_TRANSMITTED.indication TLLI=0x00000001 -test_rlcmac_prim_down_cb(): Rx L1CTL-PDCH_DATA.request fn=47 ts=6 data_len=34 data=[00 06 00 39 01 c0 00 08 01 01 d5 71 00 00 08 29 26 24 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 00 ] +test_rlcmac_prim_down_cb(): Rx L1CTL-PDCH_DATA.request fn=52 ts=6 data_len=34 data=[00 06 00 39 01 c0 00 08 01 01 d5 71 00 00 08 29 26 24 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 00 ] === test_dl_tbf_ccch_assign_requests_ul_tbf_pacch end === test_rlcmac_prim_down_cb(): Rx L1CTL-CFG_UL_TBF.request ul_tbf_nr=0 ul_slotmask=0x00 === test_ccch_pag_req1 start === -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/34308?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I94bd0de6917b004cba73d2fffc7cf69b3b5c305d Gerrit-Change-Number: 34308 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-MessageType: merged