[asterisk-dev] [Code Review] 4603: pjsip: Add external PJSIP resolver implementation using core DNS API

Mark Michelson reviewboard at asterisk.org
Thu Apr 9 16:52:00 CDT 2015



> On April 9, 2015, 8:32 p.m., Mark Michelson wrote:
> > /trunk/res/res_pjsip/pjsip_resolver.c, lines 258-260
> > <https://reviewboard.asterisk.org/r/4603/diff/1/?file=73804#file73804line258>
> >
> >     NAPTR has a few restrictions that SRV does not have. With SRV, you sort the records and you can continue to fail over to the next one in the list, no matter the priority or weight of each individual record.
> >     
> >     With NAPTR, RFC 3403 section 4.1 places the following restriction:
> >     
> >     "The important difference between Order and Preference is that once a match is found the client MUST NOT consider records with a different Order but they MAY process records with the same Order but different Preferences"
> >     
> >     This means you need to examine the Order field of the records more closely.
> >     
> >     Defining "match" is a bit tricky but I'm going to use the NAPTR example in RFC 3264 Section 4.1 as a reference. In that example, it shows three NAPTR records for the three different SIP services provided, each with a different order. It then states:
> >     
> >     "This indicates that the server supports TLS over TCP, TCP, and UDP, in that order of preference.  Since the client supports TCP and UDP, TCP will be used, targeted to a host determined by an SRV lookup of _sip._tcp.example.com."
> >     
> >     By my reading, a "match" means that you recognize the service in the NAPTR record as being SIP-related AND that you support the transport protocol that the record pertains to.
> >     
> >     In the case of this loop, then, if you find a NAPTR record with a SIP+D2X service that you recognize and whose transport you support, you should not do anything with NAPTR records that have a higher order value than this first match. You can process records with the same order but higher preference number, though. That's kosher.

Nitpicking my own comment, but I erroneously said RFC 3264 here but meant RFC 3263.


> On April 9, 2015, 8:32 p.m., Mark Michelson wrote:
> > /trunk/main/dns_query_set.c, lines 75-76
> > <https://reviewboard.asterisk.org/r/4603/diff/1/?file=73799#file73799line75>
> >
> >     I don't think it would hurt to increase this to 4 or 5, tbh.

Also while I'm here, please make this a constant.


- Mark


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


On April 8, 2015, 7:37 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4603/
> -----------------------------------------------------------
> 
> (Updated April 8, 2015, 7:37 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24947
>     https://issues.asterisk.org/jira/browse/ASTERISK-24947
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change adds the following:
> 
> 1. A query set implementation. This is an API that allows queries to be executed in parallel and once all have completed a callback is invoked.
> 2. Unit tests for the query set implementation.
> 3. An external PJSIP resolver which uses the DNS core API to do NAPTR, SRV, AAAA, and A lookups.
> 
> For the resolver it will do NAPTR, SRV, and AAAA/A lookups in parallel. If NAPTR or SRV are available it will then do more queries. And so on. Preference is NAPTR > SRV > AAAA/A, with IPv6 preferred over IPv4. For transport it will prefer TLS > TCP > UDP if no explicit transport has been provided. Configured transports on the system are taken into account to eliminate resolved addresses which have no hope of completing.
> 
> 
> Diffs
> -----
> 
>   /trunk/tests/test_dns_query_set.c PRE-CREATION 
>   /trunk/res/res_pjsip_session.c 434446 
>   /trunk/res/res_pjsip/pjsip_resolver.c PRE-CREATION 
>   /trunk/res/res_pjsip/include/res_pjsip_private.h 434446 
>   /trunk/res/res_pjsip.c 434446 
>   /trunk/main/dns_srv.c 434446 
>   /trunk/main/dns_recurring.c 434446 
>   /trunk/main/dns_query_set.c 434446 
>   /trunk/main/dns_naptr.c 434446 
>   /trunk/main/dns_core.c 434446 
>   /trunk/include/asterisk/dns_query_set.h 434446 
>   /trunk/include/asterisk/dns_internal.h 434446 
>   /trunk/include/asterisk/dns_core.h 434446 
>   /trunk/include/asterisk/autoconfig.h.in 434446 
>   /trunk/configure.ac 434446 
>   /configure UNKNOWN 
> 
> Diff: https://reviewboard.asterisk.org/r/4603/diff/
> 
> 
> Testing
> -------
> 
> Ran unit tests, they pass. Ran testsuite tests, they pass. Did spot checking using my own domains. They resolve as expected.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150409/4c207cb0/attachment-0001.html>


More information about the asterisk-dev mailing list