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.

diff --git a/pdns/README-dnsdist.md b/pdns/README-dnsdist.md
index a054eda07623849df3a7b9dbafcf9463f8347f73..1e6704e4ab1ce2bc91577382a5d15609cf766f85 100644 (file)
--- 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 f14114e9b0e8c57484c10332ed72546411258090..3508962be39a41bde6f2d70839733da9bbb4a263 100644 (file)
--- 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 5c9f2d653daf43456e657cdb300741529f3ff8c2..0ee1b136cb322e5aa4876f1e545eb75e36b5237e 100644 (file)
--- 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<uint64_t> 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 07f7e56b5f81412773866b61433709db207156ce..82b35927fd0a7d72098e237252673c005b5e9d96 100644 (file)
--- 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<uint32_t> maxTTL, boost::optional<uint32_t> minTTL) {
-        return std::make_shared<DNSDistPacketCache>(maxEntries, maxTTL ? *maxTTL : 86400, minTTL ? *minTTL : 60);
+    g_lua.writeFunction("newPacketCache", [client](size_t maxEntries, boost::optional<uint32_t> maxTTL, boost::optional<uint32_t> minTTL, boost::optional<uint32_t> servFailTTL) {
+        return std::make_shared<DNSDistPacketCache>(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 084786077e5eac08ec20f86b282075e5878e87be..275f8d0e6e29c7453f6e2089c4c36c31eba94954 100644 (file)
--- 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 805768918cb8d1e006c327e24d4026fe38969551..d86c36081804ecee295343916708e8317f8af555 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -264,7 +264,7 @@ void* responderThread(std::shared_ptr<DownstreamState> 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