[asterisk-dev] [Code Review] chan_sip: RFC compliant retransmission timeout

[David Vossel]

> On 2010-06-29 03:53:17, Nick Lewis wrote:
> > /trunk/channels/sip/include/sip.h, line 1111
> > <https://reviewboard.asterisk.org/r/749/diff/1/?file=11133#file11133line1111>
> >
> >     A timeval here may be safer.
> >     
> >     I have experienced problems in the past when the system clock time changed backwards for example when the rtc of a spare pbx has been drifting significantly in storage and ntp steps it soon after it is brought into service. (I hazily remember that the mwi timer used to freeze the whole of chan_sip.c under these circumstances). Perhaps manually changing the system time would test whether the retrans_stop_time implementation is resilient. 
> >     
> >     Come to think of it it probably is given the diff <= 0 check

Yes, that would be incredibly rare.  If that did occur, the failsafe if (diff <= 0)  check would account for it. 


> On 2010-06-29 03:53:17, Nick Lewis wrote:
> > /trunk/channels/sip/include/sip.h, line 1110
> > <https://reviewboard.asterisk.org/r/749/diff/1/?file=11133#file11133line1110>
> >
> >     pkt_sent might be more descriptive

agreed, i'll change this.


> On 2010-06-29 03:53:17, Nick Lewis wrote:
> > /trunk/channels/chan_sip.c, line 3300
> > <https://reviewboard.asterisk.org/r/749/diff/1/?file=11132#file11132line3300>
> >
> >     How does this effect registration timeouts? Should a multiple of T1 be used for global_reg_timeout, and instead of global_reg_attempts

>From OEJ: "You are confusing transaction timeouts with the number of transactions, Nick.
reg_attempts is how many times we try to REGISTER. retransmissions is how many times and with which frequency we try each SIP REGISTER transaction.

/O"


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/749/#review2300
-----------------------------------------------------------


On 2010-06-28 11:59:09, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/749/
> -----------------------------------------------------------
> 
> (Updated 2010-06-28 11:59:09)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 272652 
>   /trunk/channels/sip/include/sip.h 272649 
> 
> Diff: https://reviewboard.asterisk.org/r/749/diff
> 
> 
> Testing
> -------
> 
> I tested this with a sipp scenario that sends an INVITE but does not respond to Asterisk's 200 OK response.  I verified Asterisk continues to send retransmits until the packet times out at the correct timeout time.  I also did a sanity check to verify packets continue to be acknowledged correctly by placing some test calls.
> 
> 
> Thanks,
> 
> David
> 
>