From: Ondřej Surý Date: Fri, 19 Jun 2026 13:43:42 +0000 (+0200) Subject: Remove the ANCIENT slabheader attribute and statistics counter X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=06abfa32642630a8639e54ce22ca972ec496e793;p=thirdparty%2Fbind9.git Remove the ANCIENT slabheader attribute and statistics counter Cache headers are now unlinked from their node as soon as they expire, so a slabheader is never left in the "ancient, awaiting cleanup" state that DNS_SLABHEADERATTR_ANCIENT tracked. Drop the attribute, rename mark_ancient() to header_delete() to reflect that it now removes the header rather than flagging it (keying idempotency on list membership), and remove the ancient RRset statistics counter that recorded the state, which is now always zero; the rdataset statistics array shrinks to match. The rdataset-level 'ancient' flag and 'rndc dumpdb -expired' are unaffected: expiry is derived from the entry's TTL when the rdataset is bound, not from the slabheader attribute. --- diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index ede4eff2cf..76b4ef27dd 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -1139,7 +1139,6 @@ rdatasetstats_dump(dns_rdatastatstype_t type, uint64_t val, void *arg) { const char *typestr; bool nxrrset = false; bool stale = false; - bool ancient = false; #ifdef HAVE_LIBXML2 void *writer; int xmlrc; @@ -1165,13 +1164,12 @@ rdatasetstats_dump(dns_rdatastatstype_t type, uint64_t val, void *arg) { nxrrset = rdatastatstype_attr(type, DNS_RDATASTATSTYPE_ATTR_NXRRSET); stale = rdatastatstype_attr(type, DNS_RDATASTATSTYPE_ATTR_STALE); - ancient = rdatastatstype_attr(type, DNS_RDATASTATSTYPE_ATTR_ANCIENT); switch (dumparg->type) { case isc_statsformat_file: fp = dumparg->arg; - fprintf(fp, "%20" PRIu64 " %s%s%s%s\n", val, ancient ? "~" : "", - stale ? "#" : "", nxrrset ? "!" : "", typestr); + fprintf(fp, "%20" PRIu64 " %s%s%s\n", val, stale ? "#" : "", + nxrrset ? "!" : "", typestr); break; case isc_statsformat_xml: #ifdef HAVE_LIBXML2 @@ -1180,8 +1178,8 @@ rdatasetstats_dump(dns_rdatastatstype_t type, uint64_t val, void *arg) { TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "rrset")); TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "name")); TRY0(xmlTextWriterWriteFormatString( - writer, "%s%s%s%s", ancient ? "~" : "", - stale ? "#" : "", nxrrset ? "!" : "", typestr)); + writer, "%s%s%s", stale ? "#" : "", nxrrset ? "!" : "", + typestr)); TRY0(xmlTextWriterEndElement(writer)); /* name */ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "counter")); @@ -1194,8 +1192,8 @@ rdatasetstats_dump(dns_rdatastatstype_t type, uint64_t val, void *arg) { case isc_statsformat_json: #ifdef HAVE_JSON_C zoneobj = (json_object *)dumparg->arg; - snprintf(buf, sizeof(buf), "%s%s%s%s", ancient ? "~" : "", - stale ? "#" : "", nxrrset ? "!" : "", typestr); + snprintf(buf, sizeof(buf), "%s%s%s", stale ? "#" : "", + nxrrset ? "!" : "", typestr); obj = json_object_new_int64(val); if (obj == NULL) { return; diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h index 4c6f1b4f01..8eeaad3b60 100644 --- a/lib/dns/include/dns/rdataslab.h +++ b/lib/dns/include/dns/rdataslab.h @@ -148,8 +148,7 @@ enum { DNS_SLABHEADERATTR_CASESET = 1 << 9, DNS_SLABHEADERATTR_ZEROTTL = 1 << 10, DNS_SLABHEADERATTR_CASEFULLYLOWER = 1 << 11, - DNS_SLABHEADERATTR_ANCIENT = 1 << 12, - DNS_SLABHEADERATTR_STALE_WINDOW = 1 << 13, + DNS_SLABHEADERATTR_STALE_WINDOW = 1 << 12, }; /* clang-format off : RemoveParentheses */ diff --git a/lib/dns/include/dns/stats.h b/lib/dns/include/dns/stats.h index e4a621e731..f027cdc1e6 100644 --- a/lib/dns/include/dns/stats.h +++ b/lib/dns/include/dns/stats.h @@ -235,16 +235,13 @@ enum { * _STALE * RRset type counters only. This indicates a record that is stale * but may still be served. - * - * _ANCIENT - * RRset type counters only. This indicates a record that is marked for - * removal. */ -#define DNS_RDATASTATSTYPE_ATTR_OTHERTYPE 0x0001 -#define DNS_RDATASTATSTYPE_ATTR_NXRRSET 0x0002 -#define DNS_RDATASTATSTYPE_ATTR_NXDOMAIN 0x0004 -#define DNS_RDATASTATSTYPE_ATTR_STALE 0x0008 -#define DNS_RDATASTATSTYPE_ATTR_ANCIENT 0x0010 +enum { + DNS_RDATASTATSTYPE_ATTR_OTHERTYPE = 1 << 0, + DNS_RDATASTATSTYPE_ATTR_NXRRSET = 1 << 1, + DNS_RDATASTATSTYPE_ATTR_NXDOMAIN = 1 << 2, + DNS_RDATASTATSTYPE_ATTR_STALE = 1 << 3, +}; /*%< * Conversion macros among dns_rdatatype_t, attributes and isc_statscounter_t. diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index 671f33cf55..af077a8666 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -71,7 +71,7 @@ struct dns_master_style { /*% Does the rdataset 'r' contain a stale answer? */ #define STALE(r) (((r)->attributes.stale)) -/*% Does the rdataset 'r' contain an expired answer? */ +/*% Is the rdataset 'r' expired and awaiting cleanup? */ #define ANCIENT(r) (((r)->attributes.ancient)) /*% diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 7e423a93d5..3007f79609 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -438,8 +438,6 @@ expire_lru_headers(qpcache_t *qpdb, uint32_t idx, size_t requested, dns_slabheader_t *related = header->related; - ISC_SIEVE_UNLINK(qpdb->buckets[idx].sieve, header, lrulink); - expired += expireheader(header, nlocktypep, tlocktypep, dns_expire_lru DNS__DB_FLARG_PASS); @@ -492,9 +490,6 @@ qpcache_hit(qpcache_t *qpdb ISC_ATTR_UNUSED, dns_slabheader_t *header) { * DB Routines */ -static void -header_cleanup(qpcnode_t *node, dns_slabheader_t *header); - /* * tree_lock(write) must be held. */ @@ -732,9 +727,6 @@ update_rrsetstats(dns_stats_t *stats, const dns_typepair_t typepair, if (STALE(header)) { statattributes |= DNS_RDATASTATSTYPE_ATTR_STALE; } - if (ANCIENT(header)) { - statattributes |= DNS_RDATASTATSTYPE_ATTR_ANCIENT; - } type = DNS_RDATASTATSTYPE_VALUE(base, statattributes); if (increment) { @@ -777,14 +769,30 @@ setttl(dns_slabheader_t *header, isc_stdtime_t newts) { } static void -mark_ancient(qpcnode_t *node, dns_slabheader_t *header) { - if (ANCIENT(header)) { +header_delete(qpcnode_t *node, dns_slabheader_t *header) { + /* The slabheader has already been removed from the node headers */ + if (cds_list_empty(&header->headers_link)) { return; } - setttl(header, 0); - mark(header, DNS_SLABHEADERATTR_ANCIENT); - header_cleanup(node, header); + qpcache_t *qpdb = node->qpdb; + + cds_list_del_init(&header->headers_link); + + /* + * This place is the only place where we actually need header->typepair. + */ + update_rrsetstats(qpdb->rrsetstats, header->typepair, + atomic_load_acquire(&header->attributes), false); + + ISC_SIEVE_UNLINK(qpdb->buckets[node->locknum].sieve, header, lrulink); + + if (header->related != NULL) { + INSIST(header->related->related == header); + header->related->related = NULL; + header->related = NULL; + } + dns_slabheader_detach(&header); } @@ -797,7 +805,7 @@ expireheader(dns_slabheader_t *header, isc_rwlocktype_t *nlocktypep, size_t expired = rdataset_size(header); qpcnode_t *node = HEADERNODE(header); - mark_ancient(node, header); + header_delete(node, header); if (isc_refcount_current(&node->erefs) == 0) { qpcache_t *qpdb = node->qpdb; @@ -861,7 +869,6 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, isc_rwlocktype_t tlocktype, dns_rdataset_t *rdataset DNS__DB_FLARG) { bool stale = STALE(header); - bool ancient = ANCIENT(header); /* * Caller must be holding the node reader lock. @@ -882,7 +889,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, INSIST(rdataset->methods == NULL); /* We must be disassociated. */ /* - * Mark header stale or ancient if the RRset is no longer active. + * Mark header stale if the RRset is no longer active. */ if (!ACTIVE(header, now)) { dns_ttl_t stale_ttl = header->expire + STALE_TTL(header, qpdb); @@ -895,12 +902,6 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, if (!ZEROTTL(header) && KEEPSTALE(qpdb) && stale_ttl > now) { stale = true; - } else { - /* - * We are not keeping stale, or it is outside the - * stale window. Mark ancient, i.e. ready for cleanup. - */ - ancient = true; } } @@ -932,7 +933,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, rdataset->attributes.prefetch = true; } - if (stale && !ancient) { + if (stale) { dns_ttl_t stale_ttl = header->expire + STALE_TTL(header, qpdb); if (stale_ttl > now) { rdataset->ttl = stale_ttl - now; @@ -945,6 +946,11 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, rdataset->attributes.stale = true; rdataset->expire = header->expire; } else if (!ACTIVE(header, now)) { + /* + * The entry is expired but still present in the cache (it has + * not yet been removed); flag it so that, e.g., a cache dump + * including expired entries can mark it. + */ rdataset->attributes.ancient = true; rdataset->ttl = 0; } @@ -1086,7 +1092,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) || ANCIENT(header); + !EXISTS(header); } /* @@ -1131,7 +1137,7 @@ related_headers(dns_slabheader_t *header, dns_slabheader_t *sigheader, return false; } - REQUIRE(EXISTS(sigheader) && !ANCIENT(sigheader)); + REQUIRE(EXISTS(sigheader)); if (sigheader->typepair == typepair) { *foundp = sigheader; *foundsigp = NULL; @@ -1143,7 +1149,7 @@ related_headers(dns_slabheader_t *header, dns_slabheader_t *sigheader, return false; } - REQUIRE(EXISTS(header) && !ANCIENT(header)); + REQUIRE(EXISTS(header)); REQUIRE(!NEGATIVE(header) || sigheader == NULL); if (header->typepair == typepair) { @@ -1508,7 +1514,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, /* * We now know that there is at least one active - * non-stale rdataset at this node. + * rdataset at this node. */ empty_node = false; @@ -1862,7 +1868,7 @@ qpcnode_expiredata(dns_dbnode_t *node, void *data) { isc_rwlock_t *nlock = &qpdb->buckets[qpnode->locknum].lock; NODE_WRLOCK(nlock, &nlocktype); - if (!ANCIENT(header)) { + if (!cds_list_empty(&header->headers_link)) { dns_slabheader_t *related = header->related; (void)expireheader(header, &nlocktype, &tlocktype, @@ -2240,7 +2246,7 @@ check_ncache_block(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *header, * bind to it and leave the cache unchanged. */ if (trust >= header->trust) { - mark_ancient(qpnode, header); + header_delete(qpnode, header); return DNS_R_CONTINUE; } else { qpcache_hit(qpdb, header); @@ -2293,23 +2299,23 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, * covers all types (NXDOMAIN, * NODATA(QTYPE=ANY)). * - * Make all other data ancient so that the only + * Delete all other data so that the only * rdataset that can be found at this node is * the negative cache entry. */ - mark_ancient(qpnode, header); + header_delete(qpnode, header); continue; } else if (rdtype == dns_rdatatype_rrsig) { /* * We're adding a proof that a signature doesn't * exist. * - * Mark all existing signatures as ancient. + * Delete all existing signatures. */ if (DNS_TYPEPAIR_TYPE(header->typepair) == dns_rdatatype_rrsig) { - mark_ancient(qpnode, header); + header_delete(qpnode, header); continue; } } @@ -2337,7 +2343,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, } if (check_stale_header(header, &search)) { - mark_ancient(qpnode, header); + header_delete(qpnode, header); continue; } @@ -2515,7 +2521,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, } INSIST(oldheader->related == related); - mark_ancient(qpnode, oldheader); + header_delete(qpnode, oldheader); } else if (!EXISTS(newheader)) { /* @@ -2560,21 +2566,21 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, if (evictheader != NULL) { INSIST(evictheader->related != newheader); if (evictheader->related != NULL) { - mark_ancient(qpnode, evictheader->related); + header_delete(qpnode, evictheader->related); } - mark_ancient(qpnode, evictheader); + header_delete(qpnode, evictheader); } } /* * We've added a proof that a rdtype doesn't exist. * - * Mark the related rrsig in the cache as ancient. + * Delete the related rrsig in the cache. */ if (EXISTS(newheader) && NEGATIVE(newheader) && !dns_rdatatype_issig(rdtype) && related != NULL) { - mark_ancient(qpnode, related); + header_delete(qpnode, related); } return ISC_R_SUCCESS; @@ -3284,30 +3290,6 @@ dbiterator_origin(dns_dbiterator_t *iterator, dns_name_t *name) { return ISC_R_SUCCESS; } -static void -header_cleanup(qpcnode_t *node, dns_slabheader_t *header) { - qpcache_t *qpdb = node->qpdb; - - cds_list_del_init(&header->headers_link); - - /* - * This place is the only place where we actually need header->typepair. - */ - update_rrsetstats(qpdb->rrsetstats, header->typepair, - atomic_load_acquire(&header->attributes), false); - - if (ISC_SIEVE_LINKED(header, lrulink)) { - ISC_SIEVE_UNLINK(qpdb->buckets[node->locknum].sieve, header, - lrulink); - } - - if (header->related != NULL) { - INSIST(header->related->related == header); - header->related->related = NULL; - header->related = NULL; - } -} - static void setmaxrrperset(dns_db_t *db, uint32_t value) { qpcache_t *qpdb = (qpcache_t *)db; @@ -3352,8 +3334,7 @@ qpcnode_destroy(qpcnode_t *qpnode) { cds_list_for_each_entry_safe(header, header_next, &qpnode->headers, headers_link) { - header_cleanup(qpnode, header); - dns_slabheader_detach(&header); + header_delete(qpnode, header); } dns_name_free(&qpnode->name, qpnode->mctx); diff --git a/lib/dns/rdataslab_p.h b/lib/dns/rdataslab_p.h index 5ce39fbbc6..0f0c9604a5 100644 --- a/lib/dns/rdataslab_p.h +++ b/lib/dns/rdataslab_p.h @@ -17,9 +17,6 @@ #include -#define ANCIENT(header) \ - ((atomic_load_acquire(&(header)->attributes) & \ - DNS_SLABHEADERATTR_ANCIENT) != 0) #define CASEFULLYLOWER(header) \ ((atomic_load_acquire(&(header)->attributes) & \ DNS_SLABHEADERATTR_CASEFULLYLOWER) != 0) diff --git a/lib/dns/stats.c b/lib/dns/stats.c index cb60fcd10f..cb66392540 100644 --- a/lib/dns/stats.c +++ b/lib/dns/stats.c @@ -51,51 +51,43 @@ typedef enum { * * 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 * +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ - * | | | | | | S |NX| RRType | + * | | | | | |ND| S|NN| RRType | * +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ * * If the 8 bits for RRtype are all zero, this is an Other RRtype. */ -#define RDTYPECOUNTER_MAXTYPE 0x00ff +#define RDTYPECOUNTER_MAXTYPE ((1 << 8) - 1) /* * * Bit 7 is the NXRRSET (NX) flag and indicates whether this is a * positive (0) or a negative (1) RRset. */ -#define RDTYPECOUNTER_NXRRSET 0x0100 +#define RDTYPECOUNTER_NXRRSET (1 << 8) /* - * Then bit 5 and 6 mostly tell you if this counter is for an active, - * stale, or ancient RRtype: + * Then bit 6 tells you if this counter is for an active, + * or stale RRtype: * * S = 0 (0b00) means Active * S = 1 (0b01) means Stale - * S = 2 (0b10) means Ancient - * - * Since a counter cannot be stale and ancient at the same time, we - * treat S = 0b11 as a special case to deal with NXDOMAIN counters. */ -#define RDTYPECOUNTER_STALE (1 << 9) -#define RDTYPECOUNTER_ANCIENT (1 << 10) -#define RDTYPECOUNTER_NXDOMAIN ((1 << 9) | (1 << 10)) +#define RDTYPECOUNTER_STALE (1 << 9) /* - * S = 0b11 indicates an NXDOMAIN counter and in this case the RRtype - * field signals the expiry of this cached item: - * - * RRType = 0 (0b00) means Active - * RRType = 1 (0b01) means Stale - * RRType = 2 (0b02) means Ancient + * Bit 5 tells you if this is NXDOMAIN counter. * + * If NXDOMAIN bit is set, we use Bit 15 as Stale indicator to save extra memory + * that we would need to store RDTYPECOUNTER_NXDOMAIN | RDTYPECOUNTER_STALE. */ -#define RDTYPECOUNTER_NXDOMAIN_STALE 1 -#define RDTYPECOUNTER_NXDOMAIN_ANCIENT 2 +#define RDTYPECOUNTER_NXDOMAIN (1 << 10) +#define RDTYPECOUNTER_NXDOMAIN_STALE (1 << 0) /* - * The maximum value for rdtypecounter is for an ancient NXDOMAIN. + * The maximum value for rdtypecounter is for a stale NXDOMAIN. */ -#define RDTYPECOUNTER_MAXVAL 0x0602 +#define RDTYPECOUNTER_MAXVAL \ + (RDTYPECOUNTER_NXDOMAIN | RDTYPECOUNTER_NXDOMAIN_STALE) /* * DNSSEC sign statistics. @@ -270,16 +262,12 @@ update_rdatasetstats(dns_stats_t *stats, dns_rdatastatstype_t rrsettype, /* * This is an NXDOMAIN counter, save the expiry value - * (active, stale, or ancient) value in the RRtype part. + * (active, or stale) value in the RRtype part. */ if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) & - DNS_RDATASTATSTYPE_ATTR_ANCIENT) != 0) - { - counter |= RDTYPECOUNTER_NXDOMAIN_ANCIENT; - } else if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) & - DNS_RDATASTATSTYPE_ATTR_STALE) != 0) + DNS_RDATASTATSTYPE_ATTR_STALE) != 0) { - counter += RDTYPECOUNTER_NXDOMAIN_STALE; + counter |= RDTYPECOUNTER_NXDOMAIN_STALE; } } else { counter = rdatatype2counter(DNS_RDATASTATSTYPE_BASE(rrsettype)); @@ -291,11 +279,7 @@ update_rdatasetstats(dns_stats_t *stats, dns_rdatastatstype_t rrsettype, } if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) & - DNS_RDATASTATSTYPE_ATTR_ANCIENT) != 0) - { - counter |= RDTYPECOUNTER_ANCIENT; - } else if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) & - DNS_RDATASTATSTYPE_ATTR_STALE) != 0) + DNS_RDATASTATSTYPE_ATTR_STALE) != 0) { counter |= RDTYPECOUNTER_STALE; } @@ -471,21 +455,17 @@ rdataset_dumpcb(isc_statscounter_t counter, uint64_t value, void *arg) { rdatadumparg_t *rdatadumparg = arg; unsigned int attributes = 0; - if ((counter & RDTYPECOUNTER_NXDOMAIN) == RDTYPECOUNTER_NXDOMAIN) { + if ((counter & RDTYPECOUNTER_NXDOMAIN) != 0) { attributes |= DNS_RDATASTATSTYPE_ATTR_NXDOMAIN; /* * This is an NXDOMAIN counter, check the RRtype part for the - * expiry value (active, stale, or ancient). + * expiry value (active, or stale). */ if ((counter & RDTYPECOUNTER_MAXTYPE) == RDTYPECOUNTER_NXDOMAIN_STALE) { attributes |= DNS_RDATASTATSTYPE_ATTR_STALE; - } else if ((counter & RDTYPECOUNTER_MAXTYPE) == - RDTYPECOUNTER_NXDOMAIN_ANCIENT) - { - attributes |= DNS_RDATASTATSTYPE_ATTR_ANCIENT; } } else { if ((counter & RDTYPECOUNTER_MAXTYPE) == 0) { @@ -497,8 +477,6 @@ rdataset_dumpcb(isc_statscounter_t counter, uint64_t value, void *arg) { if ((counter & RDTYPECOUNTER_STALE) != 0) { attributes |= DNS_RDATASTATSTYPE_ATTR_STALE; - } else if ((counter & RDTYPECOUNTER_ANCIENT) != 0) { - attributes |= DNS_RDATASTATSTYPE_ATTR_ANCIENT; } } diff --git a/tests/dns/rdatasetstats_test.c b/tests/dns/rdatasetstats_test.c index 31b7a6ea5f..cbc2d65f76 100644 --- a/tests/dns/rdatasetstats_test.c +++ b/tests/dns/rdatasetstats_test.c @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -32,231 +33,130 @@ #include -static void -set_typestats(dns_stats_t *stats, dns_rdatatype_t type) { - dns_rdatastatstype_t which; - unsigned int attributes; +/* + * Each rdataset statistics counter is keyed on an RRtype plus a few + * attribute bits. The interesting combinations are the cross product of + * + * - positive RRset / NXRRSET (NODATA) / NXDOMAIN, and + * - active / stale, + * + * and the property under test is that every combination maps to its own + * counter (no aliasing) and round-trips through dns_rdatasetstats_dump(). + * + * The helpers below drive the counters from the active to the stale state; + * the verify callback then dumps *every* counter (ISC_STATSDUMP_VERBOSE) and + * asserts that exactly the counters in the expected state hold 1 and all + * others hold 0. An aliasing bug shows up as a counter holding 2 (two + * states folded onto one slot) or 0 (a state that landed on the wrong slot). + */ - attributes = 0; - which = DNS_RDATASTATSTYPE_VALUE(type, attributes); - dns_rdatasetstats_increment(stats, which); +/* A type that does not fit in 8 bits and folds onto the 'other' counter. */ +#define OTHERTYPE ((dns_rdatatype_t)1000) - attributes = DNS_RDATASTATSTYPE_ATTR_NXRRSET; - which = DNS_RDATASTATSTYPE_VALUE(type, attributes); - dns_rdatasetstats_increment(stats, which); +static void +incr(dns_stats_t *stats, dns_rdatatype_t type, unsigned int attr) { + dns_rdatasetstats_increment(stats, + DNS_RDATASTATSTYPE_VALUE(type, attr)); } static void -set_nxdomainstats(dns_stats_t *stats) { - dns_rdatastatstype_t which; - unsigned int attributes; - - attributes = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN; - which = DNS_RDATASTATSTYPE_VALUE(0, attributes); - dns_rdatasetstats_increment(stats, which); +decr(dns_stats_t *stats, dns_rdatatype_t type, unsigned int attr) { + dns_rdatasetstats_decrement(stats, + DNS_RDATASTATSTYPE_VALUE(type, attr)); } +/* Set the active positive and active NXRRSET counters for 'type'. */ static void -mark_stale(dns_stats_t *stats, dns_rdatatype_t type, int from, int to) { - dns_rdatastatstype_t which; - unsigned int attributes; - - attributes = from; - which = DNS_RDATASTATSTYPE_VALUE(type, attributes); - dns_rdatasetstats_decrement(stats, which); - - attributes |= to; - which = DNS_RDATASTATSTYPE_VALUE(type, attributes); - dns_rdatasetstats_increment(stats, which); - - attributes = DNS_RDATASTATSTYPE_ATTR_NXRRSET | from; - which = DNS_RDATASTATSTYPE_VALUE(type, attributes); - dns_rdatasetstats_decrement(stats, which); - - attributes |= to; - which = DNS_RDATASTATSTYPE_VALUE(type, attributes); - dns_rdatasetstats_increment(stats, which); +set_active(dns_stats_t *stats, dns_rdatatype_t type) { + incr(stats, type, 0); + incr(stats, type, DNS_RDATASTATSTYPE_ATTR_NXRRSET); } +/* Set the active NXDOMAIN counter. */ static void -mark_nxdomain_stale(dns_stats_t *stats, int from, int to) { - dns_rdatastatstype_t which; - unsigned int attributes; - - attributes = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN | from; - which = DNS_RDATASTATSTYPE_VALUE(0, attributes); - dns_rdatasetstats_decrement(stats, which); - - attributes |= to; - which = DNS_RDATASTATSTYPE_VALUE(0, attributes); - dns_rdatasetstats_increment(stats, which); +set_active_nxdomain(dns_stats_t *stats) { + incr(stats, 0, DNS_RDATASTATSTYPE_ATTR_NXDOMAIN); } -#define ATTRIBUTE_SET(y) ((attributes & (y)) != 0) +/* Move the positive and NXRRSET counters for 'type' from active to stale. */ static void -verify_active_counters(dns_rdatastatstype_t which, uint64_t value, void *arg) { - unsigned int attributes; -#if debug - unsigned int type; -#endif /* if debug */ - - UNUSED(which); - UNUSED(arg); - - attributes = DNS_RDATASTATSTYPE_ATTR(which); -#if debug - type = DNS_RDATASTATSTYPE_BASE(which); - - fprintf(stderr, "%s%s%s%s%s/%u, %u\n", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_OTHERTYPE) ? "O" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXRRSET) ? "!" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_ANCIENT) ? "~" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_STALE) ? "#" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXDOMAIN) ? "X" : " ", - type, (unsigned int)value); -#endif /* if debug */ - if ((attributes & DNS_RDATASTATSTYPE_ATTR_ANCIENT) == 0 && - (attributes & DNS_RDATASTATSTYPE_ATTR_STALE) == 0) - { - assert_int_equal(value, 1); - } else { - assert_int_equal(value, 0); - } +mark_stale(dns_stats_t *stats, dns_rdatatype_t type) { + unsigned int nx = DNS_RDATASTATSTYPE_ATTR_NXRRSET; + unsigned int stale = DNS_RDATASTATSTYPE_ATTR_STALE; + + decr(stats, type, 0); + incr(stats, type, stale); + decr(stats, type, nx); + incr(stats, type, nx | stale); } +/* Move the NXDOMAIN counter from active to stale. */ static void -verify_stale_counters(dns_rdatastatstype_t which, uint64_t value, void *arg) { - unsigned int attributes; -#if debug - unsigned int type; -#endif /* if debug */ - - UNUSED(which); - UNUSED(arg); +mark_stale_nxdomain(dns_stats_t *stats) { + unsigned int nxd = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN; + unsigned int stale = DNS_RDATASTATSTYPE_ATTR_STALE; - attributes = DNS_RDATASTATSTYPE_ATTR(which); -#if debug - type = DNS_RDATASTATSTYPE_BASE(which); - - fprintf(stderr, "%s%s%s%s%s/%u, %u\n", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_OTHERTYPE) ? "O" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXRRSET) ? "!" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_ANCIENT) ? "~" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_STALE) ? "#" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXDOMAIN) ? "X" : " ", - type, (unsigned int)value); -#endif /* if debug */ - if ((attributes & DNS_RDATASTATSTYPE_ATTR_STALE) != 0) { - assert_int_equal(value, 1); - } else { - assert_int_equal(value, 0); - } + decr(stats, 0, nxd); + incr(stats, 0, nxd | stale); } +/* + * Assert that a counter holds 1 exactly when its staleness matches the + * expected state passed via 'arg' (0 for active, DNS_RDATASTATSTYPE_ATTR_STALE + * for stale), and 0 otherwise. + */ static void -verify_ancient_counters(dns_rdatastatstype_t which, uint64_t value, void *arg) { - unsigned int attributes; -#if debug - unsigned int type; -#endif /* if debug */ - - UNUSED(which); - UNUSED(arg); - - attributes = DNS_RDATASTATSTYPE_ATTR(which); -#if debug - type = DNS_RDATASTATSTYPE_BASE(which); +verify_counters(dns_rdatastatstype_t which, uint64_t value, void *arg) { + unsigned int attributes = DNS_RDATASTATSTYPE_ATTR(which); + unsigned int expected = *(unsigned int *)arg; - fprintf(stderr, "%s%s%s%s%s/%u, %u\n", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_OTHERTYPE) ? "O" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXRRSET) ? "!" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_ANCIENT) ? "~" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_STALE) ? "#" : " ", - ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXDOMAIN) ? "X" : " ", - type, (unsigned int)value); -#endif /* if debug */ - if ((attributes & DNS_RDATASTATSTYPE_ATTR_ANCIENT) != 0) { + if ((attributes & DNS_RDATASTATSTYPE_ATTR_STALE) == expected) { assert_int_equal(value, 1); } else { assert_int_equal(value, 0); } } -/* - * Individual unit tests - */ /* - * Test that rdatasetstats counters are properly set when moving from - * active -> stale -> ancient. + * Populate every counter active, verify, transition all to stale, verify. */ -static void -rdatasetstats(void **state ISC_ATTR_UNUSED, bool servestale) { - unsigned int i; - unsigned int from = 0; +ISC_RUN_TEST_IMPL(active_stale) { + unsigned int active = 0; + unsigned int stale = DNS_RDATASTATSTYPE_ATTR_STALE; dns_stats_t *stats = NULL; + UNUSED(state); + dns_rdatasetstats_create(isc_g_mctx, &stats); - /* First 255 types. */ - for (i = 1; i <= 255; i++) { - set_typestats(stats, (dns_rdatatype_t)i); + /* + * The first 255 RRtypes, a type that folds onto the 'other' counter, + * and an NXDOMAIN. Setting 'other' (type 0) matters: the verify pass + * checks every slot, so each active slot must be populated. + */ + for (unsigned int i = 1; i <= 255; i++) { + set_active(stats, (dns_rdatatype_t)i); } - /* Specials */ - set_typestats(stats, (dns_rdatatype_t)1000); - set_nxdomainstats(stats); + set_active(stats, OTHERTYPE); + set_active_nxdomain(stats); - /* Check that all active counters are set to appropriately. */ - dns_rdatasetstats_dump(stats, verify_active_counters, NULL, 1); + dns_rdatasetstats_dump(stats, verify_counters, &active, + ISC_STATSDUMP_VERBOSE); - if (servestale) { - /* Mark stale */ - for (i = 1; i <= 255; i++) { - mark_stale(stats, (dns_rdatatype_t)i, 0, - DNS_RDATASTATSTYPE_ATTR_STALE); - } - mark_stale(stats, (dns_rdatatype_t)1000, 0, - DNS_RDATASTATSTYPE_ATTR_STALE); - mark_nxdomain_stale(stats, 0, DNS_RDATASTATSTYPE_ATTR_STALE); - - /* Check that all counters are set to appropriately. */ - dns_rdatasetstats_dump(stats, verify_stale_counters, NULL, 1); - - /* Set correct staleness state */ - from = DNS_RDATASTATSTYPE_ATTR_STALE; - } - - /* Mark ancient */ - for (i = 1; i <= 255; i++) { - mark_stale(stats, (dns_rdatatype_t)i, from, - DNS_RDATASTATSTYPE_ATTR_ANCIENT); + for (unsigned int i = 1; i <= 255; i++) { + mark_stale(stats, (dns_rdatatype_t)i); } - mark_stale(stats, (dns_rdatatype_t)1000, from, - DNS_RDATASTATSTYPE_ATTR_ANCIENT); - mark_nxdomain_stale(stats, from, DNS_RDATASTATSTYPE_ATTR_ANCIENT); + mark_stale(stats, OTHERTYPE); + mark_stale_nxdomain(stats); - /* - * Check that all counters are set to appropriately. - */ - dns_rdatasetstats_dump(stats, verify_ancient_counters, NULL, 1); + dns_rdatasetstats_dump(stats, verify_counters, &stale, + ISC_STATSDUMP_VERBOSE); dns_stats_detach(&stats); } -/* - * Test that rdatasetstats counters are properly set when moving from - * active -> stale -> ancient. - */ -ISC_RUN_TEST_IMPL(active_stale_ancient) { rdatasetstats(state, true); } - -/* - * Test that rdatasetstats counters are properly set when moving from - * active -> ancient. - */ -ISC_RUN_TEST_IMPL(active_ancient) { rdatasetstats(state, false); } - ISC_TEST_LIST_START -ISC_TEST_ENTRY(active_stale_ancient) -ISC_TEST_ENTRY(active_ancient) +ISC_TEST_ENTRY(active_stale) ISC_TEST_LIST_END ISC_TEST_MAIN