The PowerDNS recursor did not use to try all IP addresses of a nameserver!

It does so now, as explained in http://mailman.powerdns.com/pipermail/pdns-users/2006-March/003101.html and
http://mailman.powerdns.com/pipermail/pdns-users/2006-March/003106.html

Thanks to Mark Martin for noticing this.

git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@570 d19b8d6e-7fed-0310-83ef-9ca221ded41b

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index c29d5d4f94649af53dfbd308e5bdc643645a749b..edb50935fb943ea7078eebb0ce4dcd05b5c2f6b7 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -100,14 +100,20 @@ int SyncRes::doResolve(const string &qname, const QType &qtype, vector<DNSResour
   return res<0 ? RCode::ServFail : res;
 }
 
-string SyncRes::getA(const string &qname, int depth, set<GetBestNSAnswer>& beenthere)
+vector<string> SyncRes::getAs(const string &qname, int depth, set<GetBestNSAnswer>& beenthere)
 {
-  vector<DNSResourceRecord> res;
-  string ret;
+  typedef vector<DNSResourceRecord> res_t;
+  res_t res;
 
-  if(!doResolve(qname,QType(QType::A), res,depth+1,beenthere) && !res.empty()) 
-    ret=res[res.size()-1].content; // last entry, in case of CNAME in between
+  vector<string> ret;
 
+  if(!doResolve(qname,QType(QType::A), res,depth+1,beenthere) && !res.empty()) {
+    for(res_t::const_iterator i=res.begin(); i!= res.end(); ++i)
+      if(i->qtype.getCode()==QType::A)
+       ret.push_back(i->content);
+  }
+  if(ret.size() > 1)
+    random_shuffle(ret.begin(), ret.end());
   return ret;
 }
 
@@ -375,48 +381,55 @@ int SyncRes::doResolveAt(set<string> nameservers, string auth, const string &qna
        continue;
       }
       LOG<<prefix<<qname<<": Trying to resolve NS "<<*tns<<" ("<<1+tns-rnameservers.begin()<<"/"<<rnameservers.size()<<")"<<endl;
-      string remoteIP=getA(*tns, depth+1,beenthere);
-      if(remoteIP.empty()) {
+      typedef vector<string> remoteIPs_t;
+      remoteIPs_t remoteIPs=getAs(*tns, depth+1, beenthere);
+      if(remoteIPs.empty()) {
        LOG<<prefix<<qname<<": Failed to get IP for NS "<<*tns<<", trying next if available"<<endl;
        continue;
       }
-      LOG<<prefix<<qname<<": Resolved '"+auth+"' NS "<<*tns<<" to "<<remoteIP<<", asking '"<<qname<<"|"<<qtype.getName()<<"'"<<endl;
-
+      remoteIPs_t::const_iterator remoteIP;
       bool doTCP=false;
-
-      if(s_throttle.shouldThrottle(d_now.tv_sec, remoteIP+"|"+qname+"|"+qtype.getName())) {
-       LOG<<prefix<<qname<<": query throttled "<<endl;
-       s_throttledqueries++;
-       d_throttledqueries++;
-       continue;
-      }
-      else {
-       s_outqueries++;
-       d_outqueries++;
-      TryTCP:
-       if(doTCP) {
-         s_tcpoutqueries++;
-         d_tcpoutqueries++;
+      for(remoteIP = remoteIPs.begin(); remoteIP != remoteIPs.end(); ++remoteIP) {
+       LOG<<prefix<<qname<<": Resolved '"+auth+"' NS "<<*tns<<" to "<<*remoteIP<<", asking '"<<qname<<"|"<<qtype.getName()<<"'"<<endl;
+       
+       if(s_throttle.shouldThrottle(d_now.tv_sec, *remoteIP+"|"+qname+"|"+qtype.getName())) {
+         LOG<<prefix<<qname<<": query throttled "<<endl;
+         s_throttledqueries++;
+         d_throttledqueries++;
+         continue;
        }
-
-       int ret=d_lwr.asyncresolve(remoteIP, qname.c_str(), qtype.getCode(), doTCP);    // <- we go out on the wire!
-       if(ret != 1) {
-         if(ret==0) {
-           LOG<<prefix<<qname<<": timeout resolving"<<endl;
-           d_timeouts++;
-           s_outgoingtimeouts++;
+       else {
+         s_outqueries++;
+         d_outqueries++;
+       TryTCP:
+         if(doTCP) {
+           s_tcpoutqueries++;
+           d_tcpoutqueries++;
          }
-         else
-           LOG<<prefix<<qname<<": error resolving"<<endl;
-
-         s_nsSpeeds[toLower(*tns)].submit(1000000, &d_now); // 1 sec
          
-         s_throttle.throttle(d_now.tv_sec, remoteIP+"|"+qname+"|"+qtype.getName(),20,5);
-         continue;
+         int ret=d_lwr.asyncresolve(*remoteIP, qname.c_str(), qtype.getCode(), doTCP);    // <- we go out on the wire!
+         if(ret != 1) {
+           if(ret==0) {
+             LOG<<prefix<<qname<<": timeout resolving"<<endl;
+             d_timeouts++;
+             s_outgoingtimeouts++;
+           }
+           else
+             LOG<<prefix<<qname<<": error resolving"<<endl;
+           
+           s_nsSpeeds[toLower(*tns)].submit(1000000, &d_now); // 1 sec
+         
+           s_throttle.throttle(d_now.tv_sec, *remoteIP+"|"+qname+"|"+qtype.getName(),20,5);
+           continue;
+         }
+         gettimeofday(&d_now, 0);
+         break; // it did work!
        }
-       gettimeofday(&d_now, 0);
       }
 
+      if(remoteIP == remoteIPs.end())  // we tried all IP addresses, none worked
+       continue; 
+
       result=d_lwr.result();
       
       if(d_lwr.d_tcbit) {
@@ -431,10 +444,10 @@ int SyncRes::doResolveAt(set<string> nameservers, string auth, const string &qna
 
       if(d_lwr.d_rcode==RCode::ServFail) {
        LOG<<prefix<<qname<<": "<<*tns<<" returned a ServFail, trying sibling NS"<<endl;
-       s_throttle.throttle(d_now.tv_sec,remoteIP+"|"+qname+"|"+qtype.getName(),60,3);
+       s_throttle.throttle(d_now.tv_sec,*remoteIP+"|"+qname+"|"+qtype.getName(),60,3);
        continue;
       }
-      LOG<<prefix<<qname<<": Got "<<result.size()<<" answers from "<<*tns<<" ("<<remoteIP<<"), rcode="<<d_lwr.d_rcode<<", in "<<d_lwr.d_usec/1000<<"ms"<<endl;
+      LOG<<prefix<<qname<<": Got "<<result.size()<<" answers from "<<*tns<<" ("<<*remoteIP<<"), rcode="<<d_lwr.d_rcode<<", in "<<d_lwr.d_usec/1000<<"ms"<<endl;
       s_nsSpeeds[toLower(*tns)].submit(d_lwr.d_usec, &d_now);
 
       map<string,set<DNSResourceRecord> > tcache;
@@ -565,7 +578,7 @@ int SyncRes::doResolveAt(set<string> nameservers, string auth, const string &qna
       }
       else {
        LOG<<prefix<<qname<<": status=NS "<<*tns<<" is lame for '"<<auth<<"', trying sibling NS"<<endl;
-       s_throttle.throttle(d_now.tv_sec, remoteIP+"|"+qname+"|"+qtype.getName(),60,0);
+       s_throttle.throttle(d_now.tv_sec, *remoteIP+"|"+qname+"|"+qtype.getName(),60,0);
       }
     }
   }

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 58e71f14042a888e384a448ad86c7cbac284ed87..7fd3fe3ce75f174e33d0b6d0a9d5f7a5b5701477 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -195,7 +195,7 @@ private:
 
   inline vector<string> shuffle(set<string> &nameservers, const string &prefix);
   bool moreSpecificThan(const string& a, const string &b);
-  string getA(const string &qname, int depth, set<GetBestNSAnswer>& beenthere);
+  vector<string> getAs(const string &qname, int depth, set<GetBestNSAnswer>& beenthere);
 
   SyncRes(const SyncRes&);
   SyncRes& operator=(const SyncRes&);