<p>neels has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21989">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">hodec2: fix congestion oscillation bug<br><br>When evenly distributing congestion across cells, count the number of<br>occupied lchans surpassing congestion, and not the overall number of<br>free lchans -- which disregards congestion thresholds.<br><br>Fix the bug shown by test_congestion_no_oscillation.ho_vty added in<br>Idf88b4cf3d2f92f5560d73dae9e59af39d0494c0.<br><br>An example to illustrate what this is about:<br><br>Cell A has min-free-slots 2, and has 1 slot remaining free.<br>Cell B has min-free-slots 4, and has 2 slots remaining free.<br><br>If we decide where to place another lchan by counting congested lchans,<br>as implemented in this patch:<br>- Another lchan added, cell A ends up with a congestion count of 2: two<br>  more lchans in use than "allowed".<br>- Cell B ends up with a congestion count of 3, which is worse than 2.<br>We decide that cell A should receive the additional lchan, because it<br>will then have a lower congestion count.  However, that makes cell A<br>completely occupied, while cell B has two lchans remaining free.<br><br>There are two alternative fix variants in consideration:<br>- count the number of free lchans, but only after reaching congestion.<br>- calculate the percentage of load surpassing congestion.<br><br>When using percentage of remaining lchans, we would see that if cell A<br>receives another lchan, it would be 100% loaded above its congestion<br>threshold (2 of 2 remaining lchans in use), but cell B would only be 75%<br>loaded above its treshold (3 of 4 remaining lchans in use).  So a<br>percentage comparison would place the next lchan in cell B, leaving the<br>last lchan of cell A free.<br><br>Another option would be to count the number of remaining free lchans<br>(after the congestion threshold is surpassed), instead of the used ones<br>above the congestion threshold. But then, as soon as all cells are<br>congested, configuring different thresholds would no longer have an<br>effect. I would no longer be able to configure a particular cell to<br>remain more free than others: once congested, only that cell would fill<br>up until it reaches the same load as the other cells.<br><br>In the field, where all cells likely have the same min-free-slots<br>settings, this entire consideration is moot, because congestion counts<br>correspond 1:1 to percentage between all cells and also 1:1 to remaining<br>free slots. However, when looking at distribution across TCH/F and<br>TCH/H, it is quite likely that min-free-slots settings differ for TCH/F<br>and TCH/H, so this is in fact a thing to consider even for identically<br>configured cells.<br><br>Related: SYS#5259<br>Change-Id: Icb373dc6bfc9819446db5e96f71921781fe2026d<br>---<br>M src/osmo-bsc/handover_decision_2.c<br>M tests/handover/test_congestion_no_oscillation.ho_vty<br>M tests/handover/test_congestion_no_oscillation2.ho_vty<br>3 files changed, 11 insertions(+), 30 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/89/21989/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 b296bd1..64ee298 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>@@ -443,6 +443,7 @@</span><br><span> {</span><br><span>    uint8_t requirement = 0;</span><br><span>     unsigned int penalty_time;</span><br><span style="color: hsl(120, 100%, 40%);">+    int current_overbooked;</span><br><span>      c->requirements = 0;</span><br><span> </span><br><span>  /* Requirement A */</span><br><span>@@ -625,14 +626,17 @@</span><br><span> </span><br><span>      /* Requirement C */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* the nr of free timeslots of the target cell must be >= the</span><br><span style="color: hsl(0, 100%, 40%);">-         * free slots of the current cell _after_ handover/assignment */</span><br><span style="color: hsl(120, 100%, 40%);">+      /* the nr of lchans surpassing congestion on the target cell must be <= the lchans surpassing congestion on the</span><br><span style="color: hsl(120, 100%, 40%);">+     * current cell _after_ handover/assignment */</span><br><span style="color: hsl(120, 100%, 40%);">+        current_overbooked = c->current.min_free_tch - c->current.free_tch;</span><br><span>    if (requirement & REQUIREMENT_A_TCHF) {</span><br><span style="color: hsl(0, 100%, 40%);">-             if (c->target.free_tchf - 1 >= c->current.free_tch + 1)</span><br><span style="color: hsl(120, 100%, 40%);">+              int target_overbooked = c->target.min_free_tchf - c->target.free_tchf;</span><br><span style="color: hsl(120, 100%, 40%);">+          if (target_overbooked + 1 <= current_overbooked - 1)</span><br><span>                      requirement |= REQUIREMENT_C_TCHF;</span><br><span>   }</span><br><span>    if (requirement & REQUIREMENT_A_TCHH) {</span><br><span style="color: hsl(0, 100%, 40%);">-             if (c->target.free_tchh - 1 >= c->current.free_tch + 1)</span><br><span style="color: hsl(120, 100%, 40%);">+              int target_overbooked = c->target.min_free_tchh - c->target.free_tchh;</span><br><span style="color: hsl(120, 100%, 40%);">+          if (target_overbooked + 1 <= current_overbooked - 1)</span><br><span>                      requirement |= REQUIREMENT_C_TCHH;</span><br><span>   }</span><br><span> </span><br><span>diff --git a/tests/handover/test_congestion_no_oscillation.ho_vty b/tests/handover/test_congestion_no_oscillation.ho_vty</span><br><span>index abfaef7..a830cbe 100644</span><br><span>--- a/tests/handover/test_congestion_no_oscillation.ho_vty</span><br><span>+++ b/tests/handover/test_congestion_no_oscillation.ho_vty</span><br><span>@@ -1,6 +1,5 @@</span><br><span> # Do not oscillate handover from TCH/F to TCH/H on a neighbor due to congestion,</span><br><span> # and then back to the original cell due to RXLEV.</span><br><span style="color: hsl(0, 100%, 40%);">-# Currently this test script shows the undesired oscillation.</span><br><span> </span><br><span> create-bts trx-count 1 timeslots c+s4 TCH/F TCH/F TCH/F TCH/F  TCH/F  TCH/F PDCH</span><br><span> network</span><br><span>@@ -25,24 +24,5 @@</span><br><span> # measurements continue to be the same</span><br><span> meas-rep lchan 1 0 5 0 rxlev 20 rxqual 0 ta 0 neighbors 40</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-# FAIL: RXLEV oscillation back to bts 0</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ho from lchan 1 0 5 0 to lchan 0 0 2 0</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ts-use trx 0 0 states        *    TCH/F TCH/F -     -      -      -     *</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ts-use trx 1 0 states        *    TCH/F TCH/F TCH/F TCH/F  -      -     *</span><br><span style="color: hsl(0, 100%, 40%);">-meas-rep lchan 0 0 2 0 rxlev 40 rxqual 0 ta 0 neighbors 20</span><br><span style="color: hsl(120, 100%, 40%);">+# despite the better RXLEV, congestion prevents oscillation back to bts 0</span><br><span> expect-no-chan</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-# FAIL: congestion oscillation again to bts 1</span><br><span style="color: hsl(0, 100%, 40%);">-congestion-check</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ho from lchan 0 0 2 0 to lchan 1 0 5 0</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ts-use trx 0 0 states        *    TCH/F -     -     -      -      -     *</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ts-use trx 1 0 states        *    TCH/F TCH/F TCH/F TCH/F  TCH/H- -     *</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-# FAIL: RXLEV oscillation back to bts 0</span><br><span style="color: hsl(0, 100%, 40%);">-meas-rep lchan 1 0 5 0 rxlev 20 rxqual 0 ta 0 neighbors 40</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ho from lchan 1 0 5 0 to lchan 0 0 2 0</span><br><span style="color: hsl(0, 100%, 40%);">-meas-rep lchan 0 0 2 0 rxlev 40 rxqual 0 ta 0 neighbors 20</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-# FAIL: congestion oscillation again to bts 1</span><br><span style="color: hsl(0, 100%, 40%);">-congestion-check</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ho from lchan 0 0 2 0 to lchan 1 0 5 0</span><br><span>diff --git a/tests/handover/test_congestion_no_oscillation2.ho_vty b/tests/handover/test_congestion_no_oscillation2.ho_vty</span><br><span>index aee731d..44c4176 100644</span><br><span>--- a/tests/handover/test_congestion_no_oscillation2.ho_vty</span><br><span>+++ b/tests/handover/test_congestion_no_oscillation2.ho_vty</span><br><span>@@ -1,8 +1,5 @@</span><br><span style="color: hsl(0, 100%, 40%);">-# Almost identical to test_amr_oscillation.ho_vty, this has just two more TCH/H slots in BTS 1, and does not trigger the</span><br><span style="color: hsl(0, 100%, 40%);">-# oscillation bug. The number of free TCH/H in BTS 1 should be unrelated to the congestion status of BTS 0, which</span><br><span style="color: hsl(0, 100%, 40%);">-# illustrates that the even distribution of congestion is fundamentally flawed.</span><br><span style="color: hsl(0, 100%, 40%);">-# This test script shows the desired behavior, though by common sense there should be no reason why we see the bug in</span><br><span style="color: hsl(0, 100%, 40%);">-# test_amr_oscillation.ho_vty and not here.</span><br><span style="color: hsl(120, 100%, 40%);">+# Almost identical to test_amr_oscillation.ho_vty, this has just two more TCH/H slots in BTS 1, and did not trigger the</span><br><span style="color: hsl(120, 100%, 40%);">+# oscillation bug (which has since been fixed, so that both tests behave identically now).</span><br><span> </span><br><span> create-bts trx-count 1 timeslots c+s4 TCH/F TCH/F TCH/F TCH/F  TCH/F  TCH/F PDCH</span><br><span> network</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21989">change 21989</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/+/21989"/><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: Icb373dc6bfc9819446db5e96f71921781fe2026d </div>
<div style="display:none"> Gerrit-Change-Number: 21989 </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-CC: Jenkins Builder </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>