From: Ondřej Surý Date: Fri, 24 Jul 2026 18:46:36 +0000 (+0200) Subject: Delete cache rdatasets directly instead of tombstoning them X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f0dd25d6cf67c118b08e12f4f252a6a3a1b54e5b;p=thirdparty%2Fbind9.git Delete cache rdatasets directly instead of tombstoning them The refactoring that cleans the previous headers immediately from the cache allows additional cleanup. The cache now does not require the NONEXISTENT tombstone and dns_db_deleterdataset() can directly remove the header with associate type from the cache. This simplifies all the other code working with headers as the cache now only contains header that carry information, so the only check we need to do now is ACTIVE() check whether the TTL is still ok (+ serve-stale hacks). --- diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index baf9486ba10..f97eb3dd8c6 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -1242,8 +1242,9 @@ dns__db_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, * existing rdataset of the same type. If not merging and the * #DNS_DBADD_FORCE option is set, then the data will update the database * without regard to trust levels. If not forcing the data, then the - * rdataset will only be added if its trust level is >= the trust level of - * any existing rdataset. Forcing is only meaningful for cache databases. + * rdataset will only be added if its trust level is >= the trust level + * of any existing active (not expired) rdataset of the same type. + * Forcing is only meaningful for cache databases. * If #DNS_DBADD_EXACT is set then there must be no rdata in common between * the old and new rdata sets. If #DNS_DBADD_EXACTTTL is set then both * the old and new rdata sets must have the same ttl. @@ -1252,6 +1253,11 @@ dns__db_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, * changed, compare the old and new rdatasets; if they are equal, * return #ISC_R_SUCCESS instead of #DNS_R_UNCHANGED. * + * \li If the #DNS_DBADD_PREFETCH option is set, the rdataset is being + * added as the result of a prefetch. In a cache database this allows + * equal but less trusted data to refresh the TTL of existing address + * (A/AAAA) and DS records, which would otherwise be kept unchanged. + * * \li The 'now' field is ignored if 'db' is a zone database. If 'db' is * a cache database, then the added rdataset will expire no later than * now + rdataset->ttl. @@ -1353,9 +1359,11 @@ dns__db_deleterdataset(dns_db_t *db, dns_dbnode_t *node, * * Notes: * - * \li If 'type' is dns_rdatatype_any, then no rdatasets will exist in - * 'version' (provided that the dns_db_deleterdataset() isn't followed - * by one or more dns_db_addrdataset() calls). + * \li In a cache database, a negative cache entry is stored under the + * type it covers. Passing 'type' == dns_rdatatype_none with 'covers' + * set (the shape in which rdataset iterators return negative entries) + * deletes the cache entry for the covered type, whether it is positive + * or negative. * * Requires: * @@ -1367,16 +1375,16 @@ dns__db_deleterdataset(dns_db_t *db, dns_dbnode_t *node, * read-write version, or the database has cache semantics * and version is NULL. * - * \li 'type' is not a meta-RR type, except for dns_rdatatype_any, which is - * allowed. - * - * \li If 'covers' != 0, 'type' must be SIG. + * \li If 'covers' != 0, 'type' must be RRSIG or dns_rdatatype_none. * * Returns: * * \li #ISC_R_SUCCESS - * \li #DNS_R_UNCHANGED No rdatasets of 'type' existed - * before the operation was attempted. + * \li #DNS_R_UNCHANGED No rdatasets of 'type' existed before + * the operation was attempted. + * \li #ISC_R_NOTIMPLEMENTED 'type' is dns_rdatatype_any; or the + * database has zone semantics and 'type' + * is RRSIG with 'covers' == 0. * * \li Other results are possible, depending upon the database * implementation used. diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h index 5dae1d93802..85ceafc50b9 100644 --- a/lib/dns/include/dns/rdataslab.h +++ b/lib/dns/include/dns/rdataslab.h @@ -128,7 +128,6 @@ ISC_REFCOUNT_DECL(dns_slabheader); #endif enum { - DNS_SLABHEADERATTR_NONEXISTENT = 1 << 0, DNS_SLABHEADERATTR_STALE = 1 << 1, DNS_SLABHEADERATTR_IGNORE = 1 << 2, DNS_SLABHEADERATTR_NXDOMAIN = 1 << 3, @@ -249,16 +248,6 @@ dns_slabheader__reset(dns_slabheader_t *h, dns_dbnode_t *node, const char *func, * database node 'node'. */ -#define dns_slabheader_new(mctx, node) \ - dns_slabheader__new(mctx, node, __func__, __FILE__, __LINE__) -dns_slabheader_t * -dns_slabheader__new(isc_mem_t *mctx, dns_dbnode_t *node, const char *func, - const char *file, const unsigned int line); -/*%< - * Allocate memory for an rdataslab header and initialize it for use - * in database node 'node'. - */ - void dns_slabheader_freeproof(isc_mem_t *mctx, dns_slabheader_proof_t **proof); /*%< diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 34bfedda82c..c84039da891 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -412,15 +412,6 @@ cleanup_deadnodes_cb(void *arg); static size_t header_delete(qpcnode_t *node, dns_slabheader_t *header); -static size_t -rdataset_size(dns_slabheader_t *header) { - if (EXISTS(header)) { - return dns_rdataslab_size(header); - } - - return sizeof(*header); -} - static void flush_node(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, isc_rwlocktype_t *tlocktypep, dns_expire_t reason DNS__DB_FLARG); @@ -487,7 +478,7 @@ qpcache_miss(qpcache_t *qpdb, dns_slabheader_t *newheader, size_t purgesize = 2 * (sizeof(qpcnode_t) + dns_name_size(&HEADERNODE(newheader)->name)) + - rdataset_size(newheader) + QP_SAFETY_MARGIN; + dns_rdataslab_size(newheader) + QP_SAFETY_MARGIN; expire_lru_headers(qpdb, newheader, idx, purgesize, nlocktypep, tlocktypep DNS__DB_FLARG_PASS); @@ -727,7 +718,7 @@ update_rrsetstats(dns_stats_t *stats, const dns_typepair_t typepair, .attributes = hattributes, }; - if (!EXISTS(header) || !STATCOUNT(header)) { + if (!STATCOUNT(header)) { return; } @@ -793,7 +784,7 @@ header_delete(qpcnode_t *node, dns_slabheader_t *header) { return 0; } - size_t expired = rdataset_size(header); + size_t expired = dns_rdataslab_size(header); qpcache_t *qpdb = node->qpdb; cds_list_del_init(&header->headers_link); @@ -930,6 +921,10 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, rdataset->methods = &dns_rdataslab_rdatasetmethods; rdataset->rdclass = qpdb->common.rdclass; if (NEGATIVE(header)) { + /* + * Convert the internal represetation of a negative record + * to the rdataset representation (type=0, covers=type). + */ rdataset->type = dns_rdatatype_none; rdataset->covers = DNS_TYPEPAIR_TYPE(header->typepair); INSIST(DNS_TYPEPAIR_COVERS(header->typepair) == @@ -1099,8 +1094,7 @@ check_stale_header(dns_slabheader_t *header, qpc_search_t *search) { static bool invalid_header(dns_slabheader_t *header, qpc_search_t *search) { - return header == NULL || check_stale_header(header, search) || - !EXISTS(header); + return header == NULL || check_stale_header(header, search); } /* @@ -1145,7 +1139,6 @@ related_headers(dns_slabheader_t *header, dns_slabheader_t *sigheader, return false; } - REQUIRE(EXISTS(sigheader)); if (sigheader->typepair == typepair) { *foundp = sigheader; *foundsigp = NULL; @@ -1157,7 +1150,6 @@ related_headers(dns_slabheader_t *header, dns_slabheader_t *sigheader, return false; } - REQUIRE(EXISTS(header)); REQUIRE(!NEGATIVE(header) || sigheader == NULL); if (header->typepair == typepair) { @@ -2097,10 +2089,6 @@ qpcache_createiterator(dns_db_t *db, unsigned int options ISC_ATTR_UNUSED, static bool iterator_active(qpcache_t *qpdb, qpc_rditer_t *iterator, dns_slabheader_t *header) { - if (!EXISTS(header)) { - return false; - } - /* * If this header is still active then return it. */ @@ -2150,9 +2138,6 @@ qpcache_allrdatasets(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, NODE_RDLOCK(nlock, &nlocktype); DNS_SLABHEADER_FOREACH(header, &qpnode->headers) { - if (!EXISTS(header)) { - continue; - } if (EXPIREDOK(iterator) || iterator_active(qpdb, iterator, header)) { @@ -2321,8 +2306,8 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, * separate pass first: evicting as we go and bailing out later would * destroy lower-trust siblings before we found the secure header. */ - if (EXISTS(newheader) && NEGATIVE(newheader) && - rdtype == dns_rdatatype_any && trust < dns_trust_secure) + if (NEGATIVE(newheader) && rdtype == dns_rdatatype_any && + trust < dns_trust_secure) { DNS_SLABHEADER_FOREACH(header, &qpnode->headers) { if (ACTIVE(header, now) && @@ -2338,7 +2323,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, } DNS_SLABHEADER_FOREACH(header, &qpnode->headers) { - if (EXISTS(newheader) && NEGATIVE(newheader)) { + if (NEGATIVE(newheader)) { if (rdtype == dns_rdatatype_any) { /* * We're adding a negative cache entry which @@ -2366,8 +2351,8 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, } } } - if (EXISTS(header) && EXISTS(newheader) && NEGATIVE(header) && - !NEGATIVE(newheader) && ACTIVE(header, now)) + if (NEGATIVE(header) && !NEGATIVE(newheader) && + ACTIVE(header, now)) { /* * There's an existing NXDOMAIN or negative @@ -2427,13 +2412,6 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, } if (oldheader != NULL) { - /* - * Deleting an already non-existent rdataset has no effect. - */ - if (!EXISTS(oldheader) && !EXISTS(newheader)) { - return DNS_R_UNCHANGED; - } - /* * Trying to add an rdataset with lower trust to a cache * DB has no effect, provided that the cache data isn't @@ -2442,9 +2420,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, * policy is here. */ dns_trust_t oldtrust = header_trust(oldheader); - if (trust < oldtrust && - (ACTIVE(oldheader, now) || !EXISTS(oldheader))) - { + if (trust < oldtrust && ACTIVE(oldheader, now)) { qpcache_hit(qpdb, oldheader); bindrdataset(qpdb, qpnode, oldheader, now, nlocktype, tlocktype, @@ -2474,7 +2450,6 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, */ if (ACTIVE(oldheader, now) && oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) && - EXISTS(oldheader) && EXISTS(newheader) && newheader->trust < oldtrust && oldheader->expire < newheader->expire && dns_rdataslab_equalx( @@ -2508,7 +2483,6 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, */ if (ACTIVE(oldheader, now) && oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) && - EXISTS(oldheader) && EXISTS(newheader) && newheader->trust > oldtrust) { if (newheader->expire > oldheader->expire) { @@ -2527,7 +2501,6 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_ds) || oldheader->typepair == DNS_SIGTYPEPAIR(dns_rdatatype_ds)) && - EXISTS(oldheader) && EXISTS(newheader) && newheader->trust < oldtrust && oldheader->expire < newheader->expire && dns_rdataslab_equal(oldheader, newheader)) @@ -2554,13 +2527,6 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, INSIST(oldheader->related == related); header_delete(qpnode, oldheader); - - } else if (!EXISTS(newheader)) { - /* - * The type already doesn't exist; no point trying - * to delete it. - */ - return DNS_R_UNCHANGED; } /* @@ -2611,8 +2577,8 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, * * Delete the related rrsig in the cache. */ - if (EXISTS(newheader) && NEGATIVE(newheader) && - !dns_rdatatype_issig(rdtype) && related != NULL) + if (NEGATIVE(newheader) && !dns_rdatatype_issig(rdtype) && + related != NULL) { header_delete(qpnode, related); } @@ -2800,11 +2766,10 @@ qpcache_deleterdataset(dns_db_t *db, dns_dbnode_t *node, dns_rdatatype_t covers DNS__DB_FLARG) { qpcache_t *qpdb = (qpcache_t *)db; qpcnode_t *qpnode = (qpcnode_t *)node; - isc_result_t result; - dns_slabheader_t *newheader = NULL; + isc_result_t result = DNS_R_UNCHANGED; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; isc_rwlock_t *nlock = NULL; - uint16_t attributes = DNS_SLABHEADERATTR_NONEXISTENT; + dns_typepair_t typepair; REQUIRE(VALID_QPDB(qpdb)); REQUIRE(version == NULL); @@ -2814,24 +2779,26 @@ qpcache_deleterdataset(dns_db_t *db, dns_dbnode_t *node, return ISC_R_NOTIMPLEMENTED; } - /* Convert the negative type into positive type. */ + /* + * Convert the rdataset representation of a negative record + * (type=0, covers=type) to the internal representation (the + * other way around). + */ if (type == dns_rdatatype_none && covers != dns_rdatatype_none) { type = covers; covers = dns_rdatatype_none; - attributes |= DNS_SLABHEADERATTR_NEGATIVE; } - newheader = dns_slabheader_new(db->mctx, node); - newheader->typepair = DNS_TYPEPAIR_VALUE(type, covers); - setttl(newheader, 0); - atomic_init(&newheader->attributes, attributes); + typepair = DNS_TYPEPAIR_VALUE(type, covers); nlock = &qpdb->buckets[qpnode->locknum].lock; NODE_WRLOCK(nlock, &nlocktype); - result = add(qpdb, qpnode, newheader, DNS_DBADD_FORCE, NULL, 0, - nlocktype, isc_rwlocktype_none DNS__DB_FLARG_PASS); - if (result != ISC_R_SUCCESS) { - dns_slabheader_detach(&newheader); + DNS_SLABHEADER_FOREACH(header, &qpnode->headers) { + if (header->typepair == typepair) { + header_delete(qpnode, header); + result = ISC_R_SUCCESS; + break; + } } NODE_UNLOCK(nlock, &nlocktype); diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c index e87be113b22..107a1df4e50 100644 --- a/lib/dns/rdataslab.c +++ b/lib/dns/rdataslab.c @@ -531,42 +531,9 @@ dns_slabheader__reset(dns_slabheader_t *h, dns_dbnode_t *node, const char *func, #endif } -dns_slabheader_t * -dns_slabheader__new(isc_mem_t *mctx, dns_dbnode_t *node, const char *func, - const char *file, const unsigned int line) { - dns_slabheader_t *h = NULL; - - h = isc_mem_get(mctx, sizeof(*h)); - *h = (dns_slabheader_t){ - .headers_link = CDS_LIST_HEAD_INIT(h->headers_link), - .node = node, - .references = ISC_REFCOUNT_INITIALIZER(1), - .mctx = isc_mem_ref(mctx), - .lrulink = ISC_LINK_INITIALIZER, - }; - -#if DNS_SLABHEADER_TRACE - fprintf(stderr, - "%s:%s:%s:%u:t%" PRItid ":%p->references = %" PRIuFAST32 "\n", - __func__, func, file, line, isc_tid(), h, h->references); -#else - UNUSED(func); - UNUSED(file); - UNUSED(line); -#endif - - return h; -} - static void slabheader_destroy(dns_slabheader_t *header) { - unsigned int size; - - if (EXISTS(header)) { - size = dns_rdataslab_size(header); - } else { - size = sizeof(*header); - } + unsigned int size = dns_rdataslab_size(header); if (header->noqname != NULL) { dns_slabheader_freeproof(header->mctx, &header->noqname); diff --git a/lib/dns/rdataslab_p.h b/lib/dns/rdataslab_p.h index 0f0c9604a59..88a0eb63e91 100644 --- a/lib/dns/rdataslab_p.h +++ b/lib/dns/rdataslab_p.h @@ -23,9 +23,6 @@ #define CASESET(header) \ ((atomic_load_acquire(&(header)->attributes) & \ DNS_SLABHEADERATTR_CASESET) != 0) -#define EXISTS(header) \ - ((atomic_load_acquire(&(header)->attributes) & \ - DNS_SLABHEADERATTR_NONEXISTENT) == 0) #define IGNORE(header) \ ((atomic_load_acquire(&(header)->attributes) & \ DNS_SLABHEADERATTR_IGNORE) != 0)