laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/20341 )
Change subject: compl l3: parse Mobile Identity once ...................................................................... compl l3: parse Mobile Identity once Move two calls of osmo_mobile_identity_decode_from_l3() from bsc_find_msc() and handle_page_resp() out into a single call in bsc_compl_l3(). Prepares cosmetically for upcoming LCS patch. Change-Id: I26950b63621417da0ed3125d0dc0b06cf015cb4a --- M src/osmo-bsc/gsm_08_08.c 1 file changed, 27 insertions(+), 28 deletions(-) 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 diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c index 9dc54d1..c37b57f 100644 --- a/src/osmo-bsc/gsm_08_08.c +++ b/src/osmo-bsc/gsm_08_08.c @@ -184,13 +184,12 @@ * c) All other cases distribute the messages across connected MSCs in a round-robin fashion. */ static struct bsc_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn, - struct msgb *msg) + struct msgb *msg, const struct osmo_mobile_identity *mi) { struct gsm_network *net = conn->network; 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; @@ -213,44 +212,34 @@ 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? */ bts = conn_get_bts(conn); if (bts && pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP) { - subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi); + subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, mi); if (subscr) { msc_target = paging_get_msc(bts, subscr); bsc_subscr_put(subscr); if (is_msc_usable(msc_target, is_emerg)) { LOG_COMPL_L3(pdisc, mtype, LOGL_DEBUG, "%s matches earlier Paging from msc %d\n", - osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), msc_target->nr); + osmo_mobile_identity_to_str_c(OTC_SELECT, mi), msc_target->nr); rate_ctr_inc(&msc_target->msc_ctrs->ctr[MSC_CTR_MSCPOOL_SUBSCR_PAGED]); return msc_target; } else { LOG_COMPL_L3(pdisc, mtype, LOGL_DEBUG, "%s matches earlier Paging from msc %d, but this MSC is not connected\n", - osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), msc_target->nr); + osmo_mobile_identity_to_str_c(OTC_SELECT, mi), msc_target->nr); } msc_target = NULL; } } #define LOG_NRI(LOGLEVEL, FORMAT, ARGS...) \ - LOGP(DMSC, LOGLEVEL, "%s NRI(%d)=0x%x=%d: " FORMAT, osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), \ + LOGP(DMSC, LOGLEVEL, "%s NRI(%d)=0x%x=%d: " FORMAT, osmo_mobile_identity_to_str_c(OTC_SELECT, mi), \ net->nri_bitlen, nri_v, nri_v, ##ARGS) /* Extract NRI bits from TMSI, possibly indicating which MSC is responsible */ - if (mi.type == GSM_MI_TYPE_TMSI) { - if (osmo_tmsi_nri_v_get(&nri_v, mi.tmsi, net->nri_bitlen)) { + if (mi->type == GSM_MI_TYPE_TMSI) { + if (osmo_tmsi_nri_v_get(&nri_v, mi->tmsi, net->nri_bitlen)) { LOGP(DMSC, LOGL_ERROR, "Unable to retrieve NRI from TMSI, nri_bitlen == %u\n", net->nri_bitlen); nri_v = -1; } else if (is_lu_from_other_plmn(msg)) { @@ -328,7 +317,7 @@ msc_target = msc_round_robin_next ? : msc_round_robin_first; if (!msc_target) { 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" : ""); + osmo_mobile_identity_to_str_c(OTC_SELECT, mi), is_emerg ? " FOR EMERGENCY CALL" : ""); rate_ctr_inc(&bsc_gsmnet->bsc_ctrs->ctr[BSC_CTR_MSCPOOL_SUBSCR_NO_MSC]); if (is_emerg) rate_ctr_inc(&bsc_gsmnet->bsc_ctrs->ctr[BSC_CTR_MSCPOOL_EMERG_LOST]); @@ -336,7 +325,7 @@ } LOGP(DMSC, LOGL_DEBUG, "New subscriber %s: MSC round-robin selects msc %d\n", - osmo_mobile_identity_to_str_c(OTC_SELECT, &mi), msc_target->nr); + osmo_mobile_identity_to_str_c(OTC_SELECT, mi), msc_target->nr); if (is_null_nri) rate_ctr_inc(&msc_target->msc_ctrs->ctr[MSC_CTR_MSCPOOL_SUBSCR_REATTACH]); @@ -357,9 +346,9 @@ #undef LOG_NRI } -static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb *msg) +static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb *msg, + const struct osmo_mobile_identity *mi) { - struct osmo_mobile_identity mi; struct bsc_subscr *subscr = NULL; struct gsm_bts *bts = conn_get_bts(conn); @@ -369,12 +358,10 @@ return -1; } - if (osmo_mobile_identity_decode_from_l3(&mi, msg, false)) { - LOGP(DRSL, LOGL_ERROR, "Unable to extract Mobile Identity from Paging Response\n"); + if (mi->type == GSM_MI_TYPE_NONE) return -1; - } - subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi); + 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]); @@ -449,6 +436,7 @@ int rc = -2; struct gsm_bts *bts; struct osmo_cell_global_id *cgi; + struct osmo_mobile_identity mi; struct gsm48_hdr *gh; uint8_t pdisc, mtype; @@ -461,12 +449,23 @@ pdisc = gsm48_hdr_pdisc(gh); mtype = gsm48_hdr_msg_type(gh); + 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)); + /* 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);" + */ + } + log_set_context(LOG_CTX_BSC_SUBSCR, conn->bsub); LOGP(DMSC, LOGL_INFO, "Tx MSC COMPL L3\n"); /* find the MSC link we want to use */ - msc = bsc_find_msc(conn, msg); + msc = bsc_find_msc(conn, msg, &mi); if (!msc) { LOGP(DMSC, LOGL_ERROR, "Failed to find a MSC for a connection.\n"); rc = -1; @@ -489,7 +488,7 @@ parse_powercap(conn, msg); if (pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP) - handle_page_resp(conn, msg); + handle_page_resp(conn, msg, &mi); if (gscon_is_aoip(conn)) { gen_bss_supported_codec_list(&scl, msc, bts); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/20341 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I26950b63621417da0ed3125d0dc0b06cf015cb4a Gerrit-Change-Number: 20341 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Vadim Yanitskiy <vyanits...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-MessageType: merged