<p>osmith <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/13515">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
Vadim Yanitskiy: Looks good to me, but someone else must approve
Neels Hofmeyr: Looks good to me, but someone else must approve
osmith: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">USSD: save MO USSD's originating MSC's vlr_number<br><br>Save the source IPA name in ss_session, so we can send "invalid IMSI"<br>messages to the originating MSC.<br><br>Remove the fixed size from ss->vlr_number (we don't know the size of the<br>IPA name when it does not come from the database). Add<br>ss->vlr_number_len to give osmo_gsup_addr_send() the format it expects,<br>and to have one less place in the code where the IPA names are not<br>stored as blob.<br><br>Looking up the IPA name from struct osmo_gsup_conn could either be done<br>like in osmo_gsup_server_ccm_cb() by reading the IPA IEs (which has a<br>FIXME comment), or by finding the route associated with conn. I went<br>with the latter approach, because it seems cleaner to me.<br><br>Related: OS#3710<br>Change-Id: If5a65f471672949192061c5fe396603611123bc1<br>---<br>M src/gsup_router.c<br>M src/gsup_router.h<br>M src/hlr_ussd.c<br>3 files changed, 53 insertions(+), 14 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gsup_router.c b/src/gsup_router.c</span><br><span>index 4fedd38..df978ba 100644</span><br><span>--- a/src/gsup_router.c</span><br><span>+++ b/src/gsup_router.c</span><br><span>@@ -25,13 +25,7 @@</span><br><span> </span><br><span> #include "logging.h"</span><br><span> #include "gsup_server.h"</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-struct gsup_route {</span><br><span style="color: hsl(0, 100%, 40%);">- struct llist_head list;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t *addr;</span><br><span style="color: hsl(0, 100%, 40%);">- struct osmo_gsup_conn *conn;</span><br><span style="color: hsl(0, 100%, 40%);">-};</span><br><span style="color: hsl(120, 100%, 40%);">+#include "gsup_router.h"</span><br><span> </span><br><span> /*! Find a route for the given address.</span><br><span> * \param[in] gs gsup server</span><br><span>@@ -53,6 +47,22 @@</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! Find a GSUP connection's route (to read the IPA address from the route).</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] conn GSUP connection</span><br><span style="color: hsl(120, 100%, 40%);">+ * \return GSUP route</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+struct gsup_route *gsup_route_find_by_conn(const struct osmo_gsup_conn *conn)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsup_route *gr;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_for_each_entry(gr, &conn->server->routes, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gr->conn == conn)</span><br><span style="color: hsl(120, 100%, 40%);">+ return gr;</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%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* add a new route for the given address to the given conn */</span><br><span> int gsup_route_add(struct osmo_gsup_conn *conn, const uint8_t *addr, size_t addrlen)</span><br><span> {</span><br><span>diff --git a/src/gsup_router.h b/src/gsup_router.h</span><br><span>index 282531d..bff484e 100644</span><br><span>--- a/src/gsup_router.h</span><br><span>+++ b/src/gsup_router.h</span><br><span>@@ -3,9 +3,18 @@</span><br><span> #include <stdint.h></span><br><span> #include "gsup_server.h"</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct gsup_route {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct llist_head list;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *addr;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_gsup_conn *conn;</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> struct osmo_gsup_conn *gsup_route_find(struct osmo_gsup_server *gs,</span><br><span> const uint8_t *addr, size_t addrlen);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct gsup_route *gsup_route_find_by_conn(const struct osmo_gsup_conn *conn);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* add a new route for the given address to the given conn */</span><br><span> int gsup_route_add(struct osmo_gsup_conn *conn, const uint8_t *addr, size_t addrlen);</span><br><span> </span><br><span>diff --git a/src/hlr_ussd.c b/src/hlr_ussd.c</span><br><span>index ea69cd9..56a5a95 100644</span><br><span>--- a/src/hlr_ussd.c</span><br><span>+++ b/src/hlr_ussd.c</span><br><span>@@ -167,8 +167,11 @@</span><br><span> const struct hlr_iuse *iuse;</span><br><span> } u;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* subscriber's vlr_number, will be looked up once per session and cached here */</span><br><span style="color: hsl(0, 100%, 40%);">- char vlr_number[32];</span><br><span style="color: hsl(120, 100%, 40%);">+ /* subscriber's vlr_number</span><br><span style="color: hsl(120, 100%, 40%);">+ * MO USSD: originating MSC's vlr_number</span><br><span style="color: hsl(120, 100%, 40%);">+ * MT USSD: looked up once per session and cached here */</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *vlr_number;</span><br><span style="color: hsl(120, 100%, 40%);">+ size_t vlr_number_len;</span><br><span> </span><br><span> /* we don't keep a pointer to the osmo_gsup_{route,conn} towards the MSC/VLR here,</span><br><span> * as this might change during inter-VLR hand-over, and we simply look-up the serving MSC/VLR</span><br><span>@@ -233,24 +236,26 @@</span><br><span> int rc;</span><br><span> </span><br><span> /* Use vlr_number as looked up by the caller, or look up now. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!ss->vlr_number[0]) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ss->vlr_number) {</span><br><span> rc = db_subscr_get_by_imsi(g_hlr->dbc, ss->imsi, &subscr);</span><br><span> if (rc < 0) {</span><br><span> LOGPSS(ss, LOGL_ERROR, "Cannot find subscriber, cannot route GSUP message\n");</span><br><span> msgb_free(msg);</span><br><span> return -EINVAL;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_strlcpy(ss->vlr_number, subscr.vlr_number, sizeof(ss->vlr_number));</span><br><span style="color: hsl(120, 100%, 40%);">+ ss->vlr_number = (uint8_t *)talloc_strdup(ss, subscr.vlr_number);</span><br><span style="color: hsl(120, 100%, 40%);">+ ss->vlr_number_len = strlen(subscr.vlr_number) + 1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!ss->vlr_number[0]) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Check for empty string (all vlr_number strings end in "\0", because otherwise gsup_route_find() fails) */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ss->vlr_number_len == 1) {</span><br><span> LOGPSS(ss, LOGL_ERROR, "Cannot send GSUP message, no VLR number stored for subscriber\n");</span><br><span> msgb_free(msg);</span><br><span> return -EINVAL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- LOGPSS(ss, LOGL_DEBUG, "Tx SS/USSD to VLR '%s'\n", ss->vlr_number);</span><br><span style="color: hsl(0, 100%, 40%);">- return osmo_gsup_addr_send(gs, (uint8_t *)ss->vlr_number, strlen(ss->vlr_number) + 1, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPSS(ss, LOGL_DEBUG, "Tx SS/USSD to VLR %s\n", osmo_quote_str((char *)ss->vlr_number, ss->vlr_number_len));</span><br><span style="color: hsl(120, 100%, 40%);">+ return osmo_gsup_addr_send(gs, ss->vlr_number, ss->vlr_number_len, msg);</span><br><span> }</span><br><span> </span><br><span> static int ss_tx_to_ms(struct ss_session *ss, enum osmo_gsup_message_type gsup_msg_type,</span><br><span>@@ -500,6 +505,7 @@</span><br><span> struct hlr *hlr = conn->server->priv;</span><br><span> struct ss_session *ss;</span><br><span> struct ss_request req = {0};</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsup_route *gsup_rt;</span><br><span> </span><br><span> LOGP(DSS, LOGL_DEBUG, "%s/0x%08x: Process SS (%s)\n", gsup->imsi, gsup->session_id,</span><br><span> osmo_gsup_session_state_name(gsup->session_state));</span><br><span>@@ -529,6 +535,20 @@</span><br><span> gsup->imsi, gsup->session_id);</span><br><span> goto out_err;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Get IPA name from VLR conn and save as ss->vlr_number */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!conn_is_euse(conn)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ gsup_rt = gsup_route_find_by_conn(conn);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gsup_rt) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ss->vlr_number = (uint8_t *)talloc_strdup(ss, (const char *)gsup_rt->addr);</span><br><span style="color: hsl(120, 100%, 40%);">+ ss->vlr_number_len = strlen((const char *)gsup_rt->addr) + 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPSS(ss, LOGL_DEBUG, "Destination IPA name retrieved from GSUP route: %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_quote_str((const char *)ss->vlr_number, ss->vlr_number_len));</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPSS(ss, LOGL_NOTICE, "Could not find GSUP route, therefore can't set the destination"</span><br><span style="color: hsl(120, 100%, 40%);">+ " IPA name. We'll try to look it up later, but this should not"</span><br><span style="color: hsl(120, 100%, 40%);">+ " have happened.\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> if (ss_op_is_ussd(req.opcode)) {</span><br><span> if (conn_is_euse(conn)) {</span><br><span> /* EUSE->VLR: MT USSD. EUSE is known ('conn'), VLR is to be resolved */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13515">change 13515</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/13515"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-hlr </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: If5a65f471672949192061c5fe396603611123bc1 </div>
<div style="display:none"> Gerrit-Change-Number: 13515 </div>
<div style="display:none"> Gerrit-PatchSet: 12 </div>
<div style="display:none"> Gerrit-Owner: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>