<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/20341">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, approved
Vadim Yanitskiy: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">compl l3: parse Mobile Identity once<br><br>Move two calls of osmo_mobile_identity_decode_from_l3() from bsc_find_msc() and<br>handle_page_resp() out into a single call in bsc_compl_l3().<br><br>Prepares cosmetically for upcoming LCS patch.<br><br>Change-Id: I26950b63621417da0ed3125d0dc0b06cf015cb4a<br>---<br>M src/osmo-bsc/gsm_08_08.c<br>1 file changed, 27 insertions(+), 28 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c</span><br><span>index 9dc54d1..c37b57f 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>@@ -184,13 +184,12 @@</span><br><span> * c) All other cases distribute the messages across connected MSCs in a round-robin fashion.</span><br><span> */</span><br><span> static struct bsc_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn,</span><br><span style="color: hsl(0, 100%, 40%);">- struct msgb *msg)</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *msg, const struct osmo_mobile_identity *mi)</span><br><span> {</span><br><span> struct gsm_network *net = conn->network;</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(0, 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>@@ -213,44 +212,34 @@</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(0, 100%, 40%);">- if (osmo_mobile_identity_decode_from_l3(&mi, msg, false)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_COMPL_L3(pdisc, mtype, LOGL_ERROR, "Cannot extract Mobile Identity: %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_hexdump_c(OTC_SELECT, msg));</span><br><span style="color: hsl(0, 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(0, 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(0, 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(0, 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(0, 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(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* Has the subscriber been paged from a connected MSC? */</span><br><span> bts = conn_get_bts(conn);</span><br><span> if (bts && pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP) {</span><br><span style="color: hsl(0, 100%, 40%);">- subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi);</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(bts, subscr);</span><br><span> bsc_subscr_put(subscr);</span><br><span> if (is_msc_usable(msc_target, is_emerg)) {</span><br><span> LOG_COMPL_L3(pdisc, mtype, LOGL_DEBUG, "%s matches earlier Paging from msc %d\n",</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), msc_target->nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_mobile_identity_to_str_c(OTC_SELECT, mi), msc_target->nr);</span><br><span> rate_ctr_inc(&msc_target->msc_ctrs->ctr[MSC_CTR_MSCPOOL_SUBSCR_PAGED]);</span><br><span> return msc_target;</span><br><span> } else {</span><br><span> LOG_COMPL_L3(pdisc, mtype, LOGL_DEBUG,</span><br><span> "%s matches earlier Paging from msc %d, but this MSC is not connected\n",</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), msc_target->nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_mobile_identity_to_str_c(OTC_SELECT, mi), msc_target->nr);</span><br><span> }</span><br><span> msc_target = NULL;</span><br><span> }</span><br><span> }</span><br><span> </span><br><span> #define LOG_NRI(LOGLEVEL, FORMAT, ARGS...) \</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMSC, LOGLEVEL, "%s NRI(%d)=0x%x=%d: " FORMAT, osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), \</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DMSC, LOGLEVEL, "%s NRI(%d)=0x%x=%d: " FORMAT, osmo_mobile_identity_to_str_c(OTC_SELECT, mi), \</span><br><span> net->nri_bitlen, nri_v, nri_v, ##ARGS)</span><br><span> </span><br><span> /* Extract NRI bits from TMSI, possibly indicating which MSC is responsible */</span><br><span style="color: hsl(0, 100%, 40%);">- if (mi.type == GSM_MI_TYPE_TMSI) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (osmo_tmsi_nri_v_get(&nri_v, mi.tmsi, net->nri_bitlen)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (mi->type == GSM_MI_TYPE_TMSI) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_tmsi_nri_v_get(&nri_v, mi->tmsi, net->nri_bitlen)) {</span><br><span> LOGP(DMSC, LOGL_ERROR, "Unable to retrieve NRI from TMSI, nri_bitlen == %u\n", net->nri_bitlen);</span><br><span> nri_v = -1;</span><br><span> } else if (is_lu_from_other_plmn(msg)) {</span><br><span>@@ -328,7 +317,7 @@</span><br><span> msc_target = msc_round_robin_next ? : msc_round_robin_first;</span><br><span> if (!msc_target) {</span><br><span> 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(0, 100%, 40%);">- osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), is_emerg ? " FOR EMERGENCY CALL" : "");</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> rate_ctr_inc(&bsc_gsmnet->bsc_ctrs->ctr[BSC_CTR_MSCPOOL_SUBSCR_NO_MSC]);</span><br><span> if (is_emerg)</span><br><span> rate_ctr_inc(&bsc_gsmnet->bsc_ctrs->ctr[BSC_CTR_MSCPOOL_EMERG_LOST]);</span><br><span>@@ -336,7 +325,7 @@</span><br><span> }</span><br><span> </span><br><span> LOGP(DMSC, LOGL_DEBUG, "New subscriber %s: MSC round-robin selects msc %d\n",</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), msc_target->nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_mobile_identity_to_str_c(OTC_SELECT, mi), msc_target->nr);</span><br><span> </span><br><span> if (is_null_nri)</span><br><span> rate_ctr_inc(&msc_target->msc_ctrs->ctr[MSC_CTR_MSCPOOL_SUBSCR_REATTACH]);</span><br><span>@@ -357,9 +346,9 @@</span><br><span> #undef LOG_NRI</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb *msg)</span><br><span style="color: hsl(120, 100%, 40%);">+static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb *msg,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct osmo_mobile_identity *mi)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct osmo_mobile_identity mi;</span><br><span> struct bsc_subscr *subscr = NULL;</span><br><span> struct gsm_bts *bts = conn_get_bts(conn);</span><br><span> </span><br><span>@@ -369,12 +358,10 @@</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (osmo_mobile_identity_decode_from_l3(&mi, msg, false)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DRSL, LOGL_ERROR, "Unable to extract Mobile Identity from Paging Response\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ if (mi->type == GSM_MI_TYPE_NONE)</span><br><span> return -1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi);</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>@@ -449,6 +436,7 @@</span><br><span> int rc = -2;</span><br><span> struct gsm_bts *bts;</span><br><span> struct osmo_cell_global_id *cgi;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_mobile_identity mi;</span><br><span> struct gsm48_hdr *gh;</span><br><span> uint8_t pdisc, mtype;</span><br><span> </span><br><span>@@ -461,12 +449,23 @@</span><br><span> pdisc = gsm48_hdr_pdisc(gh);</span><br><span> mtype = gsm48_hdr_msg_type(gh);</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%);">+ /* Likely this is an invalid Complete Layer 3 message that deserves to be rejected. However, the current</span><br><span style="color: hsl(120, 100%, 40%);">+ * state of our ttcn3 tests does send invalid Layer 3 Info in some tests and expects osmo-bsc to not</span><br><span style="color: hsl(120, 100%, 40%);">+ * care about that. So, changing the behavior to rejecting on missing MI causes test failure and, if at</span><br><span style="color: hsl(120, 100%, 40%);">+ * 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 style="color: hsl(120, 100%, 40%);">+</span><br><span> log_set_context(LOG_CTX_BSC_SUBSCR, conn->bsub);</span><br><span> </span><br><span> LOGP(DMSC, LOGL_INFO, "Tx MSC COMPL L3\n");</span><br><span> </span><br><span> /* find the MSC link we want to use */</span><br><span style="color: hsl(0, 100%, 40%);">- msc = bsc_find_msc(conn, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ msc = bsc_find_msc(conn, msg, &mi);</span><br><span> if (!msc) {</span><br><span> LOGP(DMSC, LOGL_ERROR, "Failed to find a MSC for a connection.\n");</span><br><span> rc = -1;</span><br><span>@@ -489,7 +488,7 @@</span><br><span> </span><br><span> parse_powercap(conn, msg);</span><br><span> if (pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP)</span><br><span style="color: hsl(0, 100%, 40%);">- handle_page_resp(conn, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ handle_page_resp(conn, msg, &mi);</span><br><span> </span><br><span> if (gscon_is_aoip(conn)) {</span><br><span> gen_bss_supported_codec_list(&scl, msc, bts);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/20341">change 20341</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/+/20341"/><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: I26950b63621417da0ed3125d0dc0b06cf015cb4a </div>
<div style="display:none"> Gerrit-Change-Number: 20341 </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: Vadim Yanitskiy <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>