From: Henrik Nordstrom Date: Fri, 27 Jan 2012 12:47:06 +0000 (-0700) Subject: Send DNS A and AAAA queries in parallel X-Git-Tag: SQUID_3_2_0_15~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cc5cbd1d495fa423169953d5d958d6721eeae347;p=thirdparty%2Fsquid.git Send DNS A and AAAA queries in parallel 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. This also drops the dns_v4_fallback directive as it have no effect with parallel DNS lookups. This directive should be reinstanciated in future to enable pure IPv6 usage. --- diff --git a/src/cf.data.pre b/src/cf.data.pre index fe6e9904bc..52a0551227 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -111,6 +111,12 @@ DOC_START Remove this line. The HTTP/1.1 feature is now fully supported by default. DOC_END +NAME: dns_v4_fallback +TYPE: obsolete +DOC_START + Remove this line. +DOC_END + NAME: ftp_list_width TYPE: obsolete DOC_START @@ -7236,27 +7242,6 @@ DOC_START nameservers by setting this option to 'off'. DOC_END -NAME: dns_v4_fallback -TYPE: onoff -DEFAULT: on -LOC: Config.onoff.dns_require_A -IFDEF: !USE_DNSHELPER -DOC_START - Standard practice with DNS is to lookup either A or AAAA records - and use the results if it succeeds. Only looking up the other if - the first attempt fails or otherwise produces no results. - - That policy however will cause squid to produce error pages for some - servers that advertise AAAA but are unreachable over IPv6. - - If this is ON squid will always lookup both AAAA and A, using both. - If this is OFF squid will lookup AAAA and only try A if none found. - - WARNING: There are some possibly unwanted side-effects with this on: - *) Doubles the load placed by squid on the DNS network. - *) May negatively impact connection delay times. -DOC_END - NAME: dns_v4_first TYPE: onoff DEFAULT: off diff --git a/src/dns_internal.cc b/src/dns_internal.cc index 1135e0abcd..3625dc8cc4 100644 --- a/src/dns_internal.cc +++ b/src/dns_internal.cc @@ -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 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(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(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); diff --git a/src/structs.h b/src/structs.h index 15916c1890..a807ca8486 100644 --- a/src/structs.h +++ b/src/structs.h @@ -445,7 +445,6 @@ struct SquidConfig { int emailErrData; int httpd_suppress_version_string; int global_internal_static; - int dns_require_A; #if FOLLOW_X_FORWARDED_FOR int acl_uses_indirect_client;