Skip to content

Commit fc36065

Browse files
RolandSherwinRolandSherwin
committed
fix(upnp): add exponential backoff for failed port mappings
1 parent 6dcfe73 commit fc36065

File tree

5 files changed

+109
-36
lines changed

5 files changed

+109
-36
lines changed

‎Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ libp2p-swarm-test = { version = "0.6.0", path = "swarm-test" }
108108
libp2p-tcp = { version = "0.44.0", path = "transports/tcp" }
109109
libp2p-tls = { version = "0.6.2", path = "transports/tls" }
110110
libp2p-uds = { version = "0.43.0", path = "transports/uds" }
111-
libp2p-upnp = { version = "0.5.0", path = "protocols/upnp" }
111+
libp2p-upnp = { version = "0.5.1", path = "protocols/upnp" }
112112
libp2p-webrtc = { version = "0.9.0-alpha.1", path = "transports/webrtc" }
113113
libp2p-webrtc-utils = { version = "0.4.0", path = "misc/webrtc-utils" }
114114
libp2p-webrtc-websys = { version = "0.4.0", path = "transports/webrtc-websys" }

‎protocols/upnp/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
## 0.5.1
2+
3+
- Fix excessive retry attempts for failed port mappings by implementing exponential backoff.
4+
Failed mappings now retry up to 5 times with increasing delays (30s to 480s) before giving up.
5+
This prevents continuous retry loops.
6+
See [PR 6128](https://github.com/libp2p/rust-libp2p/pull/6128).
7+
18
## 0.5.0
29

310
- update igd-next to 0.16.1

‎protocols/upnp/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ name = "libp2p-upnp"
33
edition.workspace = true
44
rust-version.workspace = true
55
description = "UPnP support for libp2p transports"
6-
version = "0.5.0"
6+
version = "0.5.1"
77
license = "MIT"
88
repository = "https://github.com/libp2p/rust-libp2p"
99
keywords = ["peer-to-peer", "libp2p", "networking"]

‎protocols/upnp/src/behaviour.rs

Lines changed: 99 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ const MAPPING_DURATION: u32 = 3600;
5353
/// Renew the Mapping every half of `MAPPING_DURATION` to avoid the port being unmapped.
5454
const MAPPING_TIMEOUT: u64 = MAPPING_DURATION as u64 / 2;
5555

56+
/// Maximum number of retry attempts for failed mappings.
57+
const MAX_RETRY_ATTEMPTS: u32 = 5;
58+
59+
/// Base delay in seconds for exponential backoff (will be multiplied by 2^retry_count).
60+
const BASE_RETRY_DELAY_SECS: u64 = 30;
61+
62+
/// Maximum delay in seconds between retry attempts.
63+
const MAX_RETRY_DELAY_SECS: u64 = 1800;
64+
5665
/// A [`Gateway`] Request.
5766
#[derive(Debug)]
5867
pub(crate) enum GatewayRequest {
@@ -125,8 +134,13 @@ enum MappingState {
125134
Pending,
126135
/// Port mapping is active with the inner timeout.
127136
Active(Delay),
128-
/// Port mapping failed, we will try again.
129-
Failed,
137+
/// Port mapping failed with retry information.
138+
Failed {
139+
/// Number of times we've tried to map this port.
140+
retry_count: u32,
141+
/// When we should try again (None means no more retries).
142+
next_retry: Option<Delay>,
143+
},
130144
}
131145

132146
/// Current state of the UPnP [`Gateway`].
@@ -174,7 +188,7 @@ impl MappingList {
174188
fn renew(&mut self, gateway: &mut Gateway, cx: &mut Context<'_>) {
175189
for (mapping, state) in self.iter_mut() {
176190
match state {
177-
MappingState::Inactive | MappingState::Failed => {
191+
MappingState::Inactive => {
178192
let duration = MAPPING_DURATION;
179193
if let Err(err) = gateway.sender.try_send(GatewayRequest::AddMapping {
180194
mapping: mapping.clone(),
@@ -185,8 +199,32 @@ impl MappingList {
185199
"could not request port mapping for multiaddress on the gateway: {}",
186200
err
187201
);
202+
} else {
203+
*state = MappingState::Pending;
204+
}
205+
}
206+
MappingState::Failed {
207+
retry_count,
208+
next_retry,
209+
} => {
210+
if let Some(delay) = next_retry {
211+
if Pin::new(delay).poll(cx).is_ready() {
212+
let duration = MAPPING_DURATION;
213+
if let Err(err) = gateway.sender.try_send(GatewayRequest::AddMapping {
214+
mapping: mapping.clone(),
215+
duration,
216+
}) {
217+
tracing::debug!(
218+
multiaddress=%mapping.multiaddr,
219+
retry_count=%retry_count,
220+
"could not retry port mapping for multiaddress on the gateway: {}",
221+
err
222+
);
223+
} else {
224+
*state = MappingState::Pending;
225+
}
226+
}
188227
}
189-
*state = MappingState::Pending;
190228
}
191229
MappingState::Active(timeout) => {
192230
if Pin::new(timeout).poll(cx).is_ready() {
@@ -454,36 +492,64 @@ impl NetworkBehaviour for Behaviour {
454492
}
455493
}
456494
GatewayEvent::MapFailure(mapping, err) => {
457-
match self
458-
.mappings
459-
.insert(mapping.clone(), MappingState::Failed)
460-
.expect("mapping should exist")
461-
{
462-
MappingState::Active(_) => {
463-
tracing::debug!(
464-
address=%mapping.internal_addr,
465-
protocol=%mapping.protocol,
466-
"failed to remap UPnP mapped for protocol: {err}"
467-
);
468-
let external_multiaddr =
469-
mapping.external_addr(gateway.external_addr);
470-
self.pending_events.push_back(Event::ExpiredExternalAddr(
471-
external_multiaddr.clone(),
472-
));
473-
return Poll::Ready(ToSwarm::ExternalAddrExpired(
474-
external_multiaddr,
475-
));
476-
}
477-
MappingState::Pending => {
478-
tracing::debug!(
479-
address=%mapping.internal_addr,
480-
protocol=%mapping.protocol,
481-
"failed to map UPnP mapped for protocol: {err}"
482-
);
483-
}
484-
_ => {
485-
unreachable!()
495+
let prev_state =
496+
self.mappings.get(&mapping).expect("mapping should exist");
497+
498+
let (retry_count, was_active) = match prev_state {
499+
MappingState::Active(_) => (0, true),
500+
MappingState::Pending => (0, false),
501+
MappingState::Failed { retry_count, .. } => {
502+
(*retry_count, false)
486503
}
504+
_ => unreachable!(),
505+
};
506+
507+
let new_retry_count = retry_count + 1;
508+
let next_retry = if new_retry_count < MAX_RETRY_ATTEMPTS {
509+
let delay_secs = std::cmp::min(
510+
BASE_RETRY_DELAY_SECS
511+
.saturating_mul(2_u64.pow(retry_count)),
512+
MAX_RETRY_DELAY_SECS,
513+
);
514+
Some(Delay::new(Duration::from_secs(delay_secs)))
515+
} else {
516+
tracing::warn!(
517+
address=%mapping.internal_addr,
518+
protocol=%mapping.protocol,
519+
"giving up on UPnP mapping after {new_retry_count} attempts"
520+
);
521+
None
522+
};
523+
524+
self.mappings.insert(
525+
mapping.clone(),
526+
MappingState::Failed {
527+
retry_count: new_retry_count,
528+
next_retry,
529+
},
530+
);
531+
532+
if was_active {
533+
tracing::debug!(
534+
address=%mapping.internal_addr,
535+
protocol=%mapping.protocol,
536+
"failed to remap UPnP mapped for protocol: {err}"
537+
);
538+
let external_multiaddr =
539+
mapping.external_addr(gateway.external_addr);
540+
self.pending_events.push_back(Event::ExpiredExternalAddr(
541+
external_multiaddr.clone(),
542+
));
543+
return Poll::Ready(ToSwarm::ExternalAddrExpired(
544+
external_multiaddr,
545+
));
546+
} else {
547+
tracing::debug!(
548+
address=%mapping.internal_addr,
549+
protocol=%mapping.protocol,
550+
retry_count=%new_retry_count,
551+
"failed to map UPnP mapped for protocol: {err}"
552+
);
487553
}
488554
}
489555
GatewayEvent::Removed(mapping) => {

0 commit comments

Comments
 (0)