chan_sip: RFC compliant retransmission timeout

Retransmission of packets should not be based on how many packets were
sent, but instead on a timeout period.  Depending on whether or not the
packet is for a INVITE or NON-INVITE transaction, the number of packets
sent during the retransmission timeout period will be different, so
timing out based on the number of packets sent is not accurate.

This patch fixes this by removing the retransmit limit and only stopping
retransmission after a timeout period is reached.  By default this
timeout period is 64*(Timer T1) for both INVITE and non-INVITE
transactions.  For more information on sip timer values refer to
RFC3261 Appendix A.

Review: https://reviewboard.asterisk.org/r/749/



git-svn-id: http://svn.digium.com/svn/asterisk/trunk@276219 f38db490-d61c-443f-a65b-d21fe96a405b

2 files changed, 40 insertions, 12 deletions

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 750ade278..5d6d2f198 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -3225,11 +3225,20 @@ static int retrans_pkt(const void *data)
 	struct sip_pkt *pkt = (struct sip_pkt *)data, *prev, *cur = NULL;
 	int reschedule = DEFAULT_RETRANS;
 	int xmitres = 0;
+	/* how many ms until retrans timeout is reached */
+	int64_t diff = pkt->retrans_stop_time - ast_tvdiff_ms(ast_tvnow(), pkt->time_sent);
+
+	/* Do not retransmit if time out is reached. This will be negative if the time between
+	 * the first transmission and now is larger than our timeout period. This is a fail safe
+	 * check in case the scheduler gets behind or the clock is changed. */
+	if ((diff <= 0) || (diff > pkt->retrans_stop_time)) {
+		pkt->retrans_stop = 1;
+	}
 
 	/* Lock channel PVT */
 	sip_pvt_lock(pkt->owner);
 
-	if (pkt->retrans < MAX_RETRANS) {
+	if (!pkt->retrans_stop) {
 		pkt->retrans++;
 		if (!pkt->timer_t1) {	/* Re-schedule using timer_a and timer_t1 */
 			if (sipdebug) {
@@ -3269,19 +3278,35 @@ static int retrans_pkt(const void *data)
 		append_history(pkt->owner, "ReTx", "%d %s", reschedule, pkt->data->str);
 		xmitres = __sip_xmit(pkt->owner, pkt->data, pkt->packetlen);
 		sip_pvt_unlock(pkt->owner);
-		if (xmitres == XMIT_ERROR) {
-			ast_log(LOG_WARNING, "Network error on retransmit in dialog %s\n", pkt->owner->callid);
-		} else {
+
+		/* If there was no error during the network transmission, schedule the next retransmission,
+		 * but if the next retransmission is going to be beyond our timeout period, mark the packet's
+		 * stop_retrans value and set the next retransmit to be the exact time of timeout.  This will
+		 * allow any responses to the packet to be processed before the packet is destroyed on the next
+		 * call to this function by the scheduler. */
+		if (xmitres != XMIT_ERROR) {
+			if (reschedule >= diff) {
+				pkt->retrans_stop = 1;
+				reschedule = diff;
+			}
 			return  reschedule;
 		}
 	}
 
-	/* Too many retries */
+	/* At this point, either the packet's retransmission timed out, or there was a
+	 * transmission error, either way destroy the scheduler item and this packet. */
+
+	pkt->retransid = -1; /* Kill this scheduler item */
+
 	if (pkt->owner && pkt->method != SIP_OPTIONS && xmitres == 0) {
-		if (pkt->is_fatal || sipdebug) {/* Tell us if it's critical or if we're debugging */
-			ast_log(LOG_WARNING, "Maximum retries exceeded on transmission %s for seqno %d (%s %s) -- See doc/sip-retransmit.txt.\n",
-				pkt->owner->callid, pkt->seqno,
-				pkt->is_fatal ? "Critical" : "Non-critical", pkt->is_resp ? "Response" : "Request");
+		if (pkt->is_fatal || sipdebug) { /* Tell us if it's critical or if we're debugging */
+			ast_log(LOG_WARNING, "Retransmission timeout reached on transmission %s for seqno %d (%s %s) -- See doc/sip-retransmit.txt.\n"
+				"Packet timed out after %dms with no response\n",
+				pkt->owner->callid,
+				pkt->seqno,
+				pkt->is_fatal ? "Critical" : "Non-critical",
+				pkt->is_resp ? "Response" : "Request",
+				(int) ast_tvdiff_ms(ast_tvnow(), pkt->time_sent));
 		}
 	} else if (pkt->method == SIP_OPTIONS && sipdebug) {
 		ast_log(LOG_WARNING, "Cancelling retransmit of OPTIONs (call id %s)  -- See doc/sip-retransmit.txt.\n", pkt->owner->callid);
@@ -3294,8 +3319,6 @@ static int retrans_pkt(const void *data)
 		append_history(pkt->owner, "MaxRetries", "%s", pkt->is_fatal ? "(Critical)" : "(Non-critical)");
 	}
 
-	pkt->retransid = -1;
-
 	if (pkt->is_fatal) {
 		while(pkt->owner->owner && ast_channel_trylock(pkt->owner->owner)) {
 			sip_pvt_unlock(pkt->owner);	/* SIP_PVT, not channel */
@@ -3408,6 +3431,9 @@ static enum sip_result __sip_reliable_xmit(struct sip_pvt *p, int seqno, int res
 	if (pkt->timer_t1)
 		siptimer_a = pkt->timer_t1;
 
+	pkt->time_sent = ast_tvnow(); /* time packet was sent */
+	pkt->retrans_stop_time = 64 * (pkt->timer_t1 ? pkt->timer_t1 : DEFAULT_TIMER_T1); /* time in ms after pkt->time_sent to stop retransmission */
+
 	/* Schedule retransmission */
 	AST_SCHED_REPLACE_VARIABLE(pkt->retransid, sched, siptimer_a, retrans_pkt, pkt, 1);
 	if (sipdebug)
diff --git a/channels/sip/include/sip.h b/channels/sip/include/sip.h
index 92fa68e82..2644536cd 100644
--- a/channels/sip/include/sip.h
+++ b/channels/sip/include/sip.h
@@ -83,7 +83,6 @@
 #define DEFAULT_FREQ_NOTOK        10 * 1000   /*!< Qualification: How often to check, if the host is down... */
 
 #define DEFAULT_RETRANS           1000        /*!< How frequently to retransmit Default: 2 * 500 ms in RFC 3261 */
-#define MAX_RETRANS               6           /*!< Try only 6 times for retransmissions, a total of 7 transmissions */
 #define DEFAULT_TIMER_T1          500         /*!< SIP timer T1 (according to RFC 3261) */
 #define SIP_TRANS_TIMEOUT         64 * DEFAULT_TIMER_T1 /*!< SIP request timeout (rfc 3261) 64*T1
                                                          *  \todo Use known T1 for timeout (peerpoke)
@@ -1118,6 +1117,9 @@ struct sip_pkt {
 	int retransid;            /*!< Retransmission ID */
 	int timer_a;              /*!< SIP timer A, retransmission timer */
 	int timer_t1;             /*!< SIP Timer T1, estimated RTT or 500 ms */
+	struct timeval time_sent;  /*!< When pkt was sent */
+	int64_t retrans_stop_time; /*!< Time in ms after 'now' that retransmission must stop */
+	int retrans_stop;         /*!< Timeout is reached, stop retransmission  */
 	int packetlen;            /*!< Length of packet */
 	struct ast_str *data;
 };