<p>Stefan Sperling <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/12098">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">introduce osmo_gsup_client_create2()<br><br>Add a new API which allows creating a GSUP client connection with<br>more identification information than just a unit name. Instead of<br>being selective about which idenfifiers callers may use, allow<br>callers to pass a full-blown struct ipaccess_unit. This allows<br>applications to use entirely custom identifiers on GSUP client<br>connections.<br><br>This change is a prerequisite for inter-MSC handover because MSCs<br>will need to use unique identifiers towards the HLR, which isn't<br>very easy to do with the old osmo_gsup_client_create() API. While<br>it's always been possible to pass a unique unit_name, this is not<br>as flexible as we would like.<br><br>The old API remains for backwards compatibility.<br>struct osmo_gsup_client grows in size but is allocated internally<br>by the library; old calling code won't notice the difference.<br><br>Change-Id: Ief09677e07d6e977247185b72c605f109aa091f5<br>Related: OS#3355<br>---<br>M include/osmocom/gsupclient/gsup_client.h<br>M src/gsupclient/gsup_client.c<br>2 files changed, 49 insertions(+), 19 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/gsupclient/gsup_client.h b/include/osmocom/gsupclient/gsup_client.h</span><br><span>index 981751b..95163cd 100644</span><br><span>--- a/include/osmocom/gsupclient/gsup_client.h</span><br><span>+++ b/include/osmocom/gsupclient/gsup_client.h</span><br><span>@@ -23,6 +23,7 @@</span><br><span> </span><br><span> #include <osmocom/core/timer.h></span><br><span> #include <osmocom/gsm/oap_client.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/gsm/ipa.h></span><br><span> </span><br><span> /* a loss of GSUP between MSC and HLR is considered quite serious, let's try to recover as quickly as</span><br><span>  * possible.  Even one new connection attempt per second should be quite acceptable until the link is</span><br><span>@@ -38,7 +39,7 @@</span><br><span> typedef int (*osmo_gsup_client_read_cb_t)(struct osmo_gsup_client *gsupc, struct msgb *msg);</span><br><span> </span><br><span> struct osmo_gsup_client {</span><br><span style="color: hsl(0, 100%, 40%);">-  const char *unit_name;</span><br><span style="color: hsl(120, 100%, 40%);">+        const char *unit_name; /* same as ipa_dev->unit_name, for backwards compat */</span><br><span> </span><br><span>         struct ipa_client_conn *link;</span><br><span>        osmo_gsup_client_read_cb_t read_cb;</span><br><span>@@ -50,8 +51,16 @@</span><br><span>     struct osmo_timer_list connect_timer;</span><br><span>        int is_connected;</span><br><span>    int got_ipa_pong;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   struct ipaccess_unit *ipa_dev; /* identification information sent to IPA server */</span><br><span> };</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct osmo_gsup_client *osmo_gsup_client_create2(void *talloc_ctx,</span><br><span style="color: hsl(120, 100%, 40%);">+                                               struct ipaccess_unit *ipa_dev,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                const char *ip_addr,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                  unsigned int tcp_port,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                osmo_gsup_client_read_cb_t read_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+                                           struct osmo_oap_client_config *oapc_config);</span><br><span> struct osmo_gsup_client *osmo_gsup_client_create(void *talloc_ctx,</span><br><span>                                                  const char *unit_name,</span><br><span>                                               const char *ip_addr,</span><br><span>diff --git a/src/gsupclient/gsup_client.c b/src/gsupclient/gsup_client.c</span><br><span>index d34a22d..f259bdc 100644</span><br><span>--- a/src/gsupclient/gsup_client.c</span><br><span>+++ b/src/gsupclient/gsup_client.c</span><br><span>@@ -170,16 +170,12 @@</span><br><span>   struct ipaccess_head_ext *he = (struct ipaccess_head_ext *) msgb_l2(msg);</span><br><span>    struct osmo_gsup_client *gsupc = (struct osmo_gsup_client *)link->data;</span><br><span>   int rc;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ipaccess_unit ipa_dev = {</span><br><span style="color: hsl(0, 100%, 40%);">-                /* see gsup_client_create() on const vs non-const */</span><br><span style="color: hsl(0, 100%, 40%);">-            .unit_name = (char*)gsupc->unit_name,</span><br><span style="color: hsl(0, 100%, 40%);">-        };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  OSMO_ASSERT(ipa_dev.unit_name);</span><br><span style="color: hsl(120, 100%, 40%);">+       OSMO_ASSERT(gsupc->unit_name);</span><br><span> </span><br><span>        msg->l2h = &hh->data[0];</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  rc = ipaccess_bts_handle_ccm(link, &ipa_dev, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+        rc = ipaccess_bts_handle_ccm(link, gsupc->ipa_dev, msg);</span><br><span> </span><br><span>      if (rc < 0) {</span><br><span>             LOGP(DLGSUP, LOGL_NOTICE,</span><br><span>@@ -262,24 +258,33 @@</span><br><span>    gsup_client_send_ping(gsupc);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-struct osmo_gsup_client *osmo_gsup_client_create(void *talloc_ctx,</span><br><span style="color: hsl(0, 100%, 40%);">-                                                 const char *unit_name,</span><br><span style="color: hsl(0, 100%, 40%);">-                                          const char *ip_addr,</span><br><span style="color: hsl(0, 100%, 40%);">-                                            unsigned int tcp_port,</span><br><span style="color: hsl(0, 100%, 40%);">-                                          osmo_gsup_client_read_cb_t read_cb,</span><br><span style="color: hsl(0, 100%, 40%);">-                                             struct osmo_oap_client_config *oapc_config)</span><br><span style="color: hsl(120, 100%, 40%);">+/*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * Create a gsup client connecting to the specified IP address and TCP port.</span><br><span style="color: hsl(120, 100%, 40%);">+ * Use the provided ipaccess unit as the client-side identifier; ipa_dev should</span><br><span style="color: hsl(120, 100%, 40%);">+ * be allocated in talloc_ctx talloc_ctx as well.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] talloc_ctx talloc context.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] ipa_dev IP access unit which contains client identification information; must be allocated</span><br><span style="color: hsl(120, 100%, 40%);">+ *                    in talloc_ctx as well to ensure it lives throughout the lifetime of the connection.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] ip_addr GSUP server IP address.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] tcp_port GSUP server TCP port.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] read_cb callback for reading from the GSUP connection.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] oapc_config OPA client configuration.</span><br><span style="color: hsl(120, 100%, 40%);">+ *  \returns a GSUP client connection or NULL on failure.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+struct osmo_gsup_client *osmo_gsup_client_create2(void *talloc_ctx,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                 struct ipaccess_unit *ipa_dev,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                const char *ip_addr,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                  unsigned int tcp_port,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                osmo_gsup_client_read_cb_t read_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+                                           struct osmo_oap_client_config *oapc_config)</span><br><span> {</span><br><span>   struct osmo_gsup_client *gsupc;</span><br><span>      int rc;</span><br><span> </span><br><span>  gsupc = talloc_zero(talloc_ctx, struct osmo_gsup_client);</span><br><span>    OSMO_ASSERT(gsupc);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-     /* struct ipaccess_unit has a non-const unit_name, so let's copy to be</span><br><span style="color: hsl(0, 100%, 40%);">-       * able to have a non-const unit_name here as well. To not taint the</span><br><span style="color: hsl(0, 100%, 40%);">-     * public gsup_client API, let's store it in a const char* anyway. */</span><br><span style="color: hsl(0, 100%, 40%);">-       gsupc->unit_name = talloc_strdup(gsupc, unit_name);</span><br><span style="color: hsl(0, 100%, 40%);">-  OSMO_ASSERT(gsupc->unit_name);</span><br><span style="color: hsl(120, 100%, 40%);">+     gsupc->unit_name = (const char *)ipa_dev->unit_name; /* API backwards compat */</span><br><span style="color: hsl(120, 100%, 40%);">+ gsupc->ipa_dev = ipa_dev;</span><br><span> </span><br><span>     /* a NULL oapc_config will mark oap_state disabled. */</span><br><span>       rc = osmo_oap_client_init(oapc_config, &gsupc->oap_state);</span><br><span>@@ -313,6 +318,22 @@</span><br><span>     return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/**</span><br><span style="color: hsl(120, 100%, 40%);">+ * Like osmo_gsup_client_create2() except it expects a unit name instead</span><br><span style="color: hsl(120, 100%, 40%);">+ * of a full-blown ipacess_unit as the client-side identifier.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+struct osmo_gsup_client *osmo_gsup_client_create(void *talloc_ctx,</span><br><span style="color: hsl(120, 100%, 40%);">+                                          const char *unit_name,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                const char *ip_addr,</span><br><span style="color: hsl(120, 100%, 40%);">+                                          unsigned int tcp_port,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                osmo_gsup_client_read_cb_t read_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+                                           struct osmo_oap_client_config *oapc_config)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+       struct ipaccess_unit *ipa_dev = talloc_zero(talloc_ctx, struct ipaccess_unit);</span><br><span style="color: hsl(120, 100%, 40%);">+        ipa_dev->unit_name = talloc_strdup(ipa_dev, unit_name);</span><br><span style="color: hsl(120, 100%, 40%);">+    return osmo_gsup_client_create2(talloc_ctx, ipa_dev, ip_addr, tcp_port, read_cb, oapc_config);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> void osmo_gsup_client_destroy(struct osmo_gsup_client *gsupc)</span><br><span> {</span><br><span>    osmo_timer_del(&gsupc->connect_timer);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12098">change 12098</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/12098"/><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: Ief09677e07d6e977247185b72c605f109aa091f5 </div>
<div style="display:none"> Gerrit-Change-Number: 12098 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Stefan Sperling <stsp@stsp.name> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </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: Stefan Sperling <stsp@stsp.name> </div>