neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-bsc/+/14769
Change subject: neighbor config: allow re-using ARFCN+BSIC pairs
......................................................................
neighbor config: allow re-using ARFCN+BSIC pairs
Fix neighbor config to match OsmoBSC manual: implement the plan for neighbor
configuration that was so far only described in the manual without actually
being in operation.
This first allows re-using ARFCN+BSIC pairs in and across BSS.
So far the handover_start() code always looked for handover target cells across
*all* local cells, even if they were not listed as neighbors to a source cell.
Imply all cells as neighbors only as long as there are no explicit neighbors
configured. As soon as the first 'neighbor' line appears in a 'bts' config,
only the listed neighbors are regarded as handover target cells. (The
'neighbor-list' commands are not related to this, only the relatively new
'neighbor (bts|lac|cgi|...)' commands affect actual handover procedures.)
TTCN3 tests TC_ho_neighbor_config_1 thru _7 play through the various aspects of
neighbor configuration: both the legacy implicit all-cells-are-neighbors as
well as allowing only explicit neighbors by config.
Related: OS#4056
Related: osmo-ttcn3-hacks Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
Change-Id: I29bca59ab232eddc74e0d4698efb9c9992443983
---
M include/osmocom/bsc/handover.h
M include/osmocom/bsc/handover_fsm.h
M include/osmocom/bsc/neighbor_ident.h
M src/osmo-bsc/handover_decision_2.c
M src/osmo-bsc/handover_fsm.c
M src/osmo-bsc/handover_logic.c
M src/osmo-bsc/neighbor_ident_vty.c
M tests/bsc/bsc_test.c
8 files changed, 244 insertions(+), 62 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/69/14769/1
diff --git a/include/osmocom/bsc/handover.h b/include/osmocom/bsc/handover.h
index 322913d..b00ee60 100644
--- a/include/osmocom/bsc/handover.h
+++ b/include/osmocom/bsc/handover.h
@@ -10,6 +10,15 @@
#include <osmocom/bsc/neighbor_ident.h>
#include <osmocom/bsc/gsm_data.h>
+#define LOG_HO(conn, level, fmt, args...) do { \
+ if (conn->ho.fi) \
+ LOGPFSML(conn->ho.fi, level, "%s: " fmt, \
+ handover_status(conn), ## args); \
+ else \
+ LOGP(DHODEC, level, "%s: " fmt, \
+ handover_status(conn), ## args); \
+ } while(0)
+
struct gsm_network;
struct gsm_lchan;
struct gsm_bts;
@@ -25,6 +34,8 @@
HO_RESULT_ERROR,
};
+const char *handover_status(struct gsm_subscriber_connection *conn);
+
extern const struct value_string handover_result_names[];
inline static const char *handover_result_name(enum handover_result val)
{ return get_value_string(handover_result_names, val); }
@@ -70,8 +81,11 @@
struct gsm_lchan *lchan);
void bsc_tx_bssmap_ho_failure(struct gsm_subscriber_connection *conn);
-struct gsm_bts *bts_by_neighbor_ident(const struct gsm_network *net,
- const struct neighbor_ident_key
*search_for);
+int find_handover_target_cell(struct gsm_bts **local_target_cell_p,
+ const struct gsm0808_cell_id_list2
**remote_target_cell_p,
+ struct gsm_subscriber_connection *conn, const
struct neighbor_ident_key *search_for,
+ bool log_errors);
+
struct neighbor_ident_key *bts_ident_key(const struct gsm_bts *bts);
void handover_parse_inter_bsc_mt(struct gsm_subscriber_connection *conn,
diff --git a/include/osmocom/bsc/handover_fsm.h
b/include/osmocom/bsc/handover_fsm.h
index 7c2145e..1628d8f 100644
--- a/include/osmocom/bsc/handover_fsm.h
+++ b/include/osmocom/bsc/handover_fsm.h
@@ -4,18 +4,6 @@
#include <osmocom/bsc/debug.h>
#include <osmocom/bsc/handover.h>
-const char *handover_status(struct gsm_subscriber_connection *conn);
-
-/* This macro automatically includes a final \n, if omitted. */
-#define LOG_HO(conn, level, fmt, args...) do { \
- if (conn->ho.fi) \
- LOGPFSML(conn->ho.fi, level, "%s: " fmt, \
- handover_status(conn), ## args); \
- else \
- LOGP(DHODEC, level, "%s: " fmt, \
- handover_status(conn), ## args); \
- } while(0)
-
/* Terminology:
* Intra-Cell: stays within one BTS, this should actually be an Assignment.
* Intra-BSC: stays within one BSC, but moves between BTSes.
diff --git a/include/osmocom/bsc/neighbor_ident.h
b/include/osmocom/bsc/neighbor_ident.h
index 17bffbc..aa38276 100644
--- a/include/osmocom/bsc/neighbor_ident.h
+++ b/include/osmocom/bsc/neighbor_ident.h
@@ -47,6 +47,8 @@
void neighbor_ident_vty_init(struct gsm_network *net, struct
neighbor_ident_list *nil);
void neighbor_ident_vty_write(struct vty *vty, const char *indent, struct
gsm_bts *bts);
+bool neighbor_ident_bts_entry_exists(uint8_t from_bts);
+
#define NEIGHBOR_IDENT_VTY_KEY_PARAMS "arfcn <0-1023> bsic (<0-63>|any)"
#define NEIGHBOR_IDENT_VTY_KEY_DOC \
"ARFCN of neighbor cell\n" "ARFCN value\n" \
diff --git a/src/osmo-bsc/handover_decision_2.c
b/src/osmo-bsc/handover_decision_2.c
index a8fff63..0e24c0d 100644
--- a/src/osmo-bsc/handover_decision_2.c
+++ b/src/osmo-bsc/handover_decision_2.c
@@ -900,18 +900,8 @@
return;
}
- neighbor_bts = bts_by_neighbor_ident(bts->network, &ni);
-
- neighbor_cil = neighbor_ident_get(bts->network->neighbor_bss_cells,
&ni);
-
- if (neighbor_bts && neighbor_cil) {
- LOGPHOBTS(bts, LOGL_ERROR, "Configuration error: %s exists as
both local"
- " neighbor (bts %u) and remote-BSS neighbor (%s).
Will consider only"
- " the local-BSS neighbor.\n",
- neighbor_ident_key_name(&ni),
- neighbor_bts->nr,
gsm0808_cell_id_list_name(neighbor_cil));
- neighbor_cil = NULL;
- }
+ find_handover_target_cell(&neighbor_bts, &neighbor_cil,
+ lchan->conn, &ni, false);
if (!neighbor_bts && !neighbor_cil) {
LOGPHOBTS(bts, LOGL_DEBUG, "no neighbor ARFCN %u BSIC %u
configured for this cell\n",
diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index 6d0c2d4..d159347 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -31,6 +31,7 @@
#include <osmocom/bsc/bsc_subscriber.h>
#include <osmocom/bsc/handover_fsm.h>
+#include <osmocom/bsc/handover_cfg.h>
#include <osmocom/bsc/bsc_subscr_conn_fsm.h>
#include <osmocom/bsc/lchan_select.h>
#include <osmocom/bsc/lchan_fsm.h>
@@ -200,6 +201,9 @@
conn = req->old_lchan->conn;
OSMO_ASSERT(conn && conn->fi);
+ /* Make sure the handover target neighbor_ident_key contains the
correct source bts nr */
+ req->target_nik.from_bts = req->old_lchan->ts->trx->bts->nr;
+
/* To make sure we're allowed to start a handover, go through a gscon
event dispatch. If that is accepted, the
* same req is passed to handover_start(). */
osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_HANDOVER_START, req);
@@ -285,9 +289,10 @@
OSMO_ASSERT(req && req->old_lchan && req->old_lchan->conn);
struct gsm_subscriber_connection *conn = req->old_lchan->conn;
+ const struct neighbor_ident_key *search_for = &req->target_nik;
struct handover *ho = &conn->ho;
- struct gsm_bts *bts;
- const struct gsm0808_cell_id_list2 *cil;
+ struct gsm_bts *local_target_cell = NULL;
+ const struct gsm0808_cell_id_list2 *remote_target_cell = NULL;
if (conn->ho.fi) {
LOG_HO(conn, LOGL_ERROR, "Handover requested while another
handover is ongoing; Ignore\n");
@@ -295,6 +300,9 @@
}
handover_reset(conn);
+ /* When handover_start() is invoked by the gscon, it expects a
HANDOVER_END event. The best way to ensure this
+ * is to always create a handover_fsm instance, even if the target cell
is not resolved yet. Any failure should
+ * then call handover_end(), which ensures that the conn snaps back to
a valid state. */
handover_fsm_alloc(conn);
ho->from_hodec_id = req->from_hodec_id;
@@ -302,21 +310,25 @@
req->old_lchan->type : req->new_lchan_type;
ho->target_cell = req->target_nik;
- bts = bts_by_neighbor_ident(conn->network, &req->target_nik);
- if (bts) {
- ho->new_bts = bts;
+ if (find_handover_target_cell(&local_target_cell, &remote_target_cell,
+ conn, search_for, true))
+ goto no_handover;
+
+ if (local_target_cell) {
+ ho->new_bts = local_target_cell;
handover_start_intra_bsc(conn);
return;
}
- cil = neighbor_ident_get(conn->network->neighbor_bss_cells,
&req->target_nik);
- if (cil) {
- handover_start_inter_bsc_out(conn, cil);
+ if (remote_target_cell) {
+ handover_start_inter_bsc_out(conn, remote_target_cell);
return;
}
- LOG_HO(conn, LOGL_ERROR, "Cannot handover %s: neighbor unknown\n",
- neighbor_ident_key_name(&req->target_nik));
+ /* should never reach this, because find_handover_target_cell() would
have returned error. */
+ OSMO_ASSERT(false);
+
+no_handover:
handover_end(conn, HO_RESULT_FAIL_NO_CHANNEL);
}
diff --git a/src/osmo-bsc/handover_logic.c b/src/osmo-bsc/handover_logic.c
index 5725213..5be8383 100644
--- a/src/osmo-bsc/handover_logic.c
+++ b/src/osmo-bsc/handover_logic.c
@@ -125,42 +125,205 @@
return count;
}
-struct gsm_bts *bts_by_neighbor_ident(const struct gsm_network *net,
- const struct neighbor_ident_key
*search_for)
+/* Find out a handover target cell for the given neighbor_ident_key,
+ * and make sure there are no ambiguous matches.
+ * Given a source BTS and a target ARFCN+BSIC, find which cell is the right
handover target.
+ * ARFCN+BSIC may be re-used within and/or across BSS, so make sure that only
those cells that are explicitly
+ * listed as neighbor of the source cell are viable handover targets.
+ * The (legacy) default configuration is that, when no explicit neighbors are
listed, that all local cells are
+ * neighbors, in which case each ARFCN+BSIC must exist at most once.
+ * If there is more than one viable handover target cell found for the given
ARFCN+BSIC, that constitutes a
+ * configuration error and should not result in handover, so that the system's
misconfiguration is more likely
+ * to be found.
+ */
+int find_handover_target_cell(struct gsm_bts **local_target_cell_p,
+ const struct gsm0808_cell_id_list2
**remote_target_cell_p,
+ struct gsm_subscriber_connection *conn, const
struct neighbor_ident_key *search_for,
+ bool log_errors)
{
- struct gsm_bts *found = NULL;
- struct gsm_bts *bts;
- struct gsm_bts *wildcard_match = NULL;
+ struct gsm_network *net = conn->network;
+ struct gsm_bts *from_bts;
+ struct gsm_bts *local_target_cell = NULL;
+ const struct gsm0808_cell_id_list2 *remote_target_cell = NULL;
+ struct gsm_bts_ref *neigh;
+ bool ho_active;
+ bool as_active;
- llist_for_each_entry(bts, &net->bts_list, list) {
- struct neighbor_ident_key entry = {
- .from_bts = NEIGHBOR_IDENT_KEY_ANY_BTS,
- .arfcn = bts->c0->arfcn,
- .bsic = bts->bsic,
- };
- if (neighbor_ident_key_match(&entry, search_for, true)) {
- if (found) {
- LOGP(DHO, LOGL_ERROR, "CONFIG ERROR: Multiple
BTS match %s: %d and %d\n",
- neighbor_ident_key_name(search_for),
- found->nr, bts->nr);
- return found;
- }
- found = bts;
- }
- if (neighbor_ident_key_match(&entry, search_for, false))
- wildcard_match = bts;
+ if (local_target_cell_p)
+ *local_target_cell_p = NULL;
+ if (remote_target_cell_p)
+ *remote_target_cell_p = NULL;
+
+ if (!search_for) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR, "Handover without target
cell\n");
+ return -EINVAL;
}
- if (found)
- return found;
+ from_bts = gsm_bts_num(net, search_for->from_bts);
+ if (!from_bts) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR, "Handover without source
cell\n");
+ return -EINVAL;
+ }
- return wildcard_match;
+ ho_active = ho_get_ho_active(from_bts->ho);
+ as_active = (ho_get_algorithm(from_bts->ho) == 2)
+ && ho_get_hodec2_as_active(from_bts->ho);
+ if (!ho_active && !as_active) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR, "Cannot start Handover:
Handover and Assignment disabled for this source cell (%s)\n",
+ neighbor_ident_key_name(search_for));
+ return -EINVAL;
+ }
+
+ if (llist_empty(&from_bts->local_neighbors)
+ && !neighbor_ident_bts_entry_exists(from_bts->nr)) {
+ /* No explicit neighbor entries exist for this BTS. Hence apply
the legacy default behavior that all
+ * local cells are neighbors. */
+ struct gsm_bts *bts;
+ struct gsm_bts *wildcard_match = NULL;
+
+ LOG_HO(conn, LOGL_DEBUG, "No explicit neighbors, regarding all
local cells as neighbors\n");
+
+ llist_for_each_entry(bts, &net->bts_list, list) {
+ struct neighbor_ident_key bts_key = *bts_ident_key(bts);
+ if (neighbor_ident_key_match(&bts_key, search_for,
true)) {
+ if (local_target_cell) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR,
+ "NEIGHBOR CONFIGURATION
ERROR: Multiple local cells match %s"
+ " (BTS %d and BTS %d)."
+ " Aborting Handover
because of ambiguous network topology.\n",
+
neighbor_ident_key_name(search_for),
+ local_target_cell->nr,
bts->nr);
+ return -EINVAL;
+ }
+ local_target_cell = bts;
+ }
+ if (neighbor_ident_key_match(&bts_key, search_for,
false))
+ wildcard_match = bts;
+ }
+
+ if (!local_target_cell)
+ local_target_cell = wildcard_match;
+
+ if (!local_target_cell) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR, "Cannot Handover, no
cell matches %s\n",
+ neighbor_ident_key_name(search_for));
+ return -EINVAL;
+ }
+
+ if (local_target_cell == from_bts && !as_active) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR,
+ "Cannot start re-assignment, Assignment
disabled for this cell (%s)\n",
+ neighbor_ident_key_name(search_for));
+ return -EINVAL;
+ }
+ if (local_target_cell != from_bts && !ho_active) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR,
+ "Cannot start Handover, Handover
disabled for this cell (%s)\n",
+ neighbor_ident_key_name(search_for));
+ return -EINVAL;
+ }
+
+ if (local_target_cell_p)
+ *local_target_cell_p = local_target_cell;
+ return 0;
+ }
+
+ /* One or more local- or remote-BSS cell neighbors are configured. Find
a match among those, but also detect
+ * ambiguous matches (if multiple cells match, it is a configuration
error). */
+
+ LOG_HO(conn, LOGL_DEBUG, "There are explicit neighbors configured for
this cell\n");
+
+ /* Iterate explicit local neighbor cells */
+ llist_for_each_entry(neigh, &from_bts->local_neighbors, entry) {
+ struct gsm_bts *neigh_bts = neigh->bts;
+ struct neighbor_ident_key neigh_bts_key =
*bts_ident_key(neigh_bts);
+ neigh_bts_key.from_bts = from_bts->nr;
+
+ LOG_HO(conn, LOGL_DEBUG, "Local neighbor %s\n",
neighbor_ident_key_name(&neigh_bts_key));
+
+ if (!neighbor_ident_key_match(&neigh_bts_key, search_for,
true)) {
+ LOG_HO(conn, LOGL_DEBUG, "Doesn't match %s\n",
neighbor_ident_key_name(search_for));
+ continue;
+ }
+
+ if (local_target_cell) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR,
+ "NEIGHBOR CONFIGURATION ERROR: Multiple
BTS match %s (BTS %d and BTS %d)."
+ " Aborting Handover because of ambiguous
network topology.\n",
+ neighbor_ident_key_name(search_for),
local_target_cell->nr, neigh_bts->nr);
+ return -EINVAL;
+ }
+
+ local_target_cell = neigh_bts;
+ }
+
+ /* Any matching remote-BSS neighbor cell? */
+ remote_target_cell = neighbor_ident_get(net->neighbor_bss_cells,
search_for);
+
+ if (remote_target_cell)
+ LOG_HO(conn, LOGL_DEBUG, "Found remote target cell %s\n",
+ gsm0808_cell_id_list_name(remote_target_cell));
+
+ if (local_target_cell && remote_target_cell) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR, "NEIGHBOR CONFIGURATION ERROR:
Both a local and a remote-BSS cell match %s"
+ " (BTS %d and remote %s)."
+ " Aborting Handover because of ambiguous network
topology.\n",
+ neighbor_ident_key_name(search_for),
local_target_cell->nr,
+ gsm0808_cell_id_list_name(remote_target_cell));
+ return -EINVAL;
+ }
+
+ if (local_target_cell == from_bts && !as_active) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR,
+ "Cannot start re-assignment, Assignment disabled
for this cell (%s)\n",
+ neighbor_ident_key_name(search_for));
+ return -EINVAL;
+ }
+
+ if (((local_target_cell && local_target_cell != from_bts)
+ || remote_target_cell)
+ && !ho_active) {
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR,
+ "Cannot start Handover, Handover disabled for
this cell (%s)\n",
+ neighbor_ident_key_name(search_for));
+ return -EINVAL;
+ }
+
+ if (local_target_cell) {
+ if (local_target_cell_p)
+ *local_target_cell_p = local_target_cell;
+ return 0;
+ }
+
+ if (remote_target_cell) {
+ if (remote_target_cell_p)
+ *remote_target_cell_p = remote_target_cell;
+ return 0;
+ }
+
+ if (log_errors)
+ LOG_HO(conn, LOGL_ERROR, "Cannot handover %s: neighbor
unknown\n",
+ neighbor_ident_key_name(search_for));
+
+ return -ENODEV;
}
struct neighbor_ident_key *bts_ident_key(const struct gsm_bts *bts)
{
static struct neighbor_ident_key key;
key = (struct neighbor_ident_key){
+ .from_bts = NEIGHBOR_IDENT_KEY_ANY_BTS,
.arfcn = bts->c0->arfcn,
.bsic = bts->bsic,
};
diff --git a/src/osmo-bsc/neighbor_ident_vty.c
b/src/osmo-bsc/neighbor_ident_vty.c
index 715ee8b..f4b6407 100644
--- a/src/osmo-bsc/neighbor_ident_vty.c
+++ b/src/osmo-bsc/neighbor_ident_vty.c
@@ -389,6 +389,15 @@
return true;
}
+bool neighbor_ident_bts_entry_exists(uint8_t from_bts)
+{
+ struct nil_match_bts_data d = {
+ .bts_nr = from_bts,
+ };
+ neighbor_ident_iter(g_neighbor_cells, nil_match_bts, &d);
+ return (bool)d.found;
+}
+
static int del_all(struct vty *vty)
{
int rc;
@@ -428,7 +437,9 @@
/* Remove all remote-BSS neighbors */
while (1) {
struct neighbor_ident_key k;
- struct nil_match_bts_data d = {};
+ struct nil_match_bts_data d = {
+ .bts_nr = bts->nr,
+ };
neighbor_ident_iter(g_neighbor_cells, nil_match_bts, &d);
if (!d.found)
break;
diff --git a/tests/bsc/bsc_test.c b/tests/bsc/bsc_test.c
index 492f0c5..103e0bb 100644
--- a/tests/bsc/bsc_test.c
+++ b/tests/bsc/bsc_test.c
@@ -250,3 +250,5 @@
struct msgb *msg, int link_id, int allow_sacch) {}
void ts_fsm_alloc(struct gsm_bts_trx_ts *ts) {}
void lchan_activate(struct gsm_lchan *lchan, void *info) {}
+bool neighbor_ident_bts_entry_exists(uint8_t from_bts) { return false; }
+const char *handover_status(struct gsm_subscriber_connection *conn) { return
"x"; }
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/14769
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I29bca59ab232eddc74e0d4698efb9c9992443983
Gerrit-Change-Number: 14769
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-MessageType: newchange
