[asterisk-dev] [Code Review] SIP 484 handling fix and new Incomplete app control frame
Matthew Jordan
mjordan at digium.com
Mon Oct 10 10:15:57 CDT 2011
Pavel -
I believe that enumerates your proposal very well. It sounds like you have a well thought out and defensible position for your interpretation of overlap dialing support in chan_sip. Since the current implementation went through the community review process and brought the previous implementation more in line with the RFCs, I wouldn't want to reopen that issue and subject it to more alterations.
However, please feel free to open an issue in the JIRA tracker for a new enhancement to chan_sip's handling of overlap dialing, with the description below. Please submit your patch to reviewboard, so that the community can weigh in on it.
Thanks for the feedback and for contributing!
Matthew Jordan
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org
----- Original Message -----
From: "Pavel Troller" <patrol at sinus.cz>
To: "Matthew Jordan" <mjordan at digium.com>
Cc: "Pavel Troller" <patrol at sinus.cz>, "Asterisk Developers Mailing List" <asterisk-dev at lists.digium.com>
Sent: Monday, October 10, 2011 9:47:20 AM
Subject: Re: [asterisk-dev] [Code Review] SIP 484 handling fix and new Incomplete app control frame
Hi Matthew!
It's ok, better the late reply than none :-).
I will try to explain better my point of view.
In SIP, there is an "official" (RFC specified) way of supporting overlap dialling: Sending a 484 Address Incomplete response, which causes the calling UA to collect more digits and send a new INVITE when it gets them. From this RFC-like point of view, all is clear: Either the device understands 484, and in this case allowoverlap=yes is correct, or it doesn't, and in this case, we cannot use it. HOWEVER, I think that it's still not clear that we MUST send 404, especially if the dialplan indicates, that the number maybe exists, but it is still incomplete.
The reason for not sending 404 in this case is, that the UA may be able to continue dialling by an "unofficial" method (i.e. not especially recommended, but also not prohibited in RFC). This method uses existing standard DTMF delivery methods in SIP, i.e. either RFC2833 or SIP INFO. Most devices are able to pass DTMF using either of these methods since the Early Media (i.e. 183 Session Progress with SDP) is established. So, it's under full dialplan control to send Progress() to the SIP device soon enough (as one of the first dialplan commands) that subsequent call to Incomplete() application will come in the Early Media channel state. In this case, the Asterisk core will happily continue receiving digits using either RFC2833 or SIP info, until it's clear, whether the dialled number really exists or not. If it will prove to not exist, now it's the time to send 404 back.
So, to summarize, I propose the following possibilities for the allowoverlap SIP option:
1) allowoverlap=yes: The UA understands the 484 response and is able to fully support RFC recommended overlap dialling. Calling Incomplete() from the dialplan causes the 484 response to be sent.
2) allowoverlap=dtmf: The UA doesn't understand the 484 response, but it's capable to continue dialling with a DTMF delivery mechanism specified in the "dtmfmode" option. In this case, the dialplan must ensure that the Early Media state is properly established before the Incomplete() application is called. Calling Incomplete() from the dialplan is ignored by chan_sip in this case (as it was before this change), or (just a quick idea) the channel may verify, whether the Early Media is established and if not, send back 404, because it's clear now that we cannot get more dialling from the UA.
3) allowoverlap=no: The UA doesn't understand the 484 response and is not capable to send DTMF in the Early Media state, OR the system dial plan is not adopted for this kind of signalling. In this case, Incomplete() will send 404 immediately.
I think that implementing this cannot make any harm to the Asterisk, and it will substantially improve the flexibility of the SIP channel regarding the overlap dialling possibilities.
I hope that it's more clear to you now.
With regards,
Pavel Troller
> Hi Pavel -
>
> Sorry for the late reply to your e-mail, I was out of the office for a bit and am just now catching up on things.
>
> The idea behind our implementation for this patch was to have Asterisk allow devices to send additional digits if it was capable of doing so, or to inform the devices that the extension dialed could not be reached if the devices did not support overlap dialing. The "allowoverlap=no" on a SIP device indicates that the device does not support overlap dialing; hence, sending a SIP 484, which only applies for devices that support overlap dialing, doesn't seem to make a lot of sense. In this scenario, sending the SIP 404 is inline with how other parts of chan_sip behave in similar situations (such as when a SIP device without overlap support dials an extension that partially matches in the dialplan).
>
> There are other channel technologies that behave in a similar fashion. For example, chan_misdn and sig_pri will only wait for additional digits if the device supports overlapped dialing - otherwise, a hangup is sent to the device.
>
> I'm not sure how complicating the allowoverlap setting eases this scenario - either a device can send additional DTMF tones, in which case the Incomplete application has a purpose and a place - or it can't, in which case the device cannot proceed any further in the dialplan and hanging up immediately saves the caller 5 seconds of waiting before being hung up on. Can you clarify why a device that does not support overlap dialing should be left connected if the device it attempts to connect to returns an Address Incomplete? Note that in the case of redirecting to an automated attendant, something similar could be achieved by checking the return code of the Dial application rather than relying on the 't' extension.
>
> Thanks,
>
> Matthew Jordan
> Digium, Inc. | Software Developer
> 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
> Check us out at: http://digium.com & http://asterisk.org
>
> ----- Original Message -----
> From: "Pavel Troller" <patrol at sinus.cz>
> To: "Asterisk Developers Mailing List" <asterisk-dev at lists.digium.com>
> Sent: Friday, September 23, 2011 10:53:08 PM
> Subject: Re: [asterisk-dev] [Code Review] SIP 484 handling fix and new Incomplete app control frame
>
> Hi!
> This change broke chan_sip for me. I've found it today, after updating my 1.8
> SVN branch.
> The problem is, that my dialplans are commonly using the "incomplete" status,
> waiting for more digits.
> The extension logic sends Progress() as soon as possible, which transforms to
> 183 Session Progress SIP message with SDP, thus establishing early media. Since
> then, most SIP devices are able to dial using either RFC2833 or SIP INFO
> method, even the channel was not yet answered. This was successfully used to
> complete the dialled number in something like "pseudo-overlap mode" (from the
> SIP point of view, from the dialplan point of view it is a real overlap mode).
> Since this latest change, SIP definitely behaves better (according to the
> spec), when allowoverlap=yes is set. However, when it is NOT set, it currently
> immediately sends back 404 Not Found, which is fatal for me. There are no other
> channels, which drop the connection upon receipt of the INCOMPLETE frame; I
> don't see any explicit reason why SIP should do it, especially when there is a
> possibility of receiving the next digits as described above.
> Also, think about the scenario, when there is a "t" extension, connecting the
> caller to somebody like an operator or similar service. Even if the caller will
> not be able to continue dialling (if he uses one of the very few devices, which
> will not pass RFC2833 or INFO before real answer), he would be connected to
> this service due to timeout. With your change, it's also impossible.
> I've patched the code, that it keeps your change with sending 484 back, but
> discards the 404 case unconditionally. Maybe, the better way would be to have a
> "three-way" allowoverlap option, like:
> allowoverlap=yes ; use standard 484 Number Incomplete handling
> allowoverlap=dtmf ; ignore the Incomplete status and continue
> allowoverlap=no ; send back 404 Not found response and terminate the call.
> If you are willing to accept this change, I will prepare the patch
> accordingly and create a JIRA case for it.
> With regards,
> Pavel Troller
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviewboard.asterisk.org/r/1416/#review4275
> > -----------------------------------------------------------
> >
> > Ship it!
> >
> >
> > I don't see anything else to really point out.
> >
> > - rmudgett
> >
> >
> > On Sept. 8, 2011, 2:12 p.m., mjordan wrote:
> > >
> > > -----------------------------------------------------------
> > > This is an automatically generated e-mail. To reply, visit:
> > > https://reviewboard.asterisk.org/r/1416/
> > > -----------------------------------------------------------
> > >
> > > (Updated Sept. 8, 2011, 2:12 p.m.)
> > >
> > >
> > > Review request for Asterisk Developers, David Vossel and rmudgett.
> > >
> > >
> > > Summary
> > > -------
> > >
> > > ASTERISK-17288 was filed against chan_sip. When a response code of "484 Address Incomplete" was received by the dialplan, the channel was being hung up with a hangup cause of 111, as opposed to the RFC recommended 28. Upon resolving this, the previously implemented logic for Incomplete extension dialing was triggered (implemented under ASTERISK-11768). For SIP phones, this had a somewhat unexpected result: upon receiving the 484 response back from the dialed device, the dialplan would send no information back to the device that dialed the numbers, and, after a period of time, would timeout (transferring to either the 't' or 'i' extension, depending on the dialplan).
> > >
> > > While this was the intended effect of the pbx Incomplete logic, it was not desired by the issue reporter. What's more, for SIP phones, since no information was being forwarded back to the dialing device that the number that they dialed was incomplete, the device did not know to dial more numbers.
> > >
> > > After discussion on asterisk-dev with Kevin and the issue reporter, the following decisions were made:
> > > 1. app_dial by default should not attempt to place a call into Incomplete - functionality should be triggered by explicit dialplan usage
> > > 2. Incomplete application should notify channels that they are in Incomplete status using a control frame. That allows channel tech to automatically hangup if no more digits are possible.
> > >
> > > A new control frame AST_CONTROL_INCOMPLETE was added and is sent to the channel in the Incomplete application. Otherwise, the Incomplete application logic was left unchanged.
> > >
> > > Handling the control frame differs on the channel technology. With respect to SIP, an Incomplete implies immediately sending the 484 Address Incomplete response to the dialing device and performing a soft hangup, as a new INVITE sequence is required. If the device wants to retry, then the device will resend the URI with whatever appended digits it determines is needed to try for the next extension.
> > >
> > > For other channel technologies, it depends on their implementation and their support for overlapped dialing. DAHDI (and sig_pri) and mISDN have support for overlapped dialing - for those cases, receiving an AST_CONTROL_INCOMPLETE means either doing nothing if overlapped dialing is supported (and if the channel is in a state that it can receive additional digits), or hanging up if overlapped dialing is not supported. In the case of a hangup, the approach was taken to emulate AST_CONTROL_CONGESTION.
> > >
> > > For channels that do not support overlapped dialing, they treat a control frame of AST_CONTROL_INCOMPLETE as if it were AST_CONTROL_CONGESTION.
> > >
> > >
> > > This addresses bug ASTERISK-17288.
> > > https://issues.asterisk.org/jira/browse/ASTERISK-17288
> > >
> > >
> > > Diffs
> > > -----
> > >
> > > /branches/1.8/addons/chan_ooh323.c 334951
> > > /branches/1.8/apps/app_dial.c 334951
> > > /branches/1.8/channels/chan_alsa.c 334951
> > > /branches/1.8/channels/chan_console.c 334951
> > > /branches/1.8/channels/chan_dahdi.c 334951
> > > /branches/1.8/channels/chan_h323.c 334951
> > > /branches/1.8/channels/chan_mgcp.c 334951
> > > /branches/1.8/channels/chan_misdn.c 334951
> > > /branches/1.8/channels/chan_oss.c 334951
> > > /branches/1.8/channels/chan_sip.c 334951
> > > /branches/1.8/channels/chan_skinny.c 334951
> > > /branches/1.8/channels/chan_unistim.c 334951
> > > /branches/1.8/channels/chan_usbradio.c 334951
> > > /branches/1.8/channels/sig_pri.c 334951
> > > /branches/1.8/channels/sig_ss7.c 334951
> > > /branches/1.8/funcs/func_frame_trace.c 334951
> > > /branches/1.8/include/asterisk/frame.h 334951
> > > /branches/1.8/main/channel.c 334951
> > > /branches/1.8/main/dial.c 334951
> > > /branches/1.8/main/features.c 334951
> > > /branches/1.8/main/pbx.c 334951
> > >
> > > Diff: https://reviewboard.asterisk.org/r/1416/diff
> > >
> > >
> > > Testing
> > > -------
> > >
> > > Two new Asterisk test suites were written to test the behavior for SIP (review on https://reviewboard.asterisk.org/r/1417/ ). These two tests check that Asterisk handles the SIP 484 Address Incomplete both when the response is returned and the Incomplete application is not used, and when the Incomplete application is used.
> > >
> > > Additional testing will be needed for other channel types. DAHDI will be tested in conjunction with this review; other channels whose support are extended will require testing by the Asterisk community.
> > >
> > >
> > > Thanks,
> > >
> > > mjordan
> > >
> > >
> >
>
> > --
> > _____________________________________________________________________
> > -- Bandwidth and Colocation Provided by http://www.api-digital.com --
> >
> > asterisk-dev mailing list
> > To UNSUBSCRIBE or update options visit:
> > http://lists.digium.com/mailman/listinfo/asterisk-dev
>
> --
> _____________________________________________________________________
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
> http://lists.digium.com/mailman/listinfo/asterisk-dev
More information about the asterisk-dev
mailing list