<p>neels has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22269">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">hodec2: fix congestion balancing on dyn TS<br><br>When balancing congestion, not only look at TCH/F or TCH/H separately,<br>but also to take into account the effects on the other TCH kind from<br>using/freeing dynamic TS.<br><br>Related: OS#5298<br>Change-Id: I433df6f343650f9056b1bab926bc19ac1d867ad5<br>---<br>M src/osmo-bsc/handover_decision_2.c<br>M tests/handover/test_dyn_ts_balance_congestion.ho_vty<br>2 files changed, 106 insertions(+), 17 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/69/22269/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo-bsc/handover_decision_2.c b/src/osmo-bsc/handover_decision_2.c</span><br><span>index 379970f..c4144a2 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>@@ -106,8 +106,17 @@</span><br><span> struct gsm_lchan *lchan;</span><br><span> struct gsm_bts *bts;</span><br><span> int rxlev;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* free/min-free for the current TCH kind, same as either free_tch_f or free_tch_h below */</span><br><span> int free_tch;</span><br><span> int min_free_tch;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* free/min-free for the two TCH kinds, to calculate F<->H cross effects for dynamic timeslots */</span><br><span style="color: hsl(120, 100%, 40%);">+ int free_tchf;</span><br><span style="color: hsl(120, 100%, 40%);">+ int min_free_tchf;</span><br><span style="color: hsl(120, 100%, 40%);">+ int free_tchh;</span><br><span style="color: hsl(120, 100%, 40%);">+ int min_free_tchh;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Effects of freeing a dynamic timeslot: */</span><br><span style="color: hsl(120, 100%, 40%);">+ int lchan_frees_tchf;</span><br><span style="color: hsl(120, 100%, 40%);">+ int lchan_frees_tchh;</span><br><span> } current;</span><br><span> struct {</span><br><span> struct neighbor_ident_key nik; /* neighbor ARFCN+BSIC */</span><br><span>@@ -155,6 +164,17 @@</span><br><span> </span><br><span> static void congestion_check_cb(void *arg);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static unsigned int ts_usage_count(struct gsm_bts_trx_ts *ts)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_lchan *lchan;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int count = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ ts_for_each_lchan(lchan, ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan_state_is(lchan, LCHAN_ST_ESTABLISHED))</span><br><span style="color: hsl(120, 100%, 40%);">+ count++;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ return count;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* This function gets called on ho2 init, whenever the congestion check interval is changed, and also</span><br><span> * when the timer has fired to trigger again after the next congestion check timeout. */</span><br><span> static void reinit_congestion_timer(struct gsm_network *net)</span><br><span>@@ -659,21 +679,75 @@</span><br><span> * congestion on the current cell, hence the - 1 on the target. */</span><br><span> current_overbooked = load_above_congestion(c->current.free_tch, c->current.min_free_tch);</span><br><span> if (requirement & REQUIREMENT_A_TCHF) {</span><br><span style="color: hsl(120, 100%, 40%);">+ bool ok;</span><br><span> int32_t target_overbooked = load_above_congestion(c->target.free_tchf - 1, c->target.min_free_tchf);</span><br><span> LOGPHOLCHANTOBTS(c->current.lchan, c->target.bts, LOGL_DEBUG,</span><br><span> "current overbooked = %s%%, TCH/F target overbooked after HO = %s%%\n",</span><br><span> osmo_int_to_float_str_c(OTC_SELECT, current_overbooked, LOAD_PRECISION - 2),</span><br><span> osmo_int_to_float_str_c(OTC_SELECT, target_overbooked, LOAD_PRECISION - 2));</span><br><span style="color: hsl(0, 100%, 40%);">- if (target_overbooked < current_overbooked)</span><br><span style="color: hsl(120, 100%, 40%);">+ ok = target_overbooked < current_overbooked;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Look at dynamic timeslot effects on TCH/H: */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ok && c->target.next_tchf_reduces_tchh) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Looking at the current TCH type and the target cell's TCH/F alone, congestion balancing</span><br><span style="color: hsl(120, 100%, 40%);">+ * should happen. However, what if the target TCH/F is a dynamic timeslot -- would that cause</span><br><span style="color: hsl(120, 100%, 40%);">+ * congestion on TCH/H above the current cell's TCH/H congestion? */</span><br><span style="color: hsl(120, 100%, 40%);">+ int32_t current_tchh_overbooked = load_above_congestion(c->current.free_tchh,</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.min_free_tchh);</span><br><span style="color: hsl(120, 100%, 40%);">+ int32_t target_tchh_overbooked;</span><br><span style="color: hsl(120, 100%, 40%);">+ int target_free_tchh_after_ho = c->target.free_tchh - c->target.next_tchf_reduces_tchh;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If this is a re-assignment within the same cell, and if the current candidate would free a</span><br><span style="color: hsl(120, 100%, 40%);">+ * dynamic timeslot, then the target-overbooking after HO is reduced again by the freed dynamic</span><br><span style="color: hsl(120, 100%, 40%);">+ * TS. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (c->current.bts == c->target.bts)</span><br><span style="color: hsl(120, 100%, 40%);">+ target_free_tchh_after_ho += c->current.lchan_frees_tchh;</span><br><span style="color: hsl(120, 100%, 40%);">+ target_tchh_overbooked = load_above_congestion(target_free_tchh_after_ho,</span><br><span style="color: hsl(120, 100%, 40%);">+ c->target.min_free_tchh);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPHOLCHANTOBTS(c->current.lchan, c->target.bts, LOGL_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "dyn TS: current TCH/H overbooked = %s%%, TCH/H target overbooked after HO = %s%%\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_int_to_float_str_c(OTC_SELECT, current_tchh_overbooked, LOAD_PRECISION - 2),</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_int_to_float_str_c(OTC_SELECT, target_tchh_overbooked, LOAD_PRECISION - 2));</span><br><span style="color: hsl(120, 100%, 40%);">+ /* For the current TCH kind, a handover should only happen if things actually get better</span><br><span style="color: hsl(120, 100%, 40%);">+ * (condition is '<'). For dynamic timeslot cross effects TCH/F->TCH/H, it is fine to not make</span><br><span style="color: hsl(120, 100%, 40%);">+ * it worse. Hence the smaller-or-equal condition. */</span><br><span style="color: hsl(120, 100%, 40%);">+ ok = target_tchh_overbooked <= current_tchh_overbooked;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ok)</span><br><span> requirement |= REQUIREMENT_C_TCHF;</span><br><span> }</span><br><span> if (requirement & REQUIREMENT_A_TCHH) {</span><br><span style="color: hsl(120, 100%, 40%);">+ bool ok;</span><br><span> int32_t target_overbooked = load_above_congestion(c->target.free_tchh - 1, c->target.min_free_tchh);</span><br><span> LOGPHOLCHANTOBTS(c->current.lchan, c->target.bts, LOGL_DEBUG,</span><br><span> "current overbooked = %s%%, TCH/H target overbooked after HO = %s%%\n",</span><br><span> osmo_int_to_float_str_c(OTC_SELECT, current_overbooked, LOAD_PRECISION - 2),</span><br><span> osmo_int_to_float_str_c(OTC_SELECT, target_overbooked, LOAD_PRECISION - 2));</span><br><span style="color: hsl(0, 100%, 40%);">- if (target_overbooked < current_overbooked)</span><br><span style="color: hsl(120, 100%, 40%);">+ ok = target_overbooked < current_overbooked;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Look at dynamic timeslot effects on TCH/F: */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ok && c->target.next_tchh_reduces_tchf) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Looking at the current TCH type and the target cell's TCH/H alone, congestion balancing</span><br><span style="color: hsl(120, 100%, 40%);">+ * should happen. However, what if the target TCH/H is a dynamic timeslot -- would that cause</span><br><span style="color: hsl(120, 100%, 40%);">+ * congestion on TCH/F above the current cell's TCH/F congestion? */</span><br><span style="color: hsl(120, 100%, 40%);">+ int32_t current_tchf_overbooked = load_above_congestion(c->current.free_tchf,</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.min_free_tchf);</span><br><span style="color: hsl(120, 100%, 40%);">+ int32_t target_tchf_overbooked;</span><br><span style="color: hsl(120, 100%, 40%);">+ int target_free_tchf_after_ho = c->target.free_tchf - c->target.next_tchh_reduces_tchf;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If this is a re-assignment within the same cell, and if the current candidate would free a</span><br><span style="color: hsl(120, 100%, 40%);">+ * dynamic timeslot, then the target-overbooking after HO is reduced again by the freed dynamic</span><br><span style="color: hsl(120, 100%, 40%);">+ * TS. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (c->current.bts == c->target.bts)</span><br><span style="color: hsl(120, 100%, 40%);">+ target_free_tchf_after_ho += c->current.lchan_frees_tchf;</span><br><span style="color: hsl(120, 100%, 40%);">+ target_tchf_overbooked = load_above_congestion(target_free_tchf_after_ho,</span><br><span style="color: hsl(120, 100%, 40%);">+ c->target.min_free_tchf);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPHOLCHANTOBTS(c->current.lchan, c->target.bts, LOGL_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "dyn TS: current TCH/F overbooked = %s%%, TCH/F target overbooked after HO = %s%%\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_int_to_float_str_c(OTC_SELECT, current_tchf_overbooked, LOAD_PRECISION - 2),</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_int_to_float_str_c(OTC_SELECT, target_tchf_overbooked, LOAD_PRECISION - 2));</span><br><span style="color: hsl(120, 100%, 40%);">+ /* For the current TCH kind, a handover should only happen if things actually get better</span><br><span style="color: hsl(120, 100%, 40%);">+ * (condition is '<'). For dynamic timeslot cross effects TCH/H->TCH/F, it is fine to not make</span><br><span style="color: hsl(120, 100%, 40%);">+ * it worse. Hence the smaller-or-equal condition. */</span><br><span style="color: hsl(120, 100%, 40%);">+ ok = target_tchf_overbooked <= current_tchf_overbooked;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ok)</span><br><span> requirement |= REQUIREMENT_C_TCHH;</span><br><span> }</span><br><span> </span><br><span>@@ -904,17 +978,37 @@</span><br><span> {</span><br><span> struct gsm_lchan *next_lchan;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- c->current.free_tch = bts_count_free_ts(c->current.bts, c->current.lchan->ts->pchan_is);</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.free_tchf = bts_count_free_ts(c->current.bts, GSM_PCHAN_TCH_F);</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.min_free_tchf = ho_get_hodec2_tchf_min_slots(c->current.bts->ho);</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.free_tchh = bts_count_free_ts(c->current.bts, GSM_PCHAN_TCH_H);</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.min_free_tchh = ho_get_hodec2_tchh_min_slots(c->current.bts->ho);</span><br><span> switch (c->current.lchan->ts->pchan_is) {</span><br><span> case GSM_PCHAN_TCH_F:</span><br><span style="color: hsl(0, 100%, 40%);">- c->current.min_free_tch = ho_get_hodec2_tchf_min_slots(c->current.bts->ho);</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.free_tch = c->current.free_tchf;</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.min_free_tch = c->current.min_free_tchf;</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.lchan_frees_tchf = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (c->current.lchan->ts->pchan_on_init == GSM_PCHAN_TCH_F_TCH_H_PDCH)</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.lchan_frees_tchh = 2;</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.lchan_frees_tchh = 0;</span><br><span> break;</span><br><span> case GSM_PCHAN_TCH_H:</span><br><span style="color: hsl(0, 100%, 40%);">- c->current.min_free_tch = ho_get_hodec2_tchh_min_slots(c->current.bts->ho);</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.free_tch = c->current.free_tchh;</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.min_free_tch = c->current.min_free_tchh;</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.lchan_frees_tchh = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Freeing one of two TCH/H does not free a dyn TS and would not free a TCH/F. It has to be the last</span><br><span style="color: hsl(120, 100%, 40%);">+ * TCH/H of a dynamic timeslot that is freed to get a new TCH/F in the current cell from the handover.</span><br><span style="color: hsl(120, 100%, 40%);">+ * Hence the ts_usage_count() condition. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (c->current.lchan->ts->pchan_on_init == GSM_PCHAN_TCH_F_TCH_H_PDCH</span><br><span style="color: hsl(120, 100%, 40%);">+ && ts_usage_count(c->current.lchan->ts) == 1)</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.lchan_frees_tchf = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+ c->current.lchan_frees_tchf = 0;</span><br><span> break;</span><br><span> default:</span><br><span> break;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> c->target.free_tchf = bts_count_free_ts(c->target.bts, GSM_PCHAN_TCH_F);</span><br><span> c->target.min_free_tchf = ho_get_hodec2_tchf_min_slots(c->target.bts->ho);</span><br><span> c->target.free_tchh = bts_count_free_ts(c->target.bts, GSM_PCHAN_TCH_H);</span><br><span>@@ -1454,17 +1548,6 @@</span><br><span> || lchan->ts->pchan_on_init == GSM_PCHAN_TCH_F_PDCH;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static unsigned int ts_usage_count(struct gsm_bts_trx_ts *ts)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm_lchan *lchan;</span><br><span style="color: hsl(0, 100%, 40%);">- 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(0, 100%, 40%);">- if (lchan_state_is(lchan, LCHAN_ST_ESTABLISHED))</span><br><span style="color: hsl(0, 100%, 40%);">- count++;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- return count;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static bool is_upgrade_to_tchf(const struct ho_candidate *c, uint8_t for_requirement)</span><br><span> {</span><br><span> return c->current.lchan</span><br><span>diff --git a/tests/handover/test_dyn_ts_balance_congestion.ho_vty b/tests/handover/test_dyn_ts_balance_congestion.ho_vty</span><br><span>index ad9d6a5..2fa11b6 100644</span><br><span>--- a/tests/handover/test_dyn_ts_balance_congestion.ho_vty</span><br><span>+++ b/tests/handover/test_dyn_ts_balance_congestion.ho_vty</span><br><span>@@ -27,5 +27,11 @@</span><br><span> # not congested. No handover is performed because 50% in the target is more congestion for TCH/H than 0% in the source</span><br><span> # cell.</span><br><span> congestion-check</span><br><span style="color: hsl(0, 100%, 40%);">-# FAIL: should not increase TCH/H congestion by occupying a dyn TS</span><br><span style="color: hsl(120, 100%, 40%);">+expect-no-chan</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+# If there is no constraint on TCH/H in the target cell, the handover does take place.</span><br><span style="color: hsl(120, 100%, 40%);">+network</span><br><span style="color: hsl(120, 100%, 40%);">+ bts 1</span><br><span style="color: hsl(120, 100%, 40%);">+ handover2 min-free-slots tch/h 2</span><br><span style="color: hsl(120, 100%, 40%);">+congestion-check</span><br><span> expect-ho from lchan 0 0 1 0 to lchan 1 0 5 0</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22269">change 22269</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/+/22269"/><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: I433df6f343650f9056b1bab926bc19ac1d867ad5 </div>
<div style="display:none"> Gerrit-Change-Number: 22269 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>