5 Feb
2024
5 Feb
'24
4:26 p.m.
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
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/35775?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: Ic38b4207623ccbda3b77d4b0a47431c25de95034
Gerrit-Change-Number: 35775
Gerrit-PatchSet: 5
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged