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 <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Comment-Date: Tue, 30 Oct 2018 14:38:14 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
