neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36893?usp=email ) Change subject: 3-digit MNC: use osmo_plmn_id in struct umts_cell_id ...................................................................... 3-digit MNC: use osmo_plmn_id in struct umts_cell_id Properly represent the mnc_3_digits flag in umts_cell_id, and preserve the three digit indicator as received on the wire. Before this patch, the indicator for a three digit MNC received on the wire was discarded, and instead g_hnbgw->config.plmn.mnc_3_digits was used to convert any PLMN to string, whether it had 3 digits or not. == hnb_persistent_list: The cell id is used as primary key in the list of hnb_persistent instances. This patch prevents any collisions between 2-digit and 3-digit MNCs (however unlikely in practice this may be). == nft_kpi.c: Just like the cell ids in hnb_persistent, the ids' strings are used as primary key in nftables rulesets in nft_kpi.c -- also prevent MNC collisions there: Properly transport the 3-digit property in conversions: struct umts_cell_id <-> string Uncouple to_str conversion from the PLMN set in the hnbgw VTY cfg. Related: OS#6457 Change-Id: Id9a91c80cd2745424a916aef4736993bb7cd8ba0 --- M include/osmocom/hnbgw/hnbgw.h M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/hnbgw_hnbap.c M src/osmo-hnbgw/hnbgw_vty.c M tests/umts_cell_id/umts_cell_id_test.c M tests/umts_cell_id/umts_cell_id_test.ok 6 files changed, 94 insertions(+), 46 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve neels: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index 87844f4..9b07584 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -166,8 +166,7 @@ }; struct umts_cell_id { - uint16_t mcc; /*!< Mobile Country Code (0-999) */ - uint16_t mnc; /*!< Mobile Network Code (0-999) */ + struct osmo_plmn_id plmn; /*!< Mobile Country Code and Mobile Network Code (000-00 to 999-999) */ uint16_t lac; /*!< Locaton Area Code (1-65534) */ uint16_t rac; /*!< Routing Area Code (0-255) */ uint16_t sac; /*!< Service Area Code */ @@ -182,9 +181,7 @@ /*! are both given umts_cell_id euqal? */ static inline bool umts_cell_id_equal(const struct umts_cell_id *a, const struct umts_cell_id *b) { - if (a->mcc != b->mcc) - return false; - if (a->mnc != b->mnc) + if (osmo_plmn_cmp(&a->plmn, &b->plmn)) return false; if (a->lac != b->lac) return false; diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 4f3fafa..20a665e 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -234,12 +234,10 @@ int umts_cell_id_to_str_buf(char *buf, size_t buflen, const struct umts_cell_id *ucid) { - const char *fmtstr = "%03u-%02u-L%u-R%u-S%u-C%u"; - - if (g_hnbgw->config.plmn.mnc_3_digits) - fmtstr = "%03u-%03u-L%u-R%u-S%u-C%u"; - return snprintf(buf, buflen, fmtstr, ucid->mcc, ucid->mnc, ucid->lac, ucid->rac, - ucid->sac, ucid->cid); + struct osmo_strbuf sb = { .buf = buf, .len = buflen }; + OSMO_STRBUF_APPEND_NOLEN(sb, osmo_plmn_name_buf, &ucid->plmn); + OSMO_STRBUF_PRINTF(sb, "-L%u-R%u-S%u-C%u", ucid->lac, ucid->rac, ucid->sac, ucid->cid); + return sb.chars_needed; } char *umts_cell_id_to_str_c(void *ctx, const struct umts_cell_id *ucid) @@ -262,22 +260,38 @@ int umts_cell_id_from_str(struct umts_cell_id *ucid, const char *instr) { int rc; + char buf[4]; + const char *pos = instr; + const char *end; /* We want to use struct umts_cell_id as hashtable key. If it ever happens to contain any padding bytes, make * sure everything is deterministically zero. */ memset(ucid, 0, sizeof(*ucid)); - rc = sscanf(instr, "%hu-%hu-L%hu-R%hu-S%hu-C%u", &ucid->mcc, &ucid->mnc, &ucid->lac, &ucid->rac, &ucid->sac, &ucid->cid); + /* read MCC */ + end = strchr(pos, '-'); + if (!end || end <= pos || (end - pos) >= sizeof(buf)) + return -EINVAL; + osmo_strlcpy(buf, pos, end - pos + 1); + if (osmo_mcc_from_str(buf, &ucid->plmn.mcc)) + return -EINVAL; + pos = end + 1; + + /* read MNC -- here the number of leading zeros matters. */ + end = strchr(pos, '-'); + if (!end || end == pos || (end - pos) >= sizeof(buf)) + return -EINVAL; + osmo_strlcpy(buf, pos, end - pos + 1); + if (osmo_mnc_from_str(buf, &ucid->plmn.mnc, &ucid->plmn.mnc_3_digits)) + return -EINVAL; + pos = end + 1; + + /* parse the rest, where leading zeros do not matter */ + rc = sscanf(pos, "L%hu-R%hu-S%hu-C%u", &ucid->lac, &ucid->rac, &ucid->sac, &ucid->cid); if (rc < 0) return -errno; - if (rc != 6) - return -EINVAL; - - if (ucid->mcc > 999) - return -EINVAL; - - if (ucid->mnc > 999) + if (rc != 4) return -EINVAL; if (ucid->lac == 0 || ucid->lac == 0xffff) diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c index 29851a0..b220918 100644 --- a/src/osmo-hnbgw/hnbgw_hnbap.c +++ b/src/osmo-hnbgw/hnbgw_hnbap.c @@ -472,7 +472,6 @@ struct hnb_context *hnb, *tmp; HNBAP_HNBRegisterRequestIEs_t ies; int rc; - struct osmo_plmn_id plmn; struct osmo_fd *ofd = osmo_stream_srv_get_ofd(ctx->conn); char identity_str[256]; const char *cell_id_str; @@ -500,9 +499,7 @@ ctx->id.sac = asn1str_to_u16(&ies.sac); ctx->id.rac = asn1str_to_u8(&ies.rac); ctx->id.cid = asn1bitstr_to_u28(&ies.cellIdentity); - osmo_plmn_from_bcd(ies.plmNidentity.buf, &plmn); - ctx->id.mcc = plmn.mcc; - ctx->id.mnc = plmn.mnc; + osmo_plmn_from_bcd(ies.plmNidentity.buf, &ctx->id.plmn); cell_id_str = umts_cell_id_to_str(&ctx->id); if (getpeername(ofd->fd, &cur_osa.u.sa, &len) < 0) { @@ -761,8 +758,7 @@ { /* We don't care much about HNBAP */ LOGHNB(hnb, DHNBAP, LOGL_ERROR, "Received Unsuccessful Outcome, procedureCode %ld, criticality %ld," - " cell mcc %u mnc %u lac %u rac %u sac %u cid %u\n", msg->procedureCode, msg->criticality, - hnb->id.mcc, hnb->id.mnc, hnb->id.lac, hnb->id.rac, hnb->id.sac, hnb->id.cid); + " cell %s\n", msg->procedureCode, msg->criticality, umts_cell_id_to_str(&hnb->id)); return 0; } diff --git a/src/osmo-hnbgw/hnbgw_vty.c b/src/osmo-hnbgw/hnbgw_vty.c index 8163197..d719c29 100644 --- a/src/osmo-hnbgw/hnbgw_vty.c +++ b/src/osmo-hnbgw/hnbgw_vty.c @@ -210,8 +210,9 @@ vty_out(vty, "HNB "); vty_out_ofd_addr(vty, hnb->conn? osmo_stream_srv_get_ofd(hnb->conn) : NULL); vty_out(vty, " \"%s\"%s", hnb->identity_info, VTY_NEWLINE); - vty_out(vty, " MCC %u MNC %u LAC %u RAC %u SAC %u CID %u SCTP-stream:HNBAP=%u,RUA=%u%s", - hnb->id.mcc, hnb->id.mnc, hnb->id.lac, hnb->id.rac, hnb->id.sac, hnb->id.cid, + vty_out(vty, " MCC %s MNC %s LAC %u RAC %u SAC %u CID %u SCTP-stream:HNBAP=%u,RUA=%u%s", + osmo_mcc_name(hnb->id.plmn.mcc), osmo_mnc_name(hnb->id.plmn.mnc, hnb->id.plmn.mnc_3_digits), + hnb->id.lac, hnb->id.rac, hnb->id.sac, hnb->id.cid, hnb->hnbap_stream, hnb->rua_stream, VTY_NEWLINE); llist_for_each_entry(map, &hnb->map_list, hnb_list) { diff --git a/tests/umts_cell_id/umts_cell_id_test.c b/tests/umts_cell_id/umts_cell_id_test.c index e9183c4..a25b3d1 100644 --- a/tests/umts_cell_id/umts_cell_id_test.c +++ b/tests/umts_cell_id/umts_cell_id_test.c @@ -12,8 +12,10 @@ { .id_str = "001-01-L1-R1-S1-C1", .id = { - .mcc = 1, - .mnc = 1, + .plmn = { + .mcc = 1, + .mnc = 1, + }, .lac = 1, .rac = 1, .sac = 1, @@ -25,8 +27,11 @@ { .id_str = "001-001-L1-R1-S1-C1", .id = { - .mcc = 1, - .mnc = 1, + .plmn = { + .mcc = 1, + .mnc = 1, + .mnc_3_digits = true, + }, .lac = 1, .rac = 1, .sac = 1, @@ -36,8 +41,11 @@ { .id_str = "001-099-L1-R1-S1-C1", .id = { - .mcc = 1, - .mnc = 99, + .plmn = { + .mcc = 1, + .mnc = 99, + .mnc_3_digits = true, + }, .lac = 1, .rac = 1, .sac = 1, @@ -47,8 +55,11 @@ { .id_str = "001-99-L1-R1-S1-C1", .id = { - .mcc = 1, - .mnc = 99, + .plmn = { + .mcc = 1, + .mnc = 99, + .mnc_3_digits = false, + }, .lac = 1, .rac = 1, .sac = 1, @@ -59,8 +70,11 @@ { .id_str = "999-999-L65534-R65535-S65535-C268435455", .id = { - .mcc = 999, - .mnc = 999, + .plmn = { + .mcc = 999, + .mnc = 999, + .mnc_3_digits = true, + }, .lac = 65534, .rac = 65535, .sac = 65535, @@ -94,12 +108,7 @@ int main(void) { - struct hnbgw hnbgw_dummy = {}; struct test *t; - - /* umts_cell_id_to_str() accesses g_hnbgw->config.plmn.mnc_3_digits, so make sure it is valid mem: */ - g_hnbgw = &hnbgw_dummy; - for (t = tests; (t - tests) < ARRAY_SIZE(tests); t++) { int rc; struct umts_cell_id parsed; diff --git a/tests/umts_cell_id/umts_cell_id_test.ok b/tests/umts_cell_id/umts_cell_id_test.ok index 457f78a..2c1992f 100644 --- a/tests/umts_cell_id/umts_cell_id_test.ok +++ b/tests/umts_cell_id/umts_cell_id_test.ok @@ -6,14 +6,12 @@ "001-001-L1-R1-S1-C1" -> umts_cell_id_from_str(): ok -> umts_cell_id_to_str_buf(): ok - ERROR: conversion to umts_cell_id and back to string doesn't return the original string - -> "001-01-L1-R1-S1-C1" + -> "001-001-L1-R1-S1-C1" umts_cell_id_equal(expected, parsed): ok "001-099-L1-R1-S1-C1" -> umts_cell_id_from_str(): ok -> umts_cell_id_to_str_buf(): ok - ERROR: conversion to umts_cell_id and back to string doesn't return the original string - -> "001-99-L1-R1-S1-C1" + -> "001-099-L1-R1-S1-C1" umts_cell_id_equal(expected, parsed): ok "001-99-L1-R1-S1-C1" -> umts_cell_id_from_str(): ok -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36893?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Id9a91c80cd2745424a916aef4736993bb7cd8ba0 Gerrit-Change-Number: 36893 Gerrit-PatchSet: 4 Gerrit-Owner: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder 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-MessageType: merged