<p>neels <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/24372">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">replace ts_*_for_each_lchan() with ts_for_n_lchans()<br><br>So far we have a couple of macros iterating a specific number of lchans,<br>depending on dynamic timeslot state etc. With addition of VAMOS lchans,<br>this would become more complex and bloated.<br><br>Instead of separate iteration macros for each situation, only have one<br>that takes a number of lchans as argument. That allows to more clearly<br>pick the number of lchans, especially for non-trivial VAMOS scenarios.<br><br>Related: SYS#5315 OS#4940<br>Change-Id: Ib2c6baf73a81ba371143ba5adc912aef6f79238d<br>---<br>M include/osmocom/bsc/gsm_data.h<br>M src/osmo-bsc/abis_rsl.c<br>M src/osmo-bsc/bsc_vty.c<br>M src/osmo-bsc/bts_trx.c<br>M src/osmo-bsc/chan_alloc.c<br>M src/osmo-bsc/handover_decision_2.c<br>M src/osmo-bsc/handover_logic.c<br>M src/osmo-bsc/lchan_select.c<br>M src/osmo-bsc/timeslot_fsm.c<br>M tests/handover/handover_test.c<br>10 files changed, 28 insertions(+), 54 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h</span><br><span>index 7bfe3ab..d128db7 100644</span><br><span>--- a/include/osmocom/bsc/gsm_data.h</span><br><span>+++ b/include/osmocom/bsc/gsm_data.h</span><br><span>@@ -554,41 +554,20 @@</span><br><span> bsc_subscr_name(lchan && lchan->conn ? lchan->conn->bsub : NULL), \</span><br><span> ## args)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Iterate lchans that have an FSM allocated based based on explicit pchan kind</span><br><span style="color: hsl(0, 100%, 40%);">- * (GSM_PCHAN_* constant).</span><br><span style="color: hsl(0, 100%, 40%);">- * Remark: PDCH related lchans are not handled in BSC but in PCU, so trying to</span><br><span style="color: hsl(0, 100%, 40%);">- * iterate through GSM_PCHAN_PDCH is considered a void loop.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">-#define ts_as_pchan_for_each_lchan(lchan, ts, as_pchan) \</span><br><span style="color: hsl(0, 100%, 40%);">- for (lchan = (ts)->lchan; \</span><br><span style="color: hsl(0, 100%, 40%);">- ((lchan - (ts)->lchan) < ARRAY_SIZE((ts)->lchan)) \</span><br><span style="color: hsl(0, 100%, 40%);">- && lchan->fi \</span><br><span style="color: hsl(0, 100%, 40%);">- && lchan->nr < pchan_subslots(as_pchan); \</span><br><span style="color: hsl(0, 100%, 40%);">- lchan++)</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-/* Iterate lchans that have an FSM allocated based on current PCHAN</span><br><span style="color: hsl(0, 100%, 40%);">- * mode set in \ref ts.</span><br><span style="color: hsl(120, 100%, 40%);">+/* Iterate at most N lchans of the given timeslot.</span><br><span> * usage:</span><br><span> * struct gsm_lchan *lchan;</span><br><span> * struct gsm_bts_trx_ts *ts = get_some_timeslot();</span><br><span style="color: hsl(0, 100%, 40%);">- * ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(0, 100%, 40%);">- * LOGPLCHAN(DMAIN, LOGL_DEBUG, "hello world\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ * ts_for_n_lchans(lchan, ts, 3) {</span><br><span style="color: hsl(120, 100%, 40%);">+ * LOG_LCHAN(lchan, LOGL_DEBUG, "hello world\n");</span><br><span> * }</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">-#define ts_for_each_lchan(lchan, ts) ts_as_pchan_for_each_lchan(lchan, ts, (ts)->pchan_is)</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-/* Iterate over all possible lchans available that have an FSM allocated based</span><br><span style="color: hsl(0, 100%, 40%);">- * on PCHAN \ref ts (dynamic) configuration.</span><br><span style="color: hsl(0, 100%, 40%);">- * Iterate all lchan instances set up by this \ref ts type, including those</span><br><span style="color: hsl(0, 100%, 40%);">- * lchans currently disabled or in process of being enabled (e.g. due to dynamic</span><br><span style="color: hsl(0, 100%, 40%);">- * timeslot in switchover). Compare ts_for_each_lchan(), which iterates only the</span><br><span style="color: hsl(0, 100%, 40%);">- * enabled lchans.</span><br><span style="color: hsl(0, 100%, 40%);">- * For example, it is useful in case dynamic timeslot \ref ts is in process of</span><br><span style="color: hsl(0, 100%, 40%);">- * switching from configuration PDCH (no lchans) to TCH_F (1 lchan), where</span><br><span style="color: hsl(0, 100%, 40%);">- * pchan_is is still set to PDCH but \ref ts may contain already an \ref lchan</span><br><span style="color: hsl(0, 100%, 40%);">- * of type TCH_F which initiated the request to switch the \ts configuration.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">-#define ts_for_each_potential_lchan(lchan, ts) ts_as_pchan_for_each_lchan(lchan, ts, (ts)->pchan_on_init)</span><br><span style="color: hsl(120, 100%, 40%);">+#define ts_for_n_lchans(lchan, ts, N) \</span><br><span style="color: hsl(120, 100%, 40%);">+ for (lchan = (ts)->lchan; \</span><br><span style="color: hsl(120, 100%, 40%);">+ ((lchan - (ts)->lchan) < ARRAY_SIZE((ts)->lchan)) \</span><br><span style="color: hsl(120, 100%, 40%);">+ && lchan->fi \</span><br><span style="color: hsl(120, 100%, 40%);">+ && ((lchan - (ts)->lchan) < (N)); \</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan++)</span><br><span> </span><br><span> enum lchan_activate_for {</span><br><span> ACTIVATE_FOR_NONE,</span><br><span>diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c</span><br><span>index 71bf0bb..0436bf5 100644</span><br><span>--- a/src/osmo-bsc/abis_rsl.c</span><br><span>+++ b/src/osmo-bsc/abis_rsl.c</span><br><span>@@ -1606,7 +1606,7 @@</span><br><span> trx = gsm_bts_trx_num(bts, trx_nr);</span><br><span> for (ts_nr = 0; ts_nr < TRX_NR_TS; ts_nr++) {</span><br><span> ts = &trx->ts[ts_nr];</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_primary_lchans) {</span><br><span> if (lchan->type == GSM_LCHAN_TCH_F || lchan->type == GSM_LCHAN_TCH_H) {</span><br><span> if (bts->chan_alloc_reverse) {</span><br><span> if (lchan->fi->state == LCHAN_ST_ESTABLISHED)</span><br><span>diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c</span><br><span>index 6252d06..6f4e2ec 100644</span><br><span>--- a/src/osmo-bsc/bsc_vty.c</span><br><span>+++ b/src/osmo-bsc/bsc_vty.c</span><br><span>@@ -1691,7 +1691,7 @@</span><br><span> bool all)</span><br><span> {</span><br><span> struct gsm_lchan *lchan;</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (lchan_state_is(lchan, LCHAN_ST_UNUSED) && all == false)</span><br><span> continue;</span><br><span> dump_cb(vty, lchan);</span><br><span>@@ -1997,7 +1997,7 @@</span><br><span> if (ts->fi->state != TS_ST_IN_USE)</span><br><span> continue;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (lchan_state_is(lchan, LCHAN_ST_ESTABLISHED)</span><br><span> && (lchan->type == GSM_LCHAN_TCH_F</span><br><span> || lchan->type == GSM_LCHAN_TCH_H)) {</span><br><span>@@ -6160,7 +6160,7 @@</span><br><span> </span><br><span> for (ts_nr = 0; ts_nr < TRX_NR_TS; ts_nr++) {</span><br><span> ts = &trx->ts[ts_nr];</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_potential_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> switch (ts->pchan_on_init) {</span><br><span> case GSM_PCHAN_SDCCH8_SACCH8C:</span><br><span> case GSM_PCHAN_CCCH_SDCCH4_CBCH:</span><br><span>diff --git a/src/osmo-bsc/bts_trx.c b/src/osmo-bsc/bts_trx.c</span><br><span>index 1dfca95..64c7985 100644</span><br><span>--- a/src/osmo-bsc/bts_trx.c</span><br><span>+++ b/src/osmo-bsc/bts_trx.c</span><br><span>@@ -267,7 +267,7 @@</span><br><span> if (ts->pchan_is != pchan)</span><br><span> continue;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_primary_lchans) {</span><br><span> if (lchan_state_is(lchan, LCHAN_ST_UNUSED))</span><br><span> count++;</span><br><span> }</span><br><span>diff --git a/src/osmo-bsc/chan_alloc.c b/src/osmo-bsc/chan_alloc.c</span><br><span>index 3569d4e..402ca46 100644</span><br><span>--- a/src/osmo-bsc/chan_alloc.c</span><br><span>+++ b/src/osmo-bsc/chan_alloc.c</span><br><span>@@ -73,15 +73,7 @@</span><br><span> pl->total++;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Count allocated logical channels.</span><br><span style="color: hsl(0, 100%, 40%);">- * Note: A GSM_PCHAN_TCH_F_TCH_H_PDCH can be switched</span><br><span style="color: hsl(0, 100%, 40%);">- * to a single TCH/F or to two TCH/H. So when it's in</span><br><span style="color: hsl(0, 100%, 40%);">- * the TCH/H mode, total number of available channels</span><br><span style="color: hsl(0, 100%, 40%);">- * is 1 more than when it's in the TCH/F mode.</span><br><span style="color: hsl(0, 100%, 40%);">- * I.e. "total" count will fluctuate depending on</span><br><span style="color: hsl(0, 100%, 40%);">- * whether GSM_PCHAN_TCH_F_TCH_H_PDCH timeslot is</span><br><span style="color: hsl(0, 100%, 40%);">- * in TCH/F or TCH/H (or in NONE/PDCH) mode. */</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_primary_lchans) {</span><br><span> /* don't even count CBCH slots in total */</span><br><span> if (lchan->type == GSM_LCHAN_CBCH)</span><br><span> continue;</span><br><span>diff --git a/src/osmo-bsc/handover_decision_2.c b/src/osmo-bsc/handover_decision_2.c</span><br><span>index 84ddfa4..34bd99a 100644</span><br><span>--- a/src/osmo-bsc/handover_decision_2.c</span><br><span>+++ b/src/osmo-bsc/handover_decision_2.c</span><br><span>@@ -171,7 +171,7 @@</span><br><span> {</span><br><span> struct gsm_lchan *lchan;</span><br><span> unsigned int count = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (lchan_state_is(lchan, LCHAN_ST_ESTABLISHED))</span><br><span> count++;</span><br><span> }</span><br><span>diff --git a/src/osmo-bsc/handover_logic.c b/src/osmo-bsc/handover_logic.c</span><br><span>index c0ed10d..1e1b6c3 100644</span><br><span>--- a/src/osmo-bsc/handover_logic.c</span><br><span>+++ b/src/osmo-bsc/handover_logic.c</span><br><span>@@ -112,7 +112,7 @@</span><br><span> if (!nm_is_running(&ts->mo.nm_state))</span><br><span> continue;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (!lchan->conn)</span><br><span> continue;</span><br><span> if (!lchan->conn->ho.fi)</span><br><span>diff --git a/src/osmo-bsc/lchan_select.c b/src/osmo-bsc/lchan_select.c</span><br><span>index b494f02..dba3c3a 100644</span><br><span>--- a/src/osmo-bsc/lchan_select.c</span><br><span>+++ b/src/osmo-bsc/lchan_select.c</span><br><span>@@ -65,6 +65,7 @@</span><br><span> }</span><br><span> </span><br><span> for (j = start; j != stop; j += dir) {</span><br><span style="color: hsl(120, 100%, 40%);">+ int lchans_as_pchan;</span><br><span> ts = &trx->ts[j];</span><br><span> if (!ts_is_usable(ts))</span><br><span> continue;</span><br><span>@@ -84,7 +85,8 @@</span><br><span> }</span><br><span> </span><br><span> /* TS is (going to be) in desired pchan mode. Go ahead and check for an available lchan. */</span><br><span style="color: hsl(0, 100%, 40%);">- ts_as_pchan_for_each_lchan(lchan, ts, as_pchan) {</span><br><span style="color: hsl(120, 100%, 40%);">+ lchans_as_pchan = pchan_subslots(as_pchan);</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, lchans_as_pchan) {</span><br><span> if (lchan->fi->state == LCHAN_ST_UNUSED) {</span><br><span> LOGPLCHANALLOC("%s ss=%d is available%s\n",</span><br><span> gsm_ts_and_pchan_name(ts), lchan->nr,</span><br><span>diff --git a/src/osmo-bsc/timeslot_fsm.c b/src/osmo-bsc/timeslot_fsm.c</span><br><span>index 4fe670f..001319e 100644</span><br><span>--- a/src/osmo-bsc/timeslot_fsm.c</span><br><span>+++ b/src/osmo-bsc/timeslot_fsm.c</span><br><span>@@ -112,7 +112,7 @@</span><br><span> struct gsm_lchan *lchan;</span><br><span> int count = 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (lchan->fi->state == LCHAN_ST_UNUSED)</span><br><span> continue;</span><br><span> count++;</span><br><span>@@ -125,7 +125,7 @@</span><br><span> {</span><br><span> struct gsm_lchan *lchan;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_potential_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (lchan_state >= 0</span><br><span> && !lchan_state_is(lchan, lchan_state))</span><br><span> continue;</span><br><span>@@ -137,7 +137,7 @@</span><br><span> {</span><br><span> struct gsm_lchan *lchan;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_potential_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> osmo_fsm_inst_term(lchan->fi, OSMO_FSM_TERM_REQUEST, NULL);</span><br><span> }</span><br><span> }</span><br><span>@@ -146,7 +146,7 @@</span><br><span> {</span><br><span> struct gsm_lchan *lchan;</span><br><span> int count = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_potential_lchan(lchan, ts)</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible)</span><br><span> if (lchan->fi->state == LCHAN_ST_WAIT_TS_READY)</span><br><span> count++;</span><br><span> return count;</span><br><span>@@ -565,7 +565,7 @@</span><br><span> ts->pdch_act_allowed = true;</span><br><span> </span><br><span> /* For static TS, check validity. For dyn TS, figure out which PCHAN this should become. */</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_potential_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (lchan_state_is(lchan, LCHAN_ST_UNUSED))</span><br><span> continue;</span><br><span> </span><br><span>@@ -952,7 +952,7 @@</span><br><span> bool ts_is_lchan_waiting_for_pchan(struct gsm_bts_trx_ts *ts, enum gsm_phys_chan_config *target_pchan)</span><br><span> {</span><br><span> struct gsm_lchan *lchan;</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_potential_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (lchan->fi->state == LCHAN_ST_WAIT_TS_READY) {</span><br><span> if (target_pchan)</span><br><span> *target_pchan = gsm_pchan_by_lchan_type(lchan->type);</span><br><span>diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c</span><br><span>index 006e791..e653920 100644</span><br><span>--- a/tests/handover/handover_test.c</span><br><span>+++ b/tests/handover/handover_test.c</span><br><span>@@ -463,7 +463,8 @@</span><br><span> static void ts_clear(struct gsm_bts_trx_ts *ts)</span><br><span> {</span><br><span> struct gsm_lchan *lchan;</span><br><span style="color: hsl(0, 100%, 40%);">- ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_n_lchans(lchan, ts, ts->max_lchans_possible) {</span><br><span> if (lchan_state_is(lchan, LCHAN_ST_UNUSED))</span><br><span> continue;</span><br><span> lchan_clear(lchan);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/24372">change 24372</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bsc/+/24372"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ib2c6baf73a81ba371143ba5adc912aef6f79238d </div>
<div style="display:none"> Gerrit-Change-Number: 24372 </div>
<div style="display:none"> Gerrit-PatchSet: 11 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>