diff --git a/CHANGES b/CHANGES index a378cbc203..5a010ed0e2 100644 --- a/CHANGES +++ b/CHANGES @@ -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] diff --git a/bin/named/include/named/query.h b/bin/named/include/named/query.h index ce0da98720..c3dcab9055 100644 --- 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; diff --git a/bin/named/query.c b/bin/named/query.c index 661c81e985..72f3fddab0 100644 --- 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 @@ -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); }