<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7237">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Kevin Harwell: Looks good to me, but someone else must approve
Richard Mudgett: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_rtp_asterisk.c: Fix rtp source address learning for broken clients<br><br>Some clients do not send rtp packets every ptime ms. This can lead to<br>situations in which the rtp source learning algorithm will never learn<br>the address of the client. This has been discovered on a Mac mini with<br>a pjsip based softphone after updating to Sierra: as soon as USB<br>headsets are involved, the softphone will send the second packet 30ms<br>after the first, the third 30ms after the second and the fourth 1ms<br>after the third. So in the old implmentation the rtp source learning<br>algorithm was repeatedly reset on the fourth packet.<br><br>The patch changes the algorithm in a way that doesn't take the arrival<br>time between two consecutive packets into account but the time between<br>the first and the last packet of a learning sequence.<br><br>The patch also fixes a second problem: when a user was using a wrong<br>value for the probation setting there was a LOG_WARNING output stating<br>that the value had been set to the default value instead. However<br>the code for setting the value back to defaults was missing.<br><br>ASTERISK-27421 #close<br><br>Change-Id: If778fe07678a6fd2041eaca7cd78267d0ef4fc6c<br>---<br>M res/res_rtp_asterisk.c<br>1 file changed, 35 insertions(+), 14 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 a9bdf68..c58ac6f 100644<br>--- a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -114,6 +114,20 @@<br> #define ZFONE_PROFILE_ID 0x505a<br> <br> #define DEFAULT_LEARNING_MIN_SEQUENTIAL 4<br>+/*!<br>+ * \brief Calculate the min learning duration in ms.<br>+ *<br>+ * \details<br>+ * The min supported packet size represents 10 ms and we need to account<br>+ * for some jitter and fast clocks while learning. Some messed up devices<br>+ * have very bad jitter for a small packet sample size. Jitter can also<br>+ * be introduced by the network itself.<br>+ *<br>+ * So we'll allow packets to come in every 9ms on average for fast clocking<br>+ * with the last one coming in 5ms early for jitter.<br>+ */<br>+#define CALC_LEARNING_MIN_DURATION(count) (((count) - 1) * 9 - 5)<br>+#define DEFAULT_LEARNING_MIN_DURATION CALC_LEARNING_MIN_DURATION(DEFAULT_LEARNING_MIN_SEQUENTIAL)<br> <br> #define SRTP_MASTER_KEY_LEN 16<br> #define SRTP_MASTER_SALT_LEN 14<br>@@ -150,6 +164,7 @@<br> #endif<br> static int strictrtp = DEFAULT_STRICT_RTP; /*!< Only accept RTP frames from a defined source. If we receive an indication of a changing source, enter learning mode. */<br> static int learning_min_sequential = DEFAULT_LEARNING_MIN_SEQUENTIAL; /*!< Number of sequential RTP frames needed from a single source during learning mode to accept new source. */<br>+static int learning_min_duration = DEFAULT_LEARNING_MIN_DURATION; /*!< Lowest acceptable timeout between the first and the last sequential RTP frame. */<br> #ifdef HAVE_PJPROJECT<br> static int icesupport = DEFAULT_ICESUPPORT;<br> static struct sockaddr_in stunaddr;<br>@@ -230,7 +245,7 @@<br> struct rtp_learning_info {<br> struct ast_sockaddr proposed_address; /*!< Proposed remote address for strict RTP */<br> struct timeval start; /*!< The time learning mode was started */<br>- struct timeval received; /*!< The time of the last received packet */<br>+ struct timeval received; /*!< The time of the first received packet */<br> int max_seq; /*!< The highest sequence number received */<br> int packets; /*!< The number of remaining packets before the source is accepted */<br> };<br>@@ -2782,25 +2797,28 @@<br> */<br> static int rtp_learning_rtp_seq_update(struct rtp_learning_info *info, uint16_t seq)<br> {<br>- /*<br>- * During the learning mode 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>- if (!ast_tvzero(info->received) && ast_tvdiff_ms(ast_tvnow(), info->received) < 5) {<br>- /*<br>- * Reject a flood of packets as acceptable for learning.<br>- * Reset the needed packets.<br>- */<br>- info->packets = learning_min_sequential - 1;<br>- } else if (seq == (uint16_t) (info->max_seq + 1)) {<br>+ if (seq == (uint16_t) (info->max_seq + 1)) {<br> /* packet is in sequence */<br> info->packets--;<br> } else {<br> /* Sequence discontinuity; reset */<br> info->packets = learning_min_sequential - 1;<br>+ info->received = ast_tvnow();<br>+ }<br>+<br>+ /*<br>+ * Protect against packet floods by checking that we<br>+ * received the packet sequence in at least the minimum<br>+ * allowed time.<br>+ */<br>+ if (ast_tvzero(info->received)) {<br>+ info->received = ast_tvnow();<br>+ } else if (!info->packets && (ast_tvdiff_ms(ast_tvnow(), info->received) < learning_min_duration )) {<br>+ /* Packet flood; reset */<br>+ info->packets = learning_min_sequential - 1;<br>+ info->received = ast_tvnow();<br> }<br> info->max_seq = seq;<br>- info->received = ast_tvnow();<br> <br> return info->packets;<br> }<br>@@ -6483,6 +6501,7 @@<br> dtmftimeout = DEFAULT_DTMF_TIMEOUT;<br> strictrtp = DEFAULT_STRICT_RTP;<br> learning_min_sequential = DEFAULT_LEARNING_MIN_SEQUENTIAL;<br>+ learning_min_duration = DEFAULT_LEARNING_MIN_DURATION;<br> <br> /** This resource is not "reloaded" so much as unloaded and loaded again.<br> * In the case of the TURN related variables, the memory referenced by a<br>@@ -6547,10 +6566,12 @@<br> strictrtp = ast_true(s);<br> }<br> if ((s = ast_variable_retrieve(cfg, "general", "probation"))) {<br>- if ((sscanf(s, "%d", &learning_min_sequential) <= 0) || learning_min_sequential <= 0) {<br>+ if ((sscanf(s, "%d", &learning_min_sequential) != 1) || learning_min_sequential <= 1) {<br> ast_log(LOG_WARNING, "Value for 'probation' could not be read, using default of '%d' instead\n",<br> DEFAULT_LEARNING_MIN_SEQUENTIAL);<br>+ learning_min_sequential = DEFAULT_LEARNING_MIN_SEQUENTIAL;<br> }<br>+ learning_min_duration = CALC_LEARNING_MIN_DURATION(learning_min_sequential);<br> }<br> #ifdef HAVE_PJPROJECT<br> if ((s = ast_variable_retrieve(cfg, "general", "icesupport"))) {<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7237">change 7237</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/7237"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: If778fe07678a6fd2041eaca7cd78267d0ef4fc6c </div>
<div style="display:none"> Gerrit-Change-Number: 7237 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Pirmin Walthert <infos@nappsoft.ch> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>