[Home]

Summary:ASTERISK-15466: [patch] Setting "timerb" on chan_sip.conf doesn't work at all, in [general] or peer
Reporter:Nahuel Greco (nahuelgreco)Labels:
Date Opened:2010-01-18 18:45:19.000-0600Date Closed:2010-02-15 18:19:41.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20100204__issue16643.diff.txt
Description:Asterisk 1.6.0.20, 1.6.1.12, 1.6.2.0, trunk rev 241145, all come with an example of the "timerb" usage as a global parameter in sip.conf.sample, but setting it doesn't work at all, so his documented usage is misleading.

If you change it in the [general] section, then chan_sip.c will do nothing with that declaration. His global_timer_b variable is only assigned to "timert1"*64, and there is no presence of "timerb" in the reload_config() function for the [general] section.

If you instead set this timer for an specific peer, then is treated in the build_peer() function, but the treatment seems to be incomplete. There are two possible outcomings: the timer is not valid and the global one is used, or the timer has a lower setting that the recommended and a warning is printed... but there is no other usage of the retrieved "timerb" peer config value, so is lost and never assigned to any variable.

So, doesn't matter how you use the "timerb" parameter in sip.conf, it has no effects at all.
Comments:By: Tilghman Lesher (tilghman) 2010-02-02 15:15:05.000-0600

Well, I can parse the options, but that only goes so far.  As you noted, the option isn't used.
By: Tilghman Lesher (tilghman) 2010-02-02 15:15:36.000-0600

oej, could you look at this please?
By: Olle Johansson (oej) 2010-02-03 01:22:03.000-0600

I will. I haven't been involved in this code so it may be time to try to understand the thoughts behind it and get it right.

Thanks for reporting this!
By: Olle Johansson (oej) 2010-02-03 14:46:23.000-0600

The config part is bad, it has a circular dependency of the order in the config file...

Timer_t1 checks the value of timer_b and timer_b checks the setting of timer_t1... Bad. The checks about correct values should move until after we have parsed the complete peer.

nahuelgreco: I think you misunderstand the scanf statement, we actually set the timer_b value in the peer and use it in the dialog timeouts. Can you give a more specific example on how it is not used or hurts your communication?

tilghman: Check create_addr_from_peer where we copy timer_B to the dialog structure from the peer. Then in sip_call() we use dialog->timer_b to schedule the timeout. So I do think the code is around. Haven't tested it though.
By: Nahuel Greco (nahuelgreco) 2010-02-03 16:58:49.000-0600

oej: The first part of my bug report, concerning the "global_timer_b" variable and the "timerb" entry in the [general] section is valid. But the second one about "timerb" in the peers section is not, you are right, I misread that scanf.
By: Tilghman Lesher (tilghman) 2010-02-04 13:55:20.000-0600

Okay, patch updated.
By: Digium Subversion (svnbot) 2010-02-15 18:19:39.000-0600

Repository: asterisk
Revision: 246724

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r246724 | tilghman | 2010-02-15 18:19:38 -0600 (Mon, 15 Feb 2010) | 8 lines

Allow Timer B to be set on the peer, and ensure SIP rules are followed (or warn) in comparison to Timer T1.

(closes issue ASTERISK-15466)
Reported by: nahuelgreco
Patches:
      20100204__issue16643.diff.txt uploaded by tilghman (license 14)
Tested by: oej

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=246724