Propagate stale attribute when updating stats (#39141)

Squashed commit of the following:

commit 9b5b9fa30fbeba8ee1e95cb1028017230ed4db02
Author: Mukund Sivaraman <muks@isc.org>
Date:   Tue Apr 7 19:30:54 2015 +0530

    Remove double function prototypes

commit f3bb8cc60ae476eaa871ba10330b16425ced2d7c
Author: Mukund Sivaraman <muks@isc.org>
Date:   Tue Apr 7 19:30:34 2015 +0530

    Unify several copies of redundant code into a helper function

commit 4899fb9b2f36fc5d159fa877c0780a442a7cbdb3
Author: Mukund Sivaraman <muks@isc.org>
Date:   Thu Apr 2 00:23:53 2015 +0530

    Propagate stale attribute when updating stats

diff --git a/CHANGES b/CHANGES
index e538f64523811c4a2827e77ada61713bc431c4e5..b4843d903a94adfe2b44acea7eaad0579adf271c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+4136.  [bug]           Stale statistics counters with the leading
+                       '#' prefix (such as #NXDOMAIN) were not being
+                       updated correctly. This has been fixed. [RT #39141]
+
 4135.  [cleanup]       Log expired NTA at startup. [RT #39680]
 
 4134.  [cleanup]       Include client-ip rules when logging the number

diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h
index 4f44dd8cbd81c085d4f8dd333efd59220104394b..6eec0f9a1b2978a04a404b775f6fd677e5f90747 100644 (file)
--- a/lib/dns/include/dns/zone.h
+++ b/lib/dns/include/dns/zone.h
@@ -905,7 +905,7 @@ dns_zone_getnotifysrc4(dns_zone_t *zone);
 isc_dscp_t
 dns_zone_getnotifysrc4dscp(dns_zone_t *zone);
 /*%/
- * Get the DCSP value associated with the notify source.
+ * Get the DSCP value associated with the notify source.
  *
  * Require:
  *\li  'zone' to be a valid zone.
@@ -923,15 +923,6 @@ dns_zone_setnotifysrc4dscp(dns_zone_t *zone, isc_dscp_t dscp);
  *\li  #ISC_R_SUCCESS
  */
 
-isc_dscp_t
-dns_zone_getnotifysrc4dscp(dns_zone_t *zone);
-/*%/
- * Get the DSCP value associated with the notify source.
- *
- * Require:
- *\li  'zone' to be a valid zone.
- */
-
 isc_result_t
 dns_zone_setnotifysrc6(dns_zone_t *zone, const isc_sockaddr_t *notifysrc);
 /*%<
@@ -958,7 +949,7 @@ dns_zone_getnotifysrc6(dns_zone_t *zone);
 isc_dscp_t
 dns_zone_getnotifysrc6dscp(dns_zone_t *zone);
 /*%/
- * Get the DCSP value associated with the notify source.
+ * Get the DSCP value associated with the notify source.
  *
  * Require:
  *\li  'zone' to be a valid zone.
@@ -976,15 +967,6 @@ dns_zone_setnotifysrc6dscp(dns_zone_t *zone, isc_dscp_t dscp);
  *\li  #ISC_R_SUCCESS
  */
 
-isc_dscp_t
-dns_zone_getnotifysrc6dscp(dns_zone_t *zone);
-/*%/
- * Get the DSCP value associated with the notify source.
- *
- * Require:
- *\li  'zone' to be a valid zone.
- */
-
 void
 dns_zone_setnotifyacl(dns_zone_t *zone, dns_acl_t *acl);
 /*%<

diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index df67bbc947645840529eaba1b800ee91d724ec9b..01e91ea2ee4dca375f3062faf54e657a9d5b035b 100644 (file)
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -511,6 +511,8 @@ struct acachectl {
        (((header)->attributes & RDATASET_ATTR_RETAIN) != 0)
 #define NXDOMAIN(header) \
        (((header)->attributes & RDATASET_ATTR_NXDOMAIN) != 0)
+#define STALE(header) \
+       (((header)->attributes & RDATASET_ATTR_STALE) != 0)
 #define RESIGN(header) \
        (((header)->attributes & RDATASET_ATTR_RESIGN) != 0)
 #define OPTOUT(header) \
@@ -1018,6 +1020,9 @@ update_rrsetstats(dns_rbtdb_t *rbtdb, rdatasetheader_t *header,
        } else
                base = RBTDB_RDATATYPE_BASE(header->type);
 
+       if (STALE(header))
+               statattributes |= DNS_RDATASTATSTYPE_ATTR_STALE;
+
        type = DNS_RDATASTATSTYPE_VALUE(base, statattributes);
        if (increment)
                dns_rdatasetstats_increment(rbtdb->rrsetstats, type);
@@ -4535,6 +4540,61 @@ zone_findzonecut(dns_db_t *db, dns_name_t *name, unsigned int options,
        return (ISC_R_NOTIMPLEMENTED);
 }
 
+static isc_boolean_t
+check_stale_rdataset(dns_rbtnode_t *node, rdatasetheader_t *header,
+                    isc_rwlocktype_t *locktype, nodelock_t *lock,
+                    rbtdb_search_t *search, rdatasetheader_t **header_prev)
+{
+       if (header->rdh_ttl < search->now) {
+               /*
+                * This rdataset is stale.  If no one else is using the
+                * node, we can clean it up right now, otherwise we mark
+                * it as stale, and the node as dirty, so it will get
+                * cleaned up later.
+                */
+               if ((header->rdh_ttl < search->now - RBTDB_VIRTUAL) &&
+                   (*locktype == isc_rwlocktype_write ||
+                    NODE_TRYUPGRADE(lock) == ISC_R_SUCCESS))
+               {
+                       /*
+                        * We update the node's status only when we can
+                        * get write access; otherwise, we leave others
+                        * to this work.  Periodical cleaning will
+                        * eventually take the job as the last resort.
+                        * We won't downgrade the lock, since other
+                        * rdatasets are probably stale, too.
+                        */
+                       *locktype = isc_rwlocktype_write;
+
+                       if (dns_rbtnode_refcurrent(node) == 0) {
+                               isc_mem_t *mctx;
+
+                               /*
+                                * header->down can be non-NULL if the
+                                * refcount has just decremented to 0
+                                * but decrement_reference() has not
+                                * performed clean_cache_node(), in
+                                * which case we need to purge the stale
+                                * headers first.
+                                */
+                               mctx = search->rbtdb->common.mctx;
+                               clean_stale_headers(search->rbtdb, mctx, header);
+                               if (*header_prev != NULL)
+                                       (*header_prev)->next = header->next;
+                               else
+                                       node->data = header->next;
+                               free_rdataset(search->rbtdb, mctx, header);
+                       } else {
+                               mark_stale_header(search->rbtdb, header);
+                               *header_prev = header;
+                       }
+               } else
+                       *header_prev = header;
+               return (ISC_TRUE);
+       }
+       return (ISC_FALSE);
+}
+
 static isc_result_t
 cache_zonecut_callback(dns_rbtnode_t *node, dns_name_t *name, void *arg) {
        rbtdb_search_t *search = arg;
@@ -4565,57 +4625,10 @@ cache_zonecut_callback(dns_rbtnode_t *node, dns_name_t *name, void *arg) {
        header_prev = NULL;
        for (header = node->data; header != NULL; header = header_next) {
                header_next = header->next;
-               if (header->rdh_ttl <  search->now) {
-                       /*
-                        * This rdataset is stale.  If no one else is
-                        * using the node, we can clean it up right
-                        * now, otherwise we mark it as stale, and
-                        * the node as dirty, so it will get cleaned
-                        * up later.
-                        */
-                       if ((header->rdh_ttl <  search->now - RBTDB_VIRTUAL) &&
-                           (locktype == isc_rwlocktype_write ||
-                            NODE_TRYUPGRADE(lock) == ISC_R_SUCCESS)) {
-                               /*
-                                * We update the node's status only when we
-                                * can get write access; otherwise, we leave
-                                * others to this work.  Periodical cleaning
-                                * will eventually take the job as the last
-                                * resort.
-                                * We won't downgrade the lock, since other
-                                * rdatasets are probably stale, too.
-                                */
-                               locktype = isc_rwlocktype_write;
-
-                               if (dns_rbtnode_refcurrent(node) == 0) {
-                                       isc_mem_t *mctx;
-
-                                       /*
-                                        * header->down can be non-NULL if the
-                                        * refcount has just decremented to 0
-                                        * but decrement_reference() has not
-                                        * performed clean_cache_node(), in
-                                        * which case we need to purge the
-                                        * stale headers first.
-                                        */
-                                       mctx = search->rbtdb->common.mctx;
-                                       clean_stale_headers(search->rbtdb,
-                                                           mctx,
-                                                           header);
-                                       if (header_prev != NULL)
-                                               header_prev->next =
-                                                       header->next;
-                                       else
-                                               node->data = header->next;
-                                       free_rdataset(search->rbtdb, mctx,
-                                                     header);
-                               } else {
-                                       mark_stale_header(search->rbtdb,
-                                                         header);
-                                       header_prev = header;
-                               }
-                       } else
-                               header_prev = header;
+               if (check_stale_rdataset(node, header,
+                                        &locktype, lock, search,
+                                        &header_prev)) {
+                       /* Do nothing. */
                } else if (header->type == dns_rdatatype_dname &&
                           EXISTS(header)) {
                        dname_header = header;
@@ -4684,51 +4697,12 @@ find_deepest_zonecut(rbtdb_search_t *search, dns_rbtnode_t *node,
                found = NULL;
                foundsig = NULL;
                header_prev = NULL;
-               for (header = node->data;
-                    header != NULL;
-                    header = header_next) {
+               for (header = node->data; header != NULL; header = header_next) {
                        header_next = header->next;
-                       if (header->rdh_ttl <  search->now) {
-                               /*
-                                * This rdataset is stale.  If no one else is
-                                * using the node, we can clean it up right
-                                * now, otherwise we mark it as stale, and
-                                * the node as dirty, so it will get cleaned
-                                * up later.
-                                */
-                               if ((header->rdh_ttl <  search->now -
-                                                   RBTDB_VIRTUAL) &&
-                                   (locktype == isc_rwlocktype_write ||
-                                    NODE_TRYUPGRADE(lock) == ISC_R_SUCCESS)) {
-                                       /*
-                                        * We update the node's status only
-                                        * when we can get write access.
-                                        */
-                                       locktype = isc_rwlocktype_write;
-
-                                       if (dns_rbtnode_refcurrent(node)
-                                           == 0) {
-                                               isc_mem_t *m;
-
-                                               m = search->rbtdb->common.mctx;
-                                               clean_stale_headers(
-                                                       search->rbtdb,
-                                                       m, header);
-                                               if (header_prev != NULL)
-                                                       header_prev->next =
-                                                               header->next;
-                                               else
-                                                       node->data =
-                                                               header->next;
-                                               free_rdataset(rbtdb, m,
-                                                             header);
-                                       } else {
-                                               mark_stale_header(rbtdb,
-                                                                 header);
-                                               header_prev = header;
-                                       }
-                               } else
-                                       header_prev = header;
+                       if (check_stale_rdataset(node, header,
+                                                &locktype, lock, search,
+                                                &header_prev)) {
+                               /* Do nothing. */
                        } else if (EXISTS(header)) {
                                /*
                                 * We've found an extant rdataset.  See if
@@ -4860,49 +4834,11 @@ find_coveringnsec(rbtdb_search_t *search, dns_dbnode_t **nodep,
                foundsig = NULL;
                empty_node = ISC_TRUE;
                header_prev = NULL;
-               for (header = node->data;
-                    header != NULL;
-                    header = header_next) {
+               for (header = node->data; header != NULL; header = header_next) {
                        header_next = header->next;
-                       if (header->rdh_ttl <  now) {
-                               /*
-                                * This rdataset is stale.  If no one else is
-                                * using the node, we can clean it up right
-                                * now, otherwise we mark it as stale, and the
-                                * node as dirty, so it will get cleaned up
-                                * later.
-                                */
-                               if ((header->rdh_ttl <  now - RBTDB_VIRTUAL) &&
-                                   (locktype == isc_rwlocktype_write ||
-                                    NODE_TRYUPGRADE(lock) == ISC_R_SUCCESS)) {
-                                       /*
-                                        * We update the node's status only
-                                        * when we can get write access.
-                                        */
-                                       locktype = isc_rwlocktype_write;
-
-                                       if (dns_rbtnode_refcurrent(node)
-                                           == 0) {
-                                               isc_mem_t *m;
-
-                                               m = search->rbtdb->common.mctx;
-                                               clean_stale_headers(
-                                                       search->rbtdb,
-                                                       m, header);
-                                               if (header_prev != NULL)
-                                                       header_prev->next =
-                                                               header->next;
-                                               else
-                                                       node->data = header->next;
-                                               free_rdataset(search->rbtdb, m,
-                                                             header);
-                                       } else {
-                                               mark_stale_header(search->rbtdb,
-                                                                 header);
-                                               header_prev = header;
-                                       }
-                               } else
-                                       header_prev = header;
+                       if (check_stale_rdataset(node, header,
+                                                &locktype, lock, search,
+                                                &header_prev)) {
                                continue;
                        }
                        if (NONEXISTENT(header) ||
@@ -5085,41 +5021,10 @@ cache_find(dns_db_t *db, dns_name_t *name, dns_dbversion_t *version,
        header_prev = NULL;
        for (header = node->data; header != NULL; header = header_next) {
                header_next = header->next;
-               if (header->rdh_ttl <  now) {
-                       /*
-                        * This rdataset is stale.  If no one else is using the
-                        * node, we can clean it up right now, otherwise we
-                        * mark it as stale, and the node as dirty, so it will
-                        * get cleaned up later.
-                        */
-                       if ((header->rdh_ttl <  now - RBTDB_VIRTUAL) &&
-                           (locktype == isc_rwlocktype_write ||
-                            NODE_TRYUPGRADE(lock) == ISC_R_SUCCESS)) {
-                               /*
-                                * We update the node's status only when we
-                                * can get write access.
-                                */
-                               locktype = isc_rwlocktype_write;
-
-                               if (dns_rbtnode_refcurrent(node) == 0) {
-                                       isc_mem_t *mctx;
-
-                                       mctx = search.rbtdb->common.mctx;
-                                       clean_stale_headers(search.rbtdb, mctx,
-                                                           header);
-                                       if (header_prev != NULL)
-                                               header_prev->next =
-                                                       header->next;
-                                       else
-                                               node->data = header->next;
-                                       free_rdataset(search.rbtdb, mctx,
-                                                     header);
-                               } else {
-                                       mark_stale_header(search.rbtdb, header);
-                                       header_prev = header;
-                               }
-                       } else
-                               header_prev = header;
+               if (check_stale_rdataset(node, header,
+                                        &locktype, lock, &search,
+                                        &header_prev)) {
+                       /* Do nothing. */
                } else if (EXISTS(header)) {
                        /*
                         * We now know that there is at least one active
@@ -5391,41 +5296,10 @@ cache_findzonecut(dns_db_t *db, dns_name_t *name, unsigned int options,
        header_prev = NULL;
        for (header = node->data; header != NULL; header = header_next) {
                header_next = header->next;
-               if (header->rdh_ttl <  now) {
-                       /*
-                        * This rdataset is stale.  If no one else is using the
-                        * node, we can clean it up right now, otherwise we
-                        * mark it as stale, and the node as dirty, so it will
-                        * get cleaned up later.
-                        */
-                       if ((header->rdh_ttl < now - RBTDB_VIRTUAL) &&
-                           (locktype == isc_rwlocktype_write ||
-                            NODE_TRYUPGRADE(lock) == ISC_R_SUCCESS)) {
-                               /*
-                                * We update the node's status only when we
-                                * can get write access.
-                                */
-                               locktype = isc_rwlocktype_write;
-
-                               if (dns_rbtnode_refcurrent(node) == 0) {
-                                       isc_mem_t *mctx;
-
-                                       mctx = search.rbtdb->common.mctx;
-                                       clean_stale_headers(search.rbtdb, mctx,
-                                                           header);
-                                       if (header_prev != NULL)
-                                               header_prev->next =
-                                                       header->next;
-                                       else
-                                               node->data = header->next;
-                                       free_rdataset(search.rbtdb, mctx,
-                                                     header);
-                               } else {
-                                       mark_stale_header(search.rbtdb, header);
-                                       header_prev = header;
-                               }
-                       } else
-                               header_prev = header;
+               if (check_stale_rdataset(node, header,
+                                        &locktype, lock, &search,
+                                        &header_prev)) {
+                       /* Do nothing. */
                } else if (EXISTS(header)) {
                        /*
                         * If we found a type we were looking for, remember