<p>dexter has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/24929">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">mgcp_ratectr: refactor rate counter and set group name<br><br>The rate counter group is currently only referenced by an index. In a<br>system with multiple trunks this makes it difficult to say which rate<br>counter group belongs to which trunk sinde the index that is used does<br>not necessarly corespond to a specific trunk.<br><br>Since rate counter groups can now get a human readable name assigned, we<br>should do that.<br><br>Also E1 specific rate counters only make sense for E1-trunks, so they<br>should not be present on the virtual trunk.<br><br>Change-Id: I5e7f0e9081a06af48e284afa5c36a095b2847704<br>---<br>M include/osmocom/mgcp/mgcp_ratectr.h<br>M src/libosmo-mgcp/mgcp_protocol.c<br>M src/libosmo-mgcp/mgcp_ratectr.c<br>M src/libosmo-mgcp/mgcp_trunk.c<br>4 files changed, 51 insertions(+), 27 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/29/24929/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/mgcp/mgcp_ratectr.h b/include/osmocom/mgcp/mgcp_ratectr.h</span><br><span>index 84315e0..78c687b 100644</span><br><span>--- a/include/osmocom/mgcp/mgcp_ratectr.h</span><br><span>+++ b/include/osmocom/mgcp/mgcp_ratectr.h</span><br><span>@@ -90,5 +90,8 @@</span><br><span>      struct rate_ctr_group *e1_stats;</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-int mgcp_ratectr_global_alloc(void *ctx, struct mgcp_ratectr_global *ratectr);</span><br><span style="color: hsl(0, 100%, 40%);">-int mgcp_ratectr_trunk_alloc(void *ctx, struct mgcp_ratectr_trunk *ratectr);</span><br><span style="color: hsl(120, 100%, 40%);">+struct mgcp_config;</span><br><span style="color: hsl(120, 100%, 40%);">+struct mgcp_trunk;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+int mgcp_ratectr_global_alloc(struct mgcp_config *cfg);</span><br><span style="color: hsl(120, 100%, 40%);">+int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk);</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c</span><br><span>index 29d27d4..bab1373 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_protocol.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_protocol.c</span><br><span>@@ -1560,7 +1560,7 @@</span><br><span>               return NULL;</span><br><span>         }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        mgcp_ratectr_global_alloc(cfg, &cfg->ratectr);</span><br><span style="color: hsl(120, 100%, 40%);">+        mgcp_ratectr_global_alloc(cfg);</span><br><span> </span><br><span>      return cfg;</span><br><span> }</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_ratectr.c b/src/libosmo-mgcp/mgcp_ratectr.c</span><br><span>index 8f3703f..73c2149 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_ratectr.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_ratectr.c</span><br><span>@@ -25,6 +25,7 @@</span><br><span> #include <errno.h></span><br><span> #include <osmocom/core/stats.h></span><br><span> #include <osmocom/mgcp/mgcp_conn.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/mgcp/mgcp_trunk.h></span><br><span> #include <osmocom/mgcp/mgcp_ratectr.h></span><br><span> </span><br><span> static const struct rate_ctr_desc mgcp_general_ctr_desc[] = {</span><br><span>@@ -153,75 +154,95 @@</span><br><span>     return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*! allocate global rate counters into a given rate counter struct</span><br><span style="color: hsl(0, 100%, 40%);">- *  (called once at startup)</span><br><span style="color: hsl(0, 100%, 40%);">- *  \param[in] ctx talloc context.</span><br><span style="color: hsl(0, 100%, 40%);">- *  \param[out] ratectr struct that holds the counters</span><br><span style="color: hsl(0, 100%, 40%);">- *  \returns 0 on success, -EINVAL on failure */</span><br><span style="color: hsl(0, 100%, 40%);">-int mgcp_ratectr_global_alloc(void *ctx, struct mgcp_ratectr_global *ratectr)</span><br><span style="color: hsl(120, 100%, 40%);">+/*! allocate global rate counters</span><br><span style="color: hsl(120, 100%, 40%);">+ *  (called once at startup).</span><br><span style="color: hsl(120, 100%, 40%);">+ *  \param[in] cfg mgw configuration for which the rate counters are allocated.</span><br><span style="color: hsl(120, 100%, 40%);">+ *  \returns 0 on success, -EINVAL on failure. */</span><br><span style="color: hsl(120, 100%, 40%);">+int mgcp_ratectr_global_alloc(struct mgcp_config *cfg)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-        /* FIXME: Each new rate counter group requires a unique index. At the</span><br><span style="color: hsl(0, 100%, 40%);">-    * moment we generate an index using a counter, but perhaps there is</span><br><span style="color: hsl(0, 100%, 40%);">-     * a better way of assigning indices? */</span><br><span style="color: hsl(120, 100%, 40%);">+      struct mgcp_ratectr_global *ratectr = &cfg->ratectr;</span><br><span>  static unsigned int general_rate_ctr_index = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+       char ctr_name[512];</span><br><span> </span><br><span>      if (ratectr->mgcp_general_ctr_group == NULL) {</span><br><span>            ratectr->mgcp_general_ctr_group =</span><br><span style="color: hsl(0, 100%, 40%);">-                rate_ctr_group_alloc(ctx, &mgcp_general_ctr_group_desc, general_rate_ctr_index);</span><br><span style="color: hsl(120, 100%, 40%);">+                  rate_ctr_group_alloc(cfg, &mgcp_general_ctr_group_desc, general_rate_ctr_index);</span><br><span>             if (!ratectr->mgcp_general_ctr_group)</span><br><span>                     return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+               snprintf(ctr_name, sizeof(ctr_name), "%s:general", cfg->domain);</span><br><span style="color: hsl(120, 100%, 40%);">+         rate_ctr_group_set_name(ratectr->mgcp_general_ctr_group, ctr_name);</span><br><span>               talloc_set_destructor(ratectr->mgcp_general_ctr_group, free_rate_counter_group);</span><br><span>          general_rate_ctr_index++;</span><br><span>    }</span><br><span>    return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*! allocate trunk specific rate counters into a given rate counter struct</span><br><span style="color: hsl(0, 100%, 40%);">- *  (called once on trunk initialization)</span><br><span style="color: hsl(0, 100%, 40%);">- *  \param[in] ctx talloc context.</span><br><span style="color: hsl(0, 100%, 40%);">- *  \param[out] ratectr struct that holds the counters</span><br><span style="color: hsl(120, 100%, 40%);">+/*! allocate trunk specific rate counters</span><br><span style="color: hsl(120, 100%, 40%);">+ *  (called once on trunk initialization).</span><br><span style="color: hsl(120, 100%, 40%);">+ *  \param[in] trunk mgw trunk for which the rate counters are allocated.</span><br><span>  *  \returns 0 on success, -EINVAL on failure */</span><br><span style="color: hsl(0, 100%, 40%);">-int mgcp_ratectr_trunk_alloc(void *ctx, struct mgcp_ratectr_trunk *ratectr)</span><br><span style="color: hsl(120, 100%, 40%);">+int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-     /* FIXME: see comment in mgcp_ratectr_global_alloc()  */</span><br><span style="color: hsl(120, 100%, 40%);">+      struct mgcp_ratectr_trunk *ratectr = &trunk->ratectr;</span><br><span>         static unsigned int crcx_rate_ctr_index = 0;</span><br><span>         static unsigned int mdcx_rate_ctr_index = 0;</span><br><span>         static unsigned int dlcx_rate_ctr_index = 0;</span><br><span>         static unsigned int all_rtp_conn_rate_ctr_index = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+  char ctr_name[256];</span><br><span> </span><br><span>      if (ratectr->mgcp_crcx_ctr_group == NULL) {</span><br><span style="color: hsl(0, 100%, 40%);">-          ratectr->mgcp_crcx_ctr_group = rate_ctr_group_alloc(ctx, &mgcp_crcx_ctr_group_desc, crcx_rate_ctr_index);</span><br><span style="color: hsl(120, 100%, 40%);">+              ratectr->mgcp_crcx_ctr_group =</span><br><span style="color: hsl(120, 100%, 40%);">+                 rate_ctr_group_alloc(trunk, &mgcp_crcx_ctr_group_desc, crcx_rate_ctr_index);</span><br><span>                 if (!ratectr->mgcp_crcx_ctr_group)</span><br><span>                        return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+               snprintf(ctr_name, sizeof(ctr_name), "%s-%i:crcx", mgcp_trunk_type_strs_str(trunk->trunk_type),</span><br><span style="color: hsl(120, 100%, 40%);">+                   trunk->trunk_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+         rate_ctr_group_set_name(ratectr->mgcp_crcx_ctr_group, ctr_name);</span><br><span>          talloc_set_destructor(ratectr->mgcp_crcx_ctr_group, free_rate_counter_group);</span><br><span>             crcx_rate_ctr_index++;</span><br><span>       }</span><br><span>    if (ratectr->mgcp_mdcx_ctr_group == NULL) {</span><br><span style="color: hsl(0, 100%, 40%);">-          ratectr->mgcp_mdcx_ctr_group = rate_ctr_group_alloc(ctx, &mgcp_mdcx_ctr_group_desc, mdcx_rate_ctr_index);</span><br><span style="color: hsl(120, 100%, 40%);">+              ratectr->mgcp_mdcx_ctr_group =</span><br><span style="color: hsl(120, 100%, 40%);">+                 rate_ctr_group_alloc(trunk, &mgcp_mdcx_ctr_group_desc, mdcx_rate_ctr_index);</span><br><span>                 if (!ratectr->mgcp_mdcx_ctr_group)</span><br><span>                        return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+               snprintf(ctr_name, sizeof(ctr_name), "%s-%i:mdcx", mgcp_trunk_type_strs_str(trunk->trunk_type),</span><br><span style="color: hsl(120, 100%, 40%);">+                   trunk->trunk_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+         rate_ctr_group_set_name(ratectr->mgcp_mdcx_ctr_group, ctr_name);</span><br><span>          talloc_set_destructor(ratectr->mgcp_mdcx_ctr_group, free_rate_counter_group);</span><br><span>             mdcx_rate_ctr_index++;</span><br><span>       }</span><br><span>    if (ratectr->mgcp_dlcx_ctr_group == NULL) {</span><br><span style="color: hsl(0, 100%, 40%);">-          ratectr->mgcp_dlcx_ctr_group = rate_ctr_group_alloc(ctx, &mgcp_dlcx_ctr_group_desc, dlcx_rate_ctr_index);</span><br><span style="color: hsl(120, 100%, 40%);">+              ratectr->mgcp_dlcx_ctr_group =</span><br><span style="color: hsl(120, 100%, 40%);">+                 rate_ctr_group_alloc(trunk, &mgcp_dlcx_ctr_group_desc, dlcx_rate_ctr_index);</span><br><span>                 if (!ratectr->mgcp_dlcx_ctr_group)</span><br><span>                        return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+               snprintf(ctr_name, sizeof(ctr_name), "%s-%i:dlcx", mgcp_trunk_type_strs_str(trunk->trunk_type),</span><br><span style="color: hsl(120, 100%, 40%);">+                   trunk->trunk_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+         rate_ctr_group_set_name(ratectr->mgcp_dlcx_ctr_group, ctr_name);</span><br><span>          talloc_set_destructor(ratectr->mgcp_dlcx_ctr_group, free_rate_counter_group);</span><br><span>             dlcx_rate_ctr_index++;</span><br><span>       }</span><br><span>    if (ratectr->all_rtp_conn_stats == NULL) {</span><br><span style="color: hsl(0, 100%, 40%);">-           ratectr->all_rtp_conn_stats = rate_ctr_group_alloc(ctx, &all_rtp_conn_rate_ctr_group_desc,</span><br><span style="color: hsl(0, 100%, 40%);">-                                                                all_rtp_conn_rate_ctr_index);</span><br><span style="color: hsl(120, 100%, 40%);">+                ratectr->all_rtp_conn_stats = rate_ctr_group_alloc(trunk, &all_rtp_conn_rate_ctr_group_desc,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                              all_rtp_conn_rate_ctr_index);</span><br><span>             if (!ratectr->all_rtp_conn_stats)</span><br><span>                         return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+               snprintf(ctr_name, sizeof(ctr_name), "%s-%i:rtp_conn", mgcp_trunk_type_strs_str(trunk->trunk_type),</span><br><span style="color: hsl(120, 100%, 40%);">+                       trunk->trunk_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+         rate_ctr_group_set_name(ratectr->all_rtp_conn_stats, ctr_name);</span><br><span>           talloc_set_destructor(ratectr->all_rtp_conn_stats, free_rate_counter_group);</span><br><span>              all_rtp_conn_rate_ctr_index++;</span><br><span>       }</span><br><span style="color: hsl(0, 100%, 40%);">-       if (ratectr->e1_stats == NULL) {</span><br><span style="color: hsl(0, 100%, 40%);">-             ratectr->e1_stats = rate_ctr_group_alloc(ctx, &e1_rate_ctr_group_desc, mdcx_rate_ctr_index);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* E1 specific */</span><br><span style="color: hsl(120, 100%, 40%);">+     if (trunk->trunk_type == MGCP_TRUNK_E1 && ratectr->e1_stats == NULL) {</span><br><span style="color: hsl(120, 100%, 40%);">+          ratectr->e1_stats = rate_ctr_group_alloc(trunk, &e1_rate_ctr_group_desc, mdcx_rate_ctr_index);</span><br><span>                if (!ratectr->e1_stats)</span><br><span>                   return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+               snprintf(ctr_name, sizeof(ctr_name), "%s-%i:e1", mgcp_trunk_type_strs_str(trunk->trunk_type),</span><br><span style="color: hsl(120, 100%, 40%);">+                     trunk->trunk_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+         rate_ctr_group_set_name(ratectr->e1_stats, ctr_name);</span><br><span>             talloc_set_destructor(ratectr->e1_stats, free_rate_counter_group);</span><br><span>                mdcx_rate_ctr_index++;</span><br><span>       }</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_trunk.c b/src/libosmo-mgcp/mgcp_trunk.c</span><br><span>index 5c9a888..e1629f1 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_trunk.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_trunk.c</span><br><span>@@ -63,7 +63,7 @@</span><br><span> </span><br><span>   llist_add_tail(&trunk->entry, &cfg->trunks);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        mgcp_ratectr_trunk_alloc(cfg, &trunk->ratectr);</span><br><span style="color: hsl(120, 100%, 40%);">+        mgcp_ratectr_trunk_alloc(trunk);</span><br><span> </span><br><span>   return trunk;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/24929">change 24929</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-mgw/+/24929"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I5e7f0e9081a06af48e284afa5c36a095b2847704 </div>
<div style="display:none"> Gerrit-Change-Number: 24929 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>