From: Zbigniew Jędrzejewski-Szmek Date: Tue, 31 Jul 2018 13:09:13 +0000 (+0200) Subject: resolved: keep addresses mapped to ::0 in a separate set X-Git-Tag: v240~874^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fd373593ba9b7c8426529623a803086038db4474;p=thirdparty%2Fsystemd.git resolved: keep addresses mapped to ::0 in a separate set We'd store every 0.0.0.0 and ::0 entry as a structure without any addresses allocated. This is a somewhat common use case, let's optimize it a bit. This gives some memory savings and a bit faster response time too: 'time build/test-resolved-etc-hosts hosts' goes from 7.7s to 5.6s, and memory use as reported by valgrind for ~10000 hosts is reduced ==18097== total heap usage: 29,902 allocs, 29,902 frees, 2,136,437 bytes allocated ==18240== total heap usage: 19,955 allocs, 19,955 frees, 1,556,021 bytes allocated Also rename 'suppress' to 'found' (with reverse meaning). I think this makes the intent clearer. --- diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c index 7b381f0561f..2adee078897 100644 --- a/src/resolve/resolved-etc-hosts.c +++ b/src/resolve/resolved-etc-hosts.c @@ -26,6 +26,7 @@ static inline void etc_hosts_item_by_name_free(EtcHostsItemByName *item) { void etc_hosts_free(EtcHosts *hosts) { hosts->by_address = hashmap_free_with_destructor(hosts->by_address, etc_hosts_item_free); hosts->by_name = hashmap_free_with_destructor(hosts->by_name, etc_hosts_item_by_name_free); + hosts->no_address = set_free_free(hosts->no_address); } void manager_etc_hosts_flush(Manager *m) { @@ -36,7 +37,7 @@ void manager_etc_hosts_flush(Manager *m) { static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) { _cleanup_free_ char *address_str = NULL; struct in_addr_data address = {}; - bool suppressed = false; + bool found = false; EtcHostsItem *item; int r; @@ -99,18 +100,32 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) { if (r <= 0) return log_error_errno(r, "Hostname %s is not valid, ignoring, in line /etc/hosts:%u.", name, nr); - if (is_localhost(name)) { + found = true; + + if (is_localhost(name)) /* Suppress the "localhost" line that is often seen */ - suppressed = true; continue; - } - if (item) { - r = strv_extend(&item->names, name); + if (!item) { + /* Optimize the case where we don't need to store any addresses, by storing + * only the name in a dedicated Set instead of the hashmap */ + + r = set_ensure_allocated(&hosts->no_address, &dns_name_hash_ops); if (r < 0) return log_oom(); + + r = set_put(hosts->no_address, name); + if (r < 0) + return r; + + TAKE_PTR(name); + continue; } + r = strv_extend(&item->names, name); + if (r < 0) + return log_oom(); + bn = hashmap_get(hosts->by_name, name); if (!bn) { r = hashmap_ensure_allocated(&hosts->by_name, &dns_name_hash_ops); @@ -130,17 +145,13 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) { bn->name = TAKE_PTR(name); } - if (item) { - if (!GREEDY_REALLOC(bn->addresses, bn->n_allocated, bn->n_addresses + 1)) - return log_oom(); - - bn->addresses[bn->n_addresses++] = &item->address; - } + if (!GREEDY_REALLOC(bn->addresses, bn->n_allocated, bn->n_addresses + 1)) + return log_oom(); - suppressed = true; + bn->addresses[bn->n_addresses++] = &item->address; } - if (!suppressed) { + if (!found) { log_error("Line is missing any host names, in line /etc/hosts:%u.", nr); return -EINVAL; } @@ -310,12 +321,15 @@ int manager_etc_hosts_lookup(Manager *m, DnsQuestion* q, DnsAnswer **answer) { } bn = hashmap_get(m->etc_hosts.by_name, name); - if (!bn) - return 0; - - r = dns_answer_reserve(answer, bn->n_addresses); - if (r < 0) - return r; + if (bn) { + r = dns_answer_reserve(answer, bn->n_addresses); + if (r < 0) + return r; + } else { + /* Check if name was listed with no address. If yes, continue to return an answer. */ + if (!set_contains(m->etc_hosts.no_address, name)) + return 0; + } DNS_QUESTION_FOREACH(t, q) { if (!IN_SET(t->type, DNS_TYPE_A, DNS_TYPE_AAAA, DNS_TYPE_ANY)) @@ -338,7 +352,7 @@ int manager_etc_hosts_lookup(Manager *m, DnsQuestion* q, DnsAnswer **answer) { break; } - for (i = 0; i < bn->n_addresses; i++) { + for (i = 0; bn && i < bn->n_addresses; i++) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr = NULL; if ((!found_a && bn->addresses[i]->family == AF_INET) || diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index c9cf2f0ee6a..79a473f57ef 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -26,6 +26,7 @@ typedef struct Manager Manager; typedef struct EtcHosts { Hashmap *by_address; Hashmap *by_name; + Set *no_address; } EtcHosts; struct Manager { diff --git a/src/resolve/test-resolved-etc-hosts.c b/src/resolve/test-resolved-etc-hosts.c index 74a565fae71..5b8f1e220e3 100644 --- a/src/resolve/test-resolved-etc-hosts.c +++ b/src/resolve/test-resolved-etc-hosts.c @@ -71,6 +71,11 @@ static void test_parse_etc_hosts(const char *fname) { assert_se(bn->addresses[0]->family == AF_INET6); assert_se(memcmp(&bn->addresses[0]->address.in6, &(struct in6_addr) { .s6_addr = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5} }, 16 ) == 0); + + assert_se( set_contains(hosts.no_address, "some.where")); + assert_se( set_contains(hosts.no_address, "some.other")); + assert_se( set_contains(hosts.no_address, "black.listed")); + assert_se(!set_contains(hosts.no_address, "foobar.foo.foo")); } int main(int argc, char **argv) {