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 5dfeb88ae773fa5c806955af7c997fbf6cfcbc97..d8f2c7eb33ed334c8b21a8e9d80001598e61c5e4 100644 (file)
--- 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 = &param->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 60696454c46e97c4cdf0b8520072f723b4114ab4..865fab622097ca86efe458fd01140c0d7394e4cf 100644 (file)
--- 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 a102ca085bc7098bd77faa680b698ebd637de0c6..df05eab6d694cbe87083493c4b6a83399aa074c5 100644 (file)
--- 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 09bbfc185a2456a1945453fd5e7b8e8afcdf8879..860f80b3ed1574caadfd288c8e19956fabdc0c32 100644 (file)
--- 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;
        }