MT#18599 simplify RTCP parsing

Change-Id: Ide82c1c882589f99e10cce28646b835ae3d8831c

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;
 

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)

daemon/rtcp.h

@@ -3,6 +3,7 @@
 
 #include "str.h"
 #include "call.h"
+#include <glib.h>
 
 
 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 *);

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);
 	}
 }
 

daemon/rtcp_xr.h

@@ -13,6 +13,7 @@
 #include <glib.h>
 
 #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_ */

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 }

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;
 }