From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Wed, 27 Feb 2019 09:37:40 +0000 (+0100)
Subject: resolved: when adding RR to an answer, avoid comparing keys twice
X-Git-Tag: v242-rc1~202^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dffb8277728b8052258bff49d7157fc4e326f88f;p=thirdparty%2Fsystemd.git

resolved: when adding RR to an answer, avoid comparing keys twice

We'd call dns_resource_record_equal(), which calls dns_resource_key_equal()
internally, and then dns_resource_key_equal() a second time. Let's be
a bit smarter, and call dns_resource_key_equal() only once.

(before)
dns_resource_key_hash_func_count=514
dns_resource_key_compare_func_count=275
dns_resource_key_equal_count=62371
4.13s user 0.01s system 99% cpu 4.153 total

(after)
dns_resource_key_hash_func_count=514
dns_resource_key_compare_func_count=276
dns_resource_key_equal_count=31337
2.13s user 0.01s system 99% cpu 2.139 total
---

diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c
index d7252d3dac1..44dc6552b4f 100644
--- a/src/resolve/resolved-dns-answer.c
+++ b/src/resolve/resolved-dns-answer.c
@@ -87,40 +87,33 @@ int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFl
                 if (a->items[i].ifindex != ifindex)
                         continue;
 
-                r = dns_resource_record_equal(a->items[i].rr, rr);
+                r = dns_resource_key_equal(a->items[i].rr->key, rr->key);
                 if (r < 0)
                         return r;
-                if (r > 0) {
-                        /* Don't mix contradicting TTLs (see below) */
-                        if ((rr->ttl == 0) != (a->items[i].rr->ttl == 0))
-                                return -EINVAL;
-
-                        /* Entry already exists, keep the entry with
-                         * the higher RR. */
-                        if (rr->ttl > a->items[i].rr->ttl) {
-                                dns_resource_record_ref(rr);
-                                dns_resource_record_unref(a->items[i].rr);
-                                a->items[i].rr = rr;
-                        }
+                if (r == 0)
+                        continue;
 
-                        a->items[i].flags |= flags;
-                        return 0;
-                }
+                /* There's already an RR of the same RRset in place! Let's see if the TTLs more or less
+                 * match. We don't really care if they match precisely, but we do care whether one is 0 and
+                 * the other is not. See RFC 2181, Section 5.2. */
+                if ((rr->ttl == 0) != (a->items[i].rr->ttl == 0))
+                        return -EINVAL;
 
-                r = dns_resource_key_equal(a->items[i].rr->key, rr->key);
+                r = dns_resource_record_payload_equal(a->items[i].rr, rr);
                 if (r < 0)
                         return r;
-                if (r > 0) {
-                        /* There's already an RR of the same RRset in
-                         * place! Let's see if the TTLs more or less
-                         * match. We don't really care if they match
-                         * precisely, but we do care whether one is 0
-                         * and the other is not. See RFC 2181, Section
-                         * 5.2. */
-
-                        if ((rr->ttl == 0) != (a->items[i].rr->ttl == 0))
-                                return -EINVAL;
+                if (r == 0)
+                        continue;
+
+                /* Entry already exists, keep the entry with the higher RR. */
+                if (rr->ttl > a->items[i].rr->ttl) {
+                        dns_resource_record_ref(rr);
+                        dns_resource_record_unref(a->items[i].rr);
+                        a->items[i].rr = rr;
                 }
+
+                a->items[i].flags |= flags;
+                return 0;
         }
 
         return dns_answer_add_raw(a, rr, ifindex, flags);
diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c
index ff58a9d8101..278546da848 100644
--- a/src/resolve/resolved-dns-packet.c
+++ b/src/resolve/resolved-dns-packet.c
@@ -2258,7 +2258,7 @@ static int dns_packet_extract_answer(DnsPacket *p, DnsAnswer **ret_answer) {
                                          * be contained in questions, never in replies. Crappy
                                          * Belkin routers copy the OPT data for example, hence let's
                                          * detect this so that we downgrade early. */
-                                        log_debug("OPT RR contained RFC6975 data, ignoring.");
+                                        log_debug("OPT RR contains RFC6975 data, ignoring.");
                                         bad_opt = true;
                                         continue;
                                 }