lib/resolve: even better support for forwarder choice

- sort the list instead of just picking the best one
- prefer unknown RTTs to probe them
- verbose output of the choice

Fixes https://gitlab.labs.nic.cz/knot/resolver/issues/125
Fixes https://gitlab.labs.nic.cz/knot/resolver/issues/208

diff --git a/NEWS b/NEWS
index 32881250a4980ab80c5954671cc4cbb3b524b439..3987208ccd96ecf4a812435b49ed40777e794a1f 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,7 @@ Knot Resolver 1.3.2 (2017-07-xx)
 Bugfixes
 --------
 - daemon: check existence of config file even if rundir isn't specified
+- policy.FORWARD and STUB: use RTT tracking to choose servers (#125, #208)
 
 
 Knot Resolver 1.3.1 (2017-06-23)

diff --git a/lib/nsrep.c b/lib/nsrep.c
index cdb7a2daf43776daac98acb9807953745de53610..b66b2a019f6399ec91846cf586dffa811e6d1c54 100644 (file)
--- a/lib/nsrep.c
+++ b/lib/nsrep.c
@@ -19,6 +19,8 @@
 #include <netinet/in.h>
 #include <netdb.h>
 
+#include <arpa/inet.h>
+
 #include "lib/nsrep.h"
 #include "lib/rplan.h"
 #include "lib/resolve.h"
@@ -336,10 +338,12 @@ int kr_nsrep_copy_set(struct kr_nsrep *dst, const struct kr_nsrep *src)
        return kr_ok();
 }
 
-int kr_nsrep_elect_min_rtt(struct kr_query *qry)
+int kr_nsrep_sort(struct kr_nsrep *ns, kr_nsrep_lru_t *cache)
 {
-       /* TODO - sorting instead of searching minimum? */
-       struct kr_nsrep *ns = &qry->ns;
+       if (!ns || !cache) {
+               assert(false);
+               return kr_error(EINVAL);
+       }
 
        if (ns->addr[0].ip.sa_family == AF_UNSPEC) {
                return kr_error(EINVAL);
@@ -350,41 +354,52 @@ int kr_nsrep_elect_min_rtt(struct kr_query *qry)
                return kr_ok();
        }
 
-       const struct kr_context *ctx = ns->ctx;
-       if (!ctx) {
-               return kr_ok();
+       /* Compute the scores.  Unfortunately there's no space for scores
+        * along the addresses. */
+       unsigned scores[KR_NSREP_MAXADDR];
+       int i;
+       for (i = 0; i < KR_NSREP_MAXADDR; ++i) {
+               const struct sockaddr *sa = &ns->addr[i].ip;
+               if (sa->sa_family == AF_UNSPEC) {
+                       break;
+               }
+               unsigned *score = lru_get_try(cache, kr_inaddr(sa),
+                                               kr_family_len(sa->sa_family));
+               if (!score) {
+                       scores[i] = 1; /* prefer unknown to probe RTT */
+               } else if ((kr_rand_uint(100) < 10)
+                               && (kr_rand_uint(KR_NS_MAX_SCORE) >= *score)) {
+                       /* some probability to bump bad ones up for re-probe */
+                       scores[i] = 1;
+               } else {
+                       scores[i] = *score;
+               }
+               WITH_VERBOSE {
+                       char sa_str[INET6_ADDRSTRLEN];
+                       inet_ntop(sa->sa_family, kr_inaddr(sa), sa_str, sizeof(sa_str));
+                       kr_log_verbose("[     ][nsre] score %d for %s;\t cached RTT: %d\n",
+                                       scores[i], sa_str, score ? *score : -1);
+               }
        }
 
-       const struct sockaddr *sock = (const struct sockaddr *)(&ns->addr[0]);
-       unsigned *score = lru_get_try(ctx->cache_rtt, kr_inaddr(sock),
-                                     kr_family_len(sock->sa_family));
-       unsigned minimal_score = (score) ? *score : KR_NS_MAX_SCORE + 1;
-       int i = 1;
-       sock = (const struct sockaddr *)(&ns->addr[i]);
-       do {
-               score = lru_get_try(ctx->cache_rtt, kr_inaddr(sock),
-                                   kr_family_len(sock->sa_family));
-               if (score) {
-                       if (*score < minimal_score) {
-                               union inaddr temp_addr = ns->addr[0];
-                               ns->addr[0] = ns->addr[i];
-                               ns->addr[i] = temp_addr;
-                               ns->score = *score;
-                               ns->reputation = 0;
-                               minimal_score = ns->score;
-                       } else if ((kr_rand_uint(100) < 10) &&
-                           (kr_rand_uint(KR_NS_MAX_SCORE) >= *score)) {
-                               /* long distance probe */
-                               union inaddr temp_addr = ns->addr[0];
-                               ns->addr[0] = ns->addr[i];
-                               ns->addr[i] = temp_addr;
-                               ns->score = *score;
-                               ns->reputation = 0;
-                               break;
+       /* Select-sort the addresses. */
+       const int count = i;
+       for (i = 0; i < count - 1; ++i) {
+               /* find min from i onwards */
+               int min_i = i;
+               for (int j = i + 1; j < count; ++j) {
+                       if (scores[j] < scores[min_i]) {
+                               min_i = j;
                        }
                }
-               sock = (const struct sockaddr *)(&ns->addr[++i]);
-       } while ((i < KR_NSREP_MAXADDR) && (sock->sa_family != AF_UNSPEC));
+               /* swap the indices */
+               if (min_i != i) {
+                       SWAP(scores[min_i], scores[i]);
+                       SWAP(ns->addr[min_i], ns->addr[i]);
+               }
+       }
 
+       ns->score = scores[0];
+       ns->reputation = 0;
        return kr_ok();
 }

diff --git a/lib/nsrep.h b/lib/nsrep.h
index b4a306b52b9784de17f53aa82c289098cff2de0d..b0cdd3fcc266fcc862d3d97c1cace2057dee45a4 100644 (file)
--- a/lib/nsrep.h
+++ b/lib/nsrep.h
@@ -145,10 +145,13 @@ int kr_nsrep_update_rep(struct kr_nsrep *ns, unsigned reputation, kr_nsrep_lru_t
 int kr_nsrep_copy_set(struct kr_nsrep *dst, const struct kr_nsrep *src);
 
 /**
- * Elect address with minimal rtt within current query nsrep list
- * @note   ns reputation is zeroed
- * @param  qry          updated query
+ * Sort addresses in the query nsrep list
+ * @param  ns           updated kr_nsrep
+ * @param  cache        RTT cache
  * @return              0 or an error code
+ * @note   ns reputation is zeroed, as KR_NS_NOIP{4,6} flags are useless
+ *             in STUB/FORWARD mode.
  */
 KR_EXPORT
-int kr_nsrep_elect_min_rtt(struct kr_query *qry);
+int kr_nsrep_sort(struct kr_nsrep *ns, kr_nsrep_lru_t *cache);
+

diff --git a/lib/resolve.c b/lib/resolve.c
index 30a3b4693aded1f92b0669597cecc76fc5830ca2..682433b65b939fceebae518e056365fa23c7c21b 100644 (file)
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -1378,11 +1378,11 @@ ns_election:
                return KR_STATE_FAIL;
        }
 
-       const bool retry = (qry->flags & (QUERY_TCP|QUERY_STUB|QUERY_BADCOOKIE_AGAIN));
+       const bool retry = (qry->flags & (QUERY_TCP|QUERY_BADCOOKIE_AGAIN));
        if (qry->flags & (QUERY_AWAIT_IPV4|QUERY_AWAIT_IPV6)) {
                kr_nsrep_elect_addr(qry, request->ctx);
-       } else if (qry->flags & QUERY_FORWARD) {
-               kr_nsrep_elect_min_rtt(qry);
+       } else if (qry->flags & (QUERY_FORWARD|QUERY_STUB)) {
+               kr_nsrep_sort(&qry->ns, request->ctx->cache_rtt);
        } else if (!qry->ns.name || !retry) { /* Keep NS when requerying/stub/badcookie. */
                /* Root DNSKEY must be fetched from the hints to avoid chicken and egg problem. */
                if (qry->sname[0] == '\0' && qry->stype == KNOT_RRTYPE_DNSKEY) {