From: Otto Moerbeek Date: Wed, 17 Aug 2022 12:22:34 +0000 (+0200) Subject: Approach: each thread gets its own copy of the PorxyMapping config, which X-Git-Tag: rec-4.9.0-alpha0~22^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c375521b2f8da121e06368866658e6d9870bee58;p=thirdparty%2Fpdns.git Approach: each thread gets its own copy of the PorxyMapping config, which includes the counters. When we have a proxymapping match, we immediately have the counters available without the need for a new lookup. When reloading, make sure the old counters of lines present in the new config are preserved. Don't forget to count on PC cache hits. --- diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index 4469695996..be3792a620 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -963,6 +963,9 @@ void startDoResolve(void* p) // No match in domains, use original source useMapped = false; } + else { + ++it->second.stats.suffixMatches; + } } // No suffix match node defined, use mapped address } @@ -1812,7 +1815,7 @@ bool checkForCacheHit(bool qnameParsed, unsigned int tag, const string& data, DNSName& qname, uint16_t& qtype, uint16_t& qclass, const struct timeval& now, string& response, uint32_t& qhash, - RecursorPacketCache::OptPBData& pbData, bool tcp, const ComboAddress& source) + RecursorPacketCache::OptPBData& pbData, bool tcp, const ComboAddress& source, const ComboAddress& mappedSource) { if (!t_packetCache) { return false; @@ -1838,6 +1841,17 @@ bool checkForCacheHit(bool qnameParsed, unsigned int tag, const string& data, } } + // This is only to get the proxyMapping suffixMatch stats right i the case of a PC hit + if (t_proxyMapping && source != mappedSource) { + if (auto it = t_proxyMapping->lookup(source)) { + if (it->second.suffixMatchNode) { + if (it->second.suffixMatchNode->check(qname)) { + ++it->second.stats.suffixMatches; + } + } + } + } + g_stats.packetCacheHits++; SyncRes::s_queries++; ageDNSPacket(response, age); @@ -2028,7 +2042,7 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr but it means that the hash would not be computed. If some script decides at a later time to mark back the answer as cacheable we would cache it with a wrong tag, so better safe than sorry. */ eventTrace.add(RecEventTrace::PCacheCheck); - bool cacheHit = checkForCacheHit(qnameParsed, ctag, question, qname, qtype, qclass, g_now, response, qhash, pbData, false, source); + bool cacheHit = checkForCacheHit(qnameParsed, ctag, question, qname, qtype, qclass, g_now, response, qhash, pbData, false, source, mappedSource); eventTrace.add(RecEventTrace::PCacheCheck, cacheHit, false); if (cacheHit) { if (!g_quiet) { @@ -2265,6 +2279,7 @@ static void handleNewUDPQuestion(int fd, FDMultiplexer::funcparam_t& var) if (t_proxyMapping) { if (auto it = t_proxyMapping->lookup(source)) { mappedSource = it->second.address; + ++it->second.stats.netmaskMatches; } } if (t_remotes) { diff --git a/pdns/rec-lua-conf.hh b/pdns/rec-lua-conf.hh index 249c84d3a4..dffe928490 100644 --- a/pdns/rec-lua-conf.hh +++ b/pdns/rec-lua-conf.hh @@ -77,10 +77,17 @@ enum class AdditionalMode : uint8_t ResolveDeferred }; +struct ProxyMappingCounts +{ + uint64_t netmaskMatches{}; + uint64_t suffixMatches{}; +}; + struct ProxyByTableValue { ComboAddress address; boost::optional suffixMatchNode; + mutable ProxyMappingCounts stats{}; }; using ProxyMapping = NetmaskTree; diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc index aa8449a7d3..3c96649c2c 100644 --- a/pdns/rec_channel_rec.cc +++ b/pdns/rec_channel_rec.cc @@ -928,6 +928,27 @@ static uint64_t doGetThreadCPUMsec(int n) return tt.times.at(n); } +static ProxyMappingStats_t* pleaseGetProxyMappingStats() +{ + auto ret = new ProxyMappingStats_t; + if (t_proxyMapping) { + for (const auto& [key, entry] : *t_proxyMapping) { + ret->emplace(std::make_pair(key, ProxyMappingCounts{entry.stats.netmaskMatches, entry.stats.suffixMatches})); + } + } + return ret; +} + +static string doGetProxyMappingStats() +{ + ostringstream ret; + auto m = broadcastAccFunction(pleaseGetProxyMappingStats); + for (const auto& [key, entry] : m) { + ret << key.toString() << '\t' << entry.netmaskMatches << '\t' << entry.suffixMatches << endl; + } + return ret.str(); +} + static uint64_t calculateUptime() { return time(nullptr) - g_stats.startupTime; @@ -1138,6 +1159,26 @@ static StatsMap toRPZStatsMap(const string& name, LockGuarded(pleaseGetProxyMappingStats); + size_t count = 0; + for (const auto& [key, entry] : m) { + auto keyname = pbasename + "{netmask=\"" + key.toString() + "\",count=\""; + auto sname1 = name + "-n-" + std::to_string(count); + auto pname1 = keyname + "netmaskmatches\"}"; + entries.emplace(sname1, StatsMapEntry{pname1, std::to_string(entry.netmaskMatches)}); + auto sname2 = name + "-s-" + std::to_string(count); + auto pname2 = keyname + "suffixmatches\"}"; + entries.emplace(sname2, StatsMapEntry{pname2, std::to_string(entry.suffixMatches)}); + count++; + } + return entries; +} + static void registerAllStats1() { addGetStat("questions", &g_stats.qcounter); @@ -1406,6 +1447,9 @@ static void registerAllStats1() addGetStat("policy-hits", []() { return toRPZStatsMap("policy-hits", g_stats.policyHits); }); + addGetStat("proxy-mapping-total", []() { + return toProxyMappingStatsMap("proxy-mapping-total"); + }); } void registerAllStats() @@ -1864,9 +1908,25 @@ static string setEventTracing(T begin, T end) } } -static void* pleaseSupplantProxyMapping(std::shared_ptr pm) +static void* pleaseSupplantProxyMapping(const ProxyMapping& pm) { - t_proxyMapping = pm; + if (pm.empty()) { + t_proxyMapping = nullptr; + } + else { + // Copy the existing stats values, for the new config items also present in the old + auto newmapping = make_unique(); + for (const auto& [nm, entry] : pm) { + auto& newentry = newmapping->insert(nm); + newentry.second = entry; + if (t_proxyMapping) { + if (auto existing = t_proxyMapping->lookup(nm); existing != nullptr) { + newentry.second.stats = existing->second.stats; + } + } + } + t_proxyMapping = std::move(newmapping); + } return nullptr; } @@ -1913,6 +1973,7 @@ RecursorControlChannel::Answer RecursorControlParser::getAnswer(int s, const str "get-ntas get all configured Negative Trust Anchors\n" "get-tas get all configured Trust Anchors\n" "get-parameter [key1] [key2] .. get configuration parameters\n" + "get-proxymapping-stats get proxy mapping statistics\n" "get-qtypelist get QType statistics\n" " notice: queries from cache aren't being counted yet\n" "hash-password [work-factor] ask for a password then return the hashed version\n" @@ -2021,8 +2082,7 @@ RecursorControlChannel::Answer RecursorControlParser::getAnswer(int s, const str ProxyMapping proxyMapping; loadRecursorLuaConfig(::arg()["lua-config-file"], delayedLuaThreads, proxyMapping); startLuaConfigDelayedThreads(delayedLuaThreads, g_luaconfs.getCopy().generation); - std::shared_ptr ptr = proxyMapping.empty() ? nullptr : std::make_shared(proxyMapping); - broadcastFunction([=] { return pleaseSupplantProxyMapping(ptr); }); + broadcastFunction([=] { return pleaseSupplantProxyMapping(proxyMapping); }); g_log << Logger::Warning << "Reloaded Lua configuration file '" << ::arg()["lua-config-file"] << "', requested via control channel" << endl; return {0, "Reloaded Lua configuration file '" + ::arg()["lua-config-file"] + "'\n"}; } @@ -2166,6 +2226,9 @@ RecursorControlChannel::Answer RecursorControlParser::getAnswer(int s, const str if (cmd == "set-event-trace-enabled") { return {0, setEventTracing(begin, end)}; } + if (cmd == "get-proxymapping-stats") { + return {0, doGetProxyMappingStats()}; + } return {1, "Unknown command '" + cmd + "', try 'help'\n"}; } diff --git a/pdns/recursordist/docs/manpages/rec_control.1.rst b/pdns/recursordist/docs/manpages/rec_control.1.rst index df2e42a7dd..b3a07e7a87 100644 --- a/pdns/recursordist/docs/manpages/rec_control.1.rst +++ b/pdns/recursordist/docs/manpages/rec_control.1.rst @@ -161,6 +161,9 @@ get-tas get-parameter *KEY* [*KEY*]... Retrieves the specified configuration parameter(s). +get-proxymapping-stats + Get the list of proxy-mapped subnets and associated counters. + get-qtypelist Retrieves QType statistics. Queries from cache aren't being counted yet. diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 4b37c1e969..5460fa8c46 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -111,8 +111,8 @@ deferredAdd_t g_deferredAdds; and finally the workers */ std::vector RecThreadInfo::s_threadInfos; -std::shared_ptr g_proxyMapping; // new threads needs this to be setup -thread_local std::shared_ptr t_proxyMapping; +std::unique_ptr g_proxyMapping; // new threads needs this to be setup +thread_local std::unique_ptr t_proxyMapping; bool RecThreadInfo::s_weDistributeQueries; // if true, 1 or more threads listen on the incoming query sockets and distribute them to workers unsigned int RecThreadInfo::s_numDistributorThreads; @@ -1279,6 +1279,15 @@ static vector>& operator+=(vector& fun); / template vector broadcastAccFunction(const std::function*()>& fun); // explicit instantiation template vector> broadcastAccFunction(const std::function>*()>& fun); // explicit instantiation template ThreadTimes broadcastAccFunction(const std::function& fun); +template ProxyMappingStats_t broadcastAccFunction(const std::function& fun); static int serviceMain(int argc, char* argv[], Logr::log_t log) { @@ -1422,7 +1432,7 @@ static int serviceMain(int argc, char* argv[], Logr::log_t log) ProxyMapping proxyMapping; loadRecursorLuaConfig(::arg()["lua-config-file"], delayedLuaThreads, proxyMapping); // Initial proxy mapping - g_proxyMapping = proxyMapping.empty() ? nullptr : std::make_shared(proxyMapping); + g_proxyMapping = proxyMapping.empty() ? nullptr : std::make_unique(proxyMapping); } catch (PDNSException& e) { SLOG(g_log << Logger::Error << "Cannot load Lua configuration: " << e.reason << endl, @@ -2301,7 +2311,12 @@ static void recursorThread() t_allowNotifyFor = g_initialAllowNotifyFor; t_udpclientsocks = std::make_unique(); t_tcpClientCounts = std::make_unique(); - t_proxyMapping = g_proxyMapping; + if (g_proxyMapping) { + t_proxyMapping = make_unique(*g_proxyMapping); + } + else { + t_proxyMapping = nullptr; + } if (threadInfo.isHandler()) { if (!primeHints()) { @@ -2728,7 +2743,7 @@ int main(int argc, char** argv) for (size_t idx = 0; idx < 128; idx++) { defaultAPIDisabledStats += ", ecs-v6-response-bits-" + std::to_string(idx + 1); } - std::string defaultDisabledStats = defaultAPIDisabledStats + ", cumul-clientanswers, cumul-authanswers, policy-hits"; + std::string defaultDisabledStats = defaultAPIDisabledStats + ", cumul-clientanswers, cumul-authanswers, policy-hits, proxy-mapping-total"; ::arg().set("stats-api-blacklist", "List of statistics that are disabled when retrieving the complete list of statistics via the API (deprecated)") = defaultAPIDisabledStats; ::arg().set("stats-carbon-blacklist", "List of statistics that are prevented from being exported via Carbon (deprecated)") = defaultDisabledStats; diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh index e0b678e54b..48aeb6bfa0 100644 --- a/pdns/recursordist/rec-main.hh +++ b/pdns/recursordist/rec-main.hh @@ -233,7 +233,8 @@ extern string g_programname; extern string g_pidfname; extern RecursorControlChannel g_rcc; // only active in the handler thread -extern thread_local std::shared_ptr t_proxyMapping; +extern thread_local std::unique_ptr t_proxyMapping; +using ProxyMappingStats_t = std::unordered_map; #ifdef NOD_ENABLED extern bool g_nodEnabled; @@ -519,7 +520,7 @@ bool checkForCacheHit(bool qnameParsed, unsigned int tag, const string& data, DNSName& qname, uint16_t& qtype, uint16_t& qclass, const struct timeval& now, string& response, uint32_t& qhash, - RecursorPacketCache::OptPBData& pbData, bool tcp, const ComboAddress& source); + RecursorPacketCache::OptPBData& pbData, bool tcp, const ComboAddress& source, const ComboAddress& mappedSource); void protobufLogResponse(pdns::ProtoZero::RecMessage& message); void protobufLogResponse(const struct dnsheader* dh, LocalStateHolder& luaconfsLocal, const RecursorPacketCache::OptPBData& pbData, const struct timeval& tv, diff --git a/pdns/recursordist/rec-tcp.cc b/pdns/recursordist/rec-tcp.cc index 0747664f4a..fd99688b6e 100644 --- a/pdns/recursordist/rec-tcp.cc +++ b/pdns/recursordist/rec-tcp.cc @@ -269,6 +269,7 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var) if (t_proxyMapping) { if (auto it = t_proxyMapping->lookup(conn->d_source)) { conn->d_mappedSource = it->second.address; + ++it->second.stats.netmaskMatches; } } if (t_allowFrom && !t_allowFrom->match(&conn->d_mappedSource)) { @@ -553,7 +554,7 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var) but it means that the hash would not be computed. If some script decides at a later time to mark back the answer as cacheable we would cache it with a wrong tag, so better safe than sorry. */ dc->d_eventTrace.add(RecEventTrace::PCacheCheck); - bool cacheHit = checkForCacheHit(qnameParsed, dc->d_tag, conn->data, qname, qtype, qclass, g_now, response, dc->d_qhash, pbData, true, dc->d_source); + bool cacheHit = checkForCacheHit(qnameParsed, dc->d_tag, conn->data, qname, qtype, qclass, g_now, response, dc->d_qhash, pbData, true, dc->d_source, dc->d_mappedSource); dc->d_eventTrace.add(RecEventTrace::PCacheCheck, cacheHit, false); if (cacheHit) { @@ -652,6 +653,7 @@ void handleNewTCPQuestion(int fd, FDMultiplexer::funcparam_t&) if (!fromProxyProtocolSource && t_proxyMapping) { if (auto it = t_proxyMapping->lookup(addr)) { mappedSource = it->second.address; + ++it->second.stats.netmaskMatches; } } if (!fromProxyProtocolSource && t_allowFrom && !t_allowFrom->match(&mappedSource)) { diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc index e34ffc228f..aa803516fb 100644 --- a/pdns/ws-recursor.cc +++ b/pdns/ws-recursor.cc @@ -1149,6 +1149,11 @@ const std::map MetricDefinitionStorage::d_metrics {"maintenance-calls", MetricDefinition(PrometheusMetricType::counter, "Number of times internal maintenance has been called, including Lua maintenance")}, + + // For multicounters, state the first + {"proxy-mapping-total-n-0", + MetricDefinition(PrometheusMetricType::multicounter, + "Number of queries matching proxyMappings")}, }; #define CHECK_PROMETHEUS_METRICS 0 @@ -1171,7 +1176,7 @@ static void validatePrometheusMetrics() if (!s_metricDefinitions.getMetricDetails(metricName, metricDetails)) { SLOG(g_log << Logger::Debug << "{ \"" << metricName << "\", MetricDefinition(PrometheusMetricType::counter, \"\")}," << endl, - g_slog->info(Logr::Debug, "{ \"" << metricName << "\", MetricDefinition(PrometheusMetricType::counter, \"\")},")); + g_slog->info(Logr::Debug, "{ \"" + metricName + "\", MetricDefinition(PrometheusMetricType::counter, \"\")},")); } } }