Summary:	ASTERISK-09485: Additional Call Setup Timeout
Reporter:	Curt Moore (jcmoore)	Labels:
Date Opened:	2007-05-21 22:12:48	Date Closed:	2007-12-16 02:12:26.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) peer_t1_timerb_trunk_v2.patch.txt ( 1) peer_t1_timerb_trunk_v3.patch.txt ( 2) peer_t1_timerb_trunk.patch.txt ( 3) per_peer_t1_trunk.patch.txt ( 4) per_peer_t1.patch.txt
Description:	When dialing a SIP destination, sometimes it is desirable to have a timeout other than the timeout specified to app_dial.  This is useful in cases where you wish to try provider A and if there is no response from them in so many seconds, fail the call and try provider B instead, etc.  Previously, this could be accomplished by use of SIP OPTIONs requests by setting qualify to some specified timeout value.  If the peer was deemed unaviable or did not respond within the time specified qualify timeout, the call would fail as desired.  At times, some providers do not wish to, or cannot, receive/respond to OPTION messages for one reason or another. ****** ADDITIONAL INFORMATION ****** This patch allows for a timeout value to be set as normal using the qualify parameter in sip.conf but has the added parameter of qualifynoping which will make chan_sip not actively send OPTIONs packets to the peer.  Instead, if a SIP call is initiated to the peer and no response is received within the timeout specified in the qualify parameter, the call will fail/timeout and can be handled as desired in the dialplan. If the new qualifynoping parameter is not set, the behavior of qualify is the same as it always was. This new parameter is in addition to the SIP dialog timeout parameter.
Comments:	By: Olle Johansson (oej) 2007-05-22 08:46:56 Let's discuss this at astridevcon and see what we can do. By: Curt Moore (jcmoore) 2007-05-22 15:08:23 After talking with oej, probably the best way to address this would be to implement t1 timers on a per peer basis.  This is pretty much what my current patch does, but just not in that explicit of a manner. As a future step, additionally, we could implement a variable which could be set for the when a dial fails to a certain peer/endpoint/user and then could be checked before successive dials and even reset after a certain timeout. By: Curt Moore (jcmoore) 2007-05-23 13:09:43 Here is the patch which enables per peer T1 and also allows for a global T1 setting.  Let's try to test this while everyone is at AstriDevCon. By: Olle Johansson (oej) 2007-05-29 02:58:55 We need a patch for trunk, thanks. This does not apply cleanly to trunk. By: Curt Moore (jcmoore) 2007-05-29 13:07:47 Uploaded patch against trunk. By: Olle Johansson (oej) 2007-05-29 14:11:02 Ok, now write it as a CSH script. By: Olle Johansson (oej) 2007-05-29 14:11:19 Sorry, just joking. Apologies. By: Curt Moore (jcmoore) 2007-06-04 15:18:09 In further testing, it appears that timer B is actually the timer I was after.  This was previously SIP_TRANS_TIMEOUT and was hardcoded to 64*T1.  The revised patch now allows for setting timerb which will congest the call if a provisional response is not received in the amount of time specified by timerb. By: fossil (fossil) 2007-06-11 15:54:50 I like the last patch, but I have some comments: 1) Around line 2249 we have "if (ms < 0) { if (p->timer_t1 == 0)..." the line   p->timer_b = global_timer_b;  /* Set timer B if not set (RFC 3261) */ is indented so that it appears "if (p->timer_t1 == 0)" guards it, but it actually does not. Which way did you mean it? The comment states "Set timer B if not set", but there is no test for whether it is set or not. EDIT: It looks like it is not necessary to update p->timer_b inside sip_scheddestroy() in any case. 2) Around line 9955, we have  /* Set timer B to control transaction timeouts */  if (peer->timer_b)    p->timer_b = peer->timer_b;  else    p->timer_b = 64 * peer->timer_t1; Should we really be setting p->timer_b to 64*peer->timer_t1, or actually to 64*p->timer_t1 (p vs. peer), which was just set above that code block? Not to nitpick, but: 3) The conf parameters are "timerb" and "t1". Let's call it "timert1" to prevent confusion; also "t1" is a rather short non-descriptive name for a conf param. (You call the internal var "timer_t1" already). 4) You use "Timer B" and "T1 Timer" in cli dumps. Perhaps a more uniform way would be "Timer T1"? 5) Around line 3100, the code block  /* Set timer B to control transaction timeouts */  if (peer->timer_b)    ... uses spaces for indentation, instead of tabs. By: Curt Moore (jcmoore) 2007-06-11 17:47:50 Thanks for all of the feedback!  I've attached an updated version of the patch which corrects all of the things you pointed out. Around line 2249 I kept the line where we are explicitly setting timer_b if timer_t1 == 0.  It just seems scary to rely on timer b to have been set correctly if timer T1 is 0.  If we're going to set T1 to the global default, we'd just as well set timer_b to the global default as well. By: fossil (fossil) 2007-06-11 19:10:05 Great, thanks! Though I agree that since we are updating timer_t1 in sip_scheddestroy(), we might as well update timer_b, it just gives me the creeps that timer_t1 can even be 0 at that point (which it probably cannot, but correct me if I am wrong). IMHO, sip_scheddestroy() should not be updating timer_t1 either. It just does not have to. If I read it correctly, all it needs is to calculate the default 'ms' timeout. Otherwise, I have no problems with setting timer_b as well. By: Curt Moore (jcmoore) 2007-08-21 09:28:42 Attached updated patch against latest trunk, r80129. By: Olle Johansson (oej) 2007-12-16 01:51:31.000-0600 Sorry, lost track of this bug since it wasn't in the chan_sip category. Moving it back home. What's the status? By: Digium Subversion (svnbot) 2007-12-16 02:12:26.000-0600 Repository: asterisk Revision: 93159 U   trunk/channels/chan_sip.c U   trunk/configs/sip.conf.sample ------------------------------------------------------------------------ r93159 | oej | 2007-12-16 02:12:25 -0600 (Sun, 16 Dec 2007) | 8 lines Make more timers settable in SIP so that we can force timeout earlier on non-responsive SIP servers. Thanks, jcmoore, for the patch! Reported by: jcmoore Patches:      peer_t1_timerb_trunk_v3.patch.txt uploaded by jcmoore (license 9) (closes issue ASTERISK-9485) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=93159