<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16206">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">gsup client: add up_down_cb(), add osmo_gsup_client_create3()<br><br>For the GSUP clients in upcoming D-GSM enabled osmo-hlr, it will be necessary<br>to trigger an event as soon as a GSUP client connection becomes ready for<br>communication. Add the osmo_gsup_client->up_down_cb.<br><br>Add osmo_gsup_client_create3() to pass the up_down_cb in the arguments. Also<br>add a cb data argument to populate the already existing osmo_gsup_client->data<br>item directly from osmo_gsup_client_create3().<br><br>We need the callbacks and data pointer in the osmo_gsup_client_create()<br>function right before startup, because this function immediately starts up the<br>connection. Who knows whether callbacks might trigger right away.<br><br>Because there are so many arguments, and to prevent the need for ever new<br>versions of this function, pass the arguments as an extendable struct.<br><br>Change-Id: I6f181e42b678465bc9945f192559dc57d2083c6d<br>---<br>M include/osmocom/gsupclient/gsup_client.h<br>M src/gsupclient/gsup_client.c<br>2 files changed, 74 insertions(+), 20 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 b417ade..ea66ca1 100644</span><br><span>--- a/include/osmocom/gsupclient/gsup_client.h</span><br><span>+++ b/include/osmocom/gsupclient/gsup_client.h</span><br><span>@@ -38,6 +38,8 @@</span><br><span> /* Expects message in msg->l2h */</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 style="color: hsl(120, 100%, 40%);">+typedef bool (*osmo_gsup_client_up_down_cb_t)(struct osmo_gsup_client *gsupc, bool up);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> struct osmo_gsup_client {</span><br><span>  const char *unit_name; /* same as ipa_dev->unit_name, for backwards compat */</span><br><span> </span><br><span>@@ -53,8 +55,31 @@</span><br><span>    int got_ipa_pong;</span><br><span> </span><br><span>        struct ipaccess_unit *ipa_dev; /* identification information sent to IPA server */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+  osmo_gsup_client_up_down_cb_t up_down_cb;</span><br><span> };</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct osmo_gsup_client_config {</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! IP access unit which contains client identification information; must be allocated in talloc_ctx as well to</span><br><span style="color: hsl(120, 100%, 40%);">+        * ensure it lives throughout the lifetime of the connection. */</span><br><span style="color: hsl(120, 100%, 40%);">+      struct ipaccess_unit *ipa_dev;</span><br><span style="color: hsl(120, 100%, 40%);">+        /*! GSUP server IP address to connect to. */</span><br><span style="color: hsl(120, 100%, 40%);">+  const char *ip_addr;</span><br><span style="color: hsl(120, 100%, 40%);">+  /*! GSUP server TCP port to connect to. */</span><br><span style="color: hsl(120, 100%, 40%);">+    unsigned int tcp_port;</span><br><span style="color: hsl(120, 100%, 40%);">+        /*! OPA client configuration, or NULL. */</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%);">+   /*! callback for reading from the GSUP connection. */</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%);">+   /*! Invoked when the GSUP link is ready for communication, and when the link drops. */</span><br><span style="color: hsl(120, 100%, 40%);">+        osmo_gsup_client_up_down_cb_t up_down_cb;</span><br><span style="color: hsl(120, 100%, 40%);">+     /*! User data stored in the returned gsupc->data, as context for the callbacks. */</span><br><span style="color: hsl(120, 100%, 40%);">+ void *data;</span><br><span style="color: hsl(120, 100%, 40%);">+   /*! Marker for future extension, always pass this as false. */</span><br><span style="color: hsl(120, 100%, 40%);">+        bool more;</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_create3(void *talloc_ctx, struct osmo_gsup_client_config *config);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> struct osmo_gsup_client *osmo_gsup_client_create2(void *talloc_ctx,</span><br><span>                                                  struct ipaccess_unit *ipa_dev,</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 52985c9..4f76efc 100644</span><br><span>--- a/src/gsupclient/gsup_client.c</span><br><span>+++ b/src/gsupclient/gsup_client.c</span><br><span>@@ -97,6 +97,12 @@</span><br><span>     if (gsupc->is_connected)</span><br><span>          return;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+   if (gsupc->up_down_cb) {</span><br><span style="color: hsl(120, 100%, 40%);">+           /* When the up_down_cb() returns false, the user asks us not to retry connecting. */</span><br><span style="color: hsl(120, 100%, 40%);">+          if (!gsupc->up_down_cb(gsupc, false))</span><br><span style="color: hsl(120, 100%, 40%);">+                      return;</span><br><span style="color: hsl(120, 100%, 40%);">+       }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  gsup_client_connect(gsupc);</span><br><span> }</span><br><span> </span><br><span>@@ -139,9 +145,18 @@</span><br><span>                  gsup_client_oap_register(gsupc);</span><br><span> </span><br><span>                 osmo_timer_del(&gsupc->connect_timer);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+               if (gsupc->up_down_cb)</span><br><span style="color: hsl(120, 100%, 40%);">+                     gsupc->up_down_cb(gsupc, true);</span><br><span>   } else {</span><br><span>             osmo_timer_del(&gsupc->ping_timer);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+                if (gsupc->up_down_cb) {</span><br><span style="color: hsl(120, 100%, 40%);">+                   /* When the up_down_cb() returns false, the user asks us not to retry connecting. */</span><br><span style="color: hsl(120, 100%, 40%);">+                  if (!gsupc->up_down_cb(gsupc, false))</span><br><span style="color: hsl(120, 100%, 40%);">+                              return;</span><br><span style="color: hsl(120, 100%, 40%);">+               }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>          osmo_timer_schedule(&gsupc->connect_timer,</span><br><span>                                OSMO_GSUP_CLIENT_RECONNECT_INTERVAL, 0);</span><br><span>         }</span><br><span>@@ -263,31 +278,28 @@</span><br><span>  * Use the provided ipaccess unit as the client-side identifier; ipa_dev should</span><br><span>  * be allocated in talloc_ctx talloc_ctx as well.</span><br><span>  * \param[in] talloc_ctx talloc context.</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[in] ipa_dev IP access unit which contains client identification information; must be allocated</span><br><span style="color: hsl(0, 100%, 40%);">- *                    in talloc_ctx as well to ensure it lives throughout the lifetime of the connection.</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[in] ip_addr GSUP server IP address.</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[in] tcp_port GSUP server TCP port.</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[in] read_cb callback for reading from the GSUP connection.</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[in] oapc_config OPA client configuration.</span><br><span style="color: hsl(0, 100%, 40%);">- *  \returns a GSUP client connection or NULL on failure.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] config  Parameters for setting up the GSUP client.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \return a GSUP client connection, or NULL on failure.</span><br><span>  */</span><br><span style="color: hsl(0, 100%, 40%);">-struct osmo_gsup_client *osmo_gsup_client_create2(void *talloc_ctx,</span><br><span style="color: hsl(0, 100%, 40%);">-                                             struct ipaccess_unit *ipa_dev,</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%);">+struct osmo_gsup_client *osmo_gsup_client_create3(void *talloc_ctx, struct osmo_gsup_client_config *config)</span><br><span> {</span><br><span>      struct osmo_gsup_client *gsupc;</span><br><span>      int rc;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+   OSMO_ASSERT(config->ipa_dev->unit_name);</span><br><span style="color: hsl(120, 100%, 40%);">+</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%);">-     gsupc->unit_name = (const char *)ipa_dev->unit_name; /* API backwards compat */</span><br><span style="color: hsl(0, 100%, 40%);">-   gsupc->ipa_dev = ipa_dev;</span><br><span style="color: hsl(120, 100%, 40%);">+  *gsupc = (struct osmo_gsup_client){</span><br><span style="color: hsl(120, 100%, 40%);">+           .unit_name = (const char *)config->ipa_dev->unit_name, /* API backwards compat */</span><br><span style="color: hsl(120, 100%, 40%);">+               .ipa_dev = config->ipa_dev,</span><br><span style="color: hsl(120, 100%, 40%);">+                .read_cb = config->read_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+                .up_down_cb = config->up_down_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+          .data = config->data,</span><br><span style="color: hsl(120, 100%, 40%);">+      };</span><br><span> </span><br><span>       /* a NULL oapc_config will mark oap_state disabled. */</span><br><span style="color: hsl(0, 100%, 40%);">-  rc = osmo_oap_client_init(oapc_config, &gsupc->oap_state);</span><br><span style="color: hsl(120, 100%, 40%);">+     rc = osmo_oap_client_init(config->oapc_config, &gsupc->oap_state);</span><br><span>         if (rc != 0)</span><br><span>                 goto failed;</span><br><span> </span><br><span>@@ -295,7 +307,7 @@</span><br><span>                                             /* no e1inp */ NULL,</span><br><span>                                         0,</span><br><span>                                           /* no specific local IP:port */ NULL, 0,</span><br><span style="color: hsl(0, 100%, 40%);">-                                        ip_addr, tcp_port,</span><br><span style="color: hsl(120, 100%, 40%);">+                                            config->ip_addr, config->tcp_port,</span><br><span>                                             gsup_client_updown_cb,</span><br><span>                                               gsup_client_read_cb,</span><br><span>                                         /* default write_cb */ NULL,</span><br><span>@@ -309,9 +321,6 @@</span><br><span> </span><br><span>         if (rc < 0)</span><br><span>               goto failed;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-    gsupc->read_cb = read_cb;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>         return gsupc;</span><br><span> </span><br><span> failed:</span><br><span>@@ -319,6 +328,26 @@</span><br><span>  return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! Like osmo_gsup_client_create3() but without the up_down_cb and data arguments, and with the oapc_config argument in</span><br><span style="color: hsl(120, 100%, 40%);">+ * a different position.</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 style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+      struct osmo_gsup_client_config cfg = {</span><br><span style="color: hsl(120, 100%, 40%);">+                .ipa_dev = ipa_dev,</span><br><span style="color: hsl(120, 100%, 40%);">+           .ip_addr = ip_addr,</span><br><span style="color: hsl(120, 100%, 40%);">+           .tcp_port = tcp_port,</span><br><span style="color: hsl(120, 100%, 40%);">+         .oapc_config = oapc_config,</span><br><span style="color: hsl(120, 100%, 40%);">+           .read_cb = read_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+   };</span><br><span style="color: hsl(120, 100%, 40%);">+    return osmo_gsup_client_create3(talloc_ctx, &cfg);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /**</span><br><span>  * Like osmo_gsup_client_create2() except it expects a unit name instead</span><br><span>  * of a full-blown ipacess_unit as the client-side identifier.</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16206">change 16206</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-hlr/+/16206"/><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-Change-Id: I6f181e42b678465bc9945f192559dc57d2083c6d </div>
<div style="display:none"> Gerrit-Change-Number: 16206 </div>
<div style="display:none"> Gerrit-PatchSet: 31 </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: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>