[PATCH] osmo-pcu[master]: Simplify TS alloc: move slot check into functions

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 28 15:51:10 UTC 2017


Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/3935

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

Simplify TS alloc: move slot check into functions

Move timeslot applicability check outside of nested for loop into
separate functions and document them.

This allows us to clarify types used in TS-related computations.

Change-Id: Ic39e848da47dc11357782362fdf6206d2c1457c2
Related: OS#2282
---
M src/gprs_rlcmac_ts_alloc.cpp
1 file changed, 97 insertions(+), 113 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/35/3935/4

diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 0301d79..2763100 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -139,11 +139,11 @@
 	return -1;
 }
 
-static int find_possible_pdchs(const struct gprs_rlcmac_trx *trx, size_t max_slots, uint8_t mask,
-			       const char *mask_reason = NULL)
+static uint8_t find_possible_pdchs(const struct gprs_rlcmac_trx *trx, uint8_t max_slots, uint8_t mask,
+				   const char *mask_reason = NULL)
 {
 	unsigned ts;
-	int valid_ts_set = 0;
+	uint8_t valid_ts_set = 0;
 	int8_t last_tsc = -1; /* must be signed */
 
 	for (ts = 0; ts < ARRAY_SIZE(trx->pdch); ts++) {
@@ -430,7 +430,7 @@
 	int trx_no;
 	int tfi = -1;
 	int usf = -1;
-	int mask = 0xff;
+	uint8_t mask = 0xff;
 	const char *mask_reason = NULL;
 	const GprsMs *ms = ms_;
 	const gprs_rlcmac_tbf *tbf = tbf_;
@@ -514,7 +514,7 @@
  *  \param[in] tx_window Transmit window
  *  \returns non-negative capacity
  */
-static inline unsigned compute_capacity(const struct gprs_rlcmac_trx *trx, int rx_window, int tx_window)
+static inline unsigned compute_capacity(const struct gprs_rlcmac_trx *trx, int16_t rx_window, int16_t tx_window)
 {
 	const struct gprs_rlcmac_pdch *pdch;
 	unsigned ts, capacity = 0;
@@ -534,6 +534,84 @@
 	return capacity;
 }
 
+/*! Decide if a given slot should be skipped by multislot allocator
+ *
+ *  \param[in] ms_class Pointer to MS Class object
+ *  \param[in] check_tr Flag indicating whether we should check for Tra or Tta parameters for a given MS class
+ *  \param[in] rx_window Receive window
+ *  \param[in] rx_slot_count Number of TS in RX
+ *  \param[in] tx_window Transmit window
+ *  \param[in] tx_slot_count Number of TS in TX
+ *  \param[in,out] checked_rx array with already checked RX timeslots
+ *  \returns true if the slot should be skipped, false otherwise
+ */
+static bool skip_slot(const struct gprs_ms_multislot_class *ms_class, bool check_tr, int16_t rx_window,
+		      uint8_t rx_slot_count, int16_t tx_window, uint8_t tx_slot_count, uint32_t *checked_rx)
+{
+	uint8_t common_slot_count, req_common_slots;
+
+	/* Check compliance with TS 45.002, table 6.4.2.2.1 */
+	/* Whether to skip this round doesn not only depend on the bit
+	 * sets but also on check_tr. Therefore this check must be done
+	 * before doing the test_and_set_bit shortcut. */
+	if (ms_class->type == 1) {
+		uint16_t slot_sum = rx_slot_count + tx_slot_count;
+		/* Assume down + up / dynamic.
+		 * TODO: For ext-dynamic, down only, up only add more cases.
+		 */
+		if (slot_sum <= 6 && tx_slot_count < 3) {
+			if (!check_tr)
+				return true; /* Skip Tta */
+		} else if (slot_sum > 6 && tx_slot_count < 3) {
+			if (check_tr)
+				return true; /* Skip Tra */
+		} else
+			return true; /* No supported row in TS 45.002, table 6.4.2.2.1. */
+	}
+
+	/* Avoid repeated RX combination check */
+	if (test_and_set_bit(checked_rx, rx_window))
+		return true;
+
+	/* Check number of common slots according to TS 45.002, §6.4.2.2 */
+	common_slot_count = pcu_bitcount(tx_window & rx_window);
+	req_common_slots = OSMO_MIN(tx_slot_count, rx_slot_count);
+	if (ms_class->type == 1)
+		req_common_slots = OSMO_MIN(req_common_slots, 2);
+
+	if (req_common_slots != common_slot_count)
+		return true;
+
+	return false;
+}
+
+/*! Filter out bad slots
+ *
+ *  \param[in] mask TS selection mask
+ *  \param[in] ul_slots set of UL timeslots
+ *  \param[in] dl_slots set of DL timeslots
+ *  \param[in] rx_valid_win Mask for valid RX window value
+ *  \returns negative error code or RX window on success
+ */
+static int16_t filter_bad_slots(uint8_t mask, uint8_t ul_slots, uint8_t dl_slots, uint16_t rx_valid_win)
+{
+	uint8_t rx_good;
+	uint16_t rx_bad = (uint16_t)(0xff & ~mask) << ul_slots;
+
+	/* TODO: CHECK this calculation -> separate function for unit testing */
+	rx_bad = (rx_bad | (rx_bad >> 8)) & 0xff;
+	rx_good = dl_slots & ~rx_bad;
+	if (!rx_good)
+		return -1;
+
+	return rx_good & rx_valid_win;
+}
+
+static inline uint16_t wrap_window(uint16_t win)
+{
+	return (win | win >> 8) & 0xFF;
+}
+
 /*! Find set of slots available for allocation while taking MS class into account
  *
  *  \param[in] trx Pointer to TRX object
@@ -548,18 +626,14 @@
 	uint8_t Tx, Sum;	/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
 	uint8_t Tta, Ttb, Tra, Trb;	/* Minimum Number of Slots */
 	uint8_t Type; /* Type of Mobile */
-	int rx_window, tx_window, pdch_slots;
+	uint8_t max_slots, num_tx, mask_sel, pdch_slots, ul_ts, dl_ts;
+	int16_t rx_window, tx_window;
 	static const char *digit[10] = { "0","1","2","3","4","5","6","7","8","9" };
 	char slot_info[9] = {0};
 	int max_capacity;
 	uint8_t max_ul_slots;
 	uint8_t max_dl_slots;
-	unsigned max_slots;
-
-	unsigned ul_ts, dl_ts;
-	unsigned num_tx;
 	enum {MASK_TT, MASK_TR};
-	unsigned mask_sel;
 
 	if (ms->ms_class() >= 32) {
 		LOGP(DRLCMAC, LOGL_ERROR, "Multislot class %d out of range.\n",
@@ -656,13 +730,12 @@
 
 	/* Rotate group of TX slots: UUU-----, -UUU----, ..., UU-----U */
 	for (ul_ts = 0; ul_ts < 8; ul_ts += 1, tx_valid_win <<= 1) {
-		unsigned tx_slot_count;
-		int max_rx;
+		uint8_t tx_slot_count;
 		uint16_t rx_valid_win;
 		uint32_t checked_rx[256/32] = {0};
 
 		/* Wrap valid window */
-		tx_valid_win = (tx_valid_win | tx_valid_win >> 8) & 0xff;
+		tx_valid_win = wrap_window(tx_valid_win);
 
 		tx_window = tx_valid_win;
 
@@ -679,117 +752,28 @@
 
 		tx_slot_count = pcu_bitcount(tx_window);
 
-		max_rx = OSMO_MIN(ms_class->rx, ms_class->sum - num_tx);
-		rx_valid_win = (1 << max_rx) - 1;
+		rx_valid_win = (1 << OSMO_MIN(ms_class->rx, ms_class->sum - num_tx)) - 1;
 
 	/* Rotate group of RX slots: DDD-----, -DDD----, ..., DD-----D */
 	for (dl_ts = 0; dl_ts < 8; dl_ts += 1, rx_valid_win <<= 1) {
 		/* Wrap valid window */
-		rx_valid_win = (rx_valid_win | rx_valid_win >> 8) & 0xff;
+		rx_valid_win = wrap_window(rx_valid_win);
 
 	/* Validate with both Tta/Ttb/Trb and Ttb/Tra/Trb */
 	for (mask_sel = MASK_TT; mask_sel <= MASK_TR; mask_sel += 1) {
-		unsigned common_slot_count;
-		unsigned req_common_slots;
-		unsigned rx_slot_count;
-		uint16_t rx_bad;
-		uint8_t rx_good;
+		uint8_t rx_slot_count;
 		int capacity;
 
-		/* Filter out bad slots */
-		rx_bad = (uint16_t)(0xff & ~rx_mask[mask_sel]) << ul_ts;
-		rx_bad = (rx_bad | (rx_bad >> 8)) & 0xff;
-		rx_good = *dl_slots & ~rx_bad;
+		rx_window = filter_bad_slots(rx_mask[mask_sel], ul_ts, *dl_slots, rx_valid_win);
+		if (rx_window < 0)
+			continue;
 
-		/* TODO: CHECK this calculation -> separate function for unit
-		 * testing */
-
-		rx_window = rx_good & rx_valid_win;
 		rx_slot_count = pcu_bitcount(rx_window);
 
-#if 0
-		LOGP(DRLCMAC, LOGL_DEBUG, "n_tx=%d, n_rx=%d, mask_sel=%d, "
-			"tx=%02x, rx=%02x, mask=%02x, bad=%02x, good=%02x, "
-			"ul=%02x, dl=%02x\n",
-			tx_slot_count, rx_slot_count, mask_sel,
-			tx_window, rx_window, rx_mask[mask_sel], rx_bad, rx_good,
-			*ul_slots, *dl_slots);
-#endif
-
-		/* Check compliance with TS 45.002, table 6.4.2.2.1 */
-		/* Whether to skip this round doesn not only depend on the bit
-		 * sets but also on mask_sel. Therefore this check must be done
-		 * before doing the test_and_set_bit shortcut. */
-		if (ms_class->type == 1) {
-			unsigned slot_sum = rx_slot_count + tx_slot_count;
-			/* Assume down+up/dynamic.
-			 * TODO: For ext-dynamic, down only, up only add more
-			 *       cases.
-			 */
-			if (slot_sum <= 6 && tx_slot_count < 3) {
-			       if (mask_sel != MASK_TR)
-				       /* Skip Tta */
-				       continue;
-			} else if (slot_sum > 6 && tx_slot_count < 3) {
-				if (mask_sel != MASK_TT)
-					/* Skip Tra */
-					continue;
-			} else {
-				/* No supported row in table 6.4.2.2.1. */
-#ifdef ENABLE_TS_ALLOC_DEBUG
-				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'));
-#endif
-				continue;
-			}
-		}
-
-		/* Avoid repeated RX combination check */
-		if (test_and_set_bit(checked_rx, rx_window))
-			continue;
-
-		if (!rx_good) {
-#ifdef ENABLE_TS_ALLOC_DEBUG
-			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'));
-#endif
-			continue;
-		}
-
-		if (!rx_window)
-			continue;
-
-		/* Check number of common slots according to TS 54.002, 6.4.2.2 */
-		common_slot_count = pcu_bitcount(tx_window & rx_window);
-		req_common_slots = OSMO_MIN(tx_slot_count, rx_slot_count);
-		if (ms_class->type == 1)
-			req_common_slots = OSMO_MIN(req_common_slots, 2);
-
-		if (req_common_slots != common_slot_count) {
-#ifdef ENABLE_TS_ALLOC_DEBUG
-			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);
-#endif
-			continue;
-		}
+		if (skip_slot(ms_class, mask_sel != MASK_TT,
+			      rx_window, rx_slot_count,
+			      tx_window, tx_slot_count, checked_rx))
+ 			continue;
 
 		/* Compute capacity */
 		capacity = compute_capacity(trx, rx_window, tx_window);

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic39e848da47dc11357782362fdf6206d2c1457c2
Gerrit-PatchSet: 4
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list