[M] Change in libosmo-gprs[master]: Delay deleting UL TBF until last Pkt Ctrl Ack is fully transmitted
pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/35775?usp=email )
Change subject: Delay deleting UL TBF until last Pkt Ctrl Ack is fully transmitted ......................................................................
Delay deleting UL TBF until last Pkt Ctrl Ack is fully transmitted
This in turn delays reconfiguring the lower layers (L1CTL-CFG_UL_TBF.req mask=0x0) until the last block has been transmited.
Change-Id: Ic38b4207623ccbda3b77d4b0a47431c25de95034 --- M include/osmocom/gprs/rlcmac/tbf_ul_fsm.h M src/rlcmac/rlcmac_prim.c M src/rlcmac/sched.c M src/rlcmac/tbf_ul.c M src/rlcmac/tbf_ul_fsm.c M tests/rlcmac/rlcmac_prim_test.c M tests/rlcmac/rlcmac_prim_test.err 7 files changed, 100 insertions(+), 18 deletions(-)
Approvals: fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified
diff --git a/include/osmocom/gprs/rlcmac/tbf_ul_fsm.h b/include/osmocom/gprs/rlcmac/tbf_ul_fsm.h index e3aeb3f..6809216 100644 --- a/include/osmocom/gprs/rlcmac/tbf_ul_fsm.h +++ b/include/osmocom/gprs/rlcmac/tbf_ul_fsm.h @@ -1,6 +1,7 @@ /* Uplink TBF, 3GPP TS 44.060 */ #pragma once
+#include <stdint.h> #include <osmocom/core/fsm.h>
#include <osmocom/gprs/rlcmac/rlcmac_private.h> @@ -25,10 +26,16 @@ unsigned int pkt_acc_proc_attempts; /* 9.3.3.3.2: The block with CV=0 shall not be retransmitted more than four times. */ unsigned int last_data_block_retrans_attempts; - /* Whether the Received Packet UL ACK/NACK w/ FinalAck=1 had 'TBF Est' field to '1'. - * Used during ST_RELEASING to find out if a new UL TBF can be recreated - * when ansering the final UL ACK. */ - bool rx_final_pkt_ul_ack_nack_has_tbf_est; + struct { + /* Whether the Received Packet UL ACK/NACK w/ FinalAck=1 had 'TBF Est' field to '1'. + * Used during ST_RELEASING to find out if a new UL TBF can be recreated + * when ansering the final UL ACK. */ + bool has_tbf_est; + /* FN and TN of the registered poll to Tx last PKT CTRL ACK + * answering the final UL ACK. */ + uint32_t pkt_ctrl_ack_fn; + uint8_t pkt_ctrl_ack_tn; + } rx_final_pkt_ul_ack_nack; };
enum tbf_ul_fsm_event { @@ -39,6 +46,7 @@ GPRS_RLCMAC_TBF_UL_EV_N3104_MAX, 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_TX_COMPL_PKT_CTRL_ACK, };
struct tbf_ul_ass_ev_rx_ul_ack_nack { @@ -52,3 +60,5 @@ void gprs_rlcmac_tbf_ul_fsm_destructor(struct gprs_rlcmac_ul_tbf *ul_tbf);
enum gprs_rlcmac_tbf_ul_fsm_states gprs_rlcmac_tbf_ul_state(const struct gprs_rlcmac_ul_tbf *ul_tbf); + +bool gprs_rlcmac_ul_tbf_waiting_pkt_ctrl_ack_tx_confirmation(const struct gprs_rlcmac_ul_tbf *ul_tbf, uint32_t fn, uint8_t ts_nr); diff --git a/src/rlcmac/rlcmac_prim.c b/src/rlcmac/rlcmac_prim.c index 97e3cc9..496554e 100644 --- a/src/rlcmac/rlcmac_prim.c +++ b/src/rlcmac/rlcmac_prim.c @@ -646,6 +646,36 @@ return rc; }
+static int rlcmac_prim_handle_l1ctl_pdch_data_cnf(struct osmo_gprs_rlcmac_prim *rlcmac_prim) +{ + struct gprs_rlcmac_entity *gre; + +#if 0 + /* TODO: enable once we have originating req data in primitive coming from lower layers. */ + /* 3GPP TS 44.060 10.3.2 Uplink RLC/MAC control block: */ + enum osmo_gprs_rlcmac_ul_msg_type msg_type; + if (rlcmac_prim->l1ctl.pdch_data_cnf.data_len < 1) + return -EINVAL; + msg_type = (rlcmac_prim->l1ctl.pdch_data_cnf.data & 0xC0) >> 2; + if (msg_type != OSMO_GPRS_RLCMAC_UL_MSGT_PACKET_CONTROL_ACK) + return -EINVAL; +#endif + + llist_for_each_entry(gre, &g_rlcmac_ctx->gre_list, entry) { + if (!gre->ul_tbf) + continue; + if (!gprs_rlcmac_ul_tbf_waiting_pkt_ctrl_ack_tx_confirmation(gre->ul_tbf, + rlcmac_prim->l1ctl.pdch_data_cnf.fn, + rlcmac_prim->l1ctl.pdch_data_cnf.ts_nr)) + continue; + + osmo_fsm_inst_dispatch(gre->ul_tbf->state_fsm.fi, GPRS_RLCMAC_TBF_UL_EV_TX_COMPL_PKT_CTRL_ACK, NULL); + /* gre->ul_tbf is NULL here. */ + break; + } + return 0; +} + static int rlcmac_prim_handle_l1ctl_ccch_ready_ind(struct osmo_gprs_rlcmac_prim *rlcmac_prim) { struct gprs_rlcmac_entity *gre; @@ -670,6 +700,9 @@ case OSMO_PRIM(OSMO_GPRS_RLCMAC_L1CTL_PDCH_DATA, PRIM_OP_INDICATION): rc = rlcmac_prim_handle_l1ctl_pdch_data_ind(rlcmac_prim); break; + case OSMO_PRIM(OSMO_GPRS_RLCMAC_L1CTL_PDCH_DATA, PRIM_OP_CONFIRM): + rc = rlcmac_prim_handle_l1ctl_pdch_data_cnf(rlcmac_prim); + break; case OSMO_PRIM(OSMO_GPRS_RLCMAC_L1CTL_CCCH_DATA, PRIM_OP_INDICATION): rc = rlcmac_prim_handle_l1ctl_ccch_data_ind(rlcmac_prim); break; diff --git a/src/rlcmac/sched.c b/src/rlcmac/sched.c index 1620934..5f12d5a 100644 --- a/src/rlcmac/sched.c +++ b/src/rlcmac/sched.c @@ -158,18 +158,18 @@ /*! Select a CTRL message to transmit, based on different messages priority. * \param[in] bi RTS block indication information. * \param[in] tbfs TBF candidates having CTRL messages to send, filled in by get_ctrl_msg_tbf_candidates() - * \param[out] tbf_to_free TBF to free after sending the generated message + * \param[out] need_block_conf Whether we require Tx confirmation of this block from lower layers. */ static struct msgb *sched_select_ctrl_msg(const struct gprs_rlcmac_rts_block_ind *bi, const struct tbf_sched_ctrl_candidates *tbfs, - struct gprs_rlcmac_tbf **tbf_to_free) + bool *need_block_conf) { struct msgb *msg = NULL; struct gprs_rlcmac_entity *gre; int rc;
- /* No TBF to be freed by default: */ - *tbf_to_free = NULL; + /* No TBF needs block conf by default: */ + *need_block_conf = false;
/* 8.1.2.2 1) (EGPRS) PACKET DOWNLINK ACK/NACK w/ FinalAckInd=1 */ if (tbfs->poll_dl_ack_final_ack) { @@ -209,7 +209,6 @@ bi->ts, bi->fn, bi->usf); msg = gprs_rlcmac_gre_create_pkt_ctrl_ack(ul_tbf_as_tbf(tbfs->poll_ul_ack)->gre); /* Last UL message, freeing (after passing msg to lower layers) */ - *tbf_to_free = ul_tbf_as_tbf(tbfs->poll_ul_ack); return msg; } if (tbfs->poll_dl_ass) { @@ -288,7 +287,7 @@ { struct msgb *msg = NULL; struct tbf_sched_ctrl_candidates tbf_cand = {0}; - struct gprs_rlcmac_tbf *tbf_to_free; + bool need_block_conf; struct osmo_gprs_rlcmac_prim *rlcmac_prim_tx; int rc = 0;
@@ -296,7 +295,7 @@
get_ctrl_msg_tbf_candidates(bi, &tbf_cand);
- if ((msg = sched_select_ctrl_msg(bi, &tbf_cand, &tbf_to_free))) + if ((msg = sched_select_ctrl_msg(bi, &tbf_cand, &need_block_conf))) goto tx_msg;
if ((msg = sched_select_ul_data_msg(bi))) @@ -312,10 +311,11 @@ tx_msg: rlcmac_prim_tx = gprs_rlcmac_prim_alloc_l1ctl_pdch_data_req(bi->ts, bi->fn, msgb_data(msg), 0); rlcmac_prim_tx->l1ctl.pdch_data_req.data_len = msgb_length(msg); + /* TODO: enable once we support conditional confirmation: + * rlcmac_prim_tx->l1ctl.pdch_data_req.needs_conf = need_block_conf; + */ rc = gprs_rlcmac_prim_call_down_cb(rlcmac_prim_tx); msgb_free(msg); - if (tbf_to_free) - gprs_rlcmac_tbf_free(tbf_to_free);
ret_rc: gprs_rlcmac_pdch_ulc_expire_fn(g_rlcmac_ctx->sched.ulc[bi->ts], bi->fn); diff --git a/src/rlcmac/tbf_ul.c b/src/rlcmac/tbf_ul.c index 3d4ba2b..26d710e 100644 --- a/src/rlcmac/tbf_ul.c +++ b/src/rlcmac/tbf_ul.c @@ -212,7 +212,7 @@ return false;
/* "TBF Est field is set to '1'"" */ - if (!ul_tbf->state_fsm.rx_final_pkt_ul_ack_nack_has_tbf_est) + if (!ul_tbf->state_fsm.rx_final_pkt_ul_ack_nack.has_tbf_est) return false;
/* the mobile station has new data to transmit */ diff --git a/src/rlcmac/tbf_ul_fsm.c b/src/rlcmac/tbf_ul_fsm.c index 92d1d64..d5aa9c3 100644 --- a/src/rlcmac/tbf_ul_fsm.c +++ b/src/rlcmac/tbf_ul_fsm.c @@ -41,6 +41,7 @@ { GPRS_RLCMAC_TBF_UL_EV_N3104_MAX, "N3104_MAX" }, { 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_TX_COMPL_PKT_CTRL_ACK, "TX_COMPL_PKT_CTRL_ACK" }, { 0, NULL } };
@@ -248,6 +249,7 @@ const Packet_Uplink_Ack_Nack_t *ack = &dlbi->dl_block.u.Packet_Uplink_Ack_Nack; const PU_AckNack_GPRS_t *gprs = &ack->u.PU_AckNack_GPRS_Struct; const Ack_Nack_Description_t *ack_desc = &gprs->Ack_Nack_Description; + uint32_t poll_fn = 0xffffffff; /* Invalid FN */
if (gprs_rlcmac_ul_tbf_in_contention_resolution(ctx->ul_tbf)) { _contention_resolution_succeeded(ctx); @@ -262,7 +264,7 @@ } /* If RRBP contains valid data, schedule a response (PKT CONTROL ACK or PKT RESOURCE REQ). */ if (dlbi->dl_block.SP) { - uint32_t poll_fn = rrbp2fn(dlbi->fn, dlbi->dl_block.RRBP); + poll_fn = rrbp2fn(dlbi->fn, dlbi->dl_block.RRBP); gprs_rlcmac_pdch_ulc_reserve(g_rlcmac_ctx->sched.ulc[dlbi->ts_nr], poll_fn, GPRS_RLCMAC_PDCH_ULC_POLL_UL_ACK, ul_tbf_as_tbf(ctx->ul_tbf)); @@ -270,7 +272,9 @@ if (ack_desc->FINAL_ACK_INDICATION) { bool tbf_est = (gprs->Exist_AdditionsR99 && gprs->AdditionsR99.TBF_EST); LOGPFSMLDLBI(ctx->fi, dlbi, LOGL_DEBUG, "Final ACK received\n"); - ctx->rx_final_pkt_ul_ack_nack_has_tbf_est = tbf_est; + ctx->rx_final_pkt_ul_ack_nack.has_tbf_est = tbf_est; + ctx->rx_final_pkt_ul_ack_nack.pkt_ctrl_ack_fn = poll_fn; + ctx->rx_final_pkt_ul_ack_nack.pkt_ctrl_ack_tn = dlbi->ts_nr; tbf_ul_fsm_state_chg(fi, GPRS_RLCMAC_TBF_UL_ST_RELEASING); } break; @@ -301,8 +305,11 @@ /* Waiting for scheduler to transmit PKT CTRL ACK for the already received UL ACK/NACK FinalAck=1 */ static void st_releasing(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 gprs_rlcmac_tbf_ul_fsm_ctx *ctx = (struct gprs_rlcmac_tbf_ul_fsm_ctx *)fi->priv; switch (event) { + case GPRS_RLCMAC_TBF_UL_EV_TX_COMPL_PKT_CTRL_ACK: + gprs_rlcmac_ul_tbf_free(ctx->ul_tbf); + break; default: OSMO_ASSERT(0); } @@ -352,7 +359,7 @@ .action = st_finished, }, [GPRS_RLCMAC_TBF_UL_ST_RELEASING] = { - .in_event_mask = 0, + .in_event_mask = X(GPRS_RLCMAC_TBF_UL_EV_TX_COMPL_PKT_CTRL_ACK), .out_state_mask = 0, .name = "RELEASING", .action = st_releasing, @@ -449,3 +456,15 @@ const struct gprs_rlcmac_tbf_ul_fsm_ctx *ctx = &ul_tbf->state_fsm; return ctx->fi->state; } + +/* Whether the UL TBF requested a Tx confirmation for a Pkt Ctrl Ack it transmitted at {FN,TN}. */ +bool gprs_rlcmac_ul_tbf_waiting_pkt_ctrl_ack_tx_confirmation(const struct gprs_rlcmac_ul_tbf *ul_tbf, uint32_t fn, uint8_t ts_nr) +{ + const struct gprs_rlcmac_tbf_ul_fsm_ctx *ctx = &ul_tbf->state_fsm; + /* PKT CTRL ACK Tx confirmation is only needed for final ack, to avoid + * freeing (unregistering with lower layers) the TBF too early. */ + if (ctx->fi->state != GPRS_RLCMAC_TBF_UL_ST_RELEASING) + return false; + return ctx->rx_final_pkt_ul_ack_nack.pkt_ctrl_ack_fn == fn && + ctx->rx_final_pkt_ul_ack_nack.pkt_ctrl_ack_tn == ts_nr; +} diff --git a/tests/rlcmac/rlcmac_prim_test.c b/tests/rlcmac/rlcmac_prim_test.c index 179888a..bfc491f 100644 --- a/tests/rlcmac/rlcmac_prim_test.c +++ b/tests/rlcmac/rlcmac_prim_test.c @@ -623,6 +623,12 @@ rc = osmo_gprs_rlcmac_prim_lower_up(rlcmac_prim); OSMO_ASSERT(rc == 0);
+ /* Trigger transmission confirmation of PKT CTRL ACK */ + rlcmac_prim = osmo_gprs_rlcmac_prim_alloc_l1ctl_pdch_data_cnf(ts_nr, rts_fn, NULL, 0); + rc = osmo_gprs_rlcmac_prim_lower_up(rlcmac_prim); + OSMO_ASSERT(rc == 0); + + printf("=== %s end ===\n", __func__); cleanup_test(); } diff --git a/tests/rlcmac/rlcmac_prim_test.err b/tests/rlcmac/rlcmac_prim_test.err index 366e321..554b6d0 100644 --- a/tests/rlcmac/rlcmac_prim_test.err +++ b/tests/rlcmac/rlcmac_prim_test.err @@ -69,6 +69,8 @@ DLGLOBAL DEBUG (ts=7,fn=21,usf=0) Tx Pkt Control Ack (UL ACK/NACK poll) DLGLOBAL DEBUG GRE(00002342) Tx Packet Control Ack DLGLOBAL DEBUG Tx to lower layers: L1CTL-PDCH_DATA.request +DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_DATA.confirm +DLGLOBAL INFO UL_TBF{RELEASING}: Received Event TX_COMPL_PKT_CTRL_ACK DLGLOBAL INFO UL_TBF_ASS{IDLE}: Deallocated DLGLOBAL INFO UL_TBF{RELEASING}: Send L1CTL-CFG_UL_TBF.req ul_tbf_nr=0 (release) DLGLOBAL DEBUG Tx to lower layers: L1CTL-CFG_UL_TBF.request
-
pespin