Small things I've noticed while reading it all.
- line breaks: I believe <90 is OK, as usually the attempts to reduce
lengths impair readability
- avoid unnecessary casts; usually the type was visible
on the same line anyway
- avoid `|` on booleans
- one block gets de-indented (often badly shown in diffs)
- no need for UNRECOVERABLE_ERRORS in a header (and a weird one, too)
- recoverability from failed assertions (in case they're turned off)
break;
}
- if (query->server_selection.initialized) {
- if (selection_error != -1) {
- query->server_selection.error(query, req->upstream.transport, selection_error);
- }
+ if (query->server_selection.initialized && selection_error != -1) {
+ query->server_selection.error(query, req->upstream.transport, selection_error);
}
if (ret) {
#define EPSILON_NOMIN 1
#define EPSILON_DENOM 20
+/**
+ * If one of the errors set to true is encountered,
+ * there is no point in asking this server again.
+ */
+static const bool UNRECOVERABLE_ERRORS[] = {
+ [KR_SELECTION_QUERY_TIMEOUT] = false,
+ [KR_SELECTION_TLS_HANDSHAKE_FAILED] = false,
+ [KR_SELECTION_TCP_CONNECT_FAILED] = false,
+ [KR_SELECTION_TCP_CONNECT_TIMEOUT] = false,
+ [KR_SELECTION_REFUSED] = true,
+ [KR_SELECTION_SERVFAIL] = true,
+ [KR_SELECTION_FORMERROR] = false,
+ [KR_SELECTION_NOTIMPL] = true,
+ [KR_SELECTION_OTHER_RCODE] = true,
+ [KR_SELECTION_TRUNCATED] = false,
+ [KR_SELECTION_DNSSEC_ERROR] = true,
+ [KR_SELECTION_LAME_DELEGATION] = true,
+ [KR_SELECTION_BAD_CNAME] = true,
+};
+
+
/* Simple cache interface follows */
static knot_db_val_t cache_key(const uint8_t *ip, size_t len)
/* First value of timeout will be calculated as SRTT+4*DEFAULT_TIMEOUT
* by calc_timeout(), so it'll be equal to DEFAULT_TIMEOUT. */
static const struct rtt_state default_rtt_state = { .srtt = 0,
- .variance =
- DEFAULT_TIMEOUT / 4,
+ .variance = DEFAULT_TIMEOUT / 4,
.consecutive_timeouts = 0,
.dead_since = 0 };
* RFC6298, sec. 2. */
static unsigned calc_timeout(struct rtt_state state)
{
- int32_t timeout =
- state.srtt + MAX(4 * state.variance, MINIMAL_TIMEOUT_ADDITION);
+ int32_t timeout = state.srtt + MAX(4 * state.variance, MINIMAL_TIMEOUT_ADDITION);
return back_off_timeout(timeout, state.consecutive_timeouts);
}
static int cmp_choices(const void *a, const void *b)
{
- struct choice *a_ = (struct choice *)a;
- struct choice *b_ = (struct choice *)b;
+ const struct choice *a_ = a;
+ const struct choice *b_ = b;
int diff;
/* Address with no RTT information is better than address
return NULL;
}
- struct kr_transport *transport =
- mm_alloc(mempool, sizeof(struct kr_transport));
+ struct kr_transport *transport = mm_alloc(mempool, sizeof(struct kr_transport));
memset(transport, 0, sizeof(struct kr_transport));
int choice = 0;
.ns_name = chosen->address_state->ns_name,
.protocol = protocol,
.timeout = timeout,
- .safe_mode =
- chosen->address_state->errors[KR_SELECTION_FORMERROR],
+ .safe_mode = chosen->address_state->errors[KR_SELECTION_FORMERROR],
};
- int port;
- if (!(port = chosen->port)) {
+ int port = chosen->port;
+ if (!port) {
switch (transport->protocol) {
case KR_TRANSPORT_TLS:
port = KR_DNS_TLS_PORT;
struct kr_cache *cache = &qry->request->ctx->cache;
- uint8_t *address =
- ip_to_bytes(&transport->address, transport->address_len);
+ uint8_t *address = ip_to_bytes(&transport->address, transport->address_len);
/* This construct is a bit racy since the global state may change
* between calls to `get_rtt_state` and `put_rtt_state` but we don't
* care that much since it is rare and we only risk slightly suboptimal
VERBOSE_MSG(
qry,
- "=> id: '%05u' updating: '%s'@'%s' zone cut: '%s' with rtt %u to srtt: %d and variance: %d \n",
+ "=> id: '%05u' updating: '%s'@'%s' zone cut: '%s'"
+ " with rtt %u to srtt: %d and variance: %d \n",
qry->id, ns_name, ns_str ? ns_str : "", zonecut_str,
rtt, new_rtt_state.srtt, new_rtt_state.variance);
}
return;
}
- uint8_t *address =
- ip_to_bytes(&transport->address, transport->address_len);
+ uint8_t *address = ip_to_bytes(&transport->address, transport->address_len);
struct rtt_state old_state = addr_state->rtt_state;
struct rtt_state cur_state =
get_rtt_state(address, transport->address_len, cache);
KR_NS_TIMEOUT_ROW_DEAD) {
cur_state.dead_since = kr_now();
}
- put_rtt_state(address, transport->address_len, cur_state,
- cache);
+ put_rtt_state(address, transport->address_len, cur_state, cache);
} else {
/* `get_rtt_state` opens a cache transaction, we have to end it. */
kr_cache_commit(cache);
const struct kr_transport *transport,
enum kr_selection_error sel_error)
{
+ assert(sel_error >= KR_SELECTION_OK && sel_error < KR_SELECTION_NUMBER_OF_ERRORS);
if (!transport || !addr_state) {
/* Answers from cache have NULL transport, ignore them. */
return;
}
- if (sel_error >= KR_SELECTION_NUMBER_OF_ERRORS) {
- assert(0);
- }
-
if (sel_error == KR_SELECTION_QUERY_TIMEOUT) {
qry->server_selection.local_state->timeouts++;
// Make sure the query was chosen by this query
void kr_server_selection_init(struct kr_query *qry)
{
struct knot_mm *mempool = &qry->request->pool;
+ struct local_state *local_state = mm_alloc(mempool, sizeof(struct local_state));
+ memset(local_state, 0, sizeof(struct local_state));
+
if (qry->flags.FORWARD || qry->flags.STUB) {
qry->server_selection = (struct kr_server_selection){
.initialized = true,
.choose_transport = forward_choose_transport,
.update_rtt = forward_update_rtt,
.error = forward_error,
- .local_state =
- mm_alloc(mempool, sizeof(struct local_state)),
+ .local_state = local_state,
};
- memset(qry->server_selection.local_state, 0,
- sizeof(struct local_state));
forward_local_state_alloc(
mempool, &qry->server_selection.local_state->private,
qry->request);
.choose_transport = iter_choose_transport,
.update_rtt = iter_update_rtt,
.error = iter_error,
- .local_state =
- mm_alloc(mempool, sizeof(struct local_state)),
+ .local_state = local_state,
};
- memset(qry->server_selection.local_state, 0,
- sizeof(struct local_state));
iter_local_state_alloc(
mempool, &qry->server_selection.local_state->private);
}
/**
* @file selection.h
- * Provides server selection API (see `kr_server_selection`) and functions common to both implementations.
+ * Provides server selection API (see `kr_server_selection`)
+ * and functions common to both implementations.
*/
#include "lib/cache/api.h"
KR_SELECTION_BAD_CNAME,
/** Leave this last, as it is used as array size. */
- KR_SELECTION_NUMBER_OF_ERRORS
+ KR_SELECTION_NUMBER_OF_ERRORS
};
enum kr_transport_protocol {
KR_EXPORT
int kr_forward_add_target(struct kr_request *req, const struct sockaddr *sock);
+
+
+
+
+/* Below are internal parts shared by ./selection_{forward,iter}.c */
+
/**
* To be held per IP address in the global LMDB cache
*/
struct rtt_state {
- int32_t srtt;
- int32_t variance;
+ int32_t srtt; /**< Smoothed RTT, i.e. an estimate of round-trip time. */
+ int32_t variance; /**< An estimate of RTT's standard derivation (not variance). */
int32_t consecutive_timeouts;
/** Timestamp of pronouncing this IP bad based on KR_NS_TIMEOUT_ROW_DEAD */
uint64_t dead_since;
* @brief To be held per IP address and locally "inside" query.
*/
struct address_state {
- /** Used to distinguish old and valid records in local_state. */
+ /** Used to distinguish old and valid records in local_state; -1 means unusable IP. */
unsigned int generation;
struct rtt_state rtt_state;
knot_dname_t *ns_name;
* @param timeouts Number of timeouts that occured in this query (used for exponential backoff)
* @param mempool Memory context of current request
* @param tcp Force TCP as transport protocol
- * @param[out] choice_index Optinally index of the chosen transport in the @p choices array is stored here.
- * @return Chosen transport or NULL when no choice is viable
+ * @param[out] choice_index Optionally index of the chosen transport in the @p choices array.
+ * @return Chosen transport (on mempool) or NULL when no choice is viable
*/
struct kr_transport *select_transport(struct choice choices[], int choices_len,
struct to_resolve unresolved[],
*local_state = mm_alloc(mm, sizeof(struct forward_local_state));
memset(*local_state, 0, sizeof(struct forward_local_state));
- struct forward_local_state *forward_state =
- (struct forward_local_state *)*local_state;
+ struct forward_local_state *forward_state = *local_state;
forward_state->targets = &req->selection_context.forwarding_targets;
- forward_state->addr_states = mm_alloc(
- mm, sizeof(struct address_state) * forward_state->targets->len);
- memset(forward_state->addr_states, 0,
- sizeof(struct address_state) * forward_state->targets->len);
+ size_t as_bytes = sizeof(struct address_state) * forward_state->targets->len;
+ forward_state->addr_states = mm_alloc(mm, as_bytes);
+ memset(forward_state->addr_states, 0, as_bytes);
}
void forward_choose_transport(struct kr_query *qry,
};
}
- bool tcp =
- qry->flags.TCP | qry->server_selection.local_state->truncated;
+ bool tcp = qry->flags.TCP || qry->server_selection.local_state->truncated;
*transport =
select_transport(choices, valid, NULL, 0,
qry->server_selection.local_state->timeouts,
&local_state->addr_states[local_state->last_choice_index];
update_rtt(qry, addr_state, transport, rtt);
-}
\ No newline at end of file
+}
#define VERBOSE_MSG(qry, ...) QRVERBOSE((qry), "slct", __VA_ARGS__)
-// To be held per query and locally
+/// To be held per query and locally. Allocations are in the kr_request's mempool.
struct iter_local_state {
- trie_t *names;
- trie_t *addresses;
+ trie_t *names; /// knot_dname_t -> struct iter_name_state *
+ trie_t *addresses; /// IP address -> struct address_state *
knot_dname_t *zonecut;
/** Used to distinguish old and valid records in tries. */
unsigned int generation;
return NULL;
}
- trie_t *addresses = local_state->addresses;
- uint8_t *address =
- ip_to_bytes(&transport->address, transport->address_len);
-
- trie_val_t *address_state = trie_get_try(addresses, (char *)address,
+ uint8_t *address = ip_to_bytes(&transport->address, transport->address_len);
+ trie_val_t *address_state = trie_get_try(local_state->addresses, (char *)address,
transport->address_len);
-
if (!address_state) {
- if (transport->deduplicated) {
- /* Transport was chosen by a different query. */
- return NULL;
- }
-
- assert(0);
+ assert(transport->deduplicated);
+ /* Transport was chosen by a different query. */
+ return NULL;
}
- return (struct address_state *)*address_state;
-}
-
-static bool zonecut_changed(knot_dname_t *new, knot_dname_t *old)
-{
- return knot_dname_cmp(old, new);
+ return *address_state;
}
static void unpack_state_from_zonecut(struct iter_local_state *local_state,
local_state->names = trie_create(mm);
local_state->addresses = trie_create(mm);
} else {
- zcut_changed = zonecut_changed(zonecut->name, local_state->zonecut);
+ zcut_changed = !knot_dname_is_equal(zonecut->name, local_state->zonecut);
}
local_state->zonecut = zonecut->name;
local_state->generation++;
}
trie_it_t *it;
- unsigned int current_generation = local_state->generation;
+ const unsigned int current_generation = local_state->generation;
for (it = trie_it_begin(zonecut->nsset); !trie_it_finished(it); trie_it_next(it)) {
knot_dname_t *dname = (knot_dname_t *)trie_it_key(it, NULL);
- pack_t *addresses = (pack_t *)*trie_it_val(it);
+ pack_t *addresses = *trie_it_val(it);
trie_val_t *val = trie_get_ins(local_state->names, (char *)dname,
knot_dname_size(dname));
*val = mm_alloc(mm, sizeof(struct iter_name_state));
memset(*val, 0, sizeof(struct iter_name_state));
}
- struct iter_name_state *name_state = *(struct iter_name_state **)val;
+ struct iter_name_state *name_state = *val;
name_state->generation = current_generation;
if (zcut_changed) {
name_state->aaaa_state = RECORD_UNKNOWN;
}
- if (addresses->len == 0) {
- continue;
- }
-
- /* We have some addresses to work with, let's iterate over them. */
+ /* Iterate over all addresses of this NS (if any). */
for (uint8_t *obj = pack_head(*addresses); obj != pack_tail(*addresses);
obj = pack_obj_next(obj)) {
uint8_t *address = pack_obj_val(obj);
*tval = mm_alloc(mm, sizeof(struct address_state));
memset(*tval, 0, sizeof(struct address_state));
}
- struct address_state *address_state = (*(struct address_state **)tval);
+ struct address_state *address_state = *tval;
address_state->generation = current_generation;
address_state->ns_name = dname;
trie_it_next(it)) {
size_t address_len;
uint8_t *address = (uint8_t *)trie_it_key(it, &address_len);
- struct address_state *address_state =
- (struct address_state *)*trie_it_val(it);
+ struct address_state *address_state = *trie_it_val(it);
if (address_state->generation == local_state->generation &&
!address_state->unrecoverable_errors) {
choices[count] = (struct choice){
trie_it_t *it;
for (it = trie_it_begin(local_state->names); !trie_it_finished(it);
trie_it_next(it)) {
- struct iter_name_state *name_state =
- *(struct iter_name_state **)trie_it_val(it);
- if (name_state->generation == local_state->generation) {
- knot_dname_t *name = (knot_dname_t *)trie_it_key(it, NULL);
- if (qry->stype == KNOT_RRTYPE_DNSKEY &&
- knot_dname_in_bailiwick(name, qry->sname) > 0) {
- /* Resolving `domain. DNSKEY` can't trigger the
- * resolution of `sub.domain. A/AAAA` since it
- * will cause a cycle. */
- continue;
- }
+ struct iter_name_state *name_state = *trie_it_val(it);
+ if (name_state->generation != local_state->generation)
+ continue;
- /* FIXME: kr_rplan_satisfies(qry,…) should have been here, but this leads to failures on
- * iter_ns_badip.rpl, this is because the test requires the resolver to switch to parent
- * side after a record in cache expires. Only way to do this in the current zonecut setup is
- * to requery the same query twice in the row. So we have to allow that and only check the
- * rplan from parent upwards.
- */
- bool a_in_rplan = kr_rplan_satisfies(qry->parent, name,
- KNOT_CLASS_IN,
- KNOT_RRTYPE_A);
- bool aaaa_in_rplan =
- kr_rplan_satisfies(qry->parent, name,
- KNOT_CLASS_IN,
- KNOT_RRTYPE_AAAA);
-
- if (name_state->a_state == RECORD_UNKNOWN &&
- !qry->flags.NO_IPV4 && !a_in_rplan) {
- resolvable[count++] = (struct to_resolve){
- name, KR_TRANSPORT_RESOLVE_A
- };
- }
+ knot_dname_t *name = (knot_dname_t *)trie_it_key(it, NULL);
+ if (qry->stype == KNOT_RRTYPE_DNSKEY &&
+ knot_dname_in_bailiwick(name, qry->sname) > 0) {
+ /* Resolving `domain. DNSKEY` can't trigger the
+ * resolution of `sub.domain. A/AAAA` since it
+ * will cause a cycle. */
+ continue;
+ }
- if (name_state->aaaa_state == RECORD_UNKNOWN &&
- !qry->flags.NO_IPV6 && !aaaa_in_rplan) {
- resolvable[count++] = (struct to_resolve){
- name, KR_TRANSPORT_RESOLVE_AAAA
- };
- }
+ /* FIXME: kr_rplan_satisfies(qry,…) should have been here, but this leads to failures on
+ * iter_ns_badip.rpl, this is because the test requires the resolver to switch to parent
+ * side after a record in cache expires. Only way to do this in the current zonecut setup is
+ * to requery the same query twice in the row. So we have to allow that and only check the
+ * rplan from parent upwards.
+ */
+ bool a_in_rplan = kr_rplan_satisfies(qry->parent, name,
+ KNOT_CLASS_IN, KNOT_RRTYPE_A);
+ bool aaaa_in_rplan = kr_rplan_satisfies(qry->parent, name,
+ KNOT_CLASS_IN, KNOT_RRTYPE_AAAA);
+
+ if (name_state->a_state == RECORD_UNKNOWN &&
+ !qry->flags.NO_IPV4 && !a_in_rplan) {
+ resolvable[count++] = (struct to_resolve){
+ name, KR_TRANSPORT_RESOLVE_A
+ };
+ }
+
+ if (name_state->aaaa_state == RECORD_UNKNOWN &&
+ !qry->flags.NO_IPV6 && !aaaa_in_rplan) {
+ resolvable[count++] = (struct to_resolve){
+ name, KR_TRANSPORT_RESOLVE_AAAA
+ };
}
}
trie_it_free(it);
}
}
-void iter_choose_transport(struct kr_query *qry,
- struct kr_transport **transport)
+void iter_choose_transport(struct kr_query *qry, struct kr_transport **transport)
{
struct knot_mm *mempool = &qry->request->pool;
struct iter_local_state *local_state =
int resolvable_len = get_resolvable_names(local_state, resolvable, qry);
if (choices_len || resolvable_len) {
- bool tcp = qry->flags.TCP |
- qry->server_selection.local_state->truncated;
+ bool tcp = qry->flags.TCP || qry->server_selection.local_state->truncated;
*transport = select_transport(
choices, choices_len, resolvable, resolvable_len,
qry->server_selection.local_state->timeouts, mempool,
}
bool nxnsattack_mitigation = false;
- enum kr_transport_protocol proto =
+ const enum kr_transport_protocol proto =
*transport ? (*transport)->protocol : -1;
if (proto == KR_TRANSPORT_RESOLVE_A || proto == KR_TRANSPORT_RESOLVE_AAAA) {
if (++local_state->no_ns_addr_count > KR_COUNT_NO_NSADDR_LIMIT) {
qry->id, ip_version, ns_name, zonecut_str);
break;
default:
- VERBOSE_MSG(qry, "=> id: '%05u' choosing: '%s'@'%s' with timeout %u ms zone cut: '%s'%s\n",
- qry->id, ns_name, ns_str ? ns_str : "", (*transport)->timeout, zonecut_str,
+ VERBOSE_MSG(qry, "=> id: '%05u' choosing: '%s'@'%s'"
+ " with timeout %u ms zone cut: '%s'%s\n",
+ qry->id, ns_name, ns_str ? ns_str : "",
+ (*transport)->timeout, zonecut_str,
(*transport)->safe_mode ? " SAFEMODE" : "");
break;
}
} else {
+ const char *nxns_msg = nxnsattack_mitigation
+ ? " (stopped due to mitigation for NXNSAttack CVE-2020-12667)" : "";
VERBOSE_MSG(qry, "=> id: '%05u' no suitable transport, zone cut: '%s'%s\n",
- qry->id, zonecut_str, nxnsattack_mitigation ? " (stopped due to mitigation for NXNSAttack CVE-2020-12667)" : "");
+ qry->id, zonecut_str, nxns_msg );
}
}
}
#include "lib/selection.h"
-/**
- * If one of the errors set to true is encountered, there is no point in asking this server again.
- */
-static const bool UNRECOVERABLE_ERRORS[] = {
- [KR_SELECTION_QUERY_TIMEOUT] = false,
- [KR_SELECTION_TLS_HANDSHAKE_FAILED] = false,
- [KR_SELECTION_TCP_CONNECT_FAILED] = false,
- [KR_SELECTION_TCP_CONNECT_TIMEOUT] = false,
- [KR_SELECTION_REFUSED] = true,
- [KR_SELECTION_SERVFAIL] = true,
- [KR_SELECTION_FORMERROR] = false,
- [KR_SELECTION_NOTIMPL] = true,
- [KR_SELECTION_OTHER_RCODE] = true,
- [KR_SELECTION_TRUNCATED] = false,
- [KR_SELECTION_DNSSEC_ERROR] = true,
- [KR_SELECTION_LAME_DELEGATION] = true,
- [KR_SELECTION_BAD_CNAME] = true,
-};
-
void iter_local_state_alloc(struct knot_mm *mm, void **local_state);
-void iter_choose_transport(struct kr_query *qry,
- struct kr_transport **transport);
+void iter_choose_transport(struct kr_query *qry, struct kr_transport **transport);
void iter_error(struct kr_query *qry, const struct kr_transport *transport,
enum kr_selection_error sel_error);
void iter_update_rtt(struct kr_query *qry, const struct kr_transport *transport,
- unsigned rtt);
\ No newline at end of file
+ unsigned rtt);
projects / thirdparty / knot-resolver.git / commitdiff
