resolved: only maintain one question RR key per transaction

Let's simplify things and only maintain a single RR key per transaction
object, instead of a full DnsQuestion. Unicast DNS and LLMNR don't
support multiple questions per packet anway, and Multicast DNS suggests
coalescing questions beyond a single dns query, across the whole system.
This commit is contained in:
Lennart Poettering 2015-08-21 22:55:01 +02:00
parent 9e08a6e0ce
commit f52e61da04
10 changed files with 80 additions and 162 deletions

View File

@ -487,73 +487,66 @@ fail:
return r;
}
int dns_cache_lookup(DnsCache *c, DnsQuestion *q, int *rcode, DnsAnswer **ret) {
int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **ret) {
_cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL;
unsigned i, n = 0;
unsigned n = 0;
int r;
bool nxdomain = false;
_cleanup_free_ char *key_str = NULL;
DnsCacheItem *j, *first;
assert(c);
assert(q);
assert(key);
assert(rcode);
assert(ret);
if (q->n_keys <= 0) {
if (key->type == DNS_TYPE_ANY ||
key->class == DNS_CLASS_ANY) {
/* If we have ANY lookups we simply refresh */
r = dns_resource_key_to_string(key, &key_str);
if (r < 0)
return r;
log_debug("Ignoring cache for ANY lookup: %s", key_str);
*ret = NULL;
*rcode = 0;
*rcode = DNS_RCODE_SUCCESS;
return 0;
}
for (i = 0; i < q->n_keys; i++) {
_cleanup_free_ char *key_str = NULL;
DnsCacheItem *j;
first = hashmap_get(c->by_key, key);
if (!first) {
/* If one question cannot be answered we need to refresh */
if (q->keys[i]->type == DNS_TYPE_ANY ||
q->keys[i]->class == DNS_CLASS_ANY) {
/* If we have ANY lookups we simply refresh */
r = dns_resource_key_to_string(key, &key_str);
if (r < 0)
return r;
r = dns_resource_key_to_string(q->keys[i], &key_str);
if (r < 0)
return r;
log_debug("Cache miss for %s", key_str);
log_debug("Ignoring cache for ANY lookup: %s", key_str);
*ret = NULL;
*rcode = 0;
return 0;
}
j = hashmap_get(c->by_key, q->keys[i]);
if (!j) {
/* If one question cannot be answered we need to refresh */
r = dns_resource_key_to_string(q->keys[i], &key_str);
if (r < 0)
return r;
log_debug("Cache miss for %s", key_str);
*ret = NULL;
*rcode = 0;
return 0;
} else {
r = dns_resource_key_to_string(j->key, &key_str);
if (r < 0)
return r;
log_debug("%s cache hit for %s",
j->type == DNS_CACHE_POSITIVE ? "Positive" :
(j->type == DNS_CACHE_NODATA ? "NODATA" : "NXDOMAIN"), key_str);
}
LIST_FOREACH(by_key, j, j) {
if (j->rr)
n++;
else if (j->type == DNS_CACHE_NXDOMAIN)
nxdomain = true;
}
*ret = NULL;
*rcode = DNS_RCODE_SUCCESS;
return 0;
}
LIST_FOREACH(by_key, j, first) {
if (j->rr)
n++;
else if (j->type == DNS_CACHE_NXDOMAIN)
nxdomain = true;
}
r = dns_resource_key_to_string(key, &key_str);
if (r < 0)
return r;
log_debug("%s cache hit for %s",
nxdomain ? "NXDOMAIN" :
n > 0 ? "Positive" : "NODATA",
key_str);
if (n <= 0) {
*ret = NULL;
*rcode = nxdomain ? DNS_RCODE_NXDOMAIN : DNS_RCODE_SUCCESS;
@ -564,17 +557,13 @@ int dns_cache_lookup(DnsCache *c, DnsQuestion *q, int *rcode, DnsAnswer **ret) {
if (!answer)
return -ENOMEM;
for (i = 0; i < q->n_keys; i++) {
DnsCacheItem *j;
LIST_FOREACH(by_key, j, first) {
if (!j->rr)
continue;
j = hashmap_get(c->by_key, q->keys[i]);
LIST_FOREACH(by_key, j, j) {
if (j->rr) {
r = dns_answer_add(answer, j->rr, 0);
if (r < 0)
return r;
}
}
r = dns_answer_add(answer, j->rr, 0);
if (r < 0)
return r;
}
*ret = answer;

View File

@ -40,6 +40,6 @@ void dns_cache_flush(DnsCache *c);
void dns_cache_prune(DnsCache *c);
int dns_cache_put(DnsCache *c, DnsQuestion *q, int rcode, DnsAnswer *answer, unsigned max_rrs, usec_t timestamp, int owner_family, const union in_addr_union *owner_address);
int dns_cache_lookup(DnsCache *c, DnsQuestion *q, int *rcode, DnsAnswer **answer);
int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **answer);
int dns_cache_check_conflicts(DnsCache *cache, DnsResourceRecord *rr, int owner_family, const union in_addr_union *owner_address);

View File

@ -138,7 +138,6 @@ static int on_query_timeout(sd_event_source *s, usec_t usec, void *userdata) {
}
static int dns_query_add_transaction(DnsQuery *q, DnsScope *s, DnsResourceKey *key) {
_cleanup_(dns_question_unrefp) DnsQuestion *question = NULL;
DnsTransaction *t;
int r;
@ -149,20 +148,9 @@ static int dns_query_add_transaction(DnsQuery *q, DnsScope *s, DnsResourceKey *k
if (r < 0)
return r;
if (key) {
question = dns_question_new(1);
if (!question)
return -ENOMEM;
r = dns_question_add(question, key);
if (r < 0)
return r;
} else
question = dns_question_ref(q->question);
t = dns_scope_find_transaction(s, question, true);
t = dns_scope_find_transaction(s, key, true);
if (!t) {
r = dns_transaction_new(&t, s, question);
r = dns_transaction_new(&t, s, key);
if (r < 0)
return r;
}

View File

@ -300,42 +300,3 @@ int dns_question_cname_redirect(DnsQuestion *q, const char *name, DnsQuestion **
return 1;
}
int dns_question_endswith(DnsQuestion *q, const char *suffix) {
unsigned i;
assert(suffix);
if (!q)
return 1;
for (i = 0; i < q->n_keys; i++) {
int k;
k = dns_name_endswith(DNS_RESOURCE_KEY_NAME(q->keys[i]), suffix);
if (k <= 0)
return k;
}
return 1;
}
int dns_question_extract_reverse_address(DnsQuestion *q, int *family, union in_addr_union *address) {
unsigned i;
assert(family);
assert(address);
if (!q)
return 0;
for (i = 0; i < q->n_keys; i++) {
int k;
k = dns_name_address(DNS_RESOURCE_KEY_NAME(q->keys[i]), family, address);
if (k != 0)
return k;
}
return 0;
}

View File

@ -48,7 +48,4 @@ int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b);
int dns_question_cname_redirect(DnsQuestion *q, const char *name, DnsQuestion **ret);
int dns_question_endswith(DnsQuestion *q, const char *suffix);
int dns_question_extract_reverse_address(DnsQuestion *q, int *family, union in_addr_union *address);
DEFINE_TRIVIAL_CLEANUP_FUNC(DnsQuestion*, dns_question_unref);

View File

@ -617,11 +617,11 @@ void dns_scope_process_query(DnsScope *s, DnsStream *stream, DnsPacket *p) {
}
}
DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsQuestion *question, bool cache_ok) {
DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key, bool cache_ok) {
DnsTransaction *t;
assert(scope);
assert(question);
assert(key);
/* Try to find an ongoing transaction that is a equal or a
* superset of the specified question */
@ -636,7 +636,7 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsQuestion *questio
!t->received)
continue;
if (dns_question_is_superset(t->question, question) > 0)
if (dns_resource_key_equal(t->key, key) > 0)
return t;
}

View File

@ -85,7 +85,7 @@ int dns_scope_llmnr_membership(DnsScope *s, bool b);
void dns_scope_process_query(DnsScope *s, DnsStream *stream, DnsPacket *p);
DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsQuestion *question, bool cache_ok);
DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key, bool cache_ok);
int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr);
void dns_scope_check_conflicts(DnsScope *scope, DnsPacket *p);

View File

@ -24,6 +24,7 @@
#include "resolved-llmnr.h"
#include "resolved-dns-transaction.h"
#include "random-util.h"
#include "dns-domain.h"
DnsTransaction* dns_transaction_free(DnsTransaction *t) {
DnsQuery *q;
@ -34,7 +35,7 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) {
sd_event_source_unref(t->timeout_event_source);
dns_question_unref(t->question);
dns_resource_key_unref(t->key);
dns_packet_unref(t->sent);
dns_packet_unref(t->received);
dns_answer_unref(t->cached);
@ -76,13 +77,13 @@ void dns_transaction_gc(DnsTransaction *t) {
dns_transaction_free(t);
}
int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsQuestion *q) {
int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) {
_cleanup_(dns_transaction_freep) DnsTransaction *t = NULL;
int r;
assert(ret);
assert(s);
assert(q);
assert(key);
r = hashmap_ensure_allocated(&s->manager->dns_transactions, NULL);
if (r < 0)
@ -94,7 +95,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsQuestion *q) {
t->dns_fd = -1;
t->question = dns_question_ref(q);
t->key = dns_resource_key_ref(key);
do
random_bytes(&t->id, sizeof(t->id));
@ -266,7 +267,8 @@ static int dns_transaction_open_tcp(DnsTransaction *t) {
/* Otherwise, try to talk to the owner of a
* the IP address, in case this is a reverse
* PTR lookup */
r = dns_question_extract_reverse_address(t->question, &family, &address);
r = dns_name_address(DNS_RESOURCE_KEY_NAME(t->key), &family, &address);
if (r < 0)
return r;
if (r == 0)
@ -428,8 +430,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
}
/* Only consider responses with equivalent query section to the request */
r = dns_question_is_equal(p->question, t->question);
if (r <= 0) {
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);
return;
}
@ -518,7 +519,6 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat
static int dns_transaction_make_packet(DnsTransaction *t) {
_cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
unsigned n, added = 0;
int r;
assert(t);
@ -530,24 +530,17 @@ static int dns_transaction_make_packet(DnsTransaction *t) {
if (r < 0)
return r;
for (n = 0; n < t->question->n_keys; n++) {
r = dns_scope_good_key(t->scope, t->question->keys[n]);
if (r < 0)
return r;
if (r == 0)
continue;
r = dns_packet_append_key(p, t->question->keys[n], NULL);
if (r < 0)
return r;
added++;
}
if (added <= 0)
r = dns_scope_good_key(t->scope, t->key);
if (r < 0)
return r;
if (r == 0)
return -EDOM;
DNS_PACKET_HEADER(p)->qdcount = htobe16(added);
r = dns_packet_append_key(p, t->key, NULL);
if (r < 0)
return r;
DNS_PACKET_HEADER(p)->qdcount = htobe16(1);
DNS_PACKET_HEADER(p)->id = t->id;
t->sent = p;
@ -621,7 +614,7 @@ 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->question, &t->cached_rcode, &t->cached);
r = dns_cache_lookup(&t->scope->cache, t->key, &t->cached_rcode, &t->cached);
if (r < 0)
return r;
if (r > 0) {
@ -674,8 +667,8 @@ int dns_transaction_go(DnsTransaction *t) {
return r;
if (t->scope->protocol == DNS_PROTOCOL_LLMNR &&
(dns_question_endswith(t->question, "in-addr.arpa") > 0 ||
dns_question_endswith(t->question, "ip6.arpa") > 0)) {
(dns_name_endswith(DNS_RESOURCE_KEY_NAME(t->key), "in-addr.arpa") > 0 ||
dns_name_endswith(DNS_RESOURCE_KEY_NAME(t->key), "ip6.arpa") > 0)) {
/* RFC 4795, Section 2.4. says reverse lookups shall
* always be made via TCP on LLMNR */

View File

@ -47,7 +47,7 @@ enum DnsTransactionState {
struct DnsTransaction {
DnsScope *scope;
DnsQuestion *question;
DnsResourceKey *key;
DnsTransactionState state;
uint16_t id;
@ -84,7 +84,7 @@ struct DnsTransaction {
LIST_FIELDS(DnsTransaction, transactions_by_scope);
};
int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsQuestion *q);
int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key);
DnsTransaction* dns_transaction_free(DnsTransaction *t);
void dns_transaction_gc(DnsTransaction *t);

View File

@ -166,7 +166,6 @@ static int dns_zone_link_item(DnsZone *z, DnsZoneItem *i) {
static int dns_zone_item_probe_start(DnsZoneItem *i) {
_cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL;
_cleanup_(dns_question_unrefp) DnsQuestion *question = NULL;
DnsTransaction *t;
int r;
@ -179,17 +178,9 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) {
if (!key)
return -ENOMEM;
question = dns_question_new(1);
if (!question)
return -ENOMEM;
r = dns_question_add(question, key);
if (r < 0)
return r;
t = dns_scope_find_transaction(i->scope, question, false);
t = dns_scope_find_transaction(i->scope, key, false);
if (!t) {
r = dns_transaction_new(&t, i->scope, question);
r = dns_transaction_new(&t, i->scope, key);
if (r < 0)
return r;
}
@ -217,7 +208,6 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) {
}
dns_zone_item_ready(i);
return 0;
gc: