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) + DNS_RDATASTATSTYPE_ATTR_STALE) != 0) { - counter |= RDTYPECOUNTER_NXDOMAIN_ANCIENT; - } else if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) & - 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; - - attributes = 0; - which = DNS_RDATASTATSTYPE_VALUE(type, attributes); - dns_rdatasetstats_increment(stats, which); - - attributes = DNS_RDATASTATSTYPE_ATTR_NXRRSET; - which = DNS_RDATASTATSTYPE_VALUE(type, attributes); - dns_rdatasetstats_increment(stats, which); -} - -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); -} - -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); -} - -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); -} - -#define ATTRIBUTE_SET(y) ((attributes & (y)) != 0) -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); - } -} - -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); - - 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); - } -} - -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); - - 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) { - assert_int_equal(value, 1); - } else { - assert_int_equal(value, 0); - } -} /* - * Individual unit tests + * 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). */ +/* A type that does not fit in 8 bits and folds onto the 'other' counter. */ +#define OTHERTYPE ((dns_rdatatype_t)1000) + +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 +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 +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 +set_active_nxdomain(dns_stats_t *stats) { + incr(stats, 0, DNS_RDATASTATSTYPE_ATTR_NXDOMAIN); +} + +/* Move the positive and NXRRSET counters for 'type' from active to stale. */ +static void +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 +mark_stale_nxdomain(dns_stats_t *stats) { + unsigned int nxd = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN; + unsigned int stale = DNS_RDATASTATSTYPE_ATTR_STALE; + + decr(stats, 0, nxd); + incr(stats, 0, nxd | stale); +} + /* - * Test that rdatasetstats counters are properly set when moving from - * active -> stale -> ancient. + * 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 -rdatasetstats(void **state ISC_ATTR_UNUSED, bool servestale) { - unsigned int i; - unsigned int from = 0; +verify_counters(dns_rdatastatstype_t which, uint64_t value, void *arg) { + unsigned int attributes = DNS_RDATASTATSTYPE_ATTR(which); + unsigned int expected = *(unsigned int *)arg; + + if ((attributes & DNS_RDATASTATSTYPE_ATTR_STALE) == expected) { + assert_int_equal(value, 1); + } else { + assert_int_equal(value, 0); + } +} + +/* + * Populate every counter active, verify, transition all to stale, verify. + */ +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); - } - /* Specials */ - set_typestats(stats, (dns_rdatatype_t)1000); - set_nxdomainstats(stats); - - /* Check that all active counters are set to appropriately. */ - dns_rdatasetstats_dump(stats, verify_active_counters, NULL, 1); - - 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); - } - mark_stale(stats, (dns_rdatatype_t)1000, from, - DNS_RDATASTATSTYPE_ATTR_ANCIENT); - mark_nxdomain_stale(stats, from, DNS_RDATASTATSTYPE_ATTR_ANCIENT); - /* - * Check that all counters are set to appropriately. + * 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. */ - dns_rdatasetstats_dump(stats, verify_ancient_counters, NULL, 1); + for (unsigned int i = 1; i <= 255; i++) { + set_active(stats, (dns_rdatatype_t)i); + } + set_active(stats, OTHERTYPE); + set_active_nxdomain(stats); + + dns_rdatasetstats_dump(stats, verify_counters, &active, + ISC_STATSDUMP_VERBOSE); + + for (unsigned int i = 1; i <= 255; i++) { + mark_stale(stats, (dns_rdatatype_t)i); + } + mark_stale(stats, OTHERTYPE); + mark_stale_nxdomain(stats); + + 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