From ae6a4bbf318e197813227e50c245a00de03784a2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Nov 2015 22:51:35 +0100 Subject: [PATCH] resolved: store just the DnsAnswer instead of a DnsPacket as answer in DnsTransaction objects Previously we'd only store the DnsPacket in the DnsTransaction, and the DnsQuery would then take the DnsPacket's DnsAnswer and return it. With this change we already pull the DnsAnswer out inside the transaction. We still store the DnsPacket in the transaction, if we have it, since we still need to determine from which peer a response originates, to implement caching properly. However, the DnsQuery logic doesn't care anymore for the packet, it now only looks at answers and rcodes from the successfuly candidate. This also has the benefit of unifying how we propagate incoming packets, data from the local zone or the local cache. --- src/resolve/resolved-dns-query.c | 23 +++++------------------ src/resolve/resolved-dns-query.h | 4 ++-- src/resolve/resolved-dns-transaction.c | 25 ++++++++++++++++++------- src/resolve/resolved-dns-transaction.h | 5 +++-- 4 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index b84f5bf0f3..fe99caff37 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -998,17 +998,9 @@ static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { case DNS_TRANSACTION_SUCCESS: { /* We found a successfuly reply, merge it into the answer */ - DnsAnswer *merged, *a; + DnsAnswer *merged; - if (t->received) { - q->answer_rcode = DNS_PACKET_RCODE(t->received); - a = t->received->answer; - } else { - q->answer_rcode = t->cached_rcode; - a = t->cached; - } - - merged = dns_answer_merge(q->answer, a); + merged = dns_answer_merge(q->answer, t->answer); if (!merged) { dns_query_complete(q, DNS_TRANSACTION_RESOURCES); return; @@ -1016,6 +1008,7 @@ static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { dns_answer_unref(q->answer); q->answer = merged; + q->answer_rcode = t->answer_rcode; state = DNS_TRANSACTION_SUCCESS; break; @@ -1034,14 +1027,8 @@ static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { if (state != DNS_TRANSACTION_SUCCESS) { dns_answer_unref(q->answer); - - if (t->received) { - q->answer = dns_answer_ref(t->received->answer); - q->answer_rcode = DNS_PACKET_RCODE(t->received); - } else { - q->answer = dns_answer_ref(t->cached); - q->answer_rcode = t->cached_rcode; - } + q->answer = dns_answer_ref(t->answer); + q->answer_rcode = t->answer_rcode; state = t->state; } diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index fb16747c55..a9d7904a8d 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -71,9 +71,9 @@ struct DnsQuery { /* Discovered data */ DnsAnswer *answer; - int answer_family; - DnsProtocol answer_protocol; int answer_rcode; + DnsProtocol answer_protocol; + int answer_family; DnsSearchDomain *answer_search_domain; /* Bus client information */ diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 2fc84aec6f..ef5097db39 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -39,7 +39,8 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { dns_packet_unref(t->sent); dns_packet_unref(t->received); - dns_answer_unref(t->cached); + + dns_answer_unref(t->answer); sd_event_source_unref(t->dns_udp_event_source); safe_close(t->dns_udp_fd); @@ -137,6 +138,9 @@ static void dns_transaction_stop(DnsTransaction *t) { t->timeout_event_source = sd_event_source_unref(t->timeout_event_source); t->stream = dns_stream_free(t->stream); + + /* Note that we do not drop the UDP socket here, as we want to + * reuse it to repeat the interaction. */ } static void dns_transaction_tentative(DnsTransaction *t, DnsPacket *p) { @@ -315,6 +319,8 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { dns_server_unref(t->server); t->server = dns_server_ref(server); t->received = dns_packet_unref(t->received); + t->answer = dns_answer_unref(t->answer); + t->answer_rcode = 0; t->stream->complete = on_stream_complete; t->stream->transaction = t; @@ -454,6 +460,11 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { return; } + /* Install the answer as answer to the transaction */ + dns_answer_unref(t->answer); + t->answer = dns_answer_ref(p->answer); + t->answer_rcode = DNS_PACKET_RCODE(p); + /* Only consider responses with equivalent query section to the request */ if (p->question->n_keys != 1 || dns_resource_key_equal(p->question->keys[0], t->key) <= 0) { dns_transaction_complete(t, DNS_TRANSACTION_INVALID_REPLY); @@ -624,18 +635,18 @@ int dns_transaction_go(DnsTransaction *t) { t->n_attempts++; t->start_usec = ts; t->received = dns_packet_unref(t->received); - t->cached = dns_answer_unref(t->cached); - t->cached_rcode = 0; + t->answer = dns_answer_unref(t->answer); + t->answer_rcode = 0; /* Check the zone, but obly if this transaction is not used * for probing or verifying a zone item. */ if (set_isempty(t->zone_items)) { - r = dns_zone_lookup(&t->scope->zone, t->key, &t->cached, NULL, NULL); + r = dns_zone_lookup(&t->scope->zone, t->key, &t->answer, NULL, NULL); if (r < 0) return r; if (r > 0) { - t->cached_rcode = DNS_RCODE_SUCCESS; + t->answer_rcode = DNS_RCODE_SUCCESS; dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS); return 0; } @@ -653,11 +664,11 @@ int dns_transaction_go(DnsTransaction *t) { /* Let's then prune all outdated entries */ dns_cache_prune(&t->scope->cache); - r = dns_cache_lookup(&t->scope->cache, t->key, &t->cached_rcode, &t->cached); + r = dns_cache_lookup(&t->scope->cache, t->key, &t->answer_rcode, &t->answer); if (r < 0) return r; if (r > 0) { - if (t->cached_rcode == DNS_RCODE_SUCCESS) + if (t->answer_rcode == DNS_RCODE_SUCCESS) dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS); else dns_transaction_complete(t, DNS_TRANSACTION_FAILURE); diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index a2aa73a524..a56bcdb6d6 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -55,8 +55,9 @@ struct DnsTransaction { bool initial_jitter; DnsPacket *sent, *received; - DnsAnswer *cached; - int cached_rcode; + + DnsAnswer *answer; + int answer_rcode; usec_t start_usec; sd_event_source *timeout_event_source;