Le 18/08/2026 à 7:24 PM, Manu Nicolas a écrit :
RFC 1123 and RFC 2181 recommend retrying a truncated DNS response over
a transport capable of carrying a larger message. HAProxy currently
requires an always-on tcp@ nameserver for this. A regular UDP
nameserver instead handles TC as a resolver error.
Make unprefixed IPv4, IPv6 and hostname nameservers use UDP first, then
lazily retry a validated truncated response over TCP against the same
address and port. Explicit udp@, udp4@, udp6@ and dgram+ addresses
remain UDP-only, while tcp@ remains TCP-only. Nameservers imported from
resolv.conf also use UDP first. UDP-only nameservers retain the existing
partial handling of truncated SRV responses.
The normal response path matches the query ID and validates the
returned question before acting on TC. The fallback keeps the query ID
and type, does not consume a resolver retry or perform the A/AAAA
fallback, and reuses the existing DNS stream connection pooling and
timeout handling. A truncated TCP response follows the normal error
path and does not start another fallback.
Keep accepted_payload_size as the EDNS advertisement and UDP receive
limit. Stream responses are accepted up to the DNS protocol maximum of
65535 bytes, so a complete response is not rejected by the smaller UDP
limit that caused truncation.
Expose successful fallback dispatches through a tcp_fallback counter
in the CLI, stats, and Prometheus output. TCP connection and send
failures continue to use the existing snd_error counter.
Hi Many,
I reviewed your patch. Inlined my comments.
---
addons/promex/README | 1 +
doc/configuration.txt | 34 +++--
include/haproxy/dns-t.h | 12 +-
include/haproxy/dns.h | 4 +-
include/haproxy/resolvers-t.h | 3 +-
src/dns.c | 23 +--
src/resolvers.c | 268 ++++++++++++++++++++++++++--------
7 files changed, 261 insertions(+), 84 deletions(-)
diff --git a/addons/promex/README b/addons/promex/README
index 9ed37debe..978052ec8 100644
--- a/addons/promex/README
+++ b/addons/promex/README
@@ -430,5 +430,6 @@ listed below. Metrics from extra counters are not listed.
| haproxy_resolver_other |
| haproxy_resolver_invalid |
| haproxy_resolver_too_big |
+| haproxy_resolver_tcp_fallback |
| haproxy_resolver_outdated |
+----------------------------------------------------+
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 06a54d09c..6e438afad 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -20214,24 +20214,37 @@ resolvers <resolvers id>
A resolvers section accept the following parameters:
accepted_payload_size <nb>
- Defines the maximum payload size accepted by HAProxy and announced to all the
- name servers configured in this resolvers section.
+ Defines the maximum UDP payload size accepted by HAProxy and announced to all
+ the name servers configured in this resolvers section.
<nb> is in bytes. If not set, HAProxy announces 512. (minimal value defined
by RFC 6891)
Note: the maximum allowed value is 65535. Recommended value for UDP is
4096 and it is not recommended to exceed 8192 except if you are sure
that your system and network can handle this (over 65507 makes no
sense
- since is the maximum UDP payload size). If you are using only TCP
- nameservers to handle huge DNS responses, you should put this value
- to the max: 65535.
+ since it is the maximum UDP payload size). DNS responses received over
a
+ stream transport are accepted up to the DNS protocol maximum of 65535
+ bytes regardless of this setting.
This change must be performed in a dedicated patch. We must be sure there is no
reason to limit the payload size for TCP. Otherwise it could be seen as a
breaking change.
nameserver <name> <address>[:port] [param*]
Used to configure a nameserver. <name> of the nameserver should ne unique.
- By default the <address> is considered of type datagram. This means if an
- IPv4 or IPv6 is configured without special address prefixes (paragraph 11.)
- the UDP protocol will be used. If an stream protocol address prefix is used,
- the nameserver will be considered as a stream server (TCP for instance) and
+ By default an IPv4 or IPv6 <address> is queried over UDP. When a valid UDP
+ response has its truncated (TC) flag set, the same query is retried over TCP
+ against the same address and port. The TCP connection is established lazily,
+ only after a truncated response is received, and uses the regular DNS stream
+ connection pooling, pipelining, timeouts, and idle connection cleanup.
+
+ An explicit datagram address prefix such as "udp@", "udp4@", "udp6@", or
+ "dgram+" forces UDP-only operation and disables TCP fallback. An explicit
+ stream address prefix such as "tcp@" forces TCP-only operation.
+
+ A failure to connect or send over TCP is handled by the regular resolver
+ timeout and retry mechanism. Starting TCP fallback does not consume a retry
+ or switch the query between A and AAAA. A response received over TCP is
+ accepted up to the DNS protocol maximum of 65535 bytes.
+
+ If a stream protocol address prefix is used, the nameserver will be
+ considered as a stream server (TCP for instance) and
"server" parameters found in 5.2 paragraph which are relevant for DNS
resolving will be considered. Note: currently, in TCP mode, 4 queries are
pipelined on the same connections. A batch of idle connections are removed
@@ -20241,7 +20254,8 @@ nameserver <name> <address>[:port] [param*]
parse-resolv-conf
Adds all nameservers found in /etc/resolv.conf to this resolvers nameservers
list. Ordered as if each nameserver in /etc/resolv.conf was individually
- placed in the resolvers section in place of this directive.
+ placed in the resolvers section in place of this directive. These nameservers
+ use UDP normally and retry truncated responses over TCP.
hold <status> <period>
Upon receiving the DNS response <status>, determines whether a server's
state
diff --git a/include/haproxy/dns-t.h b/include/haproxy/dns-t.h
index 10175cfdf..18f1a6766 100644
--- a/include/haproxy/dns-t.h
+++ b/include/haproxy/dns-t.h
@@ -82,6 +82,7 @@ struct dns_additional_record {
* A name server belongs to a resolvers section.
*/
struct dns_stream_server {
+ struct dns_nameserver *ns;
struct server *srv;
struct dns_ring *ring_req;
int consecutive_errors; /* number of errors since last successful
query (atomically updated without lock) */
@@ -136,6 +137,11 @@ struct dns_session {
int shutdown;
};
+enum dns_transport {
+ DNS_TRANSPORT_DGRAM = 0,
+ DNS_TRANSPORT_STREAM,
+};
+
I'm not a big fan of adding this enum. At least not with this naming. For
send/recv functions, we can have dedicated functions. For instance
dns_rcv_dgram_nameserver/dns_rcv_dgram_nameserver and
dns_send_stream_nameserver/dns_send_stream_nameserver. About the
resolv_process_responses() function, I guess we can pass the dns server to use
and compare it against ns->dgram and ns->stream. But it is debatable and your
solution works. Maybe renaming the enum into DNS_STREAM_SERVER_TYPE is good
enough. So specifying the dns server type to use instream of the transport to use.
In addition, if possible, it could be good to add helper functions to make tests
more obvious. One to know if the server to use is a dgram server or a stream
server. And one to know if it is the fallback server or not.
Finally, to ease bugs tracking, it could be good to first add a patch to
introduce the dns server selection depending on an extra argument (or the api
change, as you want). But everything not related to the tcp fallback. And then
add another patch to perform the tcp fallback. It is important to split changes
as far as possible. Especially on resolvers. This part is a mess to debug.
/* Structure describing a name server
*/
struct dns_nameserver {
@@ -147,9 +153,10 @@ struct dns_nameserver {
int line; /* line where the section appears */
} conf; /* config information */
- int (*process_responses)(struct dns_nameserver *ns); /* callback used to process responses */
+ int (*process_responses)(struct dns_nameserver *ns, enum dns_transport
transport); /* callback used to process responses */
struct dns_dgram_server *dgram; /* used for dgram dns */
struct dns_stream_server *stream; /* used for tcp dns */
+ unsigned int fallback_tcp; /* retry truncated datagram responses
over stream */
I think we can avoid to add an extra field for that. here when fallback_tcp is
set, dgram and stream servers are both set too. So "ns->fallback_tcp != 0" is
equivalent to "(ns->dgram != NULL && ns->stream != NULL)". Because it is only
tested at few places, it is no a big deal. However, "stream" field comment must
be augmented to document this case.
EXTRA_COUNTERS(extra_counters);
char *extra_counters_storage; /* storage used for extra_counters
above */
@@ -179,7 +186,8 @@ struct dns_counters {
long long invalid; /* - malformed DNS response */
long long too_big; /* - too big response */
long long outdated; /* - outdated response (server
slower than the other ones) */
- long long truncated; /* - truncated response */;
+ long long truncated; /* - truncated response */
+ long long tcp_fallback; /* - truncated response retried
over TCP */
Removing fallback_tcp field from dns_namesaver structure should avoid confusion
with this counter. fallback_tcp / tcp_fallback. We would have always a doubt on
what info we manipulate.
} resolver;
} app; /* application specific counteurs */
};
diff --git a/include/haproxy/dns.h b/include/haproxy/dns.h
index 3e0cd5939..f9d3875de 100644
--- a/include/haproxy/dns.h
+++ b/include/haproxy/dns.h
@@ -25,8 +25,8 @@
#include <haproxy/dns-t.h>
#include <haproxy/server-t.h>
-int dns_send_nameserver(struct dns_nameserver *ns, void *buf, size_t len);
-ssize_t dns_recv_nameserver(struct dns_nameserver *ns, void *data, size_t
size);
+int dns_send_nameserver(struct dns_nameserver *ns, enum dns_transport
transport, void *buf, size_t len);
+ssize_t dns_recv_nameserver(struct dns_nameserver *ns, enum dns_transport
transport, void *data, size_t size);
int dns_dgram_init(struct dns_nameserver *ns, struct sockaddr_storage *sk);
int dns_stream_init(struct dns_nameserver *ns, struct server *s);
void dns_nameserver_deinit(struct dns_nameserver *ns);
diff --git a/include/haproxy/resolvers-t.h b/include/haproxy/resolvers-t.h
index f0c92084c..66c996878 100644
--- a/include/haproxy/resolvers-t.h
+++ b/include/haproxy/resolvers-t.h
@@ -143,7 +143,7 @@ struct resolv_response {
*/
struct resolvers {
__decl_thread(HA_SPINLOCK_T lock);
- unsigned int accepted_payload_size; /* maximum payload size we accept
for responses */
+ unsigned int accepted_payload_size; /* maximum UDP payload size we
announce and accept */
As said, this kind of change must be placed in a dedicated patch
int nb_nameservers; /* total number of nameservers in a
resolvers section */
int resolve_retries; /* number of retries before giving
up */
struct { /* time to: */
@@ -219,6 +219,7 @@ struct resolv_resolution {
int try; /* current resolution try */
int nb_queries; /* count number of queries
sent */
int nb_responses; /* count number of responses
received */
+ int nb_tcp_fallback; /* count TCP fallback
responses still outstanding */
struct resolv_response response; /* structure hosting the DNS response */
struct resolv_query_item response_query_records[DNS_MAX_QUERY_RECORDS]; /*
<response> query records */
diff --git a/src/dns.c b/src/dns.c
index 53e71de0e..6f99866b7 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -132,16 +132,16 @@ static int dns_connect_nameserver(struct dns_nameserver
*ns)
return 0;
}
-/* Sends a message to a name server
+/* Sends a message using <transport> on name server <ns>.
* It returns message length on success
* or -1 in error case
* 0 is returned in case of output ring buffer is full
*/
-int dns_send_nameserver(struct dns_nameserver *ns, void *buf, size_t len)
+int dns_send_nameserver(struct dns_nameserver *ns, enum dns_transport
transport, void *buf, size_t len)
{
int ret = -1;
- if (ns->dgram) {
+ if (transport == DNS_TRANSPORT_DGRAM && ns->dgram) {
struct dgram_conn *dgram = &ns->dgram->conn;
int fd;
@@ -190,7 +190,7 @@ int dns_send_nameserver(struct dns_nameserver *ns, void *buf, size_t len)
ns->counters->sent++;
HA_SPIN_UNLOCK(DNS_LOCK, &dgram->lock);
}
- else if (ns->stream) {
+ else if (transport == DNS_TRANSPORT_STREAM && ns->stream) {
struct ist myist;
myist = ist2(buf, len);
@@ -208,16 +208,16 @@ int dns_send_nameserver(struct dns_nameserver *ns, void
*buf, size_t len)
void dns_session_free(struct dns_session *);
-/* Receives a dns message
+/* Receives a DNS message using <transport> on name server <ns>.
* Returns message length
* 0 is returned if no more message available
* -1 in error case
*/
-ssize_t dns_recv_nameserver(struct dns_nameserver *ns, void *data, size_t size)
+ssize_t dns_recv_nameserver(struct dns_nameserver *ns, enum dns_transport
transport, void *data, size_t size)
{
ssize_t ret = -1;
- if (ns->dgram) {
+ if (transport == DNS_TRANSPORT_DGRAM && ns->dgram) {
struct dgram_conn *dgram = &ns->dgram->conn;
struct sockaddr_storage from = {0};
socklen_t fromlen = sizeof(from);
@@ -249,7 +249,7 @@ ssize_t dns_recv_nameserver(struct dns_nameserver *ns, void
*data, size_t size)
}
HA_SPIN_UNLOCK(DNS_LOCK, &dgram->lock);
}
- else if (ns->stream) {
+ else if (transport == DNS_TRANSPORT_STREAM && ns->stream) {
struct dns_stream_server *dss = ns->stream;
struct dns_session *ds;
@@ -353,7 +353,7 @@ static void dns_resolve_recv(struct dgram_conn *dgram)
HA_SPIN_UNLOCK(DNS_LOCK, &dgram->lock);
- ns->process_responses(ns);
+ ns->process_responses(ns, DNS_TRANSPORT_DGRAM);
}
/* Called when a dns network socket is ready to send data */
@@ -981,6 +981,8 @@ static void dns_session_release(struct appctx *appctx)
/* we flush pending sent queries because we never
* have responses
*/
+ if (ds->dss->ns->fallback_tcp)
+ ds->dss->ns->counters->snd_error++;
Why incrementing an error only in this case ? It is a naive question. I reallu
don't know.
ds->nb_queries -= ds->onfly_queries;
dns_queries_flush(ds);
@@ -1336,7 +1338,7 @@ static struct task *dns_process_rsp(struct task *t, void *context, unsigned int
{
struct dns_nameserver *ns = (struct dns_nameserver *)context;
- ns->process_responses(ns);
+ ns->process_responses(ns, DNS_TRANSPORT_STREAM);
return t;
}
@@ -1352,6 +1354,7 @@ int dns_stream_init(struct dns_nameserver *ns, struct
server *srv)
goto out;
}
+ dss->ns = ns;
This new field is only used to increment the snd_error counter just above. No
way to avoid it ? All cross-references in this part are really a nightmare when
debugging.
dss->srv = srv;
dss->maxconn = srv->maxconn;
diff --git a/src/resolvers.c b/src/resolvers.c
index 06fb19c41..345594a12 100644
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -96,28 +96,30 @@ enum {
RSLV_STAT_INVALID,
RSLV_STAT_TOO_BIG,
RSLV_STAT_TRUNCATED,
+ RSLV_STAT_TCP_FALLBACK,
RSLV_STAT_OUTDATED,
RSLV_STAT_END,
};
static struct stat_col resolv_stats[] = {
- [RSLV_STAT_ID] = { .name = "id", .desc = "ID" },
- [RSLV_STAT_PID] = { .name = "pid", .desc = "Parent ID"
},
- [RSLV_STAT_SENT] = { .name = "sent", .desc = "Sent" },
- [RSLV_STAT_SND_ERROR] = { .name = "send_error", .desc = "Send error"
},
- [RSLV_STAT_VALID] = { .name = "valid", .desc = "Valid" },
- [RSLV_STAT_UPDATE] = { .name = "update", .desc = "Update" },
- [RSLV_STAT_CNAME] = { .name = "cname", .desc = "CNAME" },
- [RSLV_STAT_CNAME_ERROR] = { .name = "cname_error", .desc = "CNAME
error" },
- [RSLV_STAT_ANY_ERR] = { .name = "any_err", .desc = "Any errors"
},
- [RSLV_STAT_NX] = { .name = "nx", .desc = "NX" },
- [RSLV_STAT_TIMEOUT] = { .name = "timeout", .desc = "Timeout" },
- [RSLV_STAT_REFUSED] = { .name = "refused", .desc = "Refused" },
- [RSLV_STAT_OTHER] = { .name = "other", .desc = "Other" },
- [RSLV_STAT_INVALID] = { .name = "invalid", .desc = "Invalid" },
- [RSLV_STAT_TOO_BIG] = { .name = "too_big", .desc = "Too big" },
- [RSLV_STAT_TRUNCATED] = { .name = "truncated", .desc = "Truncated"
},
- [RSLV_STAT_OUTDATED] = { .name = "outdated", .desc = "Outdated" },
+ [RSLV_STAT_ID] = { .name = "id", .desc = "ID" },
+ [RSLV_STAT_PID] = { .name = "pid", .desc = "Parent
ID" },
+ [RSLV_STAT_SENT] = { .name = "sent", .desc = "Sent" },
+ [RSLV_STAT_SND_ERROR] = { .name = "send_error", .desc = "Send
error" },
+ [RSLV_STAT_VALID] = { .name = "valid", .desc = "Valid" },
+ [RSLV_STAT_UPDATE] = { .name = "update", .desc = "Update" },
+ [RSLV_STAT_CNAME] = { .name = "cname", .desc = "CNAME" },
+ [RSLV_STAT_CNAME_ERROR] = { .name = "cname_error", .desc = "CNAME
error" },
+ [RSLV_STAT_ANY_ERR] = { .name = "any_err", .desc = "Any
errors" },
+ [RSLV_STAT_NX] = { .name = "nx", .desc = "NX" },
+ [RSLV_STAT_TIMEOUT] = { .name = "timeout", .desc = "Timeout"
},
+ [RSLV_STAT_REFUSED] = { .name = "refused", .desc = "Refused"
},
+ [RSLV_STAT_OTHER] = { .name = "other", .desc = "Other" },
+ [RSLV_STAT_INVALID] = { .name = "invalid", .desc = "Invalid"
},
+ [RSLV_STAT_TOO_BIG] = { .name = "too_big", .desc = "Too big"
},
+ [RSLV_STAT_TRUNCATED] = { .name = "truncated", .desc =
"Truncated" },
+ [RSLV_STAT_TCP_FALLBACK] = { .name = "tcp_fallback", .desc = "TCP
fallback" },
+ [RSLV_STAT_OUTDATED] = { .name = "outdated", .desc = "Outdated"
},
};
static struct dns_counters dns_counters;
@@ -185,6 +187,9 @@ static int resolv_fill_stats(struct stats_module *mod,
struct extra_counters *ct
case RSLV_STAT_TRUNCATED:
metric = mkf_u64(FN_GAUGE,
counters->app.resolver.truncated);
break;
+ case RSLV_STAT_TCP_FALLBACK:
+ metric = mkf_u64(FN_GAUGE,
counters->app.resolver.tcp_fallback);
+ break;
case RSLV_STAT_OUTDATED:
metric = mkf_u64(FN_GAUGE,
counters->app.resolver.outdated);
break;
@@ -454,9 +459,10 @@ static int resolv_send_query(struct resolv_resolution
*resolution)
int len;
/* Update resolution */
- resolution->nb_queries = 0;
- resolution->nb_responses = 0;
- resolution->last_query = now_ms;
+ resolution->nb_queries = 0;
+ resolution->nb_responses = 0;
+ resolution->nb_tcp_fallback = 0;
+ resolution->last_query = now_ms;
len = resolv_build_query(resolution->query_id, resolution->query_type,
resolvers->accepted_payload_size,
@@ -470,7 +476,9 @@ static int resolv_send_query(struct resolv_resolution
*resolution)
}
list_for_each_entry(ns, &resolvers->nameservers, list) {
- if (dns_send_nameserver(ns, trash.area, len) >= 0)
+ enum dns_transport transport = ns->dgram ? DNS_TRANSPORT_DGRAM
: DNS_TRANSPORT_STREAM;
+
+ if (dns_send_nameserver(ns, transport, trash.area, len) >= 0)
resolution->nb_queries++;
}
@@ -480,6 +488,34 @@ static int resolv_send_query(struct resolv_resolution *resolution)
return 0;
}
+/* Retries a truncated datagram query over the stream transport of the same
+ * nameserver. Returns 1 if the query was queued, 0 otherwise.
+ */
+static int resolv_send_tcp_fallback(struct resolv_resolution *resolution,
+ struct dns_nameserver *ns)
+{
+ struct resolvers *resolvers = resolution->resolvers;
+ int len;
+
+ len = resolv_build_query(resolution->query_id, resolution->query_type,
+ resolvers->accepted_payload_size,
+ resolution->hostname_dn,
resolution->hostname_dn_len,
+ trash.area, trash.size);
+ if (len < 0)
+ return 0;
+
+ if (dns_send_nameserver(ns, DNS_TRANSPORT_STREAM, trash.area, len) < 0)
+ return 0;
+
+ resolution->nb_queries++;
+ resolution->nb_tcp_fallback++;
+ resolution->last_query = now_ms;
+ LIST_DEL_INIT(&resolution->list);
+ LIST_APPEND(&resolvers->resolutions.curr, &resolution->list);
+ ns->counters->app.resolver.tcp_fallback++;
+ return 1;
+}
+
/* Prepares and sends a DNS resolution. It returns 1 if the query was sent, 0
if
* skipped and -1 if an error occurred.
*/
@@ -574,6 +610,7 @@ static void resolv_reset_resolution(struct
resolv_resolution *resolution)
resolution->last_resolution = now_ms;
resolution->nb_queries = 0;
resolution->nb_responses = 0;
+ resolution->nb_tcp_fallback = 0;
resolution->query_type = resolution->prefered_query_type;
/* clean up query id */
@@ -1006,12 +1043,14 @@ static void resolv_check_response(struct
resolv_resolution *res)
*
* The result is stored in <resolution>' response, buf_response,
* response_query_records and response_answer_records members.
+ * A truncated SRV response may be parsed when <allow_truncated_srv> is set.
*
* This function returns one of the RSLV_RESP_* code to indicate the type of
* error found.
*/
static int resolv_validate_dns_response(unsigned char *resp, unsigned char
*bufend,
- struct resolv_resolution *resolution,
int max_answer_records)
+ struct resolv_resolution *resolution,
int max_answer_records,
+ int allow_truncated_srv)
{
unsigned char *reader;
char *previous_dname, tmpname[DNS_MAX_NAME_SIZE];
@@ -1091,10 +1130,6 @@ static int resolv_validate_dns_response(unsigned char
*resp, unsigned char *bufe
if (reader + 2 > bufend)
goto invalid_resp;
r_res->header.ancount = reader[0] * 256 + reader[1];
- if (r_res->header.ancount == 0) {
- cause = RSLV_RESP_ANCOUNT_ZERO;
- goto return_error;
- }
/* Check if too many records are announced */
if (r_res->header.ancount > max_answer_records)
@@ -1149,14 +1184,24 @@ static int resolv_validate_dns_response(unsigned char
*resp, unsigned char *bufe
query->class = reader[0] * 256 + reader[1];
reader += 2;
+ if (query->type != resolution->query_type || query->class != DNS_RCLASS_IN)
+ goto invalid_resp;
+
The reason of this test is not obvious. Especially because an old response
received after a A->AAAA switch was properly parsed before your patch. It could
be good to explain this change. In addition, at first glance, it is not related
to the tcp-fallback feature. If so, it may be good to move it in a dedicated patch.
/* TRUNCATED flag must be checked after we could read the query type
* because a TRUNCATED SRV query type response can still be exploited
+ * when TCP fallback is disabled.
*/
- if (query->type != DNS_RTYPE_SRV && flags & DNS_FLAG_TRUNCATED) {
+ if ((flags & DNS_FLAG_TRUNCATED) &&
+ (query->type != DNS_RTYPE_SRV || !allow_truncated_srv)) {
cause = RSLV_RESP_TRUNCATED;
goto return_error;
}
+ if (r_res->header.ancount == 0) {
+ cause = RSLV_RESP_ANCOUNT_ZERO;
+ goto return_error;
+ }
+
/* now parsing response records */
nb_saved_records = 0;
for (i = 0; i < r_res->header.ancount; i++) {
@@ -2312,14 +2357,14 @@ void resolv_unlink_resolution(struct resolv_requester
*requester)
leave_resolver_code();
}
-/* Called when a network IO is generated on a name server socket for an incoming
- * packet. It performs the following actions:
+/* Called when a network IO is generated on <transport> for an incoming packet
+ * from <ns>. It performs the following actions:
* - check if the packet requires processing (not outdated resolution)
* - ensure the DNS packet received is valid and call requester's callback
* - call requester's error callback if invalid response
* - check the dn_name in the packet against the one sent
*/
-static int resolv_process_responses(struct dns_nameserver *ns)
+static int resolv_process_responses(struct dns_nameserver *ns, enum
dns_transport transport)
{
struct dns_counters *tmpcounters;
struct resolvers *resolvers;
@@ -2328,6 +2373,7 @@ static int resolv_process_responses(struct dns_nameserver
*ns)
unsigned char *bufend;
int buflen, dns_resp;
int max_answer_records;
+ unsigned int max_payload_size;
unsigned short query_id;
struct eb32_node *eb;
struct resolv_requester *req;
@@ -2339,14 +2385,17 @@ static int resolv_process_responses(struct
dns_nameserver *ns)
/* process all pending input messages */
while (1) {
+ max_payload_size = (transport == DNS_TRANSPORT_STREAM) ?
+ DNS_TCP_MSG_MAX_SIZE :
resolvers->accepted_payload_size;
+
No reason to compute the max payload size in the while loop. Your are using
DNS_TCP_MSG_MAX_SIZE value here but the buffer was initialized using
DNS_MAX_UDP_MESSAGE. It is puzzuling. Both are defined to the same value. It
could be good to merge them into one, for instance DNS_MAX_MSG_SIZE (in a
dedicated patch :)
/* read message received */
- memset(buf, '\0', resolvers->accepted_payload_size + 1);
- if ((buflen = dns_recv_nameserver(ns, (void *)buf, sizeof(buf)))
<= 0) {
+ memset(buf, '\0', max_payload_size + 1);
It is not related to your patch, but the memset is useless here. And have a
cost. It can be removed.
+ buflen = dns_recv_nameserver(ns, transport, buf, sizeof(buf));
+ if (buflen <= 0)
break;
- }
/* message too big */
- if (buflen > resolvers->accepted_payload_size) {
+ if (buflen > max_payload_size) {
ns->counters->app.resolver.too_big++;
continue;
}
@@ -2371,11 +2420,15 @@ static int resolv_process_responses(struct
dns_nameserver *ns)
/* known query id means a resolution in progress */
res = eb32_entry(eb, struct resolv_resolution, qid);
+ if (transport == DNS_TRANSPORT_STREAM && ns->fallback_tcp &&
+ res->nb_tcp_fallback)
+ res->nb_tcp_fallback--;> /* number of
responses received */
res->nb_responses++;
- max_answer_records = (resolvers->accepted_payload_size - DNS_HEADER_SIZE) / DNS_MIN_RECORD_SIZE;
- dns_resp = resolv_validate_dns_response(buf, bufend, res,
max_answer_records);
+ max_answer_records = (max_payload_size - DNS_HEADER_SIZE) /
DNS_MIN_RECORD_SIZE;
+ dns_resp = resolv_validate_dns_response(buf, bufend, res,
max_answer_records,
+ !ns->fallback_tcp);
switch (dns_resp) {
case RSLV_RESP_VALID:
@@ -2411,6 +2464,9 @@ static int resolv_process_responses(struct dns_nameserver
*ns)
case RSLV_RESP_TRUNCATED:
res->status = RSLV_STATUS_OTHER;
ns->counters->app.resolver.truncated++;
+ if (transport == DNS_TRANSPORT_DGRAM &&
ns->fallback_tcp &&
+ resolv_send_tcp_fallback(res, ns))
+ continue;
break;
case RSLV_RESP_NO_EXPECTED_RECORD:
@@ -2554,8 +2610,12 @@ struct task *process_resolvers(struct task *t, void
*context, unsigned int state
}
else {
/* Otherwise resend the DNS query and requeue the
resolution */
- if (!res->nb_responses || res->prefered_query_type !=
res->query_type) {
- /* No response received (a real timeout) or
fallback already done */
+ if (!res->nb_responses || res->nb_tcp_fallback ||
+ res->prefered_query_type != res->query_type) {
+ /* No response was received, a TCP fallback
response is
+ * still outstanding, or the query-type
fallback was
+ * already performed.
+ */
res->query_type = res->prefered_query_type;
res->try--;
}
@@ -3103,21 +3163,22 @@ static int
cli_io_handler_dump_resolvers_to_buffer(struct appctx *appctx)
list_for_each_entry_from(ns, &resolvers->nameservers,
list) {
chunk_reset(&trash);
chunk_appendf(&trash, " nameserver %s:\n",
ns->id);
- chunk_appendf(&trash, " sent: %lld\n",
ns->counters->sent);
- chunk_appendf(&trash, " snd_error: %lld\n",
ns->counters->snd_error);
- chunk_appendf(&trash, " valid: %lld\n",
ns->counters->app.resolver.valid);
- chunk_appendf(&trash, " update: %lld\n",
ns->counters->app.resolver.update);
- chunk_appendf(&trash, " cname: %lld\n",
ns->counters->app.resolver.cname);
- chunk_appendf(&trash, " cname_error: %lld\n",
ns->counters->app.resolver.cname_error);
- chunk_appendf(&trash, " any_err: %lld\n",
ns->counters->app.resolver.any_err);
- chunk_appendf(&trash, " nx: %lld\n",
ns->counters->app.resolver.nx);
- chunk_appendf(&trash, " timeout: %lld\n",
ns->counters->app.resolver.timeout);
- chunk_appendf(&trash, " refused: %lld\n",
ns->counters->app.resolver.refused);
- chunk_appendf(&trash, " other: %lld\n",
ns->counters->app.resolver.other);
- chunk_appendf(&trash, " invalid: %lld\n",
ns->counters->app.resolver.invalid);
- chunk_appendf(&trash, " too_big: %lld\n",
ns->counters->app.resolver.too_big);
- chunk_appendf(&trash, " truncated: %lld\n",
ns->counters->app.resolver.truncated);
- chunk_appendf(&trash, " outdated: %lld\n",
ns->counters->app.resolver.outdated);
+ chunk_appendf(&trash, " sent: %lld\n",
ns->counters->sent);
+ chunk_appendf(&trash, " snd_error: %lld\n",
ns->counters->snd_error);
+ chunk_appendf(&trash, " valid: %lld\n",
ns->counters->app.resolver.valid);
+ chunk_appendf(&trash, " update: %lld\n",
ns->counters->app.resolver.update);
+ chunk_appendf(&trash, " cname: %lld\n",
ns->counters->app.resolver.cname);
+ chunk_appendf(&trash, " cname_error: %lld\n",
ns->counters->app.resolver.cname_error);
+ chunk_appendf(&trash, " any_err: %lld\n",
ns->counters->app.resolver.any_err);
+ chunk_appendf(&trash, " nx: %lld\n",
ns->counters->app.resolver.nx);
+ chunk_appendf(&trash, " timeout: %lld\n",
ns->counters->app.resolver.timeout);
+ chunk_appendf(&trash, " refused: %lld\n",
ns->counters->app.resolver.refused);
+ chunk_appendf(&trash, " other: %lld\n",
ns->counters->app.resolver.other);
+ chunk_appendf(&trash, " invalid: %lld\n",
ns->counters->app.resolver.invalid);
+ chunk_appendf(&trash, " too_big: %lld\n",
ns->counters->app.resolver.too_big);
+ chunk_appendf(&trash, " truncated: %lld\n",
ns->counters->app.resolver.truncated);
+ chunk_appendf(&trash, " tcp_fallback: %lld\n",
ns->counters->app.resolver.tcp_fallback);
+ chunk_appendf(&trash, " outdated: %lld\n",
ns->counters->app.resolver.outdated);
if (applet_putchk(appctx, &trash) == -1)
goto full;
ctx->ns = ns;
@@ -3485,6 +3546,54 @@ void resolvers_setup_proxy(struct proxy *px)
px->options2 |= PR_O2_INDEPSTR | PR_O2_SMARTCON;
}
+/* Adds the stream transport used for TCP fallback, named <name> and targeting
+ * <sk>, to <ns>. Errors are stored in <errmsg> when it is non-NULL. Returns a
+ * combination of ERR_* flags.
+ */
+static int resolv_add_tcp_fallback(struct dns_nameserver *ns, const char *file,
+ int linenum, const char *name,
+ const struct sockaddr_storage *sk,
+ char **errmsg)
+{
+ char *address = NULL;
+ char *tcp_addr = NULL;
+ char *tcp_args[5] = { "nameserver", NULL, NULL, "", "" };
+ int err_code = 0;
+
+ address = sa2str(sk, get_host_port(sk), 0);
+ if (!address || !memprintf(&tcp_addr, "tcp@%s", address)) {
+ if (errmsg)
+ memprintf(errmsg, "parsing [%s:%d] : out of memory.",
file, linenum);
+ err_code |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+
+ tcp_args[1] = (char *)name;
+ tcp_args[2] = tcp_addr;
+ err_code |= parse_server(file, linenum, tcp_args, curr_resolvers->px,
NULL,
+
SRV_PARSE_PARSE_ADDR|SRV_PARSE_INITIAL_RESOLVE);
+ if (err_code & (ERR_FATAL|ERR_ABORT)) {
+ if (errmsg && !*errmsg)
+ memprintf(errmsg, "parsing [%s:%d] : failed to initialize
TCP fallback for nameserver '%s'.",
+ file, linenum, name);
+ err_code |= ERR_ABORT;
+ goto out;
+ }
+
+ if (dns_stream_init(ns, proxy_last_server(curr_resolvers->px)) < 0) {
It is funny, thanks to this line, I found an issue. Other TCP nameservers are
initialized using "proxy_first_server()". So, when there are several TCP
nameservers, the first server is used for all nameservers. I will fix that !
+ if (errmsg)
+ memprintf(errmsg, "parsing [%s:%d] : out of memory.",
file, linenum);
+ err_code |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+ ns->fallback_tcp = 1;
+
+out:
+ ha_free(&address);
+ ha_free(&tcp_addr);
+ return err_code;
+}
+
static int parse_resolve_conf(char **errmsg, char **warnmsg)
{
struct dns_nameserver *newnameserver = NULL;
@@ -3585,6 +3694,14 @@ static int parse_resolve_conf(char **errmsg, char
**warnmsg)
goto resolv_out;
}
+ err_code |= resolv_add_tcp_fallback(newnameserver, "/etc/resolv.conf",
+ resolv_linenum, address, sk,
errmsg);
+ if (err_code & (ERR_FATAL|ERR_ABORT)) {
+ dns_nameserver_deinit(newnameserver);
+ free(newnameserver);
+ goto resolv_out;
+ }
+
newnameserver->conf.file = strdup("/etc/resolv.conf");
if (newnameserver->conf.file == NULL) {
if (errmsg)
@@ -3751,7 +3868,9 @@ int cfg_parse_resolvers(const char *file, int linenum,
char **args, int kwm)
else if (strcmp(args[0], "nameserver") == 0) { /* nameserver definition
*/
struct dns_nameserver *newnameserver = NULL;
struct sockaddr_storage *sk;
+ char *expanded_addr = NULL;
int port1, port2;
+ int fallback_tcp = 0;
struct protocol *proto;
if (!*args[2]) {
@@ -3779,15 +3898,31 @@ int cfg_parse_resolvers(const char *file, int linenum,
char **args, int kwm)
}
}
- sk = str2sa_range(args[2], NULL, &port1, &port2, NULL, &proto, NULL,
+ expanded_addr = env_expand(strdup(args[2]));
+ if (!expanded_addr) {
+ ha_alert("parsing [%s:%d] : out of memory.\n", file,
linenum);
+ err_code |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+
Can you add a comment to explain the expanded_addr variable is used to be able
to match the transport prefix later. It is not obvious and we may be tempted to
remove it to directly use args[2].
+ sk = str2sa_range(expanded_addr, NULL, &port1, &port2, NULL,
&proto, NULL,
&errmsg, NULL, NULL, NULL,
PA_O_RESOLVE | PA_O_PORT_OK | PA_O_PORT_MAND
| PA_O_DGRAM | PA_O_STREAM | PA_O_DEFAULT_DGRAM);
if (!sk) {
ha_alert("parsing [%s:%d] : '%s %s' : %s\n", file,
linenum, args[0], args[1], errmsg);
err_code |= ERR_ALERT | ERR_FATAL;
+ ha_free(&expanded_addr);
goto out;
}
+ fallback_tcp = (proto && proto->xprt_type == PROTO_TYPE_DGRAM &&
+ (sk->ss_family == AF_INET || sk->ss_family == AF_INET6)
&&
+ strncmp(expanded_addr, "dgram+", 6) != 0 &&
+ strncmp(expanded_addr, "udp@", 4) != 0 &&
+ strncmp(expanded_addr, "udp4@", 5) != 0 &&
+ strncmp(expanded_addr, "udp6@", 5) != 0);
+ ha_free(&expanded_addr);
+
It is a bit annoying to add this kind of matching, especially because it must
remain inlined with str2sa_range. It is probably better to add an info in the
"net_addr_type" structure to know a prefix was explicitly set. Especially
because this reveals issues. "dgram+tcp@", "quic@" or "stream+udp" prefixes are
accepted for instance. I'm going to discuss it with Willy.
if ((newnameserver = calloc(1, sizeof(*newnameserver))) ==
NULL) {
ha_alert("parsing [%s:%d] : out of memory.\n", file,
linenum);
err_code |= ERR_ALERT | ERR_ABORT;
@@ -3810,11 +3945,26 @@ int cfg_parse_resolvers(const char *file, int linenum,
char **args, int kwm)
goto out;
}
}
- else if (dns_dgram_init(newnameserver, sk) < 0) {
- ha_alert("parsing [%s:%d] : out of memory.\n", file,
linenum);
- err_code |= ERR_ALERT | ERR_ABORT;
- free(newnameserver);
- goto out;
+ else {
+ if (dns_dgram_init(newnameserver, sk) < 0) {
+ ha_alert("parsing [%s:%d] : out of memory.\n",
file, linenum);
+ err_code |= ERR_ALERT | ERR_ABORT;
+ free(newnameserver);
+ goto out;
+ }
+
+ if (fallback_tcp) {
+ err_code |=
resolv_add_tcp_fallback(newnameserver, file, linenum,
+ args[1], sk,
&errmsg);
+ if (err_code & (ERR_FATAL|ERR_ABORT)) {
+ if (errmsg)
+ ha_alert("%s\n", errmsg);
+ err_code |= ERR_ABORT;
+ dns_nameserver_deinit(newnameserver);
+ free(newnameserver);
+ goto out;
+ }
+ }
}
if ((newnameserver->conf.file = strdup(file)) == NULL) {
--
Christopher Faulet
