On Wed, May 31, 2023 at 01:40:30PM +0200, Petr Menšík wrote:
> Attached my bigger attempt to solve this problem.
> 7 separate patches included.
>
> Because in rare circumstances servers might have changed since TCP
> connection process started, I serialize domain, server address and its last
> position in server array. Hacked a redirection with special -2 value from
> cache inserting, where it always reads just one server. It then tries server
> at reported position first, iterating on all servers for this domain.
>
> In rare cases there might be a problem, because it does not send source
> address or interface, which might identify correct server. But I doubt those
> would make it different in any real world examples. We have no simple
> identification for changed servers. It works in basic testing well.
>
> Added also separation of TCP and UDP last servers. It should be able to
> forward UDP to server responding just over UDP and TCP to server responding
> just TCP. That should be quite rare case, more teoretical than real-world.
> Maybe change of UDP server should change also TCP, because UDP test can be
> done in parallel.
>
> I have found also unwanted difference from UDP queries. If the response is
> REFUSED, even that were accepted as valid last_server response. Now it sets
> TCP last_server just after non-refused response, not just successful
> connection.
>
> I have tried to look into glibc, that does not seem to set any timeout for
> TCP (vc) queries. Default timeout in dig tool is 10 seconds, it does not
> seem to tweak number of SYN packets sent. I think it just measures time
> before reply arrives. I think ideally we should be able to spawn another TCP
> connection to the other server if it didn't respond in few seconds. And wait
> for fastest response from any of those. But that would require quite
> significant rework of current code.
>
> Did just a basic testing, but those changes improve tested situation.
>
> What do you think about it?
I haven't yet reviewed the patch at the begin of this thread ...
Below some feedback
> Cheers,
> Petr
> From c02cfcb0a358e04636ffd2bcc595860b25b3a440 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
> Date: Wed, 17 May 2023 21:40:19 +0200
> Subject: [PATCH 1/7] Add --dns-tcp-timeout option
>
> Changes send timeout of outgoing TCP connections. Allows waiting just
> short time for successful connection before trying another server.
> Makes possible faster switch to working server if previous is not
> responding. Default socket timeout seems too high for DNS connections.
> ---
> man/dnsmasq.8 | 4 ++++
> src/config.h | 1 +
> src/dnsmasq.h | 1 +
> src/forward.c | 10 ++++++++--
> src/option.c | 10 +++++++++-
> 5 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
> index 30429df..94fd5b1 100644
> --- a/man/dnsmasq.8
> +++ b/man/dnsmasq.8
> @@ -860,6 +860,10 @@ where this needs to be increased is when using
> web-server log file
> resolvers, which can generate large numbers of concurrent queries. This
> parameter actually controls the number of concurrent queries per server
> group, where a server group is the set of server(s) associated with a single
> domain. So if a domain has it's own server via --server=/example.com/1.2.3.4
> and 1.2.3.4 is not responding, but queries for *.example.com cannot go
> elsewhere, then other queries will not be affected. On configurations with
> many such server groups and tight resources, this value may need to be
> reduced.
> .TP
> +.B --dns-tcp-timeout=<seconds>
> +Sets send timeout for forwarded TCP connections. Can be used to reduce time
> of waiting
> +for successful TCP connection. Default value 0 skips the change of it.
> +.TP
> .B --dnssec
> Validate DNS replies and cache DNSSEC data. When forwarding DNS queries,
> dnsmasq requests the
> DNSSEC records needed to validate the replies. The replies are validated and
> the result returned as
...
> diff --git a/src/forward.c b/src/forward.c
> index ecfeebd..f323fee 100644
> --- a/src/forward.c
> +++ b/src/forward.c
> @@ -1929,8 +1929,14 @@ static ssize_t tcp_talk(int first, int last, int
> start, unsigned char *packet,
> /* Copy connection mark of incoming query to outgoing connection. */
> if (have_mark)
> setsockopt(serv->tcpfd, SOL_SOCKET, SO_MARK, &mark, sizeof(unsigned
> int));
> -#endif
> -
Thanks for removing those trailing white spaces.
> +#endif
> +
> + if (daemon->tcp_timeout>0)
> + {
> + struct timeval tv = {daemon->tcp_timeout, 0};
> + setsockopt(serv->tcpfd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv));
> + }
> +
> if ((!local_bind(serv->tcpfd, &serv->source_addr, serv->interface,
> 0, 1)))
> {
> close(serv->tcpfd);
> diff --git a/src/option.c b/src/option.c
> index 8322725..b94a5ff 100644
> --- a/src/option.c
> +++ b/src/option.c
> @@ -190,6 +190,7 @@ struct myoption {
> #define LOPT_FILTER_RR 381
> #define LOPT_NO_DHCP6 382
> #define LOPT_NO_DHCP4 383
> +#define LOPT_TCP_TIMEOUT 384
>
> #ifdef HAVE_GETOPT_LONG
> static const struct option opts[] =
> @@ -3391,7 +3393,12 @@ static int one_opt(int option, char *arg, char
> *errstr, char *gen_err, int comma
> case '0': /* --dns-forward-max */
> if (!atoi_check(arg, &daemon->ftabsize))
> ret_err(gen_err);
> - break;
;-)
> + break;
> +
> + case LOPT_TCP_TIMEOUT: /* --dns-tcp-timeout */
> + if (!atoi_check(arg, &daemon->tcp_timeout))
> + ret_err(gen_err);
> + break;
>
> case 'q': /* --log-queries */
> set_option_bool(OPT_LOG);
> @@ -5833,6 +5840,7 @@ void read_opts(int argc, char **argv, char
> *compile_opts)
> daemon->soa_expiry = SOA_EXPIRY;
> daemon->randport_limit = 1;
> daemon->host_index = SRC_AH;
> + daemon->tcp_timeout = TCP_TIMEOUT;
>
> /* See comment above make_servers(). Optimises server-read code. */
> mark_servers(0);
> --
> 2.40.1
>
> From d9a8d33f195c6c406ff28a0084d1be8b46583b08 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
> Date: Tue, 23 May 2023 21:31:05 +0200
> Subject: [PATCH 2/7] Reduce few TCP related repeated code
>
> Provide special function for repeated setting of ede code in
> pseudoheader. Introduce read_tcp reusing query length and payload.
> ---
> src/dnsmasq.h | 2 +
> src/edns0.c | 9 ++
> src/forward.c | 223 ++++++++++++++++++++------------------------------
> 3 files changed, 100 insertions(+), 134 deletions(-)
>
> diff --git a/src/dnsmasq.h b/src/dnsmasq.h
> index 4113ccb..77296ed 100644
> --- a/src/dnsmasq.h
> +++ b/src/dnsmasq.h
> @@ -1852,6 +1852,8 @@ unsigned char *find_pseudoheader(struct dns_header
> *header, size_t plen,
> size_t *len, unsigned char **p, int
> *is_sign, int *is_last);
> size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned
> char *limit,
> unsigned short udp_sz, int optno, unsigned char *opt,
> size_t optlen, int set_do, int replace);
> +size_t add_pseudoheader_ede(struct dns_header *header, size_t plen, unsigned
> char *limit,
> + unsigned short udp_sz, int ede, int set_do, int
> replace);
> size_t add_do_bit(struct dns_header *header, size_t plen, unsigned char
> *limit);
> size_t add_edns0_config(struct dns_header *header, size_t plen, unsigned
> char *limit,
> union mysockaddr *source, time_t now, int *cacheable);
....
> diff --git a/src/forward.c b/src/forward.c
> index f323fee..37696c8 100644
> --- a/src/forward.c
> +++ b/src/forward.c
....
> @@ -1694,7 +1708,6 @@ void receive_query(struct listener *listen, time_t now)
> if (extract_request(header, (size_t)n, daemon->namebuff, &type))
> {
> #ifdef HAVE_AUTH
> - struct auth_zone *zone;
> #endif
That would become
> #ifdef HAVE_AUTH
> #endif
I suggest
> -#ifdef HAVE_AUTH
> - struct auth_zone *zone;
> -#endif
(removing three lines)
> log_query_mysockaddr(F_QUERY | F_FORWARD, daemon->namebuff,
> &source_addr, auth_dns ? "auth" : "query", type);
> @@ -1704,15 +1717,8 @@ void receive_query(struct listener *listen, time_t now)
.....
Groeten
Geert Stappers
--
Silence is hard to parse
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
