1014. [bug] Some queries would cause statistics counters to

increment more than once or not at all. [RT #1321]
2001-09-21 19:08:06 +00:00
parent a3ec9e6c3b
commit 808b909f27
3 changed files with 86 additions and 75 deletions
--- a/4
+++ b/4
@@ -1,3 +1,7 @@
+
+1014.	[bug]		Some queries would cause statistics counters to
+			increment more than once or not at all. [RT #1321]
+
 1013.	[bug]		It was possible to cancel a query twice when marking
 			a server as bogus or by having a blackhole acl.
 			[RT #1776]
--- a/bin/named/include/named/query.h
+++ b/bin/named/include/named/query.h
@@ -15,7 +15,7 @@
 * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */

-/* $Id: query.h,v 1.29 2001/09/11 01:21:39 gson Exp $ */
+/* $Id: query.h,v 1.30 2001/09/21 19:08:06 gson Exp $ */

 #ifndef NAMED_QUERY_H
 #define NAMED_QUERY_H 1
@@ -46,7 +46,9 @@ struct ns_query {
 	unsigned int			fetchoptions;
 	dns_db_t *			gluedb;
 	dns_db_t *			authdb;
-	isc_boolean_t			authdbvalid;
+	dns_zone_t *			authzone;
+	isc_boolean_t			authdbset;
+	isc_boolean_t			isreferral;
 	dns_fetch_t *			fetch;
 	dns_a6context_t			a6ctx;
 	isc_bufferlist_t		namebufs;
--- a/bin/named/query.c
+++ b/bin/named/query.c
@@ -15,7 +15,7 @@
 * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */

-/* $Id: query.c,v 1.201 2001/09/19 23:08:21 gson Exp $ */
+/* $Id: query.c,v 1.202 2001/09/21 19:08:05 gson Exp $ */

 #include <config.h>

@@ -146,20 +146,55 @@ synth_rev_respond(ns_client_t *client, dns_byaddrevent_t *bevent);
 * Increment query statistics counters.
 */
 static inline void
-count_query(dns_zone_t *zone, isc_boolean_t is_zone,
-	    dns_statscounter_t counter)
-{
+inc_stats(ns_client_t *client, dns_statscounter_t counter) {
+	dns_zone_t *zone = client->query.authzone;
+	
 	REQUIRE(counter < DNS_STATS_NCOUNTERS);

 	ns_g_server->querystats[counter]++;

-	if (is_zone && zone != NULL) {
+	if (zone != NULL) {
 		isc_uint64_t *zonestats = dns_zone_getstatscounters(zone);
 		if (zonestats != NULL)
 			zonestats[counter]++;
 	}
 }

+static void
+query_send(ns_client_t *client) {
+	dns_statscounter_t counter;
+	if (client->message->rcode == dns_rcode_noerror) {
+		if (ISC_LIST_EMPTY(client->message->sections[DNS_SECTION_ANSWER])) {
+			if (client->query.isreferral) {
+				counter = dns_statscounter_referral;
+			} else {
+				counter = dns_statscounter_nxrrset;
+			}
+		} else {
+			counter = dns_statscounter_success;
+		}
+	} else if (client->message->rcode == dns_rcode_nxdomain) {
+		counter = dns_statscounter_nxdomain;
+	} else {
+		/* We end up here in case of YXDOMAIN, and maybe others */
+		counter = dns_statscounter_failure;
+	}
+	inc_stats(client, counter);
+	ns_client_send(client);
+}
+
+static void
+query_error(ns_client_t *client, isc_result_t result) {
+	inc_stats(client, dns_statscounter_failure);
+	ns_client_error(client, result);
+}
+
+static void
+query_next(ns_client_t *client, isc_result_t result) {
+	inc_stats(client, dns_statscounter_failure);
+	ns_client_next(client, result);
+}
+
 static inline void
 query_maybeputqname(ns_client_t *client) {
 	if (client->query.restarts > 0) {
@@ -208,6 +243,8 @@ query_reset(ns_client_t *client, isc_boolean_t everything) {

 	if (client->query.authdb != NULL)
 		dns_db_detach(&client->query.authdb);
+	if (client->query.authzone != NULL)
+		dns_zone_detach(&client->query.authzone);

 	/*
 	 * Clean up free versions.
@@ -249,11 +286,12 @@ query_reset(ns_client_t *client, isc_boolean_t everything) {
 	client->query.dboptions = 0;
 	client->query.fetchoptions = 0;
 	client->query.gluedb = NULL;
-	client->query.authdbvalid = ISC_FALSE;
+	client->query.authdbset = ISC_FALSE;
+	client->query.isreferral = ISC_FALSE;
 }

 static void
-query_next(ns_client_t *client) {
+query_next_callback(ns_client_t *client) {
 	query_reset(client, ISC_FALSE);
 }

@@ -476,7 +514,9 @@ ns_query_init(ns_client_t *client) {
 	client->query.qname = NULL;
 	client->query.fetch = NULL;
 	client->query.authdb = NULL;
-	client->query.authdbvalid = ISC_FALSE;
+	client->query.authzone = NULL;
+	client->query.authdbset = ISC_FALSE;
+	client->query.isreferral = ISC_FALSE;	
 	query_reset(client, ISC_FALSE);
 	result = query_newdbversion(client, 3);
 	if (result != ISC_R_SUCCESS)
@@ -560,7 +600,7 @@ query_getzonedb(ns_client_t *client, dns_name_t *name, unsigned int options,
 	 * additional data from other zones.
 	 */
 	if (!client->view->additionalfromauth &&
-	    client->query.authdbvalid &&
+	    client->query.authdbset &&
 	    db != client->query.authdb)
 		goto refuse;

@@ -861,7 +901,6 @@ query_simplefind(void *arg, dns_name_t *name, dns_rdatatype_t type,
 		    dns_rdataset_isassociated(sigrdataset))
 			dns_rdataset_disassociate(sigrdataset);
 		if (is_zone) {
-			count_query(zone, is_zone, dns_statscounter_referral);
 			if (USECACHE(client)) {
 				/*
 				 * Either the answer is in the cache, or we
@@ -926,11 +965,8 @@ query_simplefind(void *arg, dns_name_t *name, dns_rdatatype_t type,
 	}
 	/*
 	 * If we get here, the result is ISC_R_SUCCESS, and we found the
-	 * answer we were looking for in the zone.  Update the zone's
-	 * query counter.
+	 * answer we were looking for in the zone.
 	 */
-	if (result == ISC_R_SUCCESS)
-		count_query(zone, is_zone, dns_statscounter_success);

 cleanup:
 	if (dns_rdataset_isassociated(&zrdataset)) {
@@ -2055,7 +2091,7 @@ query_resume(isc_task_t *task, isc_event_t *event) {
 		if (devent->sigrdataset != NULL)
 			query_putrdataset(client, &devent->sigrdataset);
 		isc_event_free(&event);
-		ns_client_next(client, ISC_R_CANCELED);
+		query_next(client, ISC_R_CANCELED);
 		/*
 		 * This may destroy the client.
 		 */
@@ -2073,6 +2109,8 @@ query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qdomain,
 	dns_rdataset_t *rdataset, *sigrdataset;
 	unsigned int options;

+	inc_stats(client, dns_statscounter_recursion);
+
 	/*
 	 * We are about to recurse, which means that this client will
 	 * be unavailable for serving new requests for an indeterminate
@@ -2403,20 +2441,17 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		 */
 		dbuf = query_getnamebuf(client);
 		if (dbuf == NULL) {
-			count_query(zone, is_zone, dns_statscounter_failure);
 			QUERY_ERROR(DNS_R_SERVFAIL);
 			goto cleanup;
 		}
 		fname = query_newname(client, dbuf, &b);
 		if (fname == NULL) {
-			count_query(zone, is_zone, dns_statscounter_failure);
 			QUERY_ERROR(DNS_R_SERVFAIL);
 			goto cleanup;
 		}
 		tname = dns_fixedname_name(&event->foundname);
 		result = dns_name_copy(tname, fname, NULL);
 		if (result != ISC_R_SUCCESS) {
-			count_query(zone, is_zone, dns_statscounter_failure);
 			QUERY_ERROR(DNS_R_SERVFAIL);
 			goto cleanup;
 		}
@@ -2450,7 +2485,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 	result = query_getdb(client, client->query.qname, 0, &zone, &db,
 			     &version, &is_zone);
 	if (result != ISC_R_SUCCESS) {
-		count_query(NULL, ISC_FALSE, dns_statscounter_failure);
 		if (result == DNS_R_REFUSED)
 			QUERY_ERROR(DNS_R_REFUSED);
 		else
@@ -2462,9 +2496,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		authoritative = ISC_TRUE;
       
 	if (event == NULL && client->query.restarts == 0) {
-		if (is_zone)
+		if (is_zone) {
+			dns_zone_attach(zone, &client->query.authzone);
 			dns_db_attach(db, &client->query.authdb);
-		client->query.authdbvalid = ISC_TRUE;
+		}
+		client->query.authdbset = ISC_TRUE;
 	}

 db_find:
@@ -2474,21 +2510,18 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 	 */
 	dbuf = query_getnamebuf(client);
 	if (dbuf == NULL) {
-		count_query(zone, is_zone, dns_statscounter_failure);
 		QUERY_ERROR(DNS_R_SERVFAIL);
 		goto cleanup;
 	}
 	fname = query_newname(client, dbuf, &b);
 	rdataset = query_newrdataset(client);
 	if (fname == NULL || rdataset == NULL) {
-		count_query(zone, is_zone, dns_statscounter_failure);
 		QUERY_ERROR(DNS_R_SERVFAIL);
 		goto cleanup;
 	}
 	if (WANTDNSSEC(client)) {
 		sigrdataset = query_newrdataset(client);
 		if (sigrdataset == NULL) {
-			count_query(zone, is_zone, dns_statscounter_failure);
 			QUERY_ERROR(DNS_R_SERVFAIL);
 			goto cleanup;
 		}
@@ -2545,8 +2578,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 			result = dns_name_copy(client->query.qname,
 					       fname, NULL);
 			if (result != ISC_R_SUCCESS) {
-				count_query(zone, is_zone,
-					    dns_statscounter_failure);
 				QUERY_ERROR(DNS_R_SERVFAIL);
 				goto cleanup;
 			}
@@ -2564,7 +2595,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 	CTRACE("query_find: resume");
 	switch (result) {
 	case ISC_R_SUCCESS:
-		count_query(zone, is_zone, dns_statscounter_success);
 		/*
 		 * This case is handled in the main line below.
 		 */
@@ -2626,8 +2656,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 			 * recurse anyway.
 			 */
 			if (RECURSIONOK(client)) {
-				count_query(zone, is_zone,
-					    dns_statscounter_recursion);
 				result = query_recurse(client, qtype,
 						       NULL, NULL);
 				if (result == ISC_R_SUCCESS)
@@ -2635,15 +2663,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 						NS_QUERYATTR_RECURSING;
 				else {
 					/* Unable to recurse. */
-					count_query(zone, is_zone,
-						    dns_statscounter_failure);
 					QUERY_ERROR(DNS_R_SERVFAIL);
 				}
 				goto cleanup;
 			} else {
 				/* Unable to give root server referral. */
-				count_query(zone, is_zone,
-					    dns_statscounter_failure);
 				QUERY_ERROR(DNS_R_SERVFAIL);
 				goto cleanup;
 			}
@@ -2683,6 +2707,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 				 * database by setting client->query.gluedb.
 				 */
 				client->query.gluedb = db;
+				client->query.isreferral = ISC_TRUE;
 				/*
 				 * We must ensure NOADDITIONAL is off,
 				 * because the generation of
@@ -2695,9 +2720,9 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 					sigrdatasetp = &sigrdataset;
 				else
 					sigrdatasetp = NULL;
-				query_addrrset(client, &fname, &rdataset,
-					       sigrdatasetp, dbuf,
-					       DNS_SECTION_AUTHORITY);
+				query_addrrset(client, &fname,
+					       &rdataset, sigrdatasetp,
+					       dbuf, DNS_SECTION_AUTHORITY);
 				client->query.gluedb = NULL;
 			} else {
 				/*
@@ -2758,8 +2783,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 				/*
 				 * Recurse!
 				 */
-				count_query(zone, is_zone,
-					    dns_statscounter_recursion);
 				if (type == dns_rdatatype_key)
 					result = query_recurse(client, qtype,
 							       NULL, NULL);
@@ -2769,20 +2792,16 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 				if (result == ISC_R_SUCCESS)
 					client->query.attributes |=
 						NS_QUERYATTR_RECURSING;
-				else {
-					count_query(zone, is_zone,
-						    dns_statscounter_failure);
+				else
 					QUERY_ERROR(DNS_R_SERVFAIL);
-				}
 			} else {
 				/*
 				 * This is the best answer.
 				 */
-				count_query(zone, is_zone,
-					    dns_statscounter_referral);
-				client->query.gluedb = zdb;
 				client->query.attributes |=
 					NS_QUERYATTR_CACHEGLUEOK;
+				client->query.gluedb = zdb;
+				client->query.isreferral = ISC_TRUE;
 				/*
 				 * We must ensure NOADDITIONAL is off,
 				 * because the generation of
@@ -2806,7 +2825,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		goto cleanup;
 	case DNS_R_NXRRSET:
 		INSIST(is_zone);
-		count_query(zone, is_zone, dns_statscounter_nxrrset);
 		if (dns_rdataset_isassociated(rdataset)) {
 			/*
 			 * If we've got a NXT record, we need to save the
@@ -2827,7 +2845,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		 */
 		result = query_addsoa(client, db, ISC_FALSE);
 		if (result != ISC_R_SUCCESS) {
-			count_query(zone, is_zone, dns_statscounter_failure);
 			QUERY_ERROR(result);
 			goto cleanup;
 		}
@@ -2843,7 +2860,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		goto cleanup;
 	case DNS_R_NXDOMAIN:
 		INSIST(is_zone);
-		count_query(zone, is_zone, dns_statscounter_nxdomain);
 		if (dns_rdataset_isassociated(rdataset)) {
 			/*
 			 * If we've got a NXT record, we need to save the
@@ -2870,7 +2886,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		else
 			result = query_addsoa(client, db, ISC_FALSE);
 		if (result != ISC_R_SUCCESS) {
-			count_query(zone, is_zone, dns_statscounter_failure);
 			QUERY_ERROR(result);
 			goto cleanup;
 		}
@@ -2889,13 +2904,8 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		client->message->rcode = dns_rcode_nxdomain;
 		goto cleanup;
 	case DNS_R_NCACHENXDOMAIN:
-		INSIST(!is_zone);
-		count_query(NULL, is_zone, dns_statscounter_nxdomain);
-		goto ncachenxrrset;
 	case DNS_R_NCACHENXRRSET:
 		INSIST(!is_zone);
-		count_query(NULL, is_zone, dns_statscounter_nxrrset);
-	ncachenxrrset:
 		authoritative = ISC_FALSE;
 		/*
 		 * Set message rcode, if required.
@@ -3084,7 +3094,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		/*
 		 * Something has gone wrong.
 		 */
-		count_query(zone, is_zone, dns_statscounter_failure);
 		QUERY_ERROR(DNS_R_SERVFAIL);
 		goto cleanup;
 	}
@@ -3098,7 +3107,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		rdsiter = NULL;
 		result = dns_db_allrdatasets(db, node, version, 0, &rdsiter);
 		if (result != ISC_R_SUCCESS) {
-			count_query(zone, is_zone, dns_statscounter_failure);
 			QUERY_ERROR(DNS_R_SERVFAIL);
 			goto cleanup;
 		}
@@ -3174,14 +3182,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 				/*
 				 * Something went wrong.
 				 */
-				count_query(zone, is_zone,
-					    dns_statscounter_failure);
 				result = DNS_R_SERVFAIL;
 			}
 		}
 		dns_rdatasetiter_destroy(&rdsiter);
 		if (result != ISC_R_NOMORE) {
-			count_query(zone, is_zone, dns_statscounter_failure);
 			QUERY_ERROR(DNS_R_SERVFAIL);
 			goto cleanup;
 		}
@@ -3277,7 +3282,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		 * or if the client requested recursion and thus wanted
 		 * the complete answer, send an error response.
 		 */
-		 ns_client_error(client, eresult);
+		 query_error(client, eresult);
 		 ns_client_detach(&client);
 	} else if (!RECURSING(client)) {
 		/*
@@ -3292,7 +3297,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 		    client->view->auth_nxdomain == ISC_TRUE)
 			client->message->flags |= DNS_MESSAGEFLAG_AA;

-		ns_client_send(client);
+		query_send(client);
 		ns_client_detach(&client);
 	}
 	CTRACE("query_find: done");
@@ -3332,7 +3337,7 @@ ns_query_start(ns_client_t *client) {
 	/*
 	 * Ensure that appropriate cleanups occur.
 	 */
-	client->next = query_next;
+	client->next = query_next_callback;

 	if ((message->flags & DNS_MESSAGEFLAG_RD) != 0)
 		client->query.attributes |= NS_QUERYATTR_WANTRECURSION;
@@ -3369,7 +3374,7 @@ ns_query_start(ns_client_t *client) {
 	 */
 	result = dns_message_firstname(message, DNS_SECTION_QUESTION);
 	if (result != ISC_R_SUCCESS) {
-		ns_client_error(client, result);
+		query_error(client, result);
 		return;
 	}
 	dns_message_currentname(message, DNS_SECTION_QUESTION,
@@ -3382,9 +3387,9 @@ ns_query_start(ns_client_t *client) {
 			 * There's more than one QNAME in the question
 			 * section.
 			 */
-			ns_client_error(client, DNS_R_FORMERR);
+			query_error(client, DNS_R_FORMERR);
 		} else
-			ns_client_error(client, result);
+			query_error(client, result);
 		return;
 	}

@@ -3395,7 +3400,7 @@ ns_query_start(ns_client_t *client) {
 	 * Check for multiple question queries, since edns1 is dead.
 	 */
 	if (message->counts[DNS_SECTION_QUESTION] > 1) {
-		ns_client_error(client, DNS_R_FORMERR);
+		query_error(client, DNS_R_FORMERR);
 		return;
 	}

@@ -3415,19 +3420,19 @@ ns_query_start(ns_client_t *client) {
 			return;
 		case dns_rdatatype_maila:
 		case dns_rdatatype_mailb:
-			ns_client_error(client, DNS_R_NOTIMP);
+			query_error(client, DNS_R_NOTIMP);
 			return;
 		case dns_rdatatype_tkey:
 			result = dns_tkey_processquery(client->message,
 						ns_g_server->tkeyctx,
 						client->view->dynamickeys);
 			if (result == ISC_R_SUCCESS)
-				ns_client_send(client);
+				query_send(client);
 			else
-				ns_client_error(client, result);
+				query_error(client, result);
 			return;
 		default: /* TSIG, etc. */
-			ns_client_error(client, DNS_R_FORMERR);
+			query_error(client, DNS_R_FORMERR);
 			return;
 		}
 	}
@@ -3449,7 +3454,7 @@ ns_query_start(ns_client_t *client) {
 	 */
 	result = dns_message_reply(message, ISC_TRUE);
 	if (result != ISC_R_SUCCESS) {
-		ns_client_next(client, result);
+		query_next(client, result);
 		return;
 	}

@@ -3718,9 +3723,9 @@ synth_fwd_respond(ns_client_t *client, dns_adbfind_t *find) {
 static void
 synth_finish(ns_client_t *client, isc_result_t result) {
 	if (result == ISC_R_SUCCESS)
-		ns_client_send(client);
+		query_send(client);
 	else
-		ns_client_error(client, result);
+		query_error(client, result);
 	ns_client_detach(&client);	
 }