[Asterisk-code-review] res rtp asterisk: Cache local RTCP address. (asterisk[certified/13.8])

Anonymous Coward asteriskteam at digium.com
Wed Aug 10 10:37:42 CDT 2016


Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: res_rtp_asterisk: Cache local RTCP address.
......................................................................


res_rtp_asterisk: Cache local RTCP address.

When an RTCP packet is sent or received, res_rtp_asterisk generates a
Stasis event that contains the RTCP report as well as the local and
remote addresses that the report pertains to.

The addresses are determined using ast_find_ourip(). For the local
address, this will typically result in a lookup of the hostname of the
server, and then a DNS lookup of that hostname. If you do not have the
host in /etc/hosts, then this results in a full DNS lookup, which can
potentially block for some time.

This is especially problematic when performing RTCP reads, since those
are done on the same thread responsible for reading and writing media.

This patch addresses the issue by performing a lookup of the local
address when RTCP is allocated. We then use this cached local address
for the Stasis events when necessary.

ASTERISK-26280 #close
Reported by Mark Michelson

Change-Id: I3dd61882c2e57036f09f0c390cf38f7c87e9b556
---
M res/res_rtp_asterisk.c
1 file changed, 29 insertions(+), 40 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 35ef37d..c2b8e8d 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -407,6 +407,12 @@
 #ifdef HAVE_OPENSSL_SRTP
 	struct dtls_details dtls; /*!< DTLS state information */
 #endif
+
+	/* Cached local address string allows us to generate
+	 * RTCP stasis messages without having to look up our
+	 * own address every time
+	 */
+	char *local_addr_str;
 };
 
 struct rtp_red {
@@ -2655,6 +2661,7 @@
 		 * RTP instance while it's active.
 		 */
 		close(rtp->rtcp->s);
+		ast_free(rtp->rtcp->local_addr_str);
 		ast_free(rtp->rtcp);
 	}
 
@@ -3063,12 +3070,7 @@
 	int rate = rtp_get_rate(rtp->f.subclass.format);
 	int ice;
 	int header_offset = 0;
-	char *str_remote_address;
-	char *str_local_address;
 	struct ast_sockaddr remote_address = { { 0, } };
-	struct ast_sockaddr local_address = { { 0, } };
-	struct ast_sockaddr real_remote_address = { { 0, } };
-	struct ast_sockaddr real_local_address = { { 0, } };
 	struct ast_rtp_rtcp_report_block *report_block = NULL;
 	RAII_VAR(struct ast_rtp_rtcp_report *, rtcp_report,
 			ast_rtp_rtcp_report_alloc(rtp->themssrc ? 1 : 0),
@@ -3195,22 +3197,9 @@
 		}
 	}
 
-	ast_rtp_instance_get_local_address(instance, &local_address);
-	if (!ast_find_ourip(&real_local_address, &local_address, 0)) {
-		str_local_address = ast_strdupa(ast_sockaddr_stringify(&real_local_address));
-	} else {
-		str_local_address = ast_strdupa(ast_sockaddr_stringify(&local_address));
-	}
-
-	if (!ast_find_ourip(&real_remote_address, &remote_address, 0)) {
-		str_remote_address = ast_strdupa(ast_sockaddr_stringify(&real_remote_address));
-	} else {
-		str_remote_address = ast_strdupa(ast_sockaddr_stringify(&remote_address));
-	}
-
 	message_blob = ast_json_pack("{s: s, s: s}",
-			"to", str_remote_address,
-			"from", str_local_address);
+			"to", ast_sockaddr_stringify(&remote_address),
+			"from", rtp->rtcp->local_addr_str);
 	ast_rtp_publish_rtcp_message(instance, ast_rtp_rtcp_sent_type(),
 			rtcp_report,
 			message_blob);
@@ -4023,11 +4012,6 @@
 	int report_counter = 0;
 	struct ast_rtp_rtcp_report_block *report_block;
 	struct ast_frame *f = &ast_null_frame;
-	char *str_local_address;
-	char *str_remote_address;
-	struct ast_sockaddr local_address = { { 0,} };
-	struct ast_sockaddr real_local_address = { { 0, } };
-	struct ast_sockaddr real_remote_address = { { 0, } };
 
 	/* Read in RTCP data from the socket */
 	if ((res = rtcp_recvfrom(instance, rtcpdata + AST_FRIENDLY_OFFSET,
@@ -4083,8 +4067,6 @@
 	}
 
 	ast_debug(1, "Got RTCP report of %d bytes\n", res);
-
-	ast_rtp_instance_get_local_address(instance, &local_address);
 
 	while (position < packetwords) {
 		int i, pt, rc;
@@ -4201,21 +4183,10 @@
 			/* If and when we handle more than one report block, this should occur outside
 			 * this loop.
 			 */
-			if (!ast_find_ourip(&real_local_address, &local_address, 0)) {
-				str_local_address = ast_strdupa(ast_sockaddr_stringify(&real_local_address));
-			} else {
-				str_local_address = ast_strdupa(ast_sockaddr_stringify(&local_address));
-			}
-
-			if (!ast_find_ourip(&real_remote_address, &addr, 0)) {
-				str_remote_address = ast_strdupa(ast_sockaddr_stringify(&real_remote_address));
-			} else {
-				str_remote_address = ast_strdupa(ast_sockaddr_stringify(&addr));
-			}
 
 			message_blob = ast_json_pack("{s: s, s: s, s: f}",
-					"from", str_remote_address,
-					"to", str_local_address,
+					"from", ast_sockaddr_stringify(&rtp->rtcp->them),
+					"to", rtp->rtcp->local_addr_str,
 					"rtt", rtp->rtcp->rtt);
 			ast_rtp_publish_rtcp_message(instance, ast_rtp_rtcp_received_type(),
 					rtcp_report,
@@ -4772,6 +4743,8 @@
 
 	if (property == AST_RTP_PROPERTY_RTCP) {
 		if (value) {
+			struct ast_sockaddr local_addr;
+
 			if (rtp->rtcp) {
 				ast_debug(1, "Ignoring duplicate RTCP property on RTP instance '%p'\n", instance);
 				return;
@@ -4786,6 +4759,19 @@
 			ast_sockaddr_set_port(&rtp->rtcp->us,
 					      ast_sockaddr_port(&rtp->rtcp->us) + 1);
 
+			if (!ast_find_ourip(&local_addr, &rtp->rtcp->us, 0)) {
+				ast_sockaddr_set_port(&local_addr, ast_sockaddr_port(&rtp->rtcp->us));
+			} else {
+				ast_sockaddr_copy(&local_addr, &rtp->rtcp->us);
+			}
+
+			rtp->rtcp->local_addr_str = ast_strdup(ast_sockaddr_stringify(&local_addr));
+			if (!rtp->rtcp->local_addr_str) {
+				ast_free(rtp->rtcp);
+				rtp->rtcp = NULL;
+				return;
+			}
+
 			if ((rtp->rtcp->s =
 			     create_new_socket("RTCP",
 					       ast_sockaddr_is_ipv4(&rtp->rtcp->us) ?
@@ -4793,6 +4779,7 @@
 					       ast_sockaddr_is_ipv6(&rtp->rtcp->us) ?
 					       AF_INET6 : -1)) < 0) {
 				ast_debug(1, "Failed to create a new socket for RTCP on instance '%p'\n", instance);
+				ast_free(rtp->rtcp->local_addr_str);
 				ast_free(rtp->rtcp);
 				rtp->rtcp = NULL;
 				return;
@@ -4802,6 +4789,7 @@
 			if (ast_bind(rtp->rtcp->s, &rtp->rtcp->us)) {
 				ast_debug(1, "Failed to setup RTCP on RTP instance '%p'\n", instance);
 				close(rtp->rtcp->s);
+				ast_free(rtp->rtcp->local_addr_str);
 				ast_free(rtp->rtcp);
 				rtp->rtcp = NULL;
 				return;
@@ -4841,6 +4829,7 @@
 					SSL_free(rtp->rtcp->dtls.ssl);
 				}
 #endif
+				ast_free(rtp->rtcp->local_addr_str);
 				ast_free(rtp->rtcp);
 				rtp->rtcp = NULL;
 			}

-- 
To view, visit https://gerrit.asterisk.org/3448
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I3dd61882c2e57036f09f0c390cf38f7c87e9b556
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.8
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>



More information about the asterisk-code-review mailing list