Change in osmo-bsc[master]: remove extract_sub(), add bsc_subscr_find_or_create_by_mi()

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

neels gerrit-no-reply at lists.osmocom.org
Tue Jun 16 22:01:26 UTC 2020


neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/18712 )

Change subject: remove extract_sub(), add bsc_subscr_find_or_create_by_mi()
......................................................................

remove extract_sub(), add bsc_subscr_find_or_create_by_mi()

Use the new osmo_mobile_identity API to shed some code dup and simplify.
gsm48_paging_extract_mi() is now unused, drop.

(More refactoring to use osmo_mobile_identity follows in subsequent patch.)

Depends: If4f7be606e54cfa1c59084cf169785b1cbda5cf5 (libosmocore)
Change-Id: Id6cccaac64392b737b3bba8f3a22a88009adb23b
---
M TODO-RELEASE
M include/osmocom/bsc/bsc_subscriber.h
M include/osmocom/bsc/gsm_04_08_rr.h
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/bsc_subscriber.c
M src/osmo-bsc/gsm_04_08_rr.c
M src/osmo-bsc/gsm_08_08.c
7 files changed, 55 insertions(+), 58 deletions(-)

Approvals:
  Jenkins Builder: Verified
  neels: Looks good to me, approved



diff --git a/TODO-RELEASE b/TODO-RELEASE
index e2fa427..b822f8a 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -11,3 +11,5 @@
 libosmocore	struct gsm0808_diagnostics	Depends on libosmocore > 1.3.0
 libosmocore	gsm0808_diagnostics_octet_location_str()	Depends on libosmocore > 1.3.0
 libosmocore	gsm0808_diagnostics_bit_location_str()	Depends on libosmocore > 1.3.0
+libosmocore	osmo_mobile_identity	Depends on libosmocore > 1.3.0
+osmo-bsc	Mobile Identity Coding	OsmoBSC is stricter in rejecting invalid coding of Mobile Identity IEs
diff --git a/include/osmocom/bsc/bsc_subscriber.h b/include/osmocom/bsc/bsc_subscriber.h
index 93b3539..53fa7be 100644
--- a/include/osmocom/bsc/bsc_subscriber.h
+++ b/include/osmocom/bsc/bsc_subscriber.h
@@ -6,6 +6,7 @@
 
 #include <osmocom/core/linuxlist.h>
 #include <osmocom/gsm/protocol/gsm_23_003.h>
+#include <osmocom/gsm/gsm48.h>
 
 struct log_target;
 
@@ -25,11 +26,13 @@
 						     const char *imsi);
 struct bsc_subscr *bsc_subscr_find_or_create_by_tmsi(struct llist_head *list,
 						     uint32_t tmsi);
+struct bsc_subscr *bsc_subscr_find_or_create_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi);
 
 struct bsc_subscr *bsc_subscr_find_by_imsi(struct llist_head *list,
 					   const char *imsi);
 struct bsc_subscr *bsc_subscr_find_by_tmsi(struct llist_head *list,
 					   uint32_t tmsi);
+struct bsc_subscr *bsc_subscr_find_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi);
 
 void bsc_subscr_set_imsi(struct bsc_subscr *bsub, const char *imsi);
 
diff --git a/include/osmocom/bsc/gsm_04_08_rr.h b/include/osmocom/bsc/gsm_04_08_rr.h
index 06cefa9..d34e695 100644
--- a/include/osmocom/bsc/gsm_04_08_rr.h
+++ b/include/osmocom/bsc/gsm_04_08_rr.h
@@ -40,8 +40,6 @@
 
 struct msgb *gsm48_create_mm_serv_rej(enum gsm48_reject_value value);
 int gsm48_extract_mi(uint8_t *classmark2_lv, int length, char *mi_string, uint8_t *mi_type);
-int gsm48_paging_extract_mi(struct gsm48_pag_resp *resp, int length,
-			    char *mi_string, uint8_t *mi_type);
 struct msgb *gsm48_create_loc_upd_rej(uint8_t cause);
 
 struct msgb *gsm48_create_rr_status(uint8_t cause);
diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index be7c575..bc4c017 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -52,8 +52,6 @@
 
 #define OBSC_LINKID_CB(__msgb)	(__msgb)->cb[3]
 
-#define tmsi_from_string(str) strtoul(str, NULL, 10)
-
 /* 3-bit long values */
 #define EARFCN_PRIO_INVALID 8
 #define EARFCN_MEAS_BW_INVALID 8
diff --git a/src/osmo-bsc/bsc_subscriber.c b/src/osmo-bsc/bsc_subscriber.c
index 38b532a..9ddfcaa 100644
--- a/src/osmo-bsc/bsc_subscriber.c
+++ b/src/osmo-bsc/bsc_subscriber.c
@@ -75,6 +75,20 @@
 	return NULL;
 }
 
+struct bsc_subscr *bsc_subscr_find_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi)
+{
+	if (!mi)
+		return NULL;
+	switch (mi->type) {
+	case GSM_MI_TYPE_IMSI:
+		return bsc_subscr_find_by_imsi(list, mi->imsi);
+	case GSM_MI_TYPE_TMSI:
+		return bsc_subscr_find_by_tmsi(list, mi->tmsi);
+	default:
+		return NULL;
+	}
+}
+
 void bsc_subscr_set_imsi(struct bsc_subscr *bsub, const char *imsi)
 {
 	if (!bsub)
@@ -110,6 +124,20 @@
 	return bsc_subscr_get(bsub);
 }
 
+struct bsc_subscr *bsc_subscr_find_or_create_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi)
+{
+	if (!mi)
+		return NULL;
+	switch (mi->type) {
+	case GSM_MI_TYPE_IMSI:
+		return bsc_subscr_find_or_create_by_imsi(list, mi->imsi);
+	case GSM_MI_TYPE_TMSI:
+		return bsc_subscr_find_or_create_by_tmsi(list, mi->tmsi);
+	default:
+		return NULL;
+	}
+}
+
 const char *bsc_subscr_name(struct bsc_subscr *bsub)
 {
 	static char buf[32];
diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c
index 4e5a307..4630b47 100644
--- a/src/osmo-bsc/gsm_04_08_rr.c
+++ b/src/osmo-bsc/gsm_04_08_rr.c
@@ -842,16 +842,6 @@
 	return gsm48_mi_to_string(mi_string, GSM48_MI_SIZE, mi_lv+1, *mi_lv);
 }
 
-int gsm48_paging_extract_mi(struct gsm48_pag_resp *resp, int length,
-			    char *mi_string, uint8_t *mi_type)
-{
-	static const uint32_t classmark_offset =
-		offsetof(struct gsm48_pag_resp, classmark2);
-	uint8_t *classmark2_lv = (uint8_t *) &resp->classmark2;
-	return gsm48_extract_mi(classmark2_lv, length - classmark_offset,
-				mi_string, mi_type);
-}
-
 /* As per TS 03.03 Section 2.2, the IMSI has 'not more than 15 digits' */
 uint64_t str_to_imsi(const char *imsi_str)
 {
diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c
index a252203..7f54fe6 100644
--- a/src/osmo-bsc/gsm_08_08.c
+++ b/src/osmo-bsc/gsm_08_08.c
@@ -122,46 +122,6 @@
 	return cm->cm_service_type == GSM48_CMSERV_EMERGENCY;
 }
 
-/* extract a subscriber from the paging response */
-static struct bsc_subscr *extract_sub(struct gsm_subscriber_connection *conn,
-				   struct msgb *msg)
-{
-	uint8_t mi_type;
-	char mi_string[GSM48_MI_SIZE];
-	struct gsm48_hdr *gh;
-	struct gsm48_pag_resp *resp;
-	struct bsc_subscr *subscr;
-
-	if (msgb_l3len(msg) < sizeof(*gh) + sizeof(*resp)) {
-		LOGP(DMSC, LOGL_ERROR, "PagingResponse too small: %u\n", msgb_l3len(msg));
-		return NULL;
-	}
-
-	gh = msgb_l3(msg);
-	resp = (struct gsm48_pag_resp *) &gh->data[0];
-
-	gsm48_paging_extract_mi(resp, msgb_l3len(msg) - sizeof(*gh),
-				mi_string, &mi_type);
-	DEBUGP(DRR, "PAGING RESPONSE: MI(%s)=%s\n",
-		gsm48_mi_type_name(mi_type), mi_string);
-
-	switch (mi_type) {
-	case GSM_MI_TYPE_TMSI:
-		subscr = bsc_subscr_find_by_tmsi(conn->network->bsc_subscribers,
-					      tmsi_from_string(mi_string));
-		break;
-	case GSM_MI_TYPE_IMSI:
-		subscr = bsc_subscr_find_by_imsi(conn->network->bsc_subscribers,
-					      mi_string);
-		break;
-	default:
-		subscr = NULL;
-		break;
-	}
-
-	return subscr;
-}
-
 static bool is_msc_usable(struct bsc_msc_data *msc, bool is_emerg)
 {
 	if (is_emerg && !msc->allow_emerg)
@@ -184,6 +144,7 @@
 	struct gsm48_hdr *gh;
 	int8_t pdisc;
 	uint8_t mtype;
+	struct osmo_mobile_identity mi;
 	struct bsc_msc_data *msc;
 	struct bsc_msc_data *msc_target = NULL;
 	struct bsc_msc_data *msc_round_robin_next = NULL;
@@ -203,9 +164,19 @@
 
 	is_emerg = (pdisc == GSM48_PDISC_MM && mtype == GSM48_MT_MM_CM_SERV_REQ) && is_cm_service_for_emerg(msg);
 
+	if (osmo_mobile_identity_decode_from_l3(&mi, msg, false)) {
+		LOG_COMPL_L3(pdisc, mtype, LOGL_ERROR, "Cannot extract Mobile Identity: %s\n",
+			     msgb_hexdump_c(OTC_SELECT, msg));
+		/* There is no Mobile Identity to pick a matching MSC from. Likely this is an invalid Complete Layer 3
+		 * message that deserves to be rejected. However, the current state of our ttcn3 tests does send invalid
+		 * Layer 3 Info in some tests and expects osmo-bsc to not care about that. So, changing the behavior to
+		 * rejecting on missing MI causes test failure and, if at all, should happen in a separate patch.
+		 * See e.g. BSC_Tests.TC_chan_rel_rll_rel_ind: "dt := f_est_dchan('23'O, 23, '00010203040506'O);" */
+	}
+
 	/* Has the subscriber been paged from a connected MSC? */
 	if (pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP) {
-		subscr = extract_sub(conn, msg);
+		subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi);
 		if (subscr) {
 			msc_target = paging_get_msc(conn_get_bts(conn), subscr);
 			bsc_subscr_put(subscr);
@@ -241,8 +212,8 @@
 	 * them are usable -- wrap to the start. */
 	msc_target = msc_round_robin_next ? : msc_round_robin_first;
 	if (!msc_target) {
-		LOG_COMPL_L3(pdisc, mtype, LOGL_ERROR, "%sNo suitable MSC for this Complete Layer 3 request found\n",
-			     is_emerg ? "FOR EMERGENCY CALL: " : "");
+		LOG_COMPL_L3(pdisc, mtype, LOGL_ERROR, "%s%s: No suitable MSC for this Complete Layer 3 request found\n",
+			     osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), is_emerg ? " FOR EMERGENCY CALL" : "");
 		return NULL;
 	}
 
@@ -256,8 +227,15 @@
 
 static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb *msg)
 {
-	struct bsc_subscr *subscr = extract_sub(conn, msg);
+	struct osmo_mobile_identity mi;
+	struct bsc_subscr *subscr = NULL;
 
+	if (osmo_mobile_identity_decode_from_l3(&mi, msg, false)) {
+		LOGP(DRSL, LOGL_ERROR, "Unable to extract Mobile Identity from Paging Response\n");
+		return -1;
+	}
+
+	subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi);
 	if (!subscr) {
 		LOGP(DMSC, LOGL_ERROR, "Non active subscriber got paged.\n");
 		rate_ctr_inc(&conn->lchan->ts->trx->bts->bts_ctrs->ctr[BTS_CTR_PAGING_NO_ACTIVE_PAGING]);

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/18712
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Id6cccaac64392b737b3bba8f3a22a88009adb23b
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <daniel at totalueberwachung.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200616/72fc3d23/attachment.htm>


More information about the gerrit-log mailing list