<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21989">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;">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 bugs shown by<br> test_congestion_no_oscillation.ho_vty<br> test_balance_congestion_tchf_tchh.ho_vty<br><br>This implements a simple calculation for congestion load by counting<br>lchans in use above congestion. An improvement of this calculation<br>using percent follows in I55234c6c99eb02ceee52be0d7388bea14304930f.<br><br>Related: SYS#5259<br>Change-Id: Icb373dc6bfc9819446db5e96f71921781fe2026d<br>---<br>M src/osmo-bsc/handover_decision_2.c<br>M tests/handover/test_balance_congestion_tchf_tchh.ho_vty<br>M tests/handover/test_congestion_no_oscillation.ho_vty<br>M tests/handover/test_congestion_no_oscillation2.ho_vty<br>4 files changed, 14 insertions(+), 36 deletions(-)<br><br></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..c265f5f 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>@@ -1743,8 +1747,7 @@</span><br><span> goto exit;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- LOGPHOBTS(bts, LOGL_DEBUG, "Did not find a best candidate that fulfills requirement C"</span><br><span style="color: hsl(0, 100%, 40%);">- " (omitting change from AHS to AFS)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPHOBTS(bts, LOGL_DEBUG, "Did not find a best candidate that fulfills requirement C\n");</span><br><span> </span><br><span> exit:</span><br><span> /* free array */</span><br><span>diff --git a/tests/handover/test_balance_congestion_tchf_tchh.ho_vty b/tests/handover/test_balance_congestion_tchf_tchh.ho_vty</span><br><span>index d7b3cf5..7f9039f 100644</span><br><span>--- a/tests/handover/test_balance_congestion_tchf_tchh.ho_vty</span><br><span>+++ b/tests/handover/test_balance_congestion_tchf_tchh.ho_vty</span><br><span>@@ -35,8 +35,7 @@</span><br><span> set-ts-use trx 0 0 states * TCH/F TCH/F - - TCH/HH - -</span><br><span> meas-rep lchan * * * * rxlev 20 rxqual 0 ta 0</span><br><span> congestion-check</span><br><span style="color: hsl(0, 100%, 40%);">-# FAIL: should be the same as above, but seeing a handover from F to H</span><br><span style="color: hsl(0, 100%, 40%);">-expect-ho from lchan 0 0 1 0 to lchan 0 0 6 0</span><br><span style="color: hsl(120, 100%, 40%);">+expect-no-chan</span><br><span> </span><br><span> # TCH/F = +1, TCH/H = +2 above congestion. Moving a TCH/H to TCH/F would just</span><br><span> # reverse the situation to F=+2 H=+1. Nothing happens.</span><br><span>@@ -49,5 +48,4 @@</span><br><span> set-ts-use trx 0 0 states * TCH/F TCH/F - - TCH/HH TCH/HH -</span><br><span> meas-rep lchan * * * * rxlev 20 rxqual 0 ta 0</span><br><span> congestion-check</span><br><span style="color: hsl(0, 100%, 40%);">-# FAIL: should be the same as above, from lchan 0 0 5 0 to lchan 0 0 3 0, but no handover happens</span><br><span style="color: hsl(0, 100%, 40%);">-expect-no-chan</span><br><span style="color: hsl(120, 100%, 40%);">+expect-ho from lchan 0 0 5 0 to lchan 0 0 3 0</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: 7 </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: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>