dnsdist: Implement DynBlock oversampling, fix refresh issues with SMT entries

diff --git a/pdns/dnsdist-dynblocks.hh b/pdns/dnsdist-dynblocks.hh
index eaf85f09e0e74e30d07513cfc5cf312be06e62a7..87027a47ce1f9bf4cbf5a975338d97291a54a50e 100644 (file)
--- a/pdns/dnsdist-dynblocks.hh
+++ b/pdns/dnsdist-dynblocks.hh
@@ -63,6 +63,7 @@ private:
     std::map<uint8_t, uint64_t> d_rcodeCounts;
     std::map<uint16_t, uint64_t> d_qtypeCounts;
     uint64_t queries{0};
+    uint64_t responses{0};
     uint64_t respBytes{0};
   };
 
@@ -377,8 +378,8 @@ public:
   static std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> getHitsForTopSuffixes();
 
   /* get the the top offenders based on the current value of the counters */
-  static std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> getTopNetmasks();
-  static std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> getTopSuffixes();
+  static std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> getTopNetmasks(size_t topN);
+  static std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> getTopSuffixes(size_t topN);
   static void purgeExpired(const struct timespec& now);
 
   static time_t s_expiredDynBlocksPurgeInterval;

diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc
index 5d8eae4257d35c0d39b1ee1fa3428caf1099ec9a..6de9aed7716fda1cb65f538565f999fbc7f1b179 100644 (file)
--- a/pdns/dnsdist-lua.cc
+++ b/pdns/dnsdist-lua.cc
@@ -1148,8 +1148,9 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
       boost::format fmt("%-24s %8d %8d %-10s %-20s %s\n");
       g_outputBuffer = (fmt % "What" % "Seconds" % "Blocks" % "Warning" % "Action" % "Reason").str();
       for(const auto& e: slow) {
-       if(now < e.second.until)
-         g_outputBuffer+= (fmt % e.first.toString() % (e.second.until.tv_sec - now.tv_sec) % e.second.blocks % (e.second.warning ? "true" : "false") % DNSAction::typeToString(e.second.action != DNSAction::Action::None ? e.second.action : g_dynBlockAction) % e.second.reason).str();
+        if (now < e.second.until) {
+          g_outputBuffer+= (fmt % e.first.toString() % (e.second.until.tv_sec - now.tv_sec) % e.second.blocks % (e.second.warning ? "true" : "false") % DNSAction::typeToString(e.second.action != DNSAction::Action::None ? e.second.action : g_dynBlockAction) % e.second.reason).str();
+        }
       }
       auto slow2 = g_dynblockSMT.getCopy();
       slow2.visit([&now, &fmt](const SuffixMatchTree<DynBlock>& node) {

diff --git a/pdns/dnsdistdist/dnsdist-dynblocks.cc b/pdns/dnsdistdist/dnsdist-dynblocks.cc
index 95a59145593e150f728e5417f6c1ac1250d57b5a..900a25cfafc8c723d54c59de468782ee429831d4 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-dynblocks.cc
+++ b/pdns/dnsdistdist/dnsdist-dynblocks.cc
@@ -86,11 +86,11 @@ void DynBlockRulesGroup::apply(const struct timespec& now)
 
       const auto& rcodeIt = counters.d_rcodeCounts.find(rcode);
       if (rcodeIt != counters.d_rcodeCounts.cend()) {
-        if (pair.second.warningRatioExceeded(counters.queries, rcodeIt->second)) {
+        if (pair.second.warningRatioExceeded(counters.responses, rcodeIt->second)) {
           handleWarning(blocks, now, requestor, pair.second, updated);
         }
 
-        if (pair.second.ratioExceeded(counters.queries, rcodeIt->second)) {
+        if (pair.second.ratioExceeded(counters.responses, rcodeIt->second)) {
           addBlock(blocks, now, requestor, pair.second, updated);
           break;
         }
@@ -222,7 +222,13 @@ void DynBlockRulesGroup::addOrRefreshBlockSMT(SuffixMatchTree<DynBlock>& blocks,
   struct timespec until = now;
   until.tv_sec += rule.d_blockDuration;
   unsigned int count = 0;
-  const auto& got = blocks.lookup(name);
+  /* be careful, if you try to insert a longer suffix
+     lookup() might return a shorter one if it is
+     already in the tree as a final node */
+  const DynBlock* got = blocks.lookup(name);
+  if (got && got->domain != name) {
+    got = nullptr;
+  }
   bool expired = false;
 
   if (got) {
@@ -293,20 +299,34 @@ void DynBlockRulesGroup::processResponseRules(counts_t& counts, StatNode& root,
     return;
   }
 
+  struct timespec responseCutOff = now;
+
   d_respRateRule.d_cutOff = d_respRateRule.d_minTime = now;
   d_respRateRule.d_cutOff.tv_sec -= d_respRateRule.d_seconds;
+  if (d_respRateRule.d_cutOff < responseCutOff) {
+    responseCutOff = d_respRateRule.d_cutOff;
+  }
 
   d_suffixMatchRule.d_cutOff = d_suffixMatchRule.d_minTime = now;
   d_suffixMatchRule.d_cutOff.tv_sec -= d_suffixMatchRule.d_seconds;
+  if (d_suffixMatchRule.d_cutOff < responseCutOff) {
+    responseCutOff = d_suffixMatchRule.d_cutOff;
+  }
 
   for (auto& rule : d_rcodeRules) {
     rule.second.d_cutOff = rule.second.d_minTime = now;
     rule.second.d_cutOff.tv_sec -= rule.second.d_seconds;
+    if (rule.second.d_cutOff < responseCutOff) {
+      responseCutOff = rule.second.d_cutOff;
+    }
   }
 
   for (auto& rule : d_rcodeRatioRules) {
     rule.second.d_cutOff = rule.second.d_minTime = now;
     rule.second.d_cutOff.tv_sec -= rule.second.d_seconds;
+    if (rule.second.d_cutOff < responseCutOff) {
+      responseCutOff = rule.second.d_cutOff;
+    }
   }
 
   for (const auto& shard : g_rings.d_shards) {
@@ -316,8 +336,13 @@ void DynBlockRulesGroup::processResponseRules(counts_t& counts, StatNode& root,
         continue;
       }
 
+      if (c.when < responseCutOff) {
+        continue;
+      }
+
       auto& entry = counts[c.requestor];
-      ++entry.queries;
+      ++entry.responses;
+
       bool respRateMatches = d_respRateRule.matches(c.when);
       bool suffixMatchRuleMatches = d_suffixMatchRule.matches(c.when);
       bool rcodeRuleMatches = checkIfResponseCodeMatches(c);
@@ -375,20 +400,20 @@ void DynBlockMaintenance::purgeExpired(const struct timespec& now)
   }
 }
 
-std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> DynBlockMaintenance::getTopNetmasks()
+std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> DynBlockMaintenance::getTopNetmasks(size_t topN)
 {
   std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> results;
-  if (s_topN == 0) {
+  if (topN == 0) {
     return results;
   }
 
   auto blocks = g_dynblockNMG.getLocal();
   for (const auto& entry : *blocks) {
     auto& topsForReason = results[entry.second.reason];
-    if (topsForReason.size() < s_topN || topsForReason.front().second < entry.second.blocks) {
+    if (topsForReason.size() < topN || topsForReason.front().second < entry.second.blocks) {
       auto newEntry = std::make_pair(entry.first, entry.second.blocks.load());
 
-      if (topsForReason.size() >= s_topN) {
+      if (topsForReason.size() >= topN) {
         topsForReason.pop_front();
       }
 
@@ -402,20 +427,20 @@ std::map<std::string, std::list<std::pair<Netmask, unsigned int>>> DynBlockMaint
   return results;
 }
 
-std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> DynBlockMaintenance::getTopSuffixes()
+std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> DynBlockMaintenance::getTopSuffixes(size_t topN)
 {
   std::map<std::string, std::list<std::pair<DNSName, unsigned int>>> results;
-  if (s_topN == 0) {
+  if (topN == 0) {
     return results;
   }
 
   auto blocks = g_dynblockSMT.getLocal();
-  blocks->visit([&results](const SuffixMatchTree<DynBlock>& node) {
+  blocks->visit([&results, topN](const SuffixMatchTree<DynBlock>& node) {
     auto& topsForReason = results[node.d_value.reason];
-    if (topsForReason.size() < DynBlockMaintenance::s_topN || topsForReason.front().second < node.d_value.blocks) {
+    if (topsForReason.size() < topN || topsForReason.front().second < node.d_value.blocks) {
       auto newEntry = std::make_pair(node.d_value.domain, node.d_value.blocks.load());
 
-      if (topsForReason.size() >= DynBlockMaintenance::s_topN) {
+      if (topsForReason.size() >= topN) {
         topsForReason.pop_front();
       }
 
@@ -445,8 +470,10 @@ time_t DynBlockMaintenance::s_expiredDynBlocksPurgeInterval{300};
 void DynBlockMaintenance::collectMetrics()
 {
   MetricsSnapshot snapshot;
-  snapshot.smtData = getTopSuffixes();
-  snapshot.nmgData = getTopNetmasks();
+  /* over sampling to get entries that are not in the top N
+     every time a chance to be at the end */
+  snapshot.smtData = getTopSuffixes(s_topN * 5);
+  snapshot.nmgData = getTopNetmasks(s_topN * 5);
 
   {
     std::lock_guard<std::mutex> lock(s_topsMutex);

diff --git a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
index cb074855d8f81b453dea6c3c5e6260a2784c16de..caee5adcff94b356f684dca202511bbc832a0f00 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
+++ b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
@@ -714,7 +714,7 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) {
     /* now we ask for the top 20 offenders for each reason */
     StopWatch sw;
     sw.start();
-    auto top = DynBlockMaintenance::getTopNetmasks();
+    auto top = DynBlockMaintenance::getTopNetmasks(20);
     BOOST_REQUIRE_EQUAL(top.size(), 1U);
     auto offenders = top.at(reason);
     BOOST_REQUIRE_EQUAL(offenders.size(), 20U);
@@ -770,7 +770,7 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) {
     /* now we ask for the top 20 offenders for each reason */
     StopWatch sw;
     sw.start();
-    auto top = DynBlockMaintenance::getTopSuffixes();
+    auto top = DynBlockMaintenance::getTopSuffixes(20);
     BOOST_REQUIRE_EQUAL(top.size(), 1U);
     auto suffixes = top.at(reason);
     BOOST_REQUIRE_EQUAL(suffixes.size(), 20U);
@@ -825,7 +825,7 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) {
     cerr<<"added 1000000 entries in "<<std::to_string(sw.udiff()/1024)<<"ms"<<endl;
 
     sw.start();
-    auto top = DynBlockMaintenance::getTopSuffixes();
+    auto top = DynBlockMaintenance::getTopSuffixes(20);
     cerr<<"scanned 1000000 entries in "<<std::to_string(sw.udiff()/1024)<<"ms"<<endl;
 
     struct timespec expired = now;
@@ -868,7 +868,7 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) {
     BOOST_CHECK_EQUAL(g_dynblockNMG.getLocal()->size(), 1000000U);
 
     sw.start();
-    auto top = DynBlockMaintenance::getTopNetmasks();
+    auto top = DynBlockMaintenance::getTopNetmasks(20);
     cerr<<"scanned "<<g_dynblockNMG.getLocal()->size()<<" entries in "<<std::to_string(sw.udiff()/1024)<<"ms"<<endl;
 
     struct timespec expired = now;