lib: cleanup servfail soft-fails

* simplified soft-fail per-ns limit to per-query
  limit, each query gets 4 tries at resolving
* instead of locking at single servfailing NS,
  penalise it and run reelection, this may or
  may not try other servers but avoids pathologic
  case when single NS is servfailing while others
  are good but never probed
* added new nsrep update mode (addition)

diff --git a/lib/defines.h b/lib/defines.h
index 895a1b66bf912dd608c2a40a333f40bd7d8713b7..a5f653e4eddaf3a8437f91677287ce55fa3f0ef3 100644 (file)
--- a/lib/defines.h
+++ b/lib/defines.h
@@ -55,10 +55,7 @@ static inline int __attribute__((__cold__)) kr_error(int x) {
 #define KR_ITER_LIMIT 50     /* Built-in iterator limit */
 #define KR_CNAME_CHAIN_LIMIT 40 /* Built-in maximum CNAME chain length */
 #define KR_TIMEOUT_LIMIT 4   /* Maximum number of retries after timeout. */
-#define KR_QUERY_FAIL_LIMIT 40 /* Limit to the number of SERVFAIL\REFUSED
-                                * responses per query
-                                */
-#define KR_QUERY_NSRETRY_LIMIT 3 /* Maximum number of retries for single ns */
+#define KR_QUERY_NSRETRY_LIMIT 4 /* Maximum number of retries per query. */
 
 /*
  * Defines.

diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index 92209af281b9578556d6e8380be1eaf79aac894f..99abc54832327fedde9177312aa368b20263c173 100644 (file)
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -601,12 +601,10 @@ static int resolve(knot_layer_t *ctx, knot_pkt_t *pkt)
        case KNOT_RCODE_SERVFAIL: {
                DEBUG_MSG("<= rcode: %s\n", rcode ? rcode->name : "??");
                query->fails += 1;
-               query->ns.fails += 1;
-               if (query->fails >= KR_QUERY_FAIL_LIMIT ||
-                   query->ns.fails >= KR_QUERY_NSRETRY_LIMIT) {
+               if (query->fails >= KR_QUERY_NSRETRY_LIMIT) {
+                       query->fails = 0; /* Reset per-query counter. */
                        return resolve_error(pkt, req);
                } else {
-                       query->flags |= QUERY_SERVFAIL;
                        return KNOT_STATE_CONSUME;
                }
        }

diff --git a/lib/nsrep.c b/lib/nsrep.c
index 0fad08163952e2a9f7d873a2bddf3661bfe17d1a..e5c918443724cfdfc398ff82664fda0c874c9c1c 100644 (file)
--- a/lib/nsrep.c
+++ b/lib/nsrep.c
@@ -24,6 +24,7 @@
 #include "lib/resolve.h"
 #include "lib/defines.h"
 #include "lib/generic/pack.h"
+#include "contrib/ucw/lib.h"
 
 /** Some built-in unfairness ... */
 #define FAVOUR_IPV6 20 /* 20ms bonus for v6 */
@@ -176,7 +177,6 @@ int kr_nsrep_set(struct kr_query *qry, uint8_t *addr, size_t addr_len)
        qry->ns.name = (const uint8_t *)"";
        qry->ns.score = KR_NS_UNKNOWN;
        qry->ns.reputation = 0;
-       qry->ns.fails = 0;
        update_nsrep(&qry->ns, 0, addr, addr_len);
        update_nsrep(&qry->ns, 1, NULL, 0);
        return kr_ok();
@@ -186,7 +186,6 @@ int kr_nsrep_set(struct kr_query *qry, uint8_t *addr, size_t addr_len)
        (ns)->ctx = (ctx_); \
        (ns)->addr[0].ip.sa_family = AF_UNSPEC; \
        (ns)->reputation = 0; \
-       (ns)->fails = 0; \
        (ns)->score = KR_NS_MAX_SCORE + 1; \
 } while (0)
 
@@ -252,15 +251,17 @@ int kr_nsrep_update_rtt(struct kr_nsrep *ns, const struct sockaddr *addr,
        if (score <= KR_NS_GLUED) {
                score = KR_NS_GLUED + 1;
        }
-
-       if ((*cur != 0) && (umode == KR_NS_UPDATE)) {
-       /* In KR_NS_UPDATE mode, calculate smooth over last two measurements */
-               *cur = (*cur + score) / 2;
-       } else {
-       /* First measurement or KR_NS_RESET mode, reset */
-               *cur = score;
+       /* First update is always set. */
+       if (*cur == 0) {
+               umode = KR_NS_RESET;
+       }
+       /* Update score, by default smooth over last two measurements. */
+       switch (umode) {
+       case KR_NS_UPDATE: *cur = (*cur + score) / 2; break;
+       case KR_NS_RESET:  *cur = score; break;
+       case KR_NS_ADD:    *cur = MIN(KR_NS_MAX_SCORE - 1, *cur + score); break;
+       default: break;
        }
-
        return kr_ok();
 }
 

diff --git a/lib/nsrep.h b/lib/nsrep.h
index 70a73aac313e34b26de5058c2f4065aa4fb99e37..9f2e0b784174c328ca4b24690ac3fa7b74e57ab0 100644 (file)
--- a/lib/nsrep.h
+++ b/lib/nsrep.h
@@ -35,6 +35,7 @@ enum kr_ns_score {
        KR_NS_TIMEOUT   = (95 * KR_NS_MAX_SCORE) / 100,
        KR_NS_LONG      = (3 * KR_NS_TIMEOUT) / 4,
        KR_NS_UNKNOWN   = KR_NS_TIMEOUT / 2,
+       KR_NS_PENALTY   = 100,
        KR_NS_GLUED     = 10
 };
 
@@ -51,8 +52,9 @@ enum kr_ns_rep {
  * NS RTT update modes.
  */
 enum kr_ns_update_mode {
-       KR_NS_UPDATE  = 0, /**< Update as smooth over last two measurements */
-       KR_NS_RESET        /**< Set to given value */
+       KR_NS_UPDATE = 0, /**< Update as smooth over last two measurements */
+       KR_NS_RESET,      /**< Set to given value */
+       KR_NS_ADD         /**< Increment current value */
 };
 
 /**
@@ -72,7 +74,6 @@ struct kr_nsrep
 {
        unsigned score;                  /**< NS score */
        unsigned reputation;             /**< NS reputation */
-       unsigned fails;                  /**< NS fail counter */
        const knot_dname_t *name;        /**< NS name */
        struct kr_context *ctx;          /**< Resolution context */
        union {
@@ -126,7 +127,7 @@ int kr_nsrep_elect_addr(struct kr_query *qry, struct kr_context *ctx);
  * @param  addr         chosen address (NULL for first)
  * @param  score        new score (i.e. RTT), see enum kr_ns_score
  * @param  cache        LRU cache
- * @param  umode        update mode (KR_NS_UPDATE or KR_NS_RESET)
+ * @param  umode        update mode (KR_NS_UPDATE or KR_NS_RESET or KR_NS_ADD)
  * @return              0 on success, error code on failure
  */
 KR_EXPORT

diff --git a/lib/resolve.c b/lib/resolve.c
index dd70be6b7d571a74d95f82b40e9d2da159b8fda5..3782fcd842d290c03ba863720c120434fe7be55c 100644 (file)
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -464,8 +464,12 @@ int kr_resolve_consume(struct kr_request *request, const struct sockaddr *src, k
                                        DEBUG_MSG(qry, "<= server: '%s' rtt: %ld ms\n", addr_str, time_diff(&qry->timestamp, &now));
                                }
                        }
-                       if (!(qry->flags & QUERY_SERVFAIL)) {
+                       /* Do not complete NS address resolution on soft-fail. */
+                       const int rcode = knot_wire_get_rcode(packet->wire);
+                       if (rcode != KNOT_RCODE_SERVFAIL && rcode != KNOT_RCODE_REFUSED) {
                                qry->flags &= ~(QUERY_AWAIT_IPV6|QUERY_AWAIT_IPV4);
+                       } else { /* Penalize SERVFAILs. */
+                               kr_nsrep_update_rtt(&qry->ns, src, KR_NS_PENALTY, ctx->cache_rtt, KR_NS_ADD);
                        }
                /* Do not penalize validation timeouts. */
                } else if (!(qry->flags & QUERY_DNSSEC_BOGUS)) {
@@ -708,9 +712,7 @@ ns_election:
                return KNOT_STATE_FAIL;
        }
 
-       if (qry->flags & QUERY_SERVFAIL) {
-               qry->flags &= ~QUERY_SERVFAIL;
-       } else if (qry->flags & (QUERY_AWAIT_IPV4|QUERY_AWAIT_IPV6)) {
+       if (qry->flags & (QUERY_AWAIT_IPV4|QUERY_AWAIT_IPV6)) {
                kr_nsrep_elect_addr(qry, request->ctx);
        } else if (!qry->ns.name || !(qry->flags & (QUERY_TCP|QUERY_STUB))) { /* Keep NS when requerying/stub. */
                /* Root DNSKEY must be fetched from the hints to avoid chicken and egg problem. */

diff --git a/lib/rplan.h b/lib/rplan.h
index 3b2d965b7ba8f177e32da89f51425ebe1498248c..d0e8e2f3fcb2c553c1641a44ec4ddb91e17e9603 100644 (file)
--- a/lib/rplan.h
+++ b/lib/rplan.h
@@ -47,7 +47,6 @@
        X(DNSSEC_WEXPAND,  1 << 19) /**< Query response has wildcard expansion. */ \
        X(PERMISSIVE,      1 << 20) /**< Permissive resolver mode. */ \
        X(STRICT,          1 << 21) /**< Strict resolver mode. */ \
-       X(SERVFAIL,        1 << 22) /**< Query response is SERVFAIL or REFUSED. */
 
 /** Query flags */
 enum kr_query_flag {