diff options
author | 2019-08-19 18:15:51 -0400 | |
---|---|---|
committer | 2019-08-19 18:23:35 -0400 | |
commit | e8d2dcb6a619f0884ece2284a286f21b3aa77e5a (patch) | |
tree | b0c4b3d6013331f4fa4ed9dca59c918cbf52bc70 /src/vnet/dns/resolver_process.c | |
parent | dee4c2e40333580a5a8817ed217807db48224399 (diff) |
dns: handle multiple replies for single requests
The world is a mess. A single DNS request may yield multiple, subtly
different responses; all with the same DNS protocol-level ID.
Last response wins in terms of what ends up in the cache.
First response wins in terms of the response sent to the client. Hard
to do otherwise since we have no clue that more than one answer will
be forthcoming.
Type: fix
Signed-off-by: Dave Barach <dave@barachs.net>
Change-Id: I3175a40eb1fea237048d16b852a430f5ab51eaef
Diffstat (limited to 'src/vnet/dns/resolver_process.c')
-rw-r--r-- | src/vnet/dns/resolver_process.c | 52 |
1 files changed, 44 insertions, 8 deletions
diff --git a/src/vnet/dns/resolver_process.c b/src/vnet/dns/resolver_process.c index 220a4907963..4ed7c799ce2 100644 --- a/src/vnet/dns/resolver_process.c +++ b/src/vnet/dns/resolver_process.c @@ -59,6 +59,8 @@ resolve_event (dns_main_t * dm, f64 now, u8 * reply) u16 flags; u16 rcode; int i; + int entry_was_valid; + int remove_count; int rv = 0; d = (dns_header_t *) reply; @@ -72,6 +74,8 @@ resolve_event (dns_main_t * dm, f64 now, u8 * reply) if (pool_is_free_index (dm->entries, pool_index)) { vec_free (reply); + if (0) + clib_warning ("pool index %d is free", pool_index); vlib_node_increment_counter (vm, dns46_reply_node.index, DNS46_REPLY_ERROR_NO_ELT, 1); dns_cache_unlock (dm); @@ -149,8 +153,29 @@ resolve_event (dns_main_t * dm, f64 now, u8 * reply) reply: /* Save the response */ ep->dns_response = reply; - /* Pick some sensible default. */ + + /* + * Pick a sensible default cache entry expiration time. + * We don't play the 10-second timeout game. + */ ep->expiration_time = now + 600.0; + + if (0) + clib_warning ("resolving '%s', was %s valid", + ep->name, (ep->flags & DNS_CACHE_ENTRY_FLAG_VALID) ? + "already" : "not"); + /* + * The world is a mess. A single DNS request sent to e.g. 8.8.8.8 + * may yield multiple, subtly different responses - all with the same + * DNS protocol-level ID. + * + * Last response wins in terms of what ends up in the cache. + * First response wins in terms of the response sent to the client. + */ + + /* Strong hint that we may not find a pending resolution entry */ + entry_was_valid = (ep->flags & DNS_CACHE_ENTRY_FLAG_VALID) ? 1 : 0; + if (vec_len (ep->dns_response)) ep->flags |= DNS_CACHE_ENTRY_FLAG_VALID; @@ -218,17 +243,27 @@ reply: } vec_free (ep->pending_requests); + remove_count = 0; for (i = 0; i < vec_len (dm->unresolved_entries); i++) { if (dm->unresolved_entries[i] == pool_index) { vec_delete (dm->unresolved_entries, 1, i); - goto found; + remove_count++; + i--; } } - clib_warning ("pool index %d AWOL from unresolved vector", pool_index); + /* See multiple response comment above... */ + if (remove_count == 0) + { + u32 error_code = entry_was_valid ? DNS46_REPLY_ERROR_MULTIPLE_REPLY : + DNS46_REPLY_ERROR_NO_UNRESOLVED_ENTRY; + + vlib_node_increment_counter (vm, dns46_reply_node.index, error_code, 1); + dns_cache_unlock (dm); + return; + } -found: /* Deal with bogus names, server issues, etc. */ switch (rcode) { @@ -240,13 +275,13 @@ found: case DNS_RCODE_NOT_IMPLEMENTED: case DNS_RCODE_REFUSED: if (ep->server_af == 0) - clib_warning ("name server %U backfire", + clib_warning ("name server %U can't resolve '%s'", format_ip4_address, - dm->ip4_name_servers + ep->server_rotor); + dm->ip4_name_servers + ep->server_rotor, ep->name); else - clib_warning ("name server %U backfire", + clib_warning ("name server %U can't resolve '%s'", format_ip6_address, - dm->ip6_name_servers + ep->server_rotor); + dm->ip6_name_servers + ep->server_rotor, ep->name); /* FALLTHROUGH */ case DNS_RCODE_NAME_ERROR: case DNS_RCODE_FORMAT_ERROR: @@ -255,6 +290,7 @@ found: break; } + dns_cache_unlock (dm); return; } |