summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDave Barach <dave@barachs.net>2019-08-22 19:32:49 -0400
committerDave Barach <dave@barachs.net>2019-08-22 19:33:34 -0400
commit84a563ae4050cc0389dcd438fbe9ea882f2b8404 (patch)
tree18b9db5ecbd3cd9480f6f94e237720321c2b877f
parenta43c93f8554ad7418e31be3791b3fb71232f60ac (diff)
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
-rw-r--r--MAINTAINERS10
-rw-r--r--src/plugins/dns/dns.c17
-rw-r--r--src/plugins/dns/dns.h8
-rw-r--r--src/plugins/dns/resolver_process.c4
4 files changed, 22 insertions, 17 deletions
diff --git a/MAINTAINERS b/MAINTAINERS
index 7e4a9d01fea..f0c20381006 100644
--- 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 6ae0ff18a68..5c51a446d98 100644
--- 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 0a3f7a935cb..499c71567d6 100644
--- 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 802da53cec5..51fe9d76a08 100644
--- 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);