Hi Rich,

Many thanks for your feedback, much appreciated!

Yep, you've got my idea. Basically my box has multiple DNS servers setup in resolv.conf, for example, the one specified from my ISP, another public one if needed and the one from my private network (after I setup a VPN tunnel to my office network) and only the latter understand private host names (such as AAA server or remote syslog servers in the office network).

Given the current mechanism implemented by musl's resolver, different servers could reply conflicted answers, for example, NXDOMAIN from public server for private hostnames whereas NOERROR from the private DNS server. I understand your suggestion to further manipulate a local unioning nameserver to handle conflicted replies.

Frankly speaking, in my use case the situation is even more complicated. My boxes are mult-homed, for example, with eth0 and cellular interfaces, the ISP in each network will provide different DNS server. The default gateway for my device may ping-pong among that of eth0 and that of cellular interface, should the cable/eth0 connection becomes down or resumed. In this case I need to re-order nameservers in resolv.conf to put first the name servers associated with the current default gateway's interface.

Again, it is arguable that this can also be handled by changing the policy adopted by the local unioning name server (to prefer which nameserver when handling conflicts). In my mind, serialising DNS servers' queries would spare the need of setting local unioning server and changing its policy - the ordering of nameservers in resolv.conf can be adjusted anytime to put the preferred one first.

What if I introduce a new macro to provide nameserver serialisation along with parallelism? the new macro will default to off to keep the current behaviour. Should the need arise for musl users to put some weight, or precedence on name servers, they could opt in to serialisation and don't worry about further setting up local unioning name server?

Looking forward to your feedback.

Harry



On Mon, Oct 24, 2022 at 1:32 AM Rich Felker <dalias@aerifal.cx> wrote:
On Sun, Oct 23, 2022 at 09:18:01PM +1000, Qingtao Cao wrote:
> Querying all configured DNS servers in parallel boost performance but makes
> it impossible to prefer private DNS servers to public ones. Serialise DNS
> servers access from the top of the nameserver specified in resolv.conf and
> only move on to the next, or less preferred DNS server when the current one
> either timed out or replied NOERROR and NXDOMAIN for all queries
>
> When querying the next or less perferred DNS servers, only unsuccessfully
> resolved host names were queried. When replies are received, they won't replace
> a perviously successful answer. This ensures successful answers from higher
> prioritised DNS servers won't be overridden
>
> This eliminates a racy condition when different DNS servers replies differently
> to a query, the last received one wins
>
> Last but not least, using a local array to save the currently received reply to
> simplify the logic to re-shuffle answers[next] to the corresponding answers[i]
>
> Signed-off-by: Qingtao Cao <qingtao.cao@digi.com>

This patch is being submitted as one to change a very intentional and
longstanding behavior (and with that impose a new contract) and is not
going to be accepted. A basic premise of musl's resolver, that's not
just arbitrary but a consequence of a "fallback" model, is that all
the nameservers provided present a consistent view of a global dns
namespace.

Some people mistake "fallback" for "search" because it kinda appears
to work that way if you don't actually question what happens in the
corner cases. But if you're using fallback as search, failure of the
primary server to answer in time will *change the results* of the
query. This is not how fallback is supposed to work.

It would have been a lot more productive to start from a standpoint of
what you're trying to do. I assume that's "unioning" where you have a
local nameserver serving fake names that don't actually exist in the
real DNS namespace, and a real nameserver that's serving the real DNS
namespace. There are at least 2 valid ways to achieve this that don't
break in the above manner.

1. Run a unioning nameserver locally. The unioning nameserver handles
   all the complex logic of handling clashes and which upstream source
   takes precedence, querying both and only answering if it gets
   sufficient answers to draw a conclusive result about which result
   it can use (and ServFail otherwise).

2. Put your local names under an actual DNS domain (or subdomain zone)
   you actually own. Set the public DNS delegations to point to
   nameservers that don't exist. Now the public DNS will always
   ServFail. Set your private local nameserver to only return results
   for this local zone, and ServFail otherwise. As a result, neither
   is providing conflicting answers for the part of the namespace the
   other is responsible for.

There are likely other ways too.

> ---
>  src/network/res_msend.c | 100 +++++++++++++++++++++++++++-------------
>  1 file changed, 68 insertions(+), 32 deletions(-)
>
> diff --git a/src/network/res_msend.c b/src/network/res_msend.c
> index 9adaea13..130ac4fe 100644
> --- a/src/network/res_msend.c
> +++ b/src/network/res_msend.c
> @@ -10,6 +10,7 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <pthread.h>
> +#include <arpa/nameser.h>
>  #include "stdio_impl.h"
>  #include "syscall.h"
>  #include "lookup.h"
> @@ -41,11 +42,12 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries,
>       int nns = 0;
>       int family = AF_INET;
>       int rlen;
> -     int next;
> -     int i, j;
> +     int i;
>       int cs;
>       struct pollfd pfd;
>       unsigned long t0, t1, t2;
> +     int ns_index, rcode, all_done, unanswered;
> +     unsigned char answer[512];

>       pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);

> @@ -115,19 +117,27 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries,
>       pfd.fd = fd;
>       pfd.events = POLLIN;
>       retry_interval = timeout / attempts;
> -     next = 0;
> +
> +     ns_index = 0; /* Start from the top name server */
> +
> +next_ns:
>       t0 = t2 = mtime();
>       t1 = t2 - retry_interval;

> +     unanswered = 0;
> +
>       for (; t2-t0 < timeout; t2=mtime()) {
>               if (t2-t1 >= retry_interval) {
> -                     /* Query all configured namservers in parallel */
> -                     for (i=0; i<nqueries; i++)
> -                             if (!alens[i])
> -                                     for (j=0; j<nns; j++)
> -                                             sendto(fd, queries[i],
> -                                                     qlens[i], MSG_NOSIGNAL,
> -                                                     (void *)&ns[j], sl);
> +                     for (i=0; i<nqueries; i++) {
> +                             /* Only query unresolved or negative resolved host names */
> +                             rcode = answers[i][3] & 15;
> +                             if (!alens[i] || rcode != NOERROR) {
> +                                     sendto(fd, queries[i],
> +                                             qlens[i], MSG_NOSIGNAL,
> +                                             (void *)&ns[ns_index], sl);
> +                                     unanswered++;
> +                             }
> +                     }
>                       t1 = t2;
>                       servfail_retry = 2 * nqueries;
>               }
> @@ -135,51 +145,77 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries,
>               /* Wait for a response, or until time to retry */
>               if (poll(&pfd, 1, t1+retry_interval-t2) <= 0) continue;

> -             while ((rlen = recvfrom(fd, answers[next], asize, 0,
> +             while ((rlen = recvfrom(fd, answer, 512, 0,
>                 (void *)&sa, (socklen_t[1]){sl})) >= 0) {

>                       /* Ignore non-identifiable packets */
>                       if (rlen < 4) continue;

>                       /* Ignore replies from addresses we didn't send to */
> -                     for (j=0; j<nns && memcmp(ns+j, &sa, sl); j++);
> -                     if (j==nns) continue;
> +                     if (memcmp(ns+ns_index, &sa, sl))
> +                             continue;

>                       /* Find which query this answer goes with, if any */
> -                     for (i=next; i<nqueries && (
> -                             answers[next][0] != queries[i][0] ||
> -                             answers[next][1] != queries[i][1] ); i++);
> +                     for (i=0; i<nqueries && (
> +                             answer[0] != queries[i][0] ||
> +                             answer[1] != queries[i][1] ); i++);
>                       if (i==nqueries) continue;
> -                     if (alens[i]) continue;
> +
> +                     rcode = answers[i][3] & 15;
> +                     if (alens[i] && rcode == NOERROR) {
> +                             /* Do not override successful answers from a more prioritied server */
> +                             continue;
> +                     }

>                       /* Only accept positive or negative responses;
>                        * retry immediately on server failure, and ignore
>                        * all other codes such as refusal. */
> -                     switch (answers[next][3] & 15) {
> -                     case 0:
> -                     case 3:
> -                             break;
> -                     case 2:
> +                     rcode = answer[3] & 15;
> +                     switch (rcode) {
> +                     case NOERROR:
> +                     case NXDOMAIN:
> +                             alens[i] = rlen;
> +                             memcpy(answers[i], answer, rlen);
> +                             unanswered--;
> +                             continue;
> +                     case SERVFAIL:
>                               if (servfail_retry && servfail_retry--)
>                                       sendto(fd, queries[i],
>                                               qlens[i], MSG_NOSIGNAL,
> -                                             (void *)&ns[j], sl);
> +                                             (void *)&ns[ns_index], sl);
>                       default:
>                               continue;
>                       }
> +             }

> -                     /* Store answer in the right slot, or update next
> -                      * available temp slot if it's already in place. */
> -                     alens[i] = rlen;
> -                     if (i == next)
> -                             for (; next<nqueries && alens[next]; next++);
> -                     else
> -                             memcpy(answers[i], answers[next], rlen);
> +             /* Check if all sent queries have got NOERROR or NXDOMAIN replies from this server,
> +              * no need to retry it anymore even when not timed out yet. When the unanswered counter
> +              * drops to zero, any remaining NXDOMAIN replies were sure sent from this server */
> +             if (!unanswered) {
> +                     all_done = 1;
> +                     for (i = 0; i < nqueries; i++) {
> +                             rcode = answers[i][3] & 15;
> +                             if (!(alens[i] && ((rcode == NOERROR) || (rcode == NXDOMAIN)))) {
> +                                     all_done = 0;
> +                                     break;
> +                             }
> +                     }

> -                     if (next == nqueries) goto out;
> +                     if (all_done)
> +                             break;
>               }
>       }
> -out:
> +
> +     /* Check if any queries is unresolved or unsuccessful (regardless of failure code)
> +      * try the next DNS server if available */
> +     if (++ns_index < conf->nns) {
> +             for (i = 0; i < nqueries; i++) {
> +                     rcode = answers[i][3] & 15;
> +                     if (!alens[i] || rcode != NOERROR)
> +                             goto next_ns;
> +             }
> +     }
> +
>       pthread_cleanup_pop(1);

>       return 0;
> --
> 2.34.1