rec: move bogus ring from packet cache to doProcessUDPQuestion()

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 6a7a870311cb3be4a158568e084ca4d1350ea708..e433bb16e2ebe115b8b2671d581a14605ea7355d 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -1916,14 +1916,22 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr
     /* It might seem like a good idea to skip the packet cache lookup if we know that the answer is not cacheable,
        but it means that the hash would not be computed. If some script decides at a later time to mark back the answer
        as cacheable we would cache it with a wrong tag, so better safe than sorry. */
+    vState valState;
     if (qnameParsed) {
-      cacheHit = (!SyncRes::s_nopacketcache && t_packetCache->getResponsePacket(ctag, source, question, qname, qtype, qclass, g_now.tv_sec, &response, &age, &qhash, pbMessage ? &(*pbMessage) : nullptr));
+      cacheHit = (!SyncRes::s_nopacketcache && t_packetCache->getResponsePacket(ctag, question, qname, qtype, qclass, g_now.tv_sec, &response, &age, &valState, &qhash, pbMessage ? &(*pbMessage) : nullptr));
     }
     else {
-      cacheHit = (!SyncRes::s_nopacketcache && t_packetCache->getResponsePacket(ctag, source, question, g_now.tv_sec, &response, &age, &qhash, pbMessage ? &(*pbMessage) : nullptr));
+      cacheHit = (!SyncRes::s_nopacketcache && t_packetCache->getResponsePacket(ctag, question, qname, &qtype, &qclass, g_now.tv_sec, &response, &age, &valState, &qhash, pbMessage ? &(*pbMessage) : nullptr));
     }
 
     if (cacheHit) {
+      if(valState == Bogus) {
+        if(t_bogusremotes)
+          t_bogusremotes->push_back(source);
+        if(t_bogusqueryring)
+          t_bogusqueryring->push_back(make_pair(qname, qtype));
+      }
+
 #ifdef HAVE_PROTOBUF
       if(t_protobufServer && (!luaconfsLocal->protobufTaggedOnly || !pbMessage->getAppliedPolicy().empty() || !pbMessage->getPolicyTags().empty())) {
         Netmask requestorNM(source, source.sin4.sin_family == AF_INET ? luaconfsLocal->protobufMaskV4 : luaconfsLocal->protobufMaskV6);

diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc
index 4067f4bbb1bcaef7c4163251687ad264814e7b00..dd71eff8d85c96ad9588697d4bf2b612ded88e17 100644 (file)
--- a/pdns/recpacketcache.cc
+++ b/pdns/recpacketcache.cc
@@ -8,7 +8,6 @@
 #include "cachecleaner.hh"
 #include "dns.hh"
 #include "namespaces.hh"
-#include "syncres.hh"
 
 RecursorPacketCache::RecursorPacketCache()
 {
@@ -46,7 +45,7 @@ static bool qrMatch(const DNSName& qname, uint16_t qtype, uint16_t qclass, const
   return qname==rname && rtype == qtype && rclass == qclass;
 }
 
-bool RecursorPacketCache::checkResponseMatches(std::pair<packetCache_t::index<HashTag>::type::iterator, packetCache_t::index<HashTag>::type::iterator> range, const ComboAddress& source, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, RecProtoBufMessage* protobufMessage)
+bool RecursorPacketCache::checkResponseMatches(std::pair<packetCache_t::index<HashTag>::type::iterator, packetCache_t::index<HashTag>::type::iterator> range, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, RecProtoBufMessage* protobufMessage)
 {
   for(auto iter = range.first ; iter != range.second ; ++ iter) {
     // the possibility is VERY real that we get hits that are not right - birthday paradox
@@ -56,6 +55,7 @@ bool RecursorPacketCache::checkResponseMatches(std::pair<packetCache_t::index<Ha
       *age = static_cast<uint32_t>(now - iter->d_creation);
       *responsePacket = iter->d_packet;
       responsePacket->replace(0, 2, queryPacket.c_str(), 2);
+      *valState = iter->d_vstate;
     
       string::size_type i=sizeof(dnsheader);
       
@@ -67,12 +67,6 @@ bool RecursorPacketCache::checkResponseMatches(std::pair<packetCache_t::index<Ha
         i = i + labellen;
       }
 
-      if(iter->d_vstate == Bogus) {
-        if(t_bogusremotes)
-          t_bogusremotes->push_back(source);
-        if(t_bogusqueryring)
-          t_bogusqueryring->push_back(make_pair(qname, qtype));
-      }
       d_hits++;
       moveCacheItemToBack(d_packetCache, iter);
 #ifdef HAVE_PROTOBUF
@@ -101,19 +95,21 @@ bool RecursorPacketCache::checkResponseMatches(std::pair<packetCache_t::index<Ha
 bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now,
                                             std::string* responsePacket, uint32_t* age, uint32_t* qhash)
 {
-  ComboAddress source;
-  return getResponsePacket(tag, source, queryPacket, now, responsePacket, age, qhash, nullptr);
+  DNSName qname;
+  uint16_t qtype, qclass;
+  vState valState;
+  return getResponsePacket(tag, queryPacket, qname, &qtype, &qclass, now, responsePacket, age, &valState, qhash, nullptr);
 }
 
 bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now,
                                             std::string* responsePacket, uint32_t* age, uint32_t* qhash)
 {
-  ComboAddress source;
-  return getResponsePacket(tag, source, queryPacket, qname, qtype, qclass, now, responsePacket, age, qhash, nullptr);
+  vState valState;
+  return getResponsePacket(tag, queryPacket, qname, qtype, qclass, now, responsePacket, age, &valState, qhash, nullptr);
 }
 
-bool RecursorPacketCache::getResponsePacket(unsigned int tag, const ComboAddress& source, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now,
-                                            std::string* responsePacket, uint32_t* age, uint32_t* qhash, RecProtoBufMessage* protobufMessage)
+bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now,
+                                            std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, RecProtoBufMessage* protobufMessage)
 {
   *qhash = canHashPacket(queryPacket, true);
   const auto& idx = d_packetCache.get<HashTag>();
@@ -123,11 +119,11 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const ComboAddress
     d_misses++;
     return false;
   }
-  return checkResponseMatches(range, source, queryPacket, qname, qtype, qclass, now, responsePacket, age, protobufMessage);
+  return checkResponseMatches(range, queryPacket, qname, qtype, qclass, now, responsePacket, age, valState, protobufMessage);
 }
 
-bool RecursorPacketCache::getResponsePacket(unsigned int tag, const ComboAddress& source, const std::string& queryPacket, time_t now,
-                                            std::string* responsePacket, uint32_t* age, uint32_t* qhash, RecProtoBufMessage* protobufMessage)
+bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, DNSName& qname, uint16_t* qtype, uint16_t* qclass, time_t now,
+                                            std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, RecProtoBufMessage* protobufMessage)
 {
   *qhash = canHashPacket(queryPacket, true);
   const auto& idx = d_packetCache.get<HashTag>();
@@ -138,12 +134,12 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const ComboAddress
     return false;
   }
 
-  uint16_t qtype, qclass;
-  DNSName qname(queryPacket.c_str(), queryPacket.length(), sizeof(dnsheader), false, &qtype, &qclass, 0);
+  qname = DNSName(queryPacket.c_str(), queryPacket.length(), sizeof(dnsheader), false, qtype, qclass, 0);
 
-  return checkResponseMatches(range, source, queryPacket, qname, qtype, qclass, now, responsePacket, age, protobufMessage);
+  return checkResponseMatches(range, queryPacket, qname, *qtype, *qclass, now, responsePacket, age, valState, protobufMessage);
 }
 
+
 void RecursorPacketCache::insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttl)
 {
   vState valState;

diff --git a/pdns/recpacketcache.hh b/pdns/recpacketcache.hh
index 0fa182c71b438b6dbf2e4121bd62136cd6ff6ad7..7293c55f16bfcf83d3ad5c2c67fd0726ba7c400d 100644 (file)
--- a/pdns/recpacketcache.hh
+++ b/pdns/recpacketcache.hh
@@ -52,11 +52,10 @@ class RecursorPacketCache: public PacketCache
 {
 public:
   RecursorPacketCache();
-
   bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash);
   bool getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash);
-  bool getResponsePacket(unsigned int tag, const ComboAddress& source, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash, RecProtoBufMessage* protobufMessage);
-  bool getResponsePacket(unsigned int tag, const ComboAddress& source, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash, RecProtoBufMessage* protobufMessage);
+  bool getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, RecProtoBufMessage* protobufMessage);
+  bool getResponsePacket(unsigned int tag, const std::string& queryPacket, DNSName& qname, uint16_t* qtype, uint16_t* qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, RecProtoBufMessage* protobufMessage);
   void insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttl);
   void insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttl, const vState& valState, const boost::optional<RecProtoBufMessage>& protobufMessage);
   void doPruneTo(unsigned int maxSize=250000);
@@ -108,7 +107,7 @@ private:
   
   packetCache_t d_packetCache;
 
-  bool checkResponseMatches(std::pair<packetCache_t::index<HashTag>::type::iterator, packetCache_t::index<HashTag>::type::iterator> range, const ComboAddress& source, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, RecProtoBufMessage* protobufMessage);
+  bool checkResponseMatches(std::pair<packetCache_t::index<HashTag>::type::iterator, packetCache_t::index<HashTag>::type::iterator> range, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, RecProtoBufMessage* protobufMessage);
 
 public:
   void preRemoval(const Entry& entry)

diff --git a/pdns/test-recpacketcache_cc.cc b/pdns/test-recpacketcache_cc.cc
index b8e1e490f3e0ec711ed7ad6d70df0c1918b079f7..5b1c084633b18229d7955ef15b22a00e59a9d1f3 100644 (file)
--- a/pdns/test-recpacketcache_cc.cc
+++ b/pdns/test-recpacketcache_cc.cc
@@ -10,11 +10,8 @@
 #include "dns_random.hh"
 #include "iputils.hh"
 #include "recpacketcache.hh"
-#include "syncres.hh"
 #include <utility>
 
-thread_local std::unique_ptr<addrringbuf_t> t_bogusremotes;
-thread_local std::unique_ptr<boost::circular_buffer<pair<DNSName, uint16_t> > > t_bogusqueryring;
 
 BOOST_AUTO_TEST_SUITE(recpacketcache_cc)