From b5e36c223c1bbb2f49c91f06527182ef3b1c057d Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Thu, 24 Mar 2016 13:53:31 -0400 Subject: [PATCH] MT#18599 simplify RTCP parsing Change-Id: Ide82c1c882589f99e10cce28646b835ae3d8831c --- daemon/homer.c | 3 ++ daemon/rtcp.c | 102 +++++++++++++++++++++++++++++------------- daemon/rtcp.h | 34 ++++---------- daemon/rtcp_xr.c | 49 +++++++------------- daemon/rtcp_xr.h | 42 +++-------------- daemon/str.h | 1 + tests/simulator-ng.pl | 5 ++- 7 files changed, 105 insertions(+), 131 deletions(-) diff --git a/daemon/homer.c b/daemon/homer.c index 2f53502ec..b72d19c0b 100644 --- a/daemon/homer.c +++ b/daemon/homer.c @@ -11,6 +11,7 @@ #include "log.h" #include "aux.h" +#include "str.h" @@ -201,6 +202,8 @@ int homer_send(struct homer_sender *hs, GString *s, const str *id, const endpoin if (!s->len) // empty write, shouldn't happen goto out; + ilog(LOG_DEBUG, "JSON to send to Homer: '"STR_FORMAT"'", G_STR_FMT(s)); + if (send_hepv3(s, id, hs->capture_id, src, dst)) goto out; diff --git a/daemon/rtcp.c b/daemon/rtcp.c index 569436317..018f8a076 100644 --- a/daemon/rtcp.c +++ b/daemon/rtcp.c @@ -505,7 +505,15 @@ static void print_rtcp_sr(GString *log, const pjmedia_rtcp_sr* sr, GString *json ntohl(sr->sender_pcount)); } -void print_rtcp_rr(GString *log, const pjmedia_rtcp_rr* rr, pjmedia_rtcp_common *common, GString *json) { +static void print_rtcp_rr_list_start(pjmedia_rtcp_common *common, GString *json) { + if (json) + g_string_append_printf(json, "\"ssrc\":%u,\"type\":%u,\"report_count\":%u,\"report_blocks\":[", + ntohl(common->ssrc), + common->pt, + common->count); +} + +static void print_rtcp_rr(GString *log, const pjmedia_rtcp_rr* rr, GString *json) { /* Get packet loss */ u_int32_t packet_loss=0; packet_loss = (rr->total_lost_2 << 16) + @@ -523,11 +531,9 @@ void print_rtcp_rr(GString *log, const pjmedia_rtcp_rr* rr, pjmedia_rtcp_common ntohl(rr->dlsr)); if (json) - g_string_append_printf(json, "\"ssrc\":%u,\"type\":%u, \"report_blocks\":[{\"source_ssrc\":%u," + g_string_append_printf(json, "{\"source_ssrc\":%u," "\"highest_seq_no\":%u,\"fraction_lost\":%u,\"ia_jitter\":%u," - "\"packets_lost\":%u,\"lsr\":%u,\"dlsr\":%u}],\"report_count\":1,", - ntohl(rr->ssrc), - common->pt, + "\"packets_lost\":%u,\"lsr\":%u,\"dlsr\":%u},", ntohl(rr->ssrc), ntohl(rr->last_seq), rr->fract_lost, @@ -537,18 +543,29 @@ void print_rtcp_rr(GString *log, const pjmedia_rtcp_rr* rr, pjmedia_rtcp_common ntohl(rr->dlsr)); } -void parse_and_log_rtcp_report(struct stream_fd *sfd, const str *s, const endpoint_t *src) { +static void str_sanitize(GString *s) { + while (s->len > 0 && (s->str[s->len - 1] == ' ' || s->str[s->len - 1] == ',')) + g_string_truncate(s, s->len - 1); +} + +static void print_rtcp_rr_list_end(pjmedia_rtcp_common *common, GString *json) { + if (json) { + str_sanitize(json); + g_string_append_printf(json, "],"); + } +} + +void parse_and_log_rtcp_report(struct stream_fd *sfd, const str *ori_s, const endpoint_t *src) { GString *log; - pjmedia_rtcp_common *common = (pjmedia_rtcp_common*) s->s; + str iter_s, comp_s; + pjmedia_rtcp_common *common; const pjmedia_rtcp_rr *rr = NULL; const pjmedia_rtcp_sr *sr = NULL; GString *json; struct call *c = sfd->call; struct callmaster *cm = c->callmaster; - - if (s->len < sizeof(*common)) - return; + int i; log = _log_facility_rtcp ? g_string_new(NULL) : NULL; json = cm->homer ? g_string_new("{ ") : NULL; @@ -560,44 +577,65 @@ void parse_and_log_rtcp_report(struct stream_fd *sfd, const str *s, const endpoi if (log) g_string_append_printf(log, "["STR_FORMAT"] ", STR_FMT(&sfd->stream->call->callid)); - print_rtcp_common(log, common); + iter_s = *ori_s; - /* Parse RTCP */ - if (common->pt == RTCP_PT_SR) { - if (s->len < (sizeof(*common) + sizeof(*sr))) - goto out; + while (iter_s.len) { + // procedure throughout here: first assign, then str_shift with check for + // return value (does the length sanity check), then access values. + // we use iter_s to iterate compound packets and comp_s to access component + // data. - sr = (pjmedia_rtcp_sr*) ((s->s) + sizeof(pjmedia_rtcp_common)); + common = (pjmedia_rtcp_common*) iter_s.s; + comp_s = iter_s; - print_rtcp_sr(log, sr, json); + if (str_shift(&comp_s, sizeof(*common))) // puts comp_s just past the common header + break; + if (str_shift(&iter_s, (ntohs(common->length) + 1) << 2)) // puts iter_s on the next compound packet + break; - if (common->count > 0 && s->len >= (sizeof(pjmedia_rtcp_sr_pkt))) { - rr = (pjmedia_rtcp_rr*)((s->s) + (sizeof(pjmedia_rtcp_common) - + sizeof(pjmedia_rtcp_sr))); - print_rtcp_rr(log, rr, common, json); - } - } else if (common->pt == RTCP_PT_RR && common->count > 0) { - if (s->len < (sizeof(*common) + sizeof(*rr))) - goto out; + print_rtcp_common(log, common); + + /* Parse RTCP */ + switch (common->pt) { + case RTCP_PT_SR: + sr = (pjmedia_rtcp_sr*) ((comp_s.s)); + if (str_shift(&comp_s, sizeof(*sr))) + break; + + print_rtcp_sr(log, sr, json); + // fall through to RTCP_PT_RR + + case RTCP_PT_RR: + print_rtcp_rr_list_start(common, json); - rr = (pjmedia_rtcp_rr*)((s->s) + sizeof(pjmedia_rtcp_common)); - print_rtcp_rr(log, rr, common, json); + for (i = 0; i < common->count; i++) { + rr = (pjmedia_rtcp_rr*)((comp_s.s)); + if (str_shift(&comp_s, sizeof(*rr))) + break; + print_rtcp_rr(log, rr, json); + } - } else if (common->pt == RTCP_PT_XR) { - pjmedia_rtcp_xr_rx_rtcp_xr(log, s); + print_rtcp_rr_list_end(common, json); + break; + + case RTCP_PT_XR: + pjmedia_rtcp_xr_rx_rtcp_xr(log, common, &comp_s); + break; + } } - // XXX parse/support additional RTCP types - if (log) + if (log) { + str_sanitize(log); rtcplog(log->str); + } if (json) { + str_sanitize(json); g_string_append(json, " }"); homer_send(cm->homer, json, &c->callid, src, &sfd->socket.local); json = NULL; } -out: if (json) g_string_free(json, TRUE); if (log) diff --git a/daemon/rtcp.h b/daemon/rtcp.h index 2233a28e7..3bd9d8352 100644 --- a/daemon/rtcp.h +++ b/daemon/rtcp.h @@ -3,6 +3,7 @@ #include "str.h" #include "call.h" +#include struct crypto_context; @@ -30,7 +31,7 @@ typedef struct pjmedia_rtcp_sr u_int32_t rtp_ts; /**< RTP timestamp. */ u_int32_t sender_pcount; /**< Sender packet cound. */ u_int32_t sender_bcount; /**< Sender octet/bytes count. */ -} pjmedia_rtcp_sr; +} __attribute__ ((packed)) pjmedia_rtcp_sr; /** @@ -39,22 +40,24 @@ typedef struct pjmedia_rtcp_sr typedef struct pjmedia_rtcp_rr { u_int32_t ssrc; /**< SSRC identification. */ -#if defined(PJ_IS_BIG_ENDIAN) && PJ_IS_BIG_ENDIAN!=0 +#if G_BYTE_ORDER == G_BIG_ENDIAN u_int32_t fract_lost:8; /**< Fraction lost. */ u_int32_t total_lost_2:8; /**< Total lost, bit 16-23. */ u_int32_t total_lost_1:8; /**< Total lost, bit 8-15. */ u_int32_t total_lost_0:8; /**< Total lost, bit 0-7. */ -#else +#elif G_BYTE_ORDER == G_LITTLE_ENDIAN u_int32_t fract_lost:8; /**< Fraction lost. */ u_int32_t total_lost_2:8; /**< Total lost, bit 0-7. */ u_int32_t total_lost_1:8; /**< Total lost, bit 8-15. */ u_int32_t total_lost_0:8; /**< Total lost, bit 16-23. */ +#else +#error "byte order unknown" #endif u_int32_t last_seq; /**< Last sequence number. */ u_int32_t jitter; /**< Jitter. */ u_int32_t lsr; /**< Last SR. */ u_int32_t dlsr; /**< Delay since last SR. */ -} pjmedia_rtcp_rr; +} __attribute__ ((packed)) pjmedia_rtcp_rr; /** @@ -62,7 +65,7 @@ typedef struct pjmedia_rtcp_rr */ typedef struct pjmedia_rtcp_common { -#if defined(PJ_IS_BIG_ENDIAN) && PJ_IS_BIG_ENDIAN!=0 +#if G_BYTE_ORDER == G_BIG_ENDIAN unsigned version:2; /**< packet type */ unsigned p:1; /**< padding flag */ unsigned count:5; /**< varies by payload type */ @@ -77,27 +80,6 @@ typedef struct pjmedia_rtcp_common u_int32_t ssrc; /**< SSRC identification */ } pjmedia_rtcp_common; -/** - * This structure declares default RTCP packet (SR) that is sent by pjmedia. - * Incoming RTCP packet may have different format, and must be parsed - * manually by application. - */ -typedef struct pjmedia_rtcp_sr_pkt -{ - pjmedia_rtcp_common common; /**< Common header. */ - pjmedia_rtcp_sr sr; /**< Sender report. */ - pjmedia_rtcp_rr rr; /**< variable-length list */ -} pjmedia_rtcp_sr_pkt; - -/** - * This structure declares RTCP RR (Receiver Report) packet. - */ -typedef struct pjmedia_rtcp_rr_pkt -{ - pjmedia_rtcp_common common; /**< Common header. */ - pjmedia_rtcp_rr rr; /**< variable-length list */ -} pjmedia_rtcp_rr_pkt; - int rtcp_avpf2avp(str *); int rtcp_avp2savp(str *, struct crypto_context *); diff --git a/daemon/rtcp_xr.c b/daemon/rtcp_xr.c index 5238687f4..db90dd250 100644 --- a/daemon/rtcp_xr.c +++ b/daemon/rtcp_xr.c @@ -22,16 +22,6 @@ #define BT_VOIP_METRICS 7 -void print_rtcp_xr_common(GString *log, const pjmedia_rtcp_xr_pkt *rtcp_xr) { - g_string_append_printf(log, "version=%u, padding=%u, count=%u, payloadtype=%u, length=%u, ssrc=%u, ", - rtcp_xr->common.version, - rtcp_xr->common.p, - rtcp_xr->common.count, - rtcp_xr->common.pt, - rtcp_xr->common.length, - ntohl(rtcp_xr->common.ssrc)); -} - void print_rtcp_xr_rb_header(GString *log, const pjmedia_rtcp_xr_rb_header *rb_header) { g_string_append_printf(log, "rb_header_blocktype=%u, rb_header_blockspecdata=%u, rb_header_blocklength=%u, ", rb_header->bt, @@ -105,40 +95,33 @@ void print_rtcp_xr_rb_voip_mtc(GString *log, const pjmedia_rtcp_xr_rb_voip_mtc * ntohs(rb_voip_mtc->jb_abs_max)); } -void pjmedia_rtcp_xr_rx_rtcp_xr(GString *log, const str *s) { +void pjmedia_rtcp_xr_rx_rtcp_xr(GString *log, pjmedia_rtcp_common *common, str *s) { - const pjmedia_rtcp_xr_pkt *rtcp_xr = (pjmedia_rtcp_xr_pkt*) s->s; - const pjmedia_rtcp_xr_rb_rr_time *rb_rr_time = NULL; - const pjmedia_rtcp_xr_rb_dlrr *rb_dlrr = NULL; - const pjmedia_rtcp_xr_rb_stats *rb_stats = NULL; - const pjmedia_rtcp_xr_rb_voip_mtc *rb_voip_mtc = NULL; - const pjmedia_rtcp_xr_rb_header *rb_hdr = (pjmedia_rtcp_xr_rb_header*) - rtcp_xr->buf; + const pjmedia_rtcp_xr_rb_rr_time *rb_rr_time; + const pjmedia_rtcp_xr_rb_dlrr *rb_dlrr; + const pjmedia_rtcp_xr_rb_stats *rb_stats; + const pjmedia_rtcp_xr_rb_voip_mtc *rb_voip_mtc; + const pjmedia_rtcp_xr_rb_header *rb_hdr; unsigned pkt_len, rb_len; if (!log) return; - if (s->len < sizeof(*rtcp_xr)) - return; - - if (rtcp_xr->common.pt != RTCP_XR) - return; - - print_rtcp_xr_common(log, rtcp_xr); + // packet length is guaranteed to be valid from upstream - pkt_len = ntohs((u_int16_t)rtcp_xr->common.length); - - if ((pkt_len + 1) > (s->len / 4)) - return; + pkt_len = (ntohs(common->length) + 1) << 2; /* Parse report rpt_types */ - while ((int32_t*)rb_hdr < (int32_t*)s->s + pkt_len) + while (pkt_len >= sizeof(*rb_hdr)) { - rb_len = ntohs((u_int16_t)rb_hdr->length); + rb_hdr = (pjmedia_rtcp_xr_rb_header*) s->s; + rb_len = (ntohs((u_int16_t)rb_hdr->length) + 1) << 2; + if (str_shift(s, rb_len)) + break; + pkt_len -= rb_len; /* Just skip any block with length == 0 (no report content) */ - if (rb_len) { + if (rb_len > 4) { switch (rb_hdr->bt) { case BT_RR_TIME: rb_rr_time = (pjmedia_rtcp_xr_rb_rr_time*) rb_hdr; @@ -160,8 +143,6 @@ void pjmedia_rtcp_xr_rx_rtcp_xr(GString *log, const str *s) { break; } } - rb_hdr = (pjmedia_rtcp_xr_rb_header*) - ((int32_t*)rb_hdr + rb_len + 1); } } diff --git a/daemon/rtcp_xr.h b/daemon/rtcp_xr.h index 52fd95e6b..01a5e136c 100644 --- a/daemon/rtcp_xr.h +++ b/daemon/rtcp_xr.h @@ -13,6 +13,7 @@ #include #include "str.h" +#include "rtcp.h" /** * @defgroup PJMED_RTCP_XR RTCP Extended Report (XR) - RFC 3611 @@ -183,42 +184,6 @@ typedef struct pjmedia_rtcp_xr_rb_voip_mtc } pjmedia_rtcp_xr_rb_voip_mtc; -/** - * Constant of RTCP-XR content size. - */ -#define PJMEDIA_RTCP_XR_BUF_SIZE \ - sizeof(pjmedia_rtcp_xr_rb_rr_time) + \ - sizeof(pjmedia_rtcp_xr_rb_dlrr) + \ - sizeof(pjmedia_rtcp_xr_rb_stats) + \ - sizeof(pjmedia_rtcp_xr_rb_voip_mtc) - - -/** - * This structure declares RTCP XR (Extended Report) packet. - */ -typedef struct pjmedia_rtcp_xr_pkt -{ - struct { -#if defined(PJ_IS_BIG_ENDIAN) && PJ_IS_BIG_ENDIAN!=0 - unsigned version:2; /**< packet type */ - unsigned p:1; /**< padding flag */ - unsigned count:5; /**< varies by payload type */ - unsigned pt:8; /**< payload type */ -#else - unsigned count:5; /**< varies by payload type */ - unsigned p:1; /**< padding flag */ - unsigned version:2; /**< packet type */ - unsigned pt:8; /**< payload type */ -#endif - unsigned length:16; /**< packet length */ - u_int32_t ssrc; /**< SSRC identification */ - } common; - - int8_t buf[PJMEDIA_RTCP_XR_BUF_SIZE]; - /**< Content buffer */ -} pjmedia_rtcp_xr_pkt; - - /** * This function is called internally by RTCP session when it receives * incoming RTCP XR packets. @@ -226,7 +191,10 @@ typedef struct pjmedia_rtcp_xr_pkt * @param rtcp_pkt The received RTCP XR packet. * @param size Size of the incoming packet. */ -void pjmedia_rtcp_xr_rx_rtcp_xr(GString *, const str *s); +void pjmedia_rtcp_xr_rx_rtcp_xr(GString *, pjmedia_rtcp_common *common, str *s); + + +#pragma pack() #endif /* RTCP_XR_H_ */ diff --git a/daemon/str.h b/daemon/str.h index c18c60557..3993951d6 100644 --- a/daemon/str.h +++ b/daemon/str.h @@ -23,6 +23,7 @@ typedef struct _str str; #define STR_FORMAT "%.*s" #define STR_FMT(str) (str)->len, (str)->s #define STR_FMT0(str) ((str) ? (str)->len : 6), ((str) ? (str)->s : "(NULL)") +#define G_STR_FMT(gstr) (int) (gstr)->len, (gstr)->str // for glib GString #define STR_NULL ((str) { NULL, 0 }) #define STR_EMPTY ((str) { "", 0 }) #define STR_CONST_INIT(str) { str, sizeof(str)-1 } diff --git a/tests/simulator-ng.pl b/tests/simulator-ng.pl index e2894519a..983081ac7 100755 --- a/tests/simulator-ng.pl +++ b/tests/simulator-ng.pl @@ -226,8 +226,9 @@ sub rtcp_sr { my $secs = $now[0] + 2208988800; my $frac = $now[1] / 1000000 * 2**32; my $sr = pack('CCnN NNN NN', (2 << 6) | 1, 200, 12, rand(2**32), $secs, $frac, - 12345, 0, 0); - $sr .= pack('N CCCC NNNN', 0, 0, 0, 0, 0, 0, 0, 0, 0); + 12345, rand(12345), rand(4321)); + $sr .= pack('N CCCC NNNN', rand(2**32), rand(256), rand(256), rand(256), rand(256), + rand(2**32), rand(2**32), rand(2**32), rand(2**32)); return $sr; }