From: Remi Gacogne Date: Tue, 11 Jul 2023 08:37:08 +0000 (+0200) Subject: dnsdist: Delint dnsdist-healthchecks.cc X-Git-Tag: rec-5.0.0-alpha1~97^2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=65f47b8e68c303a59c5aec6bbfc4421d8616fdbb;p=thirdparty%2Fpdns.git dnsdist: Delint dnsdist-healthchecks.cc --- diff --git a/pdns/dnsdistdist/dnsdist-healthchecks.cc b/pdns/dnsdistdist/dnsdist-healthchecks.cc index e04ef5a56e..1ec3cb06d9 100644 --- a/pdns/dnsdistdist/dnsdist-healthchecks.cc +++ b/pdns/dnsdistdist/dnsdist-healthchecks.cc @@ -35,7 +35,7 @@ struct HealthCheckData { enum class TCPState : uint8_t { WritingQuery, ReadingResponseSize, ReadingResponse }; - HealthCheckData(FDMultiplexer& mplexer, const std::shared_ptr& ds, DNSName&& checkName, uint16_t checkType, uint16_t checkClass, uint16_t queryID): d_ds(ds), d_mplexer(mplexer), d_udpSocket(-1), d_checkName(std::move(checkName)), d_checkType(checkType), d_checkClass(checkClass), d_queryID(queryID) + HealthCheckData(FDMultiplexer& mplexer, std::shared_ptr downstream, DNSName&& checkName, uint16_t checkType, uint16_t checkClass, uint16_t queryID): d_ds(std::move(downstream)), d_mplexer(mplexer), d_udpSocket(-1), d_checkName(std::move(checkName)), d_checkType(checkType), d_checkClass(checkClass), d_queryID(queryID) { } @@ -57,57 +57,58 @@ struct HealthCheckData static bool handleResponse(std::shared_ptr& data) { - auto& ds = data->d_ds; + const auto& downstream = data->d_ds; try { if (data->d_buffer.size() < sizeof(dnsheader)) { ++data->d_ds->d_healthCheckMetrics.d_parseErrors; if (g_verboseHealthChecks) { - infolog("Invalid health check response of size %d from backend %s, expecting at least %d", data->d_buffer.size(), ds->getNameWithAddr(), sizeof(dnsheader)); + infolog("Invalid health check response of size %d from backend %s, expecting at least %d", data->d_buffer.size(), downstream->getNameWithAddr(), sizeof(dnsheader)); } return false; } - const dnsheader * responseHeader = reinterpret_cast(data->d_buffer.data()); - if (responseHeader->id != data->d_queryID) { + dnsheader_aligned responseHeader(data->d_buffer.data()); + if (responseHeader.get()->id != data->d_queryID) { ++data->d_ds->d_healthCheckMetrics.d_mismatchErrors; if (g_verboseHealthChecks) { - infolog("Invalid health check response id %d from backend %s, expecting %d", responseHeader->id, ds->getNameWithAddr(), data->d_queryID); + infolog("Invalid health check response id %d from backend %s, expecting %d", responseHeader.get()->id, downstream->getNameWithAddr(), data->d_queryID); } return false; } - if (!responseHeader->qr) { + if (!responseHeader.get()->qr) { ++data->d_ds->d_healthCheckMetrics.d_invalidResponseErrors; if (g_verboseHealthChecks) { - infolog("Invalid health check response from backend %s, expecting QR to be set", ds->getNameWithAddr()); + infolog("Invalid health check response from backend %s, expecting QR to be set", downstream->getNameWithAddr()); } return false; } - if (responseHeader->rcode == RCode::ServFail) { + if (responseHeader.get()->rcode == RCode::ServFail) { ++data->d_ds->d_healthCheckMetrics.d_invalidResponseErrors; if (g_verboseHealthChecks) { - infolog("Backend %s responded to health check with ServFail", ds->getNameWithAddr()); + infolog("Backend %s responded to health check with ServFail", downstream->getNameWithAddr()); } return false; } - if (ds->d_config.mustResolve && (responseHeader->rcode == RCode::NXDomain || responseHeader->rcode == RCode::Refused)) { + if (downstream->d_config.mustResolve && (responseHeader.get()->rcode == RCode::NXDomain || responseHeader.get()->rcode == RCode::Refused)) { ++data->d_ds->d_healthCheckMetrics.d_invalidResponseErrors; if (g_verboseHealthChecks) { - infolog("Backend %s responded to health check with %s while mustResolve is set", ds->getNameWithAddr(), responseHeader->rcode == RCode::NXDomain ? "NXDomain" : "Refused"); + infolog("Backend %s responded to health check with %s while mustResolve is set", downstream->getNameWithAddr(), responseHeader.get()->rcode == RCode::NXDomain ? "NXDomain" : "Refused"); } return false; } - uint16_t receivedType; - uint16_t receivedClass; - DNSName receivedName(reinterpret_cast(data->d_buffer.data()), data->d_buffer.size(), sizeof(dnsheader), false, &receivedType, &receivedClass); + uint16_t receivedType{0}; + uint16_t receivedClass{0}; + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + DNSName receivedName(reinterpret_cast(data->d_buffer.data()), static_cast(data->d_buffer.size()), sizeof(dnsheader), false, &receivedType, &receivedClass); if (receivedName != data->d_checkName || receivedType != data->d_checkType || receivedClass != data->d_checkClass) { ++data->d_ds->d_healthCheckMetrics.d_mismatchErrors; if (g_verboseHealthChecks) { - infolog("Backend %s responded to health check with an invalid qname (%s vs %s), qtype (%s vs %s) or qclass (%d vs %d)", ds->getNameWithAddr(), receivedName.toLogString(), data->d_checkName.toLogString(), QType(receivedType).toString(), QType(data->d_checkType).toString(), receivedClass, data->d_checkClass); + infolog("Backend %s responded to health check with an invalid qname (%s vs %s), qtype (%s vs %s) or qclass (%d vs %d)", downstream->getNameWithAddr(), receivedName.toLogString(), data->d_checkName.toLogString(), QType(receivedType).toString(), QType(data->d_checkType).toString(), receivedClass, data->d_checkClass); } return false; } @@ -115,13 +116,13 @@ static bool handleResponse(std::shared_ptr& data) catch (const std::exception& e) { ++data->d_ds->d_healthCheckMetrics.d_parseErrors; if (g_verboseHealthChecks) { - infolog("Error checking the health of backend %s: %s", ds->getNameWithAddr(), e.what()); + infolog("Error checking the health of backend %s: %s", downstream->getNameWithAddr(), e.what()); } return false; } catch (...) { if (g_verboseHealthChecks) { - infolog("Unknown exception while checking the health of backend %s", ds->getNameWithAddr()); + infolog("Unknown exception while checking the health of backend %s", downstream->getNameWithAddr()); } return false; } @@ -135,12 +136,13 @@ public: HealthCheckQuerySender(std::shared_ptr& data): d_data(data) { } + HealthCheckQuerySender(const HealthCheckQuerySender&) = default; + HealthCheckQuerySender(HealthCheckQuerySender&&) = default; + HealthCheckQuerySender& operator=(const HealthCheckQuerySender&) = default; + HealthCheckQuerySender& operator=(HealthCheckQuerySender&&) = default; + ~HealthCheckQuerySender() override = default; - ~HealthCheckQuerySender() - { - } - - bool active() const override + [[nodiscard]] bool active() const override { return true; } @@ -166,7 +168,7 @@ private: std::shared_ptr d_data; }; -static void healthCheckUDPCallback(int fd, FDMultiplexer::funcparam_t& param) +static void healthCheckUDPCallback(int descriptor, FDMultiplexer::funcparam_t& param) { auto data = boost::any_cast>(param); @@ -177,6 +179,7 @@ static void healthCheckUDPCallback(int fd, FDMultiplexer::funcparam_t& param) auto fromlen = from.getSocklen(); data->d_buffer.resize(512); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) got = recvfrom(data->d_udpSocket.getHandle(), &data->d_buffer.at(0), data->d_buffer.size(), 0, reinterpret_cast(&from), &fromlen); if (got < 0) { int savederrno = errno; @@ -194,7 +197,7 @@ static void healthCheckUDPCallback(int fd, FDMultiplexer::funcparam_t& param) } ++data->d_ds->d_healthCheckMetrics.d_networkErrors; data->d_ds->submitHealthCheckResult(data->d_initial, false); - data->d_mplexer.removeReadFD(fd); + data->d_mplexer.removeReadFD(descriptor); return; } } @@ -212,11 +215,11 @@ static void healthCheckUDPCallback(int fd, FDMultiplexer::funcparam_t& param) return; } - data->d_mplexer.removeReadFD(fd); + data->d_mplexer.removeReadFD(descriptor); data->d_ds->submitHealthCheckResult(data->d_initial, handleResponse(data)); } -static void healthCheckTCPCallback(int fd, FDMultiplexer::funcparam_t& param) +static void healthCheckTCPCallback(int descriptor, FDMultiplexer::funcparam_t& param) { auto data = boost::any_cast>(param); @@ -237,7 +240,7 @@ static void healthCheckTCPCallback(int fd, FDMultiplexer::funcparam_t& param) ioState = data->d_tcpHandler->tryRead(data->d_buffer, data->d_bufferPos, data->d_buffer.size()); if (ioState == IOState::Done) { data->d_bufferPos = 0; - uint16_t responseSize; + uint16_t responseSize{0}; memcpy(&responseSize, &data->d_buffer.at(0), sizeof(responseSize)); data->d_buffer.resize(ntohs(responseSize)); data->d_tcpState = HealthCheckData::TCPState::ReadingResponse; @@ -288,27 +291,27 @@ static void healthCheckTCPCallback(int fd, FDMultiplexer::funcparam_t& param) } } -bool queueHealthCheck(std::unique_ptr& mplexer, const std::shared_ptr& ds, bool initialCheck) +bool queueHealthCheck(std::unique_ptr& mplexer, const std::shared_ptr& downstream, bool initialCheck) { try { uint16_t queryID = dnsdist::getRandomDNSID(); - DNSName checkName = ds->d_config.checkName; - uint16_t checkType = ds->d_config.checkType.getCode(); - uint16_t checkClass = ds->d_config.checkClass; - dnsheader checkHeader; + DNSName checkName = downstream->d_config.checkName; + uint16_t checkType = downstream->d_config.checkType.getCode(); + uint16_t checkClass = downstream->d_config.checkClass; + dnsheader checkHeader{}; memset(&checkHeader, 0, sizeof(checkHeader)); checkHeader.qdcount = htons(1); checkHeader.id = queryID; checkHeader.rd = true; - if (ds->d_config.setCD) { + if (downstream->d_config.setCD) { checkHeader.cd = true; } - if (ds->d_config.checkFunction) { + if (downstream->d_config.checkFunction) { auto lock = g_lua.lock(); - auto ret = ds->d_config.checkFunction(checkName, checkType, checkClass, &checkHeader); + auto ret = downstream->d_config.checkFunction(checkName, checkType, checkClass, &checkHeader); checkName = std::get<0>(ret); checkType = std::get<1>(ret); checkClass = std::get<2>(ret); @@ -323,88 +326,88 @@ bool queueHealthCheck(std::unique_ptr& mplexer, const std::shared uint16_t packetSize = packet.size(); std::string proxyProtocolPayload; size_t proxyProtocolPayloadSize = 0; - if (ds->d_config.useProxyProtocol) { + if (downstream->d_config.useProxyProtocol) { proxyProtocolPayload = makeLocalProxyHeader(); proxyProtocolPayloadSize = proxyProtocolPayload.size(); - if (!ds->isDoH()) { + if (!downstream->isDoH()) { packet.insert(packet.begin(), proxyProtocolPayload.begin(), proxyProtocolPayload.end()); } } - Socket sock(ds->d_config.remote.sin4.sin_family, ds->doHealthcheckOverTCP() ? SOCK_STREAM : SOCK_DGRAM); + Socket sock(downstream->d_config.remote.sin4.sin_family, downstream->doHealthcheckOverTCP() ? SOCK_STREAM : SOCK_DGRAM); sock.setNonBlocking(); #ifdef SO_BINDTODEVICE - if (!ds->d_config.sourceItfName.empty()) { - int res = setsockopt(sock.getHandle(), SOL_SOCKET, SO_BINDTODEVICE, ds->d_config.sourceItfName.c_str(), ds->d_config.sourceItfName.length()); + if (!downstream->d_config.sourceItfName.empty()) { + int res = setsockopt(sock.getHandle(), SOL_SOCKET, SO_BINDTODEVICE, downstream->d_config.sourceItfName.c_str(), downstream->d_config.sourceItfName.length()); if (res != 0 && g_verboseHealthChecks) { - infolog("Error setting SO_BINDTODEVICE on the health check socket for backend '%s': %s", ds->getNameWithAddr(), stringerror()); + infolog("Error setting SO_BINDTODEVICE on the health check socket for backend '%s': %s", downstream->getNameWithAddr(), stringerror()); } } #endif - if (!IsAnyAddress(ds->d_config.sourceAddr)) { - if (ds->doHealthcheckOverTCP()) { + if (!IsAnyAddress(downstream->d_config.sourceAddr)) { + if (downstream->doHealthcheckOverTCP()) { sock.setReuseAddr(); } #ifdef IP_BIND_ADDRESS_NO_PORT - if (ds->d_config.ipBindAddrNoPort) { + if (downstream->d_config.ipBindAddrNoPort) { SSetsockopt(sock.getHandle(), SOL_IP, IP_BIND_ADDRESS_NO_PORT, 1); } #endif - sock.bind(ds->d_config.sourceAddr, false); + sock.bind(downstream->d_config.sourceAddr, false); } - auto data = std::make_shared(*mplexer, ds, std::move(checkName), checkType, checkClass, queryID); + auto data = std::make_shared(*mplexer, downstream, std::move(checkName), checkType, checkClass, queryID); data->d_initial = initialCheck; gettimeofday(&data->d_ttd, nullptr); - data->d_ttd.tv_sec += ds->d_config.checkTimeout / 1000; /* ms to seconds */ - data->d_ttd.tv_usec += (ds->d_config.checkTimeout % 1000) * 1000; /* remaining ms to us */ + data->d_ttd.tv_sec += static_castd_ttd.tv_sec)>(downstream->d_config.checkTimeout / 1000); /* ms to seconds */ + data->d_ttd.tv_usec += static_castd_ttd.tv_usec)>((downstream->d_config.checkTimeout % 1000) * 1000); /* remaining ms to us */ normalizeTV(data->d_ttd); - if (!ds->doHealthcheckOverTCP()) { - sock.connect(ds->d_config.remote); + if (!downstream->doHealthcheckOverTCP()) { + sock.connect(downstream->d_config.remote); data->d_udpSocket = std::move(sock); - ssize_t sent = udpClientSendRequestToBackend(ds, data->d_udpSocket.getHandle(), packet, true); + ssize_t sent = udpClientSendRequestToBackend(downstream, data->d_udpSocket.getHandle(), packet, true); if (sent < 0) { int ret = errno; if (g_verboseHealthChecks) { - infolog("Error while sending a health check query (ID %d) to backend %s: %d", queryID, ds->getNameWithAddr(), ret); + infolog("Error while sending a health check query (ID %d) to backend %s: %d", queryID, downstream->getNameWithAddr(), ret); } return false; } mplexer->addReadFD(data->d_udpSocket.getHandle(), &healthCheckUDPCallback, data, &data->d_ttd); } - else if (ds->isDoH()) { + else if (downstream->isDoH()) { InternalQuery query(std::move(packet), InternalQueryState()); query.d_proxyProtocolPayload = std::move(proxyProtocolPayload); auto sender = std::shared_ptr(new HealthCheckQuerySender(data)); - if (!sendH2Query(ds, mplexer, sender, std::move(query), true)) { + if (!sendH2Query(downstream, mplexer, sender, std::move(query), true)) { data->d_ds->submitHealthCheckResult(data->d_initial, false); } } else { - data->d_tcpHandler = std::make_unique(ds->d_config.d_tlsSubjectName, ds->d_config.d_tlsSubjectIsAddr, sock.releaseHandle(), timeval{ds->d_config.checkTimeout,0}, ds->d_tlsCtx); + data->d_tcpHandler = std::make_unique(downstream->d_config.d_tlsSubjectName, downstream->d_config.d_tlsSubjectIsAddr, sock.releaseHandle(), timeval{downstream->d_config.checkTimeout,0}, downstream->d_tlsCtx); data->d_ioState = std::make_unique(*mplexer, data->d_tcpHandler->getDescriptor()); - if (ds->d_tlsCtx) { + if (downstream->d_tlsCtx) { try { time_t now = time(nullptr); - auto tlsSession = g_sessionCache.getSession(ds->getID(), now); + auto tlsSession = g_sessionCache.getSession(downstream->getID(), now); if (tlsSession) { data->d_tcpHandler->setTLSSession(tlsSession); } } catch (const std::exception& e) { - vinfolog("Unable to restore a TLS session for the DoT healthcheck for backend %s: %s", ds->getNameWithAddr(), e.what()); + vinfolog("Unable to restore a TLS session for the DoT healthcheck for backend %s: %s", downstream->getNameWithAddr(), e.what()); } } - data->d_tcpHandler->tryConnect(ds->d_config.tcpFastOpen, ds->d_config.remote); + data->d_tcpHandler->tryConnect(downstream->d_config.tcpFastOpen, downstream->d_config.remote); - const uint8_t sizeBytes[] = { static_cast(packetSize / 256), static_cast(packetSize % 256) }; - packet.insert(packet.begin() + proxyProtocolPayloadSize, sizeBytes, sizeBytes + 2); + const std::array sizeBytes = { static_cast(packetSize / 256), static_cast(packetSize % 256) }; + packet.insert(packet.begin() + static_cast(proxyProtocolPayloadSize), sizeBytes.begin(), sizeBytes.end()); data->d_buffer = std::move(packet); auto ioState = data->d_tcpHandler->tryWrite(data->d_buffer, data->d_bufferPos, data->d_buffer.size()); @@ -422,13 +425,13 @@ bool queueHealthCheck(std::unique_ptr& mplexer, const std::shared } catch (const std::exception& e) { if (g_verboseHealthChecks) { - infolog("Error checking the health of backend %s: %s", ds->getNameWithAddr(), e.what()); + infolog("Error checking the health of backend %s: %s", downstream->getNameWithAddr(), e.what()); } return false; } catch (...) { if (g_verboseHealthChecks) { - infolog("Unknown exception while checking the health of backend %s", ds->getNameWithAddr()); + infolog("Unknown exception while checking the health of backend %s", downstream->getNameWithAddr()); } return false; } @@ -437,7 +440,7 @@ bool queueHealthCheck(std::unique_ptr& mplexer, const std::shared void handleQueuedHealthChecks(FDMultiplexer& mplexer, bool initial) { while (mplexer.getWatchedFDCount(false) > 0 || mplexer.getWatchedFDCount(true) > 0) { - struct timeval now; + struct timeval now{}; int ret = mplexer.run(&now, 100); if (ret == -1) { if (g_verboseHealthChecks) { diff --git a/pdns/dnsdistdist/dnsdist-healthchecks.hh b/pdns/dnsdistdist/dnsdist-healthchecks.hh index 825961e130..91c4c1a2a5 100644 --- a/pdns/dnsdistdist/dnsdist-healthchecks.hh +++ b/pdns/dnsdistdist/dnsdist-healthchecks.hh @@ -27,6 +27,6 @@ extern bool g_verboseHealthChecks; -bool queueHealthCheck(std::unique_ptr& mplexer, const std::shared_ptr& ds, bool initial=false); +bool queueHealthCheck(std::unique_ptr& mplexer, const std::shared_ptr& downstream, bool initial=false); void handleQueuedHealthChecks(FDMultiplexer& mplexer, bool initial=false);