From 6a8dae0c31fd17b7d24ce67d23d6f1c7f679d892 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 20 Jul 2018 11:12:45 +0800 Subject: [PATCH 1/8] Limit number of incoming connections --- substrate/network-libp2p/src/network_state.rs | 37 +++++++++++++++---- substrate/network-libp2p/src/traits.rs | 3 ++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/substrate/network-libp2p/src/network_state.rs b/substrate/network-libp2p/src/network_state.rs index 00b97eecf9a82..17908d5e87fb3 100644 --- a/substrate/network-libp2p/src/network_state.rs +++ b/substrate/network-libp2p/src/network_state.rs @@ -55,6 +55,8 @@ pub struct NetworkState { min_peers: u32, /// `max_peers` taken from the configuration. max_peers: u32, + /// `max_incoming_peers` calculated as `max_peers / max_incoming_peers_factor` from the configuration. + max_incoming_peers: u32, /// If true, only reserved peers can connect. reserved_only: atomic::AtomicBool, @@ -169,6 +171,7 @@ impl NetworkState { peerstore, min_peers: config.min_peers, max_peers: config.max_peers, + max_incoming_peers: config.max_peers / config.max_incoming_peers_factor, connections: RwLock::new(Connections { peer_by_nodeid: FnvHashMap::with_capacity_and_hasher(expected_max_peers, Default::default()), info_by_peer: FnvHashMap::with_capacity_and_hasher(expected_max_peers, Default::default()), @@ -433,7 +436,7 @@ impl NetworkState { /// Returns the number of open and pending connections with /// custom protocols. pub fn num_open_custom_connections(&self) -> u32 { - num_open_custom_connections(&self.connections.read()) + num_open_custom_connections(&self.connections.read()).total } /// Returns the number of new outgoing custom connections to peers to @@ -520,7 +523,7 @@ impl NetworkState { /// You must pass an `UnboundedSender` which will be used by the `send` /// method. Actually sending the data is not covered by this code. /// - /// The various methods of the `NetworkState` that close a connection do + /// The various methods of the `NetworkState` that close a connection do /// so by dropping this sender. pub fn custom_proto( &self, @@ -548,7 +551,8 @@ impl NetworkState { let node_is_reserved = self.reserved_peers.read().contains(&infos.id); if !node_is_reserved { if self.reserved_only.load(atomic::Ordering::Relaxed) || - num_open_connections >= self.max_peers + num_open_connections.total >= self.max_peers || + num_open_connections.incoming >= self.max_incoming_peers { debug!(target: "sub-libp2p", "Refusing node {:?} because we \ reached the max number of peers", node_id); @@ -734,10 +738,17 @@ fn is_peer_disabled( } } +struct OpenCustomConnectionsNumbers { + /// Total number of open and pending connections. + pub total: u32, + /// Incoming number of open and pending connections. + pub incoming: u32, +} + /// Returns the number of open and pending connections with /// custom protocols. -fn num_open_custom_connections(connections: &Connections) -> u32 { - connections +fn num_open_custom_connections(connections: &Connections) -> OpenCustomConnectionsNumbers { + let filtered = connections .info_by_peer .values() .filter(|info| @@ -747,8 +758,18 @@ fn num_open_custom_connections(connections: &Connections) -> u32 { _ => false } ) - ) - .count() as u32 + ); + + let mut total: u32 = 0; + let mut incoming: u32 = 0; + for info in filtered { + total += 1; + if !info.originated { + incoming += 1; + } + } + + OpenCustomConnectionsNumbers { total, incoming } } /// Parses an address of the form `/ip4/x.x.x.x/tcp/x/p2p/xxxxxx`, and adds it @@ -774,7 +795,7 @@ fn parse_and_add_to_peerstore(addr_str: &str, peerstore: &PeersStorage) .peer_or_create(&peer_id) .add_addr(addr, Duration::from_secs(100000 * 365 * 24 * 3600)), } - + Ok(peer_id) } diff --git a/substrate/network-libp2p/src/traits.rs b/substrate/network-libp2p/src/traits.rs index a20065328d97c..8d0dc2cd65e69 100644 --- a/substrate/network-libp2p/src/traits.rs +++ b/substrate/network-libp2p/src/traits.rs @@ -153,6 +153,8 @@ pub struct NetworkConfiguration { pub min_peers: u32, /// Maximum allowed number of peers pub max_peers: u32, + /// Maximum incoming peers factor. The maximum allowed number of incoming peers is calculated as `max_peers / max_incoming_peers_factor`. + pub max_incoming_peers_factor: u32, /// Maximum handshakes pub max_handshakes: u32, /// Reserved protocols. Peers with protocol get additional connection slots. @@ -188,6 +190,7 @@ impl NetworkConfiguration { use_secret: None, min_peers: 25, max_peers: 50, + max_incoming_peers_factor: 3, max_handshakes: 64, reserved_protocols: HashMap::new(), ip_filter: IpFilter::default(), From 02532cd3d23be2fe5d0ec865aca1447cfa2a4fd0 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 20 Jul 2018 13:39:58 +0800 Subject: [PATCH 2/8] Check Endpoint::Listener before checking num_open_connections.incoming --- substrate/network-libp2p/src/network_state.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/substrate/network-libp2p/src/network_state.rs b/substrate/network-libp2p/src/network_state.rs index 17908d5e87fb3..59965315535c7 100644 --- a/substrate/network-libp2p/src/network_state.rs +++ b/substrate/network-libp2p/src/network_state.rs @@ -552,7 +552,8 @@ impl NetworkState { if !node_is_reserved { if self.reserved_only.load(atomic::Ordering::Relaxed) || num_open_connections.total >= self.max_peers || - num_open_connections.incoming >= self.max_incoming_peers + (endpoint == Endpoint::Listener && + num_open_connections.incoming >= self.max_incoming_peers) { debug!(target: "sub-libp2p", "Refusing node {:?} because we \ reached the max number of peers", node_id); From 374e367a015d2710151e733ce5f57d8eadc3e8e5 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 20 Jul 2018 15:12:20 +0800 Subject: [PATCH 3/8] Maintain at least 1-1/n portion of outgoing connections --- substrate/network-libp2p/src/network_state.rs | 26 ++++++++++++------- substrate/network-libp2p/src/traits.rs | 6 ++--- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/substrate/network-libp2p/src/network_state.rs b/substrate/network-libp2p/src/network_state.rs index 59965315535c7..e80318e12e1d0 100644 --- a/substrate/network-libp2p/src/network_state.rs +++ b/substrate/network-libp2p/src/network_state.rs @@ -55,6 +55,8 @@ pub struct NetworkState { min_peers: u32, /// `max_peers` taken from the configuration. max_peers: u32, + /// `incoming_peers_factor` taken from the configuration. + incoming_peers_factor: u32, /// `max_incoming_peers` calculated as `max_peers / max_incoming_peers_factor` from the configuration. max_incoming_peers: u32, @@ -171,7 +173,8 @@ impl NetworkState { peerstore, min_peers: config.min_peers, max_peers: config.max_peers, - max_incoming_peers: config.max_peers / config.max_incoming_peers_factor, + incoming_peers_factor: config.incoming_peers_factor, + max_incoming_peers: config.max_peers / config.incoming_peers_factor, connections: RwLock::new(Connections { peer_by_nodeid: FnvHashMap::with_capacity_and_hasher(expected_max_peers, Default::default()), info_by_peer: FnvHashMap::with_capacity_and_hasher(expected_max_peers, Default::default()), @@ -433,19 +436,18 @@ impl NetworkState { } } - /// Returns the number of open and pending connections with - /// custom protocols. - pub fn num_open_custom_connections(&self) -> u32 { - num_open_custom_connections(&self.connections.read()).total - } - /// Returns the number of new outgoing custom connections to peers to /// open. This takes into account the number of active peers. pub fn should_open_outgoing_custom_connections(&self) -> u32 { + use std::cmp::max; + if self.reserved_only.load(atomic::Ordering::Relaxed) { 0 } else { - self.min_peers.saturating_sub(self.num_open_custom_connections()) + let num_open_custom_connections = num_open_custom_connections(&self.connections.read()); + let min_outgoing_peers = num_open_custom_connections.incoming * self.incoming_peers_factor.saturating_sub(1); + max(min_outgoing_peers.saturating_sub(num_open_custom_connections.outgoing), + self.min_peers.saturating_sub(num_open_custom_connections.total)) } } @@ -744,6 +746,8 @@ struct OpenCustomConnectionsNumbers { pub total: u32, /// Incoming number of open and pending connections. pub incoming: u32, + /// Outgoing number of open and pending connections. + pub outgoing: u32, } /// Returns the number of open and pending connections with @@ -770,7 +774,11 @@ fn num_open_custom_connections(connections: &Connections) -> OpenCustomConnectio } } - OpenCustomConnectionsNumbers { total, incoming } + OpenCustomConnectionsNumbers { + total, + incoming, + outgoing: total - incoming, + } } /// Parses an address of the form `/ip4/x.x.x.x/tcp/x/p2p/xxxxxx`, and adds it diff --git a/substrate/network-libp2p/src/traits.rs b/substrate/network-libp2p/src/traits.rs index 8d0dc2cd65e69..69641d7a57d34 100644 --- a/substrate/network-libp2p/src/traits.rs +++ b/substrate/network-libp2p/src/traits.rs @@ -153,8 +153,8 @@ pub struct NetworkConfiguration { pub min_peers: u32, /// Maximum allowed number of peers pub max_peers: u32, - /// Maximum incoming peers factor. The maximum allowed number of incoming peers is calculated as `max_peers / max_incoming_peers_factor`. - pub max_incoming_peers_factor: u32, + /// At most `1 / incoming_peers_factor` of incoming connections are allowed. + pub incoming_peers_factor: u32, /// Maximum handshakes pub max_handshakes: u32, /// Reserved protocols. Peers with protocol get additional connection slots. @@ -190,7 +190,7 @@ impl NetworkConfiguration { use_secret: None, min_peers: 25, max_peers: 50, - max_incoming_peers_factor: 3, + incoming_peers_factor: 3, max_handshakes: 64, reserved_protocols: HashMap::new(), ip_filter: IpFilter::default(), From f7edca6c024a11f9cec65dd1de9ae47e40de910a Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 20 Jul 2018 16:42:23 +0800 Subject: [PATCH 4/8] Remove use --- substrate/network-libp2p/src/network_state.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/network-libp2p/src/network_state.rs b/substrate/network-libp2p/src/network_state.rs index e80318e12e1d0..6283ee6810287 100644 --- a/substrate/network-libp2p/src/network_state.rs +++ b/substrate/network-libp2p/src/network_state.rs @@ -439,14 +439,12 @@ impl NetworkState { /// Returns the number of new outgoing custom connections to peers to /// open. This takes into account the number of active peers. pub fn should_open_outgoing_custom_connections(&self) -> u32 { - use std::cmp::max; - if self.reserved_only.load(atomic::Ordering::Relaxed) { 0 } else { let num_open_custom_connections = num_open_custom_connections(&self.connections.read()); let min_outgoing_peers = num_open_custom_connections.incoming * self.incoming_peers_factor.saturating_sub(1); - max(min_outgoing_peers.saturating_sub(num_open_custom_connections.outgoing), + cmp::max(min_outgoing_peers.saturating_sub(num_open_custom_connections.outgoing), self.min_peers.saturating_sub(num_open_custom_connections.total)) } } From ba11932a5c58cf6b458b236f2c71c6ad58a5403f Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 20 Jul 2018 16:42:45 +0800 Subject: [PATCH 5/8] Default incoming_peers_factor to 2 --- substrate/network-libp2p/src/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/network-libp2p/src/traits.rs b/substrate/network-libp2p/src/traits.rs index 69641d7a57d34..7ecc3d1d7fe92 100644 --- a/substrate/network-libp2p/src/traits.rs +++ b/substrate/network-libp2p/src/traits.rs @@ -190,7 +190,7 @@ impl NetworkConfiguration { use_secret: None, min_peers: 25, max_peers: 50, - incoming_peers_factor: 3, + incoming_peers_factor: 2, max_handshakes: 64, reserved_protocols: HashMap::new(), ip_filter: IpFilter::default(), From 25f33a79bd751e9638db65674420b3b3f09665a4 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Wed, 25 Jul 2018 11:56:41 +0800 Subject: [PATCH 6/8] Use max_incoming_peers and max_outgoing peers to check whether connections should be dropped --- substrate/network-libp2p/src/network_state.rs | 56 +++++++++---------- substrate/network-libp2p/src/traits.rs | 5 +- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/substrate/network-libp2p/src/network_state.rs b/substrate/network-libp2p/src/network_state.rs index a201b36645f36..919601ce8a149 100644 --- a/substrate/network-libp2p/src/network_state.rs +++ b/substrate/network-libp2p/src/network_state.rs @@ -51,14 +51,10 @@ pub struct NetworkState { /// Active connections. connections: RwLock, - /// `min_peers` taken from the configuration. - min_peers: u32, - /// `max_peers` taken from the configuration. - max_peers: u32, - /// `incoming_peers_factor` taken from the configuration. - incoming_peers_factor: u32, - /// `max_incoming_peers` calculated as `max_peers / max_incoming_peers_factor` from the configuration. + /// Maximum incoming peers. max_incoming_peers: u32, + /// Maximum outgoing peers. + max_outgoing_peers: u32, /// If true, only reserved peers can connect. reserved_only: atomic::AtomicBool, @@ -210,10 +206,8 @@ impl NetworkState { Ok(NetworkState { node_store, - min_peers: config.min_peers, - max_peers: config.max_peers, - incoming_peers_factor: config.incoming_peers_factor, - max_incoming_peers: config.max_peers / config.incoming_peers_factor, + max_outgoing_peers: config.min_peers, + max_incoming_peers: config.max_peers.saturating_sub(config.min_peers), connections: RwLock::new(Connections { peer_by_nodeid: FnvHashMap::with_capacity_and_hasher(expected_max_peers, Default::default()), info_by_peer: FnvHashMap::with_capacity_and_hasher(expected_max_peers, Default::default()), @@ -476,10 +470,8 @@ impl NetworkState { if self.reserved_only.load(atomic::Ordering::Relaxed) { 0 } else { - let num_open_custom_connections = num_open_custom_connections(&self.connections.read()); - let min_outgoing_peers = num_open_custom_connections.incoming * self.incoming_peers_factor.saturating_sub(1); - cmp::max(min_outgoing_peers.saturating_sub(num_open_custom_connections.outgoing), - self.min_peers.saturating_sub(num_open_custom_connections.total)) + let num_open_custom_connections = num_open_custom_connections(&self.connections.read(), &self.reserved_peers.read()); + self.max_outgoing_peers.saturating_sub(num_open_custom_connections.unreserved_outgoing) } } @@ -575,7 +567,7 @@ impl NetworkState { let who = accept_connection(&mut connections, &self.next_node_index, node_id.clone(), endpoint)?; - let num_open_connections = num_open_custom_connections(&connections); + let num_open_connections = num_open_custom_connections(&connections, &self.reserved_peers.read()); let infos = connections.info_by_peer.get_mut(&who) .expect("Newly-created peer id is always valid"); @@ -583,9 +575,10 @@ impl NetworkState { let node_is_reserved = self.reserved_peers.read().contains(&infos.id); if !node_is_reserved { if self.reserved_only.load(atomic::Ordering::Relaxed) || - num_open_connections.total >= self.max_peers || (endpoint == Endpoint::Listener && - num_open_connections.incoming >= self.max_incoming_peers) + num_open_connections.unreserved_incoming >= self.max_incoming_peers) || + (endpoint == Endpoint::Dialer && + num_open_connections.unreserved_outgoing >= self.max_outgoing_peers) { debug!(target: "sub-libp2p", "Refusing node {:?} because we reached the max number of peers", node_id); return Err(IoError::new(IoErrorKind::PermissionDenied, "maximum number of peers reached")) @@ -779,15 +772,15 @@ fn is_peer_disabled( struct OpenCustomConnectionsNumbers { /// Total number of open and pending connections. pub total: u32, - /// Incoming number of open and pending connections. - pub incoming: u32, - /// Outgoing number of open and pending connections. - pub outgoing: u32, + /// Unreserved incoming number of open and pending connections. + pub unreserved_incoming: u32, + /// Unreserved outgoing number of open and pending connections. + pub unreserved_outgoing: u32, } /// Returns the number of open and pending connections with /// custom protocols. -fn num_open_custom_connections(connections: &Connections) -> OpenCustomConnectionsNumbers { +fn num_open_custom_connections(connections: &Connections, reserved_peers: &FnvHashSet) -> OpenCustomConnectionsNumbers { let filtered = connections .info_by_peer .values() @@ -801,18 +794,25 @@ fn num_open_custom_connections(connections: &Connections) -> OpenCustomConnectio ); let mut total: u32 = 0; - let mut incoming: u32 = 0; + let mut unreserved_incoming: u32 = 0; + let mut unreserved_outgoing: u32 = 0; + for info in filtered { total += 1; - if !info.originated { - incoming += 1; + let node_is_reserved = reserved_peers.contains(&info.id); + if !node_is_reserved { + if !info.originated { + unreserved_incoming += 1; + } else { + unreserved_outgoing += 1; + } } } OpenCustomConnectionsNumbers { total, - incoming, - outgoing: total - incoming, + unreserved_incoming, + unreserved_outgoing, } } diff --git a/substrate/network-libp2p/src/traits.rs b/substrate/network-libp2p/src/traits.rs index 262486f0c3a62..15c9af78c50fa 100644 --- a/substrate/network-libp2p/src/traits.rs +++ b/substrate/network-libp2p/src/traits.rs @@ -155,8 +155,6 @@ pub struct NetworkConfiguration { pub min_peers: u32, /// Maximum allowed number of peers pub max_peers: u32, - /// At most `1 / incoming_peers_factor` of incoming connections are allowed. - pub incoming_peers_factor: u32, /// Maximum handshakes pub max_handshakes: u32, /// Reserved protocols. Peers with protocol get additional connection slots. @@ -192,7 +190,6 @@ impl NetworkConfiguration { use_secret: None, min_peers: 25, max_peers: 50, - incoming_peers_factor: 2, max_handshakes: 64, reserved_protocols: HashMap::new(), ip_filter: IpFilter::default(), @@ -227,7 +224,7 @@ pub enum Severity<'a> { /// it could answer. Useless(&'a str), /// Peer has behaved in an invalid manner. This doesn't necessarily need to be Byzantine, but peer - /// must have taken concrete action in order to behave in such a way which is wantanly invalid. + /// must have taken concrete action in order to behave in such a way which is wantanly invalid. Bad(&'a str), } From ef765f7f7ede651032222786ce0baa1adb9150b7 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Wed, 25 Jul 2018 11:58:01 +0800 Subject: [PATCH 7/8] Fix expected_max_peers: reserved peers are not counted in config.max_peers --- substrate/network-libp2p/src/network_state.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/substrate/network-libp2p/src/network_state.rs b/substrate/network-libp2p/src/network_state.rs index 919601ce8a149..510699650f41c 100644 --- a/substrate/network-libp2p/src/network_state.rs +++ b/substrate/network-libp2p/src/network_state.rs @@ -201,8 +201,7 @@ impl NetworkState { RwLock::new(reserved_peers) }; - let expected_max_peers = cmp::max(config.max_peers as usize, - config.reserved_nodes.len()); + let expected_max_peers = config.max_peers + config.reserved_nodes.len(); Ok(NetworkState { node_store, From 15654bfeca006bd6686a0dcad046bc6a34df94f1 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Wed, 25 Jul 2018 16:39:19 +0800 Subject: [PATCH 8/8] typo: fix test --- substrate/network-libp2p/src/network_state.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/substrate/network-libp2p/src/network_state.rs b/substrate/network-libp2p/src/network_state.rs index 510699650f41c..38e26601bfe92 100644 --- a/substrate/network-libp2p/src/network_state.rs +++ b/substrate/network-libp2p/src/network_state.rs @@ -29,7 +29,6 @@ use {Error, ErrorKind, NetworkConfiguration, NonReservedPeerMode}; use {NodeIndex, ProtocolId, SessionInfo}; use parking_lot::{Mutex, RwLock}; use rand::{self, Rng}; -use std::cmp; use std::fs; use std::io::{Error as IoError, ErrorKind as IoErrorKind, Read, Write}; use std::path::Path; @@ -201,7 +200,7 @@ impl NetworkState { RwLock::new(reserved_peers) }; - let expected_max_peers = config.max_peers + config.reserved_nodes.len(); + let expected_max_peers = config.max_peers as usize + config.reserved_nodes.len(); Ok(NetworkState { node_store,