Merge pull request #2041 from poettering/resolved-various-2

various smaller fixes, plus one that makes the build succeed again

8 files changed, 92 insertions, 66 deletions

diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c
index c5b4857017..62bb08a2e8 100644
--- a/src/resolve/resolved-bus.c
+++ b/src/resolve/resolved-bus.c
@@ -300,7 +300,7 @@ static void bus_method_resolve_address_complete(DnsQuery *q) {
 
         r = dns_query_process_cname(q);
         if (r == -ELOOP) {
-                r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_question_name(q->question));
+                r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_question_first_name(q->question));
                 goto finish;
         }
         if (r < 0)
diff --git a/src/resolve/resolved-def.h b/src/resolve/resolved-def.h
index 33f8ddb1f2..be29f51663 100644
--- a/src/resolve/resolved-def.h
+++ b/src/resolve/resolved-def.h
@@ -21,13 +21,13 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
-#define SD_RESOLVED_DNS           ((uint64_t) 1)
-#define SD_RESOLVED_LLMNR_IPV4    ((uint64_t) 2)
-#define SD_RESOLVED_LLMNR_IPV6    ((uint64_t) 4)
-#define SD_RESOLVED_NO_CNAME      ((uint64_t) 8)
-#define SD_RESOLVED_NO_TXT        ((uint64_t) 16)
-#define SD_RESOLVED_NO_ADDRESS    ((uint64_t) 32)
-#define SD_RESOLVED_NO_SEARCH     ((uint64_t) 64)
+#define SD_RESOLVED_DNS           (UINT64_C(1) << 0)
+#define SD_RESOLVED_LLMNR_IPV4    (UINT64_C(1) << 1)
+#define SD_RESOLVED_LLMNR_IPV6    (UINT64_C(1) << 2)
+#define SD_RESOLVED_NO_CNAME      (UINT64_C(1) << 5)
+#define SD_RESOLVED_NO_TXT        (UINT64_C(1) << 6)
+#define SD_RESOLVED_NO_ADDRESS    (UINT64_C(1) << 7)
+#define SD_RESOLVED_NO_SEARCH     (UINT64_C(1) << 8)
 
 #define SD_RESOLVED_LLMNR         (SD_RESOLVED_LLMNR_IPV4|SD_RESOLVED_LLMNR_IPV6)
 #define SD_RESOLVED_PROTOCOLS_ALL (SD_RESOLVED_LLMNR|SD_RESOLVED_DNS)
diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c
index b84f5bf0f3..a96cf439ad 100644
--- a/src/resolve/resolved-dns-query.c
+++ b/src/resolve/resolved-dns-query.c
@@ -94,31 +94,19 @@ static int dns_query_candidate_next_search_domain(DnsQueryCandidate *c) {
         if (c->search_domain && c->search_domain->linked) {
                 next = c->search_domain->domains_next;
 
-                if (!next) {
-                        /* We hit the last entry. Let's see if this
-                         * was the per-link search domain list. If so,
-                         * let's continue with the global one. */
-
-                        if (c->search_domain->type == DNS_SEARCH_DOMAIN_LINK)
-                                next = c->query->manager->search_domains;
-
-                        if (!next) /* Still no item? Then we really hit the end of the list. */
-                                return 0;
-                }
+                if (!next) /* We hit the end of the list */
+                        return 0;
 
         } else {
-                /* If we have, start with the per-link domains */
                 next = dns_scope_get_search_domains(c->scope);
 
-                if (!next) /* Fall back to the global search domains */
-                        next = c->scope->manager->search_domains;
-
-                if (!next) /* OK, there's really nothing. */
+                if (!next) /* OK, there's nothing. */
                         return 0;
         }
 
         dns_search_domain_unref(c->search_domain);
         c->search_domain = dns_search_domain_ref(next);
+
         return 1;
 }
 
@@ -998,17 +986,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;
-
-                        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;
-                        }
+                        DnsAnswer *merged;
 
-                        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 +996,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 +1015,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-scope.c b/src/resolve/resolved-dns-scope.c
index 20db1fbd81..fc4ae57ce0 100644
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -70,11 +70,11 @@ int dns_scope_new(Manager *m, DnsScope **ret, Link *l, DnsProtocol protocol, int
 }
 
 static void dns_scope_abort_transactions(DnsScope *s) {
-        DnsTransaction *t;
-
         assert(s);
 
-        while ((t = hashmap_first(s->transactions))) {
+        while (s->transactions) {
+                DnsTransaction *t = s->transactions;
+
                 /* Abort the transaction, but make sure it is not
                  * freed while we still look at it */
 
@@ -100,7 +100,7 @@ DnsScope* dns_scope_free(DnsScope *s) {
         while (s->query_candidates)
                 dns_query_candidate_free(s->query_candidates);
 
-        hashmap_free(s->transactions);
+        hashmap_free(s->transactions_by_key);
 
         while ((rr = ordered_hashmap_steal_first(s->conflict_queue)))
                 dns_resource_record_unref(rr);
@@ -653,7 +653,7 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key,
 
         /* Try to find an ongoing transaction that is a equal to the
          * specified question */
-        t = hashmap_get(scope->transactions, key);
+        t = hashmap_get(scope->transactions_by_key, key);
         if (!t)
                 return NULL;
 
@@ -661,7 +661,7 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key,
          * data instead of a real packet, if that's requested. */
         if (!cache_ok &&
             IN_SET(t->state, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_FAILURE) &&
-            !t->received)
+            t->answer_source != DNS_TRANSACTION_NETWORK)
                 return NULL;
 
         return t;
diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h
index 32e6961757..7876410b7d 100644
--- a/src/resolve/resolved-dns-scope.h
+++ b/src/resolve/resolved-dns-scope.h
@@ -58,9 +58,19 @@ struct DnsScope {
         usec_t resend_timeout;
         usec_t max_rtt;
 
-        Hashmap *transactions;
         LIST_HEAD(DnsQueryCandidate, query_candidates);
 
+        /* Note that we keep track of ongoing transactions in two
+         * ways: once in a hashmap, indexed by the rr key, and once in
+         * a linked list. We use the hashmap to quickly find
+         * transactions we can reuse for a key. But note that there
+         * might be multiple transactions for the same key (because
+         * the zone probing can't reuse a transaction answered from
+         * the zone or the cache), and the hashmap only tracks the
+         * most recent entry. */
+        Hashmap *transactions_by_key;
+        LIST_HEAD(DnsTransaction, transactions);
+
         LIST_FIELDS(DnsScope, scopes);
 };
 
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 2fc84aec6f..c65c9c9f49 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);
@@ -48,7 +49,8 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) {
         dns_stream_free(t->stream);
 
         if (t->scope) {
-                hashmap_remove(t->scope->transactions, t->key);
+                hashmap_remove_value(t->scope->transactions_by_key, t->key, t);
+                LIST_REMOVE(transactions_by_scope, t->scope->transactions, t);
 
                 if (t->id != 0)
                         hashmap_remove(t->scope->manager->dns_transactions, UINT_TO_PTR(t->id));
@@ -93,7 +95,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
         if (r < 0)
                 return r;
 
-        r = hashmap_ensure_allocated(&s->transactions, &dns_resource_key_hash_ops);
+        r = hashmap_ensure_allocated(&s->transactions_by_key, &dns_resource_key_hash_ops);
         if (r < 0)
                 return r;
 
@@ -102,6 +104,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
                 return -ENOMEM;
 
         t->dns_udp_fd = -1;
+        t->answer_source = _DNS_TRANSACTION_SOURCE_INVALID;
         t->key = dns_resource_key_ref(key);
 
         /* Find a fresh, unused transaction id */
@@ -116,12 +119,13 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
                 return r;
         }
 
-        r = hashmap_put(s->transactions, t->key, t);
+        r = hashmap_replace(s->transactions_by_key, t->key, t);
         if (r < 0) {
                 hashmap_remove(s->manager->dns_transactions, UINT_TO_PTR(t->id));
                 return r;
         }
 
+        LIST_PREPEND(transactions_by_scope, s->transactions, t);
         t->scope = s;
 
         if (ret)
@@ -137,6 +141,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) {
@@ -193,11 +200,12 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) {
          * should hence not attempt to access the query or transaction
          * after calling this function. */
 
-        log_debug("Transaction on scope %s on %s/%s now complete with <%s>",
+        log_debug("Transaction on scope %s on %s/%s now complete with <%s> from %s",
                   dns_protocol_to_string(t->scope->protocol),
                   t->scope->link ? t->scope->link->name : "*",
                   t->scope->family == AF_UNSPEC ? "*" : af_to_name(t->scope->family),
-                  dns_transaction_state_to_string(state));
+                  dns_transaction_state_to_string(state),
+                  t->answer_source < 0 ? "none" : dns_transaction_source_to_string(t->answer_source));
 
         t->state = state;
 
@@ -315,6 +323,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;
 
@@ -386,6 +396,8 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                 t->received = dns_packet_ref(p);
         }
 
+        t->answer_source = DNS_TRANSACTION_NETWORK;
+
         if (p->ipproto == IPPROTO_TCP) {
                 if (DNS_PACKET_TC(p)) {
                         /* Truncated via TCP? Somebody must be fucking with us */
@@ -454,6 +466,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 +641,20 @@ 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;
+        t->answer_source = _DNS_TRANSACTION_SOURCE_INVALID;
 
         /* 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;
+                        t->answer_source = DNS_TRANSACTION_ZONE;
                         dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS);
                         return 0;
                 }
@@ -653,11 +672,12 @@ 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)
+                        t->answer_source = DNS_TRANSACTION_CACHE;
+                        if (t->answer_rcode == DNS_RCODE_SUCCESS)
                                 dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS);
                         else
                                 dns_transaction_complete(t, DNS_TRANSACTION_FAILURE);
@@ -760,3 +780,10 @@ static const char* const dns_transaction_state_table[_DNS_TRANSACTION_STATE_MAX]
         [DNS_TRANSACTION_ABORTED] = "aborted",
 };
 DEFINE_STRING_TABLE_LOOKUP(dns_transaction_state, DnsTransactionState);
+
+static const char* const dns_transaction_source_table[_DNS_TRANSACTION_SOURCE_MAX] = {
+        [DNS_TRANSACTION_NETWORK] = "network",
+        [DNS_TRANSACTION_CACHE] = "cache",
+        [DNS_TRANSACTION_ZONE] = "zone",
+};
+DEFINE_STRING_TABLE_LOOKUP(dns_transaction_source, DnsTransactionSource);
diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h
index a2aa73a524..ee80dcf5a9 100644
--- a/src/resolve/resolved-dns-transaction.h
+++ b/src/resolve/resolved-dns-transaction.h
@@ -23,6 +23,7 @@
 
 typedef struct DnsTransaction DnsTransaction;
 typedef enum DnsTransactionState DnsTransactionState;
+typedef enum DnsTransactionSource DnsTransactionSource;
 
 enum DnsTransactionState {
         DNS_TRANSACTION_NULL,
@@ -39,6 +40,14 @@ enum DnsTransactionState {
         _DNS_TRANSACTION_STATE_INVALID = -1
 };
 
+enum DnsTransactionSource {
+        DNS_TRANSACTION_NETWORK,
+        DNS_TRANSACTION_CACHE,
+        DNS_TRANSACTION_ZONE,
+        _DNS_TRANSACTION_SOURCE_MAX,
+        _DNS_TRANSACTION_SOURCE_INVALID = -1
+};
+
 #include "resolved-dns-answer.h"
 #include "resolved-dns-packet.h"
 #include "resolved-dns-question.h"
@@ -55,8 +64,10 @@ struct DnsTransaction {
         bool initial_jitter;
 
         DnsPacket *sent, *received;
-        DnsAnswer *cached;
-        int cached_rcode;
+
+        DnsAnswer *answer;
+        int answer_rcode;
+        DnsTransactionSource answer_source;
 
         usec_t start_usec;
         sd_event_source *timeout_event_source;
@@ -97,6 +108,9 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state);
 const char* dns_transaction_state_to_string(DnsTransactionState p) _const_;
 DnsTransactionState dns_transaction_state_from_string(const char *s) _pure_;
 
+const char* dns_transaction_source_to_string(DnsTransactionSource p) _const_;
+DnsTransactionSource dns_transaction_source_from_string(const char *s) _pure_;
+
 /* LLMNR Jitter interval, see RFC 4795 Section 7 */
 #define LLMNR_JITTER_INTERVAL_USEC (100 * USEC_PER_MSEC)
 
@@ -106,4 +120,4 @@ DnsTransactionState dns_transaction_state_from_string(const char *s) _pure_;
 /* Maximum attempts to send LLMNR requests, see RFC 4795 Section 2.7 */
 #define LLMNR_TRANSACTION_ATTEMPTS_MAX 3
 
-#define TRANSACTION_ATTEMPTS_MAX(p) (p == DNS_PROTOCOL_LLMNR ? LLMNR_TRANSACTION_ATTEMPTS_MAX : DNS_TRANSACTION_ATTEMPTS_MAX)
+#define TRANSACTION_ATTEMPTS_MAX(p) ((p) == DNS_PROTOCOL_LLMNR ? LLMNR_TRANSACTION_ATTEMPTS_MAX : DNS_TRANSACTION_ATTEMPTS_MAX)