From: Otto Moerbeek Date: Wed, 28 Aug 2024 08:55:39 +0000 (+0200) Subject: rec: change the way incoming TCP higher than max_tcp_clients is handled X-Git-Tag: rec-5.2.0-alpha1~101^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4adf35652ddb5949377364d7578c1d191400c51;p=thirdparty%2Fpdns.git rec: change the way incoming TCP higher than max_tcp_clients is handled Keep accepting connections, but drop them immediately if we're over limit. This - avoids having lots of incoming TCP connections at the listen queue in the OS - Immediately takes effect, instead of relying on the lazy enforming of the limit No seperate counter for now, also, maybe a log message should be added? Also consider the per client limit for that. --- diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 89543ee567..6a1eb68358 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -2342,6 +2342,7 @@ static int serviceMain(Logr::log_t log) RecThreadInfo::makeThreadPipes(log); g_tcpTimeout = ::arg().asNum("client-tcp-timeout"); + g_maxTCPClients = ::arg().asNum("max-tcp-clients"); g_maxTCPPerClient = ::arg().asNum("max-tcp-per-client"); g_tcpMaxQueriesPerConn = ::arg().asNum("max-tcp-queries-per-connection"); g_maxUDPQueriesPerRound = ::arg().asNum("max-udp-queries-per-round"); @@ -2719,32 +2720,8 @@ static void runLuaMaintenance(RecThreadInfo& threadInfo, time_t& last_lua_mainte } } -static void runTCPMaintenance(RecThreadInfo& threadInfo, bool& listenOnTCP, unsigned int maxTcpClients) -{ - if (threadInfo.isTCPListener()) { - if (listenOnTCP) { - if (TCPConnection::getCurrentConnections() > maxTcpClients) { // shutdown, too many connections - for (const auto fileDesc : threadInfo.getTCPSockets()) { - t_fdm->removeReadFD(fileDesc); - } - listenOnTCP = false; - } - } - else { - if (TCPConnection::getCurrentConnections() <= maxTcpClients) { // reenable - for (const auto fileDesc : threadInfo.getTCPSockets()) { - t_fdm->addReadFD(fileDesc, handleNewTCPQuestion); - } - listenOnTCP = true; - } - } - } -} - static void recLoop() { - unsigned int maxTcpClients = ::arg().asNum("max-tcp-clients"); - bool listenOnTCP{true}; time_t last_stat = 0; time_t last_carbon = 0; time_t last_lua_maintenance = 0; @@ -2806,8 +2783,6 @@ static void recLoop() auto timeoutUsec = g_multiTasker->nextWaiterDelayUsec(500000); t_fdm->run(&g_now, static_cast(timeoutUsec / 1000)); // 'run' updates g_now for us - - runTCPMaintenance(threadInfo, listenOnTCP, maxTcpClients); } catch (const PDNSException& pdnsException) { s_rateLimitedLogger.log(g_slog->withName("runtime"), "recLoop", pdnsException); diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh index e30224e4c7..40b75a6a84 100644 --- a/pdns/recursordist/rec-main.hh +++ b/pdns/recursordist/rec-main.hh @@ -206,6 +206,7 @@ extern unsigned int g_maxMThreads; extern bool g_reusePort; extern bool g_anyToTcp; extern size_t g_tcpMaxQueriesPerConn; +extern unsigned int g_maxTCPClients; extern unsigned int g_maxTCPPerClient; extern int g_tcpTimeout; extern uint16_t g_udpTruncationThreshold; diff --git a/pdns/recursordist/rec-tcp.cc b/pdns/recursordist/rec-tcp.cc index a1af4a1088..353550d292 100644 --- a/pdns/recursordist/rec-tcp.cc +++ b/pdns/recursordist/rec-tcp.cc @@ -61,6 +61,7 @@ // worker thread(s) now no longer process TCP queries. size_t g_tcpMaxQueriesPerConn; +unsigned int g_maxTCPClients; unsigned int g_maxTCPPerClient; int g_tcpTimeout; bool g_anyToTcp; @@ -690,85 +691,80 @@ void handleNewTCPQuestion(int fileDesc, [[maybe_unused]] FDMultiplexer::funcpara ComboAddress addr; socklen_t addrlen = sizeof(addr); int newsock = accept(fileDesc, reinterpret_cast(&addr), &addrlen); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) - if (newsock >= 0) { - if (g_multiTasker->numProcesses() >= g_maxMThreads) { - t_Counters.at(rec::Counter::overCapacityDrops)++; - try { - closesocket(newsock); - } - catch (const PDNSException& e) { - SLOG(g_log << Logger::Error << "Error closing TCP socket after an over capacity drop: " << e.reason << endl, - g_slogtcpin->error(Logr::Error, e.reason, "Error closing TCP socket after an over capacity drop", "exception", Logging::Loggable("PDNSException"))); - } - return; - } - - ComboAddress destaddr; - socklen_t len = sizeof(destaddr); - getsockname(newsock, reinterpret_cast(&destaddr), &len); // if this fails, we're ok with it NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) - bool fromProxyProtocolSource = expectProxyProtocol(addr, destaddr); - if (!fromProxyProtocolSource && t_remotes) { - t_remotes->push_back(addr); + if (newsock < 0) { + return; + } + auto closeSock = [newsock](const string& msg) { + try { + closesocket(newsock); } - ComboAddress mappedSource = addr; - if (!fromProxyProtocolSource && t_proxyMapping) { - if (const auto* iter = t_proxyMapping->lookup(addr)) { - mappedSource = iter->second.address; - ++iter->second.stats.netmaskMatches; - } - } - if (!fromProxyProtocolSource && t_allowFrom && !t_allowFrom->match(&mappedSource)) { - if (!g_quiet) { - SLOG(g_log << Logger::Error << "[" << g_multiTasker->getTid() << "] dropping TCP query from " << mappedSource.toString() << ", address neither matched by allow-from nor proxy-protocol-from" << endl, - g_slogtcpin->info(Logr::Error, "dropping TCP query address neither matched by allow-from nor proxy-protocol-from", "source", Logging::Loggable(mappedSource))); - } - t_Counters.at(rec::Counter::unauthorizedTCP)++; - try { - closesocket(newsock); - } - catch (const PDNSException& e) { - SLOG(g_log << Logger::Error << "Error closing TCP socket after an ACL drop: " << e.reason << endl, - g_slogtcpin->error(Logr::Error, e.reason, "Error closing TCP socket after an ACL drop", "exception", Logging::Loggable("PDNSException"))); - } - return; + catch (const PDNSException& e) { + g_slogtcpin->error(Logr::Error, e.reason, msg, "exception", Logging::Loggable("PDNSException")); } + }; - if (g_maxTCPPerClient > 0 && t_tcpClientCounts->count(addr) > 0 && (*t_tcpClientCounts)[addr] >= g_maxTCPPerClient) { - t_Counters.at(rec::Counter::tcpClientOverflow)++; - try { - closesocket(newsock); // don't call TCPConnection::closeAndCleanup here - did not enter it in the counts yet! - } - catch (const PDNSException& e) { - SLOG(g_log << Logger::Error << "Error closing TCP socket after an overflow drop: " << e.reason << endl, - g_slogtcpin->error(Logr::Error, e.reason, "Error closing TCP socket after an overflow drop", "exception", Logging::Loggable("PDNSException"))); - } - return; - } + if (TCPConnection::getCurrentConnections() >= g_maxTCPClients) { + t_Counters.at(rec::Counter::tcpClientOverflow)++; + closeSock("Error closing TCP socket after an overflow drop"); + return; + } + if (g_multiTasker->numProcesses() >= g_maxMThreads) { + t_Counters.at(rec::Counter::overCapacityDrops)++; + closeSock("Error closing TCP socket after an over capacity drop"); + return; + } - setNonBlocking(newsock); - setTCPNoDelay(newsock); - std::shared_ptr tcpConn = std::make_shared(newsock, addr); - tcpConn->d_source = addr; - tcpConn->d_destination = destaddr; - tcpConn->d_mappedSource = mappedSource; - - if (fromProxyProtocolSource) { - tcpConn->proxyProtocolNeed = s_proxyProtocolMinimumHeaderSize; - tcpConn->data.resize(tcpConn->proxyProtocolNeed); - tcpConn->state = TCPConnection::PROXYPROTOCOLHEADER; + ComboAddress destaddr; + socklen_t len = sizeof(destaddr); + getsockname(newsock, reinterpret_cast(&destaddr), &len); // if this fails, we're ok with it NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + bool fromProxyProtocolSource = expectProxyProtocol(addr, destaddr); + if (!fromProxyProtocolSource && t_remotes) { + t_remotes->push_back(addr); + } + ComboAddress mappedSource = addr; + if (!fromProxyProtocolSource && t_proxyMapping) { + if (const auto* iter = t_proxyMapping->lookup(addr)) { + mappedSource = iter->second.address; + ++iter->second.stats.netmaskMatches; } - else { - tcpConn->state = TCPConnection::BYTE0; + } + if (!fromProxyProtocolSource && t_allowFrom && !t_allowFrom->match(&mappedSource)) { + if (!g_quiet) { + SLOG(g_log << Logger::Error << "[" << g_multiTasker->getTid() << "] dropping TCP query from " << mappedSource.toString() << ", address neither matched by allow-from nor proxy-protocol-from" << endl, + g_slogtcpin->info(Logr::Error, "dropping TCP query address neither matched by allow-from nor proxy-protocol-from", "source", Logging::Loggable(mappedSource))); } + t_Counters.at(rec::Counter::unauthorizedTCP)++; + closeSock("Error closing TCP socket after an ACL drop"); + return; + } - struct timeval ttd - { - }; - Utility::gettimeofday(&ttd, nullptr); - ttd.tv_sec += g_tcpTimeout; + if (g_maxTCPPerClient > 0 && t_tcpClientCounts->count(addr) > 0 && (*t_tcpClientCounts)[addr] >= g_maxTCPPerClient) { + t_Counters.at(rec::Counter::tcpClientOverflow)++; + closeSock("Error closing TCP socket after a client overflow drop"); + return; + } - t_fdm->addReadFD(tcpConn->getFD(), handleRunningTCPQuestion, tcpConn, &ttd); + setNonBlocking(newsock); + setTCPNoDelay(newsock); + std::shared_ptr tcpConn = std::make_shared(newsock, addr); + tcpConn->d_source = addr; + tcpConn->d_destination = destaddr; + tcpConn->d_mappedSource = mappedSource; + + if (fromProxyProtocolSource) { + tcpConn->proxyProtocolNeed = s_proxyProtocolMinimumHeaderSize; + tcpConn->data.resize(tcpConn->proxyProtocolNeed); + tcpConn->state = TCPConnection::PROXYPROTOCOLHEADER; + } + else { + tcpConn->state = TCPConnection::BYTE0; } + + timeval ttd{}; + Utility::gettimeofday(&ttd, nullptr); + ttd.tv_sec += g_tcpTimeout; + + t_fdm->addReadFD(tcpConn->getFD(), handleRunningTCPQuestion, tcpConn, &ttd); } static void TCPIOHandlerIO(int fileDesc, FDMultiplexer::funcparam_t& var);