pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31449 ) Change subject: rlcmac: ul_tbf_fsm: rework Rx UL ACK/NACK fsm events ...................................................................... rlcmac: ul_tbf_fsm: rework Rx UL ACK/NACK fsm events We'll need to get one event for each Pkt UL ACK/NACK the UL TBF receives in order to implement T3182 properly in a follow-up patch, hence rework the events sent to the FSM (merge events about Final ACK and Contention Resolution in one generic UL ACK/NACK event with some data parameters). Change-Id: I4420abd69a318bd93fde73a67239456466071497 --- M include/osmocom/gprs/rlcmac/tbf_ul_fsm.h M src/rlcmac/tbf_ul.c M src/rlcmac/tbf_ul_fsm.c M tests/rlcmac/rlcmac_prim_test.err 4 files changed, 60 insertions(+), 54 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/include/osmocom/gprs/rlcmac/tbf_ul_fsm.h b/include/osmocom/gprs/rlcmac/tbf_ul_fsm.h index cfdb677..b1f82e3 100644 --- a/include/osmocom/gprs/rlcmac/tbf_ul_fsm.h +++ b/include/osmocom/gprs/rlcmac/tbf_ul_fsm.h @@ -36,9 +36,13 @@ GPRS_RLCMAC_TBF_UL_EV_UL_ASS_COMPL, GPRS_RLCMAC_TBF_UL_EV_FIRST_UL_DATA_SENT, GPRS_RLCMAC_TBF_UL_EV_N3104_MAX, - GPRS_RLCMAC_TBF_UL_EV_CONTENTION_RESOLUTION_SUCCESS, + GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK, /* data: struct tbf_ul_ass_ev_rx_ul_ack_nack* */ GPRS_RLCMAC_TBF_UL_EV_LAST_UL_DATA_SENT, - GPRS_RLCMAC_TBF_UL_EV_FINAL_ACK_RECVD, /* data: bool TBF_EST */ +}; + +struct tbf_ul_ass_ev_rx_ul_ack_nack { + bool final_ack; + bool tbf_est; }; int gprs_rlcmac_tbf_ul_fsm_init(void); diff --git a/src/rlcmac/tbf_ul.c b/src/rlcmac/tbf_ul.c index d5e4439..fbef343 100644 --- a/src/rlcmac/tbf_ul.c +++ b/src/rlcmac/tbf_ul.c @@ -212,25 +212,6 @@ return 0; } -int gprs_rlcmac_ul_tbf_handle_final_ack(struct gprs_rlcmac_ul_tbf *ul_tbf, const RlcMacDownlink_t *dl_block) -{ - int rc = 0; - bool tbf_est = false; - const PU_AckNack_GPRS_t *ack_gprs = &dl_block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct; - - if (ack_gprs->Exist_AdditionsR99) - tbf_est = ack_gprs->AdditionsR99.TBF_EST; - - osmo_fsm_inst_dispatch(ul_tbf->state_fsm.fi, GPRS_RLCMAC_TBF_UL_EV_FINAL_ACK_RECVD, &tbf_est); - - /* range V(A)..V(S)-1 */ - //received = gprs_rlcmac_rlc_ul_window_count_unacked(ul_tbf->ulw); - /* report all outstanding packets as received */ - //gprs_rlcmac_received_lost(this, received, 0); - gprs_rlcmac_rlc_ul_window_reset(ul_tbf->ulw); - return rc; -} - int gprs_rlcmac_ul_tbf_handle_pkt_ul_ack_nack(struct gprs_rlcmac_ul_tbf *ul_tbf, const RlcMacDownlink_t *dl_block) { @@ -247,6 +228,10 @@ .cur_bit = 0, }; int rc; + struct tbf_ul_ass_ev_rx_ul_ack_nack ev_ack = { + .final_ack = ack_desc->FINAL_ACK_INDICATION, + .tbf_est = (gprs->Exist_AdditionsR99 && gprs->AdditionsR99.TBF_EST), + }; num_blocks = gprs_rlcmac_decode_gprs_acknack_bits( ack_desc, &bits, &bsn_begin, &bsn_end, ul_tbf->ulw); @@ -259,12 +244,10 @@ rc = gprs_rlcmac_ul_tbf_update_window(ul_tbf, bsn_begin, &bits); - if (gprs_rlcmac_ul_tbf_in_contention_resolution(ul_tbf)) - osmo_fsm_inst_dispatch(ul_tbf->state_fsm.fi, GPRS_RLCMAC_TBF_UL_EV_CONTENTION_RESOLUTION_SUCCESS, NULL); + osmo_fsm_inst_dispatch(ul_tbf->state_fsm.fi, GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK, &ev_ack); if (ack_desc->FINAL_ACK_INDICATION) { - LOGPTBFUL(ul_tbf, LOGL_DEBUG, "Final ACK received.\n"); - rc = gprs_rlcmac_ul_tbf_handle_final_ack(ul_tbf, dl_block); + gprs_rlcmac_rlc_ul_window_reset(ul_tbf->ulw); } else if (gprs_rlcmac_tbf_ul_state(ul_tbf) == GPRS_RLCMAC_TBF_UL_ST_FINISHED && gprs_rlcmac_rlc_ul_window_window_empty(ul_tbf->ulw)) { LOGPTBFUL(ul_tbf, LOGL_NOTICE, diff --git a/src/rlcmac/tbf_ul_fsm.c b/src/rlcmac/tbf_ul_fsm.c index cb7df60..617ed2e 100644 --- a/src/rlcmac/tbf_ul_fsm.c +++ b/src/rlcmac/tbf_ul_fsm.c @@ -35,9 +35,8 @@ { GPRS_RLCMAC_TBF_UL_EV_UL_ASS_COMPL, "UL_ASS_COMPL" }, { GPRS_RLCMAC_TBF_UL_EV_FIRST_UL_DATA_SENT, "FIRST_UL_DATA_SENT" }, { GPRS_RLCMAC_TBF_UL_EV_N3104_MAX, "N3104_MAX" }, - { GPRS_RLCMAC_TBF_UL_EV_CONTENTION_RESOLUTION_SUCCESS, "CONTENTION_RESOLUTION_SUCCESS" }, + { GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK, "RX_UL_ACK_NACK" }, { GPRS_RLCMAC_TBF_UL_EV_LAST_UL_DATA_SENT, "LAST_UL_DATA_SENT" }, - { GPRS_RLCMAC_TBF_UL_EV_FINAL_ACK_RECVD, "FINAL_ACK_RECVD" }, { 0, NULL } }; @@ -153,12 +152,16 @@ case GPRS_RLCMAC_TBF_UL_EV_N3104_MAX: reinit_pkt_acces_procedure(ctx); break; - case GPRS_RLCMAC_TBF_UL_EV_CONTENTION_RESOLUTION_SUCCESS: - LOGPFSML(ctx->fi, LOGL_INFO, "Contention resolution succeeded, stop T3166\n"); - OSMO_ASSERT(ctx->ul_tbf->ul_ass_fsm.ass_type == GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE); - OSMO_ASSERT(fi->T == 3166); - osmo_timer_del(&fi->timer); - fi->T = 0; + case GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK: + if (gprs_rlcmac_ul_tbf_in_contention_resolution(ctx->ul_tbf)) { + LOGPFSML(ctx->fi, LOGL_INFO, "Contention resolution succeeded, stop T3166\n"); + OSMO_ASSERT(ctx->ul_tbf->ul_ass_fsm.ass_type == GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE); + OSMO_ASSERT(fi->T == 3166); + osmo_timer_del(&fi->timer); + fi->T = 0; + } + /* It's impossible we receive a correct final_ack here, since we didn't + * sent last data (FSM would be in FINISHED state then) */ break; case GPRS_RLCMAC_TBF_UL_EV_LAST_UL_DATA_SENT: tbf_ul_fsm_state_chg(fi, GPRS_RLCMAC_TBF_UL_ST_FINISHED); @@ -171,16 +174,25 @@ static void st_finished(struct osmo_fsm_inst *fi, uint32_t event, void *data) { struct gprs_rlcmac_tbf_ul_fsm_ctx *ctx = (struct gprs_rlcmac_tbf_ul_fsm_ctx *)fi->priv; + struct tbf_ul_ass_ev_rx_ul_ack_nack *ctx_ul_ack_nack; switch (event) { case GPRS_RLCMAC_TBF_UL_EV_N3104_MAX: reinit_pkt_acces_procedure(ctx); break; - case GPRS_RLCMAC_TBF_UL_EV_CONTENTION_RESOLUTION_SUCCESS: - LOGPFSML(ctx->fi, LOGL_INFO, "Contention resolution succeeded, stop T3166\n"); - OSMO_ASSERT(ctx->ul_tbf->ul_ass_fsm.ass_type == GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE); - OSMO_ASSERT(fi->T == 3166); - osmo_timer_del(&fi->timer); - fi->T = 0; + case GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK: + ctx_ul_ack_nack = (struct tbf_ul_ass_ev_rx_ul_ack_nack *)data; + if (gprs_rlcmac_ul_tbf_in_contention_resolution(ctx->ul_tbf)) { + LOGPFSML(ctx->fi, LOGL_INFO, "Contention resolution succeeded, stop T3166\n"); + OSMO_ASSERT(ctx->ul_tbf->ul_ass_fsm.ass_type == GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE); + OSMO_ASSERT(fi->T == 3166); + osmo_timer_del(&fi->timer); + fi->T = 0; + } + if (ctx_ul_ack_nack->final_ack) { + LOGPFSML(ctx->fi, LOGL_DEBUG, "Final ACK received\n"); + ctx->rx_final_pkt_ul_ack_nack_has_tbf_est = ctx_ul_ack_nack->tbf_est; + tbf_ul_fsm_state_chg(fi, GPRS_RLCMAC_TBF_UL_ST_RELEASING); + } break; case GPRS_RLCMAC_TBF_UL_EV_LAST_UL_DATA_SENT: /* If LAST_UL_DATA_SENT is received in this state, it means the @@ -195,10 +207,6 @@ gprs_rlcmac_ul_tbf_free(ctx->ul_tbf); } break; - case GPRS_RLCMAC_TBF_UL_EV_FINAL_ACK_RECVD: - ctx->rx_final_pkt_ul_ack_nack_has_tbf_est = *((bool *)data); - tbf_ul_fsm_state_chg(fi, GPRS_RLCMAC_TBF_UL_ST_RELEASING); - break; default: OSMO_ASSERT(0); } @@ -236,7 +244,7 @@ .in_event_mask = X(GPRS_RLCMAC_TBF_UL_EV_FIRST_UL_DATA_SENT) | X(GPRS_RLCMAC_TBF_UL_EV_N3104_MAX) | - X(GPRS_RLCMAC_TBF_UL_EV_CONTENTION_RESOLUTION_SUCCESS) | + X(GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK) | X(GPRS_RLCMAC_TBF_UL_EV_LAST_UL_DATA_SENT), .out_state_mask = X(GPRS_RLCMAC_TBF_UL_ST_NEW) | @@ -247,9 +255,8 @@ [GPRS_RLCMAC_TBF_UL_ST_FINISHED] = { .in_event_mask = X(GPRS_RLCMAC_TBF_UL_EV_N3104_MAX) | - X(GPRS_RLCMAC_TBF_UL_EV_CONTENTION_RESOLUTION_SUCCESS) | - X(GPRS_RLCMAC_TBF_UL_EV_LAST_UL_DATA_SENT) | - X(GPRS_RLCMAC_TBF_UL_EV_FINAL_ACK_RECVD), + X(GPRS_RLCMAC_TBF_UL_EV_RX_UL_ACK_NACK) | + X(GPRS_RLCMAC_TBF_UL_EV_LAST_UL_DATA_SENT), .out_state_mask = X(GPRS_RLCMAC_TBF_UL_ST_NEW) | X(GPRS_RLCMAC_TBF_UL_ST_RELEASING), diff --git a/tests/rlcmac/rlcmac_prim_test.err b/tests/rlcmac/rlcmac_prim_test.err index 5e78c2b..885e29b 100644 --- a/tests/rlcmac/rlcmac_prim_test.err +++ b/tests/rlcmac/rlcmac_prim_test.err @@ -49,10 +49,9 @@ DLGLOBAL DEBUG - got ack for BSN=0 DLGLOBAL DEBUG - got ack for BSN=1 DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) V(B): (V(A)=2)""(V(S)-1=1) A=Acked N=Nacked U=Unacked X=Resend-Unacked I=Invalid -DLGLOBAL INFO UL_TBF{FINISHED}: Received Event CONTENTION_RESOLUTION_SUCCESS +DLGLOBAL INFO UL_TBF{FINISHED}: Received Event RX_UL_ACK_NACK DLGLOBAL INFO UL_TBF{FINISHED}: Contention resolution succeeded, stop T3166 -DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) Final ACK received. -DLGLOBAL INFO UL_TBF{FINISHED}: Received Event FINAL_ACK_RECVD +DLGLOBAL DEBUG UL_TBF{FINISHED}: Final ACK received DLGLOBAL INFO UL_TBF{FINISHED}: state_chg to RELEASING DLGLOBAL DEBUG Register POLL (TS=7 FN=21, reason=UL_ACK) DLGLOBAL INFO Rx from lower layers: L1CTL-PDCH_RTS.indication @@ -418,7 +417,7 @@ DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) ack: (BSN=0)"R"(BSN=0) R=ACK I=NACK DLGLOBAL DEBUG - got ack for BSN=0 DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) V(B): (V(A)=1)""(V(S)-1=0) A=Acked N=Nacked U=Unacked X=Resend-Unacked I=Invalid -DLGLOBAL INFO UL_TBF{FLOW}: Received Event CONTENTION_RESOLUTION_SUCCESS +DLGLOBAL INFO UL_TBF{FLOW}: Received Event RX_UL_ACK_NACK DLGLOBAL INFO UL_TBF{FLOW}: Contention resolution succeeded, stop T3166 DLGLOBAL INFO Rx from lower layers: L1CTL-PDCH_RTS.indication DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) Sending new block at BSN 1, CS=CS-2 @@ -512,10 +511,9 @@ DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) ack: (BSN=0)"R"(BSN=0) R=ACK I=NACK DLGLOBAL DEBUG - got ack for BSN=0 DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) V(B): (V(A)=1)""(V(S)-1=0) A=Acked N=Nacked U=Unacked X=Resend-Unacked I=Invalid -DLGLOBAL INFO UL_TBF{FINISHED}: Received Event CONTENTION_RESOLUTION_SUCCESS +DLGLOBAL INFO UL_TBF{FINISHED}: Received Event RX_UL_ACK_NACK DLGLOBAL INFO UL_TBF{FINISHED}: Contention resolution succeeded, stop T3166 -DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) Final ACK received. -DLGLOBAL INFO UL_TBF{FINISHED}: Received Event FINAL_ACK_RECVD +DLGLOBAL DEBUG UL_TBF{FINISHED}: Final ACK received DLGLOBAL INFO UL_TBF{FINISHED}: state_chg to RELEASING DLGLOBAL DEBUG Register POLL (TS=7 FN=17, reason=UL_ACK) DLGLOBAL INFO Rx from upper layers: GRR-UNITDATA.request -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31449 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I4420abd69a318bd93fde73a67239456466071497 Gerrit-Change-Number: 31449 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged