From: Ondřej Surý Date: Sun, 2 Feb 2025 19:36:13 +0000 (+0100) Subject: Clean up the search part in qpcache_find() X-Git-Tag: ondrej/lock-free-qpzone-reads-v1~51^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3b2fe808c4892dc673dec7a5c9e14af6d9cf80ea;p=thirdparty%2Fbind9.git Clean up the search part in qpcache_find() Slightly refactor the header search in qpcache_find(), so the scope level is reduced and the cname parts are logically grouped together. --- diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index b8897141adb..e3df2815fa3 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -1315,59 +1315,55 @@ check_stale_header(qpcnode_t *node, dns_slabheader_t *header, return true; } +/* + * Return true if we've found headers for both 'type' and RRSIG('type'), + * or (optionally, if 'negtype' is nonzero) if we've found a single + * negative header covering either 'negtype' or ANY. + */ static bool related_headers(dns_slabheader_t *header, dns_typepair_t type, dns_typepair_t sigtype, dns_typepair_t negtype, - dns_slabheader_t **foundp, dns_slabheader_t **foundsigp) { + dns_slabheader_t **foundp, dns_slabheader_t **foundsigp, + bool *matchp) { if (!EXISTS(header) || ANCIENT(header)) { return false; } if (header->type == type) { *foundp = header; + SET_IF_NOT_NULL(matchp, true); if (*foundsigp != NULL) { return true; } } else if (header->type == sigtype) { *foundsigp = header; + SET_IF_NOT_NULL(matchp, true); if (*foundp != NULL) { return true; } - - } else if (header->type == RDATATYPE_NCACHEANY || - header->type == negtype) + } else if (negtype != 0 && (header->type == RDATATYPE_NCACHEANY || + header->type == negtype)) { *foundp = header; *foundsigp = NULL; + SET_IF_NOT_NULL(matchp, true); return true; } return false; } +/* + * Return true if we've found headers for both 'type' and RRSIG('type'). + */ static bool both_headers(dns_slabheader_t *header, dns_rdatatype_t type, dns_slabheader_t **foundp, dns_slabheader_t **foundsigp) { dns_typepair_t matchtype = DNS_TYPEPAIR_VALUE(type, 0); dns_typepair_t sigmatchtype = DNS_SIGTYPE(type); - if (!EXISTS(header) || ANCIENT(header)) { - return false; - } - - if (header->type == matchtype) { - *foundp = header; - if (*foundsigp != NULL) { - return true; - } - } else if (header->type == sigmatchtype) { - *foundsigp = header; - if (*foundp != NULL) { - return true; - } - } - - return false; + return related_headers(header, matchtype, sigmatchtype, 0, foundp, + foundsigp, NULL); } static isc_result_t @@ -1599,6 +1595,15 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name, return result; } +#define MISSING_ANSWER(found, options) \ + ((found) == NULL || \ + (DNS_TRUST_ADDITIONAL((found)->trust) && \ + (((options) & DNS_DBFIND_ADDITIONALOK) == 0)) || \ + ((found)->trust == dns_trust_glue && \ + (((options) & DNS_DBFIND_GLUEOK) == 0)) || \ + (DNS_TRUST_PENDING((found)->trust) && \ + (((options) & DNS_DBFIND_PENDINGOK) == 0))) + static isc_result_t qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, dns_rdatatype_t type, unsigned int options, isc_stdtime_t now, @@ -1753,91 +1758,92 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, continue; } - if (EXISTS(header) && !ANCIENT(header)) { + if (!EXISTS(header) || ANCIENT(header)) { + continue; + } + + /* + * We now know that there is at least one active + * non-stale rdataset at this node. + */ + empty_node = false; + + if (header->noqname != NULL && + header->trust == dns_trust_secure) + { + found_noqname = true; + } + + if (!NEGATIVE(header)) { + all_negative = false; + } + + bool match = false; + if (related_headers(header, type, sigtype, negtype, &found, + &foundsig, &match) && + !MISSING_ANSWER(found, options)) + { /* - * We now know that there is at least one active - * non-stale rdataset at this node. + * We can't exit early until we have an answer with + * sufficient trust level, see MISSING_ANSWER() macro + * for details, because we might need NS or NSEC + * records. */ - empty_node = false; - if (header->noqname != NULL && - header->trust == dns_trust_secure) - { - found_noqname = true; + + break; + } + + if (match) { + /* We found something, continue with next header */ + continue; + } + + switch (header->type) { + case dns_rdatatype_cname: + if (!cname_ok) { + break; } - if (!NEGATIVE(header)) { - all_negative = false; + + found = header; + if (cnamesig != NULL) { + /* We already have CNAME signature */ + foundsig = cnamesig; + } else { + /* Look for CNAME signature instead */ + sigtype = DNS_SIGTYPE(dns_rdatatype_cname); + foundsig = NULL; + } + break; + case DNS_SIGTYPE(dns_rdatatype_cname): + if (!cname_ok) { + break; } - /* - * If we found a type we were looking for, remember - * it. - */ - if (header->type == type || - (type == dns_rdatatype_any && - DNS_TYPEPAIR_TYPE(header->type) != 0) || - (cname_ok && header->type == dns_rdatatype_cname)) - { - /* - * We've found the answer. - */ - found = header; - if (header->type == dns_rdatatype_cname && - cname_ok) - { - /* - * If we've already got the - * CNAME RRSIG, use it. - */ - if (cnamesig != NULL) { - foundsig = cnamesig; - } else { - sigtype = DNS_SIGTYPE( - dns_rdatatype_cname); - } - } - } else if (header->type == sigtype) { - /* - * We've found the RRSIG rdataset for our - * target type. Remember it. - */ - foundsig = header; - } else if (header->type == RDATATYPE_NCACHEANY || - header->type == negtype) + cnamesig = header; + break; + case dns_rdatatype_ns: + /* Remember the NS rdataset */ + nsheader = header; + break; + case DNS_SIGTYPE(dns_rdatatype_ns): + /* ...and its signature */ + nssig = header; + break; + + case dns_rdatatype_nsec: + nsecheader = header; + break; + case DNS_SIGTYPE(dns_rdatatype_nsec): + nsecsig = header; + break; + + default: + if (type == dns_rdatatype_any && + DNS_TYPEPAIR_TYPE(header->type) != 0) { - /* - * We've found a negative cache entry. - */ + /* QTYPE==ANY, so any anwers will do */ found = header; - } else if (header->type == dns_rdatatype_ns) { - /* - * Remember a NS rdataset even if we're - * not specifically looking for it, because - * we might need it later. - */ - nsheader = header; - } else if (header->type == - DNS_SIGTYPE(dns_rdatatype_ns)) - { - /* - * If we need the NS rdataset, we'll also - * need its signature. - */ - nssig = header; - } else if (header->type == dns_rdatatype_nsec) { - nsecheader = header; - } else if (header->type == - DNS_SIGTYPE(dns_rdatatype_nsec)) - { - nsecsig = header; - } else if (cname_ok && - header->type == - DNS_SIGTYPE(dns_rdatatype_cname)) - { - /* - * If we get a CNAME match, we'll also need - * its signature. - */ - cnamesig = header; + break; } } } @@ -1863,14 +1869,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, /* * If we didn't find what we were looking for... */ - if (found == NULL || - (DNS_TRUST_ADDITIONAL(found->trust) && - ((options & DNS_DBFIND_ADDITIONALOK) == 0)) || - (found->trust == dns_trust_glue && - ((options & DNS_DBFIND_GLUEOK) == 0)) || - (DNS_TRUST_PENDING(found->trust) && - ((options & DNS_DBFIND_PENDINGOK) == 0))) - { + if (MISSING_ANSWER(found, options)) { /* * Return covering NODATA NSEC record. */ @@ -2195,7 +2194,7 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, } if (related_headers(header, matchtype, sigmatchtype, negtype, - &found, &foundsig)) + &found, &foundsig, NULL)) { break; }