<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/8086">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">stats: use libosmocore rate counter for in/out_stream.err_ts_counter<br><br>The two counters: in_stream.err_ts_counter and out_stream.err_ts_counter<br>are still handcoded. To make them better accessible they should<br> be replaced with libosmocore rate counters.<br><br>- replace state.in_stream.err_ts_counter with libosmocore rate counter<br>- replace state.out_stream.err_ts_counter with libosmocore rate counter<br><br>Change-Id: I9fbd65bf2f4d1e015a05996db4c1f7ff20be2c95<br>Related: OS#2517<br>---<br>M include/osmocom/mgcp/mgcp_internal.h<br>M src/libosmo-mgcp/mgcp_conn.c<br>M src/libosmo-mgcp/mgcp_network.c<br>M src/libosmo-mgcp/mgcp_stat.c<br>M src/libosmo-mgcp/mgcp_vty.c<br>M tests/mgcp/mgcp_test.c<br>6 files changed, 58 insertions(+), 16 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/mgcp/mgcp_internal.h b/include/osmocom/mgcp/mgcp_internal.h</span><br><span>index 0da2c56..ff02768 100644</span><br><span>--- a/include/osmocom/mgcp/mgcp_internal.h</span><br><span>+++ b/include/osmocom/mgcp/mgcp_internal.h</span><br><span>@@ -28,6 +28,7 @@</span><br><span> #include <osmocom/mgcp/mgcp.h></span><br><span> #include <osmocom/core/linuxlist.h></span><br><span> #include <osmocom/core/counter.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/rate_ctr.h></span><br><span> </span><br><span> #define CI_UNUSED 0</span><br><span> </span><br><span>@@ -45,7 +46,7 @@</span><br><span> uint32_t ssrc;</span><br><span> uint16_t last_seq;</span><br><span> uint32_t last_timestamp;</span><br><span style="color: hsl(0, 100%, 40%);">- uint32_t err_ts_counter;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct rate_ctr *err_ts_ctr;</span><br><span> int32_t last_tsdelta;</span><br><span> uint32_t last_arrival_time;</span><br><span> };</span><br><span>@@ -202,6 +203,8 @@</span><br><span> uint32_t octets;</span><br><span> } stats;</span><br><span> } osmux;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ struct rate_ctr_group *rate_ctr_group;</span><br><span> };</span><br><span> </span><br><span> /*! Connection type, specifies which member of the union "u" in mgcp_conn</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c</span><br><span>index 998dbc5..280ee8b 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_conn.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_conn.c</span><br><span>@@ -26,8 +26,29 @@</span><br><span> #include <osmocom/mgcp/mgcp_common.h></span><br><span> #include <osmocom/mgcp/mgcp_endp.h></span><br><span> #include <osmocom/gsm/gsm_utils.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/rate_ctr.h></span><br><span> #include <ctype.h></span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+enum {</span><br><span style="color: hsl(120, 100%, 40%);">+ IN_STREAM_ERR_TSTMP_CTR,</span><br><span style="color: hsl(120, 100%, 40%);">+ OUT_STREAM_ERR_TSTMP_CTR,</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static const struct rate_ctr_desc rate_ctr_desc[] = {</span><br><span style="color: hsl(120, 100%, 40%);">+ [IN_STREAM_ERR_TSTMP_CTR] = {"stream_err_tstmp:in", "Inbound rtp-stream timestamp errors."},</span><br><span style="color: hsl(120, 100%, 40%);">+ [OUT_STREAM_ERR_TSTMP_CTR] = {"stream_err_tstmp:out", "Outbound rtp-stream timestamp errors."},</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+const static struct rate_ctr_group_desc rate_ctr_group_desc = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .group_name_prefix = "conn_rtp",</span><br><span style="color: hsl(120, 100%, 40%);">+ .group_description = "rtp connection statistics",</span><br><span style="color: hsl(120, 100%, 40%);">+ .class_id = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .num_ctr = 2,</span><br><span style="color: hsl(120, 100%, 40%);">+ .ctr_desc = rate_ctr_desc</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Allocate a new connection identifier. According to RFC3435, they must</span><br><span> * be unique only within the scope of the endpoint. (Caller must provide</span><br><span> * memory for id) */</span><br><span>@@ -87,6 +108,10 @@</span><br><span> static void mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct mgcp_conn *conn)</span><br><span> {</span><br><span> struct mgcp_rtp_end *end = &conn_rtp->end;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* FIXME: Each new rate counter group requires an unique index. At the</span><br><span style="color: hsl(120, 100%, 40%);">+ * moment we generate this index using this counter, but perhaps there</span><br><span style="color: hsl(120, 100%, 40%);">+ * is a more concious way to assign the indexes. */</span><br><span style="color: hsl(120, 100%, 40%);">+ static unsigned int rate_ctr_index = 0;</span><br><span> </span><br><span> conn_rtp->type = MGCP_RTP_DEFAULT;</span><br><span> conn_rtp->osmux.allocated_cid = -1;</span><br><span>@@ -108,6 +133,11 @@</span><br><span> </span><br><span> mgcp_rtp_codec_init(&end->codec);</span><br><span> mgcp_rtp_codec_init(&end->alt_codec);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ conn_rtp->rate_ctr_group = rate_ctr_group_alloc(conn, &rate_ctr_group_desc, rate_ctr_index);</span><br><span style="color: hsl(120, 100%, 40%);">+ conn_rtp->state.in_stream.err_ts_ctr = &conn_rtp->rate_ctr_group->ctr[IN_STREAM_ERR_TSTMP_CTR];</span><br><span style="color: hsl(120, 100%, 40%);">+ conn_rtp->state.out_stream.err_ts_ctr = &conn_rtp->rate_ctr_group->ctr[OUT_STREAM_ERR_TSTMP_CTR];</span><br><span style="color: hsl(120, 100%, 40%);">+ rate_ctr_index++;</span><br><span> }</span><br><span> </span><br><span> /* Cleanup rtp connection struct */</span><br><span>@@ -116,6 +146,7 @@</span><br><span> osmux_disable_conn(conn_rtp);</span><br><span> osmux_release_cid(conn_rtp);</span><br><span> mgcp_free_rtp_port(&conn_rtp->end);</span><br><span style="color: hsl(120, 100%, 40%);">+ rate_ctr_group_free(conn_rtp->rate_ctr_group);</span><br><span> }</span><br><span> </span><br><span> /*! allocate a new connection list entry.</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c</span><br><span>index 49e51a1..4144382 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_network.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_network.c</span><br><span>@@ -222,7 +222,7 @@</span><br><span> </span><br><span> if (seq == sstate->last_seq) {</span><br><span> if (timestamp != sstate->last_timestamp) {</span><br><span style="color: hsl(0, 100%, 40%);">- sstate->err_ts_counter += 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ rate_ctr_inc(sstate->err_ts_ctr);</span><br><span> LOGP(DRTP, LOGL_ERROR,</span><br><span> "The %s timestamp delta is != 0 but the sequence "</span><br><span> "number %d is the same, "</span><br><span>@@ -272,7 +272,7 @@</span><br><span> ts_alignment_error(sstate, state->packet_duration, timestamp);</span><br><span> </span><br><span> if (timestamp_error) {</span><br><span style="color: hsl(0, 100%, 40%);">- sstate->err_ts_counter += 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ rate_ctr_inc(sstate->err_ts_ctr);</span><br><span> LOGP(DRTP, LOGL_NOTICE,</span><br><span> "The %s timestamp has an alignment error of %d "</span><br><span> "on 0x%x SSRC: %u "</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_stat.c b/src/libosmo-mgcp/mgcp_stat.c</span><br><span>index 581130c..cc723bb 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_stat.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_stat.c</span><br><span>@@ -87,9 +87,9 @@</span><br><span> if (conn->conn->endp->cfg->osmux != OSMUX_USAGE_OFF) {</span><br><span> /* Error Counter */</span><br><span> nchars = snprintf(str, str_len,</span><br><span style="color: hsl(0, 100%, 40%);">- "\r\nX-Osmo-CP: EC TI=%u, TO=%u",</span><br><span style="color: hsl(0, 100%, 40%);">- conn->state.in_stream.err_ts_counter,</span><br><span style="color: hsl(0, 100%, 40%);">- conn->state.out_stream.err_ts_counter);</span><br><span style="color: hsl(120, 100%, 40%);">+ "\r\nX-Osmo-CP: EC TI=%lu, TO=%lu",</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->state.in_stream.err_ts_ctr->current,</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->state.out_stream.err_ts_ctr->current);</span><br><span> if (nchars < 0 || nchars >= str_len)</span><br><span> goto truncate;</span><br><span> </span><br><span>diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c</span><br><span>index 14ecd17..392a176 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_vty.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_vty.c</span><br><span>@@ -160,15 +160,16 @@</span><br><span> struct mgcp_rtp_codec *codec = &end->codec;</span><br><span> </span><br><span> vty_out(vty,</span><br><span style="color: hsl(0, 100%, 40%);">- " Timestamp Errs: %d->%d%s"</span><br><span style="color: hsl(120, 100%, 40%);">+ " Timestamp Errs: %lu->%lu%s"</span><br><span> " Dropped Packets: %d%s"</span><br><span> " Payload Type: %d Rate: %u Channels: %d %s"</span><br><span> " Frame Duration: %u Frame Denominator: %u%s"</span><br><span> " FPP: %d Packet Duration: %u%s"</span><br><span> " FMTP-Extra: %s Audio-Name: %s Sub-Type: %s%s"</span><br><span> " Output-Enabled: %d Force-PTIME: %d%s",</span><br><span style="color: hsl(0, 100%, 40%);">- state->in_stream.err_ts_counter,</span><br><span style="color: hsl(0, 100%, 40%);">- state->out_stream.err_ts_counter, VTY_NEWLINE,</span><br><span style="color: hsl(120, 100%, 40%);">+ state->in_stream.err_ts_ctr->current,</span><br><span style="color: hsl(120, 100%, 40%);">+ state->out_stream.err_ts_ctr->current,</span><br><span style="color: hsl(120, 100%, 40%);">+ VTY_NEWLINE,</span><br><span> end->stats.dropped_packets, VTY_NEWLINE,</span><br><span> codec->payload_type, codec->rate, codec->channels, VTY_NEWLINE,</span><br><span> codec->frame_duration_num, codec->frame_duration_den,</span><br><span>diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c</span><br><span>index f6c421a..67c5f66 100644</span><br><span>--- a/tests/mgcp/mgcp_test.c</span><br><span>+++ b/tests/mgcp/mgcp_test.c</span><br><span>@@ -1129,10 +1129,12 @@</span><br><span> uint32_t last_ssrc = 0;</span><br><span> uint32_t last_timestamp = 0;</span><br><span> uint32_t last_seqno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- int last_in_ts_err_cnt = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- int last_out_ts_err_cnt = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint64_t last_in_ts_err_cnt = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint64_t last_out_ts_err_cnt = 0;</span><br><span> struct mgcp_conn_rtp *conn = NULL;</span><br><span> struct mgcp_conn *_conn = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct rate_ctr test_ctr_in;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct rate_ctr test_ctr_out;</span><br><span> </span><br><span> printf("Testing packet error detection%s%s.\n",</span><br><span> patch_ssrc ? ", patch SSRC" : "",</span><br><span>@@ -1142,6 +1144,11 @@</span><br><span> memset(&endp, 0, sizeof(endp));</span><br><span> memset(&state, 0, sizeof(state));</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ memset(&test_ctr_in, 0, sizeof(test_ctr_in));</span><br><span style="color: hsl(120, 100%, 40%);">+ memset(&test_ctr_out, 0, sizeof(test_ctr_out));</span><br><span style="color: hsl(120, 100%, 40%);">+ state.in_stream.err_ts_ctr = &test_ctr_in;</span><br><span style="color: hsl(120, 100%, 40%);">+ state.out_stream.err_ts_ctr = &test_ctr_out;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> endp.type = &ep_typeset.rtp;</span><br><span> </span><br><span> trunk.vty_number_endpoints = 1;</span><br><span>@@ -1186,18 +1193,18 @@</span><br><span> state.in_stream.last_tsdelta, state.in_stream.last_seq);</span><br><span> </span><br><span> printf("Out TS change: %d, dTS: %d, Seq change: %d, "</span><br><span style="color: hsl(0, 100%, 40%);">- "TS Err change: in %+d, out %+d\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ "TS Err change: in +%u, out +%u\n",</span><br><span> state.out_stream.last_timestamp - last_timestamp,</span><br><span> state.out_stream.last_tsdelta,</span><br><span> state.out_stream.last_seq - last_seqno,</span><br><span style="color: hsl(0, 100%, 40%);">- state.in_stream.err_ts_counter - last_in_ts_err_cnt,</span><br><span style="color: hsl(0, 100%, 40%);">- state.out_stream.err_ts_counter - last_out_ts_err_cnt);</span><br><span style="color: hsl(120, 100%, 40%);">+ (unsigned int) (state.in_stream.err_ts_ctr->current - last_in_ts_err_cnt),</span><br><span style="color: hsl(120, 100%, 40%);">+ (unsigned int) (state.out_stream.err_ts_ctr->current - last_out_ts_err_cnt));</span><br><span> </span><br><span> printf("Stats: Jitter = %u, Transit = %d\n",</span><br><span> calc_jitter(&state), state.stats.transit);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- last_in_ts_err_cnt = state.in_stream.err_ts_counter;</span><br><span style="color: hsl(0, 100%, 40%);">- last_out_ts_err_cnt = state.out_stream.err_ts_counter;</span><br><span style="color: hsl(120, 100%, 40%);">+ last_in_ts_err_cnt = state.in_stream.err_ts_ctr->current;</span><br><span style="color: hsl(120, 100%, 40%);">+ last_out_ts_err_cnt = state.out_stream.err_ts_ctr->current;</span><br><span> last_timestamp = state.out_stream.last_timestamp;</span><br><span> last_seqno = state.out_stream.last_seq;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/8086">change 8086</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/8086"/><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-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I9fbd65bf2f4d1e015a05996db4c1f7ff20be2c95 </div>
<div style="display:none"> Gerrit-Change-Number: 8086 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>