Commit 7b679ecf authored by Marek Vavrusa's avatar Marek Vavrusa

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)
parent bc9ef681
......@@ -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.
......
......@@ -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;
}
}
......
......@@ -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();
}
......
......@@ -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
......
......@@ -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. */
......
......@@ -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 {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment