]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Send DNS A and AAAA queries in parallel
authorHenrik Nordstrom <henrik@henriknordstrom.net>
Fri, 6 Jan 2012 20:30:35 +0000 (21:30 +0100)
committerHenrik Nordstrom <henrik@henriknordstrom.net>
Fri, 6 Jan 2012 20:30:35 +0000 (21:30 +0100)
This implements sending DNS A & AAAA queries in parallel by
creating "slave" idns_query requests. Current implementation
uses the A lookup as "master" and AAAA as "slave" query.

Long term this should probably be restructured to separate "lookup
state" and "query", or even better yet to defer the DNS lookups
until connect time and perform A respective AAAA as needed only and
not look up both before attemting to connect.

src/dns_internal.cc

index 1135e0abcd2eac92926bf9777d4647f14325fe43..36eb89f9bfa9ab1e2dfe9dba1133d9d82fa7482e 100644 (file)
@@ -127,11 +127,12 @@ struct _idns_query {
     char name[NS_MAXDNAME + 1];
     char orig[NS_MAXDNAME + 1];
     ssize_t sz;
-    unsigned short msg_id; /// random query ID sent to server; changes with every query sent
+    unsigned short query_id; /// random query ID sent to server; changes with every query sent
     InstanceId<idns_query> xact_id; /// identifies our "transaction", stays constant when query is retried
 
     int nsends;
     int need_vc;
+    int pending;
 
     struct timeval start_t;
     struct timeval sent_t;
@@ -142,13 +143,12 @@ struct _idns_query {
     int attempt;
     int rcode;
     idns_query *queue;
+    idns_query *slave;  // AAAA
+    idns_query *master; // A
     unsigned short domain;
     unsigned short do_searchpath;
-    bool need_A;
-    struct {
-        int count;
-        rfc1035_rr *answers;
-    } initial_AAAA;
+    rfc1035_message *message;
+    int ancount;
 };
 InstanceIdDefinitions(idns_query,  "dns");
 
@@ -249,6 +249,7 @@ static void idnsTickleQueue(void);
 static void idnsRcodeCount(int, int);
 static CLCB idnsVCClosed;
 static unsigned short idnsQueryID(void);
+static void idnsSendSlaveAAAAQuery(idns_query *q);
 
 static void
 idnsAddNameserver(const char *buf)
@@ -670,7 +671,7 @@ idnsStats(StoreEntry * sentry)
     for (n = lru_list.head; n; n = n->next) {
         q = (idns_query *)n->data;
         storeAppendPrintf(sentry, "%#06x %4d %5d %10.3f %9.3f\n",
-                          (int) q->msg_id, (int) q->sz, q->nsends,
+                          (int) q->query_id, (int) q->sz, q->nsends,
                           tvSubDsec(q->start_t, current_time),
                           tvSubDsec(q->sent_t, current_time));
     }
@@ -906,8 +907,6 @@ idnsSendQuery(idns_query * q)
     int ns;
 
     q->start_t = current_time;
-    q->msg_id = idnsQueryID();
-    rfc1035SetQueryID(q->buf, q->msg_id);
 
     do {
         ns = q->nsends % nns;
@@ -918,7 +917,7 @@ idnsSendQuery(idns_query * q)
         } else {
             if (DnsSocketB >= 0 && nameservers[ns].S.IsIPv6())
                 y = comm_udp_sendto(DnsSocketB, nameservers[ns].S, q->buf, q->sz);
-            else if (DnsSocketA)
+            else if (DnsSocketA >= 0)
                 x = comm_udp_sendto(DnsSocketA, nameservers[ns].S, q->buf, q->sz);
         }
 
@@ -943,6 +942,7 @@ idnsSendQuery(idns_query * q)
     nameservers[ns].nqueries++;
     q->queue_t = current_time;
     dlinkAdd(q, &q->lru, &lru_list);
+    q->pending = 1;
     idnsTickleQueue();
 }
 
@@ -973,7 +973,7 @@ idnsFindQuery(unsigned short id)
     for (n = lru_list.tail; n; n = n->prev) {
         q = (idns_query*)n->data;
 
-        if (q->msg_id == id)
+        if (q->query_id == id)
             return q;
     }
 
@@ -1028,16 +1028,6 @@ idnsCallback(idns_query *q, rfc1035_rr *answers, int n, const char *error)
     }
 }
 
-void
-idnsDropMessage(rfc1035_message *message, idns_query *q)
-{
-    rfc1035MessageDestroy(&message);
-    if (q->hash.key) {
-        hash_remove_link(idns_lookup_hash, &q->hash);
-        q->hash.key = NULL;
-    }
-}
-
 static void
 idnsGrokReply(const char *buf, size_t sz, int from_ns)
 {
@@ -1092,9 +1082,11 @@ idnsGrokReply(const char *buf, size_t sz, int from_ns)
     }
 #endif
 
+    dlinkDelete(&q->lru, &lru_list);
+    q->pending = 0;
+
     if (message->tc) {
         debugs(78, 3, HERE << "Resolver requested TC (" << q->query.name << ")");
-        dlinkDelete(&q->lru, &lru_list);
         rfc1035MessageDestroy(&message);
 
         if (!q->need_vc) {
@@ -1112,7 +1104,6 @@ idnsGrokReply(const char *buf, size_t sz, int from_ns)
         return;
     }
 
-    dlinkDelete(&q->lru, &lru_list);
     idnsRcodeCount(n, q->attempt);
 
     if (n < 0) {
@@ -1131,7 +1122,7 @@ idnsGrokReply(const char *buf, size_t sz, int from_ns)
             return;
         }
 
-        if (q->rcode == 3 && q->do_searchpath && q->attempt < MAX_ATTEMPT) {
+        if (q->rcode == 3 && !q->master && q->do_searchpath && q->attempt < MAX_ATTEMPT) {
             assert(NULL == message->answer);
             strcpy(q->name, q->orig);
 
@@ -1146,17 +1137,21 @@ idnsGrokReply(const char *buf, size_t sz, int from_ns)
                 q->attempt++;
             }
 
-            idnsDropMessage(message, q);
+            rfc1035MessageDestroy(&message);
 
-            if (Ip::EnableIpv6 && q->query.qtype == RFC1035_TYPE_AAAA) {
-                debugs(78, 3, "idnsGrokReply: Trying AAAA Query for " << q->name);
-                q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, Config.dns.packet_max);
-            } else {
-                debugs(78, 3, "idnsGrokReply: Trying A Query for " << q->name);
-                // see EDNS notes at top of file why this sends 0
-                q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, 0);
+            // cleanup stale AAAA query
+            while (idns_query *slave = q->slave) {
+                dlinkDelete(&slave->lru, &lru_list);
+                q->slave = slave->slave;
+                rfc1035MessageDestroy(&slave->message);
+                cbdataFree(slave);
             }
-
+            
+            // Build new query
+            q->query_id = idnsQueryID();
+            debugs(78, 3, "idnsGrokReply: Trying A Query for " << q->name);
+            // see EDNS notes at top of file why this sends 0
+            q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query, 0);
             if (q->sz < 0) {
                 /* problem with query data -- query not sent */
                 idnsCallback(static_cast<idns_query *>(q->callback_data), NULL, 0, "Internal error");
@@ -1164,94 +1159,62 @@ idnsGrokReply(const char *buf, size_t sz, int from_ns)
                 return;
             }
 
-            idnsCacheQuery(q);
             idnsSendQuery(q);
+            if (Ip::EnableIpv6)
+                idnsSendSlaveAAAAQuery(q);
             return;
         }
     }
 
-    if (q->need_A && (Config.onoff.dns_require_A == 1 || n <= 0 ) ) {
-        /* ERROR or NO AAAA exist. Failover to A records. */
-        /*      Apparently its also a good idea to lookup and store the A records
-         *      just in case the AAAA are not available when we need them.
-         *      This could occur due to number of network failings beyond our control
-         *      thus the || above allowing the user to request always both.
-         */
-
-        if (n == 0)
-            debugs(78, 3, "idnsGrokReply: " << q->name << " has no AAAA records. Looking up A record instead.");
-        else if (q->need_A && n <= 0)
-            debugs(78, 3, "idnsGrokReply: " << q->name << " AAAA query failed. Trying A now instead.");
-        else // admin requested this.
-            debugs(78, 3, "idnsGrokReply: " << q->name << " AAAA query done. Configured to retrieve A now also.");
-
-        // move the initial message results into the failover query for merging later.
-        if (n > 0) {
-            q->initial_AAAA.count = message->ancount;
-            q->initial_AAAA.answers = message->answer;
-            message->answer = NULL;
-        }
-
-        // remove the hashed query info
-        idnsDropMessage(message, q);
+    q->message = message;
+    q->ancount = n;
 
-        // reset the query as an A query
-        q->nsends = 0;
-        // see EDNS notes at top of file why this sends 0
-        q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, 0);
-        q->need_A = false;
+    if (q->master)
+        q = q->master;
 
-        if (q->sz < 0) {
-            /* problem with query data -- query not sent */
-            idnsCallback(static_cast<idns_query *>(q->callback_data), NULL, 0, "Internal error");
-            cbdataFree(q);
+    idns_query *q2;
+    // If any of our subqueries are still pending then wait for them to complete before continuing
+    for ( q2 = q; q2; q2 = q2->slave) {
+        if (q2->pending) {
             return;
         }
-
-        idnsCacheQuery(q);
-        idnsSendQuery(q);
-        return;
     }
 
-    /** If there are two result sets from preceeding AAAA and A lookups merge them with a preference for AAAA */
-    if (q->initial_AAAA.count > 0 && n > 0) {
-        /* two sets of RR need merging */
-        rfc1035_rr *result = (rfc1035_rr*) xmalloc( sizeof(rfc1035_rr)*(n + q->initial_AAAA.count) );
-        rfc1035_rr *tmp = result;
-
-        debugs(78, 6, HERE << "Merging DNS results " << q->name << " AAAA has " << q->initial_AAAA.count << " RR, A has " << n << " RR");
-
-        if (Config.dns.v4_first) {
-            memcpy( tmp, message->answer, (sizeof(rfc1035_rr)*n) );
-            tmp += n;
-            /* free the RR object without freeing its child strings (they are now taken by the copy above) */
-            safe_free(message->answer);
-        }
-
-        memcpy(tmp, q->initial_AAAA.answers, (sizeof(rfc1035_rr)*(q->initial_AAAA.count)) );
-        tmp += q->initial_AAAA.count;
-        /* free the RR object without freeing its child strings (they are now taken by the copy above) */
-        safe_free(q->initial_AAAA.answers);
-
-        if (!Config.dns.v4_first) {
-            memcpy( tmp, message->answer, (sizeof(rfc1035_rr)*n) );
-            /* free the RR object without freeing its child strings (they are now taken by the copy above) */
-            safe_free(message->answer);
+    /* Merge results */
+    message = q->message;
+    n = q->ancount;
+    
+    while ( (q2 = q->slave) ) {
+        debugs(78, 6, HERE << "Merging DNS results " << q->name << " A has " << n << " RR, AAAA has " << q2->ancount << " RR");
+        q->slave = q2->slave;
+        if ( q2->ancount >= 0 ) {
+            if (n > 0 ) {
+                // two sets of RR need merging
+                rfc1035_rr *result = (rfc1035_rr*) xmalloc( sizeof(rfc1035_rr)*(n + q2->ancount) );
+                if (Config.dns.v4_first) {
+                    memcpy(result, message->answer, (sizeof(rfc1035_rr)*n) );
+                    memcpy(result+n, q2->message->answer, (sizeof(rfc1035_rr)*q2->ancount) );
+                } else {
+                    memcpy(result, q2->message->answer, (sizeof(rfc1035_rr)*q2->ancount) );
+                    memcpy(result+q2->ancount, message->answer, (sizeof(rfc1035_rr)*n) );
+                }
+                n += q2->ancount;
+                safe_free(message->answer);
+                message->answer = result;
+                message->ancount += q2->message->ancount;
+                safe_free(q2->message->answer);
+                q2->message->answer = NULL;
+            } else if (n < 0 || q2->ancount > 0) {
+                // first set empty / failed
+                rfc1035MessageDestroy(&message);
+                message = q->message = q2->message;
+                q2->message = NULL;
+                n = q2->ancount;
+            }
         }
-
-        n += q->initial_AAAA.count;
-        q->initial_AAAA.count = 0;
-        message->answer = result;
-        message->ancount = n;
-    } else if (q->initial_AAAA.count > 0 && n <= 0) {
-        /* initial of dual queries was the only result set. */
-        debugs(78, 6, HERE << "Merging DNS results " << q->name << " AAAA has " << q->initial_AAAA.count << " RR, A has " << n << " RR");
-        rfc1035RRDestroy(&(message->answer), n);
-        message->answer = q->initial_AAAA.answers;
-        n = q->initial_AAAA.count;
-        message->ancount = n;
+        rfc1035MessageDestroy(&q2->message);
+        cbdataFree(q2);
     }
-    /* else initial results were empty. just use the final set as authoritative */
 
     debugs(78, 6, HERE << "Sending " << n << " DNS results to caller.");
     idnsCallback(q, message->answer, n, rfc1035ErrorMessage(n));
@@ -1375,7 +1338,7 @@ idnsCheckQueue(void *unused)
 
         debugs(78, 3, "idnsCheckQueue: ID " << q->xact_id <<
                " QID 0x"  << std::hex << std::setfill('0')  <<
-               std::setw(4) << q->msg_id << ": timeout" );
+               std::setw(4) << q->query_id << ": timeout" );
 
         dlinkDelete(&q->lru, &lru_list);
 
@@ -1383,7 +1346,7 @@ idnsCheckQueue(void *unused)
             idnsSendQuery(q);
         } else {
             debugs(78, 2, "idnsCheckQueue: ID " << q->xact_id <<
-                   " QID 0x" << std::hex << q->msg_id <<
+                   " QID 0x" << std::hex << q->query_id <<
                    " : giving up after " << std::dec << q->nsends << " tries and " <<
                    std::setw(5)<< std::setprecision(2) << tvSubDsec(q->start_t, current_time) << " seconds");
 
@@ -1500,30 +1463,30 @@ dnsInit(void)
     CBDATA_INIT_TYPE(idns_query);
 
     if (DnsSocketA < 0 && DnsSocketB < 0) {
-        Ip::Address addrA; // since we don't want to alter Config.Addrs.udp_* and dont have one of our own.
+        Ip::Address addrV6; // since we don't want to alter Config.Addrs.udp_* and dont have one of our own.
 
         if (!Config.Addrs.udp_outgoing.IsNoAddr())
-            addrA = Config.Addrs.udp_outgoing;
+            addrV6 = Config.Addrs.udp_outgoing;
         else
-            addrA = Config.Addrs.udp_incoming;
+            addrV6 = Config.Addrs.udp_incoming;
 
-        Ip::Address addrB = addrA;
-        addrA.SetIPv4();
+        Ip::Address addrV4 = addrV6;
+        addrV4.SetIPv4();
 
-        if (Ip::EnableIpv6 && addrB.IsIPv6()) {
-            debugs(78, 2, "idnsInit: attempt open DNS socket to: " << addrB);
+        if (Ip::EnableIpv6 && addrV6.IsIPv6()) {
+            debugs(78, 2, "idnsInit: attempt open DNS socket to: " << addrV6);
             DnsSocketB = comm_open_listener(SOCK_DGRAM,
                                             IPPROTO_UDP,
-                                            addrB,
+                                            addrV6,
                                             COMM_NONBLOCKING,
                                             "DNS Socket IPv6");
         }
 
-        if (addrA.IsIPv4()) {
-            debugs(78, 2, "idnsInit: attempt open DNS socket to: " << addrA);
+        if (addrV4.IsIPv4()) {
+            debugs(78, 2, "idnsInit: attempt open DNS socket to: " << addrV4);
             DnsSocketA = comm_open_listener(SOCK_DGRAM,
                                             IPPROTO_UDP,
-                                            addrA,
+                                            addrV4,
                                             COMM_NONBLOCKING,
                                             "DNS Socket IPv4");
         }
@@ -1536,12 +1499,12 @@ dnsInit(void)
          */
         if (DnsSocketB >= 0) {
             comm_local_port(DnsSocketB);
-            debugs(78, 1, "DNS Socket created at " << addrB << ", FD " << DnsSocketB);
+            debugs(78, 1, "DNS Socket created at " << addrV6 << ", FD " << DnsSocketB);
             Comm::SetSelect(DnsSocketB, COMM_SELECT_READ, idnsRead, NULL, 0);
         }
         if (DnsSocketA >= 0) {
             comm_local_port(DnsSocketA);
-            debugs(78, 1, "DNS Socket created at " << addrA << ", FD " << DnsSocketA);
+            debugs(78, 1, "DNS Socket created at " << addrV4 << ", FD " << DnsSocketA);
             Comm::SetSelect(DnsSocketA, COMM_SELECT_READ, idnsRead, NULL, 0);
         }
     }
@@ -1629,6 +1592,7 @@ idnsCachedLookup(const char *key, IDNSCB * callback, void *data)
     q = cbdataAlloc(idns_query);
     // idns_query is POD so no constructors are called after allocation
     q->xact_id.change();
+    // no query_id on this instance.
 
     q->callback = callback;
 
@@ -1644,10 +1608,31 @@ idnsCachedLookup(const char *key, IDNSCB * callback, void *data)
 static void
 idnsCacheQuery(idns_query *q)
 {
-    q->hash.key = q->query.name;
+    q->hash.key = q->orig;
     hash_join(idns_lookup_hash, &q->hash);
 }
 
+static void
+idnsSendSlaveAAAAQuery(idns_query *master)
+{
+    idns_query *q = cbdataAlloc(idns_query);
+    memcpy(q->name, master->name, sizeof(q->name));
+    memcpy(q->orig, master->orig, sizeof(q->orig));
+    q->master = master;
+    q->query_id = idnsQueryID();
+    q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query, Config.dns.packet_max);
+    q->slave = master->slave;
+    
+    debugs(78, 3, "idnsALookup: buf is " << q->sz << " bytes for " << q->name <<
+           ", id = 0x" << std::hex << q->query_id);
+    if (!q->sz) {
+        cbdataFree(q);
+        return;
+    }
+    master->slave = q;
+    idnsSendQuery(q);
+}
+
 void
 idnsALookup(const char *name, IDNSCB * callback, void *data)
 {
@@ -1661,6 +1646,7 @@ idnsALookup(const char *name, IDNSCB * callback, void *data)
     q = cbdataAlloc(idns_query);
     // idns_query is POD so no constructors are called after allocation
     q->xact_id.change();
+    q->query_id = idnsQueryID();
 
     for (i = 0; i < strlen(name); i++)
         if (name[i] == '.')
@@ -1682,14 +1668,8 @@ idnsALookup(const char *name, IDNSCB * callback, void *data)
         debugs(78, 3, "idnsALookup: searchpath used for " << q->name);
     }
 
-    if (Ip::EnableIpv6) {
-        q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, Config.dns.packet_max);
-        q->need_A = true;
-    } else {
-        // see EDNS notes at top of file why this sends 0
-        q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, 0);
-        q->need_A = false;
-    }
+    // see EDNS notes at top of file why this sends 0
+    q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query, 0);
 
     if (q->sz < 0) {
         /* problem with query data -- query not sent */
@@ -1699,13 +1679,17 @@ idnsALookup(const char *name, IDNSCB * callback, void *data)
     }
 
     debugs(78, 3, "idnsALookup: buf is " << q->sz << " bytes for " << q->name <<
-           ", id = 0x" << std::hex << q->msg_id);
+           ", id = 0x" << std::hex << q->query_id);
 
     q->callback = callback;
     q->callback_data = cbdataReference(data);
 
     idnsCacheQuery(q);
     idnsSendQuery(q);
+
+    if (Ip::EnableIpv6)
+        idnsSendSlaveAAAAQuery(q);
+
 }
 
 void
@@ -1721,6 +1705,7 @@ idnsPTRLookup(const Ip::Address &addr, IDNSCB * callback, void *data)
 
     // idns_query is POD so no constructors are called after allocation
     q->xact_id.change();
+    q->query_id = idnsQueryID();
 
     if (addr.IsIPv6()) {
         struct in6_addr addr6;
@@ -1733,9 +1718,6 @@ idnsPTRLookup(const Ip::Address &addr, IDNSCB * callback, void *data)
         q->sz = rfc3596BuildPTRQuery4(addr4, q->buf, sizeof(q->buf), 0, &q->query, 0);
     }
 
-    /* PTR does not do inbound A/AAAA */
-    q->need_A = false;
-
     if (q->sz < 0) {
         /* problem with query data -- query not sent */
         callback(data, NULL, 0, "Internal error");
@@ -1749,7 +1731,7 @@ idnsPTRLookup(const Ip::Address &addr, IDNSCB * callback, void *data)
     }
 
     debugs(78, 3, "idnsPTRLookup: buf is " << q->sz << " bytes for " << ip <<
-           ", id = 0x" << std::hex << q->msg_id);
+           ", id = 0x" << std::hex << q->query_id);
 
     q->callback = callback;
     q->callback_data = cbdataReference(data);