Change in osmo-bsc[master]: gsm48_parse_meas_rep: fix parsing multi-band list

osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29836 ) Change subject: gsm48_parse_meas_rep: fix parsing multi-band list ...................................................................... gsm48_parse_meas_rep: fix parsing multi-band list 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 --- M include/osmocom/bsc/system_information.h M src/osmo-bsc/gsm_04_08_rr.c M src/osmo-bsc/system_information.c M tests/handover/test_meas_rep_multi_band.ho_vty 4 files changed, 61 insertions(+), 14 deletions(-) Approvals: fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmocom/bsc/system_information.h b/include/osmocom/bsc/system_information.h index 08d34f8..7b1cff0 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 3321a0e..ee772df 100644 --- a/src/osmo-bsc/gsm_04_08_rr.c +++ b/src/osmo-bsc/gsm_04_08_rr.c @@ -766,6 +766,56 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 48f7453..cfc9a04 100644 --- a/src/osmo-bsc/system_information.c +++ b/src/osmo-bsc/system_information.c @@ -52,7 +52,7 @@ * 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 45bcfa3..d5fa624 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 @@ 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 - - - - - - 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/29836 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ic5e4f0531e08685460948b102367825588d839ba Gerrit-Change-Number: 29836 Gerrit-PatchSet: 7 Gerrit-Owner: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged