From 034f61f83b9348664d868933dbbfd8f9f8882168 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 7 Feb 2023 16:03:32 -0500 Subject: [PATCH] p2p: Protect extra full outbound peers by network If a peer is the only one of its network, protect it from eviction. This improves the diversity of outbound connections with respect to reachable networks. Co-authored-by: Amiti Uttarwar --- src/net.cpp | 6 ++++++ src/net.h | 5 +++++ src/net_processing.cpp | 7 ++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index bfbca4baf8..00ee873c69 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1607,6 +1607,12 @@ std::unordered_set CConnman::GetReachableEmptyNetworks() const return networks; } +bool CConnman::MultipleManualOrFullOutboundConns(Network net) const +{ + AssertLockHeld(m_nodes_mutex); + return m_network_conn_counts[net] > 1; +} + void CConnman::ThreadOpenConnections(const std::vector connect) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); diff --git a/src/net.h b/src/net.h index edff55ce7a..a4c810b0ae 100644 --- a/src/net.h +++ b/src/net.h @@ -793,6 +793,9 @@ public: void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); bool CheckIncomingNonce(uint64_t nonce); + // alias for thread safety annotations only, not defined + RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); + bool ForNode(NodeId id, std::function func); void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); @@ -909,6 +912,8 @@ public: /** Return true if we should disconnect the peer for failing an inactivity check. */ bool ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const; + bool MultipleManualOrFullOutboundConns(Network net) const EXCLUSIVE_LOCKS_REQUIRED(m_nodes_mutex); + private: struct ListenSocket { public: diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6597019797..624d1b0ccb 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5146,10 +5146,12 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) // Pick the outbound-full-relay peer that least recently announced // us a new block, with ties broken by choosing the more recent // connection (higher node id) + // Protect peers from eviction if we don't have another connection + // to their network, counting both outbound-full-relay and manual peers. NodeId worst_peer = -1; int64_t oldest_block_announcement = std::numeric_limits::max(); - m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_connman.GetNodesMutex()) { AssertLockHeld(::cs_main); // Only consider outbound-full-relay peers that are not already @@ -5159,6 +5161,9 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) if (state == nullptr) return; // shouldn't be possible, but just in case // Don't evict our protected peers if (state->m_chain_sync.m_protect) return; + // If this is the only connection on a particular network that is + // OUTBOUND_FULL_RELAY or MANUAL, protect it. + if (!m_connman.MultipleManualOrFullOutboundConns(pnode->addr.GetNetwork())) return; if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) { worst_peer = pnode->GetId(); oldest_block_announcement = state->m_last_block_announcement;