1887. [func] Detect duplicates of UDP queries we are recursing on

and drop them.  New stats category "duplicates".
                        [RT #14892]

CHANGES

@@ -1,3 +1,7 @@
+1887.	[func]		Detect duplicates of UDP queries we are recursing on
+			and drop them.  New stats category "duplicates".
+			[RT #14892]
+
 1886.	[bug]		Fix unreasonably low quantum on call to
 			dns_rbt_destroy2().  Remove unnecessay unhash_node()
 			call. [RT #14919]

bin/named/query.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: query.c,v 1.266 2005/05/16 05:33:42 marka Exp $ */
+/* $Id: query.c,v 1.267 2005/06/17 01:58:21 marka Exp $ */
 
 /*! \file */
 
@@ -161,7 +161,10 @@ query_error(ns_client_t *client, isc_result_t result) {
 
 static void
 query_next(ns_client_t *client, isc_result_t result) {
-	inc_stats(client, dns_statscounter_failure);
+	if (result == DNS_R_DUPLICATE)
+		inc_stats(client, dns_statscounter_duplicate);
+	else
+		inc_stats(client, dns_statscounter_failure);
 	ns_client_next(client, result);
 }
 
@@ -2681,6 +2684,7 @@ query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qdomain,
 {
 	isc_result_t result;
 	dns_rdataset_t *rdataset, *sigrdataset;
+	isc_sockaddr_t *peeraddr;
 
 	inc_stats(client, dns_statscounter_recursion);
 
@@ -2744,14 +2748,19 @@ query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qdomain,
 
 	if (client->query.timerset == ISC_FALSE)
 		ns_client_settimeout(client, 60);
-	result = dns_resolver_createfetch(client->view->resolver,
-					  client->query.qname,
-					  qtype, qdomain, nameservers,
-					  NULL, client->query.fetchoptions,
-					  client->task,
-					  query_resume, client,
-					  rdataset, sigrdataset,
-					  &client->query.fetch);
+	if ((client->attributes & NS_CLIENTATTR_TCP) == 0)
+		peeraddr = &client->peeraddr;
+	else
+		peeraddr = NULL;
+	result = dns_resolver_createfetch2(client->view->resolver,
+					   client->query.qname,
+					   qtype, qdomain, nameservers,
+					   NULL, peeraddr, client->message->id,
+					   client->query.fetchoptions,
+					   client->task,
+					   query_resume, client,
+					   rdataset, sigrdataset,
+					   &client->query.fetch);
 
 	if (result == ISC_R_SUCCESS) {
 		/*
@@ -3219,7 +3228,10 @@ 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 {
+				else if (result == DNS_R_DUPLICATE) {
+					/* Duplicate query. */
+					QUERY_ERROR(result);
+				} else {
 					/* Unable to recurse. */
 					QUERY_ERROR(DNS_R_SERVFAIL);
 				}
@@ -3389,6 +3401,8 @@ 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 if (result == DNS_R_DUPLICATE)
+					QUERY_ERROR(result);
 				else
 					QUERY_ERROR(DNS_R_SERVFAIL);
 			} else {
@@ -3930,13 +3944,22 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
 
 	if (eresult != ISC_R_SUCCESS &&
 	    (!PARTIALANSWER(client) || WANTRECURSION(client))) {
-		/*
-		 * If we don't have any answer to give the client,
-		 * or if the client requested recursion and thus wanted
-		 * the complete answer, send an error response.
-		 */
-		 query_error(client, eresult);
-		 ns_client_detach(&client);
+		if (eresult == DNS_R_DUPLICATE) {
+			/*
+			 * This was a duplicate query that we are
+			 * recursing on.  Don't send a response now.
+			 * The original query will still cause a response.
+			 */
+			query_next(client, eresult);
+		} else {
+			/*
+			 * If we don't have any answer to give the client,
+			 * or if the client requested recursion and thus wanted
+			 * the complete answer, send an error response.
+			 */
+			query_error(client, eresult);
+		}
+		ns_client_detach(&client);
 	} else if (!RECURSING(client)) {
 		/*
 		 * We are done.  Set up sortlist data for the message

lib/dns/include/dns/resolver.h

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: resolver.h,v 1.45 2005/06/07 00:16:01 marka Exp $ */
+/* $Id: resolver.h,v 1.46 2005/06/17 01:58:23 marka Exp $ */
 
 #ifndef DNS_RESOLVER_H
 #define DNS_RESOLVER_H 1
@@ -65,9 +65,9 @@ ISC_LANG_BEGINDECLS
  * 'node', 'rdataset', and 'sigrdataset' may be bound.  It is the
  * receiver's responsibility to detach before freeing the event.
  * \brief
- * 'rdataset' and 'sigrdataset' are the values that were supplied when
- * dns_resolver_createfetch() was called.  They are returned to the
- * caller so that they may be freed.
+ * 'rdataset', 'sigrdataset', 'client' and 'id' are the values that were
+ * supplied when dns_resolver_createfetch() was called.  They are returned
+ *  to the caller so that they may be freed.
  */
 typedef struct dns_fetchevent {
 	ISC_EVENT_COMMON(struct dns_fetchevent);
@@ -79,6 +79,8 @@ typedef struct dns_fetchevent {
 	dns_rdataset_t *		rdataset;
 	dns_rdataset_t *		sigrdataset;
 	dns_fixedname_t			foundname;
+	isc_sockaddr_t *		client;
+	dns_messageid_t			id;
 } dns_fetchevent_t;
 
 /*
@@ -240,6 +242,18 @@ dns_resolver_createfetch(dns_resolver_t *res, dns_name_t *name,
 			 dns_rdataset_t *rdataset,
 			 dns_rdataset_t *sigrdataset,
 			 dns_fetch_t **fetchp);
+
+isc_result_t
+dns_resolver_createfetch2(dns_resolver_t *res, dns_name_t *name,
+			  dns_rdatatype_t type,
+			  dns_name_t *domain, dns_rdataset_t *nameservers,
+			  dns_forwarders_t *forwarders,
+			  isc_sockaddr_t *client, isc_uint16_t id,
+			  unsigned int options, isc_task_t *task,
+			  isc_taskaction_t action, void *arg,
+			  dns_rdataset_t *rdataset,
+			  dns_rdataset_t *sigrdataset,
+			  dns_fetch_t **fetchp);
 /*%<
  * Recurse to answer a question.
  *
@@ -262,6 +276,10 @@ dns_resolver_createfetch(dns_resolver_t *res, dns_name_t *name,
  *\li	The values of 'rdataset' and 'sigrdataset' will be returned in
  *	the FETCHDONE event.
  *
+ *\li	'client' and 'id' are used for duplicate query detection.  '*client'
+ *	must remain stable until after 'action' has been called or
+ *	dns_resolver_cancelfetch() is called.
+ *
  * Requires:
  *
  *\li	'res' is a valid resolver that has been frozen.
@@ -277,6 +295,8 @@ dns_resolver_createfetch(dns_resolver_t *res, dns_name_t *name,
  *
  *\li	'forwarders' is NULL.
  *
+ *\li	'client' is a valid sockaddr or NULL.
+ *
  *\li	'options' contains valid options.
  *
  *\li	'rdataset' is a valid, disassociated rdataset.
@@ -288,6 +308,7 @@ dns_resolver_createfetch(dns_resolver_t *res, dns_name_t *name,
  * Returns:
  *
  *\li	#ISC_R_SUCCESS					Success
+ *\li	#DNS_R_DUPLICATE
  *
  *\li	Many other values are possible, all of which indicate failure.
  */

lib/dns/include/dns/result.h

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: result.h,v 1.109 2005/05/19 04:59:04 marka Exp $ */
+/* $Id: result.h,v 1.110 2005/06/17 01:58:23 marka Exp $ */
 
 #ifndef DNS_RESULT_H
 #define DNS_RESULT_H 1
@@ -146,8 +146,9 @@
 #define DNS_R_MUSTBESECURE		(ISC_RESULTCLASS_DNS + 100)
 #define DNS_R_COVERINGNSEC		(ISC_RESULTCLASS_DNS + 101)
 #define DNS_R_MXISADDRESS		(ISC_RESULTCLASS_DNS + 102)
+#define DNS_R_DUPLICATE			(ISC_RESULTCLASS_DNS + 103)
 
-#define DNS_R_NRESULTS			103	/*%< Number of results */
+#define DNS_R_NRESULTS			104	/*%< Number of results */
 
 /*
  * DNS wire format rcodes.

lib/dns/include/dns/stats.h

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: stats.h,v 1.7 2005/04/29 00:23:04 marka Exp $ */
+/* $Id: stats.h,v 1.8 2005/06/17 01:58:23 marka Exp $ */
 
 #ifndef DNS_STATS_H
 #define DNS_STATS_H 1
@@ -33,10 +33,11 @@ typedef enum {
 	dns_statscounter_nxrrset = 2,    /*%< NXRRSET result */
 	dns_statscounter_nxdomain = 3,   /*%< NXDOMAIN result */
 	dns_statscounter_recursion = 4,  /*%< Recursion was used */
-	dns_statscounter_failure = 5     /*%< Some other failure */
+	dns_statscounter_failure = 5,    /*%< Some other failure */
+	dns_statscounter_duplicate = 6   /*%< Duplicate query */
 } dns_statscounter_t;
 
-#define DNS_STATS_NCOUNTERS 6
+#define DNS_STATS_NCOUNTERS 7
 
 LIBDNS_EXTERNAL_DATA extern const char *dns_statscounter_names[];
 

lib/dns/resolver.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: resolver.c,v 1.310 2005/06/07 00:27:33 marka Exp $ */
+/* $Id: resolver.c,v 1.311 2005/06/17 01:58:22 marka Exp $ */
 
 /*! \file */
 
@@ -2633,8 +2633,9 @@ fctx_start(isc_task_t *task, isc_event_t *event) {
  */
 
 static inline isc_result_t
-fctx_join(fetchctx_t *fctx, isc_task_t *task, isc_taskaction_t action,
-	  void *arg, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset,
+fctx_join(fetchctx_t *fctx, isc_task_t *task, isc_sockaddr_t *client,
+	  dns_messageid_t id, isc_taskaction_t action, void *arg,
+	  dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset,
 	  dns_fetch_t *fetch)
 {
 	isc_task_t *clone;
@@ -2664,6 +2665,8 @@ fctx_join(fetchctx_t *fctx, isc_task_t *task, isc_taskaction_t action,
 	event->rdataset = rdataset;
 	event->sigrdataset = sigrdataset;
 	event->fetch = fetch;
+	event->client = client;
+	event->id = id;
 	dns_fixedname_init(&event->foundname);
 
 	/*
@@ -6156,6 +6159,24 @@ dns_resolver_createfetch(dns_resolver_t *res, dns_name_t *name,
 			 dns_rdataset_t *rdataset,
 			 dns_rdataset_t *sigrdataset,
 			 dns_fetch_t **fetchp)
+{
+	return (dns_resolver_createfetch2(res, name, type, domain,
+					  nameservers, forwarders, NULL, 0,
+					  options, task, action, arg,
+					  rdataset, sigrdataset, fetchp));
+}
+
+isc_result_t
+dns_resolver_createfetch2(dns_resolver_t *res, dns_name_t *name,
+			  dns_rdatatype_t type,
+			  dns_name_t *domain, dns_rdataset_t *nameservers,
+			  dns_forwarders_t *forwarders,
+			  isc_sockaddr_t *client, dns_messageid_t id,
+			  unsigned int options, isc_task_t *task,
+			  isc_taskaction_t action, void *arg,
+			  dns_rdataset_t *rdataset,
+			  dns_rdataset_t *sigrdataset,
+			  dns_fetch_t **fetchp)
 {
 	dns_fetch_t *fetch;
 	fetchctx_t *fctx = NULL;
@@ -6206,6 +6227,22 @@ dns_resolver_createfetch(dns_resolver_t *res, dns_name_t *name,
 				break;
 		}
 	}
+	
+	/*
+	 * Is this a duplicate?
+	 */
+	if (fctx != NULL && client != NULL) {
+		dns_fetchevent_t *fevent;
+		for (fevent = ISC_LIST_HEAD(fctx->events);
+		     fevent != NULL;
+		     fevent = ISC_LIST_NEXT(fevent, ev_link)) {
+			if (fevent->client != NULL && fevent->id == id &&
+			    isc_sockaddr_equal(fevent->client, client)) {
+				result = DNS_R_DUPLICATE;
+				goto unlock;
+			}
+		}
+	}
 
 	/*
 	 * If we didn't have a fetch, would attach to a done fetch, this
@@ -6225,7 +6262,7 @@ dns_resolver_createfetch(dns_resolver_t *res, dns_name_t *name,
 		new_fctx = ISC_TRUE;
 	}
 
-	result = fctx_join(fctx, task, action, arg,
+	result = fctx_join(fctx, task, client, id, action, arg,
 			   rdataset, sigrdataset, fetch);
 	if (new_fctx) {
 		if (result == ISC_R_SUCCESS) {

lib/dns/result.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: result.c,v 1.120 2005/05/19 04:59:03 marka Exp $ */
+/* $Id: result.c,v 1.121 2005/06/17 01:58:22 marka Exp $ */
 
 /*! \file */
 
@@ -154,7 +154,8 @@ static const char *text[DNS_R_NRESULTS] = {
 
 	"must-be-secure",		       /*%< 100 DNS_R_MUSTBESECURE */
 	"covering NSEC record returned",       /*%< 101 DNS_R_COVERINGNSEC */
-	"MX is an address"		       /*%< 102 DNS_R_MXISADDRESS */
+	"MX is an address",		       /*%< 102 DNS_R_MXISADDRESS */
+	"duplicate query"		       /*%< 103 DNS_R_DUPLICATE */
 };
 
 static const char *rcode_text[DNS_R_NRCODERESULTS] = {

lib/dns/stats.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: stats.c,v 1.8 2005/04/29 00:22:52 marka Exp $ */
+/* $Id: stats.c,v 1.9 2005/06/17 01:58:22 marka Exp $ */
 
 /*! \file */
 
@@ -32,7 +32,8 @@ LIBDNS_EXTERNAL_DATA const char *dns_statscounter_names[DNS_STATS_NCOUNTERS] =
 	"nxrrset",
 	"nxdomain",
 	"recursion",
-	"failure"
+	"failure",
+	"duplicate"
 	};
 
 isc_result_t