This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
neels gerrit-no-reply at lists.osmocom.orgneels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/24982 ) Change subject: Revert "mgcp_ratectr: add stats items to monitor trunk usage" ...................................................................... Revert "mgcp_ratectr: add stats items to monitor trunk usage" This reverts commit 6bad138c96ef0e2a93ef7de42e897880131c0b43. Reason for revert: heap-use-after-free during 'make check' in mgcp_test.c test_retransmission() Change-Id: I96792a719c9c7273676ab9ffe0b9e2aae4c23166 Related: OS#5201 --- M include/osmocom/mgcp/mgcp_ratectr.h M include/osmocom/mgcp/mgcp_trunk.h M src/libosmo-mgcp/mgcp_endp.c M src/libosmo-mgcp/mgcp_ratectr.c M src/libosmo-mgcp/mgcp_trunk.c M tests/mgcp/mgcp_test.c 6 files changed, 21 insertions(+), 94 deletions(-) Approvals: neels: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmocom/mgcp/mgcp_ratectr.h b/include/osmocom/mgcp/mgcp_ratectr.h index 0bd6f88..78c687b 100644 --- a/include/osmocom/mgcp/mgcp_ratectr.h +++ b/include/osmocom/mgcp/mgcp_ratectr.h @@ -95,17 +95,3 @@ int mgcp_ratectr_global_alloc(struct mgcp_config *cfg); int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk); - -/* Trunk-global common stat items */ -enum { - TRUNK_STAT_ENDPOINTS_TOTAL, - TRUNK_STAT_ENDPOINTS_USED, -}; - -struct mgcp_stat_trunk { - /* Stat item group which contains general status values of the trunk. */ - struct osmo_stat_item_group *common; -}; - -int mgcp_stat_trunk_alloc(struct mgcp_trunk *trunk); - diff --git a/include/osmocom/mgcp/mgcp_trunk.h b/include/osmocom/mgcp/mgcp_trunk.h index d960428..048ac5b 100644 --- a/include/osmocom/mgcp/mgcp_trunk.h +++ b/include/osmocom/mgcp/mgcp_trunk.h @@ -52,9 +52,8 @@ unsigned int number_endpoints; struct mgcp_endpoint **endpoints; - /* rate counters and stat items to measure the trunks overall performance and health */ + /* global rate counters to measure the trunks overall performance and health */ struct mgcp_ratectr_trunk ratectr; - struct mgcp_stat_trunk stats; union { /* Virtual trunk specific */ diff --git a/src/libosmo-mgcp/mgcp_endp.c b/src/libosmo-mgcp/mgcp_endp.c index 7df4a80..ae376b0 100644 --- a/src/libosmo-mgcp/mgcp_endp.c +++ b/src/libosmo-mgcp/mgcp_endp.c @@ -29,7 +29,6 @@ #include <osmocom/abis/e1_input.h> #include <osmocom/mgcp/mgcp_e1.h> -#include <osmocom/core/stat_item.h> #define E1_RATE_MAX 64 #define E1_OFFS_MAX 8 @@ -123,12 +122,6 @@ * RSIP is executed), free them all at once. */ mgcp_conn_free_all(endp); - /* We must only decrement the stat item when the endpoint as actually - * claimed. An endpoint is claimed when a call-id is set */ - if (endp->callid) - osmo_stat_item_dec(osmo_stat_item_group_get_item(endp->trunk->stats.common, - TRUNK_STAT_ENDPOINTS_USED), 1); - /* Reset endpoint parameters and states */ talloc_free(endp->callid); endp->callid = NULL; @@ -638,8 +631,6 @@ * connection ids) */ endp->callid = talloc_strdup(endp, callid); OSMO_ASSERT(endp->callid); - osmo_stat_item_inc(osmo_stat_item_group_get_item(endp->trunk->stats.common, - TRUNK_STAT_ENDPOINTS_USED), 1); /* Allocate resources */ switch (endp->trunk->trunk_type) { diff --git a/src/libosmo-mgcp/mgcp_ratectr.c b/src/libosmo-mgcp/mgcp_ratectr.c index 740a3b0..1f8b233 100644 --- a/src/libosmo-mgcp/mgcp_ratectr.c +++ b/src/libosmo-mgcp/mgcp_ratectr.c @@ -24,11 +24,8 @@ #include <errno.h> #include <osmocom/core/stats.h> -#include <osmocom/core/stat_item.h> #include <osmocom/mgcp/mgcp_conn.h> #include <osmocom/mgcp/mgcp_trunk.h> -#include <osmocom/mgcp/mgcp_protocol.h> -#include <osmocom/mgcp/mgcp_endp.h> #include <osmocom/mgcp/mgcp_ratectr.h> static const struct rate_ctr_desc mgcp_general_ctr_desc[] = { @@ -251,41 +248,3 @@ } return 0; } - -const struct osmo_stat_item_desc trunk_stat_desc[] = { - [TRUNK_STAT_ENDPOINTS_TOTAL] = { "endpoints:total", - "Number of endpoints that exist on the trunk", - "", 60, 0 }, - [TRUNK_STAT_ENDPOINTS_USED] = { "endpoints:used", - "Number of endpoints in use", - "", 60, 0 }, -}; - -const struct osmo_stat_item_group_desc trunk_statg_desc = { - .group_name_prefix = "trunk", - .group_description = "mgw trunk", - .class_id = OSMO_STATS_CLASS_GLOBAL, - .num_items = ARRAY_SIZE(trunk_stat_desc), - .item_desc = trunk_stat_desc, -}; - -/*! allocate trunk specific stat items - * (called once on trunk initialization). - * \param[in] trunk for which the stat items are allocated. - * \returns 0 on success, -EINVAL on failure. */ -int mgcp_stat_trunk_alloc(struct mgcp_trunk *trunk) -{ - struct mgcp_stat_trunk *stats = &trunk->stats; - static unsigned int common_stat_index = 0; - char stat_name[256]; - - stats->common = osmo_stat_item_group_alloc(trunk, &trunk_statg_desc, common_stat_index); - if (!stats->common) - return -EINVAL; - snprintf(stat_name, sizeof(stat_name), "%s-%u:common", mgcp_trunk_type_strs_str(trunk->trunk_type), - trunk->trunk_nr); - osmo_stat_item_group_set_name(stats->common, stat_name); - common_stat_index++; - - return 0; -} diff --git a/src/libosmo-mgcp/mgcp_trunk.c b/src/libosmo-mgcp/mgcp_trunk.c index b2d1969..27663b4 100644 --- a/src/libosmo-mgcp/mgcp_trunk.c +++ b/src/libosmo-mgcp/mgcp_trunk.c @@ -27,7 +27,6 @@ #include <osmocom/mgcp/mgcp_trunk.h> #include <osmocom/mgcp/mgcp_e1.h> #include <osmocom/abis/e1_input.h> -#include <osmocom/core/stat_item.h> const struct value_string mgcp_trunk_type_strs[] = { { MGCP_TRUNK_VIRTUAL, "virtual" }, @@ -65,7 +64,6 @@ llist_add_tail(&trunk->entry, &cfg->trunks); mgcp_ratectr_trunk_alloc(trunk); - mgcp_stat_trunk_alloc(trunk); return trunk; } @@ -129,8 +127,7 @@ /* make the endpoints we just created available to the MGW code */ trunk->number_endpoints = number_endpoints; - osmo_stat_item_set(osmo_stat_item_group_get_item(trunk->stats.common, TRUNK_STAT_ENDPOINTS_TOTAL), - trunk->number_endpoints); + return 0; } diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 26fcc2a..bcbcc02 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1065,20 +1065,20 @@ int i; struct mgcp_endpoint endp; struct mgcp_endpoint *endpoints[1]; - struct mgcp_config *cfg; - struct mgcp_trunk *trunk; + struct mgcp_config cfg = {0}; + struct mgcp_trunk trunk; printf("Testing packet loss calculation.\n"); memset(&endp, 0, sizeof(endp)); - cfg = mgcp_config_alloc(); - trunk = mgcp_trunk_alloc(cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID); - endp.cfg = cfg; + memset(&trunk, 0, sizeof(trunk)); + + endp.cfg = &cfg; endp.type = &ep_typeset.rtp; - trunk->v.vty_number_endpoints = 1; - trunk->endpoints = endpoints; - trunk->endpoints[0] = &endp; - endp.trunk = trunk; + trunk.v.vty_number_endpoints = 1; + trunk.endpoints = endpoints; + trunk.endpoints[0] = &endp; + endp.trunk = &trunk; INIT_LLIST_HEAD(&endp.conns); for (i = 0; i < ARRAY_SIZE(pl_test_dat); ++i) { @@ -1116,8 +1116,6 @@ mgcp_conn_free_all(&endp); } - talloc_free(trunk); - talloc_free(cfg); } int mgcp_parse_stats(struct msgb *msg, uint32_t *ps, uint32_t *os, @@ -1299,10 +1297,10 @@ { int i; - struct mgcp_trunk *trunk; + struct mgcp_trunk trunk; struct mgcp_endpoint endp; struct mgcp_endpoint *endpoints[1]; - struct mgcp_config *cfg; + struct mgcp_config cfg = {0}; struct mgcp_rtp_state state; struct mgcp_rtp_end *rtp; struct osmo_sockaddr addr = { 0 }; @@ -1320,8 +1318,7 @@ patch_ssrc ? ", patch SSRC" : "", patch_ts ? ", patch timestamps" : ""); - cfg = mgcp_config_alloc(); - trunk = mgcp_trunk_alloc(cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID); + memset(&trunk, 0, sizeof(trunk)); memset(&endp, 0, sizeof(endp)); memset(&state, 0, sizeof(state)); @@ -1330,16 +1327,16 @@ state.in_stream.err_ts_ctr = &test_ctr_in; state.out_stream.err_ts_ctr = &test_ctr_out; - endp.cfg = cfg; + endp.cfg = &cfg; endp.type = &ep_typeset.rtp; - trunk->v.vty_number_endpoints = 1; - trunk->endpoints = endpoints; - trunk->endpoints[0] = &endp; - trunk->force_constant_ssrc = patch_ssrc; - trunk->force_aligned_timing = patch_ts; + trunk.v.vty_number_endpoints = 1; + trunk.endpoints = endpoints; + trunk.endpoints[0] = &endp; + trunk.force_constant_ssrc = patch_ssrc; + trunk.force_aligned_timing = patch_ts; - endp.trunk = trunk; + endp.trunk = &trunk; INIT_LLIST_HEAD(&endp.conns); _conn = mgcp_conn_alloc(NULL, &endp, MGCP_CONN_TYPE_RTP, @@ -1398,8 +1395,6 @@ force_monotonic_time_us = -1; mgcp_conn_free_all(&endp); - talloc_free(trunk); - talloc_free(cfg); } static void test_multilple_codec(void) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/24982 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I96792a719c9c7273676ab9ffe0b9e2aae4c23166 Gerrit-Change-Number: 24982 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann at sysmocom.de> Gerrit-Reviewer: dexter <pmaier at sysmocom.de> Gerrit-Reviewer: laforge <laforge at osmocom.org> Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: pespin <pespin at sysmocom.de> Gerrit-MessageType: merged -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210721/a5aee73a/attachment.htm>