laforge has submitted this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/21989 )
Change subject: hodec2: fix congestion oscillation bug
......................................................................
hodec2: fix congestion oscillation bug
When evenly distributing congestion across cells, count the number of
occupied lchans surpassing congestion, and not the overall number of
free lchans -- which disregards congestion thresholds.
Fix the bugs shown by
test_congestion_no_oscillation.ho_vty
test_balance_congestion_tchf_tchh.ho_vty
This implements a simple calculation for congestion load by counting
lchans in use above congestion. An improvement of this calculation
using percent follows in I55234c6c99eb02ceee52be0d7388bea14304930f.
Related: SYS#5259
Change-Id: Icb373dc6bfc9819446db5e96f71921781fe2026d
---
M src/osmo-bsc/handover_decision_2.c
M tests/handover/test_balance_congestion_tchf_tchh.ho_vty
M tests/handover/test_congestion_no_oscillation.ho_vty
M tests/handover/test_congestion_no_oscillation2.ho_vty
4 files changed, 14 insertions(+), 36 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
diff --git a/src/osmo-bsc/handover_decision_2.c
b/src/osmo-bsc/handover_decision_2.c
index b296bd1..c265f5f 100644
--- a/src/osmo-bsc/handover_decision_2.c
+++ b/src/osmo-bsc/handover_decision_2.c
@@ -443,6 +443,7 @@
{
uint8_t requirement = 0;
unsigned int penalty_time;
+ int current_overbooked;
c->requirements = 0;
/* Requirement A */
@@ -625,14 +626,17 @@
/* Requirement C */
- /* the nr of free timeslots of the target cell must be >= the
- * free slots of the current cell _after_ handover/assignment */
+ /* the nr of lchans surpassing congestion on the target cell must be <=
the lchans surpassing congestion on the
+ * current cell _after_ handover/assignment */
+ current_overbooked = c->current.min_free_tch - c->current.free_tch;
if (requirement & REQUIREMENT_A_TCHF) {
- if (c->target.free_tchf - 1 >= c->current.free_tch + 1)
+ int target_overbooked = c->target.min_free_tchf -
c->target.free_tchf;
+ if (target_overbooked + 1 <= current_overbooked - 1)
requirement |= REQUIREMENT_C_TCHF;
}
if (requirement & REQUIREMENT_A_TCHH) {
- if (c->target.free_tchh - 1 >= c->current.free_tch + 1)
+ int target_overbooked = c->target.min_free_tchh -
c->target.free_tchh;
+ if (target_overbooked + 1 <= current_overbooked - 1)
requirement |= REQUIREMENT_C_TCHH;
}
@@ -1743,8 +1747,7 @@
goto exit;
}
- LOGPHOBTS(bts, LOGL_DEBUG, "Did not find a best candidate that fulfills
requirement C"
- " (omitting change from AHS to AFS)\n");
+ LOGPHOBTS(bts, LOGL_DEBUG, "Did not find a best candidate that fulfills
requirement C\n");
exit:
/* free array */
diff --git a/tests/handover/test_balance_congestion_tchf_tchh.ho_vty
b/tests/handover/test_balance_congestion_tchf_tchh.ho_vty
index d7b3cf5..7f9039f 100644
--- a/tests/handover/test_balance_congestion_tchf_tchh.ho_vty
+++ b/tests/handover/test_balance_congestion_tchf_tchh.ho_vty
@@ -35,8 +35,7 @@
set-ts-use trx 0 0 states * TCH/F TCH/F - -
TCH/HH - -
meas-rep lchan * * * * rxlev 20 rxqual 0 ta 0
congestion-check
-# FAIL: should be the same as above, but seeing a handover from F to H
-expect-ho from lchan 0 0 1 0 to lchan 0 0 6 0
+expect-no-chan
# TCH/F = +1, TCH/H = +2 above congestion. Moving a TCH/H to TCH/F would just
# reverse the situation to F=+2 H=+1. Nothing happens.
@@ -49,5 +48,4 @@
set-ts-use trx 0 0 states * TCH/F TCH/F - -
TCH/HH TCH/HH -
meas-rep lchan * * * * rxlev 20 rxqual 0 ta 0
congestion-check
-# FAIL: should be the same as above, from lchan 0 0 5 0 to lchan 0 0 3 0, but
no handover happens
-expect-no-chan
+expect-ho from lchan 0 0 5 0 to lchan 0 0 3 0
diff --git a/tests/handover/test_congestion_no_oscillation.ho_vty
b/tests/handover/test_congestion_no_oscillation.ho_vty
index abfaef7..a830cbe 100644
--- a/tests/handover/test_congestion_no_oscillation.ho_vty
+++ b/tests/handover/test_congestion_no_oscillation.ho_vty
@@ -1,6 +1,5 @@
# Do not oscillate handover from TCH/F to TCH/H on a neighbor due to
congestion,
# and then back to the original cell due to RXLEV.
-# Currently this test script shows the undesired oscillation.
create-bts trx-count 1 timeslots c+s4 TCH/F TCH/F TCH/F TCH/F TCH/F TCH/F
PDCH
network
@@ -25,24 +24,5 @@
# measurements continue to be the same
meas-rep lchan 1 0 5 0 rxlev 20 rxqual 0 ta 0 neighbors 40
-# FAIL: RXLEV oscillation back to bts 0
-expect-ho from lchan 1 0 5 0 to lchan 0 0 2 0
-expect-ts-use trx 0 0 states * TCH/F TCH/F - - - -
*
-expect-ts-use trx 1 0 states * TCH/F TCH/F TCH/F TCH/F - -
*
-meas-rep lchan 0 0 2 0 rxlev 40 rxqual 0 ta 0 neighbors 20
+# despite the better RXLEV, congestion prevents oscillation back to bts 0
expect-no-chan
-
-# FAIL: congestion oscillation again to bts 1
-congestion-check
-expect-ho from lchan 0 0 2 0 to lchan 1 0 5 0
-expect-ts-use trx 0 0 states * TCH/F - - - - -
*
-expect-ts-use trx 1 0 states * TCH/F TCH/F TCH/F TCH/F TCH/H- -
*
-
-# FAIL: RXLEV oscillation back to bts 0
-meas-rep lchan 1 0 5 0 rxlev 20 rxqual 0 ta 0 neighbors 40
-expect-ho from lchan 1 0 5 0 to lchan 0 0 2 0
-meas-rep lchan 0 0 2 0 rxlev 40 rxqual 0 ta 0 neighbors 20
-
-# FAIL: congestion oscillation again to bts 1
-congestion-check
-expect-ho from lchan 0 0 2 0 to lchan 1 0 5 0
diff --git a/tests/handover/test_congestion_no_oscillation2.ho_vty
b/tests/handover/test_congestion_no_oscillation2.ho_vty
index aee731d..44c4176 100644
--- a/tests/handover/test_congestion_no_oscillation2.ho_vty
+++ b/tests/handover/test_congestion_no_oscillation2.ho_vty
@@ -1,8 +1,5 @@
-# Almost identical to test_amr_oscillation.ho_vty, this has just two more
TCH/H slots in BTS 1, and does not trigger the
-# oscillation bug. The number of free TCH/H in BTS 1 should be unrelated to
the congestion status of BTS 0, which
-# illustrates that the even distribution of congestion is fundamentally flawed.
-# This test script shows the desired behavior, though by common sense there
should be no reason why we see the bug in
-# test_amr_oscillation.ho_vty and not here.
+# Almost identical to test_amr_oscillation.ho_vty, this has just two more
TCH/H slots in BTS 1, and did not trigger the
+# oscillation bug (which has since been fixed, so that both tests behave
identically now).
create-bts trx-count 1 timeslots c+s4 TCH/F TCH/F TCH/F TCH/F TCH/F TCH/F
PDCH
network
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/21989
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Icb373dc6bfc9819446db5e96f71921781fe2026d
Gerrit-Change-Number: 21989
Gerrit-PatchSet: 7
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged
