osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-05-09 Thread Harald Welte 

Patch Set 1:

(2 comments)

https://gerrit.osmocom.org/#/c/8086/1/src/libosmo-mgcp/mgcp_conn.c
File src/libosmo-mgcp/mgcp_conn.c:

Line 139:   conn_rtp->state.in_stream.err_ts_ctr =
no need to break those lines, we don't have 80 columns limits anymore


https://gerrit.osmocom.org/#/c/8086/1/src/libosmo-mgcp/mgcp_network.c
File src/libosmo-mgcp/mgcp_network.c:

Line 517:   state->out_stream.last_tsdelta = 0;
this somehow doesn't look like counter related changes?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fbd65bf2f4d1e015a05996db4c1f7ff20be2c95
Gerrit-PatchSet: 1
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes

[PATCH] osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-05-09 Thread dexter 

Review at  https://gerrit.osmocom.org/8086

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, 65 insertions(+), 17 deletions(-)


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

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 
 #include 
 #include 
+#include 
 
 #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..0055049 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -26,7 +26,27 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+
+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
@@ -87,6 +107,10 @@
 static void mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct 
mgcp_conn *conn)
 {
struct mgcp_rtp_end *end = _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 +132,15 @@
 
mgcp_rtp_codec_init(>codec);
mgcp_rtp_codec_init(>alt_codec);
+
+   conn_rtp->rate_ctr_group =
+   rate_ctr_group_alloc(conn, _ctr_group_desc,
+rate_ctr_index);
+   conn_rtp->state.in_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[0];
+   conn_rtp->state.out_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[1];
+   rate_ctr_index++;
 }
 
 /* Cleanup rtp connection struct */
@@ -116,6 +149,7 @@
osmux_disable_conn(conn_rtp);
osmux_release_cid(conn_rtp);
mgcp_free_rtp_port(_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 6923b97..c56e433 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 "
@@ -505,13 +505,16 @@
mgcp_rtp_annex_count(endp, state, seq, transit, ssrc);
 
if (!state->initialized) {
+   /* FIXME: Move this initialization to mgcp.conn.c */

[MERGED] osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-04-18 Thread Harald Welte 
Harald Welte has submitted this change and it was merged.

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: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
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, 63 insertions(+), 15 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 0da2c56..ff02768 100644
--- a/include/osmocom/mgcp/mgcp_internal.h
+++ b/include/osmocom/mgcp/mgcp_internal.h
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #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..0055049 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -26,7 +26,27 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+
+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
@@ -87,6 +107,10 @@
 static void mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct 
mgcp_conn *conn)
 {
struct mgcp_rtp_end *end = _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 +132,15 @@
 
mgcp_rtp_codec_init(>codec);
mgcp_rtp_codec_init(>alt_codec);
+
+   conn_rtp->rate_ctr_group =
+   rate_ctr_group_alloc(conn, _ctr_group_desc,
+rate_ctr_index);
+   conn_rtp->state.in_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[0];
+   conn_rtp->state.out_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[1];
+   rate_ctr_index++;
 }
 
 /* Cleanup rtp connection struct */
@@ -116,6 +149,7 @@
osmux_disable_conn(conn_rtp);
osmux_release_cid(conn_rtp);
mgcp_free_rtp_port(_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 6923b97..c56e433 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 "
@@ -505,13 +505,16 @@

osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-04-18 Thread Harald Welte 

Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
Gerrit-PatchSet: 4
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

[PATCH] osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-04-17 Thread dexter 
Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7555

to look at the new patch set (#4).

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: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
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, 63 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/55/7555/4

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 
 #include 
 #include 
+#include 
 
 #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..0055049 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -26,7 +26,27 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+
+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
@@ -87,6 +107,10 @@
 static void mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct 
mgcp_conn *conn)
 {
struct mgcp_rtp_end *end = _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 +132,15 @@
 
mgcp_rtp_codec_init(>codec);
mgcp_rtp_codec_init(>alt_codec);
+
+   conn_rtp->rate_ctr_group =
+   rate_ctr_group_alloc(conn, _ctr_group_desc,
+rate_ctr_index);
+   conn_rtp->state.in_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[0];
+   conn_rtp->state.out_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[1];
+   rate_ctr_index++;
 }
 
 /* Cleanup rtp connection struct */
@@ -116,6 +149,7 @@
osmux_disable_conn(conn_rtp);
osmux_release_cid(conn_rtp);
mgcp_free_rtp_port(_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 6923b97..c56e433 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 "
@@ -505,13 +505,16 @@
mgcp_rtp_annex_count(endp, state, seq, transit, ssrc);

osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-04-17 Thread Harald Welte 

Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/7555/3/src/libosmo-mgcp/mgcp_conn.c
File src/libosmo-mgcp/mgcp_conn.c:

Line 37:.name = "outstream_err_ts_ctr",
in_stream with underscore, but outstream without? Let's try to make this 
consistent.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
Gerrit-PatchSet: 3
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: neels 
Gerrit-HasComments: Yes

[PATCH] osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-04-17 Thread dexter 
Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7555

to look at the new patch set (#3).

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: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
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, 63 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/55/7555/3

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 
 #include 
 #include 
+#include 
 
 #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..8f5c499 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -26,7 +26,27 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+
+const static struct rate_ctr_desc rate_ctr_desc[] = {
+   {
+   .name = "in_stream_err_ts_ctr",
+   .description = "inbound rtp-stream timestamp errors",
+   },{
+   .name = "outstream_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
@@ -87,6 +107,10 @@
 static void mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct 
mgcp_conn *conn)
 {
struct mgcp_rtp_end *end = _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 +132,15 @@
 
mgcp_rtp_codec_init(>codec);
mgcp_rtp_codec_init(>alt_codec);
+
+   conn_rtp->rate_ctr_group =
+   rate_ctr_group_alloc(conn, _ctr_group_desc,
+rate_ctr_index);
+   conn_rtp->state.in_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[0];
+   conn_rtp->state.out_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[1];
+   rate_ctr_index++;
 }
 
 /* Cleanup rtp connection struct */
@@ -116,6 +149,7 @@
osmux_disable_conn(conn_rtp);
osmux_release_cid(conn_rtp);
mgcp_free_rtp_port(_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 6923b97..c56e433 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 "
@@ -505,13 +505,16 @@
mgcp_rtp_annex_count(endp, state, seq, transit, ssrc);

osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-04-16 Thread Harald Welte 

Patch Set 2:

comments were not addressed, patch seems to just have been rebased.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
Gerrit-PatchSet: 2
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-04-12 Thread Harald Welte 

Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.osmocom.org/#/c/7555/1/include/osmocom/mgcp/mgcp_internal.h
File include/osmocom/mgcp/mgcp_internal.h:

Line 207:   struct rate_ctr_group_desc rate_ctr_group_desc;
the rate_ctr_group_desc and rate_ctr_desc should be global data (if not even 
const?).  Wat is the rationale of copying it into each and every mgcp_state?  
Only the "rate_ctr_group *" should be here, AFAICT.


https://gerrit.osmocom.org/#/c/7555/1/src/libosmo-mgcp/mgcp_conn.c
File src/libosmo-mgcp/mgcp_conn.c:

Line 119:   talloc_strdup(conn, "conn_rtp");
all of those strings should be static/const strings, no copying or dynamic 
allocations involved.  Did you check existing examples about how we use 
rate_ctr in the osmocom code?  If there are any examples like thism, please let 
me know, as we need to fix it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
Gerrit-PatchSet: 1
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes

[PATCH] osmo-mgw[master]: stats: use libosmocore rate counter for in/out_stream.err_ts...

2018-03-28 Thread dexter 

Review at  https://gerrit.osmocom.org/7555

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: I67aa7a8602f60366ef3ba2c5b1319b1b85719f64
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, 62 insertions(+), 15 deletions(-)


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

diff --git a/include/osmocom/mgcp/mgcp_internal.h 
b/include/osmocom/mgcp/mgcp_internal.h
index 0da2c56..6fa1e7a 100644
--- a/include/osmocom/mgcp/mgcp_internal.h
+++ b/include/osmocom/mgcp/mgcp_internal.h
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #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,10 @@
uint32_t octets;
} stats;
} osmux;
+
+   struct rate_ctr_group_desc rate_ctr_group_desc;
+   struct rate_ctr_desc rate_ctr_desc[2];
+   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..3762b32 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* Allocate a new connection identifier. According to RFC3435, they must
@@ -87,6 +88,10 @@
 static void mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct 
mgcp_conn *conn)
 {
struct mgcp_rtp_end *end = _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 +113,31 @@
 
mgcp_rtp_codec_init(>codec);
mgcp_rtp_codec_init(>alt_codec);
+
+   /* Set up rate counters */
+   conn_rtp->rate_ctr_group_desc.group_name_prefix =
+   talloc_strdup(conn, "conn_rtp");
+   conn_rtp->rate_ctr_group_desc.group_description =
+   talloc_strdup(conn, "rtp connection statistics");
+   conn_rtp->rate_ctr_group_desc.class_id = 1;
+   conn_rtp->rate_ctr_group_desc.num_ctr = 2;
+   conn_rtp->rate_ctr_group_desc.ctr_desc = conn_rtp->rate_ctr_desc;
+   conn_rtp->rate_ctr_desc[0].name =
+   talloc_strdup(conn, "in_stream_err_ts_ctr");
+   conn_rtp->rate_ctr_desc[0].description =
+   talloc_strdup(conn, "inbound rtp-stream timestamp errors");
+   conn_rtp->rate_ctr_desc[1].name =
+   talloc_strdup(conn, "outstream_err_ts_ctr");
+   conn_rtp->rate_ctr_desc[1].description =
+   talloc_strdup(conn, "outbound rtp stream timestamp errors");
+   conn_rtp->rate_ctr_group =
+   rate_ctr_group_alloc(conn, _rtp->rate_ctr_group_desc,
+rate_ctr_index);
+   conn_rtp->state.in_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[0];
+   conn_rtp->state.out_stream.err_ts_ctr =
+   _rtp->rate_ctr_group->ctr[1];
+   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(_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 6923b97..c56e433 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);