Skip to content

Conversation

@cskiraly
Copy link
Contributor

@cskiraly cskiraly commented Apr 4, 2025

Make UPnP more robust

  • Once a random port was mapped, we try to stick to it even if a UPnP refresh fails. Previously we were immediately moving back to try the default port, leading to frequent ENR changes.

  • We were deleting port mappings before refresh as a possible workaround. This created issues in some UPnP servers. The UPnP (and PMP) specification is explicit about the refresh requirements, and delete is clearly not needed (see p2p/nat: remove forceful port mapping in upnp #30265 (comment)). From now on we only delete when closing.

  • We were trying to add port mappings only once, and then moved on to random ports. Now we insist a bit more, so that a simple failed request won't lead to ENR changes.

Fixes #31418

@cskiraly cskiraly requested review from fjl and zsfelfoldi as code owners April 4, 2025 13:00
@cskiraly cskiraly marked this pull request as draft April 4, 2025 13:03
@cskiraly cskiraly marked this pull request as ready for review April 7, 2025 11:22
Signed-off-by: Csaba Kiraly <[email protected]>
@fjl fjl changed the title p2p/nat: Fix UPnP port reset p2p/nat: fix UPnP port reset Apr 8, 2025
Signed-off-by: Csaba Kiraly <[email protected]>
@fjl fjl merged commit a7f24c2 into ethereum:master Apr 9, 2025
3 of 4 checks passed
@fjl fjl added this to the 1.15.8 milestone Apr 9, 2025
sivaratrisrinivas pushed a commit to sivaratrisrinivas/go-ethereum that referenced this pull request Apr 21, 2025
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
jakub-freebit pushed a commit to fblch/go-ethereum that referenced this pull request Jul 3, 2025
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
jakub-freebit pushed a commit to fblch/go-ethereum that referenced this pull request Jul 23, 2025
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
howjmay pushed a commit to iotaledger/go-ethereum that referenced this pull request Aug 27, 2025
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
jakub-freebit pushed a commit to fblch/go-ethereum that referenced this pull request Sep 3, 2025
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
jakub-freebit pushed a commit to fblch/go-ethereum that referenced this pull request Sep 4, 2025
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
gballet pushed a commit to gballet/go-ethereum that referenced this pull request Sep 11, 2025
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
jakub-freebit pushed a commit to fblch/go-ethereum that referenced this pull request Sep 16, 2025
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NAT UPnP continuously remapping port

2 participants