From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Wed, 1 Mar 2023 08:28:20 +0000 (+0100)
Subject: Fix serve-stale logic in negcache by following the record cache case more closely
X-Git-Tag: rec-4.8.3~1^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=442fab1c89326f331cfee2d9d7a03ffa0c20f011;p=thirdparty%2Fpdns.git

Fix serve-stale logic in negcache by following the record cache case more closely
---

diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index 50b01abe95..0923b7de5a 100644
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -119,27 +119,31 @@ bool NegCache::get(const DNSName& qname, QType qtype, const struct timeval& now,
 
   const auto& idx = content->d_map.get<NegCacheEntry>();
   auto range = idx.equal_range(qname);
-  auto ni = range.first;
 
-  while (ni != range.second) {
+  for (auto ni = range.first; ni != range.second; ++ni) {
     // We have an entry
     if ((!typeMustMatch && ni->d_qtype == QType::ENT) || ni->d_qtype == qtype) {
       // We match the QType or the whole name is denied
       auto firstIndexIterator = content->d_map.project<CompositeKey>(ni);
 
-      if (!refresh && (serveStale || ni->d_servedStale > 0) && ni->d_ttd <= now.tv_sec && ni->d_servedStale < s_maxServedStaleExtensions) {
+      // this checks ttd, but also takes into account serve-stale
+      if (!ni->isEntryUsable(now.tv_sec, serveStale)) {
+        // Outdated
+        moveCacheItemToFront<SequenceTag>(content->d_map, firstIndexIterator);
+        continue;
+      }
+      // If we are serving this record stale (or *should*) and the ttd has passed increase ttd to
+      // the future and remember that we did. Also push a refresh task.
+      if ((serveStale || ni->d_servedStale > 0) && ni->d_ttd <= now.tv_sec && ni->d_servedStale < s_maxServedStaleExtensions) {
         updateStaleEntry(now.tv_sec, firstIndexIterator, qtype);
       }
-      if (now.tv_sec < ni->d_ttd && !(refresh && ni->d_servedStale > 0)) {
+      if (now.tv_sec < ni->d_ttd) {
         // Not expired
         ne = *ni;
         moveCacheItemToBack<SequenceTag>(content->d_map, firstIndexIterator);
-        return true;
+        return !refresh; // when refreshing, we consider everything outdated
       }
-      // expired
-      moveCacheItemToFront<SequenceTag>(content->d_map, firstIndexIterator);
     }
-    ++ni;
   }
   return false;
 }
diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index 2e989aadef..0d42afee10 100644
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -84,6 +84,12 @@ public:
         return d_ttd < now;
       }
     };
+
+    bool isEntryUsable(time_t now, bool serveStale) const
+    {
+      // When serving stale, we consider expired records
+      return d_ttd > now || serveStale || d_servedStale != 0;
+    }
   };
 
   void add(const NegCacheEntry& ne);