From: Remi Gacogne Date: Fri, 20 Nov 2020 15:29:56 +0000 (+0100) Subject: dnsdist: Add metrics for Dynamic Blocks entries X-Git-Tag: rec-4.5.0-alpha1~92^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4244240f9660765f804e7c128ab0568f92d00bf;p=thirdparty%2Fpdns.git dnsdist: Add metrics for Dynamic Blocks entries --- diff --git a/pdns/bpf-filter.cc b/pdns/bpf-filter.cc index 56418c2813..96b7acdc5e 100644 --- a/pdns/bpf-filter.cc +++ b/pdns/bpf-filter.cc @@ -36,7 +36,7 @@ static __u64 ptr_to_u64(void *ptr) } int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size, - int max_entries) + int max_entries) { union bpf_attr attr; memset(&attr, 0, sizeof(attr)); @@ -214,7 +214,7 @@ void BPFFilter::removeSocket(int sock) void BPFFilter::block(const ComboAddress& addr) { - std::unique_lock lock(d_mutex); + std::lock_guard lock(d_mutex); uint64_t counter = 0; int res = 0; @@ -263,7 +263,7 @@ void BPFFilter::block(const ComboAddress& addr) void BPFFilter::unblock(const ComboAddress& addr) { - std::unique_lock lock(d_mutex); + std::lock_guard lock(d_mutex); int res = 0; if (addr.sin4.sin_family == AF_INET) { @@ -307,7 +307,7 @@ void BPFFilter::block(const DNSName& qname, uint16_t qtype) memcpy(key.qname, keyStr.c_str(), keyStr.size()); { - std::unique_lock lock(d_mutex); + std::lock_guard lock(d_mutex); if (d_qNamesCount >= d_maxQNames) { throw std::runtime_error("Table full when trying to block " + qname.toLogString()); } @@ -340,7 +340,7 @@ void BPFFilter::unblock(const DNSName& qname, uint16_t qtype) memcpy(key.qname, keyStr.c_str(), keyStr.size()); { - std::unique_lock lock(d_mutex); + std::lock_guard lock(d_mutex); int res = bpf_delete_elem(d_qnamemap.fd, &key); if (res == 0) { @@ -355,7 +355,7 @@ void BPFFilter::unblock(const DNSName& qname, uint16_t qtype) std::vector > BPFFilter::getAddrStats() { std::vector > result; - std::unique_lock lock(d_mutex); + std::lock_guard lock(d_mutex); uint32_t v4Key = 0; uint32_t nextV4Key; @@ -404,7 +404,7 @@ std::vector > BPFFilter::getAddrStats() std::vector > BPFFilter::getQNameStats() { std::vector > result; - std::unique_lock lock(d_mutex); + std::lock_guard lock(d_mutex); struct QNameKey key = { { 0 } }; struct QNameKey nextKey = { { 0 } }; diff --git a/pdns/dnsdist-dynblocks.hh b/pdns/dnsdist-dynblocks.hh index 31bc9e7964..5e23d90025 100644 --- a/pdns/dnsdist-dynblocks.hh +++ b/pdns/dnsdist-dynblocks.hh @@ -314,6 +314,8 @@ public: d_beQuiet = quiet; } + void purgeExpired(const struct timespec& now); + private: bool checkIfQueryTypeMatches(const Rings::Query& query); @@ -366,3 +368,27 @@ private: dnsdist_ffi_stat_node_visitor_t d_smtVisitorFFI; bool d_beQuiet{false}; }; + +class DynBlockRulesMetricsCache +{ +public: + DynBlockRulesMetricsCache(size_t topN, unsigned int validity): d_validityPeriod(validity), d_topN(topN) + { + } + + std::map>> getTopNetmasks(); + std::map>> getTopSuffixes(); + void invalidate(); + void setParameters(size_t topN, unsigned int validity); + +private: + std::map>> d_cachedNetmasks; + std::map>> d_cachedSuffixes; + std::mutex d_mutex; + time_t d_netmasksValidUntil{0}; + time_t d_suffixesValidUntil{0}; + unsigned int d_validityPeriod{0}; + size_t d_topN{0}; +}; + +extern DynBlockRulesMetricsCache g_dynBlocksMetricsCache; diff --git a/pdns/dnsdist-dynbpf.cc b/pdns/dnsdist-dynbpf.cc index bdaa3c578a..74f78f12ed 100644 --- a/pdns/dnsdist-dynbpf.cc +++ b/pdns/dnsdist-dynbpf.cc @@ -26,7 +26,7 @@ bool DynBPFFilter::block(const ComboAddress& addr, const struct timespec& until) { bool inserted = false; - std::unique_lock lock(d_mutex); + std::lock_guard lock(d_mutex); if (d_excludedSubnets.match(addr)) { /* do not add a block for excluded subnets */ @@ -49,7 +49,7 @@ bool DynBPFFilter::block(const ComboAddress& addr, const struct timespec& until) void DynBPFFilter::purgeExpired(const struct timespec& now) { - std::unique_lock lock(d_mutex); + std::lock_guard lock(d_mutex); typedef nth_index::type ordered_until; ordered_until& ou = get<1>(d_entries); @@ -74,6 +74,7 @@ std::vector > DynBPFFilter:: } const auto& stats = d_bpf->getAddrStats(); + result.reserve(stats.size()); for (const auto& stat : stats) { const container_t::iterator it = d_entries.find(stat.first); if (it != d_entries.end()) { diff --git a/pdns/dnsdist-dynbpf.hh b/pdns/dnsdist-dynbpf.hh index 521006bd3d..cd69c5d13e 100644 --- a/pdns/dnsdist-dynbpf.hh +++ b/pdns/dnsdist-dynbpf.hh @@ -43,10 +43,12 @@ public: } void excludeRange(const Netmask& range) { + std::unique_lock lock(d_mutex); d_excludedSubnets.addMask(range); } void includeRange(const Netmask& range) { + std::unique_lock lock(d_mutex); d_excludedSubnets.addMask(range, false); } /* returns true if the addr wasn't already blocked, false otherwise */ diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc index 752babf151..a890e35217 100644 --- a/pdns/dnsdist-lua.cc +++ b/pdns/dnsdist-lua.cc @@ -1235,7 +1235,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) db.blocks=count; if(!got || expired) warnlog("Inserting dynamic block for %s for %d seconds: %s", domain, actualSeconds, msg); - slow.add(domain, db); + slow.add(domain, std::move(db)); } g_dynblockSMT.setState(slow); }); diff --git a/pdns/dnsdist-web.cc b/pdns/dnsdist-web.cc index 0816460262..dcd56b2201 100644 --- a/pdns/dnsdist-web.cc +++ b/pdns/dnsdist-web.cc @@ -32,6 +32,7 @@ #include "base64.hh" #include "dnsdist.hh" +#include "dnsdist-dynblocks.hh" #include "dnsdist-healthchecks.hh" #include "dnsdist-prometheus.hh" #include "dnsdist-web.hh" @@ -667,6 +668,24 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp) addRulesToPrometheusOutput(output, g_cachehitresprulactions); addRulesToPrometheusOutput(output, g_selfansweredresprulactions); + output << "# HELP dnsdist_dynblocks_nmg_top_offenders " << "Top offenders blocked by Dynamic Blocks (netmasks)" << "\n"; + output << "# TYPE dnsdist_dynblocks_nmg_top_offenders " << "gauge" << "\n"; + auto topNetmasksByReason = g_dynBlocksMetricsCache.getTopNetmasks(); + for (const auto& entry : topNetmasksByReason) { + for (const auto& netmask : entry.second) { + output << "dnsdist_dynblocks_nmg_top_offenders{reason=\"" << entry.first << "\",netmask=\"" << netmask.first.toString() << "\"} " << netmask.second << "\n"; + } + } + + output << "# HELP dnsdist_dynblocks_smt_top_offenders " << "Top offenders blocked by Dynamic Blocks (suffixes)" << "\n"; + output << "# TYPE dnsdist_dynblocks_smt_top_offenders " << "gauge" << "\n"; + auto topSuffixesByReason = g_dynBlocksMetricsCache.getTopSuffixes(); + for (const auto& entry : topSuffixesByReason) { + for (const auto& suffix : entry.second) { + output << "dnsdist_dynblocks_smt_top_offenders{reason=\"" << entry.first << "\",suffix=\"" << suffix.first.toString() << "\"} " << suffix.second << "\n"; + } + } + output << "# HELP dnsdist_info " << "Info from dnsdist, value is always 1" << "\n"; output << "# TYPE dnsdist_info " << "gauge" << "\n"; output << "dnsdist_info{version=\"" << VERSION << "\"} " << "1" << "\n"; diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index 88b347fe54..81c13458f6 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -187,7 +187,7 @@ public: class DNSResponseAction { public: - enum class Action { Allow, Delay, Drop, HeaderModify, ServFail, None }; + enum class Action : uint8_t { Allow, Delay, Drop, HeaderModify, ServFail, None }; virtual Action operator()(DNSResponse*, string* ruleresult) const =0; virtual ~DNSResponseAction() { @@ -199,34 +199,52 @@ struct DynBlock { DynBlock(): action(DNSAction::Action::None), warning(false) { + until.tv_sec = 0; + until.tv_nsec = 0; } - DynBlock(const std::string& reason_, const struct timespec& until_, const DNSName& domain_, DNSAction::Action action_): reason(reason_), until(until_), domain(domain_), action(action_), warning(false) + DynBlock(const std::string& reason_, const struct timespec& until_, const DNSName& domain_, DNSAction::Action action_): reason(reason_), domain(domain_), until(until_), action(action_), warning(false) { } - DynBlock(const DynBlock& rhs): reason(rhs.reason), until(rhs.until), domain(rhs.domain), action(rhs.action), warning(rhs.warning) + DynBlock(const DynBlock& rhs): reason(rhs.reason), domain(rhs.domain), until(rhs.until), action(rhs.action), warning(rhs.warning) + { + blocks.store(rhs.blocks); + } + + DynBlock(DynBlock&& rhs): reason(std::move(rhs.reason)), domain(std::move(rhs.domain)), until(rhs.until), action(rhs.action), warning(rhs.warning) { blocks.store(rhs.blocks); } DynBlock& operator=(const DynBlock& rhs) { - reason=rhs.reason; - until=rhs.until; - domain=rhs.domain; - action=rhs.action; + reason = rhs.reason; + until = rhs.until; + domain = rhs.domain; + action = rhs.action; + blocks.store(rhs.blocks); + warning = rhs.warning; + return *this; + } + + DynBlock& operator=(DynBlock&& rhs) + { + reason = std::move(rhs.reason); + until = rhs.until; + domain = std::move(rhs.domain); + action = rhs.action; blocks.store(rhs.blocks); - warning=rhs.warning; + warning = rhs.warning; return *this; } string reason; - struct timespec until; DNSName domain; - DNSAction::Action action; + struct timespec until; mutable std::atomic blocks; - bool warning; + DNSAction::Action action{DNSAction::Action::None}; + bool warning{false}; }; extern GlobalStateHolder> g_dynblockNMG; @@ -1154,8 +1172,6 @@ struct LocalHolders LocalStateHolder pools; }; -struct dnsheader; - vector> setupLua(bool client, const std::string& config); void tcpAcceptorThread(ClientState* p); diff --git a/pdns/dnsdistdist/dnsdist-dynblocks.cc b/pdns/dnsdistdist/dnsdist-dynblocks.cc index 808d56b259..65ae642a24 100644 --- a/pdns/dnsdistdist/dnsdist-dynblocks.cc +++ b/pdns/dnsdistdist/dnsdist-dynblocks.cc @@ -224,7 +224,6 @@ void DynBlockRulesGroup::addOrRefreshBlockSMT(SuffixMatchTree& blocks, unsigned int count = 0; const auto& got = blocks.lookup(name); bool expired = false; - DNSName domain(name.makeLowerCase()); if (got) { if (until < got->until) { @@ -241,13 +240,13 @@ void DynBlockRulesGroup::addOrRefreshBlockSMT(SuffixMatchTree& blocks, } } - DynBlock db{rule.d_blockReason, until, domain, rule.d_action}; + DynBlock db{rule.d_blockReason, until, name.makeLowerCase(), rule.d_action}; db.blocks = count; if (!d_beQuiet && (!got || expired)) { - warnlog("Inserting dynamic block for %s for %d seconds: %s", domain, rule.d_blockDuration, rule.d_blockReason); + warnlog("Inserting dynamic block for %s for %d seconds: %s", name, rule.d_blockDuration, rule.d_blockReason); } - blocks.add(domain, db); + blocks.add(name, std::move(db)); updated = true; } @@ -338,3 +337,132 @@ void DynBlockRulesGroup::processResponseRules(counts_t& counts, StatNode& root, } } } + +void DynBlockRulesGroup::purgeExpired(const struct timespec& now) +{ + { + auto blocks = g_dynblockNMG.getLocal(); + std::vector toRemove; + for (const auto& entry : *blocks) { + if (!(now < entry.second.until)) { + toRemove.push_back(entry.first); + } + } + if (!toRemove.empty()) { + auto updated = g_dynblockNMG.getCopy(); + for (const auto& entry : toRemove) { + updated.erase(entry); + } + g_dynblockNMG.setState(std::move(updated)); + } + } + + { + std::vector toRemove; + auto blocks = g_dynblockSMT.getLocal(); + blocks->visit([&toRemove, now](const SuffixMatchTree& node) { + if (!(now < node.d_value.until)) { + toRemove.push_back(node.d_value.domain); + } + }); + if (!toRemove.empty()) { + auto updated = g_dynblockSMT.getCopy(); + for (const auto& entry : toRemove) { + updated.remove(entry); + } + g_dynblockSMT.setState(std::move(updated)); + } + } +} + +std::map>> DynBlockRulesMetricsCache::getTopNetmasks() +{ + std::map>> results; + if (d_topN == 0) { + return results; + } + + time_t now = time(nullptr); + { + std::lock_guard lock(d_mutex); + if (now < d_netmasksValidUntil) { + return d_cachedNetmasks; + } + + auto blocks = g_dynblockNMG.getLocal(); + for (const auto& entry : *blocks) { + auto& topsForReason = results[entry.second.reason]; + if (topsForReason.size() < d_topN || topsForReason.front().second < entry.second.blocks) { + auto newEntry = std::make_pair(entry.first, entry.second.blocks.load()); + + if (topsForReason.size() >= d_topN) { + topsForReason.pop_front(); + } + + topsForReason.insert(std::lower_bound(topsForReason.begin(), topsForReason.end(), newEntry, [](const std::pair& a, const std::pair& b) { + return a.second < b.second; + }), + newEntry); + } + } + d_cachedNetmasks = results; + d_netmasksValidUntil = time(nullptr) + d_validityPeriod; + } + + return results; +} + +std::map>> DynBlockRulesMetricsCache::getTopSuffixes() +{ + std::map>> results; + if (d_topN == 0) { + return results; + } + + time_t now = time(nullptr); + { + std::lock_guard lock(d_mutex); + if (now < d_suffixesValidUntil) { + return d_cachedSuffixes; + } + + auto blocks = g_dynblockSMT.getLocal(); + blocks->visit([&results, this](const SuffixMatchTree& node) { + auto& topsForReason = results[node.d_value.reason]; + if (topsForReason.size() < d_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() >= d_topN) { + topsForReason.pop_front(); + } + + topsForReason.insert(std::lower_bound(topsForReason.begin(), topsForReason.end(), newEntry, [](const std::pair& a, const std::pair& b) { + return a.second < b.second; + }), + newEntry); + } + }); + d_cachedSuffixes = results; + d_suffixesValidUntil = time(nullptr) + d_validityPeriod; + } + + return results; +} + +DynBlockRulesMetricsCache g_dynBlocksMetricsCache(20, 60); + +void DynBlockRulesMetricsCache::invalidate() +{ + std::lock_guard lock(d_mutex); + d_netmasksValidUntil = 0; + d_suffixesValidUntil = 0; + d_cachedNetmasks.clear(); + d_cachedSuffixes.clear(); +} + +void DynBlockRulesMetricsCache::setParameters(size_t topN, unsigned int validity) +{ + d_validityPeriod = validity; + d_topN = topN; + invalidate(); +} diff --git a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc index b5529c87df..d950fac2c2 100644 --- a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc +++ b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc @@ -661,4 +661,228 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_Ranges) { } +BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) { + dnsheader dh; + memset(&dh, 0, sizeof(dh)); + DNSName qname("rings.powerdns.com."); + uint16_t qtype = QType::AAAA; + uint16_t size = 42; + struct timespec now; + gettime(&now); + NetmaskTree emptyNMG; + SuffixMatchTree emptySMT; + + size_t numberOfSeconds = 10; + size_t blockDuration = 60; + const auto action = DNSAction::Action::Drop; + const std::string reason = "Exceeded query rate"; + + /* 10M entries, only one shard */ + g_rings.setCapacity(10000000, 1); + + { + DynBlockRulesGroup dbrg; + dbrg.setQuiet(true); + g_rings.clear(); + g_dynblockNMG.setState(emptyNMG); + + /* block above 0 qps for numberOfSeconds seconds, no warning */ + dbrg.setQueryRate(0, 0, numberOfSeconds, reason, blockDuration, action); + + /* insert one fake query from 255 clients: + */ + for (size_t idx = 0; idx < 256; idx++) { + const ComboAddress requestor("192.0.2." + std::to_string(idx)); + g_rings.insertQuery(now, requestor, qname, qtype, size, dh); + } + + /* we apply the rules, all clients should be blocked */ + dbrg.apply(now); + BOOST_CHECK_EQUAL(g_dynblockNMG.getLocal()->size(), 256U); + + for (size_t idx = 0; idx < 256; idx++) { + const ComboAddress requestor("192.0.2." + std::to_string(idx)); + const auto& block = g_dynblockNMG.getLocal()->lookup(requestor)->second; + /* simulate that: + - .1 does 1 query + ... + - .255 does 255 queries + */ + block.blocks = idx; + } + + /* now we ask for the top 20 offenders for each reason */ + StopWatch sw; + sw.start(); + DynBlockRulesMetricsCache cache(20, 1); + auto top = cache.getTopNetmasks(); + BOOST_REQUIRE_EQUAL(top.size(), 1U); + auto offenders = top.at(reason); + BOOST_REQUIRE_EQUAL(offenders.size(), 20U); + auto it = offenders.begin(); + for (size_t idx = 236; idx < 256; idx++) { + BOOST_CHECK_EQUAL(it->first.toString(), Netmask(ComboAddress("192.0.2." + std::to_string(idx))).toString()); + BOOST_CHECK_EQUAL(it->second, idx); + ++it; + } + + struct timespec expired = now; + expired.tv_sec += blockDuration + 1; + dbrg.purgeExpired(expired); + BOOST_CHECK_EQUAL(g_dynblockNMG.getLocal()->size(), 0U); + } + + { + /* === reset everything for SMT === */ + DynBlockRulesGroup dbrg; + dbrg.setQuiet(true); + g_rings.clear(); + g_dynblockNMG.setState(emptyNMG); + g_dynblockSMT.setState(emptySMT); + + dbrg.setSuffixMatchRule(numberOfSeconds, reason, blockDuration, action, [](const StatNode& node, const StatNode::Stat& self, const StatNode::Stat& children) { + if (self.queries > 0) { + return true; + } + return false; + }); + + /* insert one fake response for 255 DNS names */ + const ComboAddress requestor("192.0.2.1"); + for (size_t idx = 0; idx < 256; idx++) { + g_rings.insertResponse(now, requestor, DNSName(std::to_string(idx)) + qname, qtype, 1000 /*usec*/, size, dh, requestor /* backend, technically, but we don't care */); + } + + /* we apply the rules, all suffixes should be blocked */ + dbrg.apply(now); + + for (size_t idx = 0; idx < 256; idx++) { + const DNSName name(DNSName(std::to_string(idx)) + qname); + const auto* block = g_dynblockSMT.getLocal()->lookup(name); + BOOST_REQUIRE(block != nullptr); + /* simulate that: + - 1.rings.powerdns.com. got 1 query + ... + - 255. does 255 queries + */ + block->blocks = idx; + } + + /* now we ask for the top 20 offenders for each reason */ + StopWatch sw; + sw.start(); + DynBlockRulesMetricsCache cache(20, 1); + auto top = cache.getTopSuffixes(); + BOOST_REQUIRE_EQUAL(top.size(), 1U); + auto suffixes = top.at(reason); + BOOST_REQUIRE_EQUAL(suffixes.size(), 20U); + auto it = suffixes.begin(); + for (size_t idx = 236; idx < 256; idx++) { + BOOST_CHECK_EQUAL(it->first, (DNSName(std::to_string(idx)) + qname)); + BOOST_CHECK_EQUAL(it->second, idx); + ++it; + } + + struct timespec expired = now; + expired.tv_sec += blockDuration + 1; + dbrg.purgeExpired(expired); + BOOST_CHECK(g_dynblockSMT.getLocal()->getNodes().empty()); + } + +#ifdef BENCH_DYNBLOCKS + { + /* now insert 1M names */ + DynBlockRulesGroup dbrg; + dbrg.setQuiet(true); + g_rings.clear(); + g_dynblockNMG.setState(emptyNMG); + g_dynblockSMT.setState(emptySMT); + + dbrg.setSuffixMatchRule(numberOfSeconds, reason, blockDuration, action, [](const StatNode& node, const StatNode::Stat& self, const StatNode::Stat& children) { + if (self.queries > 0) { + return true; + } + return false; + }); + + bool done = false; + const ComboAddress requestor("192.0.2.1"); + for (size_t idxB = 0; !done && idxB < 256; idxB++) { + for (size_t idxC = 0; !done && idxC < 256; idxC++) { + for (size_t idxD = 0; !done && idxD < 256; idxD++) { + const DNSName victim(std::to_string(idxB) + "." + std::to_string(idxC) + "." + std::to_string(idxD) + qname.toString()); + g_rings.insertResponse(now, requestor, victim, qtype, 1000 /*usec*/, size, dh, requestor /* backend, technically, but we don't care */); + if (g_rings.getNumberOfQueryEntries() == 1000000) { + done = true; + break; + } + } + } + } + + /* we apply the rules, all suffixes should be blocked */ + StopWatch sw; + sw.start(); + dbrg.apply(now); + cerr<<"added 1000000 entries in "<size()<<" entries in "<size(), 1000000U); + + sw.start(); + DynBlockRulesMetricsCache cache(20, 1); + auto top = cache.getTopNetmasks(); + cerr<<"scanned "<size()<<" entries in "<size(), 0U); + } +#endif +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh index 7f7ee0ad3c..ab1bb8680e 100644 --- a/pdns/dnsname.hh +++ b/pdns/dnsname.hh @@ -288,50 +288,55 @@ struct SuffixMatchTree template void visit(const V& v) const { - for(const auto& c : children) + for(const auto& c : children) { c.visit(v); - if(endNode) + } + + if (endNode) { v(*this); + } } - void add(const DNSName& name, const T& t) + void add(const DNSName& name, T&& t) { - add(name.getRawLabels(), t); + auto labels = name.getRawLabels(); + add(labels, std::move(t)); } - void add(std::vector labels, const T& value) const + void add(std::vector& labels, T&& value) const { - if(labels.empty()) { // this allows insertion of the root - endNode=true; - d_value=value; + if (labels.empty()) { // this allows insertion of the root + endNode = true; + d_value = std::move(value); } else if(labels.size()==1) { - auto res=children.emplace(*labels.begin(), true); - if(!res.second) { + auto res = children.emplace(*labels.begin(), true); + if (!res.second) { // we might already have had the node as an // intermediary one, but it's now an end node - if(!res.first->endNode) { + if (!res.first->endNode) { res.first->endNode = true; } } - res.first->d_value = value; + res.first->d_value = std::move(value); } else { - auto res=children.emplace(*labels.rbegin(), false); + auto res = children.emplace(*labels.rbegin(), false); labels.pop_back(); - res.first->add(labels, value); + res.first->add(labels, std::move(value)); } } void remove(const DNSName &name) const { - remove(name.getRawLabels()); + auto labels = name.getRawLabels(); + remove(labels); } /* Removes the node at `labels`, also make sure that no empty * children will be left behind in memory */ - void remove(std::vector labels) const + void remove(std::vector& labels) const { if (labels.empty()) { // this allows removal of the root endNode = false; @@ -364,28 +369,32 @@ struct SuffixMatchTree T* lookup(const DNSName& name) const { - if(children.empty()) { // speed up empty set - if(endNode) + if (children.empty()) { // speed up empty set + if (endNode) { return &d_value; + } return nullptr; } - return lookup(name.getRawLabels()); + auto labels = name.getRawLabels(); + return lookup(labels); } - T* lookup(std::vector labels) const + T* lookup(std::vector& labels) const { - if(labels.empty()) { // optimization - if(endNode) + if (labels.empty()) { // optimization + if (endNode) { return &d_value; + } return nullptr; } SuffixMatchTree smn(*labels.rbegin()); auto child = children.find(smn); - if(child == children.end()) { - if(endNode) + if (child == children.end()) { + if(endNode) { return &d_value; + } return nullptr; } labels.pop_back(); @@ -404,6 +413,7 @@ struct SuffixMatchTree } for (const auto& child : children) { auto nodes = child.getNodes(); + ret.reserve(ret.size() + nodes.size()); for (const auto &node: nodes) { ret.push_back(node + DNSName(d_name)); } diff --git a/pdns/statnode.cc b/pdns/statnode.cc index 00dad2afbd..e4c8f9edb5 100644 --- a/pdns/statnode.cc +++ b/pdns/statnode.cc @@ -34,26 +34,17 @@ StatNode::Stat StatNode::print(unsigned int depth, Stat newstat, bool silent) co } -void StatNode::visit(visitor_t visitor, Stat &newstat, unsigned int depth) const +void StatNode::visit(visitor_t visitor, Stat &newstat, unsigned int depth) const { - Stat childstat; - childstat.queries += s.queries; - childstat.noerrors += s.noerrors; - childstat.nxdomains += s.nxdomains; - childstat.servfails += s.servfails; - childstat.drops += s.drops; - childstat.bytes += s.bytes; - childstat.remotes = s.remotes; - - Stat selfstat(childstat); + Stat childstat(s); - for(const children_t::value_type& child : children) { + for (const auto& child : children) { child.second.visit(visitor, childstat, depth+8); } - visitor(this, selfstat, childstat); + visitor(this, s, childstat); - newstat+=childstat; + newstat += childstat; } @@ -61,8 +52,9 @@ void StatNode::submit(const DNSName& domain, int rcode, unsigned int bytes, boos { // cerr<<"FIRST submit called on '"< tmp = domain.getRawLabels(); - if(tmp.empty()) + if (tmp.empty()) { return; + } auto last = tmp.end() - 1; children[*last].submit(last, tmp.begin(), "", rcode, bytes, remote, 1); @@ -81,7 +73,7 @@ void StatNode::submit(std::vector::const_iterator end, std::vector::const_iterator end, std::vector::const_iterator end, std::vector::const_iterator end, std::vector #include #include "iputils.hh" @@ -56,7 +55,7 @@ public: Stat s; std::string name; std::string fullname; - unsigned int labelsCount{0}; + uint8_t labelsCount{0}; void submit(const DNSName& domain, int rcode, unsigned int bytes, boost::optional remote); diff --git a/pdns/test-dnsname_cc.cc b/pdns/test-dnsname_cc.cc index 9c1890de8a..c96baef50d 100644 --- a/pdns/test-dnsname_cc.cc +++ b/pdns/test-dnsname_cc.cc @@ -523,9 +523,10 @@ BOOST_AUTO_TEST_CASE(test_suffixmatch) { BOOST_AUTO_TEST_CASE(test_suffixmatch_tree) { SuffixMatchTree smt; DNSName ezdns("ezdns.it."); - smt.add(ezdns, ezdns); + smt.add(ezdns, DNSName(ezdns)); - smt.add(DNSName("org.").getRawLabels(), DNSName("org.")); + auto labels = DNSName("org.").getRawLabels(); + smt.add(labels, DNSName("org.")); DNSName wwwpowerdnscom("www.powerdns.com."); DNSName wwwezdnsit("www.ezdns.it."); @@ -548,14 +549,14 @@ BOOST_AUTO_TEST_CASE(test_suffixmatch_tree) { BOOST_CHECK(smt.lookup(DNSName("images.bbc.co.uk.")) == nullptr); BOOST_CHECK(smt.lookup(DNSName("www.news.gov.uk.")) == nullptr); - smt.add(g_rootdnsname, g_rootdnsname); // block the root + smt.add(g_rootdnsname, DNSName(g_rootdnsname)); // block the root BOOST_REQUIRE(smt.lookup(DNSName("a.root-servers.net."))); BOOST_CHECK_EQUAL(*smt.lookup(DNSName("a.root-servers.net.")), g_rootdnsname); DNSName apowerdnscom("a.powerdns.com."); DNSName bpowerdnscom("b.powerdns.com."); - smt.add(apowerdnscom, apowerdnscom); - smt.add(bpowerdnscom, bpowerdnscom); + smt.add(apowerdnscom, DNSName(apowerdnscom)); + smt.add(bpowerdnscom, DNSName(bpowerdnscom)); BOOST_REQUIRE(smt.lookup(apowerdnscom)); BOOST_CHECK_EQUAL(*smt.lookup(apowerdnscom), apowerdnscom); BOOST_REQUIRE(smt.lookup(bpowerdnscom)); @@ -563,8 +564,8 @@ BOOST_AUTO_TEST_CASE(test_suffixmatch_tree) { DNSName examplenet("example.net."); DNSName net("net."); - smt.add(examplenet, examplenet); - smt.add(net, net); + smt.add(examplenet, DNSName(examplenet)); + smt.add(net, DNSName(net)); BOOST_REQUIRE(smt.lookup(examplenet)); BOOST_CHECK_EQUAL(*smt.lookup(examplenet), examplenet); BOOST_REQUIRE(smt.lookup(net)); @@ -577,10 +578,10 @@ BOOST_AUTO_TEST_CASE(test_suffixmatch_tree) { BOOST_CHECK_EQUAL(*smt.lookup(examplenet), examplenet); smt = SuffixMatchTree(); - smt.add(examplenet, examplenet); - smt.add(net, net); + smt.add(examplenet, DNSName(examplenet)); + smt.add(net, DNSName(net)); smt.add(DNSName("news.bbc.co.uk."), DNSName("news.bbc.co.uk.")); - smt.add(apowerdnscom, apowerdnscom); + smt.add(apowerdnscom, DNSName(apowerdnscom)); smt.remove(DNSName("not-such-entry.news.bbc.co.uk.")); BOOST_REQUIRE(smt.lookup(DNSName("news.bbc.co.uk."))); @@ -596,8 +597,8 @@ BOOST_AUTO_TEST_CASE(test_suffixmatch_tree) { BOOST_CHECK(smt.lookup(net) == nullptr); BOOST_CHECK(smt.lookup(examplenet) == nullptr); - smt.add(examplenet, examplenet); - smt.add(net, net); + smt.add(examplenet, DNSName(examplenet)); + smt.add(net, DNSName(net)); BOOST_REQUIRE(smt.lookup(examplenet)); BOOST_CHECK_EQUAL(*smt.lookup(examplenet), examplenet); BOOST_REQUIRE(smt.lookup(net));