From: Remi Gacogne Date: Wed, 22 Jun 2022 13:44:38 +0000 (+0200) Subject: dnsdist: Add a per-server metric for non-compliant responses X-Git-Tag: auth-4.8.0-alpha0~40^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d70f95ac37c617c1b7de3589b16b48c17178bd80;p=thirdparty%2Fpdns.git dnsdist: Add a per-server metric for non-compliant responses --- diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index cc55ee4364..c46d766927 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -522,7 +522,7 @@ void IncomingTCPConnectionState::handleResponse(const struct timeval& now, TCPRe try { auto& ids = response.d_idstate; unsigned int qnameWireLength; - if (!response.d_connection || !responseContentMatches(response.d_buffer, ids.qname, ids.qtype, ids.qclass, response.d_connection->getRemote(), qnameWireLength)) { + if (!response.d_connection || !responseContentMatches(response.d_buffer, ids.qname, ids.qtype, ids.qclass, response.d_connection->getDS(), qnameWireLength)) { state->terminateClientConnection(); return; } diff --git a/pdns/dnsdist-web.cc b/pdns/dnsdist-web.cc index 0af3822a37..5779c267d2 100644 --- a/pdns/dnsdist-web.cc +++ b/pdns/dnsdist-web.cc @@ -510,6 +510,8 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp) output << "# TYPE " << statesbase << "queries " << "counter" << "\n"; output << "# HELP " << statesbase << "responses " << "Amount of responses received from this server" << "\n"; output << "# TYPE " << statesbase << "responses " << "counter" << "\n"; + output << "# HELP " << statesbase << "noncompliantresponses " << "Amount of non-compliant responses received from this server" << "\n"; + output << "# TYPE " << statesbase << "noncompliantresponses " << "counter" << "\n"; output << "# HELP " << statesbase << "drops " << "Amount of queries not answered by server" << "\n"; output << "# TYPE " << statesbase << "drops " << "counter" << "\n"; output << "# HELP " << statesbase << "latency " << "Server's latency when answering questions in milliseconds" << "\n"; @@ -569,6 +571,7 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp) output << statesbase << "status" << label << " " << (state->isUp() ? "1" : "0") << "\n"; output << statesbase << "queries" << label << " " << state->queries.load() << "\n"; output << statesbase << "responses" << label << " " << state->responses.load() << "\n"; + output << statesbase << "noncompliantresponses" << label << " " << state->nonCompliantResponses.load()<< "\n"; output << statesbase << "drops" << label << " " << state->reuseds.load() << "\n"; if (state->isUp()) { output << statesbase << "latency" << label << " " << state->latencyUsec/1000.0 << "\n"; @@ -989,6 +992,7 @@ static void addServerToJSON(Json::array& servers, int id, const std::shared_ptr< {"latency", (double)(a->latencyUsec/1000.0)}, {"queries", (double)a->queries}, {"responses", (double)a->responses}, + {"nonCompliantResponses", (double)a->nonCompliantResponses}, {"sendErrors", (double)a->sendErrors}, {"tcpDiedSendingQuery", (double)a->tcpDiedSendingQuery}, {"tcpDiedReadingResponse", (double)a->tcpDiedReadingResponse}, diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index ca7d471c2d..e79f31fb02 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -267,7 +267,7 @@ void doLatencyStats(double udiff) doAvg(g_stats.latencyAvg1000000, udiff, 1000000); } -bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const ComboAddress& remote, unsigned int& qnameWireLength) +bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const std::shared_ptr& remote, unsigned int& qnameWireLength) { if (response.size() < sizeof(dnsheader)) { return false; @@ -276,6 +276,9 @@ bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const struct dnsheader* dh = reinterpret_cast(response.data()); if (dh->qr == 0) { ++g_stats.nonCompliantResponses; + if (remote) { + ++remote->nonCompliantResponses; + } return false; } @@ -285,6 +288,9 @@ bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, } else { ++g_stats.nonCompliantResponses; + if (remote) { + ++remote->nonCompliantResponses; + } return false; } } @@ -295,10 +301,13 @@ bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, rqname = DNSName(reinterpret_cast(response.data()), response.size(), sizeof(dnsheader), false, &rqtype, &rqclass, &qnameWireLength); } catch (const std::exception& e) { - if(response.size() > 0 && static_cast(response.size()) > sizeof(dnsheader)) { - infolog("Backend %s sent us a response with id %d that did not parse: %s", remote.toStringWithPort(), ntohs(dh->id), e.what()); + if (remote && response.size() > 0 && static_cast(response.size()) > sizeof(dnsheader)) { + infolog("Backend %s sent us a response with id %d that did not parse: %s", remote->d_config.remote.toStringWithPort(), ntohs(dh->id), e.what()); } ++g_stats.nonCompliantResponses; + if (remote) { + ++remote->nonCompliantResponses; + } return false; } @@ -647,7 +656,7 @@ void responderThread(std::shared_ptr dss) int origFD = ids->origFD; unsigned int qnameWireLength = 0; - if (fd != ids->backendFD || !responseContentMatches(response, ids->qname, ids->qtype, ids->qclass, dss->d_config.remote, qnameWireLength)) { + if (fd != ids->backendFD || !responseContentMatches(response, ids->qname, ids->qtype, ids->qclass, dss, qnameWireLength)) { continue; } diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index f0b178e551..f671ed49b3 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -768,6 +768,7 @@ struct DownstreamState: public std::enable_shared_from_this stat_t reuseds{0}; stat_t queries{0}; stat_t responses{0}; + stat_t nonCompliantResponses{0}; struct { stat_t sendErrors{0}; stat_t reuseds{0}; @@ -1101,7 +1102,7 @@ void setLuaSideEffect(); // set to report a side effect, cancelling all _no_ s bool getLuaNoSideEffect(); // set if there were only explicit declarations of _no_ side effect void resetLuaSideEffect(); // reset to indeterminate state -bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const ComboAddress& remote, unsigned int& qnameWireLength); +bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const std::shared_ptr& remote, unsigned int& qnameWireLength); bool processResponse(PacketBuffer& response, LocalStateHolder >& localRespRuleActions, DNSResponse& dr, bool muted, bool receivedOverUDP); bool processRulesResult(const DNSAction::Action& action, DNSQuestion& dq, std::string& ruleresult, bool& drop); diff --git a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc index 6b73ef9fb1..ca8ed42d94 100644 --- a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc +++ b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc @@ -73,14 +73,8 @@ ProcessQueryResult processQuery(DNSQuestion& dq, ClientState& cs, LocalHolders& return ProcessQueryResult::Drop; } -static std::function s_responseContentMatches; - -bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const ComboAddress& remote, unsigned int& qnameWireLength) +bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const std::shared_ptr& remote, unsigned int& qnameWireLength) { - if (s_responseContentMatches) { - return s_responseContentMatches(response, qname, qtype, qclass, remote, qnameWireLength); - } - return true; } diff --git a/regression-tests.dnsdist/test_API.py b/regression-tests.dnsdist/test_API.py index 7aa0699362..784cdeaa14 100644 --- a/regression-tests.dnsdist/test_API.py +++ b/regression-tests.dnsdist/test_API.py @@ -129,14 +129,14 @@ class TestAPIBasics(APITestsBase): for server in content['servers']: for key in ['id', 'latency', 'name', 'weight', 'outstanding', 'qpsLimit', 'reuseds', 'state', 'address', 'pools', 'qps', 'queries', 'order', 'sendErrors', - 'dropRate', 'responses', 'tcpDiedSendingQuery', 'tcpDiedReadingResponse', + 'dropRate', 'responses', 'nonCompliantResponses', 'tcpDiedSendingQuery', 'tcpDiedReadingResponse', 'tcpGaveUp', 'tcpReadTimeouts', 'tcpWriteTimeouts', 'tcpCurrentConnections', 'tcpNewConnections', 'tcpReusedConnections', 'tlsResumptions', 'tcpAvgQueriesPerConnection', 'tcpAvgConnectionDuration', 'tcpLatency', 'protocol']: self.assertIn(key, server) for key in ['id', 'latency', 'weight', 'outstanding', 'qpsLimit', 'reuseds', - 'qps', 'queries', 'order', 'tcpLatency']: + 'qps', 'queries', 'order', 'tcpLatency', 'responses', 'nonCompliantResponses']: self.assertTrue(server[key] >= 0) self.assertTrue(server['state'] in ['up', 'down', 'UP', 'DOWN']) @@ -186,14 +186,14 @@ class TestAPIBasics(APITestsBase): for server in content['servers']: for key in ['id', 'latency', 'name', 'weight', 'outstanding', 'qpsLimit', 'reuseds', 'state', 'address', 'pools', 'qps', 'queries', 'order', 'sendErrors', - 'dropRate', 'responses', 'tcpDiedSendingQuery', 'tcpDiedReadingResponse', + 'dropRate', 'responses', 'nonCompliantResponses', 'tcpDiedSendingQuery', 'tcpDiedReadingResponse', 'tcpGaveUp', 'tcpReadTimeouts', 'tcpWriteTimeouts', 'tcpCurrentConnections', 'tcpNewConnections', 'tcpReusedConnections', 'tcpAvgQueriesPerConnection', 'tcpAvgConnectionDuration', 'tcpLatency', 'protocol']: self.assertIn(key, server) for key in ['id', 'latency', 'weight', 'outstanding', 'qpsLimit', 'reuseds', - 'qps', 'queries', 'order', 'tcpLatency']: + 'qps', 'queries', 'order', 'tcpLatency', 'responses', 'nonCompliantResponses']: self.assertTrue(server[key] >= 0) self.assertTrue(server['state'] in ['up', 'down', 'UP', 'DOWN'])