[PATCH] osmo-pcu[master]: Simplify TS alloc: replace debug printer

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

Max gerrit-no-reply at lists.osmocom.org
Thu Sep 14 10:19:33 UTC 2017


Hello Harald Welte,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/3929

to look at the new patch set (#2).

Simplify TS alloc: replace debug printer

Replace unreadable recursive debug printer with simpler functions.

Note: the new printer also correctly handle reserved TS so Control slot
overrides TS for the bits set for both Uplink and Downlink slots. This
does not change the allocation semantics of course but requires cosmetic
adjustement to TBF tests output.

This requires I72528bc1e376134c5a7b6e7a50c48e38c3f48b0a in libosmocore.

Change-Id: Ia13855877b2145cb57b1646f5562b2af3b87bcfb
Related: OS#2282
---
M src/gprs_rlcmac.h
M src/gprs_rlcmac_ts_alloc.cpp
M tests/tbf/TbfTest.err
3 files changed, 44 insertions(+), 60 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/29/3929/2

diff --git a/src/gprs_rlcmac.h b/src/gprs_rlcmac.h
index 0616069..004f77a 100644
--- a/src/gprs_rlcmac.h
+++ b/src/gprs_rlcmac.h
@@ -65,6 +65,8 @@
 	uint8_t block_payload;
 };
 
+void ts_print(char *buf, uint8_t dl_mask, uint8_t ul_mask);
+
 int gprs_rlcmac_received_lost(struct gprs_rlcmac_dl_tbf *tbf, uint16_t received,
 	uint16_t lost);
 
diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 0cebd93..9eb8464 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -81,18 +81,19 @@
 /* N/A */	{ MS_NA,MS_NA,	MS_NA,	MS_NA,	MS_NA,	MS_NA,	MS_NA,	MS_NA },
 };
 
-static char *set_flag_chars(char *buf, uint8_t val, char set_char, char unset_char = 0)
+static inline void masked_override_with(char *buf, uint8_t mask, char set_char)
 {
 	int i;
-
-	for (i = 0; i < 8; i += 1, val = val >> 1) {
-		if (val & 1)
+	for (i = 0; mask; i++, mask >>= 1)
+		if (mask & 1)
 			buf[i] = set_char;
-		else if (unset_char)
-			buf[i] = unset_char;
-	}
+}
 
-	return buf;
+void ts_print(char *buf, uint8_t dl_mask, uint8_t ul_mask)
+{
+	snprintf(buf, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(dl_mask, 'D'));
+	masked_override_with(buf, ul_mask, 'U');
+	masked_override_with(buf, ul_mask & dl_mask, 'C');
 }
 
 static bool test_and_set_bit(uint32_t *bits, size_t elem)
@@ -588,11 +589,8 @@
 	*dl_slots &= pdch_slots;
 	*ul_slots &= pdch_slots;
 
-	LOGP(DRLCMAC, LOGL_DEBUG, "- Possible DL/UL slots: (TS=0)\"%s\"(TS=7)\n",
-		set_flag_chars(set_flag_chars(set_flag_chars(slot_info,
-				*dl_slots, 'D', '.'),
-				*ul_slots, 'U'),
-				*ul_slots & *dl_slots, 'C'));
+	ts_print(slot_info, *dl_slots, *ul_slots);
+	LOGP(DRLCMAC, LOGL_DEBUG, "- Possible DL/UL slots: (TS=0)\"%s\"(TS=7)\n", slot_info);
 
 	/* Check for each UL (TX) slot */
 
@@ -704,14 +702,12 @@
 			} else {
 				/* No supported row in table 6.4.2.2.1. */
 #ifdef ENABLE_TS_ALLOC_DEBUG
+				snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
+				masked_override_with(slot_info, rx_window, 'D');
+				masked_override_with(slot_info, tx_window, 'U');
 				LOGP(DRLCMAC, LOGL_DEBUG,
-					"- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-					"combination not supported\n",
-					set_flag_chars(set_flag_chars(set_flag_chars(
-								slot_info,
-								rx_bad, 'x', '.'),
-							rx_window, 'D'),
-						tx_window, 'U'));
+				     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
+				     "combination not supported\n", slot_info);
 #endif
 				continue;
 			}
@@ -723,12 +719,11 @@
 
 		if (!rx_good) {
 #ifdef ENABLE_TS_ALLOC_DEBUG
+			snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
+			masked_override_with(slot_info, tx_window, 'U');
 			LOGP(DRLCMAC, LOGL_DEBUG,
-				"- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-				"no DL slots available\n",
-				set_flag_chars(set_flag_chars(slot_info,
-						rx_bad, 'x', '.'),
-						tx_window, 'U'));
+			     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
+			     "no DL slots available\n", slot_info);
 #endif
 			continue;
 		}
@@ -744,16 +739,13 @@
 
 		if (req_common_slots != common_slot_count) {
 #ifdef ENABLE_TS_ALLOC_DEBUG
+			snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
+			masked_override_with(slot_info, rx_window, 'D');
+			masked_override_with(slot_info, tx_window, 'U');
 			LOGP(DRLCMAC, LOGL_DEBUG,
-				"- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
-				"invalid number of common TS: %d (expected %d)\n",
-				set_flag_chars(set_flag_chars(set_flag_chars(
-							slot_info,
-							rx_bad, 'x', '.'),
-						rx_window, 'D'),
-					tx_window, 'U'),
-				common_slot_count,
-				req_common_slots);
+			     "- Skipping DL/UL slots: (TS=0)\"%s\"(TS=7), "
+			     "invalid number of common TS: %d (expected %d)\n",
+			     slot_info, common_slot_count, req_common_slots);
 #endif
 			continue;
 		}
@@ -780,16 +772,14 @@
 		}
 
 #ifdef ENABLE_TS_ALLOC_DEBUG
+		snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(rx_bad, 'x'));
+		masked_override_with(slot_info, rx_window, 'D');
+		masked_override_with(slot_info, tx_window, 'U');
+		masked_override_with(slot_info, rx_window& tx_window, 'C');
 		LOGP(DRLCMAC, LOGL_DEBUG,
-			"- Considering DL/UL slots: (TS=0)\"%s\"(TS=7), "
-			"capacity = %d\n",
-			set_flag_chars(set_flag_chars(set_flag_chars(set_flag_chars(
-					slot_info,
-					rx_bad, 'x', '.'),
-					rx_window, 'D'),
-					tx_window, 'U'),
-					rx_window & tx_window, 'C'),
-			capacity);
+		     "- Considering DL/UL slots: (TS=0)\"%s\"(TS=7), "
+		     "capacity = %d\n",
+		     slot_info, capacity);
 #endif
 
 		if (capacity <= max_capacity)
@@ -915,12 +905,10 @@
 	}
 
 	if (tbf->direction == GPRS_RLCMAC_DL_TBF) {
+		snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(reserved_dl_slots, 'd'));
+		masked_override_with(slot_info, dl_slots, 'D');
 		LOGP(DRLCMAC, LOGL_DEBUG,
-			"- Selected DL slots: (TS=0)\"%s\"(TS=7)%s\n",
-			set_flag_chars(set_flag_chars(slot_info,
-					reserved_dl_slots, 'd', '.'),
-					dl_slots, 'D'),
-			single ? ", single" : "");
+		     "- Selected DL slots: (TS=0)\"%s\"(TS=7)%s\n", slot_info, single ? ", single" : "");
 
 		/* assign downlink */
 		if (dl_slots == 0) {
@@ -953,12 +941,10 @@
 		ul_slots = 1 << ts;
 		usf[ts] = free_usf;
 
+		snprintf(slot_info, 9, OSMO_BIT_SPEC, OSMO_BIT_PRINT_EX(reserved_ul_slots, 'u'));
+		masked_override_with(slot_info, ul_slots, 'U');
 		LOGP(DRLCMAC, LOGL_DEBUG,
-			"- Selected UL slots: (TS=0)\"%s\"(TS=7)%s\n",
-			set_flag_chars(set_flag_chars(slot_info,
-					reserved_ul_slots, 'u', '.'),
-					ul_slots, 'U'),
-			single ? ", single" : "");
+		     "- Selected UL slots: (TS=0)\"%s\"(TS=7)%s\n",slot_info, single ? ", single" : "");
 
 		slotcount++;
 		first_ts = ts;
@@ -1003,12 +989,8 @@
 		ms_->set_reserved_slots(trx,
 			reserved_ul_slots, reserved_dl_slots);
 
-		LOGP(DRLCMAC, LOGL_DEBUG,
-			"- Reserved DL/UL slots: (TS=0)\"%s\"(TS=7)\n",
-			set_flag_chars(set_flag_chars(set_flag_chars(slot_info,
-				dl_slots, 'D', '.'),
-				ul_slots, 'U'),
-				ul_slots & dl_slots, 'C'));
+		ts_print(slot_info, dl_slots, ul_slots);
+		LOGP(DRLCMAC, LOGL_DEBUG, "- Reserved DL/UL slots: (TS=0)\"%s\"(TS=7)\n", slot_info);
 	}
 
 	tbf_->trx = trx;
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index 602e35b..9016eb1 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -6549,7 +6549,7 @@
 - Possible DL/UL slots: (TS=0)"..CCCC.."(TS=7)
 - Selected DL slots: (TS=0)"..ddDd.."(TS=7), single
 Using single slot at TS 4 for DL
-- Reserved DL/UL slots: (TS=0)"....C..."(TS=7)
+- Reserved DL/UL slots: (TS=0)"...DC..."(TS=7)
 - Assigning DL TS 4
 PDCH(TS 4, TRX 0): Attaching TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=NULL EGPRS), 1 TBFs, USFs = 00, TFIs = 00000001.
 - Setting Control TS 4

-- 
To view, visit https://gerrit.osmocom.org/3929
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia13855877b2145cb57b1646f5562b2af3b87bcfb
Gerrit-PatchSet: 2
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>



More information about the gerrit-log mailing list