<p>neels <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21203">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">fix TCH/H allocation: use half occupied dyn TS instead of switching more dyn TS<br><br>Change-Id: I5a8d943f31774af00664d037550be14e767d312a<br>---<br>M include/osmocom/bsc/timeslot_fsm.h<br>M src/osmo-bsc/lchan_select.c<br>M src/osmo-bsc/timeslot_fsm.c<br>M tests/handover/handover_test.c<br>4 files changed, 40 insertions(+), 22 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/bsc/timeslot_fsm.h b/include/osmocom/bsc/timeslot_fsm.h</span><br><span>index d02e156..da66136 100644</span><br><span>--- a/include/osmocom/bsc/timeslot_fsm.h</span><br><span>+++ b/include/osmocom/bsc/timeslot_fsm.h</span><br><span>@@ -50,4 +50,4 @@</span><br><span> bool ts_is_capable_of_lchant(struct gsm_bts_trx_ts *ts, enum gsm_chan_t type);</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> bool ts_is_pchan_switching(struct gsm_bts_trx_ts *ts, enum gsm_phys_chan_config *target_pchan);</span><br><span style="color: hsl(0, 100%, 40%);">-bool ts_usable_as_pchan(struct gsm_bts_trx_ts *ts, enum gsm_phys_chan_config as_pchan);</span><br><span style="color: hsl(120, 100%, 40%);">+bool ts_usable_as_pchan(struct gsm_bts_trx_ts *ts, enum gsm_phys_chan_config as_pchan, bool allow_pchan_switch);</span><br><span>diff --git a/src/osmo-bsc/lchan_select.c b/src/osmo-bsc/lchan_select.c</span><br><span>index 6d3caac..64f4939 100644</span><br><span>--- a/src/osmo-bsc/lchan_select.c</span><br><span>+++ b/src/osmo-bsc/lchan_select.c</span><br><span>@@ -32,17 +32,19 @@</span><br><span> </span><br><span> static struct gsm_lchan *</span><br><span> _lc_find_trx(struct gsm_bts_trx *trx, enum gsm_phys_chan_config pchan,</span><br><span style="color: hsl(0, 100%, 40%);">- enum gsm_phys_chan_config as_pchan)</span><br><span style="color: hsl(120, 100%, 40%);">+ enum gsm_phys_chan_config as_pchan, bool allow_pchan_switch)</span><br><span> {</span><br><span> struct gsm_lchan *lchan;</span><br><span> struct gsm_bts_trx_ts *ts;</span><br><span> int j, start, stop, dir;</span><br><span> </span><br><span> #define LOGPLCHANALLOC(fmt, args...) \</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DRLL, LOGL_DEBUG, "looking for lchan %s%s%s: " fmt, \</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DRLL, LOGL_DEBUG, "looking for lchan %s%s%s%s: " fmt, \</span><br><span> gsm_pchan_name(pchan), \</span><br><span> pchan == as_pchan ? "" : " as ", \</span><br><span style="color: hsl(0, 100%, 40%);">- pchan == as_pchan ? "" : gsm_pchan_name(as_pchan), ## args)</span><br><span style="color: hsl(120, 100%, 40%);">+ pchan == as_pchan ? "" : gsm_pchan_name(as_pchan), \</span><br><span style="color: hsl(120, 100%, 40%);">+ ((pchan != as_pchan) && !allow_pchan_switch) ? " without pchan switch" : "", \</span><br><span style="color: hsl(120, 100%, 40%);">+ ## args)</span><br><span> </span><br><span> if (!trx_is_usable(trx)) {</span><br><span> LOGPLCHANALLOC("%s trx not usable\n", gsm_trx_name(trx));</span><br><span>@@ -73,9 +75,10 @@</span><br><span> continue;</span><br><span> }</span><br><span> /* Next, is this timeslot in or can it be switched to the pchan we want to use it for? */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!ts_usable_as_pchan(ts, as_pchan)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPLCHANALLOC("%s is not usable as %s\n", gsm_ts_and_pchan_name(ts),</span><br><span style="color: hsl(0, 100%, 40%);">- gsm_pchan_name(as_pchan));</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ts_usable_as_pchan(ts, as_pchan, allow_pchan_switch)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHANALLOC("%s is not usable as %s%s\n", gsm_ts_and_pchan_name(ts),</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_pchan_name(as_pchan),</span><br><span style="color: hsl(120, 100%, 40%);">+ allow_pchan_switch ? "" : " without pchan switch");</span><br><span> continue;</span><br><span> }</span><br><span> </span><br><span>@@ -104,18 +107,28 @@</span><br><span> {</span><br><span> struct gsm_bts_trx *trx;</span><br><span> struct gsm_lchan *lc;</span><br><span style="color: hsl(120, 100%, 40%);">+ int allow_pchan_switch;</span><br><span style="color: hsl(120, 100%, 40%);">+ bool try_pchan_switch;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (bts->chan_alloc_reverse) {</span><br><span style="color: hsl(0, 100%, 40%);">- llist_for_each_entry_reverse(trx, &bts->trx_list, list) {</span><br><span style="color: hsl(0, 100%, 40%);">- lc = _lc_find_trx(trx, pchan, dyn_as_pchan);</span><br><span style="color: hsl(0, 100%, 40%);">- if (lc)</span><br><span style="color: hsl(0, 100%, 40%);">- return lc;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- llist_for_each_entry(trx, &bts->trx_list, list) {</span><br><span style="color: hsl(0, 100%, 40%);">- lc = _lc_find_trx(trx, pchan, dyn_as_pchan);</span><br><span style="color: hsl(0, 100%, 40%);">- if (lc)</span><br><span style="color: hsl(0, 100%, 40%);">- return lc;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* First find an lchan that needs no change in its timeslot pchan mode.</span><br><span style="color: hsl(120, 100%, 40%);">+ * In particular, this ensures that handover to a dynamic timeslot in TCH/H favors timeslots that are currently</span><br><span style="color: hsl(120, 100%, 40%);">+ * using only one of two TCH/H, so that we don't switch more dynamic timeslots to TCH/H than necessary.</span><br><span style="color: hsl(120, 100%, 40%);">+ * For non-dynamic timeslots, it is not necessary to do a second pass with allow_pchan_switch ==</span><br><span style="color: hsl(120, 100%, 40%);">+ * true, because they never switch anyway. */</span><br><span style="color: hsl(120, 100%, 40%);">+ try_pchan_switch = (pchan != dyn_as_pchan);</span><br><span style="color: hsl(120, 100%, 40%);">+ for (allow_pchan_switch = 0; allow_pchan_switch <= (try_pchan_switch ? 1 : 0); allow_pchan_switch++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (bts->chan_alloc_reverse) {</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_for_each_entry_reverse(trx, &bts->trx_list, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+ lc = _lc_find_trx(trx, pchan, dyn_as_pchan, (bool)allow_pchan_switch);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lc)</span><br><span style="color: hsl(120, 100%, 40%);">+ return lc;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_for_each_entry(trx, &bts->trx_list, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+ lc = _lc_find_trx(trx, pchan, dyn_as_pchan, (bool)allow_pchan_switch);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lc)</span><br><span style="color: hsl(120, 100%, 40%);">+ return lc;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>diff --git a/src/osmo-bsc/timeslot_fsm.c b/src/osmo-bsc/timeslot_fsm.c</span><br><span>index 106e6a1..d8636a1 100644</span><br><span>--- a/src/osmo-bsc/timeslot_fsm.c</span><br><span>+++ b/src/osmo-bsc/timeslot_fsm.c</span><br><span>@@ -1004,7 +1004,7 @@</span><br><span> /* Does the timeslot's *current* state allow use as this PCHAN kind? If the ts is in switchover, return</span><br><span> * true if the switchover's target PCHAN matches, i.e. an lchan for this pchan kind could be requested</span><br><span> * and will be served after the switch. (Do not check whether any lchans are actually available.) */</span><br><span style="color: hsl(0, 100%, 40%);">-bool ts_usable_as_pchan(struct gsm_bts_trx_ts *ts, enum gsm_phys_chan_config as_pchan)</span><br><span style="color: hsl(120, 100%, 40%);">+bool ts_usable_as_pchan(struct gsm_bts_trx_ts *ts, enum gsm_phys_chan_config as_pchan, bool allow_pchan_switch)</span><br><span> {</span><br><span> enum gsm_phys_chan_config target_pchan;</span><br><span> </span><br><span>@@ -1022,5 +1022,11 @@</span><br><span> if (ts_is_lchan_waiting_for_pchan(ts, &target_pchan))</span><br><span> return target_pchan == as_pchan;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- return ts_is_capable_of_pchan(ts, as_pchan);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ts_is_capable_of_pchan(ts, as_pchan))</span><br><span style="color: hsl(120, 100%, 40%);">+ return false;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!allow_pchan_switch && ts->pchan_is != as_pchan)</span><br><span style="color: hsl(120, 100%, 40%);">+ return false;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return true;</span><br><span> }</span><br><span>diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c</span><br><span>index d2d3229..6befc5a 100644</span><br><span>--- a/tests/handover/handover_test.c</span><br><span>+++ b/tests/handover/handover_test.c</span><br><span>@@ -1702,8 +1702,7 @@</span><br><span> "create-bts", "1", "c+s4", "TCH/F", "TCH/F", "TCH/F", "dyn", "dyn", "dyn", "PDCH",</span><br><span> "set-ts-use", "0", "0", "*", "-", "-", "-", "PDCH", "TCH/H-", "PDCH", "PDCH",</span><br><span> "create-ms", "0", "TCH/H", "AMR",</span><br><span style="color: hsl(0, 100%, 40%);">- /* bad: should re-use existing dyn TS instead of switching another one */</span><br><span style="color: hsl(0, 100%, 40%);">- "expect-ts-use", "0", "0", "*", "-", "-", "-", "TCH/H-", "TCH/H-", "PDCH", "PDCH",</span><br><span style="color: hsl(120, 100%, 40%);">+ "expect-ts-use", "0", "0", "*", "-", "-", "-", "PDCH", "TCH/HH", "PDCH", "PDCH",</span><br><span> NULL</span><br><span> };</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21203">change 21203</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/+/21203"/><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: I5a8d943f31774af00664d037550be14e767d312a </div>
<div style="display:none"> Gerrit-Change-Number: 21203 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </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>