<p>neels <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/18712">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
neels: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">remove extract_sub(), add bsc_subscr_find_or_create_by_mi()<br><br>Use the new osmo_mobile_identity API to shed some code dup and simplify.<br>gsm48_paging_extract_mi() is now unused, drop.<br><br>(More refactoring to use osmo_mobile_identity follows in subsequent patch.)<br><br>Depends: If4f7be606e54cfa1c59084cf169785b1cbda5cf5 (libosmocore)<br>Change-Id: Id6cccaac64392b737b3bba8f3a22a88009adb23b<br>---<br>M TODO-RELEASE<br>M include/osmocom/bsc/bsc_subscriber.h<br>M include/osmocom/bsc/gsm_04_08_rr.h<br>M include/osmocom/bsc/gsm_data.h<br>M src/osmo-bsc/bsc_subscriber.c<br>M src/osmo-bsc/gsm_04_08_rr.c<br>M src/osmo-bsc/gsm_08_08.c<br>7 files changed, 55 insertions(+), 58 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/TODO-RELEASE b/TODO-RELEASE</span><br><span>index e2fa427..b822f8a 100644</span><br><span>--- a/TODO-RELEASE</span><br><span>+++ b/TODO-RELEASE</span><br><span>@@ -11,3 +11,5 @@</span><br><span> libosmocore struct gsm0808_diagnostics Depends on libosmocore > 1.3.0</span><br><span> libosmocore gsm0808_diagnostics_octet_location_str() Depends on libosmocore > 1.3.0</span><br><span> libosmocore gsm0808_diagnostics_bit_location_str() Depends on libosmocore > 1.3.0</span><br><span style="color: hsl(120, 100%, 40%);">+libosmocore osmo_mobile_identity Depends on libosmocore > 1.3.0</span><br><span style="color: hsl(120, 100%, 40%);">+osmo-bsc Mobile Identity Coding OsmoBSC is stricter in rejecting invalid coding of Mobile Identity IEs</span><br><span>diff --git a/include/osmocom/bsc/bsc_subscriber.h b/include/osmocom/bsc/bsc_subscriber.h</span><br><span>index 93b3539..53fa7be 100644</span><br><span>--- a/include/osmocom/bsc/bsc_subscriber.h</span><br><span>+++ b/include/osmocom/bsc/bsc_subscriber.h</span><br><span>@@ -6,6 +6,7 @@</span><br><span> </span><br><span> #include <osmocom/core/linuxlist.h></span><br><span> #include <osmocom/gsm/protocol/gsm_23_003.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/gsm/gsm48.h></span><br><span> </span><br><span> struct log_target;</span><br><span> </span><br><span>@@ -25,11 +26,13 @@</span><br><span> const char *imsi);</span><br><span> struct bsc_subscr *bsc_subscr_find_or_create_by_tmsi(struct llist_head *list,</span><br><span> uint32_t tmsi);</span><br><span style="color: hsl(120, 100%, 40%);">+struct bsc_subscr *bsc_subscr_find_or_create_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi);</span><br><span> </span><br><span> struct bsc_subscr *bsc_subscr_find_by_imsi(struct llist_head *list,</span><br><span> const char *imsi);</span><br><span> struct bsc_subscr *bsc_subscr_find_by_tmsi(struct llist_head *list,</span><br><span> uint32_t tmsi);</span><br><span style="color: hsl(120, 100%, 40%);">+struct bsc_subscr *bsc_subscr_find_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi);</span><br><span> </span><br><span> void bsc_subscr_set_imsi(struct bsc_subscr *bsub, const char *imsi);</span><br><span> </span><br><span>diff --git a/include/osmocom/bsc/gsm_04_08_rr.h b/include/osmocom/bsc/gsm_04_08_rr.h</span><br><span>index 06cefa9..d34e695 100644</span><br><span>--- a/include/osmocom/bsc/gsm_04_08_rr.h</span><br><span>+++ b/include/osmocom/bsc/gsm_04_08_rr.h</span><br><span>@@ -40,8 +40,6 @@</span><br><span> </span><br><span> struct msgb *gsm48_create_mm_serv_rej(enum gsm48_reject_value value);</span><br><span> int gsm48_extract_mi(uint8_t *classmark2_lv, int length, char *mi_string, uint8_t *mi_type);</span><br><span style="color: hsl(0, 100%, 40%);">-int gsm48_paging_extract_mi(struct gsm48_pag_resp *resp, int length,</span><br><span style="color: hsl(0, 100%, 40%);">- char *mi_string, uint8_t *mi_type);</span><br><span> struct msgb *gsm48_create_loc_upd_rej(uint8_t cause);</span><br><span> </span><br><span> struct msgb *gsm48_create_rr_status(uint8_t cause);</span><br><span>diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h</span><br><span>index be7c575..bc4c017 100644</span><br><span>--- a/include/osmocom/bsc/gsm_data.h</span><br><span>+++ b/include/osmocom/bsc/gsm_data.h</span><br><span>@@ -52,8 +52,6 @@</span><br><span> </span><br><span> #define OBSC_LINKID_CB(__msgb) (__msgb)->cb[3]</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-#define tmsi_from_string(str) strtoul(str, NULL, 10)</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* 3-bit long values */</span><br><span> #define EARFCN_PRIO_INVALID 8</span><br><span> #define EARFCN_MEAS_BW_INVALID 8</span><br><span>diff --git a/src/osmo-bsc/bsc_subscriber.c b/src/osmo-bsc/bsc_subscriber.c</span><br><span>index 38b532a..9ddfcaa 100644</span><br><span>--- a/src/osmo-bsc/bsc_subscriber.c</span><br><span>+++ b/src/osmo-bsc/bsc_subscriber.c</span><br><span>@@ -75,6 +75,20 @@</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct bsc_subscr *bsc_subscr_find_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!mi)</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (mi->type) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM_MI_TYPE_IMSI:</span><br><span style="color: hsl(120, 100%, 40%);">+ return bsc_subscr_find_by_imsi(list, mi->imsi);</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM_MI_TYPE_TMSI:</span><br><span style="color: hsl(120, 100%, 40%);">+ return bsc_subscr_find_by_tmsi(list, mi->tmsi);</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> void bsc_subscr_set_imsi(struct bsc_subscr *bsub, const char *imsi)</span><br><span> {</span><br><span> if (!bsub)</span><br><span>@@ -110,6 +124,20 @@</span><br><span> return bsc_subscr_get(bsub);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct bsc_subscr *bsc_subscr_find_or_create_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!mi)</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (mi->type) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM_MI_TYPE_IMSI:</span><br><span style="color: hsl(120, 100%, 40%);">+ return bsc_subscr_find_or_create_by_imsi(list, mi->imsi);</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM_MI_TYPE_TMSI:</span><br><span style="color: hsl(120, 100%, 40%);">+ return bsc_subscr_find_or_create_by_tmsi(list, mi->tmsi);</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> const char *bsc_subscr_name(struct bsc_subscr *bsub)</span><br><span> {</span><br><span> static char buf[32];</span><br><span>diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c</span><br><span>index 4e5a307..4630b47 100644</span><br><span>--- a/src/osmo-bsc/gsm_04_08_rr.c</span><br><span>+++ b/src/osmo-bsc/gsm_04_08_rr.c</span><br><span>@@ -842,16 +842,6 @@</span><br><span> return gsm48_mi_to_string(mi_string, GSM48_MI_SIZE, mi_lv+1, *mi_lv);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-int gsm48_paging_extract_mi(struct gsm48_pag_resp *resp, int length,</span><br><span style="color: hsl(0, 100%, 40%);">- char *mi_string, uint8_t *mi_type)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- static const uint32_t classmark_offset =</span><br><span style="color: hsl(0, 100%, 40%);">- offsetof(struct gsm48_pag_resp, classmark2);</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t *classmark2_lv = (uint8_t *) &resp->classmark2;</span><br><span style="color: hsl(0, 100%, 40%);">- return gsm48_extract_mi(classmark2_lv, length - classmark_offset,</span><br><span style="color: hsl(0, 100%, 40%);">- mi_string, mi_type);</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* As per TS 03.03 Section 2.2, the IMSI has 'not more than 15 digits' */</span><br><span> uint64_t str_to_imsi(const char *imsi_str)</span><br><span> {</span><br><span>diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c</span><br><span>index a252203..7f54fe6 100644</span><br><span>--- a/src/osmo-bsc/gsm_08_08.c</span><br><span>+++ b/src/osmo-bsc/gsm_08_08.c</span><br><span>@@ -122,46 +122,6 @@</span><br><span> return cm->cm_service_type == GSM48_CMSERV_EMERGENCY;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* extract a subscriber from the paging response */</span><br><span style="color: hsl(0, 100%, 40%);">-static struct bsc_subscr *extract_sub(struct gsm_subscriber_connection *conn,</span><br><span style="color: hsl(0, 100%, 40%);">- struct msgb *msg)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t mi_type;</span><br><span style="color: hsl(0, 100%, 40%);">- char mi_string[GSM48_MI_SIZE];</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm48_hdr *gh;</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm48_pag_resp *resp;</span><br><span style="color: hsl(0, 100%, 40%);">- struct bsc_subscr *subscr;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (msgb_l3len(msg) < sizeof(*gh) + sizeof(*resp)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMSC, LOGL_ERROR, "PagingResponse too small: %u\n", msgb_l3len(msg));</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- gh = msgb_l3(msg);</span><br><span style="color: hsl(0, 100%, 40%);">- resp = (struct gsm48_pag_resp *) &gh->data[0];</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- gsm48_paging_extract_mi(resp, msgb_l3len(msg) - sizeof(*gh),</span><br><span style="color: hsl(0, 100%, 40%);">- mi_string, &mi_type);</span><br><span style="color: hsl(0, 100%, 40%);">- DEBUGP(DRR, "PAGING RESPONSE: MI(%s)=%s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- gsm48_mi_type_name(mi_type), mi_string);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- switch (mi_type) {</span><br><span style="color: hsl(0, 100%, 40%);">- case GSM_MI_TYPE_TMSI:</span><br><span style="color: hsl(0, 100%, 40%);">- subscr = bsc_subscr_find_by_tmsi(conn->network->bsc_subscribers,</span><br><span style="color: hsl(0, 100%, 40%);">- tmsi_from_string(mi_string));</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case GSM_MI_TYPE_IMSI:</span><br><span style="color: hsl(0, 100%, 40%);">- subscr = bsc_subscr_find_by_imsi(conn->network->bsc_subscribers,</span><br><span style="color: hsl(0, 100%, 40%);">- mi_string);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- default:</span><br><span style="color: hsl(0, 100%, 40%);">- subscr = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- return subscr;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static bool is_msc_usable(struct bsc_msc_data *msc, bool is_emerg)</span><br><span> {</span><br><span> if (is_emerg && !msc->allow_emerg)</span><br><span>@@ -184,6 +144,7 @@</span><br><span> struct gsm48_hdr *gh;</span><br><span> int8_t pdisc;</span><br><span> uint8_t mtype;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_mobile_identity mi;</span><br><span> struct bsc_msc_data *msc;</span><br><span> struct bsc_msc_data *msc_target = NULL;</span><br><span> struct bsc_msc_data *msc_round_robin_next = NULL;</span><br><span>@@ -203,9 +164,19 @@</span><br><span> </span><br><span> is_emerg = (pdisc == GSM48_PDISC_MM && mtype == GSM48_MT_MM_CM_SERV_REQ) && is_cm_service_for_emerg(msg);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_mobile_identity_decode_from_l3(&mi, msg, false)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_COMPL_L3(pdisc, mtype, LOGL_ERROR, "Cannot extract Mobile Identity: %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_hexdump_c(OTC_SELECT, msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ /* There is no Mobile Identity to pick a matching MSC from. Likely this is an invalid Complete Layer 3</span><br><span style="color: hsl(120, 100%, 40%);">+ * message that deserves to be rejected. However, the current state of our ttcn3 tests does send invalid</span><br><span style="color: hsl(120, 100%, 40%);">+ * Layer 3 Info in some tests and expects osmo-bsc to not care about that. So, changing the behavior to</span><br><span style="color: hsl(120, 100%, 40%);">+ * rejecting on missing MI causes test failure and, if at all, should happen in a separate patch.</span><br><span style="color: hsl(120, 100%, 40%);">+ * See e.g. BSC_Tests.TC_chan_rel_rll_rel_ind: "dt := f_est_dchan('23'O, 23, '00010203040506'O);" */</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Has the subscriber been paged from a connected MSC? */</span><br><span> if (pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP) {</span><br><span style="color: hsl(0, 100%, 40%);">- subscr = extract_sub(conn, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi);</span><br><span> if (subscr) {</span><br><span> msc_target = paging_get_msc(conn_get_bts(conn), subscr);</span><br><span> bsc_subscr_put(subscr);</span><br><span>@@ -241,8 +212,8 @@</span><br><span> * them are usable -- wrap to the start. */</span><br><span> msc_target = msc_round_robin_next ? : msc_round_robin_first;</span><br><span> if (!msc_target) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_COMPL_L3(pdisc, mtype, LOGL_ERROR, "%sNo suitable MSC for this Complete Layer 3 request found\n",</span><br><span style="color: hsl(0, 100%, 40%);">- is_emerg ? "FOR EMERGENCY CALL: " : "");</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_COMPL_L3(pdisc, mtype, LOGL_ERROR, "%s%s: No suitable MSC for this Complete Layer 3 request found\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), is_emerg ? " FOR EMERGENCY CALL" : "");</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span>@@ -256,8 +227,15 @@</span><br><span> </span><br><span> static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb *msg)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct bsc_subscr *subscr = extract_sub(conn, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_mobile_identity mi;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bsc_subscr *subscr = NULL;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_mobile_identity_decode_from_l3(&mi, msg, false)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DRSL, LOGL_ERROR, "Unable to extract Mobile Identity from Paging Response\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi);</span><br><span> if (!subscr) {</span><br><span> LOGP(DMSC, LOGL_ERROR, "Non active subscriber got paged.\n");</span><br><span> rate_ctr_inc(&conn->lchan->ts->trx->bts->bts_ctrs->ctr[BTS_CTR_PAGING_NO_ACTIVE_PAGING]);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/18712">change 18712</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bsc/+/18712"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Id6cccaac64392b737b3bba8f3a22a88009adb23b </div>
<div style="display:none"> Gerrit-Change-Number: 18712 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: daniel <daniel@totalueberwachung.de> </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>