remove recursion from packetcache

diff --git a/pdns/common_startup.cc b/pdns/common_startup.cc
index 4972f217a856698b345e9a523961e0bb607111e1..4e4839e8aff337ac952cdb65506abb34c4ae2646 100644 (file)
--- a/pdns/common_startup.cc
+++ b/pdns/common_startup.cc
@@ -144,7 +144,6 @@ void declareArguments()
   ::arg().set("carbon-interval", "Number of seconds between carbon (graphite) updates")="30";
 
   ::arg().set("cache-ttl","Seconds to store packets in the PacketCache")="20";
-  ::arg().set("recursive-cache-ttl","Seconds to store packets for recursive queries in the PacketCache")="10";
   ::arg().set("negquery-cache-ttl","Seconds to store negative query results in the QueryCache")="60";
   ::arg().set("query-cache-ttl","Seconds to store query results in the QueryCache")="20";
   ::arg().set("soa-minimum-ttl","Default SOA minimum ttl")="3600";
@@ -410,10 +409,8 @@ void *qthread(void *number)
 
     if((P->d.opcode != Opcode::Notify && P->d.opcode != Opcode::Update) && P->couldBeCached()) {
       bool haveSomething = false;
-      if (doRecursion && P->d.rd && DP->recurseFor(P))
-        haveSomething=PC.get(P, &cached, true); // does the PacketCache recognize this ruestion (recursive)?
-      if (!haveSomething)
-        haveSomething=PC.get(P, &cached, false); // does the PacketCache recognize this question?
+      if (!P->d.rd || !DP->recurseFor(P))
+        haveSomething=PC.get(P, &cached); // does the PacketCache recognize this question?
       if (haveSomething) {
         if(logDNSQueries)
           L<<"packetcache HIT"<<endl;

diff --git a/pdns/dynhandler.cc b/pdns/dynhandler.cc
index 71aa3886e44934a59f2731a047931a7fe1caca83..de792b4a92667efe16973878efbd0158a2548e38 100644 (file)
--- a/pdns/dynhandler.cc
+++ b/pdns/dynhandler.cc
@@ -159,10 +159,8 @@ string DLCCHandler(const vector<string>&parts, Utility::pid_t ppid)
       os<<"negative queries: ";
     else if(i->first=='Q')
       os<<"queries: ";
-    else if(i->first=='n')
-      os<<"non-recursive packets: ";
-    else if(i->first=='r')
-      os<<"recursive packets: ";
+    else if(i->first=='p')
+      os<<"packets: ";
     else 
       os<<"unknown: ";
 

diff --git a/pdns/packetcache.cc b/pdns/packetcache.cc
index 9a9283ad64657cac9f64aefa6de88a93d95caae6..b1b5f583bfe656efa12a0d1584217e5e1e8b4cb0 100644 (file)
--- a/pdns/packetcache.cc
+++ b/pdns/packetcache.cc
@@ -43,7 +43,6 @@ PacketCache::PacketCache()
   }
 
   d_ttl=-1;
-  d_recursivettl=-1;
 
   d_lastclean=time(0);
   d_cleanskipped=false;
@@ -56,7 +55,6 @@ PacketCache::PacketCache()
   d_statnumhit=S.getPointer("packetcache-hit");
   d_statnummiss=S.getPointer("packetcache-miss");
   d_statnumentries=S.getPointer("packetcache-size");
-  d_doRecursion=false;
 }
 
 PacketCache::~PacketCache()
@@ -73,7 +71,7 @@ PacketCache::~PacketCache()
 
 
 
-int PacketCache::get(DNSPacket *p, DNSPacket *cached, bool recursive)
+int PacketCache::get(DNSPacket *p, DNSPacket *cached)
 {
   extern StatBag S;
 
@@ -82,23 +80,14 @@ int PacketCache::get(DNSPacket *p, DNSPacket *cached, bool recursive)
 
   cleanupIfNeeded();
 
-  if(d_doRecursion && p->d.rd) { // wants recursion
-    if(!d_recursivettl) {
-      (*d_statnummiss)++;
-      return 0;
-    }
-  }
-  else { // does not
-    if(!d_ttl) {
-      (*d_statnummiss)++;
-      return 0;
-    }
+  if(!d_ttl) {
+    (*d_statnummiss)++;
+    return 0;
   }
     
   if(ntohs(p->d.qdcount)!=1) // we get confused by packets with more than one question
     return 0;
 
-  unsigned int age=0;
   string value;
   bool haveSomething;
   {
@@ -110,12 +99,10 @@ int PacketCache::get(DNSPacket *p, DNSPacket *cached, bool recursive)
     }
 
     uint16_t maxReplyLen = p->d_tcp ? 0xffff : p->getMaxReplyLen();
-    haveSomething=getEntryLocked(p->qdomain, p->qtype, PacketCache::PACKETCACHE, value, -1, recursive, maxReplyLen, p->d_dnssecOk, p->hasEDNS(), &age);
+    haveSomething=getEntryLocked(p->qdomain, p->qtype, PacketCache::PACKETCACHE, value, -1, maxReplyLen, p->d_dnssecOk, p->hasEDNS());
   }
   if(haveSomething) {
     (*d_statnumhit)++;
-    if (recursive)
-      ageDNSPacket(value, age);
     if(cached->noparse(value.c_str(), value.size()) < 0)
       return 0;
     cached->spoofQuestion(p); // for correct case
@@ -124,7 +111,7 @@ int PacketCache::get(DNSPacket *p, DNSPacket *cached, bool recursive)
     return 1;
   }
 
-  //  cerr<<"Packet cache miss for '"<<p->qdomain<<"', merits: "<<packetMeritsRecursion<<endl;
+  //  cerr<<"Packet cache miss for '"<<p->qdomain<<"'"<<endl;
   (*d_statnummiss)++;
   return 0; // bummer
 }
@@ -132,13 +119,10 @@ int PacketCache::get(DNSPacket *p, DNSPacket *cached, bool recursive)
 void PacketCache::getTTLS()
 {
   d_ttl=::arg().asNum("cache-ttl");
-  d_recursivettl=::arg().asNum("recursive-cache-ttl");
-
-  d_doRecursion=::arg().mustDo("recursor"); 
 }
 
 
-void PacketCache::insert(DNSPacket *q, DNSPacket *r, bool recursive, unsigned int maxttl)
+void PacketCache::insert(DNSPacket *q, DNSPacket *r, unsigned int maxttl)
 {
   if(d_ttl < 0)
     getTTLS();
@@ -151,22 +135,16 @@ void PacketCache::insert(DNSPacket *q, DNSPacket *r, bool recursive, unsigned in
     return;
 
   uint16_t maxReplyLen = q->d_tcp ? 0xffff : q->getMaxReplyLen();
-  unsigned int ourttl = recursive ? d_recursivettl : d_ttl;
-  if(!recursive) {
-    if(maxttl<ourttl)
-      ourttl=maxttl;
-  } else {
-    unsigned int minttl = r->getMinTTL();
-    if(minttl<ourttl)
-      ourttl=minttl;
-  }
-  insert(q->qdomain, q->qtype, PacketCache::PACKETCACHE, r->getString(), ourttl, -1, recursive,
+  unsigned int ourttl = d_ttl;
+  if(maxttl<ourttl)
+    ourttl=maxttl;
+  insert(q->qdomain, q->qtype, PacketCache::PACKETCACHE, r->getString(), ourttl, -1,
     maxReplyLen, q->d_dnssecOk, q->hasEDNS());
 }
 
-// universal key appears to be: qname, qtype, kind (packet, query cache), optionally zoneid, meritsRecursion
+// universal key appears to be: qname, qtype, kind (packet, query cache), optionally zoneid
 void PacketCache::insert(const DNSName &qname, const QType& qtype, CacheEntryType cet, const string& value, unsigned int ttl, int zoneID, 
-  bool meritsRecursion, unsigned int maxReplyLen, bool dnssecOk, bool EDNS)
+  unsigned int maxReplyLen, bool dnssecOk, bool EDNS)
 {
   cleanupIfNeeded();
 
@@ -181,7 +159,6 @@ void PacketCache::insert(const DNSName &qname, const QType& qtype, CacheEntryTyp
   val.qtype=qtype.getCode();
   val.value=value;
   val.ctype=cet;
-  val.meritsRecursion=meritsRecursion;
   val.maxReplyLen = maxReplyLen;
   val.dnssecOk = dnssecOk;
   val.zoneID = zoneID;
@@ -219,7 +196,6 @@ void PacketCache::insert(const DNSName &qname, const QType& qtype, CacheEntryTyp
   val.qtype=qtype.getCode();
   val.drs=value;
   val.ctype=cet;
-  val.meritsRecursion=false;
   val.maxReplyLen = 0;
   val.dnssecOk = false;
   val.zoneID = zoneID;
@@ -319,13 +295,13 @@ bool PacketCache::getEntry(const DNSName &qname, const QType& qtype, CacheEntryT
 }
 
 
-bool PacketCache::getEntryLocked(const DNSName &qname, const QType& qtype, CacheEntryType cet, string& value, int zoneID, bool meritsRecursion,
-  unsigned int maxReplyLen, bool dnssecOK, bool hasEDNS, unsigned int *age)
+bool PacketCache::getEntryLocked(const DNSName &qname, const QType& qtype, CacheEntryType cet, string& value, int zoneID,
+  unsigned int maxReplyLen, bool dnssecOK, bool hasEDNS)
 {
   uint16_t qt = qtype.getCode();
   //cerr<<"Lookup for maxReplyLen: "<<maxReplyLen<<endl;
   auto& mc=getMap(qname);
-  //  cmap_t::const_iterator i=mc.d_map.find(tie(qname, qt, cet, zoneID, meritsRecursion, maxReplyLen, dnssecOK, hasEDNS, *age));
+  //  cmap_t::const_iterator i=mc.d_map.find(tie(qname, qt, cet, zoneID, maxReplyLen, dnssecOK, hasEDNS));
 
   auto& idx = boost::multi_index::get<UnorderedNameTag>(mc.d_map);
   auto range=idx.equal_range(tie(qname, qt, cet, zoneID));
@@ -334,10 +310,8 @@ bool PacketCache::getEntryLocked(const DNSName &qname, const QType& qtype, Cache
     return false;
   time_t now=time(0);
   for(auto iter = range.first ; iter != range.second; ++iter) {
-    if(meritsRecursion == iter->meritsRecursion && maxReplyLen == iter->maxReplyLen && dnssecOK == iter->dnssecOk && hasEDNS == iter->hasEDNS ) {
+    if(maxReplyLen == iter->maxReplyLen && dnssecOK == iter->dnssecOk && hasEDNS == iter->hasEDNS ) {
       if(iter->ttd > now) {
-        if (age)
-          *age = now - iter->created;
         value = iter->value;
         return true;
       }
@@ -368,17 +342,14 @@ bool PacketCache::getEntryLocked(const DNSName &qname, const QType& qtype, Cache
 
 map<char,int> PacketCache::getCounts()
 {
-  int recursivePackets=0, nonRecursivePackets=0, queryCacheEntries=0, negQueryCacheEntries=0;
+  int packets=0, queryCacheEntries=0, negQueryCacheEntries=0;
 
   for(auto& mc : d_maps) {
     ReadLock l(&mc.d_mut);
     
     for(cmap_t::const_iterator iter = mc.d_map.begin() ; iter != mc.d_map.end(); ++iter) {
       if(iter->ctype == PACKETCACHE)
-       if(iter->meritsRecursion)
-         recursivePackets++;
-       else
-         nonRecursivePackets++;
+         packets++;
       else if(iter->ctype == QUERYCACHE) {
        if(iter->value.empty())
          negQueryCacheEntries++;
@@ -391,8 +362,7 @@ map<char,int> PacketCache::getCounts()
 
   ret['!']=negQueryCacheEntries;
   ret['Q']=queryCacheEntries;
-  ret['n']=nonRecursivePackets;
-  ret['r']=recursivePackets;
+  ret['p']=packets;
   return ret;
 }
 

diff --git a/pdns/packetcache.hh b/pdns/packetcache.hh
index 8949b6f0d423c5f88cd666a906d2094ab8b7135e..357acedd1aa2f2ea0f5dc38f6fc1c0d9bd1e4be1 100644 (file)
--- a/pdns/packetcache.hh
+++ b/pdns/packetcache.hh
@@ -55,16 +55,16 @@ public:
   ~PacketCache();
   enum CacheEntryType { PACKETCACHE, QUERYCACHE};
 
-  void insert(DNSPacket *q, DNSPacket *r, bool recursive, unsigned int maxttl=UINT_MAX);  //!< We copy the contents of *p into our cache. Do not needlessly call this to insert questions already in the cache as it wastes resources
+  void insert(DNSPacket *q, DNSPacket *r, unsigned int maxttl=UINT_MAX);  //!< We copy the contents of *p into our cache. Do not needlessly call this to insert questions already in the cache as it wastes resources
 
-  void insert(const DNSName &qname, const QType& qtype, CacheEntryType cet, const string& value, unsigned int ttl, int zoneID=-1, bool meritsRecursion=false,
+  void insert(const DNSName &qname, const QType& qtype, CacheEntryType cet, const string& value, unsigned int ttl, int zoneID=-1,
     unsigned int maxReplyLen=512, bool dnssecOk=false, bool EDNS=false);
 
   void insert(const DNSName &qname, const QType& qtype, CacheEntryType cet, const vector<DNSZoneRecord>& content, unsigned int ttl, int zoneID=-1);
 
-  int get(DNSPacket *p, DNSPacket *q, bool recursive); //!< We return a dynamically allocated copy out of our cache. You need to delete it. You also need to spoof in the right ID with the DNSPacket.spoofID() method.
+  int get(DNSPacket *p, DNSPacket *q); //!< We return a dynamically allocated copy out of our cache. You need to delete it. You also need to spoof in the right ID with the DNSPacket.spoofID() method.
   bool getEntry(const DNSName &qname, const QType& qtype, CacheEntryType cet, string& entry, int zoneID=-1,
-    bool meritsRecursion=false, unsigned int maxReplyLen=512, bool dnssecOk=false, bool hasEDNS=false, unsigned int *age=0);
+    unsigned int maxReplyLen=512, bool dnssecOk=false, bool hasEDNS=false);
   bool getEntry(const DNSName &qname, const QType& qtype, CacheEntryType cet, vector<DNSZoneRecord>& entry, int zoneID=-1);
   
 
@@ -78,13 +78,13 @@ public:
   map<char,int> getCounts();
 private:
   bool getEntryLocked(const DNSName &content, const QType& qtype, CacheEntryType cet, string& entry, int zoneID=-1,
-    bool meritsRecursion=false, unsigned int maxReplyLen=512, bool dnssecOk=false, bool hasEDNS=false, unsigned int *age=0);
+    unsigned int maxReplyLen=512, bool dnssecOk=false, bool hasEDNS=false);
   bool getEntryLocked(const DNSName &content, const QType& qtype, CacheEntryType cet, vector<DNSZoneRecord>& entry, int zoneID=-1);
 
 
   struct CacheEntry
   {
-    CacheEntry() { qtype = ctype = 0; zoneID = -1; meritsRecursion=false; dnssecOk=false; hasEDNS=false; created=0; ttd=0; maxReplyLen=512;}
+    CacheEntry() { qtype = ctype = 0; zoneID = -1; dnssecOk=false; hasEDNS=false; created=0; ttd=0; maxReplyLen=512;}
 
     DNSName qname;
     string value;
@@ -97,7 +97,6 @@ private:
     int zoneID;
     unsigned int maxReplyLen;
 
-    bool meritsRecursion;
     bool dnssecOk;
     bool hasEDNS;
   };
@@ -116,12 +115,11 @@ private:
                         member<CacheEntry,uint16_t,&CacheEntry::qtype>,
                         member<CacheEntry,uint16_t, &CacheEntry::ctype>,
                         member<CacheEntry,int, &CacheEntry::zoneID>,
-                        member<CacheEntry,bool, &CacheEntry::meritsRecursion>,
                         member<CacheEntry,unsigned int, &CacheEntry::maxReplyLen>,
                         member<CacheEntry,bool, &CacheEntry::dnssecOk>,
                         member<CacheEntry,bool, &CacheEntry::hasEDNS>
                         >,
-                      composite_key_compare<CanonDNSNameCompare, std::less<uint16_t>, std::less<uint16_t>, std::less<int>, std::less<bool>, 
+                      composite_key_compare<CanonDNSNameCompare, std::less<uint16_t>, std::less<uint16_t>, std::less<int>,
                           std::less<unsigned int>, std::less<bool>, std::less<bool> >
                        >,
       hashed_non_unique<tag<UnorderedNameTag>, composite_key<CacheEntry,
@@ -156,8 +154,6 @@ private:
   AtomicCounter *d_statnumentries;
 
   int d_ttl;
-  int d_recursivettl;
-  bool d_doRecursion;
 
   static const unsigned int s_mincleaninterval=1000, s_maxcleaninterval=300000;
 };

diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc
index 055aed6869b91fae02dbb3a738b16149ae878d91..254a564a5e8d91c17892d9736dacc62b4a277db4 100644 (file)
--- a/pdns/packethandler.cc
+++ b/pdns/packethandler.cc
@@ -1507,7 +1507,7 @@ DNSPacket *PacketHandler::questionOrRecurse(DNSPacket *p, bool *shouldRecurse)
       
     r->wrapup(); // needed for inserting in cache
     if(!noCache)
-      PC.insert(p, r, false, r->getMinTTL()); // in the packet cache
+      PC.insert(p, r, r->getMinTTL()); // in the packet cache
   }
   catch(DBException &e) {
     L<<Logger::Error<<"Backend reported condition which prevented lookup ("+e.reason+") sending out servfail"<<endl;

diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index 18549fe639af2333a508ca7d77f5bf5d93c3b268..98ea991aeef19fe1e82035ff6e3d1ca58a65f94b 100644 (file)
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -108,8 +108,6 @@ void loadMainConfig(const std::string& configdir)
   S.declare("query-cache-hit","Number of hits on the query cache");
   S.declare("query-cache-miss","Number of misses on the query cache");
   ::arg().set("max-cache-entries", "Maximum number of cache entries")="1000000";
-  ::arg().set("recursor","If recursion is desired, IP address of a recursing nameserver")="no"; 
-  ::arg().set("recursive-cache-ttl","Seconds to store packets for recursive queries in the PacketCache")="10";
   ::arg().set("cache-ttl","Seconds to store packets in the PacketCache")="20";              
   ::arg().set("negquery-cache-ttl","Seconds to store negative query results in the QueryCache")="60";
   ::arg().set("query-cache-ttl","Seconds to store query results in the QueryCache")="20";              

diff --git a/pdns/tcpreceiver.cc b/pdns/tcpreceiver.cc
index e29de4aaffaaf906ed9bd34db3f2ae66d44058fd..7eb16f433600157df6856ea639123da70ce21b0c 100644 (file)
--- a/pdns/tcpreceiver.cc
+++ b/pdns/tcpreceiver.cc
@@ -399,7 +399,7 @@ void *TCPNameserver::doConnection(void *data)
       }
 
 
-      if(!packet->d.rd && packet->couldBeCached() && PC.get(packet.get(), cached.get(), false)) { // short circuit - does the PacketCache recognize this question?
+      if(!packet->d.rd && packet->couldBeCached() && PC.get(packet.get(), cached.get())) { // short circuit - does the PacketCache recognize this question?
         if(logDNSQueries)
           L<<"packetcache HIT"<<endl;
         cached->setRemote(&packet->d_remote);

diff --git a/pdns/test-packetcache_cc.cc b/pdns/test-packetcache_cc.cc
index e6ceb7ddcf06ccd107746fda654f7fa0ac2dfafb..31c6304fa270eab8cb69aef0b1ba754a3b7290e1 100644 (file)
--- a/pdns/test-packetcache_cc.cc
+++ b/pdns/test-packetcache_cc.cc
@@ -21,10 +21,8 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheSimple) {
 
   ::arg().set("max-cache-entries", "Maximum number of cache entries")="1000000";
   ::arg().set("cache-ttl","Seconds to store packets in the PacketCache")="20";
-  ::arg().set("recursive-cache-ttl","Seconds to store packets for recursive queries in the PacketCache")="10";
   ::arg().set("negquery-cache-ttl","Seconds to store negative query results in the QueryCache")="60";
   ::arg().set("query-cache-ttl","Seconds to store query results in the QueryCache")="20";
-  ::arg().set("recursor","If recursion is desired, IP address of a recursing nameserver")="no"; 
 
   S.declare("deferred-cache-inserts","Amount of cache inserts that were deferred because of maintenance");
   S.declare("deferred-cache-lookup","Amount of cache lookups that were deferred because of maintenance");
@@ -208,46 +206,38 @@ BOOST_AUTO_TEST_CASE(test_PacketCachePacket) {
 
     r.parse((char*)&pak[0], pak.size());
 
-    PC.insert(&q, &r, false, 3600);
+    PC.insert(&q, &r, 3600);
 
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 1);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 1);
     BOOST_CHECK_EQUAL(r2.qdomain, r.qdomain);
 
     PC.purge("www.powerdns.com");
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 0);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 0);
 
-    PC.insert(&q, &r, false, 3600);
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 1);
+    PC.insert(&q, &r, 3600);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 1);
     BOOST_CHECK_EQUAL(r2.qdomain, r.qdomain);
     PC.purge("com$");
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 0);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 0);
 
-    PC.insert(&q, &r, false, 3600);
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 1);
+    PC.insert(&q, &r, 3600);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 1);
     BOOST_CHECK_EQUAL(r2.qdomain, r.qdomain);
     PC.purge("powerdns.com$");
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 0);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 0);
 
-    PC.insert(&q, &r, false, 3600);
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 1);
+    PC.insert(&q, &r, 3600);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 1);
     BOOST_CHECK_EQUAL(r2.qdomain, r.qdomain);
     PC.purge("www.powerdns.com$");
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 0);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 0);
 
-    PC.insert(&q, &r, false, 3600);
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, true), 0);
-    PC.purge("www.powerdns.com$");
-
-    PC.insert(&q, &r, true, 3600);
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, false), 0);
-    PC.purge("www.powerdns.com$");
-
-    PC.insert(&q, &r, true, 3600);
+    PC.insert(&q, &r, 3600);
     PC.purge("www.powerdns.net");
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, true), 1);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 1);
     BOOST_CHECK_EQUAL(r2.qdomain, r.qdomain);
     PC.purge("net$");
-    BOOST_CHECK_EQUAL(PC.get(&q, &r2, true), 1);
+    BOOST_CHECK_EQUAL(PC.get(&q, &r2), 1);
     BOOST_CHECK_EQUAL(r2.qdomain, r.qdomain);
     PC.purge("www.powerdns.com$");
     BOOST_CHECK_EQUAL(PC.size(), 0);