Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/8086 )

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

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

The two counters: in_stream.err_ts_counter and out_stream.err_ts_counter
are still handcoded. To make them better accessible they should
 be replaced with libosmocore rate counters.

- replace state.in_stream.err_ts_counter with libosmocore rate counter
- replace state.out_stream.err_ts_counter with libosmocore rate counter

Change-Id: I9fbd65bf2f4d1e015a05996db4c1f7ff20be2c95
Related: OS#2517
---
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, 58 insertions(+), 16 deletions(-)

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



diff --git a/include/osmocom/mgcp/mgcp_internal.h 
b/include/osmocom/mgcp/mgcp_internal.h
index 0da2c56..ff02768 100644
--- a/include/osmocom/mgcp/mgcp_internal.h
+++ b/include/osmocom/mgcp/mgcp_internal.h
@@ -28,6 +28,7 @@
 #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

@@ -45,7 +46,7 @@
        uint32_t ssrc;
        uint16_t last_seq;
        uint32_t last_timestamp;
-       uint32_t err_ts_counter;
+       struct rate_ctr *err_ts_ctr;
        int32_t last_tsdelta;
        uint32_t last_arrival_time;
 };
@@ -202,6 +203,8 @@
                        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 998dbc5..280ee8b 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -26,8 +26,29 @@
 #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>

+enum {
+       IN_STREAM_ERR_TSTMP_CTR,
+       OUT_STREAM_ERR_TSTMP_CTR,
+};
+
+static const struct rate_ctr_desc rate_ctr_desc[] = {
+       [IN_STREAM_ERR_TSTMP_CTR] = {"stream_err_tstmp:in", "Inbound rtp-stream 
timestamp errors."},
+       [OUT_STREAM_ERR_TSTMP_CTR] = {"stream_err_tstmp:out", "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
  * memory for id) */
@@ -87,6 +108,10 @@
 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;
@@ -108,6 +133,11 @@

        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[IN_STREAM_ERR_TSTMP_CTR];
+       conn_rtp->state.out_stream.err_ts_ctr = 
&conn_rtp->rate_ctr_group->ctr[OUT_STREAM_ERR_TSTMP_CTR];
+       rate_ctr_index++;
 }

 /* Cleanup rtp connection struct */
@@ -116,6 +146,7 @@
        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 49e51a1..4144382 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) {
-                       sstate->err_ts_counter += 1;
+                       rate_ctr_inc(sstate->err_ts_ctr);
                        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) {
-               sstate->err_ts_counter += 1;
+               rate_ctr_inc(sstate->err_ts_ctr);
                LOGP(DRTP, LOGL_NOTICE,
                     "The %s timestamp has an alignment error of %d "
                     "on 0x%x SSRC: %u "
diff --git a/src/libosmo-mgcp/mgcp_stat.c b/src/libosmo-mgcp/mgcp_stat.c
index 581130c..cc723bb 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=%u, TO=%u",
-                                 conn->state.in_stream.err_ts_counter,
-                                 conn->state.out_stream.err_ts_counter);
+                                 "\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);
                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 14ecd17..392a176 100644
--- a/src/libosmo-mgcp/mgcp_vty.c
+++ b/src/libosmo-mgcp/mgcp_vty.c
@@ -160,15 +160,16 @@
        struct mgcp_rtp_codec *codec = &end->codec;

        vty_out(vty,
-               "   Timestamp Errs: %d->%d%s"
+               "   Timestamp Errs: %lu->%lu%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_counter,
-               state->out_stream.err_ts_counter, VTY_NEWLINE,
+               state->in_stream.err_ts_ctr->current,
+               state->out_stream.err_ts_ctr->current,
+               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 f6c421a..67c5f66 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1129,10 +1129,12 @@
        uint32_t last_ssrc = 0;
        uint32_t last_timestamp = 0;
        uint32_t last_seqno = 0;
-       int last_in_ts_err_cnt = 0;
-       int last_out_ts_err_cnt = 0;
+       uint64_t last_in_ts_err_cnt = 0;
+       uint64_t 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" : "",
@@ -1142,6 +1144,11 @@
        memset(&endp, 0, sizeof(endp));
        memset(&state, 0, sizeof(state));

+       memset(&test_ctr_in, 0, sizeof(test_ctr_in));
+       memset(&test_ctr_out, 0, sizeof(test_ctr_out));
+       state.in_stream.err_ts_ctr = &test_ctr_in;
+       state.out_stream.err_ts_ctr = &test_ctr_out;
+
        endp.type = &ep_typeset.rtp;

        trunk.vty_number_endpoints = 1;
@@ -1186,18 +1193,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 %+d, out %+d\n",
+                      "TS Err change: in +%u, out +%u\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_counter - last_in_ts_err_cnt,
-                      state.out_stream.err_ts_counter - last_out_ts_err_cnt);
+                      (unsigned int) (state.in_stream.err_ts_ctr->current - 
last_in_ts_err_cnt),
+                      (unsigned int) (state.out_stream.err_ts_ctr->current - 
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_counter;
-               last_out_ts_err_cnt = state.out_stream.err_ts_counter;
+               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_timestamp = state.out_stream.last_timestamp;
                last_seqno = state.out_stream.last_seq;
        }

--
To view, visit https://gerrit.osmocom.org/8086
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9fbd65bf2f4d1e015a05996db4c1f7ff20be2c95
Gerrit-Change-Number: 8086
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pma...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>

Reply via email to