Change in osmo-mgw[master]: add aggregated rtp connection stats to osmo-mgw

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/11519 )

Change subject: add aggregated rtp connection stats to osmo-mgw
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

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

https://gerrit.osmocom.org/#/c/11519/1/include/osmocom/mgcp/mgcp.h@197
PS1, Line 197: 	/* Rate counter group which contains stats for processed CDCX commands. */
CRCX


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

https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_conn.c@41
PS1, Line 41: 	[RTP_OCTETS_TX_CTR] = {"rtp:octets_tx", "Outbound rtp octets."},
This looks unrelated. Different patch?


https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_conn.c@248
PS1, Line 248: 	OSMO_ASSERT(conn_stats->desc->num_ctr + 1 == all_stats->desc->num_ctr);
Can you explain this verification?


https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_protocol.c
File src/libosmo-mgcp/mgcp_protocol.c:

https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_protocol.c@108
PS1, Line 108: static const struct rate_ctr_desc all_rtp_conn_rate_ctr_desc[] = {
Let's better use a macro instead of manually keeping them in sync. That's going to break eventually.

Another possibility: Make an init function which creates this dynamically based on the other one and then appends RTP_NUM_CONNECTIONS.


https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_protocol.c@1659
PS1, Line 1659: 	alloc_rate_counters(tcfg, tcfg->cfg);
why removing mgcp here and other places?



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I80d36181600901ae2e0f321dc02b5d54ddc94139
Gerrit-Change-Number: 11519
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 30 Oct 2018 14:38:14 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181030/42f649cb/attachment.htm>