Change in osmo-pcu[master]: tbf: Don't log rlcmac_diag() output in separate lines

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/.

laforge gerrit-no-reply at lists.osmocom.org
Tue Sep 22 19:21:51 UTC 2020 

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

Change subject: tbf: Don't log rlcmac_diag() output in separate lines
......................................................................

tbf: Don't log rlcmac_diag() output in separate lines

Output of all diag in different lines is really confusing, since the
user reads a timeout ocurred and then later in another line something
like "Downlink ACK was received" while no GSMTAP message shows any ACK.

Change-Id: I6a7d79c16c930f0712bc73b308409ececb1946ba
---
M src/tbf.cpp
M src/tbf.h
M tests/tbf/TbfTest.err
3 files changed, 31 insertions(+), 22 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.cpp b/src/tbf.cpp
index 0447edc..9545511 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -20,6 +20,7 @@
  */
 
 #include <new>
+#include <sstream>
 
 #include <bts.h>
 #include <tbf.h>
@@ -647,8 +648,13 @@
 	LOGPTBF(tbf, LOGL_NOTICE, "%s timeout expired, freeing TBF\n",
 		get_value_string(tbf_timers_names, t));
 
-	if (run_diag)
-		tbf->rlcmac_diag();
+	if (run_diag) {
+		LOGPTBF(tbf, LOGL_NOTICE, "%s timeout expired, freeing TBF: %s\n",
+			get_value_string(tbf_timers_names, t), tbf->rlcmac_diag().c_str());
+	} else {
+		LOGPTBF(tbf, LOGL_NOTICE, "%s timeout expired, freeing TBF\n",
+			get_value_string(tbf_timers_names, t));
+	}
 
 	tbf_free(tbf);
 }
@@ -816,8 +822,9 @@
 
 	if (ul_tbf && ul_tbf->handle_ctrl_ack()) {
 		if (!ul_tbf->ctrl_ack_to_toggle()) {
-			LOGPTBF(this, LOGL_NOTICE, "Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ACK\n");
-			rlcmac_diag();
+			LOGPTBF(this, LOGL_NOTICE,
+				"Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ACK: %s\n",
+				rlcmac_diag().c_str());
 		}
 		bts->do_rate_ctr_inc(CTR_RLC_ACK_TIMEDOUT);
 		bts->do_rate_ctr_inc(CTR_PUAN_POLL_TIMEDOUT);
@@ -835,8 +842,8 @@
 	} else if (ul_ass_state == GPRS_RLCMAC_UL_ASS_WAIT_ACK) {
 		if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ASS))) {
 			LOGPTBF(this, LOGL_NOTICE,
-				"Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT.\n");
-			rlcmac_diag();
+				"Timeout for polling PACKET CONTROL ACK for PACKET UPLINK ASSIGNMENT: %s\n",
+				rlcmac_diag().c_str());
 			state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ASS);
 		}
 		ul_ass_state = GPRS_RLCMAC_UL_ASS_NONE;
@@ -854,8 +861,8 @@
 	} else if (dl_ass_state == GPRS_RLCMAC_DL_ASS_WAIT_ACK) {
 		if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ASS))) {
 			LOGPTBF(this, LOGL_NOTICE,
-				"Timeout for polling PACKET CONTROL ACK for PACKET DOWNLINK ASSIGNMENT.\n");
-			rlcmac_diag();
+				"Timeout for polling PACKET CONTROL ACK for PACKET DOWNLINK ASSIGNMENT: %s\n",
+				rlcmac_diag().c_str());
 			state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_DL_ASS);
 		}
 		dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE;
@@ -875,8 +882,8 @@
 
 		if (!(dl_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK))) {
 			LOGPTBF(this, LOGL_NOTICE,
-				"Timeout for polling PACKET DOWNLINK ACK.\n");
-			dl_tbf->rlcmac_diag();
+				"Timeout for polling PACKET DOWNLINK ACK: %s\n",
+				dl_tbf->rlcmac_diag().c_str());
 			dl_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK);
 		}
 
@@ -1169,22 +1176,24 @@
 	}
 }
 
-int gprs_rlcmac_tbf::rlcmac_diag()
+std::string gprs_rlcmac_tbf::rlcmac_diag()
 {
+	std::ostringstream os;
+	os << "|";
 	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH)))
-		LOGPTBF(this, LOGL_NOTICE, "Assignment was on CCCH\n");
+		os << "Assignment was on CCCH|";
 	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_PACCH)))
-		LOGPTBF(this, LOGL_NOTICE, "Assignment was on PACCH\n");
+		os << "Assignment was on PACCH|";
 	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_UL_DATA)))
-		LOGPTBF(this, LOGL_NOTICE, "Uplink data was received\n");
+		os << "Uplink data was received|";
 	else if (direction == GPRS_RLCMAC_UL_TBF)
-		LOGPTBF(this, LOGL_NOTICE, "No uplink data received yet\n");
+		os << "No uplink data received yet|";
 	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_DL_ACK)))
-		LOGPTBF(this, LOGL_NOTICE, "Downlink ACK was received\n");
+		os << "Downlink ACK was received|";
 	else if (direction == GPRS_RLCMAC_DL_TBF)
-		LOGPTBF(this, LOGL_NOTICE, "No downlink ACK received yet\n");
+		os << "No downlink ACK received yet|";
 
-	return 0;
+	return os.str();
 }
 
 struct msgb *gprs_rlcmac_tbf::create_dl_ass(uint32_t fn, uint8_t ts)
diff --git a/src/tbf.h b/src/tbf.h
index 528bee3..99aba67 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -20,6 +20,8 @@
 
 #ifdef __cplusplus
 
+#include <string>
+
 #include "llc.h"
 #include "rlc.h"
 #include "cxx_linuxlist.h"
@@ -212,7 +214,7 @@
 
 	uint8_t tsc() const;
 
-	int rlcmac_diag();
+	std::string rlcmac_diag();
 
 	bool n_inc(enum tbf_counters n);
 	void n_reset(enum tbf_counters n);
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index 681e7a3..ce23802 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -1725,9 +1725,7 @@
 Searching for first unallocated TFI: TRX=0
  Found TFI=1.
 TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=FINISHED) poll timeout for FN=2654292, TS=7 (curr FN 2654335)
-TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=FINISHED) Timeout for polling PACKET DOWNLINK ACK.
-TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=FINISHED) Assignment was on PACCH
-TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=FINISHED) No downlink ACK received yet
+TBF(TFI=0 TLLI=0xf1223344 DIR=DL STATE=FINISHED) Timeout for polling PACKET DOWNLINK ACK: |Assignment was on PACCH|No downlink ACK received yet|
 +++++++++++++++++++++++++ RX : Uplink Control Block +++++++++++++++++++++++++
 ------------------------- RX : Uplink Control Block -------------------------
 Creating MS object, TLLI = 0x00000000

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

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I6a7d79c16c930f0712bc73b308409ececb1946ba
Gerrit-Change-Number: 20243
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-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200922/96fa6193/attachment.htm>

More information about the gerrit-log mailing list