<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6361">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_rtp_asterisk: Only learn a new source in learn state.<br><br>This change moves the logic which learns a new source address<br>for RTP so it only occurs in the learning state. The learning<br>state is entered on initial allocation of RTP or if we are<br>told that the remote address for the media has changed. While<br>in the learning state if we continue to receive media from<br>the original source we restart the learning process. It is<br>only once we receive a sufficient number of RTP packets from<br>the new source that we will switch to it. Once this is done<br>the closed state is entered where all packets that do not<br>originate from the expected source are dropped.<br><br>The learning process has also been improved to take into<br>account the time between received packets so a flood of them<br>while in the learning state does not cause media to be switched.<br><br>Finally RTCP now drops packets which are not for the learned<br>SSRC if strict RTP is enabled.<br><br>ASTERISK-27013<br><br>Change-Id: I56a96e993700906355e79bc880ad9d4ad3ab129c<br>---<br>M res/res_rtp_asterisk.c<br>1 file changed, 82 insertions(+), 69 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c<br>index 65ab902..81443cd 100644<br>--- a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -219,8 +219,9 @@<br> <br> /*! \brief RTP learning mode tracking information */<br> struct rtp_learning_info {<br>- int max_seq; /*!< The highest sequence number received */<br>- int packets; /*!< The number of remaining packets before the source is accepted */<br>+ int max_seq; /*!< The highest sequence number received */<br>+ int packets; /*!< The number of remaining packets before the source is accepted */<br>+ struct timeval received; /*!< The time of the last received packet */<br> };<br> <br> #ifdef HAVE_OPENSSL_SRTP<br>@@ -327,7 +328,6 @@<br> * but these are in place to keep learning mode sequence values sealed from their normal counterparts.<br> */<br> struct rtp_learning_info rtp_source_learn; /* Learning mode track for the expected RTP source */<br>- struct rtp_learning_info alt_source_learn; /* Learning mode tracking for a new RTP source after one has been chosen */<br> <br> struct rtp_red *red;<br> <br>@@ -2822,6 +2822,7 @@<br> {<br> info->max_seq = seq - 1;<br> info->packets = learning_min_sequential;<br>+ memset(&info->received, 0, sizeof(info->received));<br> }<br> <br> /*!<br>@@ -2836,6 +2837,13 @@<br> */<br> static int rtp_learning_rtp_seq_update(struct rtp_learning_info *info, uint16_t seq)<br> {<br>+ if (!ast_tvzero(info->received) && ast_tvdiff_ms(ast_tvnow(), info->received) < 5) {<br>+ /* During the probation period the minimum amount of media we'll accept is<br>+ * 10ms so give a reasonable 5ms buffer just in case we get it sporadically.<br>+ */<br>+ return 1;<br>+ }<br>+<br> if (seq == info->max_seq + 1) {<br> /* packet is in sequence */<br> info->packets--;<br>@@ -2844,6 +2852,7 @@<br> info->packets = learning_min_sequential - 1;<br> }<br> info->max_seq = seq;<br>+ info->received = ast_tvnow();<br> <br> return (info->packets == 0);<br> }<br>@@ -3109,7 +3118,6 @@<br> rtp->strict_rtp_state = (strictrtp ? STRICT_RTP_LEARN : STRICT_RTP_OPEN);<br> if (strictrtp) {<br> rtp_learning_seq_init(&rtp->rtp_source_learn, (uint16_t)rtp->seqno);<br>- rtp_learning_seq_init(&rtp->alt_source_learn, (uint16_t)rtp->seqno);<br> }<br> <br> /* Create a new socket for us to listen on and use */<br>@@ -4774,17 +4782,6 @@<br> <br> packetwords = size / 4;<br> <br>- if (ast_rtp_instance_get_prop(transport, AST_RTP_PROPERTY_NAT)) {<br>- /* Send to whoever sent to us */<br>- if (ast_sockaddr_cmp(&transport_rtp->rtcp->them, addr)) {<br>- ast_sockaddr_copy(&transport_rtp->rtcp->them, addr);<br>- if (rtpdebug) {<br>- ast_debug(0, "RTCP NAT: Got RTCP from other end. Now sending to address %s\n",<br>- ast_sockaddr_stringify(&transport_rtp->rtcp->them));<br>- }<br>- }<br>- }<br>-<br> ast_debug(1, "Got RTCP report of %zu bytes\n", size);<br> <br> while (position < packetwords) {<br>@@ -4838,6 +4835,25 @@<br> /* The child is the parent! We don't need to unlock it. */<br> child = NULL;<br> rtp = transport_rtp;<br>+ }<br>+<br>+ if ((rtp->strict_rtp_state != STRICT_RTP_OPEN) && (rtcp_report->ssrc != rtp->themssrc)) {<br>+ /* Skip over this RTCP record as it does not contain the correct SSRC */<br>+ position += (length + 1);<br>+ ast_debug(1, "%p -- Received RTCP report from %s, dropping due to strict RTP protection. Received SSRC '%u' but expected '%u'\n",<br>+ rtp, ast_sockaddr_stringify(addr), rtcp_report->ssrc, rtp->themssrc);<br>+ continue;<br>+ }<br>+<br>+ if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_NAT)) {<br>+ /* Send to whoever sent to us */<br>+ if (ast_sockaddr_cmp(&rtp->rtcp->them, addr)) {<br>+ ast_sockaddr_copy(&rtp->rtcp->them, addr);<br>+ if (rtpdebug) {<br>+ ast_debug(0, "RTCP NAT: Got RTCP from other end. Now sending to address %s\n",<br>+ ast_sockaddr_stringify(&rtp->rtcp->them));<br>+ }<br>+ }<br> }<br> <br> i += 2; /* Advance past header and ssrc */<br>@@ -5282,59 +5298,6 @@<br> return &ast_null_frame;<br> }<br> <br>- /* If strict RTP protection is enabled see if we need to learn the remote address or if we need to drop the packet */<br>- if (rtp->strict_rtp_state == STRICT_RTP_LEARN) {<br>- ast_debug(1, "%p -- Probation learning mode pass with source address %s\n", rtp, ast_sockaddr_stringify(&addr));<br>- /* For now, we always copy the address. */<br>- ast_sockaddr_copy(&rtp->strict_rtp_address, &addr);<br>-<br>- /* Send the rtp and the seqno from header to rtp_learning_rtp_seq_update to see whether we can exit or not*/<br>- if (rtp_learning_rtp_seq_update(&rtp->rtp_source_learn, seqno)) {<br>- ast_debug(1, "%p -- Probation at seq %d with %d to go; discarding frame\n",<br>- rtp, rtp->rtp_source_learn.max_seq, rtp->rtp_source_learn.packets);<br>- return &ast_null_frame;<br>- }<br>-<br>- ast_verb(4, "%p -- Probation passed - setting RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr));<br>- rtp->strict_rtp_state = STRICT_RTP_CLOSED;<br>- }<br>- if (rtp->strict_rtp_state == STRICT_RTP_CLOSED) {<br>- if (!ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {<br>- /* Always reset the alternate learning source */<br>- rtp_learning_seq_init(&rtp->alt_source_learn, seqno);<br>- } else {<br>- /* Start trying to learn from the new address. If we pass a probationary period with<br>- * it, that means we've stopped getting RTP from the original source and we should<br>- * switch to it.<br>- */<br>- if (rtp_learning_rtp_seq_update(&rtp->alt_source_learn, seqno)) {<br>- ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection. Will switch to it in %d packets\n",<br>- rtp, ast_sockaddr_stringify(&addr), rtp->alt_source_learn.packets);<br>- return &ast_null_frame;<br>- }<br>- ast_verb(4, "%p -- Switching RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr));<br>- ast_sockaddr_copy(&rtp->strict_rtp_address, &addr);<br>- }<br>- }<br>-<br>- /* If symmetric RTP is enabled see if the remote side is not what we expected and change where we are sending audio */<br>- if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_NAT)) {<br>- if (ast_sockaddr_cmp(&remote_address, &addr)) {<br>- /* do not update the originally given address, but only the remote */<br>- ast_rtp_instance_set_incoming_source_address(instance, &addr);<br>- ast_sockaddr_copy(&remote_address, &addr);<br>- if (rtp->rtcp && rtp->rtcp->type == AST_RTP_INSTANCE_RTCP_STANDARD) {<br>- ast_sockaddr_copy(&rtp->rtcp->them, &addr);<br>- ast_sockaddr_set_port(&rtp->rtcp->them, ast_sockaddr_port(&addr) + 1);<br>- }<br>- rtp->rxseqno = 0;<br>- ast_set_flag(rtp, FLAG_NAT_ACTIVE);<br>- if (rtpdebug)<br>- ast_debug(0, "RTP NAT: Got audio from other end. Now sending to address %s\n",<br>- ast_sockaddr_stringify(&remote_address));<br>- }<br>- }<br>-<br> /* If the version is not what we expected by this point then just drop the packet */<br> if (version != 2) {<br> return &ast_null_frame;<br>@@ -5355,6 +5318,52 @@<br> } else {<br> /* The child is the parent! We don't need to unlock it. */<br> child = NULL;<br>+ }<br>+<br>+ /* If strict RTP protection is enabled see if we need to learn the remote address or if we need to drop the packet */<br>+ if (rtp->strict_rtp_state == STRICT_RTP_LEARN) {<br>+ if (!ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {<br>+ /* We are learning a new address but have received traffic from the existing address,<br>+ * accept it but reset the current learning for the new source so it only takes over<br>+ * once sufficient traffic has been received. */<br>+ rtp_learning_seq_init(&rtp->rtp_source_learn, seqno);<br>+ } else {<br>+ /* Start trying to learn from the new address. If we pass a probationary period with<br>+ * it, that means we've stopped getting RTP from the original source and we should<br>+ * switch to it.<br>+ */<br>+ if (rtp_learning_rtp_seq_update(&rtp->rtp_source_learn, seqno)) {<br>+ ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection. Will switch to it in %d packets\n",<br>+ rtp, ast_sockaddr_stringify(&addr), rtp->rtp_source_learn.packets);<br>+ return &ast_null_frame;<br>+ }<br>+ ast_sockaddr_copy(&rtp->strict_rtp_address, &addr);<br>+<br>+ ast_verb(4, "%p -- Probation passed - setting RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr));<br>+ rtp->strict_rtp_state = STRICT_RTP_CLOSED;<br>+ }<br>+ } else if (rtp->strict_rtp_state == STRICT_RTP_CLOSED && ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {<br>+ ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection.\n",<br>+ rtp, ast_sockaddr_stringify(&addr));<br>+ return &ast_null_frame;<br>+ }<br>+<br>+ /* If symmetric RTP is enabled see if the remote side is not what we expected and change where we are sending audio */<br>+ if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_NAT)) {<br>+ if (ast_sockaddr_cmp(&remote_address, &addr)) {<br>+ /* do not update the originally given address, but only the remote */<br>+ ast_rtp_instance_set_incoming_source_address(instance, &addr);<br>+ ast_sockaddr_copy(&remote_address, &addr);<br>+ if (rtp->rtcp && rtp->rtcp->type == AST_RTP_INSTANCE_RTCP_STANDARD) {<br>+ ast_sockaddr_copy(&rtp->rtcp->them, &addr);<br>+ ast_sockaddr_set_port(&rtp->rtcp->them, ast_sockaddr_port(&addr) + 1);<br>+ }<br>+ rtp->rxseqno = 0;<br>+ ast_set_flag(rtp, FLAG_NAT_ACTIVE);<br>+ if (rtpdebug)<br>+ ast_debug(0, "RTP NAT: Got audio from other end. Now sending to address %s\n",<br>+ ast_sockaddr_stringify(&remote_address));<br>+ }<br> }<br> <br> /* If we are currently sending DTMF to the remote party send a continuation packet */<br>@@ -5851,7 +5860,11 @@<br> <br> rtp->rxseqno = 0;<br> <br>- if (strictrtp && rtp->strict_rtp_state != STRICT_RTP_OPEN) {<br>+ if (strictrtp && rtp->strict_rtp_state != STRICT_RTP_OPEN && !ast_sockaddr_isnull(addr) &&<br>+ ast_sockaddr_cmp(addr, &rtp->strict_rtp_address)) {<br>+ /* We only need to learn a new strict source address if we've been told the source is<br>+ * changing to something different.<br>+ */<br> rtp->strict_rtp_state = STRICT_RTP_LEARN;<br> rtp_learning_seq_init(&rtp->rtp_source_learn, rtp->seqno);<br> }<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6361">change 6361</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6361"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15.0 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I56a96e993700906355e79bc880ad9d4ad3ab129c </div>
<div style="display:none"> Gerrit-Change-Number: 6361 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>