From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 26 Feb 2018 14:13:00 +0000 (+0000)
Subject: rec: Only update the ECS cache index when needed
X-Git-Tag: dnsdist-1.3.0~68^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d0fcc32b56de19422dcf014f5645f05b66121328;p=thirdparty%2Fpdns.git

rec: Only update the ECS cache index when needed

We don't need to update the ECS index when replacing an existing
entry, except if the entry has expired, because then we might have
removed it from the ECS index.
---

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index ea9a4612f5..23dee223fb 100644
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -272,14 +272,21 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType& qt
     isNew = true;
   }
 
-  /* don't bother building an ecsIndex if we don't have any netmask-specific entries */
-  if (ednsmask && !ednsmask->empty()) {
-    auto ecsIndexKey = boost::make_tuple(qname, qt.getCode());
-    auto ecsIndex = d_ecsIndex.find(ecsIndexKey);
-    if (ecsIndex == d_ecsIndex.end()) {
-      ecsIndex = d_ecsIndex.insert(ECSIndexEntry(qname, qt.getCode())).first;
+  /* if we are inserting a new entry or updating an expired one (in which case the
+     ECS index might have been removed but the entry still exists because it has not
+     been garbage collected yet) we might need to update the ECS index.
+     Otherwise it should already be indexed and we don't need to update it.
+  */
+  if (isNew || stored->d_ttd <= now) {
+    /* don't bother building an ecsIndex if we don't have any netmask-specific entries */
+    if (ednsmask && !ednsmask->empty()) {
+      auto ecsIndexKey = boost::make_tuple(qname, qt.getCode());
+      auto ecsIndex = d_ecsIndex.find(ecsIndexKey);
+      if (ecsIndex == d_ecsIndex.end()) {
+        ecsIndex = d_ecsIndex.insert(ECSIndexEntry(qname, qt.getCode())).first;
+      }
+      ecsIndex->addMask(*ednsmask);
     }
-    ecsIndex->addMask(*ednsmask);
   }
 
   time_t maxTTD=std::numeric_limits<time_t>::max();
diff --git a/pdns/recursordist/test-recursorcache_cc.cc b/pdns/recursordist/test-recursorcache_cc.cc
index 7d42aa03fa..0780b62b2f 100644
--- a/pdns/recursordist/test-recursorcache_cc.cc
+++ b/pdns/recursordist/test-recursorcache_cc.cc
@@ -679,6 +679,16 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheECSIndex) {
   BOOST_CHECK_EQUAL(MRC.ecsIndexSize(), 0);
   BOOST_CHECK_EQUAL(MRC.size(), 1);
 
+  /* add back the entry while it still exists in the cache but has been removed from the ECS index.
+     It should be added back to the ECS index, and we should be able to retrieve it */
+  MRC.replace(now + ttl + 1, power, QType(QType::A), records, signatures, authRecords, true, Netmask("192.0.2.0/31"));
+  BOOST_CHECK_EQUAL(MRC.size(), 1);
+  BOOST_CHECK_EQUAL(MRC.ecsIndexSize(), 1);
+  retrieved.clear();
+  BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), false, &retrieved, ComboAddress("192.0.2.1")), ttd - now);
+  BOOST_REQUIRE_EQUAL(retrieved.size(), 1);
+  BOOST_CHECK_EQUAL(getRR<ARecordContent>(retrieved.at(0))->getCA().toString(), dr1Content.toString());
+
   /* wipe everything */
   MRC.doPrune(0);
   BOOST_CHECK_EQUAL(MRC.size(), 0);