Change in osmo-mgw[master]: mgcp_ratectr: refactor rate counter and set group name

dexter gerrit-no-reply at lists.osmocom.org
Tue Jul 13 12:18:20 UTC 2021


dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/24929 )


Change subject: mgcp_ratectr: refactor rate counter and set group name
......................................................................

mgcp_ratectr: refactor rate counter and set group name

The rate counter group is currently only referenced by an index. In a
system with multiple trunks this makes it difficult to say which rate
counter group belongs to which trunk sinde the index that is used does
not necessarly corespond to a specific trunk.

Since rate counter groups can now get a human readable name assigned, we
should do that.

Also E1 specific rate counters only make sense for E1-trunks, so they
should not be present on the virtual trunk.

Change-Id: I5e7f0e9081a06af48e284afa5c36a095b2847704
---
M include/osmocom/mgcp/mgcp_ratectr.h
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_ratectr.c
M src/libosmo-mgcp/mgcp_trunk.c
4 files changed, 51 insertions(+), 27 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/29/24929/1

diff --git a/include/osmocom/mgcp/mgcp_ratectr.h b/include/osmocom/mgcp/mgcp_ratectr.h
index 84315e0..78c687b 100644
--- a/include/osmocom/mgcp/mgcp_ratectr.h
+++ b/include/osmocom/mgcp/mgcp_ratectr.h
@@ -90,5 +90,8 @@
 	struct rate_ctr_group *e1_stats;
 };
 
-int mgcp_ratectr_global_alloc(void *ctx, struct mgcp_ratectr_global *ratectr);
-int mgcp_ratectr_trunk_alloc(void *ctx, struct mgcp_ratectr_trunk *ratectr);
+struct mgcp_config;
+struct mgcp_trunk;
+
+int mgcp_ratectr_global_alloc(struct mgcp_config *cfg);
+int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk);
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 29d27d4..bab1373 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -1560,7 +1560,7 @@
 		return NULL;
 	}
 
-        mgcp_ratectr_global_alloc(cfg, &cfg->ratectr);
+        mgcp_ratectr_global_alloc(cfg);
 
 	return cfg;
 }
diff --git a/src/libosmo-mgcp/mgcp_ratectr.c b/src/libosmo-mgcp/mgcp_ratectr.c
index 8f3703f..73c2149 100644
--- a/src/libosmo-mgcp/mgcp_ratectr.c
+++ b/src/libosmo-mgcp/mgcp_ratectr.c
@@ -25,6 +25,7 @@
 #include <errno.h>
 #include <osmocom/core/stats.h>
 #include <osmocom/mgcp/mgcp_conn.h>
+#include <osmocom/mgcp/mgcp_trunk.h>
 #include <osmocom/mgcp/mgcp_ratectr.h>
 
 static const struct rate_ctr_desc mgcp_general_ctr_desc[] = {
@@ -153,75 +154,95 @@
 	return 0;
 }
 
-/*! allocate global rate counters into a given rate counter struct
- *  (called once at startup)
- *  \param[in] ctx talloc context.
- *  \param[out] ratectr struct that holds the counters
- *  \returns 0 on success, -EINVAL on failure */
-int mgcp_ratectr_global_alloc(void *ctx, struct mgcp_ratectr_global *ratectr)
+/*! allocate global rate counters
+ *  (called once at startup).
+ *  \param[in] cfg mgw configuration for which the rate counters are allocated.
+ *  \returns 0 on success, -EINVAL on failure. */
+int mgcp_ratectr_global_alloc(struct mgcp_config *cfg)
 {
-	/* FIXME: Each new rate counter group requires a unique index. At the
-	 * moment we generate an index using a counter, but perhaps there is
-	 * a better way of assigning indices? */
+	struct mgcp_ratectr_global *ratectr = &cfg->ratectr;
 	static unsigned int general_rate_ctr_index = 0;
+	char ctr_name[512];
 
 	if (ratectr->mgcp_general_ctr_group == NULL) {
 		ratectr->mgcp_general_ctr_group =
-		    rate_ctr_group_alloc(ctx, &mgcp_general_ctr_group_desc, general_rate_ctr_index);
+		    rate_ctr_group_alloc(cfg, &mgcp_general_ctr_group_desc, general_rate_ctr_index);
 		if (!ratectr->mgcp_general_ctr_group)
 			return -EINVAL;
+		snprintf(ctr_name, sizeof(ctr_name), "%s:general", cfg->domain);
+		rate_ctr_group_set_name(ratectr->mgcp_general_ctr_group, ctr_name);
 		talloc_set_destructor(ratectr->mgcp_general_ctr_group, free_rate_counter_group);
 		general_rate_ctr_index++;
 	}
 	return 0;
 }
 
-/*! allocate trunk specific rate counters into a given rate counter struct
- *  (called once on trunk initialization)
- *  \param[in] ctx talloc context.
- *  \param[out] ratectr struct that holds the counters
+/*! allocate trunk specific rate counters
+ *  (called once on trunk initialization).
+ *  \param[in] trunk mgw trunk for which the rate counters are allocated.
  *  \returns 0 on success, -EINVAL on failure */
-int mgcp_ratectr_trunk_alloc(void *ctx, struct mgcp_ratectr_trunk *ratectr)
+int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk)
 {
-	/* FIXME: see comment in mgcp_ratectr_global_alloc()  */
+	struct mgcp_ratectr_trunk *ratectr = &trunk->ratectr;
 	static unsigned int crcx_rate_ctr_index = 0;
 	static unsigned int mdcx_rate_ctr_index = 0;
 	static unsigned int dlcx_rate_ctr_index = 0;
 	static unsigned int all_rtp_conn_rate_ctr_index = 0;
+	char ctr_name[256];
 
 	if (ratectr->mgcp_crcx_ctr_group == NULL) {
-		ratectr->mgcp_crcx_ctr_group = rate_ctr_group_alloc(ctx, &mgcp_crcx_ctr_group_desc, crcx_rate_ctr_index);
+		ratectr->mgcp_crcx_ctr_group =
+		    rate_ctr_group_alloc(trunk, &mgcp_crcx_ctr_group_desc, crcx_rate_ctr_index);
 		if (!ratectr->mgcp_crcx_ctr_group)
 			return -EINVAL;
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%i:crcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
+			 trunk->trunk_nr);
+		rate_ctr_group_set_name(ratectr->mgcp_crcx_ctr_group, ctr_name);
 		talloc_set_destructor(ratectr->mgcp_crcx_ctr_group, free_rate_counter_group);
 		crcx_rate_ctr_index++;
 	}
 	if (ratectr->mgcp_mdcx_ctr_group == NULL) {
-		ratectr->mgcp_mdcx_ctr_group = rate_ctr_group_alloc(ctx, &mgcp_mdcx_ctr_group_desc, mdcx_rate_ctr_index);
+		ratectr->mgcp_mdcx_ctr_group =
+		    rate_ctr_group_alloc(trunk, &mgcp_mdcx_ctr_group_desc, mdcx_rate_ctr_index);
 		if (!ratectr->mgcp_mdcx_ctr_group)
 			return -EINVAL;
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%i:mdcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
+			 trunk->trunk_nr);
+		rate_ctr_group_set_name(ratectr->mgcp_mdcx_ctr_group, ctr_name);
 		talloc_set_destructor(ratectr->mgcp_mdcx_ctr_group, free_rate_counter_group);
 		mdcx_rate_ctr_index++;
 	}
 	if (ratectr->mgcp_dlcx_ctr_group == NULL) {
-		ratectr->mgcp_dlcx_ctr_group = rate_ctr_group_alloc(ctx, &mgcp_dlcx_ctr_group_desc, dlcx_rate_ctr_index);
+		ratectr->mgcp_dlcx_ctr_group =
+		    rate_ctr_group_alloc(trunk, &mgcp_dlcx_ctr_group_desc, dlcx_rate_ctr_index);
 		if (!ratectr->mgcp_dlcx_ctr_group)
 			return -EINVAL;
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%i:dlcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
+			 trunk->trunk_nr);
+		rate_ctr_group_set_name(ratectr->mgcp_dlcx_ctr_group, ctr_name);
 		talloc_set_destructor(ratectr->mgcp_dlcx_ctr_group, free_rate_counter_group);
 		dlcx_rate_ctr_index++;
 	}
 	if (ratectr->all_rtp_conn_stats == NULL) {
-		ratectr->all_rtp_conn_stats = rate_ctr_group_alloc(ctx, &all_rtp_conn_rate_ctr_group_desc,
-								 all_rtp_conn_rate_ctr_index);
+		ratectr->all_rtp_conn_stats = rate_ctr_group_alloc(trunk, &all_rtp_conn_rate_ctr_group_desc,
+								   all_rtp_conn_rate_ctr_index);
 		if (!ratectr->all_rtp_conn_stats)
 			return -EINVAL;
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%i:rtp_conn", mgcp_trunk_type_strs_str(trunk->trunk_type),
+			 trunk->trunk_nr);
+		rate_ctr_group_set_name(ratectr->all_rtp_conn_stats, ctr_name);
 		talloc_set_destructor(ratectr->all_rtp_conn_stats, free_rate_counter_group);
 		all_rtp_conn_rate_ctr_index++;
 	}
-	if (ratectr->e1_stats == NULL) {
-		ratectr->e1_stats = rate_ctr_group_alloc(ctx, &e1_rate_ctr_group_desc, mdcx_rate_ctr_index);
+
+	/* E1 specific */
+	if (trunk->trunk_type == MGCP_TRUNK_E1 && ratectr->e1_stats == NULL) {
+		ratectr->e1_stats = rate_ctr_group_alloc(trunk, &e1_rate_ctr_group_desc, mdcx_rate_ctr_index);
 		if (!ratectr->e1_stats)
 			return -EINVAL;
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%i:e1", mgcp_trunk_type_strs_str(trunk->trunk_type),
+			 trunk->trunk_nr);
+		rate_ctr_group_set_name(ratectr->e1_stats, ctr_name);
 		talloc_set_destructor(ratectr->e1_stats, free_rate_counter_group);
 		mdcx_rate_ctr_index++;
 	}
diff --git a/src/libosmo-mgcp/mgcp_trunk.c b/src/libosmo-mgcp/mgcp_trunk.c
index 5c9a888..e1629f1 100644
--- a/src/libosmo-mgcp/mgcp_trunk.c
+++ b/src/libosmo-mgcp/mgcp_trunk.c
@@ -63,7 +63,7 @@
 
 	llist_add_tail(&trunk->entry, &cfg->trunks);
 
-        mgcp_ratectr_trunk_alloc(cfg, &trunk->ratectr);
+        mgcp_ratectr_trunk_alloc(trunk);
 
 	return trunk;
 }

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/24929
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I5e7f0e9081a06af48e284afa5c36a095b2847704
Gerrit-Change-Number: 24929
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210713/64d21f46/attachment.htm>


More information about the gerrit-log mailing list