From 55f600b702f890de443f9f13a984ecf3e1970d0c Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Mon, 26 Jul 2021 15:54:39 +0200 Subject: [PATCH] Move RELEASING tbf_state transition to tbf_fsm PdchUlcTest output changes because the original state NULL is not expected when transactioning to RELEASING upon MAX N310* being hit. In any case, none of those events should happen in NULL state, but we don't really care about TBF states there so we are fine with whatever the state is. Related: OS#2709 Change-Id: I516b8d989a0d705e5664f8aeaf7d108e0105aa16 --- src/tbf.cpp | 8 +++---- src/tbf.h | 1 - src/tbf_dl.cpp | 3 ++- src/tbf_fsm.c | 44 +++++++++++++++++++++++++++++++++++---- src/tbf_fsm.h | 3 +++ src/tbf_ul.cpp | 2 +- tests/tbf/TbfTest.err | 1 + tests/ulc/PdchUlcTest.err | 20 +++++++++--------- 8 files changed, 61 insertions(+), 21 deletions(-) diff --git a/src/tbf.cpp b/src/tbf.cpp index 7b5fce20..c68d5051 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -640,7 +640,7 @@ void gprs_rlcmac_tbf::poll_timeout(struct gprs_rlcmac_pdch *pdch, uint32_t poll_ if (state_is(TBF_ST_FINISHED)) { if (ul_tbf->n_inc(N3103)) { bts_do_rate_ctr_inc(bts, CTR_PUAN_POLL_FAILED); - TBF_SET_STATE(ul_tbf, TBF_ST_RELEASING); + osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_MAX_N3103, NULL); T_START(ul_tbf, T3169, 3169, "MAX N3103 reached", false); return; } @@ -659,7 +659,7 @@ void gprs_rlcmac_tbf::poll_timeout(struct gprs_rlcmac_pdch *pdch, uint32_t poll_ bts_do_rate_ctr_inc(bts, CTR_RLC_ASS_TIMEDOUT); bts_do_rate_ctr_inc(bts, CTR_PUA_POLL_TIMEDOUT); if (n_inc(N3105)) { - TBF_SET_STATE(this, TBF_ST_RELEASING); + osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_MAX_N3105, NULL); T_START(this, T3195, 3195, "MAX N3105 reached", true); bts_do_rate_ctr_inc(bts, CTR_RLC_ASS_FAILED); bts_do_rate_ctr_inc(bts, CTR_PUA_POLL_FAILED); @@ -678,7 +678,7 @@ void gprs_rlcmac_tbf::poll_timeout(struct gprs_rlcmac_pdch *pdch, uint32_t poll_ bts_do_rate_ctr_inc(bts, CTR_RLC_ASS_TIMEDOUT); bts_do_rate_ctr_inc(bts, CTR_PDA_POLL_TIMEDOUT); if (n_inc(N3105)) { - TBF_SET_STATE(this, TBF_ST_RELEASING); + osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_MAX_N3105, NULL); T_START(this, T3195, 3195, "MAX N3105 reached", true); bts_do_rate_ctr_inc(bts, CTR_RLC_ASS_FAILED); bts_do_rate_ctr_inc(bts, CTR_PDA_POLL_FAILED); @@ -709,7 +709,7 @@ void gprs_rlcmac_tbf::poll_timeout(struct gprs_rlcmac_pdch *pdch, uint32_t poll_ } if (dl_tbf->n_inc(N3105)) { - TBF_SET_STATE(dl_tbf, TBF_ST_RELEASING); + osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_MAX_N3105, NULL); T_START(dl_tbf, T3195, 3195, "MAX N3105 reached", true); bts_do_rate_ctr_inc(bts, CTR_PDAN_POLL_FAILED); bts_do_rate_ctr_inc(bts, CTR_RLC_ACK_FAILED); diff --git a/src/tbf.h b/src/tbf.h index adc648ce..ded3a3c3 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -158,7 +158,6 @@ enum tbf_counters { /* TBF counters from 3GPP TS 44.060 ยง13.4 */ #define T_START(tbf, t, T, r, f) tbf->t_start(t, T, r, f, __FILE__, __LINE__) -#define TBF_SET_STATE(t, st) do { tbf_fsm_state_chg(t->state_fsm.fi, st); } while(0) #define TBF_SET_ASS_STATE_DL(t, st) do { t->set_ass_state_dl(st, __FILE__, __LINE__); } while(0) #define TBF_SET_ASS_STATE_UL(t, st) do { t->set_ass_state_ul(st, __FILE__, __LINE__); } while(0) #define TBF_SET_ACK_STATE(t, st) do { t->set_ack_state(st, __FILE__, __LINE__); } while(0) diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index 6614366b..f264cfc3 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -1271,7 +1271,8 @@ int gprs_rlcmac_dl_tbf::abort() * (partly) encoded in chunk 1 of block V(A). (optional) */ } - TBF_SET_STATE(this, TBF_ST_RELEASING); + /* This state change looks unneeded and can probably be dropped at some point: */ + tbf_fsm_state_chg(this->state_fsm.fi, TBF_ST_RELEASING); /* reset rlc states */ m_window.reset(); diff --git a/src/tbf_fsm.c b/src/tbf_fsm.c index d1b48d56..1ba1d616 100644 --- a/src/tbf_fsm.c +++ b/src/tbf_fsm.c @@ -49,6 +49,9 @@ const struct value_string tbf_fsm_event_names[] = { { TBF_EV_LAST_DL_DATA_SENT, "LAST_DL_DATA_SENT" }, { TBF_EV_LAST_UL_DATA_RECVD, "LAST_UL_DATA_RECVD" }, { TBF_EV_FINAL_ACK_RECVD, "FINAL_ACK_RECVD" }, + { TBF_EV_MAX_N3101 , "MAX_N3101" }, + { TBF_EV_MAX_N3103 , "MAX_N3103" }, + { TBF_EV_MAX_N3105 , "MAX_N3105" }, { 0, NULL } }; @@ -154,6 +157,10 @@ static void st_flow(struct osmo_fsm_inst *fi, uint32_t event, void *data) case we receive more DL data to tx */ tbf_fsm_state_chg(fi, TBF_ST_WAIT_RELEASE); break; + case TBF_EV_MAX_N3101: + case TBF_EV_MAX_N3105: + tbf_fsm_state_chg(fi, TBF_ST_RELEASING); + break; default: OSMO_ASSERT(0); } @@ -169,6 +176,27 @@ static void st_finished(struct osmo_fsm_inst *fi, uint32_t event, void *data) case we receive more DL data to tx */ tbf_fsm_state_chg(fi, TBF_ST_WAIT_RELEASE); break; + case TBF_EV_MAX_N3103: + tbf_fsm_state_chg(fi, TBF_ST_RELEASING); + break; + case TBF_EV_MAX_N3105: + tbf_fsm_state_chg(fi, TBF_ST_RELEASING); + break; + default: + OSMO_ASSERT(0); + } +} + +static void st_wait_release(struct osmo_fsm_inst *fi, uint32_t event, void *data) +{ + switch (event) { + case TBF_EV_FINAL_ACK_RECVD: + /* ignore, duplicate ACK, we already know about since we are in WAIT_RELEASE */ + break; + case TBF_EV_MAX_N3101: + case TBF_EV_MAX_N3105: + tbf_fsm_state_chg(fi, TBF_ST_RELEASING); + break; default: OSMO_ASSERT(0); } @@ -219,7 +247,9 @@ static struct osmo_fsm_state tbf_fsm_states[] = { .in_event_mask = X(TBF_EV_LAST_DL_DATA_SENT) | X(TBF_EV_LAST_UL_DATA_RECVD) | - X(TBF_EV_FINAL_ACK_RECVD), + X(TBF_EV_FINAL_ACK_RECVD) | + X(TBF_EV_MAX_N3101) | + X(TBF_EV_MAX_N3105), .out_state_mask = X(TBF_ST_FINISHED) | X(TBF_ST_WAIT_RELEASE) | @@ -229,18 +259,24 @@ static struct osmo_fsm_state tbf_fsm_states[] = { }, [TBF_ST_FINISHED] = { .in_event_mask = - X(TBF_EV_FINAL_ACK_RECVD), + X(TBF_EV_FINAL_ACK_RECVD) | + X(TBF_EV_MAX_N3103) | + X(TBF_EV_MAX_N3105), .out_state_mask = - X(TBF_ST_WAIT_RELEASE), + X(TBF_ST_WAIT_RELEASE) | + X(TBF_ST_RELEASING), .name = "FINISHED", .action = st_finished, }, [TBF_ST_WAIT_RELEASE] = { .in_event_mask = - 0, + X(TBF_EV_FINAL_ACK_RECVD) | + X(TBF_EV_MAX_N3101) | + X(TBF_EV_MAX_N3105), .out_state_mask = X(TBF_ST_RELEASING), .name = "WAIT_RELEASE", + .action = st_wait_release, }, [TBF_ST_RELEASING] = { .in_event_mask = diff --git a/src/tbf_fsm.h b/src/tbf_fsm.h index 1dba80ff..ae0d6aef 100644 --- a/src/tbf_fsm.h +++ b/src/tbf_fsm.h @@ -35,6 +35,9 @@ enum tbf_fsm_event { TBF_EV_LAST_DL_DATA_SENT, /* DL TBF sends RLCMAC block containing last DL avilable data buffered */ TBF_EV_LAST_UL_DATA_RECVD, /* UL TBF sends RLCMAC block containing last UL data (cv=0) */ TBF_EV_FINAL_ACK_RECVD, /* DL ACK/NACK with FINAL_ACK=1 received from MS */ + TBF_EV_MAX_N3101, /* MAX N3101 (max usf timeout) reached (UL TBF) */ + TBF_EV_MAX_N3103, /* MAX N3103 (max Pkt Ctrl Ack for last UL ACK/NACK timeout) reached (UL TBF) */ + TBF_EV_MAX_N3105, /* MAX N3105 (max poll timeout) reached (UL/DL TBF) */ }; enum tbf_fsm_states { diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp index 3eec555d..a7a7c3e4 100644 --- a/src/tbf_ul.cpp +++ b/src/tbf_ul.cpp @@ -817,7 +817,7 @@ gprs_rlc_window *gprs_rlcmac_ul_tbf::window() void gprs_rlcmac_ul_tbf::usf_timeout() { if (n_inc(N3101)) { - TBF_SET_STATE(this, TBF_ST_RELEASING); + osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_MAX_N3101, NULL); T_START(this, T3169, 3169, "MAX N3101 reached", false); return; } diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err index c6cea787..62a3a51e 100644 --- a/tests/tbf/TbfTest.err +++ b/tests/tbf/TbfTest.err @@ -3083,6 +3083,7 @@ TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW) N3101 8 => 9 (< MAX 10) PDCH(bts=0,trx=0,ts=7) Expiring FN=2654379 but previous FN=2654318 is still reserved! PDCH(bts=0,trx=0,ts=7) Timeout for registered USF (FN=2654318): TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW) TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW) N3101 exceeded MAX (10) +TBF(UL-TFI_0){FLOW}: Received Event MAX_N3101 TBF(UL-TFI_0){FLOW}: state_chg to RELEASING TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=RELEASING) starting timer T3169 [MAX N3101 reached] with 5 sec. 0 microsec, cur_fn=2654379 PDCH(bts=0,trx=0,ts=7) Expiring FN=2654379 but previous FN=2654322 is still reserved! diff --git a/tests/ulc/PdchUlcTest.err b/tests/ulc/PdchUlcTest.err index edc7a118..e72d59fc 100644 --- a/tests/ulc/PdchUlcTest.err +++ b/tests/ulc/PdchUlcTest.err @@ -22,25 +22,25 @@ PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715635): TBF(TFI=0 TLLI= PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715639 is still reserved! PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715639): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715643 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715643): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715643): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=4 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=4): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=4): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=8 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=8): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=8): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=13 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=13): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=13): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=17 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=17): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=17): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=21 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=21): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=21): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=26 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=26): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=26): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=30 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=30): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=30): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=34 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=34): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=34): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=39 is still reserved! -PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=39): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING) +PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=39): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL) PDCH(bts=0,trx=0,ts=0) POLL scheduled at FN 26 + 13 = 39 PDCH(bts=0,trx=0,ts=0) UL block already scheduled at FN 91 + 13 = 104 PDCH(bts=0,trx=0,ts=0) POLL scheduled at FN 91 + 17 = 108