From 5bdc3eca68b4c1c0a0515da52dbbd8c2fe993d93 Mon Sep 17 00:00:00 2001 From: Oliver Smith Date: Wed, 19 Oct 2022 16:51:35 +0200 Subject: [PATCH] gsm48_parse_meas_rep: fix parsing multi-band list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When looking up "BCCH-FREQ-NCELL i" from the measurement report, don't treat the BCCH channel list as one list sorted by ascending ARFCN. Instead, treat it as two sub lists, one for the same band, and one for channels in different bands, as described in 3GPP TS 04.08 § 10.5.2.20. This fixes getting the wrong ARFCN from measurement reports in multi-band BSS, which leads to failing handovers. Fixes: OS#5717 Related: osmo-ttcn3-hacks I4fe6bb9e4b5a69ea6204585ebdf1f157a68a8286 Change-Id: Ic5e4f0531e08685460948b102367825588d839ba --- include/osmocom/bsc/system_information.h | 1 + src/osmo-bsc/gsm_04_08_rr.c | 62 +++++++++++++++++-- src/osmo-bsc/system_information.c | 2 +- .../handover/test_meas_rep_multi_band.ho_vty | 10 +-- 4 files changed, 61 insertions(+), 14 deletions(-) diff --git a/include/osmocom/bsc/system_information.h b/include/osmocom/bsc/system_information.h index 08d34f8bf..7b1cff0f2 100644 --- a/include/osmocom/bsc/system_information.h +++ b/include/osmocom/bsc/system_information.h @@ -7,6 +7,7 @@ struct gsm_bts; +int band_compatible(const struct gsm_bts *bts, int arfcn); int generate_cell_chan_alloc(struct gsm_bts *bts); int generate_cell_chan_list(uint8_t *chan_list, struct gsm_bts *bts); int gsm_generate_si(struct gsm_bts *bts, enum osmo_sysinfo_type type); diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c index 3321a0ed6..ee772df6c 100644 --- a/src/osmo-bsc/gsm_04_08_rr.c +++ b/src/osmo-bsc/gsm_04_08_rr.c @@ -766,6 +766,56 @@ int gsm48_rx_rr_modif_ack(struct msgb *msg) return 0; } +/* Get the ARFCN from the BCCH channel list by the index "BCCH-FREQ-NCELL i" + * (idx) as described in 3GPP TS 144.018 § 10.5.2.20. The BCCH channel list is + * split into two sub lists, each ascendingly ordered by ARFCN > 0: + * 1) SI* and SI*bis entries + * 2) SI*ter entries (if available) + * ARFCN 0 is at the end of each sub list. + * Both sub lists are stored in one bitvec (nbv), iterate twice through it. */ +static int neigh_list_get_arfcn(struct gsm_bts *bts, const struct bitvec *nbv, unsigned int idx) +{ + unsigned int arfcn, i = 0; + + /* First sub list, ARFCN > 0 */ + for (arfcn = 1; arfcn < nbv->data_len * 8; arfcn++) { + if (bitvec_get_bit_pos(nbv, arfcn) == ZERO) + continue; + /* Skip SI*ter */ + if (!band_compatible(bts, arfcn)) + continue; + if (i == idx) + return arfcn; + i++; + } + + /* First sub list, ARFCN == 0 (last position) */ + if (bitvec_get_bit_pos(nbv, 0) == ONE && band_compatible(bts, 0)) { + if (i == idx) + return 0; + i++; + } + + /* Second sub list, ARFCN > 0 */ + for (arfcn = 1; arfcn < nbv->data_len * 8; arfcn++) { + if (bitvec_get_bit_pos(nbv, arfcn) == ZERO) + continue; + /* Require SI*ter */ + if (band_compatible(bts, arfcn)) + continue; + if (i == idx) + return arfcn; + i++; + } + + /* Second sub list, ARFCN == 0 (last position) */ + if (bitvec_get_bit_pos(nbv, 0) == ONE && !band_compatible(bts, 0) && i == idx) + return 0; + + LOGP(DRR, LOGL_ERROR, "Invalid BCCH channel list index %d in measurement report\n", idx); + return 0; +} + int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); @@ -800,7 +850,7 @@ int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg) mrc = &rep->cell[0]; mrc->rxlev = data[3] & 0x3f; mrc->neigh_idx = data[4] >> 3; - mrc->arfcn = bitvec_get_nth_set_bit(nbv, mrc->neigh_idx + 1); + mrc->arfcn = neigh_list_get_arfcn(bts, nbv, mrc->neigh_idx); mrc->bsic = ((data[4] & 0x07) << 3) | (data[5] >> 5); if (rep->num_cell < 2) return 0; @@ -808,7 +858,7 @@ int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg) mrc = &rep->cell[1]; mrc->rxlev = ((data[5] & 0x1f) << 1) | (data[6] >> 7); mrc->neigh_idx = (data[6] >> 2) & 0x1f; - mrc->arfcn = bitvec_get_nth_set_bit(nbv, mrc->neigh_idx + 1); + mrc->arfcn = neigh_list_get_arfcn(bts, nbv, mrc->neigh_idx); mrc->bsic = ((data[6] & 0x03) << 4) | (data[7] >> 4); if (rep->num_cell < 3) return 0; @@ -816,7 +866,7 @@ int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg) mrc = &rep->cell[2]; mrc->rxlev = ((data[7] & 0x0f) << 2) | (data[8] >> 6); mrc->neigh_idx = (data[8] >> 1) & 0x1f; - mrc->arfcn = bitvec_get_nth_set_bit(nbv, mrc->neigh_idx + 1); + mrc->arfcn = neigh_list_get_arfcn(bts, nbv, mrc->neigh_idx); mrc->bsic = ((data[8] & 0x01) << 5) | (data[9] >> 3); if (rep->num_cell < 4) return 0; @@ -824,7 +874,7 @@ int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg) mrc = &rep->cell[3]; mrc->rxlev = ((data[9] & 0x07) << 3) | (data[10] >> 5); mrc->neigh_idx = data[10] & 0x1f; - mrc->arfcn = bitvec_get_nth_set_bit(nbv, mrc->neigh_idx + 1); + mrc->arfcn = neigh_list_get_arfcn(bts, nbv, mrc->neigh_idx); mrc->bsic = data[11] >> 2; if (rep->num_cell < 5) return 0; @@ -832,7 +882,7 @@ int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg) mrc = &rep->cell[4]; mrc->rxlev = ((data[11] & 0x03) << 4) | (data[12] >> 4); mrc->neigh_idx = ((data[12] & 0xf) << 1) | (data[13] >> 7); - mrc->arfcn = bitvec_get_nth_set_bit(nbv, mrc->neigh_idx + 1); + mrc->arfcn = neigh_list_get_arfcn(bts, nbv, mrc->neigh_idx); mrc->bsic = (data[13] >> 1) & 0x3f; if (rep->num_cell < 6) return 0; @@ -840,7 +890,7 @@ int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg) mrc = &rep->cell[5]; mrc->rxlev = ((data[13] & 0x01) << 5) | (data[14] >> 3); mrc->neigh_idx = ((data[14] & 0x07) << 2) | (data[15] >> 6); - mrc->arfcn = bitvec_get_nth_set_bit(nbv, mrc->neigh_idx + 1); + mrc->arfcn = neigh_list_get_arfcn(bts, nbv, mrc->neigh_idx); mrc->bsic = data[15] & 0x3f; return 0; diff --git a/src/osmo-bsc/system_information.c b/src/osmo-bsc/system_information.c index 48f745359..cfc9a0450 100644 --- a/src/osmo-bsc/system_information.c +++ b/src/osmo-bsc/system_information.c @@ -52,7 +52,7 @@ struct gsm0808_cell_id_list2; * array. DCS1800 and PCS1900 can not be used at the same time so conserve * memory and do the below. */ -static int band_compatible(const struct gsm_bts *bts, int arfcn) +int band_compatible(const struct gsm_bts *bts, int arfcn) { enum gsm_band band; diff --git a/tests/handover/test_meas_rep_multi_band.ho_vty b/tests/handover/test_meas_rep_multi_band.ho_vty index 45bcfa3b7..d5fa62448 100644 --- a/tests/handover/test_meas_rep_multi_band.ho_vty +++ b/tests/handover/test_meas_rep_multi_band.ho_vty @@ -38,14 +38,10 @@ expect-ts-use trx 4 0 states * - - - - - - - meas-rep lchan 0 0 1 0 rxlev 20 rxqual 0 ta 0 neighbors 35 0 0 0 # If the BSC parsed the list correctly, it will request a handover to BTS 4. -# expect-ho from lchan 0 0 1 0 to lchan 4 0 1 0 +expect-ho from lchan 0 0 1 0 to lchan 4 0 1 0 -# FIXME: parses IDX=0 as ARFCN=0 instead of ARFCN=800, no handover is done -expect-no-chan - -# FIXME: should be on TRX 4 after handover -expect-ts-use trx 0 0 states * TCH/F - - - - - - +expect-ts-use trx 0 0 states * - - - - - - - expect-ts-use trx 1 0 states * - - - - - - - expect-ts-use trx 2 0 states * - - - - - - - expect-ts-use trx 3 0 states * - - - - - - - -expect-ts-use trx 4 0 states * - - - - - - - +expect-ts-use trx 4 0 states * TCH/F - - - - - -