]> granicus.if.org Git - pdns/commitdiff
packet cache improvements for recursive answers
authorKees Monshouwer <mind04@monshouwer.org>
Sat, 17 May 2014 17:17:00 +0000 (19:17 +0200)
committermind04 <mind04@monshouwer.org>
Sat, 17 May 2014 19:52:48 +0000 (21:52 +0200)
- fix tagging of recursive packets
- age ttl in packet cache for recursive results
- respect allow-recursion in packet cache

pdns/common_startup.cc
pdns/dnsproxy.cc
pdns/packetcache.cc
pdns/packetcache.hh
pdns/packethandler.cc
pdns/tcpreceiver.cc

index b8e144ef22492c9138f721df205eaf6c12d9caba..1fa1d08622922248bab9d644d9eeaf759396b46c 100644 (file)
@@ -252,6 +252,7 @@ void *qthread(void *number)
 
   int diff;
   bool logDNSQueries = ::arg().mustDo("log-dns-queries");
+  bool doRecursion = ::arg().mustDo("recursor");
   bool skipfirst=true;
   unsigned int maintcount = 0;
   UDPNameserver *NS = N;
@@ -305,23 +306,29 @@ void *qthread(void *number)
             "', do = " <<P->d_dnssecOk <<", bufsize = "<< P->getMaxReplyLen()<<": ";
     }
 
-
-    if((P->d.opcode != Opcode::Notify && P->d.opcode != Opcode::Update) && P->couldBeCached() && PC.get(P, &cached)) { // short circuit - does the PacketCache recognize this question?
-      if(logDNSQueries)
-        L<<"packetcache HIT"<<endl;
-      cached.setRemote(&P->d_remote);  // inlined
-      cached.setSocket(P->getSocket());                               // inlined
-      cached.d_anyLocal = P->d_anyLocal;
-      cached.setMaxReplyLen(P->getMaxReplyLen());
-      cached.d.rd=P->d.rd; // copy in recursion desired bit 
-      cached.d.id=P->d.id;
-      cached.commitD(); // commit d to the packet                        inlined
-
-      NS->send(&cached);   // answer it then                              inlined
-      diff=P->d_dt.udiff();                                                    
-      avg_latency=(int)(0.999*avg_latency+0.001*diff); // 'EWMA'
-      
-      continue;
+    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 (haveSomething) {
+        if(logDNSQueries)
+          L<<"packetcache HIT"<<endl;
+        cached.setRemote(&P->d_remote);  // inlined
+        cached.setSocket(P->getSocket());                               // inlined
+        cached.d_anyLocal = P->d_anyLocal;
+        cached.setMaxReplyLen(P->getMaxReplyLen());
+        cached.d.rd=P->d.rd; // copy in recursion desired bit
+        cached.d.id=P->d.id;
+        cached.commitD(); // commit d to the packet                        inlined
+
+        NS->send(&cached);   // answer it then                              inlined
+        diff=P->d_dt.udiff();
+        avg_latency=(int)(0.999*avg_latency+0.001*diff); // 'EWMA'
+
+        continue;
+      }
     }
     
     if(distributor->isOverloaded()) {
index 981d850fd4482243b9aa15d93405ab78d4824b28..2bc18b64332cb43389532fa1aa684bd8727e6a3f 100644 (file)
@@ -210,7 +210,7 @@ void DNSProxy::mainloop(void)
         if(sendmsg(i->second.outsock, &msgh, 0) < 0)
           L<<Logger::Warning<<"dnsproxy.cc: Error sending reply with sendmsg (socket="<<i->second.outsock<<"): "<<strerror(errno)<<endl;
         
-        PC.insert(&q, &p);
+        PC.insert(&q, &p, true);
         i->second.created=0;
       }
     }
index 84fc6babbc9a154de2522529f267af423c0cb741..cd244a58e6dbd659d7b7e8f9cd554c2d8ba9890d 100644 (file)
@@ -51,7 +51,7 @@ PacketCache::~PacketCache()
   WriteLock l(&d_mut);
 }
 
-int PacketCache::get(DNSPacket *p, DNSPacket *cached)
+int PacketCache::get(DNSPacket *p, DNSPacket *cached, bool recursive)
 {
   extern StatBag S;
 
@@ -75,10 +75,10 @@ int PacketCache::get(DNSPacket *p, DNSPacket *cached)
     }
   }
     
-  bool packetMeritsRecursion=d_doRecursion && p->d.rd;
   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;
   {
@@ -89,13 +89,14 @@ int PacketCache::get(DNSPacket *p, DNSPacket *cached)
     }
 
     uint16_t maxReplyLen = p->d_tcp ? 0xffff : p->getMaxReplyLen();
-    haveSomething=getEntryLocked(p->qdomain, p->qtype, PacketCache::PACKETCACHE, value, -1, packetMeritsRecursion, maxReplyLen, p->d_dnssecOk, p->hasEDNS());
+    haveSomething=getEntryLocked(p->qdomain, p->qtype, PacketCache::PACKETCACHE, value, -1, recursive, maxReplyLen, p->d_dnssecOk, p->hasEDNS(), &age);
   }
   if(haveSomething) {
     (*d_statnumhit)++;
-    if(cached->noparse(value.c_str(), value.size()) < 0) {
+    if (recursive)
+      ageDNSPacket(value, age);
+    if(cached->noparse(value.c_str(), value.size()) < 0)
       return 0;
-    }
     cached->spoofQuestion(p); // for correct case
     cached->qdomain=p->qdomain;
     cached->qtype=p->qtype;
@@ -116,7 +117,7 @@ void PacketCache::getTTLS()
 }
 
 
-void PacketCache::insert(DNSPacket *q, DNSPacket *r, unsigned int maxttl)
+void PacketCache::insert(DNSPacket *q, DNSPacket *r, bool recursive, unsigned int maxttl)
 {
   if(d_ttl < 0)
     getTTLS();
@@ -128,12 +129,17 @@ void PacketCache::insert(DNSPacket *q, DNSPacket *r, unsigned int maxttl)
   if(q->qclass != QClass::IN) // we only cache the INternet
     return;
 
-  bool packetMeritsRecursion=d_doRecursion && q->d.rd;
   uint16_t maxReplyLen = q->d_tcp ? 0xffff : q->getMaxReplyLen();
-  unsigned int ourttl = packetMeritsRecursion ? d_recursivettl : d_ttl;
-  if(maxttl<ourttl)
-    ourttl=maxttl;
-  insert(q->qdomain, q->qtype, PacketCache::PACKETCACHE, r->getString(), ourttl, -1, packetMeritsRecursion,
+  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,
     maxReplyLen, q->d_dnssecOk, q->hasEDNS());
 }
 
@@ -150,7 +156,8 @@ void PacketCache::insert(const string &qname, const QType& qtype, CacheEntryType
   
   //cerr<<"Inserting qname '"<<qname<<"', cet: "<<(int)cet<<", qtype: "<<qtype.getName()<<", ttl: "<<ttl<<", maxreplylen: "<<maxReplyLen<<", hasEDNS: "<<EDNS<<endl;
   CacheEntry val;
-  val.ttd=time(0)+ttl;
+  val.created=time(0);
+  val.ttd=val.created+ttl;
   val.qname=qname;
   val.qtype=qtype.getCode();
   val.value=value;
@@ -261,7 +268,7 @@ int PacketCache::purge(const string &match)
 }
 // called from ueberbackend
 bool PacketCache::getEntry(const string &qname, const QType& qtype, CacheEntryType cet, string& value, int zoneID, bool meritsRecursion, 
-  unsigned int maxReplyLen, bool dnssecOk, bool hasEDNS)
+  unsigned int maxReplyLen, bool dnssecOk, bool hasEDNS, unsigned int *age)
 {
   if(d_ttl<0) 
     getTTLS();
@@ -276,21 +283,24 @@ bool PacketCache::getEntry(const string &qname, const QType& qtype, CacheEntryTy
     return false;
   }
 
-  return getEntryLocked(qname, qtype, cet, value, zoneID, meritsRecursion, maxReplyLen, dnssecOk, hasEDNS);
+  return getEntryLocked(qname, qtype, cet, value, zoneID, meritsRecursion, maxReplyLen, dnssecOk, hasEDNS, age);
 }
 
 
 bool PacketCache::getEntryLocked(const string &qname, const QType& qtype, CacheEntryType cet, string& value, int zoneID, bool meritsRecursion,
-  unsigned int maxReplyLen, bool dnssecOK, bool hasEDNS)
+  unsigned int maxReplyLen, bool dnssecOK, bool hasEDNS, unsigned int *age)
 {
   uint16_t qt = qtype.getCode();
   //cerr<<"Lookup for maxReplyLen: "<<maxReplyLen<<endl;
-  cmap_t::const_iterator i=d_map.find(tie(qname, qt, cet, zoneID, meritsRecursion, maxReplyLen, dnssecOK, hasEDNS));
+  cmap_t::const_iterator i=d_map.find(tie(qname, qt, cet, zoneID, meritsRecursion, maxReplyLen, dnssecOK, hasEDNS, *age));
   time_t now=time(0);
   bool ret=(i!=d_map.end() && i->ttd > now);
-  if(ret)
+  if(ret) {
+    if (age)
+      *age = now - i->created;
     value = i->value;
-  
+  }
+
   return ret;
 }
 
index 902247b182a2781a3a6c7e951bd8c0f17fb160be..a447a8813cf74f440c47811b57274bc40a83b021 100644 (file)
@@ -72,14 +72,14 @@ public:
   ~PacketCache();
   enum CacheEntryType { PACKETCACHE, QUERYCACHE};
 
-  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(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(const string &qname, const QType& qtype, CacheEntryType cet, const string& value, unsigned int ttl, int zoneID=-1, bool meritsRecursion=false,
     unsigned int maxReplyLen=512, bool dnssecOk=false, bool EDNS=false);
 
-  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 string &content, const QType& qtype, CacheEntryType cet, string& entry, int zoneID=-1, 
-    bool meritsRecursion=false, unsigned int maxReplyLen=512, bool dnssecOk=false, bool hasEDNS=false);
+  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.
+  bool getEntry(const string &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);
 
   int size(); //!< number of entries in the cache
   void cleanup(); //!< force the cache to preen itself from expired packets
@@ -88,8 +88,8 @@ public:
 
   map<char,int> getCounts();
 private:
-  bool getEntryLocked(const string &content, const QType& qtype, CacheEntryType cet, string& entry, int zoneID=-1, 
-    bool meritsRecursion=false, unsigned int maxReplyLen=512, bool dnssecOk=false, bool hasEDNS=false);
+  bool getEntryLocked(const string &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);
   struct CacheEntry
   {
     CacheEntry() { qtype = ctype = 0; zoneID = -1; meritsRecursion=false; dnssecOk=false; hasEDNS=false;}
@@ -98,6 +98,7 @@ private:
     uint16_t qtype;
     uint16_t ctype;
     int zoneID;
+    time_t created;
     time_t ttd;
     bool meritsRecursion;
     unsigned int maxReplyLen;
index 47454dc9b4eaa0212ed87dc90cd72b03f26547cb..9cea755e7778bde0cc9a093e2d7adf8dbee42558 100644 (file)
@@ -1506,7 +1506,7 @@ DNSPacket *PacketHandler::questionOrRecurse(DNSPacket *p, bool *shouldRecurse)
       
     r->wrapup(); // needed for inserting in cache
     if(!noCache)
-      PC.insert(p, r, r->getMinTTL()); // in the packet cache
+      PC.insert(p, r, false, r->getMinTTL()); // in the packet cache
   }
   catch(DBException &e) {
     L<<Logger::Error<<"Backend reported condition which prevented lookup ("+e.reason+") sending out servfail"<<endl;
index 5483db1f0ea0f54a46498fba35367e3caeecc16c..f2ca024007fbe96d8205ede5c545d2d015d57c62 100644 (file)
@@ -318,7 +318,7 @@ void *TCPNameserver::doConnection(void *data)
       }
 
 
-      if(!packet->d.rd && packet->couldBeCached() && PC.get(packet.get(), cached.get())) { // short circuit - does the PacketCache recognize this question?
+      if(!packet->d.rd && packet->couldBeCached() && PC.get(packet.get(), cached.get(), false)) { // short circuit - does the PacketCache recognize this question?
         if(logDNSQueries)
           L<<"packetcache HIT"<<endl;
         cached->setRemote(&packet->d_remote);