3855. [bug] Limit smoothed round trip time aging to no more than

                        once a second. [RT #32909]

(cherry picked from commit 0fe07891819138ad6e1de45f279cff940d170542)

diff --git a/CHANGES b/CHANGES
index 3e246486944dc6a2626959c31601fc9c03086cef..16aac4da19278d1ca3c5ba973efc5c47b25878d2 100644 (file)
--- 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 de86f98a439856fead2fcebc66ec962261ca7090..601eeebf60a35f49bd1cd5bf1c758f5bedb3d7e5 100644 (file)
--- 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 d13943d60569dbeca8aece2a1400a445179e3553..0383f35f04495223e6b0f251ad4e590748baf8f7 100644 (file)
--- 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 debfc7eb92bddc52b4134e58449787810d06234d..3437a265ba31eb7acbf93e97eb6498adb503ccfd 100644 (file)
--- 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);
        }
 
        /*