From: Remi Gacogne Date: Thu, 6 Mar 2025 08:44:30 +0000 (+0100) Subject: dnsdist: Limit # of proxy protocol-enabled outgoing TCP connections X-Git-Tag: dnsdist-2.0.0-alpha1~6^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a2d874c4f970813a768a6c6f2be62eb87be4db61;p=thirdparty%2Fpdns.git dnsdist: Limit # of proxy protocol-enabled outgoing TCP connections TCP worker threads keep a cache of outgoing TCP connections to a backend to be able to reuse them for subsequent queries. Proxy protocol-enabled outgoing TCP connections are trickier because the proxy protocol payload is sent only once at the beginning of the TCP connection, contains the source and destination addresses and ports, and thus the connections can only be reused with the exact same incoming TCP connection. For this reason these connections are stored in a specific structure of the incoming connection, instead of the TCP worker connection cache. However, we can only reuse a given proxy protocol-enabled outgoing TCP connection for a subsequent query if the TLV values contained in the proxy-protocol payload associated to the new query are exactly the same than the ones associated to the existing query. Up until now, we would keep an unbounded amount of proxy protocol-enabled connections around if the TLV values were, for example, randomly assigned per query. This commit sets a limit on the number of such connections we will keep around: we will keep at most N connections, where N is the ratio between the number of concurrent queries on a single TCP connection supported by the backend and the number of concurrent queries on a single TCP connection supported by the frontend, with a hard cap to 5. --- diff --git a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh index e7c3a1a3b9..4e66bd58bf 100644 --- a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh +++ b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh @@ -115,7 +115,6 @@ public: return false; } - std::shared_ptr getOwnedDownstreamConnection(const std::shared_ptr& backend, const std::unique_ptr>& tlvs); std::shared_ptr getDownstreamConnection(std::shared_ptr& backend, const std::unique_ptr>& tlvs, const struct timeval& now); void registerOwnedDownstreamConnection(std::shared_ptr& conn); diff --git a/pdns/dnsdistdist/dnsdist-tcp.cc b/pdns/dnsdistdist/dnsdist-tcp.cc index 5454a156a8..db61d8817f 100644 --- a/pdns/dnsdistdist/dnsdist-tcp.cc +++ b/pdns/dnsdistdist/dnsdist-tcp.cc @@ -99,14 +99,41 @@ size_t IncomingTCPConnectionState::clearAllDownstreamConnections() return t_downstreamTCPConnectionsManager.clear(); } +static std::pair, bool> getOwnedDownstreamConnection(std::map, std::deque>>& ownedConnectionsToBackend, const std::shared_ptr& backend, const std::unique_ptr>& tlvs) +{ + bool tlvsMismatch = false; + auto connIt = ownedConnectionsToBackend.find(backend); + if (connIt == ownedConnectionsToBackend.end()) { + DEBUGLOG("no owned connection found for " << backend->getName()); + return {nullptr, tlvsMismatch}; + } + + for (auto& conn : connIt->second) { + if (conn->canBeReused(true)) { + if (conn->matchesTLVs(tlvs)) { + DEBUGLOG("Got one owned connection accepting more for " << backend->getName()); + conn->setReused(); + return {conn, tlvsMismatch}; + } + DEBUGLOG("Found one connection to " << backend->getName() << " but with different TLV values"); + tlvsMismatch = true; + } + DEBUGLOG("not accepting more for " << backend->getName()); + } + + return {nullptr, tlvsMismatch}; +} + std::shared_ptr IncomingTCPConnectionState::getDownstreamConnection(std::shared_ptr& backend, const std::unique_ptr>& tlvs, const struct timeval& now) { - auto downstream = getOwnedDownstreamConnection(backend, tlvs); + auto [downstream, tlvsMismatch] = getOwnedDownstreamConnection(d_ownedConnectionsToBackend, backend, tlvs); if (!downstream) { /* we don't have a connection to this backend owned yet, let's get one (it might not be a fresh one, though) */ downstream = t_downstreamTCPConnectionsManager.getConnectionToDownstream(d_threadData.mplexer, backend, now, std::string()); - if (backend->d_config.useProxyProtocol) { + // if we had an existing connection but the TLVs are different, they are likely unique per query so do not bother keeping the connection + // around + if (backend->d_config.useProxyProtocol && !tlvsMismatch) { registerOwnedDownstreamConnection(downstream); } } @@ -260,29 +287,26 @@ void IncomingTCPConnectionState::resetForNewQuery() d_state = State::waitingForQuery; } -std::shared_ptr IncomingTCPConnectionState::getOwnedDownstreamConnection(const std::shared_ptr& backend, const std::unique_ptr>& tlvs) -{ - auto connIt = d_ownedConnectionsToBackend.find(backend); - if (connIt == d_ownedConnectionsToBackend.end()) { - DEBUGLOG("no owned connection found for " << backend->getName()); - return nullptr; - } - - for (auto& conn : connIt->second) { - if (conn->canBeReused(true) && conn->matchesTLVs(tlvs)) { - DEBUGLOG("Got one owned connection accepting more for " << backend->getName()); - conn->setReused(); - return conn; - } - DEBUGLOG("not accepting more for " << backend->getName()); - } - - return nullptr; -} - void IncomingTCPConnectionState::registerOwnedDownstreamConnection(std::shared_ptr& conn) { - d_ownedConnectionsToBackend[conn->getDS()].push_front(conn); + const auto& downstream = conn->getDS(); + auto& queue = d_ownedConnectionsToBackend[downstream]; + // how many proxy-protocol enabled connections do we want to keep around? + // - they are only usable for this incoming connection because of the proxy protocol header containing the source and destination addresses and ports + // - if we have TLV values, and they are unique per query, keeping these is useless + // - if there is no, or identical, TLV values, a single outgoing connection is enough if maxInFlight == 1, or if incoming maxInFlight == outgoing maxInFlight + // so it makes sense to keep a few of them around if incoming maxInFlight is greater than outgoing maxInFlight + + auto incomingMaxInFlightQueriesPerConn = d_ci.cs->d_maxInFlightQueriesPerConn; + incomingMaxInFlightQueriesPerConn = std::max(incomingMaxInFlightQueriesPerConn, static_cast(1U)); + auto outgoingMaxInFlightQueriesPerConn = downstream->d_config.d_maxInFlightQueriesPerConn; + outgoingMaxInFlightQueriesPerConn = std::max(outgoingMaxInFlightQueriesPerConn, static_cast(1U)); + size_t maxCachedOutgoingConnections = std::min(static_cast(std::round(incomingMaxInFlightQueriesPerConn / outgoingMaxInFlightQueriesPerConn)), static_cast(5U)); + + queue.push_front(conn); + if (queue.size() > maxCachedOutgoingConnections) { + queue.pop_back(); + } } /* called when the buffer has been set and the rules have been processed, and only from handleIO (sometimes indirectly via handleQuery) */