From: Mark Andrews Date: Wed, 21 May 2014 00:08:52 +0000 (+1000) Subject: 3855. [bug] Limit smoothed round trip time aging to no more than X-Git-Tag: v9.11.0a1~1618 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0fe07891819138ad6e1de45f279cff940d170542;p=thirdparty%2Fbind9.git 3855. [bug] Limit smoothed round trip time aging to no more than once a second. [RT #32909] --- diff --git a/CHANGES b/CHANGES index 1b68d90139a..a850f007dbd 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +3855. [bug] Limit smoothed round trip time aging to no more than + once a second. [RT #32909] + 3854. [cleanup] Report unrecognized options, if any, in the final configure summary. [RT #36014] diff --git a/lib/dns/adb.c b/lib/dns/adb.c index de86f98a439..601eeebf60a 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -259,6 +259,7 @@ struct dns_adbentry { isc_uint16_t sitlen; isc_stdtime_t expires; + isc_stdtime_t lastage; /*%< * A nonzero 'expires' field indicates that the entry should * persist until that time. This allows entries found @@ -334,6 +335,8 @@ static inline isc_boolean_t unlink_entry(dns_adb_t *, dns_adbentry_t *); static isc_boolean_t kill_name(dns_adbname_t **, isc_eventtype_t); static void water(void *, int); static void dump_entry(FILE *, dns_adbentry_t *, isc_boolean_t, isc_stdtime_t); +static void adjustsrtt(dns_adbaddrinfo_t *addr, unsigned int rtt, + unsigned int factor, isc_stdtime_t now); /* * MUST NOT overlap DNS_ADBFIND_* flags! @@ -1808,6 +1811,7 @@ new_adbentry(dns_adb_t *adb) { e->sitlen = 0; isc_random_get(&r); e->srtt = (r & 0x1f) + 1; + e->lastage = 0; e->expires = 0; ISC_LIST_INIT(e->lameinfo); ISC_LINK_INIT(e, plink); @@ -4000,8 +4004,7 @@ dns_adb_adjustsrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, unsigned int rtt, unsigned int factor) { int bucket; - isc_uint64_t new_srtt; - isc_stdtime_t now; + isc_stdtime_t now = 0; REQUIRE(DNS_ADB_VALID(adb)); REQUIRE(DNS_ADBADDRINFO_VALID(addr)); @@ -4010,12 +4013,43 @@ dns_adb_adjustsrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, bucket = addr->entry->lock_bucket; LOCK(&adb->entrylocks[bucket]); + if (addr->entry->expires == 0 || factor == DNS_ADB_RTTADJAGE) + isc_stdtime_get(&now); + adjustsrtt(addr, rtt, factor, now); + + UNLOCK(&adb->entrylocks[bucket]); +} + +void +dns_adb_agesrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, isc_stdtime_t now) { + int bucket; + + REQUIRE(DNS_ADB_VALID(adb)); + REQUIRE(DNS_ADBADDRINFO_VALID(addr)); + + bucket = addr->entry->lock_bucket; + LOCK(&adb->entrylocks[bucket]); + + adjustsrtt(addr, 0, DNS_ADB_RTTADJAGE, now); + + UNLOCK(&adb->entrylocks[bucket]); +} + +static void +adjustsrtt(dns_adbaddrinfo_t *addr, unsigned int rtt, unsigned int factor, + isc_stdtime_t now) +{ + isc_uint64_t new_srtt; if (factor == DNS_ADB_RTTADJAGE) { - new_srtt = addr->entry->srtt; - new_srtt <<= 9; - new_srtt -= addr->entry->srtt; - new_srtt >>= 9; + if (addr->entry->lastage != now) { + new_srtt = addr->entry->srtt; + new_srtt <<= 9; + new_srtt -= addr->entry->srtt; + new_srtt >>= 9; + addr->entry->lastage = now; + } else + new_srtt = addr->entry->srtt; } else new_srtt = (addr->entry->srtt / 10 * factor) + (rtt / 10 * (10 - factor)); @@ -4023,12 +4057,8 @@ dns_adb_adjustsrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, addr->entry->srtt = (unsigned int) new_srtt; addr->srtt = (unsigned int) new_srtt; - if (addr->entry->expires == 0) { - isc_stdtime_get(&now); + if (addr->entry->expires == 0) addr->entry->expires = now + ADB_ENTRY_WINDOW; - } - - UNLOCK(&adb->entrylocks[bucket]); } void diff --git a/lib/dns/include/dns/adb.h b/lib/dns/include/dns/adb.h index d13943d6056..0383f35f044 100644 --- a/lib/dns/include/dns/adb.h +++ b/lib/dns/include/dns/adb.h @@ -512,7 +512,12 @@ dns_adb_marklame(dns_adb_t *adb, dns_adbaddrinfo_t *addr, dns_name_t *qname, */ /* - * A reasonable default for RTT adjustments + * Reasonable defaults for RTT adjustments + * + * (Note: these values function both as scaling factors and as + * indicators of the type of RTT adjustment operation taking place. + * Adjusting the scaling factors is fine, as long as they all remain + * unique values.) */ #define DNS_ADB_RTTADJDEFAULT 7 /*%< default scale */ #define DNS_ADB_RTTADJREPLACE 0 /*%< replace with our rtt */ @@ -523,18 +528,6 @@ dns_adb_adjustsrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, unsigned int rtt, unsigned int factor); /*%< * Mix the round trip time into the existing smoothed rtt. - - * The formula used - * (where srtt is the existing rtt value, and rtt and factor are arguments to - * this function): - * - *\code - * new_srtt = (old_srtt / 10 * factor) + (rtt / 10 * (10 - factor)); - *\endcode - * - * XXXRTH Do we want to publish the formula? What if we want to change how - * this works later on? Recommend/require that the units are - * microseconds? * * Requires: * @@ -550,6 +543,24 @@ dns_adb_adjustsrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, * srtt value. This may include changes made by others. */ +void +dns_adb_agesrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, isc_stdtime_t now); +/* + * dns_adb_agesrtt is equivalent to dns_adb_adjustsrtt with factor + * equal to DNS_ADB_RTTADJAGE and the current time passed in. + * + * Requires: + * + *\li adb be valid. + * + *\li addr be valid. + * + * Note: + * + *\li The srtt in addr will be updated to reflect the new global + * srtt value. This may include changes made by others. + */ + void dns_adb_changeflags(dns_adb_t *adb, dns_adbaddrinfo_t *addr, unsigned int bits, unsigned int mask); diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index debfc7eb92b..3437a265ba3 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -820,6 +820,7 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, dns_adbfind_t *find; dns_adbaddrinfo_t *addrinfo; isc_socket_t *socket; + isc_stdtime_t now; query = *queryp; fctx = query->fctx; @@ -925,13 +926,13 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, * Age RTTs of servers not tried. */ factor = DNS_ADB_RTTADJAGE; + isc_stdtime_get(&now); if (finish != NULL) for (addrinfo = ISC_LIST_HEAD(fctx->forwaddrs); addrinfo != NULL; addrinfo = ISC_LIST_NEXT(addrinfo, publink)) if (UNMARKED(addrinfo)) - dns_adb_adjustsrtt(fctx->adb, addrinfo, - 0, factor); + dns_adb_agesrtt(fctx->adb, addrinfo, now); if (finish != NULL && TRIEDFIND(fctx)) for (find = ISC_LIST_HEAD(fctx->finds); @@ -941,16 +942,15 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, addrinfo != NULL; addrinfo = ISC_LIST_NEXT(addrinfo, publink)) if (UNMARKED(addrinfo)) - dns_adb_adjustsrtt(fctx->adb, addrinfo, - 0, factor); + dns_adb_agesrtt(fctx->adb, addrinfo, + now); if (finish != NULL && TRIEDALT(fctx)) { for (addrinfo = ISC_LIST_HEAD(fctx->altaddrs); addrinfo != NULL; addrinfo = ISC_LIST_NEXT(addrinfo, publink)) if (UNMARKED(addrinfo)) - dns_adb_adjustsrtt(fctx->adb, addrinfo, - 0, factor); + dns_adb_agesrtt(fctx->adb, addrinfo, now); for (find = ISC_LIST_HEAD(fctx->altfinds); find != NULL; find = ISC_LIST_NEXT(find, publink)) @@ -958,8 +958,8 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, addrinfo != NULL; addrinfo = ISC_LIST_NEXT(addrinfo, publink)) if (UNMARKED(addrinfo)) - dns_adb_adjustsrtt(fctx->adb, addrinfo, - 0, factor); + dns_adb_agesrtt(fctx->adb, addrinfo, + now); } /*