MEDIUM: thread/dns: Make DNS thread-safe
authorChristopher Faulet <cfaulet@haproxy.com>
Wed, 4 Oct 2017 14:17:58 +0000 (16:17 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 31 Oct 2017 12:58:33 +0000 (13:58 +0100)
include/common/hathreads.h
include/types/dns.h
src/cfgparse.c
src/dns.c
src/server.c

index 39d8220..242bece 100644 (file)
@@ -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++) {
index 1c68551..bdc8cfc 100644 (file)
@@ -25,6 +25,7 @@
 #include <eb32tree.h>
 
 #include <common/mini-clist.h>
+#include <common/hathreads.h>
 
 #include <types/connection.h>
 #include <types/obj_type.h>
@@ -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 */
 };
 
index ca2d5d7..d693bfb 100644 (file)
@@ -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;
index 3383faa..1fdc461 100644 (file)
--- 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);
 
index 648f3dd..f0b912f 100644 (file)
@@ -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