Skip to content

Commit a01cc5a

Browse files
mhs-emlogicbluca
authored andcommitted
resolved: if one transaction completes, expect other transactions within candidate to succeed quickly
Fixes systemd#22575, as suggested by poettering in systemd#35514. Intended as a workaround for some buggy routers, which refuse to send empty replies. If systemd-resolved starts two DnsTransactions, one for A and one for AAAA, and the domain in question has no AAAA entry, then the server will send a reply for A and no reply for AAAA. Correct behavior for the server would be to send an empty reply for AAAA. systemd-resolved would previously keep retrying the AAAA transaction, and eventually timeout the whole query, returning an error to the caller. Now, if the server replies to one query and not another, we cut short the timeout and return the partial result. Returning the partial result allows the rest of the system to keep working. It matches how e.g. glibc libnss_dns behaves. (cherry picked from commit 0da73fa) (cherry picked from commit 1748265)
1 parent 3fc24a0 commit a01cc5a

File tree

6 files changed

+88
-30
lines changed

6 files changed

+88
-30
lines changed

src/resolve/resolved-dns-query.c

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "resolved-dns-query.h"
1111
#include "resolved-dns-synthesize.h"
1212
#include "resolved-etc-hosts.h"
13+
#include "resolved-timeouts.h"
1314
#include "string-util.h"
1415

1516
#define QUERIES_MAX 2048
@@ -48,6 +49,8 @@ static void dns_query_candidate_stop(DnsQueryCandidate *c) {
4849

4950
assert(c);
5051

52+
(void) event_source_disable(c->timeout_event_source);
53+
5154
/* Detach all the DnsTransactions attached to this query */
5255

5356
while ((t = set_steal_first(c->transactions))) {
@@ -62,6 +65,8 @@ static void dns_query_candidate_abandon(DnsQueryCandidate *c) {
6265

6366
assert(c);
6467

68+
(void) event_source_disable(c->timeout_event_source);
69+
6570
/* Abandon all the DnsTransactions attached to this query */
6671

6772
while ((t = set_steal_first(c->transactions))) {
@@ -94,6 +99,8 @@ static DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c) {
9499
if (!c)
95100
return NULL;
96101

102+
c->timeout_event_source = sd_event_source_disable_unref(c->timeout_event_source);
103+
97104
dns_query_candidate_stop(c);
98105
dns_query_candidate_unlink(c);
99106

@@ -312,6 +319,30 @@ static int dns_query_candidate_setup_transactions(DnsQueryCandidate *c) {
312319
return r;
313320
}
314321

322+
static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c);
323+
324+
static int on_candidate_timeout(sd_event_source *s, usec_t usec, void *userdata) {
325+
DnsQueryCandidate *c = userdata;
326+
327+
assert(s);
328+
assert(c);
329+
330+
log_debug("Accepting incomplete query candidate after expedited timeout on partial success.");
331+
dns_query_accept(c->query, c);
332+
333+
return 0;
334+
}
335+
336+
static bool dns_query_candidate_has_partially_succeeded(DnsQueryCandidate *c) {
337+
DnsTransaction *t;
338+
339+
SET_FOREACH(t, c->transactions)
340+
if (t->state == DNS_TRANSACTION_SUCCESS)
341+
return true;
342+
343+
return false;
344+
}
345+
315346
void dns_query_candidate_notify(DnsQueryCandidate *c) {
316347
DnsTransactionState state;
317348
int r;
@@ -323,11 +354,24 @@ void dns_query_candidate_notify(DnsQueryCandidate *c) {
323354

324355
state = dns_query_candidate_state(c);
325356

326-
if (DNS_TRANSACTION_IS_LIVE(state))
357+
if (DNS_TRANSACTION_IS_LIVE(state)) {
358+
if (dns_query_candidate_has_partially_succeeded(c))
359+
(void) event_reset_time_relative(
360+
c->query->manager->event,
361+
&c->timeout_event_source,
362+
CLOCK_BOOTTIME,
363+
CANDIDATE_EXPEDITED_TIMEOUT_USEC, /* accuracy_usec= */ 0,
364+
on_candidate_timeout, c,
365+
/* priority= */ 0, "candidate-timeout",
366+
/* force_reset= */ false);
367+
327368
return;
369+
}
328370

329371
if (state != DNS_TRANSACTION_SUCCESS && c->search_domain) {
330372

373+
(void) event_source_disable(c->timeout_event_source);
374+
331375
r = dns_query_candidate_next_search_domain(c);
332376
if (r < 0)
333377
goto fail;

src/resolve/resolved-dns-query.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ struct DnsQueryCandidate {
2525
DnsSearchDomain *search_domain;
2626

2727
Set *transactions;
28+
sd_event_source *timeout_event_source;
2829

2930
LIST_FIELDS(DnsQueryCandidate, candidates_by_query);
3031
LIST_FIELDS(DnsQueryCandidate, candidates_by_scope);

src/resolve/resolved-dns-scope.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "resolved-dns-zone.h"
1717
#include "resolved-llmnr.h"
1818
#include "resolved-mdns.h"
19+
#include "resolved-timeouts.h"
1920
#include "socket-util.h"
2021
#include "strv.h"
2122

src/resolve/resolved-dns-transaction.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,10 @@
1414
#include "resolved-dns-transaction.h"
1515
#include "resolved-dnstls.h"
1616
#include "resolved-llmnr.h"
17+
#include "resolved-timeouts.h"
1718
#include "string-table.h"
1819

1920
#define TRANSACTIONS_MAX 4096
20-
#define TRANSACTION_TCP_TIMEOUT_USEC (10U*USEC_PER_SEC)
21-
22-
/* After how much time to repeat classic DNS requests */
23-
#define DNS_TIMEOUT_USEC (SD_RESOLVED_QUERY_TIMEOUT_USEC / DNS_TRANSACTION_ATTEMPTS_MAX)
2421

2522
static void dns_transaction_reset_answer(DnsTransaction *t) {
2623
assert(t);
@@ -1632,13 +1629,10 @@ static usec_t transaction_get_resend_timeout(DnsTransaction *t) {
16321629

16331630
case DNS_PROTOCOL_DNS:
16341631

1635-
/* When we do TCP, grant a much longer timeout, as in this case there's no need for us to quickly
1636-
* resend, as the kernel does that anyway for us, and we really don't want to interrupt it in that
1637-
* needlessly. */
16381632
if (t->stream)
16391633
return TRANSACTION_TCP_TIMEOUT_USEC;
16401634

1641-
return DNS_TIMEOUT_USEC;
1635+
return TRANSACTION_UDP_TIMEOUT_USEC;
16421636

16431637
case DNS_PROTOCOL_MDNS:
16441638
if (t->probing)

src/resolve/resolved-dns-transaction.h

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -203,24 +203,3 @@ DnsTransactionState dns_transaction_state_from_string(const char *s) _pure_;
203203

204204
const char* dns_transaction_source_to_string(DnsTransactionSource p) _const_;
205205
DnsTransactionSource dns_transaction_source_from_string(const char *s) _pure_;
206-
207-
/* LLMNR Jitter interval, see RFC 4795 Section 7 */
208-
#define LLMNR_JITTER_INTERVAL_USEC (100 * USEC_PER_MSEC)
209-
210-
/* mDNS probing interval, see RFC 6762 Section 8.1 */
211-
#define MDNS_PROBING_INTERVAL_USEC (250 * USEC_PER_MSEC)
212-
213-
/* Maximum attempts to send DNS requests, across all DNS servers */
214-
#define DNS_TRANSACTION_ATTEMPTS_MAX 24
215-
216-
/* Maximum attempts to send LLMNR requests, see RFC 4795 Section 2.7 */
217-
#define LLMNR_TRANSACTION_ATTEMPTS_MAX 3
218-
219-
/* Maximum attempts to send MDNS requests, see RFC 6762 Section 8.1 */
220-
#define MDNS_TRANSACTION_ATTEMPTS_MAX 3
221-
222-
#define TRANSACTION_ATTEMPTS_MAX(p) ((p) == DNS_PROTOCOL_LLMNR ? \
223-
LLMNR_TRANSACTION_ATTEMPTS_MAX : \
224-
(p) == DNS_PROTOCOL_MDNS ? \
225-
MDNS_TRANSACTION_ATTEMPTS_MAX : \
226-
DNS_TRANSACTION_ATTEMPTS_MAX)

src/resolve/resolved-timeouts.h

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/* SPDX-License-Identifier: LGPL-2.1-or-later */
2+
#pragma once
3+
4+
#include "time-util.h"
5+
#include "resolved-def.h"
6+
7+
/* LLMNR Jitter interval, see RFC 4795 Section 7 */
8+
#define LLMNR_JITTER_INTERVAL_USEC (100 * USEC_PER_MSEC)
9+
10+
/* mDNS probing interval, see RFC 6762 Section 8.1 */
11+
#define MDNS_PROBING_INTERVAL_USEC (250 * USEC_PER_MSEC)
12+
13+
/* Maximum attempts to send DNS requests, across all DNS servers */
14+
#define DNS_TRANSACTION_ATTEMPTS_MAX 24
15+
16+
/* Maximum attempts to send LLMNR requests, see RFC 4795 Section 2.7 */
17+
#define LLMNR_TRANSACTION_ATTEMPTS_MAX 3
18+
19+
/* Maximum attempts to send MDNS requests, see RFC 6762 Section 8.1 */
20+
#define MDNS_TRANSACTION_ATTEMPTS_MAX 3
21+
22+
#define TRANSACTION_ATTEMPTS_MAX(p) (\
23+
(p) == DNS_PROTOCOL_LLMNR ? \
24+
LLMNR_TRANSACTION_ATTEMPTS_MAX : \
25+
(p) == DNS_PROTOCOL_MDNS ? \
26+
MDNS_TRANSACTION_ATTEMPTS_MAX : \
27+
DNS_TRANSACTION_ATTEMPTS_MAX)
28+
29+
/* After how much time to repeat classic DNS requests */
30+
#define TRANSACTION_UDP_TIMEOUT_USEC (SD_RESOLVED_QUERY_TIMEOUT_USEC / DNS_TRANSACTION_ATTEMPTS_MAX)
31+
32+
/* When we do TCP, grant a much longer timeout, as in this case there's no need for us to quickly
33+
* resend, as the kernel does that anyway for us, and we really don't want to interrupt it in that
34+
* needlessly. */
35+
#define TRANSACTION_TCP_TIMEOUT_USEC (10 * USEC_PER_SEC)
36+
37+
/* Should be longer than transaction timeout for a single UDP transaction, so we get at least
38+
* one transaction retry before timeouting the whole candidate */
39+
#define CANDIDATE_EXPEDITED_TIMEOUT_USEC (TRANSACTION_UDP_TIMEOUT_USEC + 1 * USEC_PER_SEC)

0 commit comments

Comments
 (0)