[asterisk-dev] [Code Review] SIP: Re-send non-100 provisional responses every 60 seconds until a final response is sent

[Mark Michelson]

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


For the most part, this seems pretty good. One thing I need to caution you about when creating the trunk version of this is that Asterisk treats a 182 response in a meaningful way. A 182 response is the only way that made sense to indicate a redirecting update over SIP. I would recommend not retransmitting a 182 if it was the last provisional response sent by Asterisk and instead just send a 183 with no SDP.


/branches/1.4/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/315/#comment2346>

    I understand the cast is here because data is const, but it seems like it would make a lot more sense to cast to sip_pvt* instead of void*.



/branches/1.4/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/315/#comment2348>

    This isn't necessary since p was allocated using calloc(). The pointer will be NULL anyway.



/branches/1.4/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/315/#comment2347>

    This works all right since only static strings are ever passed to this function, but the possibility exists for p->last_provisional to point to data in automatic storage. For now, at least, I think a comment is in order so that people don't try to do anything dumb here.


- Mark


On 2009-07-20 17:31:40, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/315/
> -----------------------------------------------------------
> 
> (Updated 2009-07-20 17:31:40)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The issue here is that Asterisk does not send a non-100 response at every minute for calls that are in the provisional response state. This causes most UACs and/or proxies to terminate the call after 3 minutes. There are many legitimate reasons why a call could remain in an unanswered state for more than 3 minutes such as early-media (183), call queuing (182), call forwarding (181) and ringing (180). 
> Following is quote from section 13.3.1.1 of RFC 3261 which explains what a UAS should do under such a circumstance:
> 
>    If the UAS desires an extended period of time to answer the INVITE,
>    it will need to ask for an "extension" in order to prevent proxies
>    from canceling the transaction. A proxy has the option of canceling
>    a transaction when there is a gap of 3 minutes between responses in a
>    transaction. To prevent cancellation, the UAS MUST send a non-100
>    provisional response at every minute, to handle the possibility of
>    lost provisional responses.
> 
> This patch is my first attempt at coming up with a fix for this.  Basically it will re-send the last non-100 1xx message that has been sent, defaulting to 183 Session Progress w/ no SDP if no non-100 provisional messages have been sent, until a final response is sent.  It appears that most phones I've tested this situation against with the dialplan:
> 
> exten => 123,1,Wait(360)
> 
> will generate ringing locally upon receipt of a 183 Session Progress w/o SDP--which should not indicate that ringing is necessarily taking place.  But, most situations we will be resending a 183 w/ SDP or 180 Ringing response.  It would be nice to add the ability for app_queue to return a "call queued" state so we could properly send a 182 Queued, but that is a topic for another day.
> 
> Anyway, I'd like some feedback as to whether this looks like it is on the right track or not!  It would probably be wise to make this option configurable, which I haven't done yet.  Thanks.
> 
> 
> This addresses bug 11157.
>     https://issues.asterisk.org/view.php?id=11157
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/channels/chan_sip.c 206937 
> 
> Diff: https://reviewboard.asterisk.org/r/315/diff
> 
> 
> Testing
> -------
> 
> Tested against:
> [default]
> exten => 123,1,Queue(test|r)
> 
> exten => 234,1,Progress
> exten => 234,n,Wait(360)
> 
> exten => 345,1,Ringing
> exten => 345,n,Wait(360)
> 
> exten => 456,1,Wait(360)
> 
> exten => 567,1,Dial(Local/456 at default,,r)
> 
> 
> Thanks,
> 
> Terry
> 
>