From b277b68eb47ca4fefddd56de56ac699bf8cd147d Mon Sep 17 00:00:00 2001 From: chromatic Date: Thu, 25 Nov 2021 13:22:11 -0800 Subject: [PATCH] Set maximum/minimum connection thresholds This uses the constant in src/net.h for the minimum allowed number of connections. The limit of new max connections is silently capped to the number of available file descriptors. This value is not exposed in the UI or RPC messages so as not to leak interesting or important system details. The floor of maximum connections is set to the number of connections required for this node to operate. --- qa/rpc-tests/setmaxconnections.py | 36 +++++++++++++++++++------------ src/init.cpp | 5 ++++- src/net.cpp | 10 +++++++-- src/net.h | 2 ++ src/rpc/net.cpp | 9 ++++---- 5 files changed, 41 insertions(+), 21 deletions(-) diff --git a/qa/rpc-tests/setmaxconnections.py b/qa/rpc-tests/setmaxconnections.py index 232c83a5a..e4874b4ad 100644 --- a/qa/rpc-tests/setmaxconnections.py +++ b/qa/rpc-tests/setmaxconnections.py @@ -6,6 +6,8 @@ # # Exercise setmaxconnections RPC command # +# from the constant MAX_ADDNODE_CONNECTIONS in src/net.h +MINIMUM_CONNECTIONS = 8 from test_framework.mininode import * from test_framework.test_framework import BitcoinTestFramework @@ -63,9 +65,9 @@ class SetMaxConnectionCountTest (BitcoinTestFramework): def run_test(self): self.test_rpc_argument_validation() - self.test_node_connection_changes(5) - self.test_node_connection_changes(10) - self.test_node_connection_changes(3) + self.test_node_connection_changes(12) + self.test_node_connection_changes(20) + self.test_node_connection_changes(9) # max_count has to be at least 20 # min_count can be closer to 20 @@ -78,7 +80,7 @@ class SetMaxConnectionCountTest (BitcoinTestFramework): first_node.setmaxconnections() raise AssertionError("Must check for no parameter provided") except JSONRPCException as e: - assert("1. \"maxconnectioncount\"" in e.error['message']) + assert("1. maxconnectioncount" in e.error['message']) try: first_node.setmaxconnections("good doge bad doge") @@ -88,21 +90,27 @@ class SetMaxConnectionCountTest (BitcoinTestFramework): try: first_node.setmaxconnections(-1) - raise AssertionError("Must check for parameter value >= 0") + raise AssertionError(f"Must check for parameter value >= {MINIMUM_CONNECTIONS}") except JSONRPCException as e: - assert("maxconnectioncount must be >= 0" in e.error['message']) + assert(f"maxconnectioncount must be >= {MINIMUM_CONNECTIONS}" in e.error['message']) try: - first_node.setmaxconnections(0) + first_node.setmaxconnections(7) + raise AssertionError(f"Must check for parameter value >= {MINIMUM_CONNECTIONS}") + except JSONRPCException as e: + assert(f"maxconnectioncount must be >= {MINIMUM_CONNECTIONS}" in e.error['message']) + + try: + first_node.setmaxconnections(MINIMUM_CONNECTIONS) assert(True) except JSONRPCException as e: - raise AssertionError("Must allow parameter value >= 0") + raise AssertionError(f"Must allow parameter value >= {MINIMUM_CONNECTIONS}") def test_node_connection_changes(self, extras): first_node = self.nodes[0] - # 9 is 8 outgoing connections plus 1 feeler - first_node.setmaxconnections(9 + extras) + # MINIMUM_CONNECTIONS outgoing connections plus 1 feeler + first_node.setmaxconnections(1 + MINIMUM_CONNECTIONS + extras) client_nodes = [] self.connect_nodes(client_nodes, extras) @@ -119,17 +127,17 @@ class SetMaxConnectionCountTest (BitcoinTestFramework): for node in client_nodes: node.close() - first_node.setmaxconnections(0) + first_node.setmaxconnections(MINIMUM_CONNECTIONS) x = first_node.getconnectioncount() - assert(x == 0) + assert(x <= MINIMUM_CONNECTIONS) def test_node_disconnections(self, max_count, min_count): first_node = self.nodes[0] attempted_nodes = [] - # 9 is 8 outgoing connections plus 1 feeler - first_node.setmaxconnections(9 + max_count) + # MINIMUM_CONNECTIONS outgoing connections plus 1 feeler + first_node.setmaxconnections(1 + MINIMUM_CONNECTIONS + max_count) client_nodes = [] self.connect_nodes(client_nodes, max_count) diff --git a/src/init.cpp b/src/init.cpp index 192cc6a2c..e82250f2c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -803,6 +803,7 @@ ServiceFlags nRelevantServices = NODE_NETWORK; int nMaxConnections; int nUserMaxConnections; int nFD; +int nAvailableFds; ServiceFlags nLocalServices = NODE_NETWORK; } @@ -903,7 +904,8 @@ bool AppInitParameterInteraction() nFD = RaiseFileDescriptorLimit(nMaxConnections + MIN_CORE_FILEDESCRIPTORS + MAX_ADDNODE_CONNECTIONS); if (nFD < MIN_CORE_FILEDESCRIPTORS) return InitError(_("Not enough file descriptors available.")); - nMaxConnections = std::min(nFD - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS, nMaxConnections); + nAvailableFds = nFD - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS; + nMaxConnections = std::min(nAvailableFds, nMaxConnections); if (nMaxConnections < nUserMaxConnections) InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections)); @@ -1660,6 +1662,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) connOptions.nLocalServices = nLocalServices; connOptions.nRelevantServices = nRelevantServices; connOptions.nMaxConnections = nMaxConnections; + connOptions.nAvailableFds = nAvailableFds; connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections); connOptions.nMaxAddnode = MAX_ADDNODE_CONNECTIONS; connOptions.nMaxFeeler = 1; diff --git a/src/net.cpp b/src/net.cpp index 9dfa56ea3..0d1fcd1fe 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1408,7 +1408,7 @@ void CConnman::ThreadSocketHandler() // if (vNodesCopy.size() > (size_t)nMaxConnections) { - LogPrintf("%s: attempting to reduce connections: max=%u current=%u", __func__, vNodesCopy.size(), nMaxConnections); + LogPrintf("%s: attempting to reduce connections: max=%u current=%u\n", __func__, nMaxConnections, vNodesCopy.size()); DisconnectUnusedNodes(); DeleteDisconnectedNodes(); AttemptToEvictConnection(); @@ -1424,7 +1424,11 @@ void CConnman::ThreadSocketHandler() void CConnman::SetMaxConnections(int newMaxConnections) { - nMaxConnections = newMaxConnections; + newMaxConnections = std::max(newMaxConnections, MAX_ADDNODE_CONNECTIONS); + nMaxConnections = std::min(newMaxConnections, nAvailableFds); + if (nMaxConnections != newMaxConnections) { + LogPrintf("%s: capped new maxconnections request of %d to %d\n", __func__, newMaxConnections, nMaxConnections); + } } @@ -2226,6 +2230,7 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : nSeed0(nSeed0In), nSe semAddnode = NULL; nMaxConnections = 0; nMaxOutbound = 0; + nAvailableFds = 0; nMaxAddnode = 0; nBestHeight = 0; clientInterface = NULL; @@ -2248,6 +2253,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c nMaxOutbound = std::min((connOptions.nMaxOutbound), nMaxConnections); nMaxAddnode = connOptions.nMaxAddnode; nMaxFeeler = connOptions.nMaxFeeler; + nAvailableFds = connOptions.nAvailableFds; nSendBufferMaxSize = connOptions.nSendBufferMaxSize; nReceiveFloodSize = connOptions.nReceiveFloodSize; diff --git a/src/net.h b/src/net.h index bc5dfc2b4..fa2060943 100644 --- a/src/net.h +++ b/src/net.h @@ -140,6 +140,7 @@ public: int nMaxOutbound = 0; int nMaxAddnode = 0; int nMaxFeeler = 0; + int nAvailableFds = 0; int nBestHeight = 0; CClientUIInterface* uiInterface = nullptr; unsigned int nSendBufferMaxSize = 0; @@ -388,6 +389,7 @@ private: int nMaxOutbound; int nMaxAddnode; int nMaxFeeler; + int nAvailableFds; std::atomic nBestHeight; CClientUIInterface* clientInterface; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 396686fe0..9e9bdb085 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -48,24 +48,25 @@ UniValue getconnectioncount(const JSONRPCRequest& request) UniValue setmaxconnections(const JSONRPCRequest& request) { int newMaxCount = 0; + const std::string minConnCount = to_string(MAX_ADDNODE_CONNECTIONS); if (request.fHelp || request.params.size() != 1) throw runtime_error( "setmaxconnections\n" "\nSets the maximum number of connections to other nodes.\n" "\nArguments:\n" - "1. \"maxconnectioncount\" (numeric, required) The new maximum connection count (must be >= 0)\n" + "1. maxconnectioncount (numeric, required) The new maximum connection count (must be >= " + minConnCount + ")\n" "\nResult:\n" "n (boolean) True or false connection count\n" "\nExamples:\n" + HelpExampleCli("setmaxconnections", "20") - + HelpExampleRpc("setmaxconnections", "0") + + HelpExampleRpc("setmaxconnections", minConnCount) ); else newMaxCount = request.params[0].get_int(); - if (newMaxCount < 0) - throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: maxconnectioncount must be >= 0"); + if (newMaxCount < MAX_ADDNODE_CONNECTIONS) + throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: maxconnectioncount must be >= " + minConnCount); if(!g_connman) throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");