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 baf9486ba100d63ee49c4c88578ff721c6be6887..f97eb3dd8c6c36b7db11b4d6d96411ff687e7ade 100644 (file)
--- 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 5dae1d938024be35f0cf83cb8e1fab6cfed86ab2..85ceafc50b9d21a9d9e7cb0a84491b0747627117 100644 (file)
--- 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 34bfedda82cf23e5809d00aac4de84bbf9c38041..c84039da891fbc79f0d57e523570e1b3a556db66 100644 (file)
--- 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 e87be113b2239eb4f147464a9f8c86ec42a0d4b9..107a1df4e50beb6dbcfa82b095b7ba0e86996674 100644 (file)
--- 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 0f0c9604a5951941bfe3d54680bca905309a57ee..88a0eb63e91cd5083b2b5a9378fb768b6f1887ae 100644 (file)
--- 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)