[Asterisk-code-review] res rtp asterisk.c: Fix rtp source address learning for brok... (asterisk[master])
Pirmin Walthert
asteriskteam at digium.com
Sat Nov 18 02:53:50 CST 2017
Pirmin Walthert has uploaded this change for review. ( https://gerrit.asterisk.org/7276
Change subject: res_rtp_asterisk.c: Fix rtp source address learning for broken clients
......................................................................
res_rtp_asterisk.c: Fix rtp source address learning for broken clients
Some clients do not send rtp packets every ptime ms. This can lead to
situations in which the rtp source learning algorithm will never learn
the address of the client. This has been discovered on a Mac mini with
a pjsip based softphone after updating to Sierra: as soon as USB
headsets are involved, the softphone will send the second packet 30ms
after the first, the third 30ms after the second and the fourth 1ms
after the third. So in the old implmentation the rtp source learning
algorithm was repeatedly reset on the fourth packet.
The patch changes the algorithm in a way that doesn't take the arrival
time between two consecutive packets into account but the time between
the first and the last packet of a learning sequence.
The patch also fixes a second problem: when a user was using a wrong
value for the probation setting there was a LOG_WARNING output stating
that the value had been set to the default value instead. However
the code for setting the value back to defaults was missing.
ASTERISK-27421 #close
Change-Id: If778fe07678a6fd2041eaca7cd78267d0ef4fc6c
---
M res/res_rtp_asterisk.c
1 file changed, 35 insertions(+), 14 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/76/7276/1
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 15ca150..e28eb0a 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -113,6 +113,20 @@
#define ZFONE_PROFILE_ID 0x505a
#define DEFAULT_LEARNING_MIN_SEQUENTIAL 4
+/*!
+ * \brief Calculate the min learning duration in ms.
+ *
+ * \details
+ * The min supported packet size represents 10 ms and we need to account
+ * for some jitter and fast clocks while learning. Some messed up devices
+ * have very bad jitter for a small packet sample size. Jitter can also
+ * be introduced by the network itself.
+ *
+ * So we'll allow packets to come in every 9ms on average for fast clocking
+ * with the last one coming in 5ms early for jitter.
+ */
+#define CALC_LEARNING_MIN_DURATION(count) (((count) - 1) * 9 - 5)
+#define DEFAULT_LEARNING_MIN_DURATION CALC_LEARNING_MIN_DURATION(DEFAULT_LEARNING_MIN_SEQUENTIAL)
#define SRTP_MASTER_KEY_LEN 16
#define SRTP_MASTER_SALT_LEN 14
@@ -151,6 +165,7 @@
#endif
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. */
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. */
+static int learning_min_duration = DEFAULT_LEARNING_MIN_DURATION; /*!< Lowest acceptable timeout between the first and the last sequential RTP frame. */
#ifdef HAVE_PJPROJECT
static int icesupport = DEFAULT_ICESUPPORT;
static struct sockaddr_in stunaddr;
@@ -231,7 +246,7 @@
struct rtp_learning_info {
struct ast_sockaddr proposed_address; /*!< Proposed remote address for strict RTP */
struct timeval start; /*!< The time learning mode was started */
- struct timeval received; /*!< The time of the last received packet */
+ struct timeval received; /*!< The time of the first received packet */
int max_seq; /*!< The highest sequence number received */
int packets; /*!< The number of remaining packets before the source is accepted */
};
@@ -3065,25 +3080,28 @@
*/
static int rtp_learning_rtp_seq_update(struct rtp_learning_info *info, uint16_t seq)
{
- /*
- * During the learning mode the minimum amount of media we'll accept is
- * 10ms so give a reasonable 5ms buffer just in case we get it sporadically.
- */
- if (!ast_tvzero(info->received) && ast_tvdiff_ms(ast_tvnow(), info->received) < 5) {
- /*
- * Reject a flood of packets as acceptable for learning.
- * Reset the needed packets.
- */
- info->packets = learning_min_sequential - 1;
- } else if (seq == (uint16_t) (info->max_seq + 1)) {
+ if (seq == (uint16_t) (info->max_seq + 1)) {
/* packet is in sequence */
info->packets--;
} else {
/* Sequence discontinuity; reset */
info->packets = learning_min_sequential - 1;
+ info->received = ast_tvnow();
+ }
+
+ /*
+ * Protect against packet floods by checking that we
+ * received the packet sequence in at least the minimum
+ * allowed time.
+ */
+ if (ast_tvzero(info->received)) {
+ info->received = ast_tvnow();
+ } else if (!info->packets && (ast_tvdiff_ms(ast_tvnow(), info->received) < learning_min_duration )) {
+ /* Packet flood; reset */
+ info->packets = learning_min_sequential - 1;
+ info->received = ast_tvnow();
}
info->max_seq = seq;
- info->received = ast_tvnow();
return info->packets;
}
@@ -7140,6 +7158,7 @@
dtmftimeout = DEFAULT_DTMF_TIMEOUT;
strictrtp = DEFAULT_STRICT_RTP;
learning_min_sequential = DEFAULT_LEARNING_MIN_SEQUENTIAL;
+ learning_min_duration = DEFAULT_LEARNING_MIN_DURATION;
/** This resource is not "reloaded" so much as unloaded and loaded again.
* In the case of the TURN related variables, the memory referenced by a
@@ -7204,10 +7223,12 @@
strictrtp = ast_true(s);
}
if ((s = ast_variable_retrieve(cfg, "general", "probation"))) {
- if ((sscanf(s, "%d", &learning_min_sequential) <= 0) || learning_min_sequential <= 0) {
+ if ((sscanf(s, "%d", &learning_min_sequential) != 1) || learning_min_sequential <= 1) {
ast_log(LOG_WARNING, "Value for 'probation' could not be read, using default of '%d' instead\n",
DEFAULT_LEARNING_MIN_SEQUENTIAL);
+ learning_min_sequential = DEFAULT_LEARNING_MIN_SEQUENTIAL;
}
+ learning_min_duration = CALC_LEARNING_MIN_DURATION(learning_min_sequential);
}
#ifdef HAVE_PJPROJECT
if ((s = ast_variable_retrieve(cfg, "general", "icesupport"))) {
--
To view, visit https://gerrit.asterisk.org/7276
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If778fe07678a6fd2041eaca7cd78267d0ef4fc6c
Gerrit-Change-Number: 7276
Gerrit-PatchSet: 1
Gerrit-Owner: Pirmin Walthert <infos at nappsoft.ch>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171118/7666727e/attachment.html>
More information about the asterisk-code-review
mailing list