3 files changed, 121 insertions, 59 deletions

diff --git a/src/vnet/dns/dns.c b/src/vnet/dns/dns.c
index 3285bedc903..eac95450485 100644
--- a/src/vnet/dns/dns.c
+++ b/src/vnet/dns/dns.c
@@ -248,7 +248,8 @@ vnet_dns_send_dns4_request (dns_main_t * dm,
   fib_index = fib_table_find (prefix.fp_proto, 0 /* default VRF for now */ );
   if (fib_index == (u32) ~ 0)
     {
-      clib_warning ("no fib table");
+      if (0)
+	clib_warning ("no fib table");
       return;
     }
 
@@ -257,7 +258,8 @@ vnet_dns_send_dns4_request (dns_main_t * dm,
   /* Couldn't find route to destination. Bail out. */
   if (fei == FIB_NODE_INDEX_INVALID)
     {
-      clib_warning ("no route to DNS server");
+      if (0)
+	clib_warning ("no route to DNS server");
       return;
     }
 
@@ -265,9 +267,10 @@ vnet_dns_send_dns4_request (dns_main_t * dm,
 
   if (sw_if_index == ~0)
     {
-      clib_warning
-	("route to %U exists, fei %d, get_resolving_interface returned"
-	 " ~0", format_ip4_address, &prefix.fp_addr, fei);
+      if (0)
+	clib_warning
+	  ("route to %U exists, fei %d, get_resolving_interface returned"
+	   " ~0", format_ip4_address, &prefix.fp_addr, fei);
       return;
     }
 
@@ -365,7 +368,8 @@ vnet_dns_send_dns6_request (dns_main_t * dm,
   fib_index = fib_table_find (prefix.fp_proto, 0 /* default VRF for now */ );
   if (fib_index == (u32) ~ 0)
     {
-      clib_warning ("no fib table");
+      if (0)
+	clib_warning ("no fib table");
       return;
     }
 
@@ -1061,6 +1065,7 @@ vnet_dns_cname_indirection_nolock (dns_main_t * dm, u32 ep_index, u8 * reply)
 	goto found_last_request;
       }
   clib_warning ("pool elt %d supposedly pending, but not found...", ep_index);
+  return -1;
 
 found_last_request:
 
@@ -2206,46 +2211,51 @@ format_dns_cache (u8 * s, va_list * args)
       return s;
     }
 
-  /* *INDENT-OFF* */
-  pool_foreach (ep, dm->entries,
-  ({
-    if (ep->flags & DNS_CACHE_ENTRY_FLAG_VALID)
-      {
-        ASSERT (ep->dns_response);
-        if (ep->flags & DNS_CACHE_ENTRY_FLAG_STATIC)
-          ss = "[S] ";
-        else
-          ss = "    ";
+  s = format (s, "DNS cache contains %d entries\n", pool_elts (dm->entries));
 
-        if (verbose < 2 && ep->flags & DNS_CACHE_ENTRY_FLAG_CNAME)
-          s = format (s, "%s%s -> %s", ss, ep->name, ep->cname);
-        else
-          s = format (s, "%s%s -> %U", ss, ep->name,
-                      format_dns_reply,
-                      ep->dns_response,
-                      verbose);
-        if (!(ep->flags & DNS_CACHE_ENTRY_FLAG_STATIC))
+  if (verbose > 0)
+    {
+      /* *INDENT-OFF* */
+      pool_foreach (ep, dm->entries,
+      ({
+        if (ep->flags & DNS_CACHE_ENTRY_FLAG_VALID)
           {
-            f64 time_left = ep->expiration_time - now;
-            if (time_left > 0.0)
-              s = format (s, "  TTL left %.1f", time_left);
+            ASSERT (ep->dns_response);
+            if (ep->flags & DNS_CACHE_ENTRY_FLAG_STATIC)
+              ss = "[S] ";
             else
-              s = format (s, "  EXPIRED");
+              ss = "    ";
 
-            if (verbose > 2)
-              s = format (s, "    %d client notifications pending\n",
-                          vec_len(ep->pending_requests));
+            if (verbose < 2 && ep->flags & DNS_CACHE_ENTRY_FLAG_CNAME)
+              s = format (s, "%s%s -> %s", ss, ep->name, ep->cname);
+            else
+              s = format (s, "%s%s -> %U", ss, ep->name,
+                          format_dns_reply,
+                          ep->dns_response,
+                          verbose);
+            if (!(ep->flags & DNS_CACHE_ENTRY_FLAG_STATIC))
+              {
+                f64 time_left = ep->expiration_time - now;
+                if (time_left > 0.0)
+                  s = format (s, "  TTL left %.1f", time_left);
+                else
+                  s = format (s, "  EXPIRED");
+
+                if (verbose > 2)
+                  s = format (s, "    %d client notifications pending\n",
+                              vec_len(ep->pending_requests));
+              }
           }
-      }
-    else
-      {
-        ASSERT (ep->dns_request);
-        s = format (s, "[P] %U", format_dns_reply, ep->dns_request,
-                    verbose);
-      }
-    vec_add1 (s, '\n');
-  }));
-  /* *INDENT-ON* */
+        else
+          {
+            ASSERT (ep->dns_request);
+            s = format (s, "[P] %U", format_dns_reply, ep->dns_request,
+                        verbose);
+          }
+        vec_add1 (s, '\n');
+      }));
+      /* *INDENT-ON* */
+    }
 
   dns_cache_unlock (dm);
 
@@ -2764,6 +2774,7 @@ vnet_send_dns4_reply (dns_main_t * dm, dns_pending_request_t * pr,
   dns_rr_t *rr;
   u8 *rrptr;
   int is_fail = 0;
+  int is_recycle = (b0 != 0);
 
   ASSERT (ep && ep->dns_response);
 
@@ -2804,6 +2815,16 @@ vnet_send_dns4_reply (dns_main_t * dm, dns_pending_request_t * pr,
 	return;
       b0 = vlib_get_buffer (vm, bi);
     }
+  else
+    {
+      /* Use the buffer we were handed. Reinitialize it... */
+      vlib_buffer_t bt = { };
+      /* push/pop the reference count */
+      u8 save_ref_count = b0->ref_count;
+      vlib_buffer_copy_template (b0, &bt);
+      b0->ref_count = save_ref_count;
+      bi = vlib_get_buffer_index (vm, b0);
+    }
 
   if (b0->flags & VLIB_BUFFER_NEXT_PRESENT)
     vlib_buffer_free_one (vm, b0->next_buffer);
@@ -2817,9 +2838,6 @@ vnet_send_dns4_reply (dns_main_t * dm, dns_pending_request_t * pr,
    */
   b0->flags |= (VNET_BUFFER_F_LOCALLY_ORIGINATED
 		| VLIB_BUFFER_TOTAL_LENGTH_VALID);
-  b0->current_data = 0;
-  b0->current_length = 0;
-  b0->total_length_not_including_first_buffer = 0;
   vnet_buffer (b0)->sw_if_index[VLIB_RX] = 0;	/* "local0" */
   vnet_buffer (b0)->sw_if_index[VLIB_TX] = 0;	/* default VRF for now */
 
@@ -2971,12 +2989,18 @@ found_src_address:
   udp->checksum = 0;
   vec_free (reply);
 
-  /* Ship it to ip4_lookup */
-  f = vlib_get_frame_to_node (vm, ip4_lookup_node.index);
-  to_next = vlib_frame_vector_args (f);
-  to_next[0] = bi;
-  f->n_vectors = 1;
-  vlib_put_frame_to_node (vm, ip4_lookup_node.index, f);
+  /*
+   * Ship pkts made out of whole cloth to ip4_lookup
+   * Caller will ship recycled dns reply packets to ip4_lookup
+   */
+  if (is_recycle == 0)
+    {
+      f = vlib_get_frame_to_node (vm, ip4_lookup_node.index);
+      to_next = vlib_frame_vector_args (f);
+      to_next[0] = bi;
+      f->n_vectors = 1;
+      vlib_put_frame_to_node (vm, ip4_lookup_node.index, f);
+    }
 }
 
 /*
diff --git a/src/vnet/dns/dns.h b/src/vnet/dns/dns.h
index e6944de0ca9..6d0901020f9 100644
--- a/src/vnet/dns/dns.h
+++ b/src/vnet/dns/dns.h
@@ -150,7 +150,9 @@ _(DISABLED, "DNS pkts punted (feature disabled)")       \
 _(PROCESSED, "DNS reply pkts processed")                \
 _(NO_ELT, "No DNS pool element")                        \
 _(FORMAT_ERROR, "DNS format errors")                    \
-_(TEST_DROP, "DNS reply pkt dropped for test purposes")
+_(TEST_DROP, "DNS reply pkt dropped for test purposes") \
+_(MULTIPLE_REPLY, "DNS multiple reply packets")	        \
+_(NO_UNRESOLVED_ENTRY, "No unresolved entry for pkt")
 
 typedef enum
 {
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;
 }