[asterisk-dev] [Code Review]: Asterisk Support of SIP Connect 1.1

Thu Nov 10 12:07:54 CST 2011

> On None, Olle E Johansson wrote:
> > First, submit the patch through our bug tracker to make sure we have a proper license agreement. Also, please fill in the form of the first page here in review board.
> > 
> > Secondly, I can't make sense of having a "sip connect" setting. I would like to propose that you separate the path work, which we already have a patch for, from the gin work. Some of your changes of id's should be handled by the dial plan (adding a + can be done there for e.164 phone numbers). Some of them may not be doable and we need to discuss it.
> > 
> > So instead of a big patch, please break it up and let us review smaller patches. 
> > 
> > I can not accept this as part of asterisk as it stands, but really want to see the functionality in asterisk. Let's continue working on this. Thank you very much for contributing this code!
> > 
> > /O
> 
> Andrew Olmsted wrote:
>     Hi Olle,
>     
>     I worked on this patch with Neeharika and appreciate your comments.  We are currently testing e.164 in the dial plan and if this is successful I will take it out of the patch.
>     
>     For the RFC 6140 changes for GIN support do you have a recommendation on being able to turn this on? We are considering a RCF6140 flag on a per-peer basis, but you said you didn't see the point to a sip connect setting.
>     
>     Taking e.164 out of the patch will already make it much smaller, and doing the path work with the previously submitted patch will make it smaller still. Hopefully reducing the patch this much will make it easier to review. Should we resubmit the patch here after implementing your recommendations?
>     
>     Thanks,
>     Andrew
> 
> Olle E Johansson wrote:
>     Andrew - can you please verify that the Path: header support is written by you and not the patch we already have in the issue tracker? licensing is important for us. Thanks.
>     
>     I would propose we do take the code in bit by bit, starting with the Path code.
>     
>     /O
> 
> Olle E Johansson wrote:
>     In regards of a SIP connect setting - no. We need to refer to standards. And we need to be able to configure a peer's E.164 phone numbers to do this right. We have no such configuration today, as everything is in the dial plan. What happens if we place a call from the dial plan to a number not associated with the GIN registration? Should it be refused by the SIP channel?
>     
>     Many architecture things to discuss here.
> 
> Andrew Olmsted wrote:
>     Olle; the path code in this patch was written by me. I was working against 1.8.0 for a reference implementation at the time and was not aware that there was already a patch in trunk. I will take a look at the patch in trunk to see what duplication exists; is this the patch you were referring to: https://reviewboard.asterisk.org/r/991/ (from schmidts later in this thread)?
> 
> Olle E Johansson wrote:
>     Ok. I just wanted to be very clear on the license. THanks for confirming. Yes, the patch in 991 is the one I referred to. It's waiting for an update or a replacement. If you can extract a similar patch for just Path and add it as a separate issue and review that's a good first step. Nothing is in trunk yet.
> 
> Andrew Olmsted wrote:
>     Hmm, I am a bit confused looking at patch 991 because the copy_route and build_path are exactly what I have implemented down to the comments. These functions definitely exist as present in this patch in my source tree and I cannot fathom how they came to be there if not be me. I did the majority of the coding in January of this year so it isn't entirely fresh in my mind.
>     
>     The only thing I can think of is I was searching for a function to do this and came across patch 991. I really don't remember this but it is the only thing that makes sense. Sorry for the confusion, but I cannot claim ownership of this section of code.

Ok. Great to have that cleared up. I also suffer from memory loss. Let's work on 991 together and get that stuff committed, since nothing has happened since my review.

You can reach me on oej (at) edvina.net as email if you want to discuss. Over e-mail we can exchange chat/skype handles.

/O

- Olle E

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

On Oct. 18, 2011, 3:11 p.m., Neeharika Allanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1515/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2011, 3:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This Chan-SIP patch brings Asterisk into compliance with the SIPconnect1.1.  SIPconnect1.1 is a newly released SIP Forum specification that describes a common set of signaling and media interworking procedures for the SIP Trunk interface between a SIP-based IP-PBX and a SIP-enabled Service Provider network. This patch, coupled with specific Asterisk configuration settings, will enable Asterisk to comply with the normative SIP-PBX requirements specified in SIPconnect1.1.
> 
> The patch diff listings being submitted are against Asterisk version 1.8.11.The patch itself has been tested against the 1.8.0 version of Asterisk for the following SIPconnect1.1 functions/capabilities:
> 
> Security
> -TLS
> -SIP Digest
> 
> Registration (RFC 6140)
> -Basic GIN registration
> -did not test the GIN interactions with the GRUU and reg-event package extensions)
> 
> Calling features
> -Basic DID/DOD calls
> -Calling name/number delivery with and without privacy
> -Early media
> -Call Forwarding
> -Call Transfer (attended and blind)
> -Emergency calls
> -DTMF relay
> 
> 
> This addresses bug ASTERISK-18705.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18705
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 333472 
>   /trunk/channels/sip/include/sip.h 333472 
>   /trunk/configs/sip.conf.sample 333472 
>   /trunk/include/asterisk/strings.h 333472 
> 
> Diff: https://reviewboard.asterisk.org/r/1515/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Neeharika
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111110/7e33c6bd/attachment.htm>