In dns_slabheader_t structure, change .ttl to .expire

The old name was misleading as it never meant time-to-live, e.g. number
of seconds from now when the header should expire.  The true meaning was
an expiration time e.g. now + ttl.  This was the original design bug
that caused the slip when we assigned header->ttl to rdataset->ttl.
Because the name was matching, nobody has questioned the correctness of
the code both during the MR review and during the numerous re-reviews
when we were searching for the cause of the 54 year TTL.

diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h
index 82660b5d9d628cbd5141c4f4b28704043a2aef11..ff2057808f4b0cc3e3d9c13c20218098680dfdb2 100644 (file)
--- a/lib/dns/include/dns/rdataslab.h
+++ b/lib/dns/include/dns/rdataslab.h
@@ -72,7 +72,7 @@ struct dns_slabheader {
         */
        dns_trust_t    trust;
        uint32_t       serial;
-       dns_ttl_t      ttl;
+       isc_stdtime_t  expire;
        dns_typepair_t type;
 
        _Atomic(uint16_t) count;

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 566490bc710accd125083aaffe15c2e01748343b..589ddd0aa118db9db5cfee478119a57defa491a3 100644 (file)
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -108,8 +108,9 @@
 #define STALE_TTL(header, qpdb) \
        (NXDOMAIN(header) ? 0 : qpdb->common.serve_stale_ttl)
 
-#define ACTIVE(header, now) \
-       (((header)->ttl > (now)) || ((header)->ttl == (now) && ZEROTTL(header)))
+#define ACTIVE(header, now)            \
+       (((header)->expire > (now)) || \
+        ((header)->expire == (now) && ZEROTTL(header)))
 
 #define EXPIREDOK(iterator) \
        (((iterator)->common.options & DNS_DB_EXPIREDOK) != 0)
@@ -925,10 +926,10 @@ mark(dns_slabheader_t *header, uint_least16_t flag) {
 }
 
 static void
-setttl(dns_slabheader_t *header, dns_ttl_t newttl) {
-       dns_ttl_t oldttl = header->ttl;
+setttl(dns_slabheader_t *header, isc_stdtime_t newts) {
+       isc_stdtime_t oldts = header->expire;
 
-       header->ttl = newttl;
+       header->expire = newts;
 
        if (header->db == NULL || !dns_db_iscache(header->db)) {
                return;
@@ -937,18 +938,17 @@ setttl(dns_slabheader_t *header, dns_ttl_t newttl) {
        /*
         * This is a cache. Adjust the heaps if necessary.
         */
-       if (header->heap == NULL || header->heap_index == 0 || newttl == oldttl)
-       {
+       if (header->heap == NULL || header->heap_index == 0 || newts == oldts) {
                return;
        }
 
-       if (newttl < oldttl) {
+       if (newts < oldts) {
                isc_heap_increased(header->heap, header->heap_index);
        } else {
                isc_heap_decreased(header->heap, header->heap_index);
        }
 
-       if (newttl == 0) {
+       if (newts == 0) {
                isc_heap_delete(header->heap, header->heap_index);
        }
 }
@@ -1049,7 +1049,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header,
         * Mark header stale or ancient if the RRset is no longer active.
         */
        if (!ACTIVE(header, now)) {
-               dns_ttl_t stale_ttl = header->ttl + STALE_TTL(header, qpdb);
+               dns_ttl_t stale_ttl = header->expire + STALE_TTL(header, qpdb);
                /*
                 * If this data is in the stale window keep it and if
                 * DNS_DBFIND_STALEOK is not set we tell the caller to
@@ -1072,7 +1072,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header,
        rdataset->rdclass = qpdb->common.rdclass;
        rdataset->type = DNS_TYPEPAIR_TYPE(header->type);
        rdataset->covers = DNS_TYPEPAIR_COVERS(header->type);
-       rdataset->ttl = header->ttl - now;
+       rdataset->ttl = header->expire - now;
        rdataset->trust = header->trust;
        rdataset->resign = 0;
 
@@ -1090,7 +1090,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header,
        }
 
        if (stale && !ancient) {
-               dns_ttl_t stale_ttl = header->ttl + STALE_TTL(header, qpdb);
+               dns_ttl_t stale_ttl = header->expire + STALE_TTL(header, qpdb);
                if (stale_ttl > now) {
                        rdataset->ttl = stale_ttl - now;
                } else {
@@ -1180,7 +1180,8 @@ check_stale_header(qpcnode_t *node, dns_slabheader_t *header,
                   isc_rwlocktype_t *nlocktypep, isc_rwlock_t *nlock,
                   qpc_search_t *search, dns_slabheader_t **header_prev) {
        if (!ACTIVE(header, search->now)) {
-               dns_ttl_t stale = header->ttl + STALE_TTL(header, search->qpdb);
+               dns_ttl_t stale = header->expire +
+                                 STALE_TTL(header, search->qpdb);
                /*
                 * If this data is in the stale window keep it and if
                 * DNS_DBFIND_STALEOK is not set we tell the caller to
@@ -1239,7 +1240,7 @@ check_stale_header(qpcnode_t *node, dns_slabheader_t *header,
                 * it as ancient, and the node as dirty, so it will get
                 * cleaned up later.
                 */
-               if ((header->ttl < search->now - QPDB_VIRTUAL) &&
+               if ((header->expire < search->now - QPDB_VIRTUAL) &&
                    (*nlocktypep == isc_rwlocktype_write ||
                     NODE_TRYUPGRADE(nlock, nlocktypep) == ISC_R_SUCCESS))
                {
@@ -2218,7 +2219,7 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
        for (header = qpnode->data; header != NULL; header = header_next) {
                header_next = header->next;
                if (!ACTIVE(header, now)) {
-                       if ((header->ttl + STALE_TTL(header, qpdb) <
+                       if ((header->expire + STALE_TTL(header, qpdb) <
                             now - QPDB_VIRTUAL) &&
                            (nlocktype == isc_rwlocktype_write ||
                             NODE_TRYUPGRADE(nlock, &nlocktype) ==
@@ -2476,7 +2477,7 @@ ttl_sooner(void *v1, void *v2) {
        dns_slabheader_t *h1 = v1;
        dns_slabheader_t *h2 = v2;
 
-       return h1->ttl < h2->ttl;
+       return h1->expire < h2->expire;
 }
 
 /*%
@@ -3019,8 +3020,8 @@ find_header:
                         * Honour the new ttl if it is less than the
                         * older one.
                         */
-                       if (header->ttl > newheader->ttl) {
-                               setttl(header, newheader->ttl);
+                       if (header->expire > newheader->expire) {
+                               setttl(header, newheader->expire);
                        }
                        if (header->last_used != now) {
                                ISC_LIST_UNLINK(
@@ -3061,8 +3062,8 @@ find_header:
                    !header_nx && !newheader_nx &&
                    header->trust <= newheader->trust)
                {
-                       if (newheader->ttl > header->ttl) {
-                               newheader->ttl = header->ttl;
+                       if (newheader->expire > header->expire) {
+                               newheader->expire = header->expire;
                        }
                }
                if (ACTIVE(header, now) &&
@@ -3081,8 +3082,8 @@ find_header:
                         * Honour the new ttl if it is less than the
                         * older one.
                         */
-                       if (header->ttl > newheader->ttl) {
-                               setttl(header, newheader->ttl);
+                       if (header->expire > newheader->expire) {
+                               setttl(header, newheader->expire);
                        }
                        if (header->last_used != now) {
                                ISC_LIST_UNLINK(
@@ -3726,7 +3727,7 @@ rdatasetiter_destroy(dns_rdatasetiter_t **iteratorp DNS__DB_FLARG) {
 static bool
 iterator_active(qpcache_t *qpdb, qpc_rditer_t *iterator,
                dns_slabheader_t *header) {
-       dns_ttl_t stale_ttl = header->ttl + STALE_TTL(header, qpdb);
+       dns_ttl_t stale_ttl = header->expire + STALE_TTL(header, qpdb);
 
        /*
         * Is this a "this rdataset doesn't exist" record?
@@ -4283,7 +4284,7 @@ expire_ttl_headers(qpcache_t *qpdb, unsigned int locknum,
                        return;
                }
 
-               dns_ttl_t ttl = header->ttl;
+               dns_ttl_t ttl = header->expire;
 
                if (!cache_is_overmem) {
                        /* Only account for stale TTL if cache is not overmem */

diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index 96282b78522fa72ef6c459303622a6f407dfd135..9771163fdc7cd0cf42de86ffbab4fd99fbeef9ce 100644 (file)
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -985,7 +985,7 @@ bindrdataset(qpzonedb_t *qpdb, qpznode_t *node, dns_slabheader_t *header,
        rdataset->rdclass = qpdb->common.rdclass;
        rdataset->type = DNS_TYPEPAIR_TYPE(header->type);
        rdataset->covers = DNS_TYPEPAIR_COVERS(header->type);
-       rdataset->ttl = header->ttl - now;
+       rdataset->ttl = header->expire - now;
        rdataset->trust = header->trust;
 
        if (OPTOUT(header)) {
@@ -1890,10 +1890,10 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
                                flags |= DNS_RDATASLAB_EXACT;
                        }
                        if ((options & DNS_DBADD_EXACTTTL) != 0 &&
-                           newheader->ttl != header->ttl)
+                           newheader->expire != header->expire)
                        {
                                result = DNS_R_NOTEXACT;
-                       } else if (newheader->ttl != header->ttl) {
+                       } else if (newheader->expire != header->expire) {
                                flags |= DNS_RDATASLAB_FORCE;
                        }
                        if (result == ISC_R_SUCCESS) {
@@ -2181,7 +2181,7 @@ loading_addrdataset(void *arg, const dns_name_t *name,
        newheader = (dns_slabheader_t *)region.base;
        *newheader = (dns_slabheader_t){
                .type = DNS_TYPEPAIR_VALUE(rdataset->type, rdataset->covers),
-               .ttl = rdataset->ttl + loadctx->now,
+               .expire = rdataset->ttl + loadctx->now,
                .trust = rdataset->trust,
                .node = (dns_dbnode_t *)node,
                .serial = 1,
@@ -4687,7 +4687,7 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
        };
 
        dns_slabheader_reset(newheader, db, (dns_dbnode_t *)node);
-       newheader->ttl = rdataset->ttl;
+       newheader->expire = rdataset->ttl;
        if (rdataset->ttl == 0U) {
                DNS_SLABHEADER_SETATTR(newheader, DNS_SLABHEADERATTR_ZEROTTL);
        }
@@ -4802,7 +4802,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 
        newheader = (dns_slabheader_t *)region.base;
        dns_slabheader_reset(newheader, db, (dns_dbnode_t *)node);
-       newheader->ttl = rdataset->ttl;
+       newheader->expire = rdataset->ttl;
        newheader->type = DNS_TYPEPAIR_VALUE(rdataset->type, rdataset->covers);
        atomic_init(&newheader->attributes, 0);
        newheader->serial = version->serial;
@@ -4852,7 +4852,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
                result = ISC_R_SUCCESS;
                if ((options & DNS_DBSUB_EXACT) != 0) {
                        flags |= DNS_RDATASLAB_EXACT;
-                       if (newheader->ttl != header->ttl) {
+                       if (newheader->expire != header->expire) {
                                result = DNS_R_NOTEXACT;
                        }
                }
@@ -4899,7 +4899,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
                        dns_slabheader_destroy(&newheader);
                        newheader = dns_slabheader_new((dns_db_t *)qpdb,
                                                       (dns_dbnode_t *)node);
-                       newheader->ttl = 0;
+                       newheader->expire = 0;
                        newheader->type = topheader->type;
                        atomic_init(&newheader->attributes,
                                    DNS_SLABHEADERATTR_NONEXISTENT);
@@ -4983,7 +4983,7 @@ qpzone_deleterdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 
        newheader = dns_slabheader_new(db, (dns_dbnode_t *)node);
        newheader->type = DNS_TYPEPAIR_VALUE(type, covers);
-       newheader->ttl = 0;
+       newheader->expire = 0;
        atomic_init(&newheader->attributes, DNS_SLABHEADERATTR_NONEXISTENT);
        newheader->serial = version->serial;