Break addnode out from the outbound connection limits.

Previously addnodes were in competition with outbound connections
 for access to the eight outbound slots.

One result of this is that frequently a node with several addnode
 configured peers would end up connected to none of them, because
 while the addnode loop was in its two minute sleep the automatic
 connection logic would fill any free slots with random peers.
 This is particularly unwelcome to users trying to maintain links
 to specific nodes for fast block relay or purposes.

Another result is that a group of nine or more nodes which are
 have addnode configured towards each other can become partitioned
 from the public network.

This commit introduces a new limit of eight connections just for
 addnode peers which is not subject to any of the other connection
 limitations (including maxconnections).

The choice of eight is sufficient so that under no condition would
 a user find themselves connected to fewer addnoded peers than
 previously.  It is also low enough that users who are confused
 about the significance of more connections and have gotten too
 copy-and-paste happy will not consume more than twice the slot
 usage of a typical user.

Any additional load on the network resulting from this will likely
 be offset by a reduction in users applying even more wasteful
 workaround for the prior behavior.

The retry delays are reduced to avoid nodes sitting around without
 their added peers up, but are still sufficient to prevent overly
 aggressive repeated connections.  The reduced delays also make
 the system much more responsive to the addnode RPC.

Ban-disconnects are also exempted for peers added via addnode since
 the outbound addnode logic ignores bans.  Previously it would ban
 an addnode then immediately reconnect to it.

A minor change was also made to CSemaphoreGrant so that it is
 possible to re-acquire via an object whos grant was moved.
This commit is contained in:
Gregory Maxwell 2016-12-11 04:39:26 +00:00
parent ce43630d1e
commit 50bd12ce0c
6 changed files with 48 additions and 13 deletions

View file

@ -871,11 +871,11 @@ bool AppInitParameterInteraction()
nMaxConnections = std::max(nUserMaxConnections, 0); nMaxConnections = std::max(nUserMaxConnections, 0);
// Trim requested connection counts, to fit into system limitations // Trim requested connection counts, to fit into system limitations
nMaxConnections = std::max(std::min(nMaxConnections, (int)(FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS)), 0); nMaxConnections = std::max(std::min(nMaxConnections, (int)(FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS)), 0);
nFD = RaiseFileDescriptorLimit(nMaxConnections + MIN_CORE_FILEDESCRIPTORS); nFD = RaiseFileDescriptorLimit(nMaxConnections + MIN_CORE_FILEDESCRIPTORS + MAX_ADDNODE_CONNECTIONS);
if (nFD < MIN_CORE_FILEDESCRIPTORS) if (nFD < MIN_CORE_FILEDESCRIPTORS)
return InitError(_("Not enough file descriptors available.")); return InitError(_("Not enough file descriptors available."));
nMaxConnections = std::min(nFD - MIN_CORE_FILEDESCRIPTORS, nMaxConnections); nMaxConnections = std::min(nFD - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS, nMaxConnections);
if (nMaxConnections < nUserMaxConnections) if (nMaxConnections < nUserMaxConnections)
InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections)); InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections));
@ -1109,7 +1109,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
LogPrintf("Default data directory %s\n", GetDefaultDataDir().string()); LogPrintf("Default data directory %s\n", GetDefaultDataDir().string());
LogPrintf("Using data directory %s\n", GetDataDir().string()); LogPrintf("Using data directory %s\n", GetDataDir().string());
LogPrintf("Using config file %s\n", GetConfigFile(GetArg("-conf", BITCOIN_CONF_FILENAME)).string()); LogPrintf("Using config file %s\n", GetConfigFile(GetArg("-conf", BITCOIN_CONF_FILENAME)).string());
LogPrintf("Using at most %i connections (%i file descriptors available)\n", nMaxConnections, nFD); LogPrintf("Using at most %i automatic connections (%i file descriptors available)\n", nMaxConnections, nFD);
InitSignatureCache(); InitSignatureCache();
@ -1565,6 +1565,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
connOptions.nRelevantServices = nRelevantServices; connOptions.nRelevantServices = nRelevantServices;
connOptions.nMaxConnections = nMaxConnections; connOptions.nMaxConnections = nMaxConnections;
connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections); connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections);
connOptions.nMaxAddnode = MAX_ADDNODE_CONNECTIONS;
connOptions.nMaxFeeler = 1; connOptions.nMaxFeeler = 1;
connOptions.nBestHeight = chainActive.Height(); connOptions.nBestHeight = chainActive.Height();
connOptions.uiInterface = &uiInterface; connOptions.uiInterface = &uiInterface;

View file

@ -621,6 +621,7 @@ void CNode::copyStats(CNodeStats &stats)
X(nVersion); X(nVersion);
X(cleanSubVer); X(cleanSubVer);
X(fInbound); X(fInbound);
X(fAddnode);
X(nStartingHeight); X(nStartingHeight);
X(nSendBytes); X(nSendBytes);
X(mapSendBytesPerMsgCmd); X(mapSendBytesPerMsgCmd);
@ -1631,7 +1632,7 @@ void CConnman::ThreadOpenConnections()
{ {
LOCK(cs_vNodes); LOCK(cs_vNodes);
BOOST_FOREACH(CNode* pnode, vNodes) { BOOST_FOREACH(CNode* pnode, vNodes) {
if (!pnode->fInbound) { if (!pnode->fInbound && !pnode->fAddnode) {
setConnected.insert(pnode->addr.GetGroup()); setConnected.insert(pnode->addr.GetGroup());
nOutbound++; nOutbound++;
} }
@ -1776,27 +1777,35 @@ void CConnman::ThreadOpenAddedConnections()
vAddedNodes = mapMultiArgs.at("-addnode"); vAddedNodes = mapMultiArgs.at("-addnode");
} }
for (unsigned int i = 0; true; i++) while (true)
{ {
CSemaphoreGrant grant(*semAddnode);
std::vector<AddedNodeInfo> vInfo = GetAddedNodeInfo(); std::vector<AddedNodeInfo> vInfo = GetAddedNodeInfo();
bool tried = false;
for (const AddedNodeInfo& info : vInfo) { for (const AddedNodeInfo& info : vInfo) {
if (!info.fConnected) { if (!info.fConnected) {
CSemaphoreGrant grant(*semOutbound); if (!grant.TryAcquire()) {
// If we've used up our semaphore and need a new one, lets not wait here since while we are waiting
// the addednodeinfo state might change.
break;
}
// If strAddedNode is an IP/port, decode it immediately, so // If strAddedNode is an IP/port, decode it immediately, so
// OpenNetworkConnection can detect existing connections to that IP/port. // OpenNetworkConnection can detect existing connections to that IP/port.
tried = true;
CService service(LookupNumeric(info.strAddedNode.c_str(), Params().GetDefaultPort())); CService service(LookupNumeric(info.strAddedNode.c_str(), Params().GetDefaultPort()));
OpenNetworkConnection(CAddress(service, NODE_NONE), false, &grant, info.strAddedNode.c_str(), false); OpenNetworkConnection(CAddress(service, NODE_NONE), false, &grant, info.strAddedNode.c_str(), false, false, true);
if (!interruptNet.sleep_for(std::chrono::milliseconds(500))) if (!interruptNet.sleep_for(std::chrono::milliseconds(500)))
return; return;
} }
} }
if (!interruptNet.sleep_for(std::chrono::minutes(2))) // Retry every 60 seconds if a connection was attempted, otherwise two seconds
if (!interruptNet.sleep_for(std::chrono::seconds(tried ? 60 : 2)));
return; return;
} }
} }
// if successful, this moves the passed grant to the constructed node // if successful, this moves the passed grant to the constructed node
bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler) bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler, bool fAddnode)
{ {
// //
// Initiate outbound network connection // Initiate outbound network connection
@ -1825,6 +1834,8 @@ bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
pnode->fOneShot = true; pnode->fOneShot = true;
if (fFeeler) if (fFeeler)
pnode->fFeeler = true; pnode->fFeeler = true;
if (fAddnode)
pnode->fAddnode = true;
return true; return true;
} }
@ -2076,8 +2087,10 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : nSeed0(nSeed0In), nSe
nSendBufferMaxSize = 0; nSendBufferMaxSize = 0;
nReceiveFloodSize = 0; nReceiveFloodSize = 0;
semOutbound = NULL; semOutbound = NULL;
semAddnode = NULL;
nMaxConnections = 0; nMaxConnections = 0;
nMaxOutbound = 0; nMaxOutbound = 0;
nMaxAddnode = 0;
nBestHeight = 0; nBestHeight = 0;
clientInterface = NULL; clientInterface = NULL;
flagInterruptMsgProc = false; flagInterruptMsgProc = false;
@ -2099,6 +2112,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
nLocalServices = connOptions.nLocalServices; nLocalServices = connOptions.nLocalServices;
nMaxConnections = connOptions.nMaxConnections; nMaxConnections = connOptions.nMaxConnections;
nMaxOutbound = std::min((connOptions.nMaxOutbound), nMaxConnections); nMaxOutbound = std::min((connOptions.nMaxOutbound), nMaxConnections);
nMaxAddnode = connOptions.nMaxAddnode;
nMaxFeeler = connOptions.nMaxFeeler; nMaxFeeler = connOptions.nMaxFeeler;
nSendBufferMaxSize = connOptions.nSendBufferMaxSize; nSendBufferMaxSize = connOptions.nSendBufferMaxSize;
@ -2151,6 +2165,10 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
// initialize semaphore // initialize semaphore
semOutbound = new CSemaphore(std::min((nMaxOutbound + nMaxFeeler), nMaxConnections)); semOutbound = new CSemaphore(std::min((nMaxOutbound + nMaxFeeler), nMaxConnections));
} }
if (semAddnode == NULL) {
// initialize semaphore
semAddnode = new CSemaphore(nMaxAddnode);
}
// //
// Start threads // Start threads
@ -2227,6 +2245,10 @@ void CConnman::Stop()
if (threadSocketHandler.joinable()) if (threadSocketHandler.joinable())
threadSocketHandler.join(); threadSocketHandler.join();
if (semAddnode)
for (int i=0; i<nMaxAddnode; i++)
semOutbound->post();
if (fAddressesInitialized) if (fAddressesInitialized)
{ {
DumpData(); DumpData();
@ -2254,6 +2276,8 @@ void CConnman::Stop()
vhListenSocket.clear(); vhListenSocket.clear();
delete semOutbound; delete semOutbound;
semOutbound = NULL; semOutbound = NULL;
delete semAddnode;
semAddnode = NULL;
} }
void CConnman::DeleteNode(CNode* pnode) void CConnman::DeleteNode(CNode* pnode)
@ -2554,6 +2578,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
strSubVer = ""; strSubVer = "";
fWhitelisted = false; fWhitelisted = false;
fOneShot = false; fOneShot = false;
fAddnode = false;
fClient = false; // set by version message fClient = false; // set by version message
fFeeler = false; fFeeler = false;
fSuccessfullyConnected = false; fSuccessfullyConnected = false;

View file

@ -58,8 +58,10 @@ static const unsigned int MAX_ADDR_TO_SEND = 1000;
static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 4 * 1000 * 1000; static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 4 * 1000 * 1000;
/** Maximum length of strSubVer in `version` message */ /** Maximum length of strSubVer in `version` message */
static const unsigned int MAX_SUBVERSION_LENGTH = 256; static const unsigned int MAX_SUBVERSION_LENGTH = 256;
/** Maximum number of outgoing nodes */ /** Maximum number of automatic outgoing nodes */
static const int MAX_OUTBOUND_CONNECTIONS = 8; static const int MAX_OUTBOUND_CONNECTIONS = 8;
/** Maximum number of addnode outgoing nodes */
static const int MAX_ADDNODE_CONNECTIONS = 8;
/** -listen default */ /** -listen default */
static const bool DEFAULT_LISTEN = true; static const bool DEFAULT_LISTEN = true;
/** -upnp default */ /** -upnp default */
@ -135,6 +137,7 @@ public:
ServiceFlags nRelevantServices = NODE_NONE; ServiceFlags nRelevantServices = NODE_NONE;
int nMaxConnections = 0; int nMaxConnections = 0;
int nMaxOutbound = 0; int nMaxOutbound = 0;
int nMaxAddnode = 0;
int nMaxFeeler = 0; int nMaxFeeler = 0;
int nBestHeight = 0; int nBestHeight = 0;
CClientUIInterface* uiInterface = nullptr; CClientUIInterface* uiInterface = nullptr;
@ -151,7 +154,7 @@ public:
bool BindListenPort(const CService &bindAddr, std::string& strError, bool fWhitelisted = false); bool BindListenPort(const CService &bindAddr, std::string& strError, bool fWhitelisted = false);
bool GetNetworkActive() const { return fNetworkActive; }; bool GetNetworkActive() const { return fNetworkActive; };
void SetNetworkActive(bool active); void SetNetworkActive(bool active);
bool OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = NULL, const char *strDest = NULL, bool fOneShot = false, bool fFeeler = false); bool OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = NULL, const char *strDest = NULL, bool fOneShot = false, bool fFeeler = false, bool fAddnode = false);
bool CheckIncomingNonce(uint64_t nonce); bool CheckIncomingNonce(uint64_t nonce);
bool ForNode(NodeId id, std::function<bool(CNode* pnode)> func); bool ForNode(NodeId id, std::function<bool(CNode* pnode)> func);
@ -414,8 +417,10 @@ private:
ServiceFlags nRelevantServices; ServiceFlags nRelevantServices;
CSemaphore *semOutbound; CSemaphore *semOutbound;
CSemaphore *semAddnode;
int nMaxConnections; int nMaxConnections;
int nMaxOutbound; int nMaxOutbound;
int nMaxAddnode;
int nMaxFeeler; int nMaxFeeler;
std::atomic<int> nBestHeight; std::atomic<int> nBestHeight;
CClientUIInterface* clientInterface; CClientUIInterface* clientInterface;
@ -529,6 +534,7 @@ public:
int nVersion; int nVersion;
std::string cleanSubVer; std::string cleanSubVer;
bool fInbound; bool fInbound;
bool fAddnode;
int nStartingHeight; int nStartingHeight;
uint64_t nSendBytes; uint64_t nSendBytes;
mapMsgCmdSize mapSendBytesPerMsgCmd; mapMsgCmdSize mapSendBytesPerMsgCmd;
@ -626,6 +632,7 @@ public:
bool fWhitelisted; // This peer can bypass DoS banning. bool fWhitelisted; // This peer can bypass DoS banning.
bool fFeeler; // If true this node is being used as a short lived feeler. bool fFeeler; // If true this node is being used as a short lived feeler.
bool fOneShot; bool fOneShot;
bool fAddnode;
bool fClient; bool fClient;
const bool fInbound; const bool fInbound;
bool fSuccessfullyConnected; bool fSuccessfullyConnected;

View file

@ -2644,6 +2644,8 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
state.fShouldBan = false; state.fShouldBan = false;
if (pto->fWhitelisted) if (pto->fWhitelisted)
LogPrintf("Warning: not punishing whitelisted peer %s!\n", pto->addr.ToString()); LogPrintf("Warning: not punishing whitelisted peer %s!\n", pto->addr.ToString());
else if (pto->fAddnode)
LogPrintf("Warning: not punishing addnoded peer %s!\n", pto->addr.ToString());
else { else {
pto->fDisconnect = true; pto->fDisconnect = true;
if (pto->addr.IsLocal()) if (pto->addr.IsLocal())

View file

@ -152,6 +152,7 @@ UniValue getpeerinfo(const JSONRPCRequest& request)
// their ver message. // their ver message.
obj.push_back(Pair("subver", stats.cleanSubVer)); obj.push_back(Pair("subver", stats.cleanSubVer));
obj.push_back(Pair("inbound", stats.fInbound)); obj.push_back(Pair("inbound", stats.fInbound));
obj.push_back(Pair("addnode", stats.fAddnode));
obj.push_back(Pair("startingheight", stats.nStartingHeight)); obj.push_back(Pair("startingheight", stats.nStartingHeight));
if (fStateStats) { if (fStateStats) {
obj.push_back(Pair("banscore", statestats.nMisbehavior)); obj.push_back(Pair("banscore", statestats.nMisbehavior));

View file

@ -264,7 +264,6 @@ public:
grant.Release(); grant.Release();
grant.sem = sem; grant.sem = sem;
grant.fHaveGrant = fHaveGrant; grant.fHaveGrant = fHaveGrant;
sem = NULL;
fHaveGrant = false; fHaveGrant = false;
} }