Change in osmo-pcu[master]: tbf_dl: Fix m_last_dl_drained_fn not set under some conditions

laforge gerrit-no-reply at lists.osmocom.org
Wed Mar 3 07:29:52 UTC 2021


laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/23197 )

Change subject: tbf_dl: Fix m_last_dl_drained_fn not set under some conditions
......................................................................

tbf_dl: Fix m_last_dl_drained_fn not set under some conditions

Old commit getting rid of LLC UI dummy and updating create_new_bsn()
function introduced a bug by not moving update of value m_last_dl_drained_fn
prior to a new break introduced.
As a result, the value is not updated in the case LLC queue becomes
drained but last few bytes are drained at exactly that moment.
Furthermore, then the IDLE tbf timer (X2031, keep_open())) returns always
true since according to it the drain never happened.

The impact of the bug is basically delaying a bit more than expected the
time the TBF stays in IDLE state with the TBF release process yet
to be started.

Related: OS#4849
Fixes: 7d0f9a0ec383fcfca934731bd6979b6be6629c90
Change-Id: I7420aeffda3500bcdc990291e4a56511af433ff9
---
M src/tbf_dl.cpp
M tests/tbf/TbfTest.err
2 files changed, 8 insertions(+), 8 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/tbf_dl.cpp b/src/tbf_dl.cpp
index 2896378..d963644 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -684,6 +684,10 @@
 		int payload_written = 0;
 
 		if (llc_frame_length(&m_llc) == 0) {
+			/* The data just drained, store the current fn */
+			if (m_last_dl_drained_fn < 0)
+				m_last_dl_drained_fn = fn;
+
 			/* It is not clear, when the next real data will
 			 * arrive, so request a DL ack/nack now */
 			request_dl_ack();
@@ -715,10 +719,6 @@
 			 * space-1 octets */
 			m_llc.put_dummy_frame(space - 1);
 
-			/* The data just drained, store the current fn */
-			if (m_last_dl_drained_fn < 0)
-				m_last_dl_drained_fn = fn;
-
 			LOGPTBFDL(this, LOGL_DEBUG,
 				  "Empty chunk, added LLC dummy command of size %d, drained_since=%d\n",
 				  llc_frame_length(&m_llc), frames_since_last_drain(fn));
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index 1b0739f..515b649 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -431,8 +431,8 @@
 Scheduling data message at RTS for DL TFI=0 (TRX=0, TS=4) prio=4 mcs_mode_restrict=EGPRS
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) downlink (V(A)==21 .. V(S)==21) mcs_mode_restrict=EGPRS
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Sending new dummy block at BSN 21, CS=CS-1
-TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Empty chunk, added LLC dummy command of size 19, drained_since=0
-TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Keep idle TBF open: 0/43 -> yes
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Empty chunk, added LLC dummy command of size 19, drained_since=4
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Keep idle TBF open: 4/43 -> yes
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Complete DL frame, len=19
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) data block (BSN 21, CS-1): 4d 43 c0 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) need_padding 0 spb_status 0 spb 0 (BSN1 21 BSN2 -1)
@@ -448,8 +448,8 @@
 Scheduling data message at RTS for DL TFI=0 (TRX=0, TS=4) prio=4 mcs_mode_restrict=EGPRS
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) downlink (V(A)==22 .. V(S)==22) mcs_mode_restrict=EGPRS
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Sending new dummy block at BSN 22, CS=CS-1
-TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Empty chunk, added LLC dummy command of size 19, drained_since=108
-TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Keep idle TBF open: 108/43 -> no
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Empty chunk, added LLC dummy command of size 19, drained_since=112
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Keep idle TBF open: 112/43 -> no
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) Complete DL frame, len=19
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) changes state from FLOW to FINISHED
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FINISHED) data block (BSN 22, CS-1): 4d 43 c0 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23197
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I7420aeffda3500bcdc990291e4a56511af433ff9
Gerrit-Change-Number: 23197
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210303/9646978f/attachment.htm>


More information about the gerrit-log mailing list