From 84a563ae4050cc0389dcd438fbe9ea882f2b8404 Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Thu, 22 Aug 2019 19:32:49 -0400 Subject: dns: fix trivial multi-thread deadlock Add a simple lock trace mechanism Type: fix Ticket: VPP-1752 Signed-off-by: Dave Barach Change-Id: Idc82b1ad59adb0f7c185d27ced57e9a4c25ce62f --- MAINTAINERS | 10 +++++----- src/plugins/dns/dns.c | 17 ++++++++--------- src/plugins/dns/dns.h | 8 +++++++- src/plugins/dns/resolver_process.c | 4 ++-- 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 M: Neale Ranns F: src/vnet/dhcp/ -VNET DNS -I: dns -M: Dave Barach -F: src/vnet/dns/ - VNET TLS and TLS engine plugins I: tls M: Florin Coras @@ -268,6 +263,11 @@ I: abf M: Neale Ranns F: src/plugins/abf/ +Plugin - Simple DNS name resolver +I: dns +M: Dave Barach +F: src/plugins/dns/ + Plugin - Group Based Policy (GBP) I: gbp M: Neale Ranns 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); -- cgit 1.2.3-korg