From: Štěpán Balážik Date: Fri, 9 Oct 2020 15:59:35 +0000 (+0200) Subject: selection_iter: treat resolving A and AAAA records for NS names equally X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=18a804fa3d95378967656415e32b9013c8df57d7;p=thirdparty%2Fknot-resolver.git selection_iter: treat resolving A and AAAA records for NS names equally Before, there was some bias towards resolving AAAA records first and resolving A records only when IPv6 is broken or not available. --- diff --git a/lib/resolve.c b/lib/resolve.c index 5dfeb88ae..d8f2c7eb3 100644 --- a/lib/resolve.c +++ b/lib/resolve.c @@ -1156,7 +1156,7 @@ static int zone_cut_check(struct kr_request *request, struct kr_query *qry, knot } -int ns_resolve_addr(struct kr_query *qry, struct kr_request *param, struct kr_transport *transport) +int ns_resolve_addr(struct kr_query *qry, struct kr_request *param, struct kr_transport *transport, uint16_t next_type) { struct kr_rplan *rplan = ¶m->rplan; struct kr_context *ctx = param->ctx; @@ -1165,16 +1165,11 @@ int ns_resolve_addr(struct kr_query *qry, struct kr_request *param, struct kr_tr /* Start NS queries from root, to avoid certain cases * where a NS drops out of cache and the rest is unavailable, * this would lead to dependency loop in current zone cut. - * Prefer IPv6 and continue with IPv4 if not available. */ - uint16_t next_type = 0; - if (!(qry->flags.AWAIT_IPV6) && - !(ctx->options.NO_IPV6)) { - next_type = KNOT_RRTYPE_AAAA; + + if (next_type == KNOT_RRTYPE_AAAA) { qry->flags.AWAIT_IPV6 = true; - } else if (!(qry->flags.AWAIT_IPV4) && - !(ctx->options.NO_IPV4)) { - next_type = KNOT_RRTYPE_A; + } else { qry->flags.AWAIT_IPV4 = true; } /* Bail out if the query is already pending or dependency loop. */ @@ -1333,11 +1328,15 @@ int kr_resolve_produce(struct kr_request *request, struct kr_transport **transpo return KR_STATE_PRODUCE; } - if ((*transport)->protocol == KR_TRANSPORT_NOADDR) { - int ret = ns_resolve_addr(qry, qry->request, *transport); + if ((*transport)->protocol == KR_TRANSPORT_RESOLVE_A || (*transport)->protocol == KR_TRANSPORT_RESOLVE_AAAA) { + uint16_t type = (*transport)->protocol == KR_TRANSPORT_RESOLVE_A ? KNOT_RRTYPE_A : KNOT_RRTYPE_AAAA; + int ret = ns_resolve_addr(qry, qry->request, *transport, type); if (ret) { - qry->flags.AWAIT_IPV4 = false; - qry->flags.AWAIT_IPV6 = false; + if (type == KNOT_RRTYPE_A) { + qry->flags.AWAIT_IPV4 = false; + } else { + qry->flags.AWAIT_IPV6 = false; + } } ITERATE_LAYERS(request, qry, reset); return KR_STATE_PRODUCE; diff --git a/lib/selection.c b/lib/selection.c index 60696454c..865fab622 100644 --- a/lib/selection.c +++ b/lib/selection.c @@ -189,7 +189,7 @@ void shuffle_choices(struct choice choices[], int choices_len) { // Performs the actual selection (currently epsilon-greedy with epsilon = 0.05). struct kr_transport *choose_transport(struct choice choices[], int choices_len, - knot_dname_t **unresolved, + struct to_resolve *unresolved, int unresolved_len, int timeouts, struct knot_mm *mempool, @@ -200,6 +200,8 @@ struct kr_transport *choose_transport(struct choice choices[], return NULL; } + printf("choices %d, to_resolve %d\n", choices_len, unresolved_len); + struct kr_transport *transport = mm_alloc(mempool, sizeof(struct kr_transport)); memset(transport, 0, sizeof(struct kr_transport)); int choice = 0; @@ -210,8 +212,8 @@ struct kr_transport *choose_transport(struct choice choices[], if (index < unresolved_len) { // We will resolve a new NS name *transport = (struct kr_transport) { - .protocol = KR_TRANSPORT_NOADDR, - .name = unresolved[index] + .protocol = unresolved[index].type, + .name = unresolved[index].name }; return transport; } else { @@ -227,9 +229,10 @@ struct kr_transport *choose_transport(struct choice choices[], // Don't try the same server again when there are other choices to be explored if (choices[choice].address_state->error_count && unresolved_len) { + int index = kr_rand_bytes(1) % unresolved_len; *transport = (struct kr_transport) { - .protocol = KR_TRANSPORT_NOADDR, - .name = unresolved[kr_rand_bytes(1) % unresolved_len] + .name = unresolved[index].name, + .protocol = unresolved[index].type, }; return transport; } diff --git a/lib/selection.h b/lib/selection.h index a102ca085..df05eab6d 100644 --- a/lib/selection.h +++ b/lib/selection.h @@ -53,7 +53,8 @@ static const bool UNRECOVERABLE_ERRORS[] = { }; enum kr_transport_protocol { - KR_TRANSPORT_NOADDR = 0, /**< Selected name with no address, it has to be resolved first.*/ + KR_TRANSPORT_RESOLVE_A, /**< Selected name with no IPv4 address, it has to be resolved first.*/ + KR_TRANSPORT_RESOLVE_AAAA, /**< Selected name with no IPv6 address, it has to be resolved first.*/ KR_TRANSPORT_UDP, KR_TRANSPORT_TCP, KR_TRANSPORT_TLS, @@ -157,6 +158,17 @@ struct choice { uint16_t port; /**< used to overwrite the port number; if zero, `choose_transport` determines it*/ }; +/** + * Array of these is description of names to be resolved (i.e. name without some address) + * + */ +struct to_resolve +{ + knot_dname_t *name; + enum kr_transport_protocol type; +}; + + /** * @brief Based on passed choices, choose the next transport. * @@ -172,7 +184,7 @@ struct choice { * @return Chosen transport or NULL when no choice is viable */ struct kr_transport *choose_transport(struct choice choices[], int choices_len, - knot_dname_t *unresolved[], int unresolved_len, + struct to_resolve unresolved[], int unresolved_len, int timeouts, struct knot_mm *mempool, bool tcp, size_t *out_forward_index); diff --git a/lib/selection_iter.c b/lib/selection_iter.c index 09bbfc185..860f80b3e 100644 --- a/lib/selection_iter.c +++ b/lib/selection_iter.c @@ -14,7 +14,7 @@ // To be held per query and locally struct iter_local_state { - trie_t *unresolved_names; + trie_t *names; trie_t *addresses; unsigned int generation; // Used to distinguish old and valid records in tries }; @@ -22,6 +22,8 @@ struct iter_local_state { // To be held per NS name and locally struct iter_name_state { unsigned int generation; + bool a_resolved; + bool aaaa_resolved; }; void iter_local_state_alloc(struct knot_mm *mm, void **local_state) { @@ -79,10 +81,10 @@ void iter_update_state_from_rtt_cache(struct iter_local_state *local_state, stru void iter_update_state_from_zonecut(struct iter_local_state *local_state, struct kr_zonecut *zonecut, struct knot_mm *mm) { - if (local_state->unresolved_names == NULL || local_state->addresses == NULL) { + if (local_state->names == NULL || local_state->addresses == NULL) { // Local state initialization memset(local_state, 0, sizeof(struct iter_local_state)); - local_state->unresolved_names = trie_create(mm); + local_state->names = trie_create(mm); local_state->addresses = trie_create(mm); } @@ -95,29 +97,37 @@ void iter_update_state_from_zonecut(struct iter_local_state *local_state, struct knot_dname_t *dname = (knot_dname_t *)trie_it_key(it, NULL); pack_t *addresses = (pack_t *)*trie_it_val(it); + trie_val_t *val = trie_get_ins(local_state->names, (char *)dname, knot_dname_size(dname)); + if (!*val) { + // that we encountered for the first time + *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; + if (addresses->len == 0) { // Name with no address - trie_val_t *val = trie_get_ins(local_state->unresolved_names, (char *)dname, knot_dname_size(dname)); - if (!*val) { - // that we encountered for the first time - *val = mm_alloc(mm, sizeof(struct iter_name_state)); - memset(*val, 0, sizeof(struct iter_name_state)); - } - (*(struct iter_name_state **)val)->generation = current_generation; + name_state->generation = current_generation; } else { // We have some addresses to work with, let's iterate over them for(uint8_t *obj = pack_head(*addresses); obj != pack_tail(*addresses); obj = pack_obj_next(obj)) { uint8_t *address = pack_obj_val(obj); size_t address_len = pack_obj_len(obj); - trie_val_t *val = trie_get_ins(local_state->addresses, (char *)address, address_len); - if (!*val) { + trie_val_t *tval = trie_get_ins(local_state->addresses, (char *)address, address_len); + if (!*tval) { // We have have not seen this address before. - *val = mm_alloc(mm, sizeof(struct address_state)); - memset(*val, 0, sizeof(struct address_state)); + *tval = mm_alloc(mm, sizeof(struct address_state)); + memset(*tval, 0, sizeof(struct address_state)); } - struct address_state *address_state = (*(struct address_state **)val); + struct address_state *address_state = (*(struct address_state **)tval); address_state->generation = current_generation; address_state->name = dname; + + if (address_len == sizeof(struct in_addr)) { + name_state->a_resolved = true; + } else if (address_len == sizeof(struct in6_addr)) { + name_state->aaaa_resolved = true; + } } } } @@ -153,10 +163,10 @@ void iter_choose_transport(struct kr_query *qry, struct kr_transport **transport trie_it_free(it); int num_addresses = trie_weight(local_state->addresses); - int num_unresolved_names = trie_weight(local_state->unresolved_names); + int num_names = trie_weight(local_state->names); struct choice choices[num_addresses]; // Some will get unused, oh well - knot_dname_t *unresolved_names[num_unresolved_names]; + struct to_resolve unresolved_types[2*num_names]; int valid_addresses = 0; for(it = trie_it_begin(local_state->addresses); !trie_it_finished(it); trie_it_next(it)) { @@ -176,20 +186,25 @@ void iter_choose_transport(struct kr_query *qry, struct kr_transport **transport trie_it_free(it); - int to_resolve = 0; - for(it = trie_it_begin(local_state->unresolved_names); !trie_it_finished(it); trie_it_next(it)) { + int num_to_resolve = 0; + 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); - unresolved_names[to_resolve++] = name; + if (!name_state->a_resolved) { + unresolved_types[num_to_resolve++] = (struct to_resolve){name, KR_TRANSPORT_RESOLVE_A}; + } + if (!name_state->aaaa_resolved) { + unresolved_types[num_to_resolve++] = (struct to_resolve){name, KR_TRANSPORT_RESOLVE_AAAA}; + } } } trie_it_free(it); - if (valid_addresses || to_resolve) { + if (valid_addresses || num_to_resolve) { bool tcp = qry->flags.TCP | qry->server_selection.truncated; - *transport = choose_transport(choices, valid_addresses, unresolved_names, to_resolve, qry->server_selection.timeouts, mempool, tcp, NULL); + *transport = choose_transport(choices, valid_addresses, unresolved_types, num_to_resolve, qry->server_selection.timeouts, mempool, tcp, NULL); } else { *transport = NULL; }