From 5070c7f5c74a143f74775515d65fad01f398ecd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 6 Feb 2024 14:05:08 +0100 Subject: [PATCH 1/5] Fix missing RRSIG for CNAME with different slabheader order The cachedb was missing piece of code (already found in zonedb) that would make lookups in the slabheaders to miss the RRSIGs for CNAME if the order of CNAME and RRSIG(CNAME) was reversed in the node->data. --- lib/dns/rbt-cachedb.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/dns/rbt-cachedb.c b/lib/dns/rbt-cachedb.c index ea2ff08996..f4f3d25be6 100644 --- a/lib/dns/rbt-cachedb.c +++ b/lib/dns/rbt-cachedb.c @@ -909,13 +909,18 @@ cache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, */ found = header; if (header->type == dns_rdatatype_cname && - cname_ok && cnamesig != NULL) + cname_ok) { /* * If we've already got the * CNAME RRSIG, use it. */ - foundsig = cnamesig; + if (cnamesig != NULL) { + foundsig = cnamesig; + } else { + sigtype = + RBTDB_RDATATYPE_SIGCNAME; + } } } else if (header->type == sigtype) { /* From 3ac482be7fd058d284e89873021339579fad0615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 29 Jan 2024 16:36:30 +0100 Subject: [PATCH 2/5] Optimize the slabheader placement for certain RRTypes Mark the infrastructure RRTypes as "priority" types and place them at the beginning of the rdataslab header data graph. The non-priority types either go right after the priority types (if any). --- lib/dns/rbtdb.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 900b580058..3169ebb6ee 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -335,6 +335,30 @@ dns__rbtdb_setttl(dns_slabheader_t *header, dns_ttl_t newttl) { } } +static bool +prio_type(dns_typepair_t type) { + switch (type) { + case dns_rdatatype_soa: + case DNS_TYPEPAIR_VALUE(dns_rdatatype_rrsig, dns_rdatatype_soa): + case dns_rdatatype_a: + case DNS_TYPEPAIR_VALUE(dns_rdatatype_rrsig, dns_rdatatype_a): + case dns_rdatatype_aaaa: + case DNS_TYPEPAIR_VALUE(dns_rdatatype_rrsig, dns_rdatatype_aaaa): + case dns_rdatatype_nsec: + case DNS_TYPEPAIR_VALUE(dns_rdatatype_rrsig, dns_rdatatype_nsec): + case dns_rdatatype_nsec3: + case DNS_TYPEPAIR_VALUE(dns_rdatatype_rrsig, dns_rdatatype_nsec3): + case dns_rdatatype_ns: + case DNS_TYPEPAIR_VALUE(dns_rdatatype_rrsig, dns_rdatatype_ns): + case dns_rdatatype_ds: + case DNS_TYPEPAIR_VALUE(dns_rdatatype_rrsig, dns_rdatatype_ds): + case dns_rdatatype_cname: + case DNS_TYPEPAIR_VALUE(dns_rdatatype_rrsig, dns_rdatatype_cname): + return (true); + } + return (false); +} + /*% * These functions allow the heap code to rank the priority of each * element. It returns true if v1 happens "sooner" than v2. @@ -2482,6 +2506,7 @@ dns__rbtdb_add(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_changed_t *changed = NULL; dns_slabheader_t *topheader = NULL, *topheader_prev = NULL; dns_slabheader_t *header = NULL, *sigheader = NULL; + dns_slabheader_t *prioheader = NULL; unsigned char *merged = NULL; isc_result_t result; bool header_nx; @@ -2615,6 +2640,9 @@ dns__rbtdb_add(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, for (topheader = rbtnode->data; topheader != NULL; topheader = topheader->next) { + if (prio_type(topheader->type)) { + prioheader = topheader; + } if (topheader->type == newheader->type || topheader->type == negtype) { @@ -2999,9 +3027,21 @@ find_header: /* * No rdatasets of the given type exist at the node. */ - newheader->next = rbtnode->data; - newheader->down = NULL; - rbtnode->data = newheader; + INSIST(newheader->down == NULL); + + if (prio_type(newheader->type)) { + /* This is a priority type, prepend it */ + newheader->next = rbtnode->data; + rbtnode->data = newheader; + } else if (prioheader != NULL) { + /* Append after the priority headers */ + newheader->next = prioheader->next; + prioheader->next = newheader; + } else { + /* There were no priority headers */ + newheader->next = rbtnode->data; + rbtnode->data = newheader; + } } } From 3f774c2a8ac46b2bbf1b3a4fa1d8d8c3ed3d78a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 29 Jan 2024 16:36:30 +0100 Subject: [PATCH 3/5] Optimize cname_and_other_data to stop as earliest as possible Stop the cname_and_other_data processing if we already know that the result is true. Also, we know that CNAME will be placed in the priority headers, so we can stop looking for CNAME if we haven't found CNAME and we are past the priority headers. --- bin/tests/system/resolver/tests.sh | 4 ++-- lib/dns/rbtdb.c | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh index d9f2ab6f7a..65ef9b928b 100755 --- a/bin/tests/system/resolver/tests.sh +++ b/bin/tests/system/resolver/tests.sh @@ -482,10 +482,10 @@ dig_with_opts @10.53.0.5 fetchall.tld any >dig.out.2.${n} || ret=1 ttl2=$(awk '/"A" "short" "ttl"/ { print $2 }' dig.out.2.${n}) sleep 1 # check that prefetch occurred; -# note that only one record is prefetched, which is the TXT record in this case, +# note that only one record is prefetched, which is the AAAA record in this case, # because of the order of the records in the cache dig_with_opts @10.53.0.5 fetchall.tld any >dig.out.3.${n} || ret=1 -ttl3=$(awk '/"A" "short" "ttl"/ { print $2 }' dig.out.3.${n}) +ttl3=$(awk '/::1/ { print $2 }' dig.out.3.${n}) test "${ttl3:-0}" -gt "${ttl2:-1}" || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 3169ebb6ee..7b2df98ca7 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -2391,7 +2391,7 @@ dns__rbtdb_allrdatasets(dns_db_t *db, dns_dbnode_t *node, static bool cname_and_other_data(dns_rbtnode_t *node, uint32_t serial) { dns_slabheader_t *header = NULL, *header_next = NULL; - bool cname, other_data; + bool cname = false, other_data = false; dns_rdatatype_t rdtype; /* @@ -2401,10 +2401,16 @@ cname_and_other_data(dns_rbtnode_t *node, uint32_t serial) { /* * Look for CNAME and "other data" rdatasets active in our version. */ - cname = false; - other_data = false; for (header = node->data; header != NULL; header = header_next) { header_next = header->next; + if (!prio_type(header->type)) { + /* + * CNAME is in the priority list, so if we are done + * with the priority list, we know there will not be + * CNAME, so we are safe to skip the rest of the types. + */ + return (false); + } if (header->type == dns_rdatatype_cname) { /* * Look for an active extant CNAME. @@ -2464,10 +2470,9 @@ cname_and_other_data(dns_rbtnode_t *node, uint32_t serial) { } } } - } - - if (cname && other_data) { - return (true); + if (cname && other_data) { + return (true); + } } return (false); From 6e81717cffe650216ce1bd17f2fe1713690661d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 6 Feb 2024 18:05:41 +0100 Subject: [PATCH 4/5] Add CHANGES note for [GL !8675] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index ddc71de48f..0cd20df58d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6338. [func] Optimize slabheader placement, so the infrastructure + records are put in the beginning of the slabheader + linked list. [GL !8675] + 6337. [bug] Nsupdate could assert while shutting down. [GL #4529] 6336. [func] Expose the zones with the 'first refresh' flag set in From 840412947109d761bab1f6ce1e3f19cde888c7b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 8 Feb 2024 08:30:38 +0100 Subject: [PATCH 5/5] Use DNS_QPGC_MAYBE instead of DNS_QPGC_ALL for more realistic load In the benchmarks, DNS_QPGC_ALL was trying to hard to cleanup QP and this was slowing down QP too much. Use DNS_QPGC_MAYBE instead that we are going to use anyway for more realistic load - this also shows the memory usage matching the real loads. --- tests/bench/load-names.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bench/load-names.c b/tests/bench/load-names.c index f4562aa6db..bf41222311 100644 --- a/tests/bench/load-names.c +++ b/tests/bench/load-names.c @@ -374,7 +374,7 @@ add_qp(void *qp, size_t count) { static void sqz_qp(void *qp) { - dns_qp_compact(qp, DNS_QPGC_ALL); + dns_qp_compact(qp, DNS_QPGC_MAYBE); } static isc_result_t