Change in osmo-bsc[master]: replace ts_*_for_each_lchan() with ts_for_n_lchans()

neels gerrit-no-reply at lists.osmocom.org
Tue Jun 1 17:29:49 UTC 2021


neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24372 )

Change subject: replace ts_*_for_each_lchan() with ts_for_n_lchans()
......................................................................

replace ts_*_for_each_lchan() with ts_for_n_lchans()

So far we have a couple of macros iterating a specific number of lchans,
depending on dynamic timeslot state etc. With addition of VAMOS lchans,
this would become more complex and bloated.

Instead of separate iteration macros for each situation, only have one
that takes a number of lchans as argument. That allows to more clearly
pick the number of lchans, especially for non-trivial VAMOS scenarios.

Related: SYS#5315 OS#4940
Change-Id: Ib2c6baf73a81ba371143ba5adc912aef6f79238d
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/bts_trx.c
M src/osmo-bsc/chan_alloc.c
M src/osmo-bsc/handover_decision_2.c
M src/osmo-bsc/handover_logic.c
M src/osmo-bsc/lchan_select.c
M src/osmo-bsc/timeslot_fsm.c
M tests/handover/handover_test.c
10 files changed, 28 insertions(+), 54 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve



diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 7bfe3ab..d128db7 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -554,41 +554,20 @@
 	     bsc_subscr_name(lchan && lchan->conn ? lchan->conn->bsub : NULL), \
 	     ## args)
 
-/* Iterate lchans that have an FSM allocated based based on explicit pchan kind
- * (GSM_PCHAN_* constant).
- * Remark: PDCH related lchans are not handled in BSC but in PCU, so trying to
- * 	  iterate through GSM_PCHAN_PDCH is considered a void loop.
- */
-#define ts_as_pchan_for_each_lchan(lchan, ts, as_pchan) \
-	for (lchan = (ts)->lchan; \
-	     ((lchan - (ts)->lchan) < ARRAY_SIZE((ts)->lchan)) \
-	     && lchan->fi \
-	     && lchan->nr < pchan_subslots(as_pchan); \
-	     lchan++)
-
-/* Iterate lchans that have an FSM allocated based on current PCHAN
- * mode set in \ref ts.
+/* Iterate at most N lchans of the given timeslot.
  * usage:
  * struct gsm_lchan *lchan;
  * struct gsm_bts_trx_ts *ts = get_some_timeslot();
- * ts_for_each_lchan(lchan, ts) {
- * 	LOGPLCHAN(DMAIN, LOGL_DEBUG, "hello world\n");
+ * ts_for_n_lchans(lchan, ts, 3) {
+ *         LOG_LCHAN(lchan, LOGL_DEBUG, "hello world\n");
  * }
  */
-#define ts_for_each_lchan(lchan, ts) ts_as_pchan_for_each_lchan(lchan, ts, (ts)->pchan_is)
-
-/* Iterate over all possible lchans available that have an FSM allocated based
- * on PCHAN \ref ts (dynamic) configuration.
- * Iterate all lchan instances set up by this \ref ts type, including those
- * lchans currently disabled or in process of being enabled (e.g. due to dynamic
- * timeslot in switchover). Compare ts_for_each_lchan(), which iterates only the
- * enabled lchans.
- * For example, it is useful in case dynamic timeslot \ref ts is in process of
- * switching from configuration PDCH (no lchans) to TCH_F (1 lchan), where
- * pchan_is is still set to PDCH but \ref ts may contain already an \ref lchan
- * of type TCH_F which initiated the request to switch the \ts configuration.
- */
-#define ts_for_each_potential_lchan(lchan, ts) ts_as_pchan_for_each_lchan(lchan, ts, (ts)->pchan_on_init)
+#define ts_for_n_lchans(lchan, ts, N) \
+	for (lchan = (ts)->lchan; \
+	     ((lchan - (ts)->lchan) < ARRAY_SIZE((ts)->lchan)) \
+	     && lchan->fi \
+	     && ((lchan - (ts)->lchan) < (N)); \
+	     lchan++)
 
 enum lchan_activate_for {
 	ACTIVATE_FOR_NONE,
diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 71bf0bb..0436bf5 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -1606,7 +1606,7 @@
 		trx = gsm_bts_trx_num(bts, trx_nr);
 		for (ts_nr = 0; ts_nr < TRX_NR_TS; ts_nr++) {
 			ts = &trx->ts[ts_nr];
-			ts_for_each_lchan(lchan, ts) {
+			ts_for_n_lchans(lchan, ts, ts->max_primary_lchans) {
 				if (lchan->type == GSM_LCHAN_TCH_F || lchan->type == GSM_LCHAN_TCH_H) {
 					if (bts->chan_alloc_reverse) {
 						if (lchan->fi->state == LCHAN_ST_ESTABLISHED)
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 6252d06..6f4e2ec 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -1691,7 +1691,7 @@
 			     bool all)
 {
 	struct gsm_lchan *lchan;
-	ts_for_each_lchan(lchan, ts) {
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 		if (lchan_state_is(lchan, LCHAN_ST_UNUSED) && all == false)
 			continue;
 		dump_cb(vty, lchan);
@@ -1997,7 +1997,7 @@
 					if (ts->fi->state != TS_ST_IN_USE)
 						continue;
 
-					ts_for_each_lchan(lchan, ts) {
+					ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 						if (lchan_state_is(lchan, LCHAN_ST_ESTABLISHED)
 						    && (lchan->type == GSM_LCHAN_TCH_F
 							|| lchan->type == GSM_LCHAN_TCH_H)) {
@@ -6160,7 +6160,7 @@
 
 	for (ts_nr = 0; ts_nr < TRX_NR_TS; ts_nr++) {
 		ts = &trx->ts[ts_nr];
-		ts_for_each_potential_lchan(lchan, ts) {
+		ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 			switch (ts->pchan_on_init) {
 			case GSM_PCHAN_SDCCH8_SACCH8C:
 			case GSM_PCHAN_CCCH_SDCCH4_CBCH:
diff --git a/src/osmo-bsc/bts_trx.c b/src/osmo-bsc/bts_trx.c
index 1dfca95..64c7985 100644
--- a/src/osmo-bsc/bts_trx.c
+++ b/src/osmo-bsc/bts_trx.c
@@ -267,7 +267,7 @@
 		if (ts->pchan_is != pchan)
 			continue;
 
-		ts_for_each_lchan(lchan, ts) {
+		ts_for_n_lchans(lchan, ts, ts->max_primary_lchans) {
 			if (lchan_state_is(lchan, LCHAN_ST_UNUSED))
 				count++;
 		}
diff --git a/src/osmo-bsc/chan_alloc.c b/src/osmo-bsc/chan_alloc.c
index 3569d4e..402ca46 100644
--- a/src/osmo-bsc/chan_alloc.c
+++ b/src/osmo-bsc/chan_alloc.c
@@ -73,15 +73,7 @@
 				pl->total++;
 			}
 
-			/* Count allocated logical channels.
-			 * Note: A GSM_PCHAN_TCH_F_TCH_H_PDCH can be switched
-			 * to a single TCH/F or to two TCH/H. So when it's in
-			 * the TCH/H mode, total number of available channels
-			 * is 1 more than when it's in the TCH/F mode.
-			 * I.e. "total" count will fluctuate depending on
-			 * whether GSM_PCHAN_TCH_F_TCH_H_PDCH timeslot is
-			 * in TCH/F or TCH/H (or in NONE/PDCH) mode. */
-			ts_for_each_lchan(lchan, ts) {
+			ts_for_n_lchans(lchan, ts, ts->max_primary_lchans) {
 				/* don't even count CBCH slots in total */
 				if (lchan->type == GSM_LCHAN_CBCH)
 					continue;
diff --git a/src/osmo-bsc/handover_decision_2.c b/src/osmo-bsc/handover_decision_2.c
index 84ddfa4..34bd99a 100644
--- a/src/osmo-bsc/handover_decision_2.c
+++ b/src/osmo-bsc/handover_decision_2.c
@@ -171,7 +171,7 @@
 {
 	struct gsm_lchan *lchan;
 	unsigned int count = 0;
-	ts_for_each_lchan(lchan, ts) {
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 		if (lchan_state_is(lchan, LCHAN_ST_ESTABLISHED))
 			count++;
 	}
diff --git a/src/osmo-bsc/handover_logic.c b/src/osmo-bsc/handover_logic.c
index c0ed10d..1e1b6c3 100644
--- a/src/osmo-bsc/handover_logic.c
+++ b/src/osmo-bsc/handover_logic.c
@@ -112,7 +112,7 @@
 			if (!nm_is_running(&ts->mo.nm_state))
 				continue;
 
-			ts_for_each_lchan(lchan, ts) {
+			ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 				if (!lchan->conn)
 					continue;
 				if (!lchan->conn->ho.fi)
diff --git a/src/osmo-bsc/lchan_select.c b/src/osmo-bsc/lchan_select.c
index b494f02..dba3c3a 100644
--- a/src/osmo-bsc/lchan_select.c
+++ b/src/osmo-bsc/lchan_select.c
@@ -65,6 +65,7 @@
 	}
 
 	for (j = start; j != stop; j += dir) {
+		int lchans_as_pchan;
 		ts = &trx->ts[j];
 		if (!ts_is_usable(ts))
 			continue;
@@ -84,7 +85,8 @@
 		}
 
 		/* TS is (going to be) in desired pchan mode. Go ahead and check for an available lchan. */
-		ts_as_pchan_for_each_lchan(lchan, ts, as_pchan) {
+		lchans_as_pchan = pchan_subslots(as_pchan);
+		ts_for_n_lchans(lchan, ts, lchans_as_pchan) {
 			if (lchan->fi->state == LCHAN_ST_UNUSED) {
 				LOGPLCHANALLOC("%s ss=%d is available%s\n",
 					       gsm_ts_and_pchan_name(ts), lchan->nr,
diff --git a/src/osmo-bsc/timeslot_fsm.c b/src/osmo-bsc/timeslot_fsm.c
index 4fe670f..001319e 100644
--- a/src/osmo-bsc/timeslot_fsm.c
+++ b/src/osmo-bsc/timeslot_fsm.c
@@ -112,7 +112,7 @@
 	struct gsm_lchan *lchan;
 	int count = 0;
 
-	ts_for_each_lchan(lchan, ts) {
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 		if (lchan->fi->state == LCHAN_ST_UNUSED)
 			continue;
 		count++;
@@ -125,7 +125,7 @@
 {
 	struct gsm_lchan *lchan;
 
-	ts_for_each_potential_lchan(lchan, ts) {
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 		if (lchan_state >= 0
 		    && !lchan_state_is(lchan, lchan_state))
 			continue;
@@ -137,7 +137,7 @@
 {
 	struct gsm_lchan *lchan;
 
-	ts_for_each_potential_lchan(lchan, ts) {
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 		osmo_fsm_inst_term(lchan->fi, OSMO_FSM_TERM_REQUEST, NULL);
 	}
 }
@@ -146,7 +146,7 @@
 {
 	struct gsm_lchan *lchan;
 	int count = 0;
-	ts_for_each_potential_lchan(lchan, ts)
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible)
 		if (lchan->fi->state == LCHAN_ST_WAIT_TS_READY)
 			count++;
 	return count;
@@ -565,7 +565,7 @@
 	ts->pdch_act_allowed = true;
 
 	/* For static TS, check validity. For dyn TS, figure out which PCHAN this should become. */
-	ts_for_each_potential_lchan(lchan, ts) {
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 		if (lchan_state_is(lchan, LCHAN_ST_UNUSED))
 			continue;
 
@@ -952,7 +952,7 @@
 bool ts_is_lchan_waiting_for_pchan(struct gsm_bts_trx_ts *ts, enum gsm_phys_chan_config *target_pchan)
 {
 	struct gsm_lchan *lchan;
-	ts_for_each_potential_lchan(lchan, ts) {
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 		if (lchan->fi->state == LCHAN_ST_WAIT_TS_READY) {
 			if (target_pchan)
 				*target_pchan = gsm_pchan_by_lchan_type(lchan->type);
diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c
index 006e791..e653920 100644
--- a/tests/handover/handover_test.c
+++ b/tests/handover/handover_test.c
@@ -463,7 +463,8 @@
 static void ts_clear(struct gsm_bts_trx_ts *ts)
 {
 	struct gsm_lchan *lchan;
-	ts_for_each_lchan(lchan, ts) {
+
+	ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {
 		if (lchan_state_is(lchan, LCHAN_ST_UNUSED))
 			continue;
 		lchan_clear(lchan);

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ib2c6baf73a81ba371143ba5adc912aef6f79238d
Gerrit-Change-Number: 24372
Gerrit-PatchSet: 11
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
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/20210601/711670c0/attachment.htm>


More information about the gerrit-log mailing list