From: Bert Hubert <bert.hubert@netherlabs.nl>
Date: Sat, 4 Mar 2006 11:42:37 +0000 (+0000)
Subject: The PowerDNS recursor did not use to try all IP addresses of a nameserver!
X-Git-Tag: pdns-2.9.20~18
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bfea0d0b4d7d50d442be9084ef0779051fd3f808;p=thirdparty%2Fpdns.git

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 c29d5d4f94..edb50935fb 100644
--- 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 58e71f1404..7fd3fe3ce7 100644
--- 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&);