From cd32435d859ef47d282fafa43b0e04e0551db839 Mon Sep 17 00:00:00 2001 From: Darius Clark Date: Sat, 9 Sep 2023 05:37:25 -0400 Subject: [PATCH] fix(ping): honor ping interval in case of errors This PR adds a delay to ping connection handler after exceeding the failure rate to prevent opening a outbound stream right after an error. Resolves #4410. Pull-Request: #4423. --- Cargo.lock | 2 +- Cargo.toml | 2 +- protocols/ping/CHANGELOG.md | 8 ++++++++ protocols/ping/Cargo.toml | 2 +- protocols/ping/src/handler.rs | 18 +++++++++++------- protocols/ping/src/protocol.rs | 3 +-- 6 files changed, 23 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9f0fc338c08..7451366120c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2985,7 +2985,7 @@ dependencies = [ [[package]] name = "libp2p-ping" -version = "0.43.0" +version = "0.43.1" dependencies = [ "async-std", "either", diff --git a/Cargo.toml b/Cargo.toml index 734b2d14efb..154bdf38f8e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,7 +84,7 @@ libp2p-mplex = { version = "0.40.0", path = "muxers/mplex" } libp2p-muxer-test-harness = { path = "muxers/test-harness" } libp2p-noise = { version = "0.43.1", path = "transports/noise" } libp2p-perf = { version = "0.2.0", path = "protocols/perf" } -libp2p-ping = { version = "0.43.0", path = "protocols/ping" } +libp2p-ping = { version = "0.43.1", path = "protocols/ping" } libp2p-plaintext = { version = "0.40.0", path = "transports/plaintext" } libp2p-pnet = { version = "0.23.0", path = "transports/pnet" } libp2p-quic = { version = "0.9.2", path = "transports/quic" } diff --git a/protocols/ping/CHANGELOG.md b/protocols/ping/CHANGELOG.md index fd0c0824ef0..7b46cdc3626 100644 --- a/protocols/ping/CHANGELOG.md +++ b/protocols/ping/CHANGELOG.md @@ -1,3 +1,11 @@ +## 0.43.1 - unreleased + +- Honor ping interval in case of errors. + Previously, we would immediately open another ping stream if the current one failed. + See [PR 4423]. + +[PR 4423]: https://github.com/libp2p/rust-libp2p/pull/4423 + ## 0.43.0 - Raise MSRV to 1.65. diff --git a/protocols/ping/Cargo.toml b/protocols/ping/Cargo.toml index f66222efff7..29cf5985b76 100644 --- a/protocols/ping/Cargo.toml +++ b/protocols/ping/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-ping" edition = "2021" rust-version = { workspace = true } description = "Ping protocol for libp2p" -version = "0.43.0" +version = "0.43.1" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/protocols/ping/src/handler.rs b/protocols/ping/src/handler.rs index 0cf5c6e5f07..522663196e6 100644 --- a/protocols/ping/src/handler.rs +++ b/protocols/ping/src/handler.rs @@ -303,6 +303,7 @@ impl ConnectionHandler for Handler { return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(Ok(rtt))); } Poll::Ready(Err(e)) => { + self.interval.reset(self.config.interval); self.pending_errors.push_front(e); } }, @@ -321,13 +322,16 @@ impl ConnectionHandler for Handler { self.outbound = Some(OutboundState::OpenStream); break; } - None => { - self.outbound = Some(OutboundState::OpenStream); - let protocol = SubstreamProtocol::new(ReadyUpgrade::new(PROTOCOL_NAME), ()); - return Poll::Ready(ConnectionHandlerEvent::OutboundSubstreamRequest { - protocol, - }); - } + None => match self.interval.poll_unpin(cx) { + Poll::Pending => break, + Poll::Ready(()) => { + self.outbound = Some(OutboundState::OpenStream); + let protocol = SubstreamProtocol::new(ReadyUpgrade::new(PROTOCOL_NAME), ()); + return Poll::Ready(ConnectionHandlerEvent::OutboundSubstreamRequest { + protocol, + }); + } + }, } } diff --git a/protocols/ping/src/protocol.rs b/protocols/ping/src/protocol.rs index 59e583a8b3b..28549e1c198 100644 --- a/protocols/ping/src/protocol.rs +++ b/protocols/ping/src/protocol.rs @@ -35,8 +35,7 @@ pub const PROTOCOL_NAME: StreamProtocol = StreamProtocol::new("/ipfs/ping/1.0.0" /// /// At most a single inbound and outbound substream is kept open at /// any time. In case of a ping timeout or another error on a substream, the -/// substream is dropped. If a configurable number of consecutive -/// outbound pings fail, the connection is closed. +/// substream is dropped. /// /// Successful pings report the round-trip time. ///