[MERGED] osmo-mgw[master]: Revert "stats: use libosmocore rate counter for in/out_strea...

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/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Mon Apr 23 08:39:09 UTC 2018


Harald Welte has submitted this change and it was merged.

Change subject: Revert "stats: use libosmocore rate counter for in/out_stream.err_ts_counter"
......................................................................


Revert "stats: use libosmocore rate counter for in/out_stream.err_ts_counter"

This reverts commit 7181cc1f026a0b63a061296aba4e10a9cadaf2c8.
The tests are broken on i686, arm (non 64bit systems).

Change-Id: I15f3c78f8410d709733ed5692ba94ba17559d7e1
---
M include/osmocom/mgcp/mgcp_internal.h
M src/libosmo-mgcp/mgcp_conn.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_stat.c
M src/libosmo-mgcp/mgcp_vty.c
M tests/mgcp/mgcp_test.c
6 files changed, 15 insertions(+), 63 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/mgcp/mgcp_internal.h b/include/osmocom/mgcp/mgcp_internal.h
index ff02768..0da2c56 100644
--- a/include/osmocom/mgcp/mgcp_internal.h
+++ b/include/osmocom/mgcp/mgcp_internal.h
@@ -28,7 +28,6 @@
 #include <osmocom/mgcp/mgcp.h>
 #include <osmocom/core/linuxlist.h>
 #include <osmocom/core/counter.h>
-#include <osmocom/core/rate_ctr.h>
 
 #define CI_UNUSED 0
 
@@ -46,7 +45,7 @@
 	uint32_t ssrc;
 	uint16_t last_seq;
 	uint32_t last_timestamp;
-	struct rate_ctr *err_ts_ctr;
+	uint32_t err_ts_counter;
 	int32_t last_tsdelta;
 	uint32_t last_arrival_time;
 };
@@ -203,8 +202,6 @@
 			uint32_t octets;
 		} stats;
 	} osmux;
-
-	struct rate_ctr_group *rate_ctr_group;
 };
 
 /*! Connection type, specifies which member of the union "u" in mgcp_conn
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index 0055049..998dbc5 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -26,27 +26,7 @@
 #include <osmocom/mgcp/mgcp_common.h>
 #include <osmocom/mgcp/mgcp_endp.h>
 #include <osmocom/gsm/gsm_utils.h>
-#include <osmocom/core/rate_ctr.h>
 #include <ctype.h>
-
-const static struct rate_ctr_desc rate_ctr_desc[] = {
-	{
-		.name = "in_stream_err_ts_ctr",
-		.description = "inbound rtp-stream timestamp errors",
-	},{
-		.name = "out_stream_err_ts_ctr",
-		.description = "outbound rtp-stream timestamp errors",
-	}
-};
-
-const static struct rate_ctr_group_desc rate_ctr_group_desc = {
-	.group_name_prefix = "conn_rtp",
-	.group_description = "rtp connection statistics",
-	.class_id = 1,
-	.num_ctr = 2,
-	.ctr_desc = rate_ctr_desc
-};
-
 
 /* Allocate a new connection identifier. According to RFC3435, they must
  * be unique only within the scope of the endpoint. (Caller must provide
@@ -107,10 +87,6 @@
 static void mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct mgcp_conn *conn)
 {
 	struct mgcp_rtp_end *end = &conn_rtp->end;
-	/* FIXME: Each new rate counter group requires an unique index. At the
-	 * moment we generate this index using this counter, but perhaps there
-	 * is a more concious way to assign the indexes. */
-	static unsigned int rate_ctr_index = 0;
 
 	conn_rtp->type = MGCP_RTP_DEFAULT;
 	conn_rtp->osmux.allocated_cid = -1;
@@ -132,15 +108,6 @@
 
 	mgcp_rtp_codec_init(&end->codec);
 	mgcp_rtp_codec_init(&end->alt_codec);
-
-	conn_rtp->rate_ctr_group =
-	    rate_ctr_group_alloc(conn, &rate_ctr_group_desc,
-				 rate_ctr_index);
-	conn_rtp->state.in_stream.err_ts_ctr =
-	    &conn_rtp->rate_ctr_group->ctr[0];
-	conn_rtp->state.out_stream.err_ts_ctr =
-	    &conn_rtp->rate_ctr_group->ctr[1];
-	rate_ctr_index++;
 }
 
 /* Cleanup rtp connection struct */
@@ -149,7 +116,6 @@
 	osmux_disable_conn(conn_rtp);
 	osmux_release_cid(conn_rtp);
 	mgcp_free_rtp_port(&conn_rtp->end);
-	rate_ctr_group_free(conn_rtp->rate_ctr_group);
 }
 
 /*! allocate a new connection list entry.
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index c56e433..6923b97 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -222,7 +222,7 @@
 
 	if (seq == sstate->last_seq) {
 		if (timestamp != sstate->last_timestamp) {
-			rate_ctr_inc(sstate->err_ts_ctr);
+			sstate->err_ts_counter += 1;
 			LOGP(DRTP, LOGL_ERROR,
 			     "The %s timestamp delta is != 0 but the sequence "
 			     "number %d is the same, "
@@ -272,7 +272,7 @@
 	    ts_alignment_error(sstate, state->packet_duration, timestamp);
 
 	if (timestamp_error) {
-		rate_ctr_inc(sstate->err_ts_ctr);
+		sstate->err_ts_counter += 1;
 		LOGP(DRTP, LOGL_NOTICE,
 		     "The %s timestamp has an alignment error of %d "
 		     "on 0x%x SSRC: %u "
@@ -505,16 +505,13 @@
 	mgcp_rtp_annex_count(endp, state, seq, transit, ssrc);
 
 	if (!state->initialized) {
-		/* FIXME: Move this initialization to mgcp.conn.c */
 		state->initialized = 1;
 		state->in_stream.last_seq = seq - 1;
 		state->in_stream.ssrc = state->patch.orig_ssrc = ssrc;
 		state->in_stream.last_tsdelta = 0;
 		state->packet_duration =
 		    mgcp_rtp_packet_duration(endp, rtp_end);
-		state->out_stream.last_seq = seq - 1;
-		state->out_stream.ssrc = state->patch.orig_ssrc = ssrc;
-		state->out_stream.last_tsdelta = 0;
+		state->out_stream = state->in_stream;
 		state->out_stream.last_timestamp = timestamp;
 		state->out_stream.ssrc = ssrc - 1;	/* force output SSRC change */
 		LOGP(DRTP, LOGL_INFO,
diff --git a/src/libosmo-mgcp/mgcp_stat.c b/src/libosmo-mgcp/mgcp_stat.c
index cc723bb..581130c 100644
--- a/src/libosmo-mgcp/mgcp_stat.c
+++ b/src/libosmo-mgcp/mgcp_stat.c
@@ -87,9 +87,9 @@
 	if (conn->conn->endp->cfg->osmux != OSMUX_USAGE_OFF) {
 		/* Error Counter */
 		nchars = snprintf(str, str_len,
-				  "\r\nX-Osmo-CP: EC TI=%lu, TO=%lu",
-				  conn->state.in_stream.err_ts_ctr->current,
-				  conn->state.out_stream.err_ts_ctr->current);
+				  "\r\nX-Osmo-CP: EC TI=%u, TO=%u",
+				  conn->state.in_stream.err_ts_counter,
+				  conn->state.out_stream.err_ts_counter);
 		if (nchars < 0 || nchars >= str_len)
 			goto truncate;
 
diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c
index 392a176..14ecd17 100644
--- a/src/libosmo-mgcp/mgcp_vty.c
+++ b/src/libosmo-mgcp/mgcp_vty.c
@@ -160,16 +160,15 @@
 	struct mgcp_rtp_codec *codec = &end->codec;
 
 	vty_out(vty,
-		"   Timestamp Errs: %lu->%lu%s"
+		"   Timestamp Errs: %d->%d%s"
 		"   Dropped Packets: %d%s"
 		"   Payload Type: %d Rate: %u Channels: %d %s"
 		"   Frame Duration: %u Frame Denominator: %u%s"
 		"   FPP: %d Packet Duration: %u%s"
 		"   FMTP-Extra: %s Audio-Name: %s Sub-Type: %s%s"
 		"   Output-Enabled: %d Force-PTIME: %d%s",
-		state->in_stream.err_ts_ctr->current,
-		state->out_stream.err_ts_ctr->current,
-	        VTY_NEWLINE,
+		state->in_stream.err_ts_counter,
+		state->out_stream.err_ts_counter, VTY_NEWLINE,
 		end->stats.dropped_packets, VTY_NEWLINE,
 		codec->payload_type, codec->rate, codec->channels, VTY_NEWLINE,
 		codec->frame_duration_num, codec->frame_duration_den,
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 0af3799..f6c421a 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1133,8 +1133,6 @@
 	int last_out_ts_err_cnt = 0;
 	struct mgcp_conn_rtp *conn = NULL;
 	struct mgcp_conn *_conn = NULL;
-	struct rate_ctr test_ctr_in;
-	struct rate_ctr test_ctr_out;
 
 	printf("Testing packet error detection%s%s.\n",
 	       patch_ssrc ? ", patch SSRC" : "",
@@ -1143,11 +1141,6 @@
 	memset(&trunk, 0, sizeof(trunk));
 	memset(&endp, 0, sizeof(endp));
 	memset(&state, 0, sizeof(state));
-
-	test_ctr_in.current = 0;
-	test_ctr_out.current = 0;
-	state.in_stream.err_ts_ctr = &test_ctr_in;
-	state.out_stream.err_ts_ctr = &test_ctr_out;
 
 	endp.type = &ep_typeset.rtp;
 
@@ -1193,18 +1186,18 @@
 		       state.in_stream.last_tsdelta, state.in_stream.last_seq);
 
 		printf("Out TS change: %d, dTS: %d, Seq change: %d, "
-		       "TS Err change: in +%lu, out +%lu\n",
+		       "TS Err change: in %+d, out %+d\n",
 		       state.out_stream.last_timestamp - last_timestamp,
 		       state.out_stream.last_tsdelta,
 		       state.out_stream.last_seq - last_seqno,
-		       state.in_stream.err_ts_ctr->current - last_in_ts_err_cnt,
-		       state.out_stream.err_ts_ctr->current - last_out_ts_err_cnt);
+		       state.in_stream.err_ts_counter - last_in_ts_err_cnt,
+		       state.out_stream.err_ts_counter - last_out_ts_err_cnt);
 
 		printf("Stats: Jitter = %u, Transit = %d\n",
 		       calc_jitter(&state), state.stats.transit);
 
-		last_in_ts_err_cnt = state.in_stream.err_ts_ctr->current;
-		last_out_ts_err_cnt = state.out_stream.err_ts_ctr->current;
+		last_in_ts_err_cnt = state.in_stream.err_ts_counter;
+		last_out_ts_err_cnt = state.out_stream.err_ts_counter;
 		last_timestamp = state.out_stream.last_timestamp;
 		last_seqno = state.out_stream.last_seq;
 	}

-- 
To view, visit https://gerrit.osmocom.org/7891
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I15f3c78f8410d709733ed5692ba94ba17559d7e1
Gerrit-PatchSet: 1
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>



More information about the gerrit-log mailing list