<p>neels has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21204">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">hodec 2: favor moving dyn TS<br><br>Change-Id: Ic221b8d2687cdec0bf94410c84a4da43853f0900<br>---<br>M src/osmo-bsc/handover_decision_2.c<br>M tests/handover/handover_test.c<br>2 files changed, 105 insertions(+), 65 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/04/21204/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 c818dbb..68ead67 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>@@ -1362,6 +1362,100 @@</span><br><span>       }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static void collect_candidates_on_ts(struct gsm_bts_trx_ts *ts, struct ho_candidate *clist, unsigned int *candidates,</span><br><span style="color: hsl(120, 100%, 40%);">+                                  int tchf_congestion, int tchh_congestion,</span><br><span style="color: hsl(120, 100%, 40%);">+                                     bool only_dyn_ts)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+     int j;</span><br><span style="color: hsl(120, 100%, 40%);">+        struct gsm_lchan *lc;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+       if (!ts_is_usable(ts))</span><br><span style="color: hsl(120, 100%, 40%);">+                return;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     switch (ts->pchan_on_init) {</span><br><span style="color: hsl(120, 100%, 40%);">+       case GSM_PCHAN_TCH_F_TCH_H_PDCH:</span><br><span style="color: hsl(120, 100%, 40%);">+      case GSM_PCHAN_TCH_F_PDCH:</span><br><span style="color: hsl(120, 100%, 40%);">+            if (!only_dyn_ts)</span><br><span style="color: hsl(120, 100%, 40%);">+                     return;</span><br><span style="color: hsl(120, 100%, 40%);">+               break;</span><br><span style="color: hsl(120, 100%, 40%);">+        default:</span><br><span style="color: hsl(120, 100%, 40%);">+              if (only_dyn_ts)</span><br><span style="color: hsl(120, 100%, 40%);">+                      return;</span><br><span style="color: hsl(120, 100%, 40%);">+               break;</span><br><span style="color: hsl(120, 100%, 40%);">+        }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* (Do not consider dynamic TS that are in PDCH mode) */</span><br><span style="color: hsl(120, 100%, 40%);">+      switch (ts->pchan_is) {</span><br><span style="color: hsl(120, 100%, 40%);">+    case GSM_PCHAN_TCH_F:</span><br><span style="color: hsl(120, 100%, 40%);">+         /* No need to collect TCH/F candidates if no TCH/F needs to be moved. */</span><br><span style="color: hsl(120, 100%, 40%);">+              if (tchf_congestion == 0)</span><br><span style="color: hsl(120, 100%, 40%);">+                     return;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+             lc = &ts->lchan[0];</span><br><span style="color: hsl(120, 100%, 40%);">+            /* omit if channel not active */</span><br><span style="color: hsl(120, 100%, 40%);">+              if (lc->type != GSM_LCHAN_TCH_F</span><br><span style="color: hsl(120, 100%, 40%);">+                || !lchan_state_is(lc, LCHAN_ST_ESTABLISHED))</span><br><span style="color: hsl(120, 100%, 40%);">+                     break;</span><br><span style="color: hsl(120, 100%, 40%);">+                /* omit if there is an ongoing ho/as */</span><br><span style="color: hsl(120, 100%, 40%);">+               if (!lc->conn || lc->conn->assignment.new_lchan</span><br><span style="color: hsl(120, 100%, 40%);">+                  || lc->conn->ho.fi)</span><br><span style="color: hsl(120, 100%, 40%);">+                 break;</span><br><span style="color: hsl(120, 100%, 40%);">+                /* We desperately want to resolve congestion, ignore rxlev when</span><br><span style="color: hsl(120, 100%, 40%);">+                * collecting candidates by passing include_weaker_rxlev=true. */</span><br><span style="color: hsl(120, 100%, 40%);">+             collect_candidates_for_lchan(lc, clist, candidates, NULL, true);</span><br><span style="color: hsl(120, 100%, 40%);">+              break;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+      case GSM_PCHAN_TCH_H:</span><br><span style="color: hsl(120, 100%, 40%);">+         /* No need to collect TCH/H candidates if no TCH/H needs to be moved. */</span><br><span style="color: hsl(120, 100%, 40%);">+              if (tchh_congestion == 0)</span><br><span style="color: hsl(120, 100%, 40%);">+                     return;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+             for (j = 0; j < 2; j++) {</span><br><span style="color: hsl(120, 100%, 40%);">+                  lc = &ts->lchan[j];</span><br><span style="color: hsl(120, 100%, 40%);">+                    /* omit if channel not active */</span><br><span style="color: hsl(120, 100%, 40%);">+                      if (lc->type != GSM_LCHAN_TCH_H</span><br><span style="color: hsl(120, 100%, 40%);">+                        || !lchan_state_is(lc, LCHAN_ST_ESTABLISHED))</span><br><span style="color: hsl(120, 100%, 40%);">+                             return;</span><br><span style="color: hsl(120, 100%, 40%);">+                       /* omit of there is an ongoing ho/as */</span><br><span style="color: hsl(120, 100%, 40%);">+                       if (!lc->conn</span><br><span style="color: hsl(120, 100%, 40%);">+                          || lc->conn->assignment.new_lchan</span><br><span style="color: hsl(120, 100%, 40%);">+                       || lc->conn->ho.fi)</span><br><span style="color: hsl(120, 100%, 40%);">+                         return;</span><br><span style="color: hsl(120, 100%, 40%);">+                       /* We desperately want to resolve congestion, ignore rxlev when</span><br><span style="color: hsl(120, 100%, 40%);">+                        * collecting candidates by passing include_weaker_rxlev=true. */</span><br><span style="color: hsl(120, 100%, 40%);">+                     collect_candidates_for_lchan(lc, clist, candidates, NULL, true);</span><br><span style="color: hsl(120, 100%, 40%);">+              }</span><br><span style="color: hsl(120, 100%, 40%);">+             break;</span><br><span style="color: hsl(120, 100%, 40%);">+        default:</span><br><span style="color: hsl(120, 100%, 40%);">+              break;</span><br><span style="color: hsl(120, 100%, 40%);">+        }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* loop through all active lchan and collect candidates */</span><br><span style="color: hsl(120, 100%, 40%);">+static void collect_candidates_on_bts(struct gsm_bts *bts, struct ho_candidate *clist, unsigned int *candidates,</span><br><span style="color: hsl(120, 100%, 40%);">+                                int tchf_congestion, int tchh_congestion,</span><br><span style="color: hsl(120, 100%, 40%);">+                                     bool only_dyn_ts)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+    struct gsm_bts_trx *trx;</span><br><span style="color: hsl(120, 100%, 40%);">+      int i;</span><br><span style="color: hsl(120, 100%, 40%);">+</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%);">+              if (!trx_is_usable(trx))</span><br><span style="color: hsl(120, 100%, 40%);">+                      continue;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+           if (only_dyn_ts) {</span><br><span style="color: hsl(120, 100%, 40%);">+                    /* iterate dyn TS in reverse, to favor moving "later" timeslots */</span><br><span style="color: hsl(120, 100%, 40%);">+                  for (i = 7; i >= 0; i--)</span><br><span style="color: hsl(120, 100%, 40%);">+                           collect_candidates_on_ts(&trx->ts[i], clist, candidates, tchf_congestion,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                       tchh_congestion, only_dyn_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+               } else {</span><br><span style="color: hsl(120, 100%, 40%);">+                      for (i = 0; i < 8; i++)</span><br><span style="color: hsl(120, 100%, 40%);">+                            collect_candidates_on_ts(&trx->ts[i], clist, candidates, tchf_congestion,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                       tchh_congestion, only_dyn_ts);</span><br><span style="color: hsl(120, 100%, 40%);">+               }</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*</span><br><span>  * Handover/assignment check after timer timeout:</span><br><span>  *</span><br><span>@@ -1413,10 +1507,7 @@</span><br><span>  */</span><br><span> static int bts_resolve_congestion(struct gsm_bts *bts, int tchf_congestion, int tchh_congestion)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-        struct gsm_lchan *lc;</span><br><span style="color: hsl(0, 100%, 40%);">-   struct gsm_bts_trx *trx;</span><br><span style="color: hsl(0, 100%, 40%);">-        struct gsm_bts_trx_ts *ts;</span><br><span style="color: hsl(0, 100%, 40%);">-      int i, j;</span><br><span style="color: hsl(120, 100%, 40%);">+     int i;</span><br><span>       struct ho_candidate *clist;</span><br><span>  unsigned int candidates;</span><br><span>     struct ho_candidate *best_cand = NULL, *worst_cand = NULL;</span><br><span>@@ -1437,68 +1528,18 @@</span><br><span> </span><br><span>     /* allocate array of all bts */</span><br><span>      clist = talloc_zero_array(tall_bsc_ctx, struct ho_candidate,</span><br><span style="color: hsl(0, 100%, 40%);">-            bts->num_trx * 8 * 2 * (1 + ARRAY_SIZE(lc->neigh_meas)));</span><br><span style="color: hsl(120, 100%, 40%);">+               bts->num_trx * 8 * 2 * (1 + ARRAY_SIZE(((struct gsm_lchan*)0)->neigh_meas)));</span><br><span>  if (!clist)</span><br><span>          return 0;</span><br><span> </span><br><span>        candidates = 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     /* loop through all active lchan and collect candidates */</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%);">-                if (!trx_is_usable(trx))</span><br><span style="color: hsl(0, 100%, 40%);">-                        continue;</span><br><span style="color: hsl(120, 100%, 40%);">+     /* First collect candidates for dyn TS, to favor freeing PDCH. This specifically makes a difference when</span><br><span style="color: hsl(120, 100%, 40%);">+       * rxlev for a target cell are equal, particularly when considering re-assignment within the same cell. */</span><br><span style="color: hsl(120, 100%, 40%);">+    collect_candidates_on_bts(bts, clist, &candidates, tchf_congestion, tchh_congestion, true);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-             for (i = 0; i < 8; i++) {</span><br><span style="color: hsl(0, 100%, 40%);">-                    ts = &trx->ts[i];</span><br><span style="color: hsl(0, 100%, 40%);">-                        if (!ts_is_usable(ts))</span><br><span style="color: hsl(0, 100%, 40%);">-                          continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-                       /* (Do not consider dynamic TS that are in PDCH mode) */</span><br><span style="color: hsl(0, 100%, 40%);">-                        switch (ts->pchan_is) {</span><br><span style="color: hsl(0, 100%, 40%);">-                      case GSM_PCHAN_TCH_F:</span><br><span style="color: hsl(0, 100%, 40%);">-                           /* No need to collect TCH/F candidates if no TCH/F needs to be moved. */</span><br><span style="color: hsl(0, 100%, 40%);">-                                if (tchf_congestion == 0)</span><br><span style="color: hsl(0, 100%, 40%);">-                                       continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-                               lc = &ts->lchan[0];</span><br><span style="color: hsl(0, 100%, 40%);">-                              /* omit if channel not active */</span><br><span style="color: hsl(0, 100%, 40%);">-                                if (lc->type != GSM_LCHAN_TCH_F</span><br><span style="color: hsl(0, 100%, 40%);">-                                  || !lchan_state_is(lc, LCHAN_ST_ESTABLISHED))</span><br><span style="color: hsl(0, 100%, 40%);">-                                       break;</span><br><span style="color: hsl(0, 100%, 40%);">-                          /* omit if there is an ongoing ho/as */</span><br><span style="color: hsl(0, 100%, 40%);">-                         if (!lc->conn || lc->conn->assignment.new_lchan</span><br><span style="color: hsl(0, 100%, 40%);">-                                    || lc->conn->ho.fi)</span><br><span style="color: hsl(0, 100%, 40%);">-                                   break;</span><br><span style="color: hsl(0, 100%, 40%);">-                          /* We desperately want to resolve congestion, ignore rxlev when</span><br><span style="color: hsl(0, 100%, 40%);">-                          * collecting candidates by passing include_weaker_rxlev=true. */</span><br><span style="color: hsl(0, 100%, 40%);">-                               collect_candidates_for_lchan(lc, clist, &candidates, NULL, true);</span><br><span style="color: hsl(0, 100%, 40%);">-                           break;</span><br><span style="color: hsl(0, 100%, 40%);">-                  case GSM_PCHAN_TCH_H:</span><br><span style="color: hsl(0, 100%, 40%);">-                           /* No need to collect TCH/H candidates if no TCH/H needs to be moved. */</span><br><span style="color: hsl(0, 100%, 40%);">-                                if (tchh_congestion == 0)</span><br><span style="color: hsl(0, 100%, 40%);">-                                       continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-                               for (j = 0; j < 2; j++) {</span><br><span style="color: hsl(0, 100%, 40%);">-                                    lc = &ts->lchan[j];</span><br><span style="color: hsl(0, 100%, 40%);">-                                      /* omit if channel not active */</span><br><span style="color: hsl(0, 100%, 40%);">-                                        if (lc->type != GSM_LCHAN_TCH_H</span><br><span style="color: hsl(0, 100%, 40%);">-                                          || !lchan_state_is(lc, LCHAN_ST_ESTABLISHED))</span><br><span style="color: hsl(0, 100%, 40%);">-                                               continue;</span><br><span style="color: hsl(0, 100%, 40%);">-                                       /* omit of there is an ongoing ho/as */</span><br><span style="color: hsl(0, 100%, 40%);">-                                 if (!lc->conn</span><br><span style="color: hsl(0, 100%, 40%);">-                                            || lc->conn->assignment.new_lchan</span><br><span style="color: hsl(0, 100%, 40%);">-                                         || lc->conn->ho.fi)</span><br><span style="color: hsl(0, 100%, 40%);">-                                           continue;</span><br><span style="color: hsl(0, 100%, 40%);">-                                       /* We desperately want to resolve congestion, ignore rxlev when</span><br><span style="color: hsl(0, 100%, 40%);">-                                  * collecting candidates by passing include_weaker_rxlev=true. */</span><br><span style="color: hsl(0, 100%, 40%);">-                                       collect_candidates_for_lchan(lc, clist, &candidates, NULL, true);</span><br><span style="color: hsl(0, 100%, 40%);">-                           }</span><br><span style="color: hsl(0, 100%, 40%);">-                               break;</span><br><span style="color: hsl(0, 100%, 40%);">-                  default:</span><br><span style="color: hsl(0, 100%, 40%);">-                                break;</span><br><span style="color: hsl(0, 100%, 40%);">-                  }</span><br><span style="color: hsl(0, 100%, 40%);">-               }</span><br><span style="color: hsl(0, 100%, 40%);">-       }</span><br><span style="color: hsl(120, 100%, 40%);">+     /* Then collect candidates for non-dyn TS */</span><br><span style="color: hsl(120, 100%, 40%);">+  collect_candidates_on_bts(bts, clist, &candidates, tchf_congestion, tchh_congestion, false);</span><br><span> </span><br><span>         if (!candidates) {</span><br><span>           LOGPHOBTS(bts, LOGL_DEBUG, "No neighbor cells qualify to solve congestion\n");</span><br><span>diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c</span><br><span>index 6befc5a..804ff4b 100644</span><br><span>--- a/tests/handover/handover_test.c</span><br><span>+++ b/tests/handover/handover_test.c</span><br><span>@@ -1679,16 +1679,15 @@</span><br><span>      "congestion-check",</span><br><span>        "expect-chan", "0", "6",</span><br><span>       "ack-chan",</span><br><span style="color: hsl(0, 100%, 40%);">-   /* Not so good: rather than moving static TCH/F, we should favor freeing dyn TS, for more PDCH */</span><br><span style="color: hsl(0, 100%, 40%);">-       "expect-ho", "0", "1",</span><br><span style="color: hsl(120, 100%, 40%);">+  "expect-ho", "0", "5",</span><br><span>         "ho-complete",</span><br><span style="color: hsl(0, 100%, 40%);">-        "expect-ts-use", "0", "0", "*", "-", "TCH/F", "TCH/F", "TCH/F", "TCH/F", "TCH/H-", "PDCH",</span><br><span style="color: hsl(120, 100%, 40%);">+      "expect-ts-use", "0", "0", "*", "TCH/F", "TCH/F", "TCH/F", "TCH/F", "PDCH", "TCH/H-", "PDCH",</span><br><span>  "congestion-check",</span><br><span>        "expect-chan", "0", "6",</span><br><span>       "ack-chan",</span><br><span style="color: hsl(0, 100%, 40%);">-   "expect-ho", "0", "2",</span><br><span style="color: hsl(120, 100%, 40%);">+  "expect-ho", "0", "4",</span><br><span>         "ho-complete",</span><br><span style="color: hsl(0, 100%, 40%);">-        "expect-ts-use", "0", "0", "*", "-", "-", "TCH/F", "TCH/F", "TCH/F", "TCH/HH", "PDCH",</span><br><span style="color: hsl(120, 100%, 40%);">+  "expect-ts-use", "0", "0", "*", "TCH/F", "TCH/F", "TCH/F", "PDCH", "PDCH", "TCH/HH", "PDCH",</span><br><span>   "congestion-check",</span><br><span>        "expect-no-chan",</span><br><span>  NULL</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21204">change 21204</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/+/21204"/><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: Ic221b8d2687cdec0bf94410c84a4da43853f0900 </div>
<div style="display:none"> Gerrit-Change-Number: 21204 </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>