[asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

[Olle E Johansson]

> On March 13, 2014, 4:31 p.m., Olle E Johansson wrote:
> > We really should NOT add this code. Instead, we should clean up the code so all of Asterisk use the same resolver and relies on the system configuration. Having a separate DNS resolver, maybe even using a separate DNS server in the PJSIP channel is not a good solution and should be avoided at all cost. 
> > 
> > The good thing with this review is that it opened my eyes of some stuff in PJSIP that should not be there in the way we use it.
> 
> Matt Jordan wrote:
>     DNS support in Asterisk has had problems for a long time and is something that I think we would all like to see improved upon. The state of such support as it exists in the core today is limited: modules and the core can either use dnsmgr – which only gives you a single IP address result but does make an attempt at querying and managing the DNS records – or they can use ast_gethostbyname, which is a wrapper around a reentrant version of gethostbyname. In the case of the former, users get a small (albeit limited) benefit of having the records be managed internally; in the case of the latter users only get the standard gethostbyname behavior. Neither option provides asynchronous DNS querying, nor does either option providethe ability to handle multiple SRV records.
>     
>     The situation is the way it is today not because parsing and returning multiple DNS records is particularly hard, or because properly weighting the results is extremely difficult. Even making the querying of records asynchronous is doable. While not trivial changes, they are well understood problems and the API in dns.h/srv.h/dnsmgr.h could be modified accordingly. The hard part is making use of the DNS information.
>     
>     The legacy IP based channel drivers in Asterisk are not structured in such a fashion that the location of the thing that Asterisk communicates with is separate from the IP address. This was an unfortunate design decision; separating out the concept of location from the IP address is now extremely non-trivial. I can't stress this enough: it is an incredibly large task to go do this work – particularly when we consider the size and complexity of those channel drivers. Unfortunately, chan_sip and chan_iax2 are both places where this would be beneficial, but also cases where making use of multiple SRV records or asynchronous DNS would be extremely challenging.
>     
>     That gets to the crux of the problem: even if we had the world's best DNS resolver in the Asterisk core, the vast majority of places in Asterisk that you would like to make use of it would not easily be able to do so.
>     
>     There is, of course, benefit to having better DNS support in the core of Asterisk outside of those channel drivers. New things would be able to make use of it easier, and improvements in the DNS support would be shared by all things that use it. I think it would be great to see patches that improve the DNS support in the core. This should include asynchronous querying, multiple SRV records, as well as other cool kinds of things such as DANE. At the same time, since the vast majority of existing channel drivers can't make use of that information, it is of admittedly limited benefit unless other changes take place.
>     
>     This is, unfortunately, one of those places where making use of a third party stack does make things more difficult for us. Despite all the benefits PJSIP brings - and it is a lot! - there are bound to be some downsides, and its integration with its DNS resolver could be considered one of them. I can see two possible approaches available for us to go make use of a core DNS resolver:
>     
>     (1) Modify PJSIP to support a pluggable DNS resolver. The PJSIP endpoint does support having a DNS resolver be registered to it; unfortunately, today this is an opaque structure that the endpoint uses with pjlib-util/resolver.h. We would have to provide a shim between pj_dns_resolver and the core resolver, and then update PJSIP to directly use the shim – with callbacks between a PJSIP version of the shim and pj_dns_resolver used internally in that project by default. This is doable, but again, not a trivial amount of work. Since the changes would be substantial, we would be forking PJSIP (again) and pushing our changes up-stream (again). This is not something we should shy away from when necessary, but it is something we should keep in mind when making substantial changes in that project.
>     (2) We could eschew any usage of PJSIP's DNS resolution by simply performing all of the resolution ourselves and pre-populating the PJSIP message structs in the res_pjsip* modules with the data. We _think_ that this would work: PJSIP should detect that it already has an address and not bother with any resolution if we did that. This has several obvious drawbacks however: (a) we take the burden of when and how to do resolution out of the library, which often has better knowledge of when to do this action than the res_pjsip* modules (which generally sit at a slightly higher place in the overall SIP stack), and (b) it strikes me as a bit hackish and hence more prone to error.
>     
>     Both of these approaches are doable, but both are a lot of work and can't even be started until there is a functioning DNS core in Asterisk. Doing it without having that core makes little sense; a substantial amount of work would have to be done in PJSIP and chan_pjsip, and for no benefit over what is already provided. I wouldn't start that work until the DNS core - with asynchronous querying and multiple SRV records - existed in Asterisk trunk (or 12).
>     
>     So with the proposed change, where are we today?
>     
>     * We have the ability - with very few function calls - to enable very good DNS support in chan_pjsip. This is unobtrusive: it does not break the existing behavior of Asterisk, nor does it even change the external appearance of chan_pjsip. It just starts to make life better.
>     * This change is non-systemic. It only affects chan_pjsip; it would be extremely difficult for a non-PJSIP channel driver to make use of the resolver (for better or worse). I don't see this as a slippery slope - or at least if it is, it is a very short slope. There are not a plethora of other IP VoIP stacks clamouring to be used with Asterisk - if we had 30 other channel drivers, each with their own way of doing resolution, I would probably feel different. However, that is not the case.
>     * This change does not preclude changes to the Asterisk core. If someone would like to go improve DNS support in the core of Asterisk, I think that would be fantastic – I would love to see that work occur. If it does occur, there are paths forward for making use of that resolver in chan_pjsip. Turning the PJSIP resolve on today does not preclude that from happening, nor does it mean we can't make chan_pjsip use the core resolver in the future.
>     
>     Not including this change does not seem to buy us anything, save for some semblance of architectural purity. While I would love for there to be only one way to perform DNS resolution, that feels like a long term goal – and sacrificing the practicality of delivering a feature that a large number of Asterisk users have wanted for an extremely long time doesn't feel worth it to me.

For every poorly designed bad feature I can think of I can find a large number of Asterisk users that want it. It's simply not a good argument. We create this software and need some sort of architecture when we do. The current DNSmanager is broken in so many ways I can't even begin to describe it and it is not asynch, asterisk still stops if we're not getting an answer. We just break differently.

So why don't we continue down this path and include a full blown SIP proxy in chan_pjsip, and a HTTP server in chan_pjsip and much more. Let's add a b2bua to chan_pjsip while we're at it... ;-)


Even if it can ba done easily, doesn't mean it's right. We do need to manage our product. Adding the ability to configure a different set of DNS servers for a module or even a full server application is a bad thing. That's the first part we should not do. After that, we need to fix our DNS architecture. 


- Olle E


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


On March 13, 2014, 11:42 a.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3343/
> -----------------------------------------------------------
> 
> (Updated March 13, 2014, 11:42 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23435
>     https://issues.asterisk.org/jira/browse/ASTERISK-23435
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change adds a configuration option for setting nameservers to be used by the PJSIP DNS client. If this option is not set then the system nameservers are retrieved and used instead.
> 
> This also allows the nameservers to be changed by doing a reload.
> 
> In case others are wondering as Olle was:
> 
> PJLIB-Util (part of pjproject) provides a DNS client which can optionally (but is highly suggested) to be used with PJSIP. It provides asynchronous DNS, SRV lookups, multiple record support, etc. Right now this isn't enabled so we are simply doing A/AAAA record lookups. The reason it's not enabled is that explicit nameservers *must* be provided to it when enabling it. It will not use the system ones by itself. The change up on reviewboard enables it by default using the system nameservers it finds, but with the ability to override or completely disable it if a user wants. The reason I also provide reload functionality is that people in #asterisk-dev expressed a concern that users may change nameservers but don't want to restart Asterisk, which is understandable. 
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 410470 
>   /branches/12/res/res_pjsip/include/res_pjsip_private.h 410470 
>   /branches/12/res/res_pjsip/config_global.c 410470 
>   /branches/12/res/res_pjsip.c 410470 
>   /branches/12/main/dns.c 410470 
>   /branches/12/include/asterisk/dns.h 410470 
>   /branches/12/configs/pjsip.conf.sample 410470 
>   /branches/12/CHANGES 410470 
> 
> Diff: https://reviewboard.asterisk.org/r/3343/diff/
> 
> 
> Testing
> -------
> 
> Explicitly set nameservers and confirmed they were used by PJSIP. Disabled it and confirmed that the DNS client was disabled. Set to auto (explicitly and by default) and confirmed that the system nameservers were used.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140313/a8784f66/attachment.html>