From: Kees Monshouwer Date: Mon, 24 Oct 2016 19:29:42 +0000 (+0200) Subject: remove recursion from packetcache X-Git-Tag: rec-4.1.0-alpha1~231^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=071d2d90baa7d35d18ff469b1c72b92709d2341c;p=thirdparty%2Fpdns.git remove recursion from packetcache --- diff --git a/pdns/common_startup.cc b/pdns/common_startup.cc index 4972f217a8..4e4839e8af 100644 --- 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"<&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 9a9283ad64..b1b5f583bf 100644 --- 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 '"<qdomain<<"', merits: "<qdomain<<"'"<d_tcp ? 0xffff : q->getMaxReplyLen(); - unsigned int ourttl = recursive ? d_recursivettl : d_ttl; - if(!recursive) { - if(maxttlgetMinTTL(); - if(minttlqdomain, q->qtype, PacketCache::PACKETCACHE, r->getString(), ourttl, -1, recursive, + unsigned int ourttl = d_ttl; + if(maxttlqdomain, 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: "<(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 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 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 8949b6f0d4..357acedd1a 100644 --- 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& 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& entry, int zoneID=-1); @@ -78,13 +78,13 @@ public: map 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& 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, member, member, - member, member, member, member >, - composite_key_compare, std::less, std::less, std::less, + composite_key_compare, std::less, std::less, std::less, std::less, std::less > >, hashed_non_unique, composite_keywrapup(); // 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<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"<setRemote(&packet->d_remote); diff --git a/pdns/test-packetcache_cc.cc b/pdns/test-packetcache_cc.cc index e6ceb7ddcf..31c6304fa2 100644 --- 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);