From: Kees Monshouwer Date: Mon, 18 Jun 2018 19:32:25 +0000 (+0200) Subject: rec: move bogus ring from packet cache to doProcessUDPQuestion() X-Git-Tag: dnsdist-1.3.1~25^2~1 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8467ec2670b5f87391f9d1612c25a0c4cd3b4c8a;p=pdns rec: move bogus ring from packet cache to doProcessUDPQuestion() --- diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index 6a7a87031..e433bb16e 100644 --- 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 4067f4bbb..dd71eff8d 100644 --- 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::type::iterator, packetCache_t::index::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::type::iterator, packetCache_t::index::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(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::paird_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(); @@ -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(); @@ -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 0fa182c71..7293c55f1 100644 --- 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& protobufMessage); void doPruneTo(unsigned int maxSize=250000); @@ -108,7 +107,7 @@ private: packetCache_t d_packetCache; - bool checkResponseMatches(std::pair::type::iterator, packetCache_t::index::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::type::iterator, packetCache_t::index::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 b8e1e490f..5b1c08463 100644 --- 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 -thread_local std::unique_ptr t_bogusremotes; -thread_local std::unique_ptr > > t_bogusqueryring; BOOST_AUTO_TEST_SUITE(recpacketcache_cc)