<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6336">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, 46 insertions(+), 33 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 4bbbc86..5b5e226 100644<br>--- a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -181,6 +181,7 @@<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>+ struct timeval received; /*!< The time of the last received packet */<br> };<br> <br> #ifdef HAVE_OPENSSL_SRTP<br>@@ -264,7 +265,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>@@ -1902,6 +1902,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>@@ -1916,6 +1917,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>@@ -1924,6 +1932,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>@@ -2079,7 +2088,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>@@ -3485,16 +3493,6 @@<br> <br> packetwords = res / 4;<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> ast_debug(1, "Got RTCP report of %d bytes\n", res);<br> <br> while (position < packetwords) {<br>@@ -3514,6 +3512,24 @@<br> if (rtpdebug)<br> ast_debug(1, "RTCP Read too short\n");<br> return &ast_null_frame;<br>+ }<br>+<br>+ if ((rtp->strict_rtp_state != STRICT_RTP_OPEN) && (ntohl(rtcpheader[i + 1]) != 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), ntohl(rtcpheader[i + 1]), 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> if (rtcp_debug_test_addr(&addr)) {<br>@@ -3892,24 +3908,11 @@<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>+ /* 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> /* Hmm, not the strict address. Perhaps we're getting audio from the alternate? */<br> if (!ast_sockaddr_cmp(&rtp->alt_rtp_address, &addr)) {<br>@@ -3921,15 +3924,21 @@<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>+ 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->alt_source_learn.packets);<br>+ rtp, ast_sockaddr_stringify(&addr), rtp->rtp_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>+ 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>@@ -4324,7 +4333,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/6336">change 6336</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/6336"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: certified/11.6 </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: 6336 </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>