[asterisk-dev] [Code Review] 4528: dns: Add SRV recording parsing, sorting/weighting, and unit tests

[Olle E Johansson]

> On March 31, 2015, 5:28 p.m., Mark Michelson wrote:
> > /trunk/main/dns_srv.c, lines 131-170
> > <https://reviewboard.asterisk.org/r/4528/diff/2/?file=72947#file72947line131>
> >
> >     Reading the language of RFC 2782, I'm unclear if what you're doing here is actually correct:
> >     
> >     "To select a target to be contacted next, arrange all SRV RRs (that have not been ordered yet) in any order, except that all those with weight 0 are placed at the beginning of the list."
> >     
> >     I can interpret this two different ways:
> >     
> >     1) Select all records of a given priority, and place them in a set, ensuring that the zero-weight records are at the beginning of the set. Then run the crazy weighting algorithm.
> >     
> >     2) Select all records of a given priority. Take all 0-weighted records and place them into the sorted list of records first. Then run the crazy weighting algorithm on the rest of the records to determine their place in the list after the 0-weighted records.
> >     
> >     After re-reading a few times, I *think* they mean to do #1. If that's the case, then between the first highlighted list traversal and the while loop, you should place all 0-weight records at the front of temp_list. If they mean #2, then between the first highlighted list traversal and hte while loop, you should remove all 0-weighted records from temp_list and append them to the end of new_list.

Just to be picky - the weight applies to a set of the same priority.

Also note (but this may not be related to this issue...) that the list now contains names that may resolve to multiple IPs of multiple address families. All of these addresses for a name will have to be used, not just one. I.E. If a hostname ends up in five A addresses and one AAAA - there are six connections attempts before that host fails.


- Olle E


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


On March 26, 2015, 7:50 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4528/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 7:50 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change adds the following:
> 
> 1. As SRV records are added to a result the information is parsed and stored away in additional storage in the record. The SRV API can then be used to return this information.
> 2. Before invoking the DNS query callback the list of records on the result are sorted based on priority and weight.
> 3. Unit tests have been added which verify the record parsing, sorting, and weighting. There are also some off nominal which cover the cases when an invalid/corrupt record is received.
> 4. A unit test has also been added to res_resolver_unbound which adds an SRV record to a zone and confirms it is retrieved and parsed correctly.
> 
> 
> Diffs
> -----
> 
>   /trunk/tests/test_dns_srv.c PRE-CREATION 
>   /trunk/res/res_resolver_unbound.c 433370 
>   /trunk/main/dns_srv.c 433370 
>   /trunk/main/dns_core.c 433370 
>   /trunk/include/asterisk/dns_internal.h 433370 
> 
> Diff: https://reviewboard.asterisk.org/r/4528/diff/
> 
> 
> Testing
> -------
> 
> Executed unit tests and confirmed they pass.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

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