This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/18323 ) Change subject: tbf_ul: Fix UL ACK not sent to MS if intermediate UL block is lost ...................................................................... tbf_ul: Fix UL ACK not sent to MS if intermediate UL block is lost In normal conditions ACKing of UL blocks is only sent every SEND_ACK_AFTER_FRAMES (20) frames. Which means if CV=0 is received (and hence no more packets are received) less than 20 frames before a lost, the PCU won't ask for a retransmission and wait there until some timer destroys the TBF. This issue is shown by TTCN3 test PCU_Tests.ttcn TC_ul_intermediate_retrans. Unit tests triggering this condition are adapted. Some similar tests are not triggering it because BSN/CV relation being used is totally wrong (like CV=0 being sent on a BSN with previous value than others). Change-Id: I9b4ef7b7277efa645bdb5becf2e9f6b32c99a9b1 --- M src/tbf_ul.cpp M src/tbf_ul.h M tests/tbf/TbfTest.cpp M tests/tbf/TbfTest.err 4 files changed, 25 insertions(+), 16 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/23/18323/1 diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp index 9899580..b3a341a 100644 --- a/src/tbf_ul.cpp +++ b/src/tbf_ul.cpp @@ -188,6 +188,9 @@ const struct gprs_rlc_data_info *rlc, uint8_t *data, struct pcu_l1_meas *meas) { + const struct gprs_rlc_data_block_info *rdbi; + struct gprs_rlc_data *block; + int8_t rssi = meas->have_rssi ? meas->rssi : 0; const uint16_t ws = m_window.ws(); @@ -218,10 +221,8 @@ for (block_idx = 0; block_idx < rlc->num_data_blocks; block_idx++) { int num_chunks; uint8_t *rlc_data; - const struct gprs_rlc_data_block_info *rdbi = - &rlc->block_info[block_idx]; + rdbi = &rlc->block_info[block_idx]; bool need_rlc_data = false; - struct gprs_rlc_data *block; LOGPTBFUL(this, LOGL_DEBUG, "Got %s RLC data block: CV=%d, BSN=%d, SPB=%d, PI=%d, E=%d, TI=%d, bitoffs=%d\n", @@ -330,14 +331,13 @@ assemble_forward_llc(m_rlc.block(index)); } - /* Check CV of last frame in buffer */ + /* Last frame in buffer: */ + block = m_rlc.block(m_window.mod_sns(m_window.v_r() - 1)); + rdbi = &block->block_info; + + /* Check if we already received all data TBF had to send: */ if (this->state_is(GPRS_RLCMAC_FLOW) /* still in flow state */ && this->m_window.v_q() == this->m_window.v_r()) { /* if complete */ - struct gprs_rlc_data *block = - m_rlc.block(m_window.mod_sns(m_window.v_r() - 1)); - const struct gprs_rlc_data_block_info *rdbi = - &block->block_info; - LOGPTBFUL(this, LOGL_DEBUG, "No gaps in received block, last block: BSN=%d CV=%d\n", rdbi->bsn, rdbi->cv); @@ -351,13 +351,13 @@ /* If TLLI is included or if we received half of the window, we send * an ack/nack */ - maybe_schedule_uplink_acknack(rlc); + maybe_schedule_uplink_acknack(rlc, rdbi->cv == 0); return 0; } void gprs_rlcmac_ul_tbf::maybe_schedule_uplink_acknack( - const gprs_rlc_data_info *rlc) + const gprs_rlc_data_info *rlc, bool countdown_finished) { bool require_ack = false; bool have_ti = rlc->block_info[0].ti || @@ -373,10 +373,14 @@ LOGPTBFUL(this, LOGL_DEBUG, "Scheduling Ack/Nack, because TLLI is included.\n"); } - if (state_is(GPRS_RLCMAC_FINISHED)) { + if (countdown_finished) { require_ack = true; - LOGPTBFUL(this, LOGL_DEBUG, - "Scheduling final Ack/Nack, because all data was received and last block has CV==0.\n"); + if (state_is(GPRS_RLCMAC_FLOW)) + LOGPTBFUL(this, LOGL_DEBUG, + "Scheduling Ack/Nack, because some data is missing and last block has CV==0.\n"); + else if (state_is(GPRS_RLCMAC_FINISHED)) + LOGPTBFUL(this, LOGL_DEBUG, + "Scheduling final Ack/Nack, because all data was received and last block has CV==0.\n"); } if ((m_rx_counter % SEND_ACK_AFTER_FRAMES) == 0) { require_ack = true; @@ -384,7 +388,6 @@ "Scheduling Ack/Nack, because %d frames received.\n", SEND_ACK_AFTER_FRAMES); } - if (!require_ack) return; diff --git a/src/tbf_ul.h b/src/tbf_ul.h index 85da4f6..c6df0cd 100644 --- a/src/tbf_ul.h +++ b/src/tbf_ul.h @@ -21,6 +21,8 @@ #ifdef __cplusplus +#include <stdbool.h> + #include "tbf.h" /* * TBF instance @@ -97,7 +99,7 @@ struct rate_ctr_group *m_ul_egprs_ctrs; protected: - void maybe_schedule_uplink_acknack(const gprs_rlc_data_info *rlc); + void maybe_schedule_uplink_acknack(const gprs_rlc_data_info *rlc, bool countdown_finished); /* Please note that all variables below will be reset when changing * from WAIT RELEASE back to FLOW state (re-use of TBF). diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp index 36f9ced..3f2925a 100644 --- a/tests/tbf/TbfTest.cpp +++ b/tests/tbf/TbfTest.cpp @@ -1459,6 +1459,7 @@ pdch = &the_bts->bts_data()->trx[trx_no].pdch[ts_no]; pdch->rcv_block(&data[0], sizeof(data), *fn, &meas); + ul_tbf->create_ul_ack(*fn, ts_no); request_dl_rlc_block(ul_tbf, fn); diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err index a1530c1..57787c5 100644 --- a/tests/tbf/TbfTest.err +++ b/tests/tbf/TbfTest.err @@ -7193,6 +7193,9 @@ TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW EGPRS) Got MCS-4 RLC data block: CV=0, BSN=64, SPB=0, PI=0, E=1, TI=0, bitoffs=33 TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW EGPRS) BSN 64 storing in window (1..192) TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW EGPRS) data_length=44, data=80 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 95 +TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW EGPRS) Scheduling Ack/Nack, because some data is missing and last block has CV==0. +TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW EGPRS) changes UL ACK state from GPRS_RLCMAC_UL_ACK_NONE to GPRS_RLCMAC_UL_ACK_SEND_ACK +TBF(TFI=0 TLLI=0xf1223344 DIR=UL STATE=FLOW EGPRS) changes UL ACK state from GPRS_RLCMAC_UL_ACK_SEND_ACK to GPRS_RLCMAC_UL_ACK_NONE Received RTS for PDCH: TRX=0 TS=7 FN=2654279 block_nr=10 scheduling USF=0 for required uplink resource of UL TFI=0 TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=ASSIGN EGPRS) start Packet Downlink Assignment (PACCH) +++++++++++++++++++++++++ TX : Packet Downlink Assignment +++++++++++++++++++++++++ -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/18323 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I9b4ef7b7277efa645bdb5becf2e9f6b32c99a9b1 Gerrit-Change-Number: 18323 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin at sysmocom.de> Gerrit-MessageType: newchange -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200515/ccac34ab/attachment.htm>