dns: fix trivial multi-thread deadlock

Add a simple lock trace mechanism

Type: fix
Ticket: VPP-1752

Signed-off-by: Dave Barach <dave@barachs.net>
Change-Id: Idc82b1ad59adb0f7c185d27ced57e9a4c25ce62f

diff --git a/MAINTAINERS b/MAINTAINERS
index 7e4a9d0..f0c2038 100644 (file)
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -245,11 +245,6 @@ M: Dave Barach <dave@barachs.net>
 M:     Neale Ranns <nranns@cisco.com>
 F:     src/vnet/dhcp/
 
-VNET DNS
-I:     dns
-M:     Dave Barach <dave@barachs.net>
-F:     src/vnet/dns/
-
 VNET TLS and TLS engine plugins
 I:     tls
 M:     Florin Coras <fcoras@cisco.com>
@@ -268,6 +263,11 @@ I: abf
 M:     Neale Ranns <nranns@cisco.com>
 F:     src/plugins/abf/
 
+Plugin - Simple DNS name resolver
+I:     dns
+M:     Dave Barach <dave@barachs.net>
+F:     src/plugins/dns/
+
 Plugin - Group Based Policy (GBP)
 I:     gbp
 M:     Neale Ranns <nranns@cisco.com>

diff --git a/src/plugins/dns/dns.c b/src/plugins/dns/dns.c
index 6ae0ff1..5c51a44 100644 (file)
--- a/src/plugins/dns/dns.c
+++ b/src/plugins/dns/dns.c
@@ -68,7 +68,7 @@ dns_cache_clear (dns_main_t * dm)
   if (dm->is_enabled == 0)
     return VNET_API_ERROR_NAME_RESOLUTION_NOT_ENABLED;
 
-  dns_cache_lock (dm);
+  dns_cache_lock (dm, 1);
 
   /* *INDENT-OFF* */
   pool_foreach (ep, dm->entries,
@@ -721,7 +721,7 @@ dns_delete_by_name (dns_main_t * dm, u8 * name)
   if (dm->is_enabled == 0)
     return VNET_API_ERROR_NAME_RESOLUTION_NOT_ENABLED;
 
-  dns_cache_lock (dm);
+  dns_cache_lock (dm, 2);
   p = hash_get_mem (dm->cache_entry_by_name, name);
   if (!p)
     {
@@ -755,7 +755,7 @@ delete_random_entry (dns_main_t * dm)
     return VNET_API_ERROR_UNSPECIFIED;
 #endif
 
-  dns_cache_lock (dm);
+  dns_cache_lock (dm, 3);
   limit = pool_elts (dm->entries);
   start_index = random_u32 (&dm->random_seed) % limit;
 
@@ -792,7 +792,7 @@ dns_add_static_entry (dns_main_t * dm, u8 * name, u8 * dns_reply_data)
   if (dm->is_enabled == 0)
     return VNET_API_ERROR_NAME_RESOLUTION_NOT_ENABLED;
 
-  dns_cache_lock (dm);
+  dns_cache_lock (dm, 4);
   p = hash_get_mem (dm->cache_entry_by_name, name);
   if (p)
     {
@@ -844,7 +844,7 @@ vnet_dns_resolve_name (dns_main_t * dm, u8 * name, dns_pending_request_t * t,
   if (name[0] == 0)
     return VNET_API_ERROR_INVALID_VALUE;
 
-  dns_cache_lock (dm);
+  dns_cache_lock (dm, 5);
 search_again:
   p = hash_get_mem (dm->cache_entry_by_name, name);
   if (p)
@@ -903,9 +903,8 @@ search_again:
              name = ep->cname;
              goto search_again;
            }
-
-         /* Note: caller must drop the lock! */
          *retp = ep;
+         dns_cache_unlock (dm);
          return (0);
        }
       else
@@ -2133,7 +2132,7 @@ format_dns_cache (u8 * s, va_list * args)
       return s;
     }
 
-  dns_cache_lock (dm);
+  dns_cache_lock (dm, 6);
 
   if (name)
     {
@@ -2736,7 +2735,7 @@ test_dns_expire_command_fn (vlib_main_t * vm,
   else
     return clib_error_return (0, "no name provided");
 
-  dns_cache_lock (dm);
+  dns_cache_lock (dm, 7);
 
   p = hash_get_mem (dm->cache_entry_by_name, name);
   if (!p)

diff --git a/src/plugins/dns/dns.h b/src/plugins/dns/dns.h
index 0a3f7a9..499c715 100644 (file)
--- a/src/plugins/dns/dns.h
+++ b/src/plugins/dns/dns.h
@@ -99,6 +99,7 @@ typedef struct
   /** Find cached record by name */
   uword *cache_entry_by_name;
   clib_spinlock_t cache_lock;
+  int cache_lock_tag;
 
   /** enable / disable flag */
   int is_enabled;
@@ -198,11 +199,14 @@ void vnet_dns_create_resolver_process (dns_main_t * dm);
 format_function_t format_dns_reply;
 
 static inline void
-dns_cache_lock (dns_main_t * dm)
+dns_cache_lock (dns_main_t * dm, int tag)
 {
   if (dm->cache_lock)
     {
+      ASSERT (tag);
+      ASSERT (dm->cache_lock_tag == 0);
       clib_spinlock_lock (&dm->cache_lock);
+      dm->cache_lock_tag = tag;
     }
 }
 
@@ -211,6 +215,8 @@ dns_cache_unlock (dns_main_t * dm)
 {
   if (dm->cache_lock)
     {
+      ASSERT (dm->cache_lock_tag);
+      dm->cache_lock_tag = 0;
       clib_spinlock_unlock (&dm->cache_lock);
     }
 }

diff --git a/src/plugins/dns/resolver_process.c b/src/plugins/dns/resolver_process.c
index 802da53..51fe9d7 100644 (file)
--- a/src/plugins/dns/resolver_process.c
+++ b/src/plugins/dns/resolver_process.c
@@ -70,7 +70,7 @@ resolve_event (dns_main_t * dm, f64 now, u8 * reply)
 
   /* $$$ u16 limits cache to 65K entries, fix later multiple dst ports */
   pool_index = clib_net_to_host_u16 (d->id);
-  dns_cache_lock (dm);
+  dns_cache_lock (dm, 10);
 
   if (pool_is_free_index (dm->entries, pool_index))
     {
@@ -306,7 +306,7 @@ retry_scan (dns_main_t * dm, f64 now)
 
   for (i = 0; i < vec_len (dm->unresolved_entries); i++)
     {
-      dns_cache_lock (dm);
+      dns_cache_lock (dm, 11);
       ep = pool_elt_at_index (dm->entries, dm->unresolved_entries[i]);
 
       ASSERT ((ep->flags & DNS_CACHE_ENTRY_FLAG_VALID) == 0);