Skip to content

Commit

Permalink
Avoid counting failed connect attempts when probably offline.
Browse files Browse the repository at this point in the history
If a node is offline failed outbound connection attempts will crank up
 the addrman counter and effectively blow away our state.

This change reduces the problem by only counting attempts made while
 the node believes it has outbound connections to at least two
 netgroups.

Connect and addnode connections are also not counted, as there is no
 reason to unequally penalize them for their more frequent
 connections -- though there should be no real effect from this
 unless their addnode configureation is later removed.

Wasteful repeated connection attempts while only a few connections are
 up are avoided via nLastTry.

This is still somewhat incomplete protection because our outbound
 peers could be down but not timed out or might all be on 'local'
 networks (although the requirement for multiple netgroups helps).
  • Loading branch information
gmaxwell authored and Fuzzbawls committed Mar 24, 2020
1 parent 01651b6 commit b988ce6
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/activemasternode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ void CActiveMasternode::ManageStatus()

LogPrintf("CActiveMasternode::ManageStatus() - Checking inbound connection to '%s'\n", service.ToString());

CNode* pnode = ConnectNode((CAddress)service, NULL, false);
CNode* pnode = ConnectNode((CAddress)service, NULL, false, true);
if (!pnode) {
notCapableReason = "Could not connect to " + service.ToString();
LogPrintf("CActiveMasternode::ManageStatus() - not capable: %s\n", notCapableReason);
Expand Down
4 changes: 2 additions & 2 deletions src/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP
return fNew;
}

void CAddrMan::Attempt_(const CService& addr, int64_t nTime)
void CAddrMan::Attempt_(const CService& addr, bool fCountFailure, int64_t nTime)
{
CAddrInfo* pinfo = Find(addr);

Expand All @@ -329,7 +329,7 @@ void CAddrMan::Attempt_(const CService& addr, int64_t nTime)

// update info
info.nLastTry = nTime;
info.nAttempts++;
if (fCountFailure) info.nAttempts++;
}

CAddrInfo CAddrMan::Select_(bool newOnly)
Expand Down
6 changes: 3 additions & 3 deletions src/addrman.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ class CAddrMan
bool Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty);

//! Mark an entry as attempted to connect.
void Attempt_(const CService& addr, int64_t nTime);
void Attempt_(const CService& addr, bool fCountFailure, int64_t nTime);

//! Select an address to connect to, if newOnly is set to true, only the new table is selected from.
CAddrInfo Select_(bool newOnly);
Expand Down Expand Up @@ -532,12 +532,12 @@ class CAddrMan
}

//! Mark an entry as connection attempted to.
void Attempt(const CService& addr, int64_t nTime = GetAdjustedTime())
void Attempt(const CService& addr, bool fCountFailure, int64_t nTime = GetAdjustedTime())
{
{
LOCK(cs);
Check();
Attempt_(addr, nTime);
Attempt_(addr, fCountFailure, nTime);
Check();
}
}
Expand Down
20 changes: 10 additions & 10 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ CNode* FindNode(const CService& addr)
return NULL;
}

CNode* ConnectNode(CAddress addrConnect, const char* pszDest, bool obfuScationMaster)
CNode* ConnectNode(CAddress addrConnect, const char* pszDest, bool obfuScationMaster, bool fCountFailure)
{
if (pszDest == NULL) {
// we clean masternode connections in CMasternodeMan::ProcessMasternodeConnections()
Expand Down Expand Up @@ -419,7 +419,7 @@ CNode* ConnectNode(CAddress addrConnect, const char* pszDest, bool obfuScationMa
return NULL;
}

addrman.Attempt(addrConnect);
addrman.Attempt(addrConnect, fCountFailure);

// Add node
CNode* pnode = new CNode(hSocket, addrConnect, pszDest ? pszDest : "", false);
Expand All @@ -437,7 +437,7 @@ CNode* ConnectNode(CAddress addrConnect, const char* pszDest, bool obfuScationMa
} else if (!proxyConnectionFailed) {
// If connecting to the node failed, and failure is not caused by a problem connecting to
// the proxy, mark this as an attempt.
addrman.Attempt(addrConnect);
addrman.Attempt(addrConnect, fCountFailure);
}

return NULL;
Expand Down Expand Up @@ -1338,7 +1338,7 @@ void static ProcessOneShot()
CAddress addr;
CSemaphoreGrant grant(*semOutbound, true);
if (grant) {
if (!OpenNetworkConnection(addr, &grant, strDest.c_str(), true))
if (!OpenNetworkConnection(addr, false, &grant, strDest.c_str(), true))
AddOneShot(strDest);
}
}
Expand All @@ -1351,7 +1351,7 @@ void ThreadOpenConnections()
ProcessOneShot();
for (std::string strAddr : mapMultiArgs["-connect"]) {
CAddress addr;
OpenNetworkConnection(addr, NULL, strAddr.c_str());
OpenNetworkConnection(addr, false, NULL, strAddr.c_str());
for (int i = 0; i < 10 && i < nLoop; i++) {
MilliSleep(500);
}
Expand Down Expand Up @@ -1432,7 +1432,7 @@ void ThreadOpenConnections()
}

if (addrConnect.IsValid())
OpenNetworkConnection(addrConnect, &grant);
OpenNetworkConnection(addrConnect, (int)setConnected.size() >= min(nMaxConnections - 1, 2), &grant);
}
}

Expand All @@ -1454,7 +1454,7 @@ void ThreadOpenAddedConnections()
for (std::string& strAddNode : lAddresses) {
CAddress addr;
CSemaphoreGrant grant(*semOutbound);
OpenNetworkConnection(addr, &grant, strAddNode.c_str());
OpenNetworkConnection(addr, false, &grant, strAddNode.c_str());
MilliSleep(500);
}
MilliSleep(120000); // Retry every 2 minutes
Expand Down Expand Up @@ -1496,15 +1496,15 @@ void ThreadOpenAddedConnections()
}
for (std::vector<CService>& vserv : lservAddressesToAdd) {
CSemaphoreGrant grant(*semOutbound);
OpenNetworkConnection(CAddress(vserv[i % vserv.size()]), &grant);
OpenNetworkConnection(CAddress(vserv[i % vserv.size()]), false, &grant);
MilliSleep(500);
}
MilliSleep(120000); // Retry every 2 minutes
}
}

// if successful, this moves the passed grant to the constructed node
bool OpenNetworkConnection(const CAddress& addrConnect, CSemaphoreGrant* grantOutbound, const char* pszDest, bool fOneShot)
bool OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* pszDest, bool fOneShot)
{
//
// Initiate outbound network connection
Expand All @@ -1518,7 +1518,7 @@ bool OpenNetworkConnection(const CAddress& addrConnect, CSemaphoreGrant* grantOu
} else if (FindNode(pszDest))
return false;

CNode* pnode = ConnectNode(addrConnect, pszDest);
CNode* pnode = ConnectNode(addrConnect, pszDest, false, fCountFailure);
boost::this_thread::interruption_point();

if (!pnode)
Expand Down
4 changes: 2 additions & 2 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ CNode* FindNode(const CNetAddr& ip);
CNode* FindNode(const CSubNet& subNet);
CNode* FindNode(const std::string& addrName);
CNode* FindNode(const CService& ip);
CNode* ConnectNode(CAddress addrConnect, const char* pszDest = NULL, bool obfuScationMaster = false);
bool OpenNetworkConnection(const CAddress& addrConnect, CSemaphoreGrant* grantOutbound = NULL, const char* strDest = NULL, bool fOneShot = false);
CNode* ConnectNode(CAddress addrConnect, const char* pszDest = NULL, bool obfuScationMaster = false, bool fCountFailure = false);
bool OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound = NULL, const char* strDest = NULL, bool fOneShot = false);
void MapPort(bool fUseUPnP);
unsigned short GetListenPort();
bool BindListenPort(const CService& bindAddr, std::string& strError, bool fWhitelisted = false);
Expand Down
2 changes: 1 addition & 1 deletion src/obfuscation-relay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ void CObfuScationRelay::RelayThroughNode(int nRank)

if (pmn != NULL) {
//printf("RelayThroughNode %s\n", pmn->addr.ToString().c_str());
CNode* pnode = ConnectNode((CAddress)pmn->addr, NULL, false);
CNode* pnode = ConnectNode((CAddress)pmn->addr, NULL, false, true);
if (pnode) {
//printf("Connected\n");
pnode->PushMessage("dsr", (*this));
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/masternode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ UniValue masternodeconnect(const UniValue& params, bool fHelp)

CService addr = CService(strAddress);

CNode* pnode = ConnectNode((CAddress)addr, NULL, false);
CNode* pnode = ConnectNode((CAddress)addr, NULL, false, true);
if (pnode) {
pnode->Release();
return NullUniValue;
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ UniValue addnode(const UniValue& params, bool fHelp)

if (strCommand == "onetry") {
CAddress addr;
OpenNetworkConnection(addr, NULL, strNode.c_str());
OpenNetworkConnection(addr, false, NULL, strNode.c_str());
return NullUniValue;
}

Expand Down

0 comments on commit b988ce6

Please sign in to comment.