From e09dded59596c5d84152ab199b139032daa03ac3 Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Wed, 8 May 2024 15:27:55 -0400 Subject: [PATCH 1/3] init: fixes fd accounting regarding poll/select We are computing our file descriptors limits based on whether we use poll or select. However, we are taking that into account only partially (subtracting from fd_max in one case, but from nFD later on). Moreover, nBind is also only accounted for partially. Simplify and fix this logic --- src/init.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 1e6d296551413..67628650defbc 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -973,18 +973,16 @@ bool AppInitParameterInteraction(const ArgsManager& args) nMaxConnections = std::max(nUserMaxConnections, 0); nFD = RaiseFileDescriptorLimit(nMaxConnections + MIN_CORE_FILEDESCRIPTORS + MAX_ADDNODE_CONNECTIONS + nBind + NUM_FDS_MESSAGE_CAPTURE); - -#ifdef USE_POLL - int fd_max = nFD; -#else - int fd_max = FD_SETSIZE; + // If we are using select instead of poll, our actual limit may be even smaller +#ifndef USE_POLL + nFD = std::min(FD_SETSIZE, nFD); #endif - // Trim requested connection counts, to fit into system limitations - // in std::min(...) to work around FreeBSD compilation issue described in #2695 - nMaxConnections = std::max(std::min(nMaxConnections, fd_max - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS - NUM_FDS_MESSAGE_CAPTURE), 0); if (nFD < MIN_CORE_FILEDESCRIPTORS) return InitError(_("Not enough file descriptors available.")); - nMaxConnections = std::min(nFD - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS - NUM_FDS_MESSAGE_CAPTURE, nMaxConnections); + + // Trim requested connection counts, to fit into system limitations + // in std::min(...) to work around FreeBSD compilation issue described in #2695 + nMaxConnections = std::max(std::min(nMaxConnections, nFD - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS - NUM_FDS_MESSAGE_CAPTURE), 0); if (nMaxConnections < nUserMaxConnections) InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections)); From 0a7030d312c874213eb1d7127cc701e85841bf3f Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Wed, 8 May 2024 15:09:05 -0400 Subject: [PATCH 2/3] init: improves file descriptors accounting and docs The current logic for file descriptor accounting is pretty convoluted and hard to follow. This is partially caused by the lack of documentation plus non-intuitive variable naming (which was more intuitive when fewer things were accounted for, but hasn't aged well). This has led to this accounting being error-prone and hard to maintain (as shown in he previous commit). Redefine some of the constants, plus document what are we accounting for so this can be extended more easily Remove FreeBSD workaround to #2695 --- src/init.cpp | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 67628650defbc..3d97092922295 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -142,11 +142,12 @@ static constexpr bool DEFAULT_STOPAFTERBLOCKIMPORT{false}; // Win32 LevelDB doesn't use filedescriptors, and the ones used for // accessing block files don't count towards the fd_set size limit // anyway. -#define MIN_CORE_FILEDESCRIPTORS 0 +#define MIN_LEVELDB_FDS 0 #else -#define MIN_CORE_FILEDESCRIPTORS 150 +#define MIN_LEVELDB_FDS 150 #endif +static const int MIN_CORE_FDS = MIN_LEVELDB_FDS + NUM_FDS_MESSAGE_CAPTURE; static const char* DEFAULT_ASMAP_FILENAME="ip_asn.map"; /** @@ -820,7 +821,7 @@ namespace { // Variables internal to initialization process only int nMaxConnections; int nUserMaxConnections; -int nFD; +int available_fds; ServiceFlags nLocalServices = ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); int64_t peer_connect_timeout; std::set g_enabled_filter_types; @@ -967,22 +968,28 @@ bool AppInitParameterInteraction(const ArgsManager& args) return InitError(Untranslated("Cannot set -listen=0 together with -listenonion=1")); } - // Make sure enough file descriptors are available + // Make sure enough file descriptors are available. We need to reserve enough FDs to account for the bare minimum, + // plus all manual connections and all bound interfaces. Any remainder will be available for connection sockets + + // Number of bound interfaces (we have at least one) int nBind = std::max(nUserBind, size_t(1)); + // Maximum number of connections with other nodes, this accounts for all types of outbounds and inbounds except for manual nUserMaxConnections = args.GetIntArg("-maxconnections", DEFAULT_MAX_PEER_CONNECTIONS); nMaxConnections = std::max(nUserMaxConnections, 0); + // Reserve enough FDs to account for the bare minimum, plus any manual connections, plus the bound interfaces + int min_required_fds = MIN_CORE_FDS + MAX_ADDNODE_CONNECTIONS + nBind; - nFD = RaiseFileDescriptorLimit(nMaxConnections + MIN_CORE_FILEDESCRIPTORS + MAX_ADDNODE_CONNECTIONS + nBind + NUM_FDS_MESSAGE_CAPTURE); + // Try raising the FD limit to what we need (available_fds may be smaller than the requested amount if this fails) + available_fds = RaiseFileDescriptorLimit(nMaxConnections + min_required_fds); // If we are using select instead of poll, our actual limit may be even smaller #ifndef USE_POLL - nFD = std::min(FD_SETSIZE, nFD); + available_fds = std::min(FD_SETSIZE, available_fds); #endif - if (nFD < MIN_CORE_FILEDESCRIPTORS) - return InitError(_("Not enough file descriptors available.")); + if (available_fds < min_required_fds) + return InitError(strprintf(_("Not enough file descriptors available. %d available, %d required."), available_fds, min_required_fds)); // Trim requested connection counts, to fit into system limitations - // in std::min(...) to work around FreeBSD compilation issue described in #2695 - nMaxConnections = std::max(std::min(nMaxConnections, nFD - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS - NUM_FDS_MESSAGE_CAPTURE), 0); + nMaxConnections = std::min(available_fds - min_required_fds, nMaxConnections); if (nMaxConnections < nUserMaxConnections) InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections)); @@ -1128,7 +1135,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return false; } - LogPrintf("Using at most %i automatic connections (%i file descriptors available)\n", nMaxConnections, nFD); + LogPrintf("Using at most %i automatic connections (%i file descriptors available)\n", nMaxConnections, available_fds); // Warn about relative -datadir path. if (args.IsArgSet("-datadir") && !args.GetPathArg("-datadir").is_absolute()) { From fe76929d4d68a4ca6d27c58c1e243a64fdc70b2a Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Fri, 24 May 2024 11:55:53 -0400 Subject: [PATCH 3/3] init: error out if -maxconnections is negative --- src/init.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 3d97092922295..79cb91a1520c8 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -975,12 +975,14 @@ bool AppInitParameterInteraction(const ArgsManager& args) int nBind = std::max(nUserBind, size_t(1)); // Maximum number of connections with other nodes, this accounts for all types of outbounds and inbounds except for manual nUserMaxConnections = args.GetIntArg("-maxconnections", DEFAULT_MAX_PEER_CONNECTIONS); - nMaxConnections = std::max(nUserMaxConnections, 0); + if (nUserMaxConnections < 0){ + return InitError(Untranslated("-maxconnections must be greater or equal than zero")); + } // Reserve enough FDs to account for the bare minimum, plus any manual connections, plus the bound interfaces int min_required_fds = MIN_CORE_FDS + MAX_ADDNODE_CONNECTIONS + nBind; // Try raising the FD limit to what we need (available_fds may be smaller than the requested amount if this fails) - available_fds = RaiseFileDescriptorLimit(nMaxConnections + min_required_fds); + available_fds = RaiseFileDescriptorLimit(nUserMaxConnections + min_required_fds); // If we are using select instead of poll, our actual limit may be even smaller #ifndef USE_POLL available_fds = std::min(FD_SETSIZE, available_fds); @@ -989,7 +991,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) return InitError(strprintf(_("Not enough file descriptors available. %d available, %d required."), available_fds, min_required_fds)); // Trim requested connection counts, to fit into system limitations - nMaxConnections = std::min(available_fds - min_required_fds, nMaxConnections); + nMaxConnections = std::min(available_fds - min_required_fds, nUserMaxConnections); if (nMaxConnections < nUserMaxConnections) InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections));