[Asterisk-code-review] chan sip: Enable Session-Timers for SIP over TCP (and TLS). (asterisk[14])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: chan_sip: Enable Session-Timers for SIP over TCP (and TLS).
......................................................................


chan_sip: Enable Session-Timers for SIP over TCP (and TLS).

Asterisk defaults to timers=accept/refresher=uas. In that scenario, only in that
scenario, Sessions-Timers (RFC 4028) had no effect via TCP. This change enables
Session-Timers for SIP over TCP (and for SIP over TLS).

However with longer international calls via TCP, the SIP channel might break,
because all hops on the Internet route must stay online (have not a single power
outage, for example). Therefore with Session-Timers enabled (which are enabled
at default), you might see dropped calls. Consequently even with this change,
you might be better-off going for session-timers=refuse in your sip.conf.

ASTERISK-19968 #close

Change-Id: I1cd33453c77c56c8e1394cd60a6f17bb61c1d957
(cherry picked from commit 66c9dfb272322b21192f58383ae519ceb44e474c)
---
M CHANGES
M channels/chan_sip.c
2 files changed, 30 insertions(+), 32 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/CHANGES b/CHANGES
index e0cadc5..c7cc7bb 100644
--- a/CHANGES
+++ b/CHANGES
@@ -432,6 +432,16 @@
    The option determines how many seconds into a call before faxdetect
    is disabled for the call.  Setting the value to zero disables the timeout.
 
+chan_sip
+------------------
+ * Session-Timers (RFC 4028) work for TCP (and TLS) transports as well now.
+   Previously Asterisk dropped calls only with UDP transports. However with
+   longer international calls via TCP, the SIP channel might break, because
+   all hops on the Internet route must stay online (have not a single power
+   outage, for example). Therefore with Session-Timers enabled (which are
+   enabled at default), you might see additional dropped calls. Consequently
+   please, consider to go for session-timers=refuse in your sip.conf.
+
 res_pjsip
 ------------------
  * Added "fax_detect_timeout" to endpoint.
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 514f6bf..e05c22e 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -4206,19 +4206,6 @@
 		p->pendinginvite = seqno;
 	}
 
-	/* If the transport is something reliable (TCP or TLS) then don't really send this reliably */
-	/* I removed the code from retrans_pkt that does the same thing so it doesn't get loaded into the scheduler */
-	/*! \todo According to the RFC some packets need to be retransmitted even if its TCP, so this needs to get revisited */
-	if (!(p->socket.type & AST_TRANSPORT_UDP)) {
-		xmitres = __sip_xmit(p, data);	/* Send packet */
-		if (xmitres == XMIT_ERROR) {	/* Serious network trouble, no need to try again */
-			append_history(p, "XmitErr", "%s", fatal ? "(Critical)" : "(Non-critical)");
-			return AST_FAILURE;
-		} else {
-			return AST_SUCCESS;
-		}
-	}
-
 	pkt = ao2_alloc_options(sizeof(*pkt), sip_pkt_dtor, AO2_ALLOC_OPT_LOCK_NOLOCK);
 	if (!pkt) {
 		return AST_FAILURE;
@@ -4254,6 +4241,10 @@
 
 	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 */
+
+	if (!(p->socket.type & AST_TRANSPORT_UDP)) {
+		pkt->retrans_stop = 1;
+	}
 
 	/* Schedule retransmission */
 	ao2_t_ref(pkt, +1, "Schedule packet retransmission");
@@ -24671,6 +24662,7 @@
 	char *c_copy = ast_strdupa(c);
 	/* Skip the Cseq and its subsequent spaces */
 	const char *msg = ast_skip_blanks(ast_skip_nonblanks(c_copy));
+	int ack_res = FALSE;
 
 	if (!msg)
 		msg = "";
@@ -24685,28 +24677,24 @@
 		}
 	}
 
-	if (p->socket.type == AST_TRANSPORT_UDP) {
-		int ack_res = FALSE;
+	/* Acknowledge whatever it is destined for */
+	if ((resp >= 100) && (resp <= 199)) {
+		/* NON-INVITE messages do not ack a 1XX response. RFC 3261 section 17.1.2.2 */
+		if (sipmethod == SIP_INVITE) {
+			ack_res = __sip_semi_ack(p, seqno, 0, sipmethod);
+		}
+	} else {
+		ack_res = __sip_ack(p, seqno, 0, sipmethod);
+	}
 
-		/* Acknowledge whatever it is destined for */
-		if ((resp >= 100) && (resp <= 199)) {
-			/* NON-INVITE messages do not ack a 1XX response. RFC 3261 section 17.1.2.2 */
-			if (sipmethod == SIP_INVITE) {
-				ack_res = __sip_semi_ack(p, seqno, 0, sipmethod);
-			}
-		} else {
-			ack_res = __sip_ack(p, seqno, 0, sipmethod);
+	if (ack_res == FALSE) {
+		/* RFC 3261 13.2.2.4 and 17.1.1.2 - We must re-send ACKs to re-transmitted final responses */
+		if (sipmethod == SIP_INVITE && resp >= 200) {
+			transmit_request(p, SIP_ACK, seqno, XMIT_UNRELIABLE, resp < 300 ? TRUE: FALSE);
 		}
 
-		if (ack_res == FALSE) {
-			/* RFC 3261 13.2.2.4 and 17.1.1.2 - We must re-send ACKs to re-transmitted final responses */
-			if (sipmethod == SIP_INVITE && resp >= 200) {
-				transmit_request(p, SIP_ACK, seqno, XMIT_UNRELIABLE, resp < 300 ? TRUE: FALSE);
-			}
-
-			append_history(p, "Ignore", "Ignoring this retransmit\n");
-			return;
-		}
+		append_history(p, "Ignore", "Ignoring this retransmit\n");
+		return;
 	}
 
 	/* If this is a NOTIFY for a subscription clear the flag that indicates that we have a NOTIFY pending */

-- 
To view, visit https://gerrit.asterisk.org/3893
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I1cd33453c77c56c8e1394cd60a6f17bb61c1d957
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Steve Davies <steve at one47.co.uk>