From: Mark Zealey <mark@markandruth.co.uk>
Date: Mon, 2 Dec 2013 08:18:42 +0000 (+0200)
Subject: There are certain situations in which getAuth() may want to be overridden by a
X-Git-Tag: rec-3.6.0-rc1~245^2~5
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c14bc34ac0ce511248e2daf3b8ac7ed91ffbcbbf;p=thirdparty%2Fpdns.git

There are certain situations in which getAuth() may want to be overridden by a
backend for performance reasons. The attached patch converts getAuth() into a
virtual backend function. The default is for no change in functionality. In my
tests this leads to very significant performance improvements under certain
situations

fixes PowerDNS/pdns#581
---

diff --git a/pdns/dnsbackend.cc b/pdns/dnsbackend.cc
index 2a5ef2d28a..1cc552bbf9 100644
--- a/pdns/dnsbackend.cc
+++ b/pdns/dnsbackend.cc
@@ -26,6 +26,7 @@
 #include "logger.hh"
 
 #include <sys/types.h>
+#include <pdns/packetcache.hh>
 #include "dnspacket.hh"
 #include "dns.hh"
 
@@ -43,6 +44,31 @@ bool DNSBackend::getRemote(DNSPacket *p, struct sockaddr *sa, Utility::socklen_t
   return true;
 }
 
+bool DNSBackend::getAuth(DNSPacket *p, SOAData *sd, const string &target, int *zoneId, const size_t best_match_len)
+{
+  bool found=false;
+  string subdomain(target);
+  do {
+    if( best_match_len >= subdomain.length() )
+      break;
+
+    if( this->getSOA( subdomain, *sd, p ) ) {
+      sd->qname = subdomain;
+      if(zoneId)
+        *zoneId = sd->domain_id;
+
+      if(p->qtype.getCode() == QType::DS && pdns_iequals(subdomain, target)) {
+        // Found authoritative zone but look for parent zone with 'DS' record.
+        found=true;
+      } else
+        return true;
+    }
+  }
+  while( chopOff( subdomain ) );   // 'www.powerdns.org' -> 'powerdns.org' -> 'org' -> ''
+
+  return found;
+}
+
 void DNSBackend::setArgPrefix(const string &prefix)
 {
   d_prefix=prefix;
@@ -310,3 +336,238 @@ bool DNSBackend::calculateSOASerial(const string& domain, const SOAData& sd, tim
 
     return true;
 }
+
+#if 0
+#define DEBUGLOG(msg) L<<Logger::Error<<msg<<endl
+#else
+#define DEBUGLOG(msg) do {} while(0)
+#endif
+
+/* String a is the query key, string b is the result from the database. Trys to
+ * be intelegent and only return matching sections (ie up to the last point at
+ * which there were .'s in both strings, however because the - character is
+ * legal and < . in ascii it does not gaurentee to return a section point.
+ */
+inline unsigned int compare_domains( const string &a, const string &b ) {
+    int aLen = a.length(), bLen = b.length(), n = 0, last_dot = 0;
+    const unsigned char *aPtr = (const unsigned char*)a.c_str(),
+            *bPtr = (const unsigned char*)b.c_str();
+
+    while( 1 ) {
+        if( n == aLen ) {
+            if( n == bLen || bPtr[n] <= '.' )
+                return n;
+            else
+                return last_dot;
+        }
+
+        if( n == bLen ) {
+            if( n == aLen || aPtr[n] <= '.' )
+                return n;
+            else
+                return last_dot;
+        }
+
+        if( aPtr[n] != bPtr[n] ) {
+            if( aPtr[n] <= '.' && bPtr[n] <= '.' )
+                return n;
+            else
+                return last_dot;
+        }
+
+        if( aPtr[n] == '.' )        // Therefore bPtr[n] == '.' as from above they are the same
+            last_dot = n;
+
+        n++;
+    }
+
+    // Should never get here
+    return 0;
+}
+
+/* This is a subclass of DNSBackend that, assuming you have your zones reversed
+ * and stored in an ordered fashion, will be able to look up SOA's much quicker
+ * than the DNSBackend code. The normal case for a SOA that exists is 1 backend
+ * query no matter how much the depth (although if there are sub-SOA's then
+ * this could require one or two more queries). The normal case for an SOA that
+ * does not exist is 2 or 3 queries depending on the system, although this will
+ * be reduced if the negative cache is active.
+ *
+ * The subclass MUST implement getAuthZone(string &reversed_zone_name) which,
+ * given a reversed zone name will return false if there was some sort of error,
+ * otherwise returns true and sets reversed_zone_name to be the exact entry
+ * found, otherwise the entry directly preceeding where it would be.
+ *
+ * The subclass MUST implement getAuthData( const string &rev_zone_name, SOAData *soa, int *zoneId )
+ * which is basically the same as getSOA() but is called with the reversed zone name
+ */
+enum {
+    GET_AUTH_NEG_DONTCACHE, // not found but don't cache this fact
+    GET_AUTH_NEG_CACHE,     // not found and negcache this
+    GET_AUTH_SUCCESS,       // entry found
+};
+
+#undef PC
+extern PacketCache PC;
+
+bool _add_to_negcache( const string &zone ) {
+    static int negqueryttl=::arg().asNum("negquery-cache-ttl");
+    // add the zone to the negative query cache and return false
+    if(negqueryttl) {
+        DEBUGLOG("Adding to neg qcache: " << zone);
+        PC.insert(zone, QType(QType::SOA), PacketCache::QUERYCACHE, "", negqueryttl, 0);
+    }
+    return false;
+}
+
+inline int DNSReversedBackend::_getAuth(DNSPacket *p, SOAData *soa, const string &inZone, int *zoneId, const string &querykey, const size_t best_match_len) {
+    static int negqueryttl=::arg().asNum("negquery-cache-ttl");
+
+    DEBUGLOG("SOA Query: " <<querykey);
+
+    /* Got a match from a previous backend that was longer than this - no need
+     * to continue. This is something of an optimization as we would hit the
+     * similar test below in any cases that this was hit, although we would run
+     * the risk of something being added to the neg-querycache that may
+     * interfear with future queries
+     */
+    if( best_match_len >= querykey.length() ) {
+        DEBUGLOG("Best match was better from a different client");
+        return GET_AUTH_NEG_DONTCACHE;
+    }
+
+    /* Look up in the negative querycache to see if we have already tried and
+     * failed to look up this zone */
+    if( negqueryttl ) {
+        string content;
+        bool ret = PC.getEntry( inZone, QType(QType::SOA), PacketCache::QUERYCACHE, content, 0 );
+        if( ret && content.empty() ) {
+            DEBUGLOG("Found in neg qcache: " << inZone << ":" << content << ":" << ret << ":");
+            return GET_AUTH_NEG_DONTCACHE;
+        }
+    }
+
+    /* Find the SOA entry on- or before- the position that we want in the b-tree */
+    string foundkey = querykey;
+    if( !getAuthZone( foundkey ) )
+        return GET_AUTH_NEG_CACHE;
+
+    unsigned int diff_point = compare_domains( querykey, foundkey );
+    DEBUGLOG("Queried: " << querykey << " and found record: " <<foundkey << " : diff point is: " << diff_point);
+
+    // Got a match from a previous backend that was longer than this - no need
+    // to continue.
+    if( best_match_len && best_match_len >= diff_point ) {
+        DEBUGLOG("Best match was better from a different client");
+        return GET_AUTH_NEG_DONTCACHE;
+    }
+
+    // Strings totally different
+    if( diff_point == 0 )
+        return GET_AUTH_NEG_CACHE;
+
+    /* If the strings are the same (ie diff_point == querykey.length()) then we
+     * have found the exact record.
+     *
+     * If the strings are the same up to the end of the key we pulled from the
+     * database, then we have found the true SOA for this zone. (the string we
+     * pulled from the database could be longer than the key we searched for,
+     * but if that is the case then it cannot be a sub-key because of the
+     * reliance to get the key at the requested position OR before)
+     *
+     * Otherwise, the strings are different up to a certain point. In this
+     * case, we need to retry the query as we may have the case of a subdomain
+     * in the database eg a.b.com and b.com SOA records. If we then query for
+     * www.b.com we will hit the a.b.com SOA record so we want to trim back to
+     * the . before the difference and retry the query (ie b.com). Note that
+     * because some legal dns characters come *before* the . in ascii if we try
+     * searching for www.a.com our db query may return www-a.com in which case
+     * we retry the search from the point at which the strings differ (ie
+     * a.com)
+     *
+     * To speed up future decisions for subdomains, if the negative cache is
+     * available we will make a note there if the subdomain we queried for does
+     * not exist.
+     */
+    if( diff_point != querykey.length()
+            && diff_point != foundkey.length() ) {
+        // XXX If we found some way of getting the exact domain without having
+        // to try this (ie walk the database btree up until the point where it
+        // differs), we could have a 60% performance improvement in some
+        // situations.
+
+        string shortzone = inZone.substr( inZone.length() - diff_point, string::npos );
+        string shortquerykey = querykey.substr( 0, diff_point );
+        DEBUGLOG("Retrying for " << shortzone << " querykey: " << shortquerykey);
+
+        int ret = _getAuth( p, soa, shortzone, zoneId, shortquerykey, best_match_len );
+
+        if( ret == GET_AUTH_NEG_CACHE )
+            _add_to_negcache( shortzone );
+
+        return ret;
+    }
+
+    // Found record successfully now, fill in the data.
+    if( getAuthData( *soa, p ) ) {
+        /* all the keys are reversed. rather than reversing them again it is
+         * presumably quicker to just substring the zone down to size */
+        soa->qname = inZone.substr( inZone.length() - foundkey.length(), string::npos );
+        if(zoneId)
+            *zoneId = soa->domain_id;
+
+        DEBUGLOG("Successfully got record: " <<foundkey << " : " << querykey.substr( 0, foundkey.length() ) << " : " << soa->qname);
+
+        return GET_AUTH_SUCCESS;
+    }
+
+    return GET_AUTH_NEG_CACHE;
+}
+
+bool DNSReversedBackend::getAuth(DNSPacket *p, SOAData *soa, const string &inZone, int *zoneId, const size_t best_match_len) {
+    // Reverse the lowercased query string
+    string zone = toLower(inZone);
+    string querykey( zone.rbegin(), zone.rend() );
+
+    int ret = _getAuth( p, soa, inZone, zoneId, querykey, best_match_len );
+
+    /* If this is disabled then we would just cache the tree structure not the
+     * leaves which should give the best performance and a nice small negcache
+     * size
+     */
+    if( ret == GET_AUTH_NEG_CACHE )
+        _add_to_negcache( inZone );
+
+    return ret == GET_AUTH_SUCCESS;
+}
+
+/* getAuthData() is very similar to getSOA() so implement a default getSOA
+ * based on that. This will only be called very occasionally for example during
+ * an AXFR */
+bool DNSReversedBackend::_getSOA(const string &querykey, SOAData &soa, DNSPacket *p)
+{
+    string searchkey( querykey );
+
+    if( !getAuthZone( searchkey ) )
+        return false;
+
+    DEBUGLOG("search key " << searchkey << " query key " << querykey);
+
+    if( querykey.compare( searchkey ) != 0 )
+        return false;
+
+    return getAuthData( soa, p );
+}
+
+bool DNSReversedBackend::getSOA(const string &inZone, SOAData &soa, DNSPacket *p)
+{
+    // prepare the query string
+    string zone = toLower( inZone );
+    string querykey( zone.rbegin(), zone.rend() );
+
+    if( !_getSOA( querykey, soa, p ) )
+        return false;
+
+    soa.qname = inZone;
+    return true;
+}
diff --git a/pdns/dnsbackend.hh b/pdns/dnsbackend.hh
index 906ce81cb3..c41342c19f 100644
--- a/pdns/dnsbackend.hh
+++ b/pdns/dnsbackend.hh
@@ -89,7 +89,6 @@ struct TSIGKey {
 
 class DNSPacket;
 
-
 //! This virtual base class defines the interface for backends for the ahudns. 
 /** To create a backend, inherit from this class and implement functions for all virtual methods.
     Methods should not throw an exception if they are sure they did not find the requested data. However,
@@ -108,7 +107,6 @@ public:
   virtual void lookup(const QType &qtype, const string &qdomain, DNSPacket *pkt_p=0, int zoneId=-1)=0; 
   virtual bool get(DNSResourceRecord &)=0; //!< retrieves one DNSResource record, returns false if no more were available
 
-
   //! Initiates a list of the specified domain
   /** Once initiated, DNSResourceRecord objects can be retrieved using get(). Should return false
       if the backend does not consider itself responsible for the id passed.
@@ -140,6 +138,9 @@ public:
 
   virtual void getAllDomains(vector<DomainInfo> *domains) { }
 
+  /** Determines if we are authoritative for a zone, and at what level */
+  virtual bool getAuth(DNSPacket *p, SOAData *sd, const string &target, int *zoneId, const size_t best_match_len);
+
   struct KeyData {
     unsigned int id;
     unsigned int flags;
@@ -330,6 +331,37 @@ private:
   string d_prefix;
 };
 
+class DNSReversedBackend : public DNSBackend {
+    public:
+        /* Given rev_zone (the reversed name of the zone we are looking for the
+         * SOA record for), return the equivelent of
+         *     SELECT name
+         *     FROM soa_records
+         *     WHERE name <= rev_zone
+         *     ORDER BY name DESC
+         *
+         * ie we want either an exact hit on the record, or the immediately
+         * preceeding record when sorted lexographically.
+         *
+         * Return true if something has been found, false if not
+         */
+        virtual bool getAuthZone( string &rev_zone ) { return false; };  // Must be overridden
+
+        /* Once the record has been found, this will be called to get the data
+         * associated with the record so the backend can set up soa and zoneId
+         * respectively.  soa->qname does not need to be set. Return false if
+         * there is a problem getting the data.
+         * */
+        virtual bool getAuthData( SOAData &soa, DNSPacket *p=0) { return false; };  // Must be overridden
+
+        bool getAuth(DNSPacket *p, SOAData *soa, const string &inZone, int *zoneId, const size_t best_match_len);
+        inline int _getAuth(DNSPacket *p, SOAData *soa, const string &inZone, int *zoneId, const string &querykey, const size_t best_match_len);
+
+        /* Only called for stuff like signing or AXFR transfers */
+        bool _getSOA(const string &rev_zone, SOAData &soa, DNSPacket *p);
+        virtual bool getSOA(const string &inZone, SOAData &soa, DNSPacket *p);
+};
+
 class BackendFactory
 {
 public:
diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc
index 2ee0bb7509..7143a071b3 100644
--- a/pdns/packethandler.cc
+++ b/pdns/packethandler.cc
@@ -135,7 +135,7 @@ int PacketHandler::findMboxFW(DNSPacket *p, DNSPacket *r, string &target)
 
   SOAData sd;
   int zoneId;
-  if(!getAuth(p, &sd, target, &zoneId))
+  if(!B.getAuth(p, &sd, target, &zoneId))
     return false;
 
   B.lookup(QType(QType::MBOXFW),string("%@")+target,p, zoneId);
@@ -313,7 +313,6 @@ int PacketHandler::doChaosRequest(DNSPacket *p, DNSPacket *r, string &target)
   return 0;
 }
 
-
 /** Determines if we are authoritative for a zone, and at what level */
 bool PacketHandler::getAuth(DNSPacket *p, SOAData *sd, const string &target, int *zoneId)
 {
@@ -885,7 +884,7 @@ void PacketHandler::synthesiseRRSIGs(DNSPacket* p, DNSPacket* r)
 
   SOAData sd;
   sd.db=(DNSBackend *)-1; // force uncached answer
-  getAuth(p, &sd, p->qdomain, 0);
+  B.getAuth(p, &sd, p->qdomain, 0);
 
   bool narrow;
   NSEC3PARAMRecordContent ns3pr;
@@ -1264,7 +1263,7 @@ DNSPacket *PacketHandler::questionOrRecurse(DNSPacket *p, bool *shouldRecurse)
       return r;
     }
     
-    if(!getAuth(p, &sd, target, 0)) {
+    if(!B.getAuth(p, &sd, target, 0)) {
       DLOG(L<<Logger::Error<<"We have no authority over zone '"<<target<<"'"<<endl);
       if(r->d.ra) {
         DLOG(L<<Logger::Error<<"Recursion is available for this remote, doing that"<<endl);
diff --git a/pdns/packethandler.hh b/pdns/packethandler.hh
index e48ad457ae..c182ef31ff 100644
--- a/pdns/packethandler.hh
+++ b/pdns/packethandler.hh
@@ -76,7 +76,6 @@ private:
   int doChaosRequest(DNSPacket *p, DNSPacket *r, string &target);
   bool addDNSKEY(DNSPacket *p, DNSPacket *r, const SOAData& sd);
   bool addNSEC3PARAM(DNSPacket *p, DNSPacket *r, const SOAData& sd);
-  bool getAuth(DNSPacket *p, SOAData *sd, const string &target, int *zoneId);
   bool getTLDAuth(DNSPacket *p, SOAData *sd, const string &target, int *zoneId);
   int doAdditionalProcessingAndDropAA(DNSPacket *p, DNSPacket *r, const SOAData& sd);
   bool doDNSSECProcessing(DNSPacket* p, DNSPacket *r);
diff --git a/pdns/ueberbackend.cc b/pdns/ueberbackend.cc
index 8fdebc89e7..0f675e10b2 100644
--- a/pdns/ueberbackend.cc
+++ b/pdns/ueberbackend.cc
@@ -300,6 +300,84 @@ void UeberBackend::check_op_requests()
     cur_op_request = NONE;
 }
 
+bool UeberBackend::getAuth(DNSPacket *p, SOAData *sd, const string &target, int *zoneId)
+{
+  size_t best_match_len = 0;
+  bool from_cache = false;  // Was this result fetched from the cache?
+
+  check_op_requests();
+
+  // If not special case of caching explicitly disabled (sd->db = -1), first
+  // find the best match from the cache. If DS then we need to find parent so
+  // dont bother with caching as it confuses matters.
+  if( sd->db != (DNSBackend *)-1 && d_cache_ttl && p->qtype != QType::DS ) {
+      string subdomain(target);
+      int cstat, loops = 0;
+      do {
+        d_question.qtype = QType::SOA;
+        d_question.qname = subdomain;
+        d_question.zoneId = -1;
+
+        cstat = cacheHas(d_question,d_answers);
+
+        if(cstat==1 && !d_answers.empty()) {
+          fillSOAData(d_answers[0].content,*sd);
+          sd->domain_id = d_answers[0].domain_id;
+          sd->ttl = d_answers[0].ttl;
+          sd->db = 0;
+          sd->qname = subdomain;
+          //L<<Logger::Error<<"Best cache match: " << sd->qname << " itteration " << loops <<endl;
+
+          // Found first time round this must be the best match
+          if( loops == 0 )
+            return true;
+
+          from_cache = true;
+          best_match_len = sd->qname.length();
+
+          break;
+        }
+        loops++;
+      }
+      while( chopOff( subdomain ) );   // 'www.powerdns.org' -> 'powerdns.org' -> 'org' -> ''
+  }
+
+  for(vector<DNSBackend *>::const_iterator i=backends.begin(); i!=backends.end();++i)
+    if((*i)->getAuth(p, sd, target, zoneId, best_match_len)) {
+        best_match_len = sd->qname.length();
+        from_cache = false;
+
+        // Shortcut for the case that we got a direct hit - no need to go
+        // through the other backends then.
+        if( best_match_len == target.length() )
+            goto auth_found;
+    }
+
+  if( best_match_len == 0 )
+      return false;
+
+auth_found:
+    // Insert into cache. Don't cache if the query was a DS
+    if( d_cache_ttl && ! from_cache && p->qtype != QType::DS ) {
+        //L<<Logger::Error<<"Saving auth cache for " << sd->qname <<endl;
+        d_question.qtype = QType::SOA;
+        d_question.qname = sd->qname;
+        d_question.zoneId = -1;
+
+        DNSResourceRecord rr;
+        rr.qname = sd->qname;
+        rr.qtype = QType::SOA;
+        rr.content = serializeSOAData(*sd);
+        rr.ttl = sd->ttl;
+        rr.domain_id = sd->domain_id;
+        vector<DNSResourceRecord> rrs;
+        rrs.push_back(rr);
+        addCache(d_question, rrs);
+    }
+
+    return true;
+}
+
 /** special trick - if sd.db is set to -1, the cache is ignored */
 bool UeberBackend::getSOA(const string &domain, SOAData &sd, DNSPacket *p)
 {
diff --git a/pdns/ueberbackend.hh b/pdns/ueberbackend.hh
index 702887ad9d..b4ea73d811 100644
--- a/pdns/ueberbackend.hh
+++ b/pdns/ueberbackend.hh
@@ -115,6 +115,7 @@ public:
 
   void lookup(const QType &, const string &qdomain, DNSPacket *pkt_p=0,  int zoneId=-1);
 
+  bool getAuth(DNSPacket *p, SOAData *sd, const string &target, int *zoneId);
   bool getSOA(const string &domain, SOAData &sd, DNSPacket *p=0);
   bool list(const string &target, int domain_id);
   bool get(DNSResourceRecord &r);
diff --git a/regression-tests/bind-add-zone/expected_result.bind b/regression-tests/bind-add-zone/expected_result.bind
index e531b86ce6..cfb53189d4 100644
--- a/regression-tests/bind-add-zone/expected_result.bind
+++ b/regression-tests/bind-add-zone/expected_result.bind
@@ -30,7 +30,7 @@ Reply to question for qname='ns1.addzone.com.', qtype=A
 Rcode: 0, RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0
 Reply to question for qname='ns1.test.com.', qtype=A
 Loaded zone addzone.com from addzone.com
-1
+0
 Already loaded
 Rcode: 2, RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0
 Reply to question for qname='ns1.addzone.com.', qtype=A