pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33263 ) Change subject: Avoid using N3105 in UL TBFs ...................................................................... Avoid using N3105 in UL TBFs This counter is only used in DL TBFs as per TS 44.060. section 13.4 "N3105": "When the network after sending a RRBP field in the _downlink RLC data block_ ..." (DL data blocks are only sent in DL TBFs). section 8.1.2.1: "If N3105 = N3105max, the network shall release the downlink TBF internally and start timer T3195 for that TBF. When T3195 expires, the network may reuse the TFI" Change-Id: I4d4f4d4d3e6e0539ea8ec2395bed00d059b84e04 --- M src/pdch.cpp M src/tbf.cpp M src/tbf_ul_fsm.c M tests/tbf/TbfTest.err 4 files changed, 34 insertions(+), 42 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/pdch.cpp b/src/pdch.cpp index 2cee011..f871d1d 100644 --- a/src/pdch.cpp +++ b/src/pdch.cpp @@ -391,8 +391,8 @@ } LOGPTBF(tbf, LOGL_DEBUG, "[DOWNLINK] UPLINK ASSIGNED\n"); pdch_ulc_release_fn(ulc, fn); - /* reset N3105 */ - tbf->n_reset(N3105); + if (tbf_direction(tbf) == GPRS_RLCMAC_DL_TBF) + tbf->n_reset(N3105); osmo_fsm_inst_dispatch(tbf->ul_ass_fsm.fi, TBF_UL_ASS_EV_RX_ASS_CTRL_ACK, NULL); new_tbf = ms_ul_tbf(ms); @@ -422,8 +422,8 @@ } LOGPTBF(tbf, LOGL_DEBUG, "[UPLINK] DOWNLINK ASSIGNED\n"); pdch_ulc_release_fn(ulc, fn); - /* reset N3105 */ - tbf->n_reset(N3105); + if (tbf_direction(tbf) == GPRS_RLCMAC_DL_TBF) + tbf->n_reset(N3105); osmo_fsm_inst_dispatch(tbf->dl_ass_fsm.fi, TBF_DL_ASS_EV_RX_ASS_CTRL_ACK, NULL); new_tbf = ms_dl_tbf(ms); diff --git a/src/tbf.cpp b/src/tbf.cpp index 47256f8..885c602 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -316,6 +316,9 @@ case N3103: OSMO_ASSERT(direction == GPRS_RLCMAC_UL_TBF); break; + case N3105: + OSMO_ASSERT(direction == GPRS_RLCMAC_DL_TBF); + break; default: break; } @@ -342,6 +345,7 @@ chk = bts->n3103; break; case N3105: + OSMO_ASSERT(direction == GPRS_RLCMAC_DL_TBF); chk = bts->n3105; break; default: @@ -513,7 +517,7 @@ } bts_do_rate_ctr_inc(bts, CTR_RLC_ASS_TIMEDOUT); bts_do_rate_ctr_inc(bts, CTR_PUA_POLL_TIMEDOUT); - if (n_inc(N3105)) { + if (this->direction == GPRS_RLCMAC_DL_TBF && n_inc(N3105)) { osmo_fsm_inst_dispatch(this->state_fi, TBF_EV_MAX_N3105, NULL); bts_do_rate_ctr_inc(bts, CTR_RLC_ASS_FAILED); bts_do_rate_ctr_inc(bts, CTR_PUA_POLL_FAILED); @@ -532,7 +536,7 @@ } bts_do_rate_ctr_inc(bts, CTR_RLC_ASS_TIMEDOUT); bts_do_rate_ctr_inc(bts, CTR_PDA_POLL_TIMEDOUT); - if (n_inc(N3105)) { + if (this->direction == GPRS_RLCMAC_DL_TBF && n_inc(N3105)) { osmo_fsm_inst_dispatch(this->state_fi, TBF_EV_MAX_N3105, NULL); bts_do_rate_ctr_inc(bts, CTR_RLC_ASS_FAILED); bts_do_rate_ctr_inc(bts, CTR_PDA_POLL_FAILED); diff --git a/src/tbf_ul_fsm.c b/src/tbf_ul_fsm.c index 13dc9fb..231ccd2 100644 --- a/src/tbf_ul_fsm.c +++ b/src/tbf_ul_fsm.c @@ -152,12 +152,6 @@ } tbf_ul_fsm_state_chg(fi, TBF_ST_FLOW); break; - case TBF_EV_MAX_N3105: - /* We are going to release, so abort any Pkt Ul Ass pending to be scheduled: */ - osmo_fsm_inst_dispatch(tbf_ul_ass_fi(ctx->tbf), TBF_UL_ASS_EV_ABORT, NULL); - ctx->T_release = 3195; - tbf_ul_fsm_state_chg(fi, TBF_ST_RELEASING); - break; default: OSMO_ASSERT(0); } @@ -199,10 +193,6 @@ ctx->T_release = 3169; tbf_ul_fsm_state_chg(fi, TBF_ST_RELEASING); break; - case TBF_EV_MAX_N3105: - ctx->T_release = 3195; - tbf_ul_fsm_state_chg(fi, TBF_ST_RELEASING); - break; default: OSMO_ASSERT(0); } @@ -246,10 +236,6 @@ ctx->T_release = 3169; tbf_ul_fsm_state_chg(fi, TBF_ST_RELEASING); break; - case TBF_EV_MAX_N3105: - ctx->T_release = 3195; - tbf_ul_fsm_state_chg(fi, TBF_ST_RELEASING); - break; default: OSMO_ASSERT(0); } @@ -278,11 +264,6 @@ static void st_releasing(struct osmo_fsm_inst *fi, uint32_t event, void *data) { switch (event) { - case TBF_EV_MAX_N3105: - /* This may be received here if the TBF had several polls - * allocated concurrently and several failed each increasing N3105 - * over MAX_N3015. We are already releasing, ignore.*/ - break; default: OSMO_ASSERT(0); } @@ -320,8 +301,7 @@ .in_event_mask = X(TBF_EV_ASSIGN_ADD_CCCH) | X(TBF_EV_ASSIGN_ADD_PACCH) | - X(TBF_EV_ASSIGN_ACK_PACCH) | - X(TBF_EV_MAX_N3105), + X(TBF_EV_ASSIGN_ACK_PACCH), .out_state_mask = X(TBF_ST_FLOW) | X(TBF_ST_FINISHED), @@ -334,8 +314,7 @@ X(TBF_EV_FIRST_UL_DATA_RECVD) | X(TBF_EV_CONTENTION_RESOLUTION_MS_SUCCESS) | X(TBF_EV_LAST_UL_DATA_RECVD) | - X(TBF_EV_MAX_N3101) | - X(TBF_EV_MAX_N3105), + X(TBF_EV_MAX_N3101), .out_state_mask = X(TBF_ST_ASSIGN) | X(TBF_ST_FINISHED) | @@ -347,18 +326,15 @@ .in_event_mask = X(TBF_EV_CONTENTION_RESOLUTION_MS_SUCCESS) | X(TBF_EV_FINAL_UL_ACK_CONFIRMED) | - X(TBF_EV_MAX_N3103) | - X(TBF_EV_MAX_N3105), + X(TBF_EV_MAX_N3103), .out_state_mask = X(TBF_ST_RELEASING), .name = "FINISHED", .action = st_finished, }, [TBF_ST_RELEASING] = { - .in_event_mask = - X(TBF_EV_MAX_N3105), - .out_state_mask = - 0, + .in_event_mask = 0, + .out_state_mask = 0, .name = "RELEASING", .action = st_releasing, .onenter = st_releasing_on_enter, diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err index 36e1c19..7ffebea 100644 --- a/tests/tbf/TbfTest.err +++ b/tests/tbf/TbfTest.err @@ -9658,7 +9658,6 @@ PDCH(bts=0,trx=0,ts=7) Expiring FN=56 but previous FN=2654231 is still reserved! PDCH(bts=0,trx=0,ts=7) Timeout for registered POLL (FN=2654231, reason=UL_ASS): TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} poll timeout for FN=2654231, TS=7 (curr FN 56) -TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} N3105 0 => 1 (< MAX 8) UL_ASS_TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){WAIT_ACK}: Received Event ASS_POLL_TIMEOUT TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT: |Assignment was on PACCH|No uplink data received yet| UL_ASS_TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){WAIT_ACK}: state_chg to SEND_ASS @@ -9715,7 +9714,6 @@ PDCH(bts=0,trx=0,ts=7) Expiring FN=60 but previous FN=2654231 is still reserved! PDCH(bts=0,trx=0,ts=7) Timeout for registered POLL (FN=2654231, reason=UL_ASS): TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} poll timeout for FN=2654231, TS=7 (curr FN 60) -TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} N3105 1 => 2 (< MAX 8) UL_ASS_TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){WAIT_ACK}: Received Event ASS_POLL_TIMEOUT TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT: |Assignment was on PACCH|No uplink data received yet| UL_ASS_TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){WAIT_ACK}: state_chg to SEND_ASS @@ -9772,7 +9770,6 @@ PDCH(bts=0,trx=0,ts=7) Expiring FN=65 but previous FN=2654231 is still reserved! PDCH(bts=0,trx=0,ts=7) Timeout for registered POLL (FN=2654231, reason=UL_ASS): TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){ASSIGN} TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){ASSIGN} poll timeout for FN=2654231, TS=7 (curr FN 65) -TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){ASSIGN} N3105 0 => 1 (< MAX 8) UL_ASS_TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){WAIT_ACK}: Received Event ASS_POLL_TIMEOUT TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){ASSIGN} Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT: |Assignment was on PACCH|No uplink data received yet| UL_ASS_TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){WAIT_ACK}: state_chg to SEND_ASS @@ -9829,7 +9826,6 @@ PDCH(bts=0,trx=0,ts=7) Expiring FN=69 but previous FN=2654231 is still reserved! PDCH(bts=0,trx=0,ts=7) Timeout for registered POLL (FN=2654231, reason=UL_ASS): TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} poll timeout for FN=2654231, TS=7 (curr FN 69) -TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} N3105 2 => 3 (< MAX 8) UL_ASS_TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){WAIT_ACK}: Received Event ASS_POLL_TIMEOUT TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){ASSIGN} Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT: |Assignment was on PACCH|No uplink data received yet| UL_ASS_TBF(UL:TFI-0-0-0:E:TLLI-0xffeeddcc){WAIT_ACK}: state_chg to SEND_ASS @@ -9886,7 +9882,6 @@ PDCH(bts=0,trx=0,ts=7) Expiring FN=73 but previous FN=2654231 is still reserved! PDCH(bts=0,trx=0,ts=7) Timeout for registered POLL (FN=2654231, reason=UL_ASS): TBF(UL:TFI-0-0-2:E:TLLI-0xffeeddce){ASSIGN} TBF(UL:TFI-0-0-2:E:TLLI-0xffeeddce){ASSIGN} poll timeout for FN=2654231, TS=7 (curr FN 73) -TBF(UL:TFI-0-0-2:E:TLLI-0xffeeddce){ASSIGN} N3105 0 => 1 (< MAX 8) UL_ASS_TBF(UL:TFI-0-0-2:E:TLLI-0xffeeddce){WAIT_ACK}: Received Event ASS_POLL_TIMEOUT TBF(UL:TFI-0-0-2:E:TLLI-0xffeeddce){ASSIGN} Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT: |Assignment was on PACCH|No uplink data received yet| UL_ASS_TBF(UL:TFI-0-0-2:E:TLLI-0xffeeddce){WAIT_ACK}: state_chg to SEND_ASS @@ -9943,7 +9938,6 @@ PDCH(bts=0,trx=0,ts=7) Expiring FN=78 but previous FN=2654231 is still reserved! PDCH(bts=0,trx=0,ts=7) Timeout for registered POLL (FN=2654231, reason=UL_ASS): TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){ASSIGN} TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){ASSIGN} poll timeout for FN=2654231, TS=7 (curr FN 78) -TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){ASSIGN} N3105 1 => 2 (< MAX 8) UL_ASS_TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){WAIT_ACK}: Received Event ASS_POLL_TIMEOUT TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){ASSIGN} Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT: |Assignment was on PACCH|No uplink data received yet| UL_ASS_TBF(UL:TFI-0-0-1:E:TLLI-0xffeeddcd){WAIT_ACK}: state_chg to SEND_ASS @@ -9997,7 +9991,6 @@ PDCH(bts=0,trx=0,ts=7) Expiring FN=82 but previous FN=2654231 is still reserved! PDCH(bts=0,trx=0,ts=7) Timeout for registered POLL (FN=2654231, reason=UL_ASS): TBF(UL:TFI-0-0-3:E:TLLI-0xffeeddcf){ASSIGN} TBF(UL:TFI-0-0-3:E:TLLI-0xffeeddcf){ASSIGN} poll timeout for FN=2654231, TS=7 (curr FN 82) -TBF(UL:TFI-0-0-3:E:TLLI-0xffeeddcf){ASSIGN} N3105 0 => 1 (< MAX 8) UL_ASS_TBF(UL:TFI-0-0-3:E:TLLI-0xffeeddcf){WAIT_ACK}: Received Event ASS_POLL_TIMEOUT TBF(UL:TFI-0-0-3:E:TLLI-0xffeeddcf){ASSIGN} Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT: |Assignment was on PACCH|No uplink data received yet| UL_ASS_TBF(UL:TFI-0-0-3:E:TLLI-0xffeeddcf){WAIT_ACK}: state_chg to SEND_ASS -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33263 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I4d4f4d4d3e6e0539ea8ec2395bed00d059b84e04 Gerrit-Change-Number: 33263 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-MessageType: merged