From e021d8eff8fb889dca1f84d07fbb9193411ca09a Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 27 Jun 2005 00:20:04 +0000 Subject: [PATCH] 1891. [func] Limit the number of recursive clients that can be waiting for a single query () to resolve. New options clients-per-query and max-clients-per-query. --- CHANGES | 5 ++ bin/named/config.c | 4 +- bin/named/named.conf.docbook | 8 +- bin/named/query.c | 12 ++- bin/named/server.c | 15 +++- doc/arm/Bv9ARM-book.xml | 40 ++++++++- lib/dns/include/dns/resolver.h | 10 ++- lib/dns/include/dns/stats.h | 7 +- lib/dns/resolver.c | 145 +++++++++++++++++++++++++++++++-- lib/dns/stats.c | 5 +- lib/isccfg/namedconf.c | 4 +- 11 files changed, 233 insertions(+), 22 deletions(-) diff --git a/CHANGES b/CHANGES index 7ebf5edc9b..9db2868c3d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +1891. [func] Limit the number of recursive clients that can be + waiting for a single query () to + resolve. New options clients-per-query and + max-clients-per-query. + 1890. [func] Add a system test for named-checkconf. [RT #14931] 1889. [func] The lame cache is now done on a diff --git a/bin/named/config.c b/bin/named/config.c index d88fff8cef..7bb14b6680 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: config.c,v 1.47.18.10 2005/05/19 04:59:50 marka Exp $ */ +/* $Id: config.c,v 1.47.18.11 2005/06/27 00:19:55 marka Exp $ */ /*! \file */ @@ -134,6 +134,8 @@ options {\n\ max-acache-size 0;\n\ dnssec-enable no; /* Make yes for 9.4. */ \n\ integrity-check yes;\n\ + clients-per-query 10;\n\ + max-clients-per-query 100;\n\ " " /* zone */\n\ diff --git a/bin/named/named.conf.docbook b/bin/named/named.conf.docbook index 49f36dc0d6..a529f9fc62 100644 --- a/bin/named/named.conf.docbook +++ b/bin/named/named.conf.docbook @@ -17,7 +17,7 @@ - PERFORMANCE OF THIS SOFTWARE. --> - + Aug 13, 2004 @@ -239,6 +239,9 @@ options { max-cache-ttl integer; transfer-format ( many-answers | one-answer ); max-cache-size size_no_default; + max-acache-size size_no_default; + clients-per-query number; + max-clients-per-query number; check-names ( master | slave | response ) ( fail | warn | ignore ); check-mx ( fail | warn | ignore ); @@ -371,6 +374,9 @@ view string optional_class max-cache-ttl integer; transfer-format ( many-answers | one-answer ); max-cache-size size_no_default; + max-acache-size size_no_default; + clients-per-query number; + max-clients-per-query number; check-names ( master | slave | response ) ( fail | warn | ignore ); check-mx ( fail | warn | ignore ); diff --git a/bin/named/query.c b/bin/named/query.c index da6b9df4b3..f36fd61b91 100644 --- a/bin/named/query.c +++ b/bin/named/query.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: query.c,v 1.257.18.11 2005/06/22 22:05:42 marka Exp $ */ +/* $Id: query.c,v 1.257.18.12 2005/06/27 00:19:57 marka Exp $ */ /*! \file */ @@ -163,6 +163,8 @@ static void query_next(ns_client_t *client, isc_result_t result) { if (result == DNS_R_DUPLICATE) inc_stats(client, dns_statscounter_duplicate); + else if (result == DNS_R_DROP) + inc_stats(client, dns_statscounter_dropped); else inc_stats(client, dns_statscounter_failure); ns_client_next(client, result); @@ -3228,7 +3230,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) { + else if (result == DNS_R_DUPLICATE || + result == DNS_R_DROP) { /* Duplicate query. */ QUERY_ERROR(result); } else { @@ -3401,7 +3404,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) + else if (result == DNS_R_DUPLICATE || + result == DNS_R_DROP) QUERY_ERROR(result); else QUERY_ERROR(DNS_R_SERVFAIL); @@ -3944,7 +3948,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype) if (eresult != ISC_R_SUCCESS && (!PARTIALANSWER(client) || WANTRECURSION(client))) { - if (eresult == DNS_R_DUPLICATE) { + if (eresult == DNS_R_DUPLICATE || eresult == DNS_R_DROP) { /* * This was a duplicate query that we are * recursing on. Don't send a response now. diff --git a/bin/named/server.c b/bin/named/server.c index 7ebe327136..5477bce320 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: server.c,v 1.419.18.26 2005/06/22 22:05:42 marka Exp $ */ +/* $Id: server.c,v 1.419.18.27 2005/06/27 00:19:57 marka Exp $ */ /*! \file */ @@ -764,6 +764,7 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, dns_order_t *order = NULL; isc_uint32_t udpsize; unsigned int check = 0; + isc_uint32_t max_clients_per_query; REQUIRE(DNS_VIEW_VALID(view)); @@ -1243,6 +1244,18 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, result = ns_config_get(maps, "provide-ixfr", &obj); INSIST(result == ISC_R_SUCCESS); view->provideixfr = cfg_obj_asboolean(obj); + + obj = NULL; + result = ns_config_get(maps, "max-clients-per-query", &obj); + INSIST(result == ISC_R_SUCCESS); + max_clients_per_query = cfg_obj_asuint32(obj); + + obj = NULL; + result = ns_config_get(maps, "clients-per-query", &obj); + INSIST(result == ISC_R_SUCCESS); + dns_resolver_setclientsperquery(view->resolver, + cfg_obj_asuint32(obj), + max_clients_per_query); obj = NULL; result = ns_config_get(maps, "dnssec-enable", &obj); diff --git a/doc/arm/Bv9ARM-book.xml b/doc/arm/Bv9ARM-book.xml index e30e18bb9c..148d2cee95 100644 --- a/doc/arm/Bv9ARM-book.xml +++ b/doc/arm/Bv9ARM-book.xml @@ -18,7 +18,7 @@ - PERFORMANCE OF THIS SOFTWARE. --> - + BIND 9 Administrator Reference Manual @@ -1522,6 +1522,42 @@ controls { + + + clients-per-query + max-clients-per-query + + clients-per-query + and max-clients-per-query set the + initial value (minimum) and maximum number of recursive + simultanious clients for any given query + (<qname,qtype,qclass>) that the server will accept + before dropping additional clients. named will attempt to + self tune this value and changes will be logged. The + default values are 10 and 100. + + + This value should reflect how many queries come in for + a given name in the time it takes to resolve that name. + If the number of queries exceed this value named will + assume that it is dealing with a non-responsive zone + and will drop additional queries. If it gets a response + after dropping queries it will raise the estimate. The + estimate will then be lowered in 20 minutes if it has + remained unchanged. + + + If clients-per-query is set to zero + then there is no limit on the number of clients per query + and no queries will be dropped. + + + If max-clients-per-query is set to zero + then there is no upper bound other than imposed by + recurive-clients. + + + @@ -4396,6 +4432,8 @@ category notify { null; }; use-additional-cache yes_or_no ; acache-cleaning-interval number; max-acache-size size_spec ; + clients-per-query number ; + max-clients-per-query number ; masterfile-format (text|raw) ; }; diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h index 4e207b7391..65a4040830 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: resolver.h,v 1.40.18.6 2005/06/17 02:04:32 marka Exp $ */ +/* $Id: resolver.h,v 1.40.18.7 2005/06/27 00:20:02 marka Exp $ */ #ifndef DNS_RESOLVER_H #define DNS_RESOLVER_H 1 @@ -309,6 +309,7 @@ dns_resolver_createfetch2(dns_resolver_t *res, dns_name_t *name, * *\li #ISC_R_SUCCESS Success *\li #DNS_R_DUPLICATE + *\li #DNS_R_DROP * *\li Many other values are possible, all of which indicate failure. */ @@ -457,6 +458,13 @@ dns_resolver_setmustbesecure(dns_resolver_t *resolver, dns_name_t *name, isc_boolean_t dns_resolver_getmustbesecure(dns_resolver_t *resolver, dns_name_t *name); +void +dns_resolver_setclientsperquery(dns_resolver_t *resolver, + isc_uint32_t min, isc_uint32_t max); + +void +dns_resolver_getclientsperquery(dns_resolver_t *resolver, isc_uint32_t *cur, + isc_uint32_t *min, isc_uint32_t *max); ISC_LANG_ENDDECLS #endif /* DNS_RESOLVER_H */ diff --git a/lib/dns/include/dns/stats.h b/lib/dns/include/dns/stats.h index 08968cc23f..6cd95acaed 100644 --- a/lib/dns/include/dns/stats.h +++ b/lib/dns/include/dns/stats.h @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: stats.h,v 1.5.18.3 2005/06/17 02:04:33 marka Exp $ */ +/* $Id: stats.h,v 1.5.18.4 2005/06/27 00:20:03 marka Exp $ */ #ifndef DNS_STATS_H #define DNS_STATS_H 1 @@ -34,10 +34,11 @@ typedef enum { dns_statscounter_nxdomain = 3, /*%< NXDOMAIN result */ dns_statscounter_recursion = 4, /*%< Recursion was used */ dns_statscounter_failure = 5, /*%< Some other failure */ - dns_statscounter_duplicate = 6 /*%< Duplicate query */ + dns_statscounter_duplicate = 6, /*%< Duplicate query */ + dns_statscounter_dropped = 7 /*%< Duplicate query */ } dns_statscounter_t; -#define DNS_STATS_NCOUNTERS 7 +#define DNS_STATS_NCOUNTERS 8 LIBDNS_EXTERNAL_DATA extern const char *dns_statscounter_names[]; diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 79ddb4d220..82c5f2f533 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: resolver.c,v 1.284.18.29 2005/06/23 06:14:52 marka Exp $ */ +/* $Id: resolver.c,v 1.284.18.30 2005/06/27 00:20:00 marka Exp $ */ /*! \file */ @@ -166,6 +166,7 @@ struct fetchctx { fetchstate state; isc_boolean_t want_shutdown; isc_boolean_t cloned; + isc_boolean_t spilled; unsigned int references; isc_event_t control_event; ISC_LINK(struct fetchctx) link; @@ -307,12 +308,16 @@ struct dns_resolver { isc_rwlock_t mbslock; #endif dns_rbt_t * mustbesecure; + unsigned int spillatmax; + unsigned int spillatmin; + isc_timer_t * spillattimer; /* Locked by lock. */ unsigned int references; isc_boolean_t exiting; isc_eventlist_t whenshutdown; unsigned int activebuckets; isc_boolean_t priming; + unsigned int spillat; /* Locked by primelock. */ dns_fetch_t * primefetch; /* Locked by nlock. */ @@ -433,8 +438,7 @@ fctx_starttimer(fetchctx_t *fctx) { * no further idle events are delivered. */ return (isc_timer_reset(fctx->timer, isc_timertype_once, - &fctx->expires, NULL, - ISC_TRUE)); + &fctx->expires, NULL, ISC_TRUE)); } static inline void @@ -713,6 +717,9 @@ static inline void fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { dns_fetchevent_t *event, *next_event; isc_task_t *task; + unsigned int count = 0; + isc_interval_t i; + isc_boolean_t logit = ISC_FALSE; /* * Caller must be holding the appropriate bucket lock. @@ -737,6 +744,31 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { fctx->type == dns_rdatatype_rrsig); isc_task_sendanddetach(&task, ISC_EVENT_PTR(&event)); + count++; + } + + if ((fctx->attributes & FCTX_ATTR_HAVEANSWER) != 0 && + fctx->spilled && + (count < fctx->res->spillatmax || fctx->res->spillatmax == 0)) { + LOCK(&fctx->res->lock); + if (count == fctx->res->spillat && !fctx->res->exiting) { + fctx->res->spillat += 5; + if (fctx->res->spillat > fctx->res->spillatmax && + fctx->res->spillatmax != 0) + fctx->res->spillat = fctx->res->spillatmax; + isc_interval_set(&i, 20 * 60, 0); + result = isc_timer_reset(fctx->res->spillattimer, + isc_timertype_ticker, NULL, + &i, ISC_TRUE); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + logit = ISC_TRUE; + } + UNLOCK(&fctx->res->lock); + if (logit) + isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_NOTICE, + "clients-per-query increased to %u", + count + 1); } } @@ -2726,6 +2758,7 @@ fctx_create(dns_resolver_t *res, dns_name_t *name, dns_rdatatype_t type, fctx->restarts = 0; fctx->timeouts = 0; fctx->attributes = 0; + fctx->spilled = ISC_FALSE; dns_name_init(&fctx->nsname, NULL); fctx->nsfetch = NULL; @@ -5665,6 +5698,7 @@ destroy(dns_resolver_t *res) { #if USE_MBSLOCK isc_rwlock_destroy(&res->mbslock); #endif + isc_timer_detach(&res->spillattimer); res->magic = 0; isc_mem_put(res->mctx, res, sizeof(*res)); } @@ -5703,11 +5737,42 @@ empty_bucket(dns_resolver_t *res) { UNLOCK(&res->lock); } +static void +spillattimer_countdown(isc_task_t *task, isc_event_t *event) { + dns_resolver_t *res = event->ev_arg; + isc_result_t result; + unsigned int count; + isc_boolean_t logit = ISC_FALSE; + + REQUIRE(VALID_RESOLVER(res)); + + UNUSED(task); + + LOCK(&res->lock); + INSIST(!res->exiting); + if (res->spillat > res->spillatmin) { + res->spillat--; + logit = ISC_TRUE; + } + if (res->spillat <= res->spillatmin) { + result = isc_timer_reset(res->spillattimer, + isc_timertype_inactive, NULL, + NULL, ISC_TRUE); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + } + count = res->spillat; + UNLOCK(&res->lock); + if (logit) + isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_NOTICE, + "clients-per-query decreased to %u", count); +} + isc_result_t dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr, unsigned int ntasks, isc_socketmgr_t *socketmgr, - isc_timermgr_t *timermgr, + isc_timermgr_t *timermgr, unsigned int options, dns_dispatchmgr_t *dispatchmgr, dns_dispatch_t *dispatchv4, @@ -5717,6 +5782,7 @@ dns_resolver_create(dns_view_t *view, dns_resolver_t *res; isc_result_t result = ISC_R_SUCCESS; unsigned int i, buckets_created = 0; + isc_task_t *task = NULL; char name[16]; /* @@ -5746,6 +5812,9 @@ dns_resolver_create(dns_view_t *view, res->udpsize = RECV_BUFFER_SIZE; res->algorithms = NULL; res->mustbesecure = NULL; + res->spillatmin = res->spillat = 10; + res->spillatmax = 100; + res->spillattimer = NULL; res->nbuckets = ntasks; res->activebuckets = ntasks; @@ -5806,10 +5875,22 @@ dns_resolver_create(dns_view_t *view, if (result != ISC_R_SUCCESS) goto cleanup_nlock; + task = NULL; + result = isc_task_create(taskmgr, 0, &task); + if (result != ISC_R_SUCCESS) + goto cleanup_primelock; + + result = isc_timer_create(timermgr, isc_timertype_inactive, NULL, NULL, + task, spillattimer_countdown, res, + &res->spillattimer); + isc_task_detach(&task); + if (result != ISC_R_SUCCESS) + goto cleanup_primelock; + #if USE_ALGLOCK result = isc_rwlock_init(&res->alglock, 0, 0); if (result != ISC_R_SUCCESS) - goto cleanup_primelock; + goto cleanup_spillattimer; #endif #if USE_MBSLOCK result = isc_rwlock_init(&res->mbslock, 0, 0); @@ -5830,9 +5911,12 @@ dns_resolver_create(dns_view_t *view, #endif #endif #if USE_ALGLOCK || USE_MBSLOCK + cleanup_spillattimer: + isc_timer_detach(&res->spillattimer); +#endif + cleanup_primelock: DESTROYLOCK(&res->primelock); -#endif cleanup_nlock: DESTROYLOCK(&res->nlock); @@ -6034,6 +6118,7 @@ dns_resolver_shutdown(dns_resolver_t *res) { unsigned int i; fetchctx_t *fctx; isc_socket_t *sock; + isc_result_t result; REQUIRE(VALID_RESOLVER(res)); @@ -6070,6 +6155,10 @@ dns_resolver_shutdown(dns_resolver_t *res) { } if (res->activebuckets == 0) send_shutdown_events(res); + result = isc_timer_reset(res->spillattimer, + isc_timertype_inactive, NULL, + NULL, ISC_TRUE); + RUNTIME_CHECK(result == ISC_R_SUCCESS); } UNLOCK(&res->lock); @@ -6160,10 +6249,11 @@ dns_resolver_createfetch2(dns_resolver_t *res, dns_name_t *name, { dns_fetch_t *fetch; fetchctx_t *fctx = NULL; - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; unsigned int bucketnum; isc_boolean_t new_fctx = ISC_FALSE; isc_event_t *event; + unsigned int count = 0; UNUSED(forwarders); @@ -6221,6 +6311,19 @@ dns_resolver_createfetch2(dns_resolver_t *res, dns_name_t *name, result = DNS_R_DUPLICATE; goto unlock; } + count++; + } + } + if (count >= res->spillatmin && res->spillatmin != 0) { + if (!fctx->spilled) { + LOCK(&fctx->res->lock); + if (count >= res->spillat) + fctx->spilled = ISC_TRUE; + UNLOCK(&fctx->res->lock); + } + if (fctx->spilled) { + result = DNS_R_DROP; + goto unlock; } } @@ -6669,3 +6772,31 @@ dns_resolver_getmustbesecure(dns_resolver_t *resolver, dns_name_t *name) { #endif return (value); } + +void +dns_resolver_getclientsperquery(dns_resolver_t *resolver, isc_uint32_t *cur, + isc_uint32_t *min, isc_uint32_t *max) +{ + REQUIRE(VALID_RESOLVER(resolver)); + + LOCK(&resolver->lock); + if (cur != NULL) + *cur = resolver->spillat; + if (min != NULL) + *min = resolver->spillatmin; + if (max != NULL) + *max = resolver->spillatmax; + UNLOCK(&resolver->lock); +} + +void +dns_resolver_setclientsperquery(dns_resolver_t *resolver, isc_uint32_t min, + isc_uint32_t max) +{ + REQUIRE(VALID_RESOLVER(resolver)); + + LOCK(&resolver->lock); + resolver->spillatmin = resolver->spillat = min; + resolver->spillatmax = max; + UNLOCK(&resolver->lock); +} diff --git a/lib/dns/stats.c b/lib/dns/stats.c index 7cb992bb7a..660046fe7f 100644 --- a/lib/dns/stats.c +++ b/lib/dns/stats.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: stats.c,v 1.6.18.3 2005/06/17 02:04:31 marka Exp $ */ +/* $Id: stats.c,v 1.6.18.4 2005/06/27 00:20:02 marka Exp $ */ /*! \file */ @@ -33,7 +33,8 @@ LIBDNS_EXTERNAL_DATA const char *dns_statscounter_names[DNS_STATS_NCOUNTERS] = "nxdomain", "recursion", "failure", - "duplicate" + "duplicate", + "dropped" }; isc_result_t diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index 11599e7b4b..7cf3c67fb5 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: namedconf.c,v 1.30.18.22 2005/06/20 01:19:45 marka Exp $ */ +/* $Id: namedconf.c,v 1.30.18.23 2005/06/27 00:20:04 marka Exp $ */ /*! \file */ @@ -745,6 +745,8 @@ view_clauses[] = { { "use-additional-cache", &cfg_type_boolean, 0 }, { "acache-cleaning-interval", &cfg_type_uint32, 0 }, { "max-acache-size", &cfg_type_sizenodefault, 0 }, + { "clients-per-query", &cfg_type_uint32, 0 }, + { "max-clients-per-query", &cfg_type_uint32, 0 }, { NULL, NULL, 0 } };