Skip to content

fix(upnp): add exponential backoff for failed port mappings #6128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 1, 2025
+116 −38
Merged

fix(upnp): add exponential backoff for failed port mappings #6128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions protocols/upnp/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
checks active mappings on the gateway.
See [PR 6127](https://github.com/libp2p/rust-libp2p/pull/6127).

- Fix excessive retry attempts for failed port mappings by implementing exponential backoff.
Failed mappings now retry up to 5 times with increasing delays (30s to 480s) before giving up.
This prevents continuous retry loops.
See [PR 6128](https://github.com/libp2p/rust-libp2p/pull/6128).

## 0.5.0

- update igd-next to 0.16.1
Expand Down
149 changes: 111 additions & 38 deletions protocols/upnp/src/behaviour.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ const MAPPING_DURATION: u32 = 3600;
/// Renew the Mapping every half of `MAPPING_DURATION` to avoid the port being unmapped.
const MAPPING_TIMEOUT: u64 = MAPPING_DURATION as u64 / 2;

/// Maximum number of retry attempts for failed mappings.
const MAX_RETRY_ATTEMPTS: u32 = 5;

/// Base delay in seconds for exponential backoff (will be multiplied by 2^retry_count).
const BASE_RETRY_DELAY_SECS: u64 = 30;

/// Maximum delay in seconds between retry attempts.
const MAX_RETRY_DELAY_SECS: u64 = 1800;

/// A [`Gateway`] Request.
#[derive(Debug)]
pub(crate) enum GatewayRequest {
Expand Down Expand Up @@ -122,11 +131,19 @@ enum MappingState {
/// Port mapping is inactive, will be requested or re-requested on the next iteration.
Inactive,
/// Port mapping/removal has been requested on the gateway.
Pending,
Pending {
/// Number of times we've tried to map this port.
retry_count: u32,
},
/// Port mapping is active with the inner timeout.
Active(Delay),
/// Port mapping failed, we will try again.
Failed,
/// Port mapping failed with retry information.
Failed {
/// Number of times we've tried to map this port.
retry_count: u32,
/// When we should try again (None means no more retries).
next_retry: Option<Delay>,
},
}

/// Current state of the UPnP [`Gateway`].
Expand Down Expand Up @@ -174,7 +191,7 @@ impl MappingList {
fn renew(&mut self, gateway: &mut Gateway, cx: &mut Context<'_>) {
for (mapping, state) in self.iter_mut() {
match state {
MappingState::Inactive | MappingState::Failed => {
MappingState::Inactive => {
let duration = MAPPING_DURATION;
if let Err(err) = gateway.sender.try_send(GatewayRequest::AddMapping {
mapping: mapping.clone(),
Expand All @@ -185,8 +202,34 @@ impl MappingList {
"could not request port mapping for multiaddress on the gateway: {}",
err
);
} else {
*state = MappingState::Pending { retry_count: 0 };
}
}
MappingState::Failed {
retry_count,
next_retry,
} => {
if let Some(delay) = next_retry {
if Pin::new(delay).poll(cx).is_ready() {
let duration = MAPPING_DURATION;
if let Err(err) = gateway.sender.try_send(GatewayRequest::AddMapping {
mapping: mapping.clone(),
duration,
}) {
tracing::debug!(
multiaddress=%mapping.multiaddr,
retry_count=%retry_count,
"could not retry port mapping for multiaddress on the gateway: {}",
err
);
} else {
*state = MappingState::Pending {
retry_count: *retry_count,
};
}
}
}
*state = MappingState::Pending;
}
MappingState::Active(timeout) => {
if Pin::new(timeout).poll(cx).is_ready() {
Expand All @@ -203,7 +246,7 @@ impl MappingList {
}
}
}
MappingState::Pending => {}
MappingState::Pending { .. } => {}
}
}
}
Expand Down Expand Up @@ -317,7 +360,8 @@ impl NetworkBehaviour for Behaviour {
);
}

self.mappings.insert(mapping, MappingState::Pending);
self.mappings
.insert(mapping, MappingState::Pending { retry_count: 0 });
}
GatewayState::GatewayNotFound => {
tracing::debug!(
Expand Down Expand Up @@ -351,7 +395,8 @@ impl NetworkBehaviour for Behaviour {
err
);
}
self.mappings.insert(mapping, MappingState::Pending);
self.mappings
.insert(mapping, MappingState::Pending { retry_count: 0 });
}
}
}
Expand Down Expand Up @@ -427,7 +472,7 @@ impl NetworkBehaviour for Behaviour {
.insert(mapping.clone(), new_state)
.expect("mapping should exist")
{
MappingState::Pending => {
MappingState::Pending { .. } => {
let external_multiaddr =
mapping.external_addr(gateway.external_addr);
self.pending_events.push_back(Event::NewExternalAddr(
Expand All @@ -453,36 +498,64 @@ impl NetworkBehaviour for Behaviour {
}
}
GatewayEvent::MapFailure(mapping, err) => {
match self
.mappings
.insert(mapping.clone(), MappingState::Failed)
.expect("mapping should exist")
{
MappingState::Active(_) => {
tracing::debug!(
address=%mapping.internal_addr,
protocol=%mapping.protocol,
"failed to remap UPnP mapped for protocol: {err}"
);
let external_multiaddr =
mapping.external_addr(gateway.external_addr);
self.pending_events.push_back(Event::ExpiredExternalAddr(
external_multiaddr.clone(),
));
return Poll::Ready(ToSwarm::ExternalAddrExpired(
external_multiaddr,
));
}
MappingState::Pending => {
tracing::debug!(
address=%mapping.internal_addr,
protocol=%mapping.protocol,
"failed to map UPnP mapped for protocol: {err}"
);
}
_ => {
unreachable!()
let prev_state =
self.mappings.get(&mapping).expect("mapping should exist");

let (retry_count, was_active) = match prev_state {
MappingState::Active(_) => (0, true),
MappingState::Pending { retry_count } => (*retry_count, false),
MappingState::Failed { retry_count, .. } => {
(*retry_count, false)
}
_ => unreachable!(),
};

let new_retry_count = retry_count + 1;
let next_retry = if new_retry_count < MAX_RETRY_ATTEMPTS {
let delay_secs = std::cmp::min(
BASE_RETRY_DELAY_SECS
.saturating_mul(2_u64.pow(retry_count)),
MAX_RETRY_DELAY_SECS,
);
Some(Delay::new(Duration::from_secs(delay_secs)))
} else {
tracing::warn!(
address=%mapping.internal_addr,
protocol=%mapping.protocol,
"giving up on UPnP mapping after {new_retry_count} attempts"
);
None
};

self.mappings.insert(
mapping.clone(),
MappingState::Failed {
retry_count: new_retry_count,
next_retry,
},
);

if was_active {
tracing::debug!(
address=%mapping.internal_addr,
protocol=%mapping.protocol,
"failed to remap UPnP mapped for protocol: {err}"
);
let external_multiaddr =
mapping.external_addr(gateway.external_addr);
self.pending_events.push_back(Event::ExpiredExternalAddr(
external_multiaddr.clone(),
));
return Poll::Ready(ToSwarm::ExternalAddrExpired(
external_multiaddr,
));
} else {
tracing::debug!(
address=%mapping.internal_addr,
protocol=%mapping.protocol,
retry_count=%new_retry_count,
"failed to map UPnP mapped for protocol: {err}"
);
}
}
GatewayEvent::Removed(mapping) => {
Expand Down
Loading