From 0f08e82b8028b5d6b55993d2578c7c27f7fe8544 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 29 Feb 2016 16:22:04 +0100 Subject: [PATCH] dnsdist: Add a specific TTL for ServFail responses Before this commit we used the maxTTL for Server Failure responses as well, and it might not be a good idea. Fixes #3469. --- pdns/README-dnsdist.md | 9 +++++---- pdns/dnsdist-cache.cc | 21 ++++++++++++++------- pdns/dnsdist-cache.hh | 5 +++-- pdns/dnsdist-lua2.cc | 4 ++-- pdns/dnsdist-tcp.cc | 2 +- pdns/dnsdist.cc | 2 +- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/pdns/README-dnsdist.md b/pdns/README-dnsdist.md index a054eda076..1e6704e4ab 100644 --- a/pdns/README-dnsdist.md +++ b/pdns/README-dnsdist.md @@ -712,14 +712,15 @@ The first step is to define a cache, then to assign that cache to the chosen poo the default one being represented by the empty string: ``` -pc = newPacketCache(10000, 86400, 600) +pc = newPacketCache(10000, 86400, 600, 60) getPool(""):setCache(pc) ``` The first parameter is the maximum number of entries stored in the cache, the second one, optional, is the maximum lifetime of an entry in the cache, in seconds, -and the last one, optional too, is the minimum TTL an entry should have to be considered -for insertion in the cache. +the third one, optional too, is the minimum TTL an entry should have to be considered +for insertion in the cache, and the last one, still optional, is the TTL used for a +Server Failure response. Performance tuning @@ -1035,7 +1036,7 @@ instantiate a server with additional parameters * `expunge(n)`: remove entries from the cache, leaving at most `n` entries * `expungeByName(DNSName [, qtype=ANY])`: remove entries matching the supplied DNSName and type from the cache * `isFull()`: return true if the cache has reached the maximum number of entries - * `newPacketCache(maxEntries, maxTTL=86400, minTTL=60)`: return a new PacketCache + * `newPacketCache(maxEntries[, maxTTL=86400, minTTL=60, servFailTTL=60])`: return a new PacketCache * `printStats()`: print the cache stats (hits, misses, deferred lookups and deferred inserts) * `purgeExpired(n)`: remove expired entries from the cache until there is at most `n` entries remaining in the cache * `toString()`: return the number of entries in the Packet Cache, and the maximum number of entries diff --git a/pdns/dnsdist-cache.cc b/pdns/dnsdist-cache.cc index f14114e9b0..3508962be3 100644 --- a/pdns/dnsdist-cache.cc +++ b/pdns/dnsdist-cache.cc @@ -2,7 +2,7 @@ #include "dnsdist-cache.hh" #include "dnsparser.hh" -DNSDistPacketCache::DNSDistPacketCache(size_t maxEntries, uint32_t maxTTL, uint32_t minTTL): d_maxEntries(maxEntries), d_maxTTL(maxTTL), d_minTTL(minTTL) +DNSDistPacketCache::DNSDistPacketCache(size_t maxEntries, uint32_t maxTTL, uint32_t minTTL, uint32_t servFailTTL): d_maxEntries(maxEntries), d_maxTTL(maxTTL), d_servFailTTL(servFailTTL), d_minTTL(minTTL) { pthread_rwlock_init(&d_lock, 0); /* we reserve maxEntries + 1 to avoid rehashing from occuring @@ -22,17 +22,24 @@ bool DNSDistPacketCache::cachedValueMatches(const CacheValue& cachedValue, const return true; } -void DNSDistPacketCache::insert(uint32_t key, const DNSName& qname, uint16_t qtype, uint16_t qclass, const char* response, uint16_t responseLen, bool tcp) +void DNSDistPacketCache::insert(uint32_t key, const DNSName& qname, uint16_t qtype, uint16_t qclass, const char* response, uint16_t responseLen, bool tcp, bool servFail) { if (responseLen == 0) return; - uint32_t minTTL = getMinTTL(response, responseLen); - if (minTTL > d_maxTTL) - minTTL = d_maxTTL; + uint32_t minTTL; - if (minTTL < d_minTTL) - return; + if (servFail) { + minTTL = d_servFailTTL; + } + else { + minTTL = getMinTTL(response, responseLen); + if (minTTL > d_maxTTL) + minTTL = d_maxTTL; + + if (minTTL < d_minTTL) + return; + } { TryReadLock r(&d_lock); diff --git a/pdns/dnsdist-cache.hh b/pdns/dnsdist-cache.hh index 5c9f2d653d..0ee1b136cb 100644 --- a/pdns/dnsdist-cache.hh +++ b/pdns/dnsdist-cache.hh @@ -7,10 +7,10 @@ class DNSDistPacketCache : boost::noncopyable { public: - DNSDistPacketCache(size_t maxEntries, uint32_t maxTTL=86400, uint32_t minTTL=60); + DNSDistPacketCache(size_t maxEntries, uint32_t maxTTL=86400, uint32_t minTTL=60, uint32_t servFailTTL=60); ~DNSDistPacketCache(); - void insert(uint32_t key, const DNSName& qname, uint16_t qtype, uint16_t qclass, const char* response, uint16_t responseLen, bool tcp); + void insert(uint32_t key, const DNSName& qname, uint16_t qtype, uint16_t qclass, const char* response, uint16_t responseLen, bool tcp, bool servFail=false); bool get(const unsigned char* query, uint16_t queryLen, const DNSName& qname, uint16_t qtype, uint16_t qclass, uint16_t consumed, uint16_t queryId, char* response, uint16_t* responseLen, bool tcp, uint32_t* keyOut, bool skipAging=false); void purgeExpired(size_t upTo=0); void expunge(size_t upTo=0); @@ -55,5 +55,6 @@ private: std::atomic d_lookupCollisions{0}; size_t d_maxEntries; uint32_t d_maxTTL; + uint32_t d_servFailTTL; uint32_t d_minTTL; }; diff --git a/pdns/dnsdist-lua2.cc b/pdns/dnsdist-lua2.cc index 07f7e56b5f..82b35927fd 100644 --- a/pdns/dnsdist-lua2.cc +++ b/pdns/dnsdist-lua2.cc @@ -531,8 +531,8 @@ void moreLua(bool client) }); g_lua.registerFunction("getCache", &ServerPool::getCache); - g_lua.writeFunction("newPacketCache", [client](size_t maxEntries, boost::optional maxTTL, boost::optional minTTL) { - return std::make_shared(maxEntries, maxTTL ? *maxTTL : 86400, minTTL ? *minTTL : 60); + g_lua.writeFunction("newPacketCache", [client](size_t maxEntries, boost::optional maxTTL, boost::optional minTTL, boost::optional servFailTTL) { + return std::make_shared(maxEntries, maxTTL ? *maxTTL : 86400, minTTL ? *minTTL : 60, servFailTTL ? *servFailTTL : 60); }); g_lua.registerFunction("toString", &DNSDistPacketCache::toString); g_lua.registerFunction("isFull", &DNSDistPacketCache::isFull); diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index 084786077e..275f8d0e6e 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -479,7 +479,7 @@ void* tcpClientThread(int pipefd) } if (packetCache && !dq.skipCache) { - packetCache->insert(cacheKey, qname, qtype, qclass, response, responseLen, true); + packetCache->insert(cacheKey, qname, qtype, qclass, response, responseLen, true, dh->rcode == RCode::ServFail); } #ifdef HAVE_DNSCRYPT diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 805768918c..d86c360818 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -264,7 +264,7 @@ void* responderThread(std::shared_ptr state) g_stats.responses++; if (ids->packetCache && !ids->skipCache) { - ids->packetCache->insert(ids->cacheKey, qname, qtype, qclass, response, responseLen, false); + ids->packetCache->insert(ids->cacheKey, qname, qtype, qclass, response, responseLen, false, dh->rcode == RCode::ServFail); } #ifdef HAVE_DNSCRYPT -- 2.47.2