From da0c630e141e3c3fab633a1c7a0686295e2c9411 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 24 Aug 2015 23:15:51 +0200 Subject: [PATCH 1/4] resolved: replace transaction list by hashmap Right now we keep track of ongoing transactions in a linked listed for each scope. Replace this by a hashmap that is indexed by the RR key. Given that all ongoing transactions will be placed in pretty much the same scopes usually this should optimize behaviour. We used to require a list here, since we wanted to do "superset" query checks, but this became obsolete since transactions are now single-key instead of multi-key. --- src/resolve/resolved-dns-scope.c | 33 ++++++++++++-------------- src/resolve/resolved-dns-scope.h | 2 +- src/resolve/resolved-dns-transaction.c | 18 ++++++++++---- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 57a2c7d6c15..336269ca8a6 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -78,8 +78,7 @@ DnsScope* dns_scope_free(DnsScope *s) { dns_scope_llmnr_membership(s, false); - while ((t = s->transactions)) { - + while ((t = hashmap_steal_first(s->transactions))) { /* Abort the transaction, but make sure it is not * freed while we still look at it */ @@ -90,6 +89,8 @@ DnsScope* dns_scope_free(DnsScope *s) { dns_transaction_free(t); } + hashmap_free(s->transactions); + while ((rr = ordered_hashmap_steal_first(s->conflict_queue))) dns_resource_record_unref(rr); @@ -623,24 +624,20 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key, assert(scope); assert(key); - /* Try to find an ongoing transaction that is a equal or a - * superset of the specified question */ + /* Try to find an ongoing transaction that is a equal to the + * specified question */ + t = hashmap_get(scope->transactions, key); + if (!t) + return NULL; - LIST_FOREACH(transactions_by_scope, t, scope->transactions) { + /* Refuse reusing transactions that completed based on cached + * 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) + return NULL; - /* Refuse reusing transactions that completed based on - * cached 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) - continue; - - if (dns_resource_key_equal(t->key, key) > 0) - return t; - } - - return NULL; + return t; } static int dns_scope_make_conflict_packet( diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h index 47f285db47f..88fb8224e24 100644 --- a/src/resolve/resolved-dns-scope.h +++ b/src/resolve/resolved-dns-scope.h @@ -60,7 +60,7 @@ struct DnsScope { usec_t resend_timeout; usec_t max_rtt; - LIST_HEAD(DnsTransaction, transactions); + Hashmap *transactions; LIST_FIELDS(DnsScope, scopes); }; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 81156dfa45a..608decd3ac8 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -35,7 +35,6 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { sd_event_source_unref(t->timeout_event_source); - dns_resource_key_unref(t->key); dns_packet_unref(t->sent); dns_packet_unref(t->received); dns_answer_unref(t->cached); @@ -47,12 +46,14 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { dns_stream_free(t->stream); if (t->scope) { - LIST_REMOVE(transactions_by_scope, t->scope->transactions, t); + hashmap_remove(t->scope->transactions, t->key); if (t->id != 0) hashmap_remove(t->scope->manager->dns_transactions, UINT_TO_PTR(t->id)); } + dns_resource_key_unref(t->key); + while ((q = set_steal_first(t->queries))) set_remove(q->transactions, t); set_free(t->queries); @@ -89,14 +90,18 @@ 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); + if (r < 0) + return r; + t = new0(DnsTransaction, 1); if (!t) return -ENOMEM; t->dns_fd = -1; - t->key = dns_resource_key_ref(key); + /* Find a fresh, unused transaction id */ do random_bytes(&t->id, sizeof(t->id)); while (t->id == 0 || @@ -108,7 +113,12 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) return r; } - LIST_PREPEND(transactions_by_scope, s->transactions, t); + r = hashmap_put(s->transactions, t->key, t); + if (r < 0) { + hashmap_remove(s->manager->dns_transactions, UINT_TO_PTR(t->id)); + return r; + } + t->scope = s; if (ret) From d634711b261dc72bb48765e8574ba43d455e82ec Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 24 Aug 2015 23:44:33 +0200 Subject: [PATCH 2/4] resolved: remove duplicate handling of "no servers" query result So far we handled immediate "no server" query results differently from "no server" results we ran into during operation: the former would cause the dns_query_go() call to fail with ESRCH, the later would result in the query completion callback to be called. Remove the duplicate codepaths, by always going through the completion callback. This allows us to remove quite a number of lines for handling the ESRCH. This commit should not alter behaviour at all. --- src/resolve/resolved-bus.c | 16 ---------------- src/resolve/resolved-dns-query.c | 8 ++------ 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 1f23834ce3c..12c17003e9f 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -226,10 +226,6 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) { * query, this time with the cname */ if (added <= 0) { r = dns_query_go(q); - if (r == -ESRCH) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_NAME_SERVERS, "No appropriate name servers or networks for name found"); - goto finish; - } if (r < 0) { r = sd_bus_reply_method_errno(q->request, -r, NULL); goto finish; @@ -346,10 +342,6 @@ static int bus_method_resolve_hostname(sd_bus_message *message, void *userdata, r = dns_query_go(q); if (r < 0) { dns_query_free(q); - - if (r == -ESRCH) - sd_bus_error_setf(error, BUS_ERROR_NO_NAME_SERVERS, "No appropriate name servers or networks for name found"); - return r; } @@ -494,10 +486,6 @@ static int bus_method_resolve_address(sd_bus_message *message, void *userdata, s r = dns_query_go(q); if (r < 0) { dns_query_free(q); - - if (r == -ESRCH) - sd_bus_error_setf(error, BUS_ERROR_NO_NAME_SERVERS, "No appropriate name servers or networks for name found"); - return r; } @@ -647,10 +635,6 @@ static int bus_method_resolve_record(sd_bus_message *message, void *userdata, sd r = dns_query_go(q); if (r < 0) { dns_query_free(q); - - if (r == -ESRCH) - sd_bus_error_setf(error, BUS_ERROR_NO_NAME_SERVERS, "No appropriate name servers or networks for name found"); - return r; } diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index d9f5b342b2e..c0b4c8ba818 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -651,12 +651,8 @@ int dns_query_go(DnsQuery *q) { DnsTransactionState state = DNS_TRANSACTION_NO_SERVERS; dns_query_synthesize_reply(q, &state); - if (state != DNS_TRANSACTION_NO_SERVERS) { - dns_query_complete(q, state); - return 1; - } - - return -ESRCH; + dns_query_complete(q, state); + return 1; } r = dns_query_add_transaction_split(q, first); From 3fa4999b5d29e6ee368ed2fd5d65928cfae9435e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 24 Aug 2015 23:46:24 +0200 Subject: [PATCH 3/4] resolve-host: support parsing numeric interface names If the user specifies an interface by its ifindex we should handle this nicely. Hence let's try to parse the ifindex as a number before we try to resolve it as an interface name. --- src/resolve-host/resolve-host.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/resolve-host/resolve-host.c b/src/resolve-host/resolve-host.c index 4a7d1e31733..9847effb533 100644 --- a/src/resolve-host/resolve-host.c +++ b/src/resolve-host/resolve-host.c @@ -490,7 +490,7 @@ static int parse_argv(int argc, char *argv[]) { { "version", no_argument, NULL, ARG_VERSION }, { "type", required_argument, NULL, 't' }, { "class", required_argument, NULL, 'c' }, - { "legend", optional_argument, NULL, ARG_LEGEND }, + { "legend", optional_argument, NULL, ARG_LEGEND }, { "protocol", required_argument, NULL, 'p' }, {} }; @@ -520,11 +520,21 @@ static int parse_argv(int argc, char *argv[]) { arg_family = AF_INET6; break; - case 'i': - arg_ifindex = if_nametoindex(optarg); - if (arg_ifindex <= 0) - return log_error_errno(errno, "Unknown interfaces %s: %m", optarg); + case 'i': { + int ifi; + + if (safe_atoi(optarg, &ifi) >= 0 && ifi > 0) + arg_ifindex = ifi; + else { + ifi = if_nametoindex(optarg); + if (ifi <= 0) + return log_error_errno(errno, "Unknown interface %s: %m", optarg); + + arg_ifindex = ifi; + } + break; + } case 't': if (streq(optarg, "help")) { From 9318cdd37467c3275fe72328a7a45986f57e6a7d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 24 Aug 2015 23:47:28 +0200 Subject: [PATCH 4/4] resolved: change error code when trying to resolve direct LLMNR PTR RRs If we try to resoolve an LLMNR PTR RR we shall connect via TCP directly to the specified IP address. We already refuse to do this if the address to resolve is of a different address family as the transaction's scope. The error returned was EAFNOSUPPORT. Let's change this to ESRCH which is how we indicate "not server available" when connecting for LLMNR or DNS, since that's what this really is: we have no server we could connect to in this address family. This allows us to ensure that no server errors are always handled the same way. --- src/resolve/resolved-dns-transaction.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 608decd3ac8..bbbc6377a81 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -284,7 +284,7 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { if (r == 0) return -EINVAL; if (family != t->scope->family) - return -EAFNOSUPPORT; + return -ESRCH; fd = dns_scope_tcp_socket(t->scope, family, &address, LLMNR_PORT, NULL); }