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 89543ee5671b475ea6d73e4cb7863363faf30f8f..6a1eb68358ba2af78b6898913d6f43b762e55310 100644 (file)
--- 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<int>(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 e30224e4c736d68079d7a30d290dad1ff3603594..40b75a6a848c5edee274e1592bdd807d9c7ab762 100644 (file)
--- 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 a1af4a10889e9084a3c381c42e15a32cdbf70961..353550d292665ecf96d9ffce6fe3fb531fce497e 100644 (file)
--- 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<struct sockaddr*>(&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<sockaddr*>(&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<TCPConnection> tcpConn = std::make_shared<TCPConnection>(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<sockaddr*>(&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<TCPConnection> tcpConn = std::make_shared<TCPConnection>(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);