More fine grained capping of packet cache TTL

Distinguish between negative answers (NxDomain and NoData), failure to resolve
(ServFail and completely empty answers) and ordinary answers when capping the
packet cache TTL. A new setting (packetcache-negative-ttl) is created for that.

NoData is: empty answer section but SOA record present in authority section.

diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst
index bf7b41f6ec8fd0d64baa0f424ad8471e300e05cd..457267444614dbda01d36bd61cbf73983b286e5c 100644 (file)
--- a/pdns/recursordist/docs/settings.rst
+++ b/pdns/recursordist/docs/settings.rst
@@ -1586,6 +1586,19 @@ If an answer containing an NSEC3 record with more iterations is received, its DN
 
 Maximum number of seconds to cache an item in the packet cache, no matter what the original TTL specified.
 
+.. _setting-packetcache-negative-ttl:
+
+``packetcache-negative-ttl``
+----------------------------
+.. versionadded:: 4.9.0
+
+-  Integer
+-  Default: 60
+
+Maximum number of seconds to cache an ``NxDomain`` or ``NoData`` answer in the packetcache.
+This setting's maximum is capped to `packetcache-ttl`_.
+i.e. setting ``packetcache-ttl=15`` and keeping ``packetcache-negative-ttl`` at the default will lower ``packetcache-negative-ttl`` to ``15``.
+
 .. _setting-packetcache-servfail-ttl:
 
 ``packetcache-servfail-ttl``

diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
index 1c6ddf5ff26900138bff93188e3a703945ca1f9c..69c70f5d7ed42fa16cb72521f72b31af69f5e6eb 100644 (file)
--- a/pdns/recursordist/pdns_recursor.cc
+++ b/pdns/recursordist/pdns_recursor.cc
@@ -420,10 +420,14 @@ static void handleRPZCustom(const DNSRecord& spoofed, const QType& qtype, SyncRe
   }
 }
 
-static bool addRecordToPacket(DNSPacketWriter& pw, const DNSRecord& rec, uint32_t& minTTL, uint32_t ttlCap, const uint16_t maxAnswerSize)
+static bool addRecordToPacket(DNSPacketWriter& pw, const DNSRecord& rec, uint32_t& minTTL, uint32_t ttlCap, const uint16_t maxAnswerSize, bool& seenAuthSOA)
 {
   pw.startRecord(rec.d_name, rec.d_type, (rec.d_ttl > ttlCap ? ttlCap : rec.d_ttl), rec.d_class, rec.d_place);
 
+  if (rec.d_type == QType::SOA && rec.d_place == DNSResourceRecord::AUTHORITY) {
+    seenAuthSOA = true;
+  }
+
   if (rec.d_type != QType::OPT) // their TTL ain't real
     minTTL = min(minTTL, rec.d_ttl);
 
@@ -875,6 +879,20 @@ static void dumpTrace(const string& trace, const timeval& timev)
   // fclose by unique_ptr does implicit flush
 }
 
+static uint32_t capPacketCacheTTL(const struct dnsheader& hdr, uint32_t ttl, bool seenAuthSOA)
+{
+  if (hdr.rcode == RCode::NXDomain || (hdr.rcode == RCode::NoError && hdr.ancount == 0 && seenAuthSOA)) {
+    ttl = std::min(ttl, SyncRes::s_packetcachenegativettl);
+  }
+  else if ((hdr.rcode != RCode::NoError && hdr.rcode != RCode::NXDomain) || (hdr.ancount == 0 && hdr.nscount == 0)) {
+    ttl = min(ttl, SyncRes::s_packetcacheservfailttl);
+  }
+  else {
+    ttl = std::min(ttl, SyncRes::s_packetcachettl);
+  }
+  return ttl;
+}
+
 void startDoResolve(void* p)
 {
   auto dc = std::unique_ptr<DNSComboWriter>(reinterpret_cast<DNSComboWriter*>(p));
@@ -984,6 +1002,7 @@ void startDoResolve(void* p)
        If we have a TTL cap, this value can't be larger than the
        cap no matter what. */
     uint32_t minTTL = dc->d_ttlCap;
+    bool seenAuthSOA = false;
 
     sr.d_eventTrace = std::move(dc->d_eventTrace);
     sr.setId(MT->getTid());
@@ -1413,7 +1432,7 @@ void startDoResolve(void* p)
           continue;
         }
 
-        if (!addRecordToPacket(pw, *i, minTTL, dc->d_ttlCap, maxanswersize)) {
+        if (!addRecordToPacket(pw, *i, minTTL, dc->d_ttlCap, maxanswersize, seenAuthSOA)) {
           needCommit = false;
           break;
         }
@@ -1655,11 +1674,7 @@ void startDoResolve(void* p)
     }
 
     if (t_packetCache && !variableAnswer && !sr.wasVariable()) {
-      const auto& hdr = pw.getHeader();
-      if ((hdr->rcode != RCode::NoError && hdr->rcode != RCode::NXDomain) || (hdr->ancount == 0 && hdr->nscount == 0)) {
-        minTTL = min(minTTL, SyncRes::s_packetcacheservfailttl);
-      }
-      minTTL = min(minTTL, SyncRes::s_packetcachettl);
+      minTTL = capPacketCacheTTL(*pw.getHeader(), minTTL, seenAuthSOA);
       t_packetCache->insertResponsePacket(dc->d_tag, dc->d_qhash, std::move(dc->d_query), dc->d_mdp.d_qname,
                                           dc->d_mdp.d_qtype, dc->d_mdp.d_qclass,
                                           string((const char*)&*packet.begin(), packet.size()),

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 82365a91350c2fecdb9271bb3261d5f77d924564..4ea5166d23c8c4b193be5ead6accc8c8438fab87 100644 (file)
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -1510,10 +1510,12 @@ static int serviceMain(int argc, char* argv[], Logr::log_t log)
   SyncRes::s_maxnegttl = ::arg().asNum("max-negative-ttl");
   SyncRes::s_maxbogusttl = ::arg().asNum("max-cache-bogus-ttl");
   SyncRes::s_maxcachettl = max(::arg().asNum("max-cache-ttl"), 15);
+
   SyncRes::s_packetcachettl = ::arg().asNum("packetcache-ttl");
-  // Cap the packetcache-servfail-ttl to the packetcache-ttl
-  uint32_t packetCacheServFailTTL = ::arg().asNum("packetcache-servfail-ttl");
-  SyncRes::s_packetcacheservfailttl = (packetCacheServFailTTL > SyncRes::s_packetcachettl) ? SyncRes::s_packetcachettl : packetCacheServFailTTL;
+  // Cap the packetcache-servfail-ttl and packetcache-negative-ttl to packetcache-ttl
+  SyncRes::s_packetcacheservfailttl = std::min(static_cast<unsigned int>(::arg().asNum("packetcache-servfail-ttl")), SyncRes::s_packetcachettl);
+  SyncRes::s_packetcachenegativettl = std::min(static_cast<unsigned int>(::arg().asNum("packetcache-negative-ttl")), SyncRes::s_packetcachettl);
+
   SyncRes::s_serverdownmaxfails = ::arg().asNum("server-down-max-fails");
   SyncRes::s_serverdownthrottletime = ::arg().asNum("server-down-throttle-time");
   SyncRes::s_nonresolvingnsmaxfails = ::arg().asNum("non-resolving-ns-max-fails");
@@ -2695,6 +2697,7 @@ int main(int argc, char** argv)
     ::arg().set("packetcache-ttl", "maximum number of seconds to keep a cached entry in packetcache") = "3600";
     ::arg().set("max-packetcache-entries", "maximum number of entries to keep in the packetcache") = "500000";
     ::arg().set("packetcache-servfail-ttl", "maximum number of seconds to keep a cached servfail entry in packetcache") = "60";
+    ::arg().set("packetcache-negative-ttl", "maximum number of seconds to keep a cached NxDomain or NoData entry in packetcache") = "60";
     ::arg().set("server-id", "Returned when queried for 'id.server' TXT or NSID, defaults to hostname, set custom or 'disabled'") = "";
     ::arg().set("stats-ringbuffer-entries", "maximum number of packets to store statistics for") = "10000";
     ::arg().set("version-string", "string reported on version.pdns or version.bind") = fullVersionString();

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index 88b5aaaaaef80e182a22e1e800788abec2a2fd22..31654b06ef13d000e9d92ac4373653e01457da8f 100644 (file)
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -436,6 +436,7 @@ unsigned int SyncRes::s_minimumTTL;
 unsigned int SyncRes::s_minimumECSTTL;
 unsigned int SyncRes::s_packetcachettl;
 unsigned int SyncRes::s_packetcacheservfailttl;
+unsigned int SyncRes::s_packetcachenegativettl;
 unsigned int SyncRes::s_serverdownmaxfails;
 unsigned int SyncRes::s_serverdownthrottletime;
 unsigned int SyncRes::s_nonresolvingnsmaxfails;

diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh
index 74693009a772938708c0076f9f147f40eb3d585a..496ea28efea150c7982f27e46b52497f61fc2281 100644 (file)
--- a/pdns/recursordist/syncres.hh
+++ b/pdns/recursordist/syncres.hh
@@ -518,6 +518,7 @@ public:
   static unsigned int s_maxcachettl;
   static unsigned int s_packetcachettl;
   static unsigned int s_packetcacheservfailttl;
+  static unsigned int s_packetcachenegativettl;
   static unsigned int s_serverdownmaxfails;
   static unsigned int s_serverdownthrottletime;
   static unsigned int s_nonresolvingnsmaxfails;

diff --git a/regression-tests.recursor-dnssec/test_PacketCache.py b/regression-tests.recursor-dnssec/test_PacketCache.py
index f797822d8a007611978530a6bf24c3dc7cffd4a4..80b015f6f9dd5533f742db347153a61605d05b95 100644 (file)
--- a/regression-tests.recursor-dnssec/test_PacketCache.py
+++ b/regression-tests.recursor-dnssec/test_PacketCache.py
@@ -21,6 +21,7 @@ class PacketCacheRecursorTest(RecursorTest):
     _apiKey = 'secretapikey'
     _config_template = """
     packetcache-ttl=10
+    packetcache-negative-ttl=8
     packetcache-servfail-ttl=5
     auth-zones=example=configs/%s/example.zone
     webserver=yes
@@ -141,13 +142,13 @@ f 3600 IN CNAME f            ; CNAME loop: dirty trick to get a ServFail in an a
         self.assertRRsetInAnswer(res, expected)
         self.checkPacketCacheMetrics(6, 4)
 
-        # NXDomain should get default packetcache TTL (10)
+        # NXDomain should get negative packetcache TTL (8)
         query = dns.message.make_query('nxdomain.example.', 'A', want_dnssec=True)
         res = self.sendUDPQuery(query)
         self.assertRcodeEqual(res, dns.rcode.NXDOMAIN)
         self.checkPacketCacheMetrics(6, 5)
 
-        # NoData should get default packetcache TTL (10)
+        # NoData should get negative packetcache TTL (8)
         query = dns.message.make_query('a.example.', 'AAAA', want_dnssec=True)
         res = self.sendUDPQuery(query)
         self.assertRcodeEqual(res, dns.rcode.NOERROR)
@@ -166,8 +167,8 @@ f 3600 IN CNAME f            ; CNAME loop: dirty trick to get a ServFail in an a
         try:
             ret = subprocess.check_output(rec_controlCmd, stderr=subprocess.STDOUT)
             self.assertTrue((b"a.example. 10 A  ; tag 0 udp\n" in ret) or (b"a.example. 9 A  ; tag 0 udp\n" in ret))
-            self.assertTrue((b"nxdomain.example. 10 A  ; tag 0 udp\n" in ret) or (b"nxdomain.example. 9 A  ; tag 0 udp\n" in ret))
-            self.assertTrue((b"a.example. 10 AAAA  ; tag 0 udp\n" in ret) or (b"a.example. 9 AAAA  ; tag 0 udp\n" in ret))
+            self.assertTrue((b"nxdomain.example. 8 A  ; tag 0 udp\n" in ret) or (b"nxdomain.example. 7 A  ; tag 0 udp\n" in ret))
+            self.assertTrue((b"a.example. 8 AAAA  ; tag 0 udp\n" in ret) or (b"a.example. 7 AAAA  ; tag 0 udp\n" in ret))
             self.assertTrue((b"f.example. 5 A  ; tag 0 udp\n" in ret) or (b"f.example. 4 A  ; tag 0 udp\n" in ret))
 
         except subprocess.CalledProcessError as e: