On 26/06/17 14:17, Hans Dedecker wrote: > On Sun, Jun 25, 2017 at 12:15 AM, Simon Kelley <si...@thekelleys.org.uk> > wrote: >> On 14/06/17 14:46, Hans Dedecker wrote: >>> If a DNS server replies REFUSED for a given DNS query in strict order mode >>> no failover to the next DNS server is triggered as the logic in reply_query >>> excluded strict order mode by mistake. >> >> The above may well be true. >>> >>> Also checking for not strict order mode makes the failover logic related >>> to REFUSED death code as it also checks for forwardall being 0 which can >>> only be the case for strict order mode. >> >> but this is not true. In non-strict-order mode, the query gets forwarded >> to a single server (forwardall == 0) and is the query gets resent from >> the client after timeout, then it gets sent to all servers, and >> forwardall != 0 > Thanks for the explanation regarding the non strict order mode logic; > it was not completely clear to me when I made the patch how non strict > order mode behaved precisely. I will respin the patch based on this. >>> >>> Fix this by checking for strict order mode now so the failover logic in >>> case REFUSED is replied is triggered in case forwardall is 0 for a given >>> forward record. In case all servers mode is configured the fail over logic >>> won't be triggered just as before. >>> >> >> The patch now inhibits sending the query to all other servers when >> strict-order is NOT set. I think it makes more sense to just delete the >> option_bool(OPT_ORDER) condition completely. > The strict order mode for dnsmasq on routers is used quite a lot as > ISPs have often configured primary and secondary DNS servers in their > network. They want the primary DNS server to be contacted first; only > in case of timeout or other error condition like refused fallback to > the secondary DNS server(s) is requested. > Most ISPs don't like all DNS servers being contacted for a given > client query as they perceive it as dns traffic duplication; in the > world of routers strict order mode has still its use.
I think you misunderstood my point, I don't want to remove the --strict-order option, just to remove that line of code completely (which is what your V2 patch does - I shall apply it right now.) Cheers, Simon. > > > Hans >> >> >> Cheers, >> >> Simon. >> >> >>> Signed-off-by: Hans Dedecker <dedec...@gmail.com> >>> Signed-off-by: Mi Feng <bear....@gmail.com> >>> --- >>> >>> Fixes dns failover issue reported in LEDE >>> (https://bugs.lede-project.org/index.php?do=details&task_id=841) >>> >>> src/forward.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/forward.c b/src/forward.c >>> index 83f392d..0ce3612 100644 >>> --- a/src/forward.c >>> +++ b/src/forward.c >>> @@ -790,7 +790,7 @@ void reply_query(int fd, int family, time_t now) >>> /* Note: if we send extra options in the EDNS0 header, we can't recreate >>> the query from the reply. */ >>> if (RCODE(header) == REFUSED && >>> - !option_bool(OPT_ORDER) && >>> + option_bool(OPT_ORDER) && >>> forward->forwardall == 0 && >>> !(forward->flags & FREC_HAS_EXTRADATA)) >>> /* for broken servers, attempt to send to another one. */ >>> >> >
Description: OpenPGP digital signature
_______________________________________________ Dnsmasq-discuss mailing list Dnsmasqemail@example.com http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss