From b906b64eb76643feaede1da5987a0c4d466c581b Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Fri, 6 Jan 2023 11:23:46 +0100 Subject: [PATCH 1/3] i2p: reuse created I2P sessions if not used In the case of `i2pacceptincoming=0` we use transient addresses (destinations) for ourselves for each outbound connection. It may happen that we * create the session (and thus our address/destination too) * fail to connect to the particular peer (e.g. if they are offline) * dispose the unused session. This puts unnecessary load on the I2P network because session creation is not cheap. Is exaggerated if `onlynet=i2p` is used in which case we will be trying to connect to I2P peers more often. To help with this, save the created but unused sessions and pick them later instead of creating new ones. Alleviates: https://github.com/bitcoin/bitcoin/issues/26754 --- src/net.cpp | 23 ++++++++++++++++++++++- src/net.h | 33 +++++++++++++++++++++++++++------ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 960d0ee8412..93b88c7d604 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -436,6 +436,7 @@ static CAddress GetBindAddress(const Sock& sock) CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type) { + AssertLockNotHeld(m_unused_i2p_sessions_mutex); assert(conn_type != ConnectionType::INBOUND); if (pszDest == nullptr) { @@ -496,8 +497,23 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo if (m_i2p_sam_session) { connected = m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed); } else { - i2p_transient_session = std::make_unique(proxy.proxy, &interruptNet); + { + LOCK(m_unused_i2p_sessions_mutex); + if (m_unused_i2p_sessions.empty()) { + i2p_transient_session = + std::make_unique(proxy.proxy, &interruptNet); + } else { + i2p_transient_session.swap(m_unused_i2p_sessions.front()); + m_unused_i2p_sessions.pop(); + } + } connected = i2p_transient_session->Connect(addrConnect, conn, proxyConnectionFailed); + if (!connected) { + LOCK(m_unused_i2p_sessions_mutex); + if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) { + m_unused_i2p_sessions.emplace(i2p_transient_session.release()); + } + } } if (connected) { @@ -1047,6 +1063,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, bool CConnman::AddConnection(const std::string& address, ConnectionType conn_type) { + AssertLockNotHeld(m_unused_i2p_sessions_mutex); std::optional max_connections; switch (conn_type) { case ConnectionType::INBOUND: @@ -1509,6 +1526,7 @@ void CConnman::DumpAddresses() void CConnman::ProcessAddrFetch() { + AssertLockNotHeld(m_unused_i2p_sessions_mutex); std::string strDest; { LOCK(m_addr_fetches_mutex); @@ -1577,6 +1595,7 @@ int CConnman::GetExtraBlockRelayCount() const void CConnman::ThreadOpenConnections(const std::vector connect) { + AssertLockNotHeld(m_unused_i2p_sessions_mutex); SetSyscallSandboxPolicy(SyscallSandboxPolicy::NET_OPEN_CONNECTION); FastRandomContext rng; // Connect to specific addresses @@ -1928,6 +1947,7 @@ std::vector CConnman::GetAddedNodeInfo() const void CConnman::ThreadOpenAddedConnections() { + AssertLockNotHeld(m_unused_i2p_sessions_mutex); SetSyscallSandboxPolicy(SyscallSandboxPolicy::NET_ADD_CONNECTION); while (true) { @@ -1957,6 +1977,7 @@ void CConnman::ThreadOpenAddedConnections() // if successful, this moves the passed grant to the constructed node void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, ConnectionType conn_type) { + AssertLockNotHeld(m_unused_i2p_sessions_mutex); assert(conn_type != ConnectionType::INBOUND); // diff --git a/src/net.h b/src/net.h index 31d17ea76cf..caab411e5bf 100644 --- a/src/net.h +++ b/src/net.h @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -743,7 +744,7 @@ public: bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type); + 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); bool ForNode(NodeId id, std::function func); @@ -819,7 +820,7 @@ public: * - Max total outbound connection capacity filled * - Max connection capacity for type is filled */ - bool AddConnection(const std::string& address, ConnectionType conn_type); + bool AddConnection(const std::string& address, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); size_t GetNodeCount(ConnectionDirection) const; void GetNodeStats(std::vector& vstats) const; @@ -885,10 +886,10 @@ private: bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions); bool InitBinds(const Options& options); - void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex); void AddAddrFetch(const std::string& strDest) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex); - void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex); - void ThreadOpenConnections(std::vector connect) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex); + void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_unused_i2p_sessions_mutex); + void ThreadOpenConnections(std::vector connect) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex); void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); void ThreadI2PAcceptIncoming(); void AcceptConnection(const ListenSocket& hListenSocket); @@ -955,7 +956,7 @@ private: bool AlreadyConnectedToAddress(const CAddress& addr); bool AttemptToEvictConnection(); - CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type); + CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const; void DeleteNode(CNode* pnode); @@ -1126,6 +1127,26 @@ private: */ std::vector m_onion_binds; + /** + * Mutex protecting m_i2p_sam_sessions. + */ + Mutex m_unused_i2p_sessions_mutex; + + /** + * A pool of created I2P SAM transient sessions that should be used instead + * of creating new ones in order to reduce the load on the I2P network. + * Creating a session in I2P is not cheap, thus if this is not empty, then + * pick an entry from it instead of creating a new session. If connecting to + * a host fails, then the created session is put to this pool for reuse. + */ + std::queue> m_unused_i2p_sessions GUARDED_BY(m_unused_i2p_sessions_mutex); + + /** + * Cap on the size of `m_unused_i2p_sessions`, to ensure it does not + * unexpectedly use too much memory. + */ + static constexpr size_t MAX_UNUSED_I2P_SESSIONS_SIZE{10}; + /** * RAII helper to atomically create a copy of `m_nodes` and add a reference * to each of the nodes. The nodes are released when this object is destroyed. From 801b405f85b413631427c2d8cc1f8447309ea5d8 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Fri, 6 Jan 2023 17:11:26 +0100 Subject: [PATCH 2/3] i2p: lower the number of tunnels for transient sessions This will lower the load on the I2P network. Since we use one transient session for connecting to just one peer, a higher number of tunnels is unnecessary. This was suggested in: https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1365449401 https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1367356129 The options are documented in: https://geti2p.net/en/docs/protocol/i2cp#options A tunnel is unidirectional, so even if we make a single outbound connection we still need an inbound tunnel to receive the messages sent to us over that connection. Alleviates: https://github.com/bitcoin/bitcoin/issues/26754 --- src/i2p.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/i2p.cpp b/src/i2p.cpp index 586ee649a75..e2df6f7e69c 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -380,7 +380,9 @@ void Session::CreateIfNotCreatedAlready() // in the reply in DESTINATION=. const Reply& reply = SendRequestAndGetReply( *sock, - strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=TRANSIENT SIGNATURE_TYPE=7", session_id)); + strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=TRANSIENT SIGNATURE_TYPE=7 " + "inbound.quantity=1 outbound.quantity=1", + session_id)); m_private_key = DecodeI2PBase64(reply.Get("DESTINATION")); } else { From 3c1de032de01e551992975eb374465300a655f44 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Fri, 6 Jan 2023 17:41:31 +0100 Subject: [PATCH 3/3] i2p: use consistent number of tunnels with i2pd and Java I2P The default number of tunnels in the Java implementation is 2 and in the C++ i2pd it is 5. Pick a mid-number (3) and explicitly set it in order to get a consistent behavior with both routers. Do this for persistent sessions which are created once at startup and can be used to open up to ~10 outbound connections and can accept up to ~125 incoming connections. Transient sessions already set number of tunnels to 1. Suggested in: https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1367356129 https://geti2p.net/en/docs/api/samv3 Alleviates: https://github.com/bitcoin/bitcoin/issues/26754 --- src/i2p.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/i2p.cpp b/src/i2p.cpp index e2df6f7e69c..802d39d67af 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -398,7 +398,8 @@ void Session::CreateIfNotCreatedAlready() const std::string& private_key_b64 = SwapBase64(EncodeBase64(m_private_key)); SendRequestAndGetReply(*sock, - strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s", + strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s " + "inbound.quantity=3 outbound.quantity=3", session_id, private_key_b64)); }