From 2d7298182db8ce8ed84749c89e82f412cacda60f Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Tue, 15 Sep 2015 18:30:26 +0200 Subject: mgcp: Make the packet loss calculation per session Somehow we end up with calculations of 65536 for the packet loss that is RTP_SEQ_MOD. So there might have been a sequence overflow but no packets to count against it. I have no idea how this can happen and could not reproduce it. But when we change the SSRC we should re-initialize the state. Separate the total packets and octets from the packets we count per SSRC. Related: OW#1489 --- openbsc/include/openbsc/mgcp_internal.h | 4 ++-- openbsc/src/libmgcp/mgcp_network.c | 10 ++++++---- openbsc/src/libmgcp/mgcp_protocol.c | 3 +-- openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c | 2 +- openbsc/tests/mgcp/mgcp_test.c | 4 ++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/openbsc/include/openbsc/mgcp_internal.h b/openbsc/include/openbsc/mgcp_internal.h index 1f836595b..076fa4a4b 100644 --- a/openbsc/include/openbsc/mgcp_internal.h +++ b/openbsc/include/openbsc/mgcp_internal.h @@ -72,6 +72,7 @@ struct mgcp_rtp_state { uint32_t stats_jitter; int32_t stats_transit; int stats_cycles; + unsigned int stats_packets; }; struct mgcp_rtp_codec { @@ -273,8 +274,7 @@ void mgcp_rtp_end_config(struct mgcp_endpoint *endp, int expect_ssrc_change, uint32_t mgcp_rtp_packet_duration(struct mgcp_endpoint *endp, struct mgcp_rtp_end *rtp); -void mgcp_state_calc_loss(struct mgcp_rtp_state *s, struct mgcp_rtp_end *, - uint32_t *expected, int *loss); +void mgcp_state_calc_loss(struct mgcp_rtp_state *s, uint32_t *expected, int *loss); uint32_t mgcp_state_calc_jitter(struct mgcp_rtp_state *); /* payload processing default functions */ diff --git a/openbsc/src/libmgcp/mgcp_network.c b/openbsc/src/libmgcp/mgcp_network.c index c39b627c8..9d627aef0 100644 --- a/openbsc/src/libmgcp/mgcp_network.c +++ b/openbsc/src/libmgcp/mgcp_network.c @@ -350,6 +350,7 @@ void mgcp_rtp_annex_count(struct mgcp_endpoint *endp, struct mgcp_rtp_state *sta state->stats_jitter = 0; state->stats_transit = transit; state->stats_cycles = 0; + state->stats_packets = 0; } else { uint16_t udelta; @@ -705,6 +706,7 @@ static int rtp_data_net(struct osmo_fd *fd, unsigned int what) } proto = fd == &endp->net_end.rtp ? MGCP_PROTO_RTP : MGCP_PROTO_RTCP; + endp->net_state.stats_packets += 1; endp->net_end.packets += 1; endp->net_end.octets += rc; @@ -797,6 +799,7 @@ static int rtp_data_bts(struct osmo_fd *fd, unsigned int what) } /* do this before the loop handling */ + endp->bts_state.stats_packets += 1; endp->bts_end.packets += 1; endp->bts_end.octets += rc; @@ -1026,8 +1029,7 @@ int mgcp_free_rtp_port(struct mgcp_rtp_end *end) void mgcp_state_calc_loss(struct mgcp_rtp_state *state, - struct mgcp_rtp_end *end, uint32_t *expected, - int *loss) + uint32_t *expected, int *loss) { *expected = state->stats_cycles + state->stats_max_seq; *expected = *expected - state->stats_base_seq + 1; @@ -1042,8 +1044,8 @@ void mgcp_state_calc_loss(struct mgcp_rtp_state *state, * Make sure the sign is correct and use the biggest * positive/negative number that fits. */ - *loss = *expected - end->packets; - if (*expected < end->packets) { + *loss = *expected - state->stats_packets; + if (*expected < state->stats_packets) { if (*loss > 0) *loss = INT_MIN; } else { diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c index ab98be1d8..4a3ff83c1 100644 --- a/openbsc/src/libmgcp/mgcp_protocol.c +++ b/openbsc/src/libmgcp/mgcp_protocol.c @@ -1522,8 +1522,7 @@ void mgcp_format_stats(struct mgcp_endpoint *endp, char *msg, size_t size) uint32_t expected, jitter; int ploss; int nchars; - mgcp_state_calc_loss(&endp->net_state, &endp->net_end, - &expected, &ploss); + mgcp_state_calc_loss(&endp->net_state, &expected, &ploss); jitter = mgcp_state_calc_jitter(&endp->net_state); nchars = snprintf(msg, size, diff --git a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c index f19cb4c97..c5254c158 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c +++ b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c @@ -342,7 +342,7 @@ static void remember_pending_dlcx(struct nat_sccp_connection *con, uint32_t tran stats->net_os = endp->net_end.octets; stats->bts_pr = endp->bts_end.packets; stats->bts_or = endp->bts_end.octets; - mgcp_state_calc_loss(&endp->bts_state, &endp->bts_end, + mgcp_state_calc_loss(&endp->bts_state, &stats->bts_expected, &stats->bts_loss); stats->bts_jitter = mgcp_state_calc_jitter(&endp->bts_state); diff --git a/openbsc/tests/mgcp/mgcp_test.c b/openbsc/tests/mgcp/mgcp_test.c index b2cb938ce..3a5702b7b 100644 --- a/openbsc/tests/mgcp/mgcp_test.c +++ b/openbsc/tests/mgcp/mgcp_test.c @@ -753,8 +753,8 @@ static void test_packet_loss_calc(void) state.stats_max_seq = pl_test_dat[i].max_seq; state.stats_cycles = pl_test_dat[i].cycles; - rtp.packets = pl_test_dat[i].packets; - mgcp_state_calc_loss(&state, &rtp, &expected, &loss); + state.stats_packets = pl_test_dat[i].packets; + mgcp_state_calc_loss(&state, &expected, &loss); if (loss != pl_test_dat[i].loss || expected != pl_test_dat[i].expected) { printf("FAIL: Wrong exp/loss at idx(%d) Loss(%d vs. %d) Exp(%u vs. %u)\n", -- cgit v1.2.3