From: Christopher Faulet Date: Wed, 4 Oct 2017 14:17:58 +0000 (+0200) Subject: MEDIUM: thread/dns: Make DNS thread-safe X-Git-Tag: v1.8-rc1~119 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b2812a624096280489a0c7a1aabfbe1c94884d12;p=thirdparty%2Fhaproxy.git MEDIUM: thread/dns: Make DNS thread-safe --- diff --git a/include/common/hathreads.h b/include/common/hathreads.h index 39d82208e8..242bece4e4 100644 --- a/include/common/hathreads.h +++ b/include/common/hathreads.h @@ -168,6 +168,7 @@ enum lock_label { LUA_LOCK, NOTIF_LOCK, SPOE_APPLET_LOCK, + DNS_LOCK, LOCK_LABELS }; struct lock_stat { @@ -256,7 +257,7 @@ static inline void show_lock_stats() "UPDATED_SERVERS", "LBPRM", "SIGNALS", "STK_TABLE", "STK_SESS", "APPLETS", "PEER", "BUF_WQ", "STREAMS", "SSL", "SSL_GEN_CERTS", "PATREF", "PATEXP", "PATLRU", "VARS", "COMP_POOL", "LUA", - "NOTIF", "SPOE_APPLET" }; + "NOTIF", "SPOE_APPLET", "DNS" }; int lbl; for (lbl = 0; lbl < LOCK_LABELS; lbl++) { diff --git a/include/types/dns.h b/include/types/dns.h index 1c68551729..bdc8cfc5e8 100644 --- a/include/types/dns.h +++ b/include/types/dns.h @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -192,6 +193,9 @@ struct dns_resolvers { struct eb_root query_ids; /* tree to quickly lookup/retrieve query ids currently in use * used by each nameserver, but stored in resolvers since there must * be a unique relation between an eb_root and an eb_node (resolution) */ +#ifdef USE_THREAD + HA_SPINLOCK_T lock; +#endif struct list list; /* resolvers list */ }; diff --git a/src/cfgparse.c b/src/cfgparse.c index ca2d5d79db..d693bfbf56 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -2182,6 +2182,7 @@ int cfg_parse_resolvers(const char *file, int linenum, char **args, int kwm) LIST_INIT(&curr_resolvers->nameservers); LIST_INIT(&curr_resolvers->resolutions.curr); LIST_INIT(&curr_resolvers->resolutions.wait); + SPIN_INIT(&curr_resolvers->lock); } else if (strcmp(args[0], "nameserver") == 0) { /* nameserver definition */ struct sockaddr_storage *sk; diff --git a/src/dns.c b/src/dns.c index 3383faa701..1fdc461e60 100644 --- a/src/dns.c +++ b/src/dns.c @@ -45,7 +45,7 @@ struct list dns_resolvers = LIST_HEAD_INIT(dns_resolvers); struct list dns_srvrq_list = LIST_HEAD_INIT(dns_srvrq_list); -static int64_t dns_query_id_seed = 0; /* random seed */ +static THREAD_LOCAL int64_t dns_query_id_seed = 0; /* random seed */ static struct pool_head *dns_answer_item_pool = NULL; static struct pool_head *dns_resolution_pool = NULL; static unsigned int resolution_uuid = 1; @@ -125,6 +125,8 @@ struct dns_srvrq *new_dns_srvrq(struct server *srv, char *fqdn) /* 2 bytes random generator to generate DNS query ID */ static inline uint16_t dns_rnd16(void) { + if (!dns_query_id_seed) + dns_query_id_seed = now_ms; dns_query_id_seed ^= dns_query_id_seed << 13; dns_query_id_seed ^= dns_query_id_seed >> 7; dns_query_id_seed ^= dns_query_id_seed << 17; @@ -1444,6 +1446,7 @@ static void dns_resolve_recv(struct dgram_conn *dgram) return; resolvers = ns->resolvers; + SPIN_LOCK(DNS_LOCK, &resolvers->lock); /* process all pending input messages */ while (1) { @@ -1604,6 +1607,7 @@ static void dns_resolve_recv(struct dgram_conn *dgram) continue; } dns_update_resolvers_timeout(resolvers); + SPIN_UNLOCK(DNS_LOCK, &resolvers->lock); } /* Called when a resolvers network socket is ready to send data */ @@ -1628,6 +1632,8 @@ static void dns_resolve_send(struct dgram_conn *dgram) return; resolvers = ns->resolvers; + SPIN_LOCK(DNS_LOCK, &resolvers->lock); + list_for_each_entry(res, &resolvers->resolutions.curr, list) { int ret; @@ -1653,6 +1659,7 @@ static void dns_resolve_send(struct dgram_conn *dgram) ns->counters.snd_error++; res->nb_queries++; } + SPIN_UNLOCK(DNS_LOCK, &resolvers->lock); } /* Processes DNS resolution. First, it checks the active list to detect expired @@ -1665,6 +1672,8 @@ static struct task *dns_process_resolvers(struct task *t) struct dns_resolution *res, *resback; int exp; + SPIN_LOCK(DNS_LOCK, &resolvers->lock); + /* Handle all expired resolutions from the active list */ list_for_each_entry_safe(res, resback, &resolvers->resolutions.curr, list) { /* When we find the first resolution in the future, then we can @@ -1733,6 +1742,7 @@ static struct task *dns_process_resolvers(struct task *t) } dns_update_resolvers_timeout(resolvers); + SPIN_UNLOCK(DNS_LOCK, &resolvers->lock); return t; } @@ -2015,9 +2025,6 @@ static void __dns_init(void) dns_answer_item_pool = create_pool("dns_answer_item", sizeof(struct dns_answer_item), MEM_F_SHARED); dns_resolution_pool = create_pool("dns_resolution", sizeof(struct dns_resolution), MEM_F_SHARED); - /* give a first random value to our dns query_id seed */ - dns_query_id_seed = random(); - hap_register_post_check(dns_finalize_config); hap_register_post_deinit(dns_deinit); diff --git a/src/server.c b/src/server.c index 648f3dd263..f0b912f66d 100644 --- a/src/server.c +++ b/src/server.c @@ -3751,11 +3751,12 @@ int srv_set_fqdn(struct server *srv, const char *hostname) char *hostname_dn; int hostname_len, hostname_dn_len; + SPIN_LOCK(DNS_LOCK, &srv->resolvers->lock); /* run time DNS resolution was not active for this server * and we can't enable it at run time for now. */ if (!srv->dns_requester) - return -1; + goto err; chunk_reset(&trash); hostname_len = strlen(hostname); @@ -3763,13 +3764,13 @@ int srv_set_fqdn(struct server *srv, const char *hostname) hostname_dn_len = dns_str_to_dn_label(hostname, hostname_len + 1, hostname_dn, trash.size); if (hostname_dn_len == -1) - return -1; + goto err; resolution = srv->dns_requester->resolution; if (resolution && resolution->hostname_dn && !strcmp(resolution->hostname_dn, hostname_dn)) - return 0; + goto end; dns_unlink_resolution(srv->dns_requester); @@ -3779,11 +3780,18 @@ int srv_set_fqdn(struct server *srv, const char *hostname) srv->hostname_dn = strdup(hostname_dn); srv->hostname_dn_len = hostname_dn_len; if (!srv->hostname || !srv->hostname_dn) - return -1; + goto err; if (dns_link_resolution(srv, OBJ_TYPE_SERVER) == -1) - return -1; + goto err; + + end: + SPIN_UNLOCK(DNS_LOCK, &srv->resolvers->lock); return 0; + + err: + SPIN_UNLOCK(DNS_LOCK, &srv->resolvers->lock); + return -1; } /* Sets the server's address (srv->addr) from srv->lastaddr which was filled