ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dns: fix trivial multi-thread deadlock

Browse Source 
Add a simple lock trace mechanism

Type: fix
Ticket: VPP-1752

Signed-off-by: Dave Barach <dave@barachs.net>
Change-Id: Idc82b1ad59adb0f7c185d27ced57e9a4c25ce62f
This commit is contained in:
Dave Barach
2019-08-22 19:32:49 -04:00
parent a43c93f855
commit 84a563ae40
4 changed files with 22 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
MAINTAINERS
View File

@ -245,11 +245,6 @@ M: Dave Barach <dave@barachs.net>
M: Neale Ranns <nranns@cisco.com> M: Neale Ranns <nranns@cisco.com>
F: src/vnet/dhcp/ F: src/vnet/dhcp/
VNET DNS
I: dns
M: Dave Barach <dave@barachs.net>
F: src/vnet/dns/
VNET TLS and TLS engine plugins VNET TLS and TLS engine plugins
I: tls I: tls
M: Florin Coras <fcoras@cisco.com> M: Florin Coras <fcoras@cisco.com>
@ -268,6 +263,11 @@ I: abf
M: Neale Ranns <nranns@cisco.com> M: Neale Ranns <nranns@cisco.com>
F: src/plugins/abf/ 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) Plugin - Group Based Policy (GBP)
I: gbp I: gbp
M: Neale Ranns <nranns@cisco.com> M: Neale Ranns <nranns@cisco.com>

17
src/plugins/dns/dns.c
View File

@ -68,7 +68,7 @@ dns_cache_clear (dns_main_t * dm)
if (dm->is_enabled == 0) if (dm->is_enabled == 0)
return VNET_API_ERROR_NAME_RESOLUTION_NOT_ENABLED; return VNET_API_ERROR_NAME_RESOLUTION_NOT_ENABLED;
dns_cache_lock (dm); dns_cache_lock (dm, 1);
/* *INDENT-OFF* */ /* *INDENT-OFF* */
pool_foreach (ep, dm->entries, pool_foreach (ep, dm->entries,
@ -721,7 +721,7 @@ dns_delete_by_name (dns_main_t * dm, u8 * name)
if (dm->is_enabled == 0) if (dm->is_enabled == 0)
return VNET_API_ERROR_NAME_RESOLUTION_NOT_ENABLED; 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); p = hash_get_mem (dm->cache_entry_by_name, name);
if (!p) if (!p)
{ {
@ -755,7 +755,7 @@ delete_random_entry (dns_main_t * dm)
return VNET_API_ERROR_UNSPECIFIED; return VNET_API_ERROR_UNSPECIFIED;
#endif #endif
dns_cache_lock (dm); dns_cache_lock (dm, 3);
limit = pool_elts (dm->entries); limit = pool_elts (dm->entries);
start_index = random_u32 (&dm->random_seed) % limit; 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) if (dm->is_enabled == 0)
return VNET_API_ERROR_NAME_RESOLUTION_NOT_ENABLED; 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); p = hash_get_mem (dm->cache_entry_by_name, name);
if (p) 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) if (name[0] == 0)
return VNET_API_ERROR_INVALID_VALUE; return VNET_API_ERROR_INVALID_VALUE;
dns_cache_lock (dm); dns_cache_lock (dm, 5);
search_again: search_again:
p = hash_get_mem (dm->cache_entry_by_name, name); p = hash_get_mem (dm->cache_entry_by_name, name);
if (p) if (p)
@ -903,9 +903,8 @@ search_again:
name = ep->cname; name = ep->cname;
goto search_again; goto search_again;
} }
/* Note: caller must drop the lock! */
*retp = ep; *retp = ep;
dns_cache_unlock (dm);
return (0); return (0);
} }
else else
@ -2133,7 +2132,7 @@ format_dns_cache (u8 * s, va_list * args)
return s; return s;
} }
dns_cache_lock (dm); dns_cache_lock (dm, 6);
if (name) if (name)
{ {
@ -2736,7 +2735,7 @@ test_dns_expire_command_fn (vlib_main_t * vm,
else else
return clib_error_return (0, "no name provided"); 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); p = hash_get_mem (dm->cache_entry_by_name, name);
if (!p) if (!p)

8
src/plugins/dns/dns.h
View File

@ -99,6 +99,7 @@ typedef struct
/** Find cached record by name */ /** Find cached record by name */
uword *cache_entry_by_name; uword *cache_entry_by_name;
clib_spinlock_t cache_lock; clib_spinlock_t cache_lock;
int cache_lock_tag;
/** enable / disable flag */ /** enable / disable flag */
int is_enabled; int is_enabled;
@ -198,11 +199,14 @@ void vnet_dns_create_resolver_process (dns_main_t * dm);
format_function_t format_dns_reply; format_function_t format_dns_reply;
static inline void static inline void
dns_cache_lock (dns_main_t * dm) dns_cache_lock (dns_main_t * dm, int tag)
{ {
if (dm->cache_lock) if (dm->cache_lock)
{ {
ASSERT (tag);
ASSERT (dm->cache_lock_tag == 0);
clib_spinlock_lock (&dm->cache_lock); 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) if (dm->cache_lock)
{ {
ASSERT (dm->cache_lock_tag);
dm->cache_lock_tag = 0;
clib_spinlock_unlock (&dm->cache_lock); clib_spinlock_unlock (&dm->cache_lock);
} }
} }

4
src/plugins/dns/resolver_process.c
View File

@ -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 */ /* $$$ u16 limits cache to 65K entries, fix later multiple dst ports */
pool_index = clib_net_to_host_u16 (d->id); 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)) 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++) 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]); ep = pool_elt_at_index (dm->entries, dm->unresolved_entries[i]);
ASSERT ((ep->flags & DNS_CACHE_ENTRY_FLAG_VALID) == 0); ASSERT ((ep->flags & DNS_CACHE_ENTRY_FLAG_VALID) == 0);