diff options
author | Dave Barach <dave@barachs.net> | 2019-08-22 19:32:49 -0400 |
---|---|---|
committer | Dave Barach <dave@barachs.net> | 2019-08-22 19:33:34 -0400 |
commit | 84a563ae4050cc0389dcd438fbe9ea882f2b8404 (patch) | |
tree | 18b9db5ecbd3cd9480f6f94e237720321c2b877f | |
parent | a43c93f8554ad7418e31be3791b3fb71232f60ac (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-- | MAINTAINERS | 10 | ||||
-rw-r--r-- | src/plugins/dns/dns.c | 17 | ||||
-rw-r--r-- | src/plugins/dns/dns.h | 8 | ||||
-rw-r--r-- | 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 <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); |