From 21f0f88b10dac957d1d01a288f6961200436c1d3 Mon Sep 17 00:00:00 2001 From: Bert Hubert Date: Fri, 18 Jan 2013 19:04:44 +0000 Subject: [PATCH] fix average delay stats where we'd not return the right value after the first statistic was submitted (only after the second) stop doing additional processing for the recursor (double check we still do it for glue in the embedded auth server! probably not) stop using ANY to lookup A and AAAA records in one go, reflection attacks killed ANY over at UltraDNS start treating the IPv4 and IPv6 addresses of nameservers separately, since their performance is often very different (and this fixes our A/AAAA->ANY lookup delay) git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@3064 d19b8d6e-7fed-0310-83ef-9ca221ded41b --- pdns/misc.hh | 13 ++++++++ pdns/syncres.cc | 85 ++++++++++++++++++++++++++----------------------- pdns/syncres.hh | 23 ++++++------- 3 files changed, 70 insertions(+), 51 deletions(-) diff --git a/pdns/misc.hh b/pdns/misc.hh index b5cf0a978..7b1489aaa 100644 --- a/pdns/misc.hh +++ b/pdns/misc.hh @@ -403,6 +403,19 @@ struct CIStringCompare: public std::binary_function } }; +struct CIStringPairCompare: public std::binary_function, pair, bool> +{ + bool operator()(const pair& a, const pair& b) const + { + if(pdns_ilexicographical_compare(a.first, b.first)) + return true; + if(pdns_ilexicographical_compare(b.first, a.first)) + return false; + return a.second < b.second; + } +}; + + pair splitField(const string& inp, char sepa); inline bool isCanonical(const string& dom) diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 5b2892e91..6352e097a 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -17,6 +17,7 @@ */ #include +#include #include "utility.hh" #include "syncres.hh" #include @@ -451,10 +452,9 @@ static bool ipv6First(const ComboAddress& a, const ComboAddress& b) } #endif -/** This function explicitly goes out for A addresses, but if configured to use IPv6 as well, will also return any IPv6 addresses in the cache - Additionally, it will return the 'best' address up front, and the rest shuffled +/** This function explicitly goes out for A or AAAA addresses */ -vector SyncRes::getAs(const string &qname, int depth, set& beenthere) +vector SyncRes::getAddrs(const string &qname, int type, int depth, set& beenthere) { typedef vector res_t; res_t res; @@ -462,19 +462,19 @@ vector SyncRes::getAs(const string &qname, int depth, set ret_t; ret_t ret; - if(!doResolve(qname, s_doIPv6 ? QType(QType::ADDR) : QType(QType::A), res,depth+1,beenthere) && !res.empty()) { // this consults cache, OR goes out + if(!doResolve(qname, QType(type), res,depth+1,beenthere) && !res.empty()) { // this consults cache, OR goes out for(res_t::const_iterator i=res.begin(); i!= res.end(); ++i) { if(i->qtype.getCode()==QType::A || i->qtype.getCode()==QType::AAAA) { - ret.push_back(ComboAddress(i->content, 53)); + ret.push_back(ComboAddress(i->content, 53)); } } } - + if(ret.size() > 1) { random_shuffle(ret.begin(), ret.end(), dns_random); // move 'best' address for this nameserver name up front - nsspeeds_t::iterator best = t_sstorage->nsSpeeds.find(qname); + nsspeeds_t::iterator best = t_sstorage->nsSpeeds.find(make_pair(qname, type)); if(best != t_sstorage->nsSpeeds.end()) for(ret_t::iterator i=ret.begin(); i != ret.end(); ++i) { @@ -730,25 +730,28 @@ bool SyncRes::moreSpecificThan(const string& a, const string &b) struct speedOrder { - speedOrder(map &speeds) : d_speeds(speeds) {} - bool operator()(const string &a, const string &b) const + speedOrder(map &speeds) : d_speeds(speeds) {} + bool operator()(const SyncRes::typedns_t &a, const SyncRes::typedns_t &b) const { return d_speeds[a] < d_speeds[b]; } - map& d_speeds; + map& d_speeds; }; -inline vector SyncRes::shuffleInSpeedOrder(set &nameservers, const string &prefix) +inline vector SyncRes::shuffleInSpeedOrder(set &tnameservers, const string &prefix) { - vector rnameservers; - rnameservers.reserve(nameservers.size()); - map speeds; - - for(set::const_iterator i=nameservers.begin();i!=nameservers.end();++i) { - rnameservers.push_back(*i); + vector rnameservers; + rnameservers.reserve(tnameservers.size()*(1+1*s_doIPv6)); + BOOST_FOREACH(const string& str, tnameservers) { + rnameservers.push_back(make_pair(str, QType::A)); + if(s_doIPv6) + rnameservers.push_back(make_pair(str, QType::AAAA)); + } + map speeds; + BOOST_FOREACH(const typedns_t& val, rnameservers) { double speed; - speed=t_sstorage->nsSpeeds[*i].get(&d_now); - speeds[*i]=speed; + speed=t_sstorage->nsSpeeds[val].get(&d_now); + speeds[val]=speed; } random_shuffle(rnameservers.begin(),rnameservers.end(), dns_random); speedOrder so(speeds); @@ -756,14 +759,14 @@ inline vector SyncRes::shuffleInSpeedOrder(set if(doLog()) { LOG(prefix<<"Nameservers: "); - for(vector::const_iterator i=rnameservers.begin();i!=rnameservers.end();++i) { + for(vector::const_iterator i=rnameservers.begin();i!=rnameservers.end();++i) { if(i!=rnameservers.begin()) { LOG(", "); if(!((i-rnameservers.begin())%3)) { LOG(endl<first<<"|"<<((i->second==QType::A) ? "A" : "AAAA") <<"(" << (boost::format("%0.2f") % (speeds[*i]/1000.0)).str() <<"ms)"); } LOG(endl); } @@ -804,9 +807,9 @@ int SyncRes::doResolveAt(set nameservers, string auth, LOG(prefix< rnameservers=shuffleInSpeedOrder(nameservers, doLog() ? (prefix+qname+": ") : string() ); - - for(vector::const_iterator tns=rnameservers.begin();;++tns) { + vector rnameservers = shuffleInSpeedOrder(nameservers, doLog() ? (prefix+qname+": ") : string() ); + + for(vector::const_iterator tns=rnameservers.begin();;++tns) { if(tns==rnameservers.end()) { LOG(prefix< nameservers, string auth, } return -1; } - if(pdns_iequals(qname, *tns) && qtype.getCode()==QType::A && rnameservers.size() > 1) { + // this line needs to identify the 'self-resolving' behaviour, but we get it wrong now + if(pdns_iequals(qname, tns->first) && qtype.getCode()==QType::A && rnameservers.size() > (unsigned)(1+1*s_doIPv6)) { LOG(prefix< nameservers, string auth, bool pierceDontQuery=false; bool sendRDQuery=false; LWResult lwr; - if(tns->empty()) { + if(tns->first.empty()) { LOG(prefix<first<<"|"<< (tns->second == QType::A ? "A" : "AAAA") << "' ("<<1+tns-rnameservers.begin()<<"/"<<(unsigned int)rnameservers.size()<<")"<first)) { LOG(prefix<empty()) { + string txtAddr = tns->first; + if(!tns->first.empty()) { sendRDQuery = txtAddr[0] == '+'; txtAddr=txtAddr.c_str()+1; } @@ -852,17 +856,18 @@ int SyncRes::doResolveAt(set nameservers, string auth, pierceDontQuery=true; } else { - remoteIPs=getAs(*tns, depth+1, beenthere); + remoteIPs=getAddrs(tns->first, tns->second, depth+1, beenthere); pierceDontQuery=false; } if(remoteIPs.empty()) { - LOG(prefix<second == QType::A ? "IP" : "IPv6") << " for NS "<first<<", trying next if available"<first<<" to: "); for(remoteIP = remoteIPs.begin(); remoteIP != remoteIPs.end(); ++remoteIP) { if(remoteIP != remoteIPs.begin()) { LOG(", "); @@ -895,8 +900,7 @@ int SyncRes::doResolveAt(set nameservers, string auth, s_tcpoutqueries++; d_tcpoutqueries++; } - resolveret=asyncresolveWrapper(*remoteIP, qname, - (qtype.getCode() == QType::ADDR ? QType::ANY : qtype.getCode()), + resolveret=asyncresolveWrapper(*remoteIP, qname, qtype.getCode(), doTCP, sendRDQuery, &d_now, &lwr); // <- we go out on the wire! if(resolveret != 1) { if(resolveret==0) { @@ -928,7 +932,7 @@ int SyncRes::doResolveAt(set nameservers, string auth, break; // this IP address worked! wasLame:; // well, it didn't - LOG(prefix<toString() <<") is lame for '"<first<<" ("<< remoteIP->toString() <<") is lame for '"<throttle.throttle(d_now.tv_sec, make_tuple(*remoteIP, qname, qtype.getCode()), 60, 100); // lame } } @@ -947,11 +951,11 @@ int SyncRes::doResolveAt(set nameservers, string auth, } if(lwr.d_rcode==RCode::ServFail) { - LOG(prefix<first<<" returned a ServFail, trying sibling IP or NS"<throttle.throttle(d_now.tv_sec,make_tuple(*remoteIP, qname, qtype.getCode()),60,3); // servfail continue; } - LOG(prefix<toString() <<"), rcode="<first<<" ("<< remoteIP->toString() <<"), rcode="<sin4.sin_family==AF_INET6) @@ -1126,7 +1130,7 @@ int SyncRes::doResolveAt(set nameservers, string auth, nameservers=nsset; break; } - else if(isCanonical(*tns)) { + else if(isCanonical(tns->first)) { // means: not OOB (I think) goto wasLame; } } @@ -1168,7 +1172,8 @@ void SyncRes::addCruft(const string &qname, vector& ret) host=string(k->content.c_str() + fields[3].first, fields[3].second - fields[3].first); else continue; - doResolve(host, s_doAAAAAdditionalProcessing ? QType(QType::ADDR) : QType(QType::A), addit, 1, beenthere); + // we used to do additional processing here.. no more + // doResolve(host, QType(QType::A), addit, 1, beenthere); } if(!addit.empty()) { diff --git a/pdns/syncres.hh b/pdns/syncres.hh index 1eab1a55f..66825368c 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -134,14 +134,17 @@ public: if(d_needinit) { d_last=now; + d_lastget=now; d_needinit=false; + d_val = val; } + else { + float diff= makeFloat(d_last - now); - float diff= makeFloat(d_last - now); - - d_last=now; - double factor=exp(diff)/2.0; // might be '0.5', or 0.0001 - d_val=(float)((1-factor)*val+ (float)factor*d_val); + d_last=now; + double factor=exp(diff)/2.0; // might be '0.5', or 0.0001 + d_val=(float)((1-factor)*val+ (float)factor*d_val); + } } double get(struct timeval* tv) @@ -301,8 +304,8 @@ public: ComboAddress d_best; }; - typedef map nsspeeds_t; - + typedef pair typedns_t; + typedef map nsspeeds_t; struct EDNSStatus { @@ -314,8 +317,6 @@ public: typedef map ednsstatus_t; - - static bool s_noEDNSPing; static bool s_noEDNS; @@ -375,9 +376,9 @@ private: string getBestNSNamesFromCache(const string &qname,set& nsset, bool* flawedNSSet, int depth, set&beenthere); void addAuthorityRecords(const string& qname, vector& ret, int depth); - inline vector shuffleInSpeedOrder(set &nameservers, const string &prefix); + inline vector shuffleInSpeedOrder(set &nameservers, const string &prefix); bool moreSpecificThan(const string& a, const string &b); - vector getAs(const string &qname, int depth, set& beenthere); + vector getAddrs(const string &qname, int type, int depth, set& beenthere); private: ostringstream d_trace; -- 2.49.0