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 <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: osmith <osm...@sysmocom.de>
Gerrit-MessageType: merged
