[M] Change in osmo-hnbgw[master]: 3-digit MNC: use osmo_plmn_id in struct umts_cell_id - gerrit-log

10 Jun 2024

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



    