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

Author: RolandSherwin

protocols/upnp/CHANGELOG.md

@@ -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

protocols/upnp/src/behaviour.rs

@@ -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 {
@@ -125,8 +134,13 @@ enum MappingState {
     Pending,
     /// 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`].
@@ -174,7 +188,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(),
@@ -185,8 +199,32 @@ impl MappingList {
                             "could not request port mapping for multiaddress on the gateway: {}",
                             err
                         );
+                    } else {
+                        *state = MappingState::Pending;
+                    }
+                }
+                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;
+                            }
+                        }
                     }
-                    *state = MappingState::Pending;
                 }
                 MappingState::Active(timeout) => {
                     if Pin::new(timeout).poll(cx).is_ready() {
@@ -453,36 +491,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 => (0, 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) => {