Change in osmo-pcu[master]: tbf_ul: Fix UL ACK not sent to MS if intermediate UL block is lost

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.org
Fri May 15 16:57:19 UTC 2020


pespin 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>


More information about the gerrit-log mailing list