p2p/nat: remove forceful port mapping in upnp (#30265)

Here we are modifying the port mapping logic so that existing port
mappings will only be removed when they were previously created by geth.

The AddAnyPortMapping functionality has been adapted to work consistently
between the IGDv1 and IGDv2 backends.
This commit is contained in:
Nathan Jo 2025-04-04 17:56:55 +09:00 committed by GitHub
parent 9f83e9e673
commit ff365afc63
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 30 additions and 22 deletions

View file

@ -49,6 +49,9 @@ func (n *pmp) AddMapping(protocol string, extport, intport int, name string, lif
if lifetime <= 0 {
return 0, errors.New("lifetime must not be <= 0")
}
if extport == 0 {
extport = intport
}
// Note order of port arguments is switched between our
// AddMapping and the client's AddPortMapping.
res, err := n.c.AddPortMapping(strings.ToLower(protocol), intport, extport, int(lifetime/time.Second))

View file

@ -86,15 +86,15 @@ func (n *upnp) AddMapping(protocol string, extport, intport int, desc string, li
}
protocol = strings.ToUpper(protocol)
lifetimeS := uint32(lifetime / time.Second)
n.DeleteMapping(protocol, extport, intport)
err = n.withRateLimit(func() error {
return n.client.AddPortMapping("", uint16(extport), protocol, uint16(intport), ip.String(), true, desc, lifetimeS)
})
if err == nil {
return uint16(extport), nil
if extport == 0 {
extport = intport
} else {
// Only delete port mapping if the external port was already used by geth.
n.DeleteMapping(protocol, extport, intport)
}
// Try addAnyPortMapping if mapping specified port didn't work.
// Try to add port mapping, preferring the specified external port.
err = n.withRateLimit(func() error {
p, err := n.addAnyPortMapping(protocol, extport, intport, ip, desc, lifetimeS)
if err == nil {
@ -105,18 +105,28 @@ func (n *upnp) AddMapping(protocol string, extport, intport int, desc string, li
return uint16(extport), err
}
// addAnyPortMapping tries to add a port mapping with the specified external port.
// If the external port is already in use, it will try to assign another port.
func (n *upnp) addAnyPortMapping(protocol string, extport, intport int, ip net.IP, desc string, lifetimeS uint32) (uint16, error) {
if client, ok := n.client.(*internetgateway2.WANIPConnection2); ok {
return client.AddAnyPortMapping("", uint16(extport), protocol, uint16(intport), ip.String(), true, desc, lifetimeS)
}
// It will retry with a random port number if the client does
// not support AddAnyPortMapping.
extport = n.randomPort()
// For IGDv1 and v1 services we should first try to add with extport.
err := n.client.AddPortMapping("", uint16(extport), protocol, uint16(intport), ip.String(), true, desc, lifetimeS)
if err != nil {
return 0, err
if err == nil {
return uint16(extport), nil
}
return uint16(extport), nil
// If above fails, we retry with a random port.
// We retry several times because of possible port conflicts.
for i := 0; i < 3; i++ {
extport = n.randomPort()
err := n.client.AddPortMapping("", uint16(extport), protocol, uint16(intport), ip.String(), true, desc, lifetimeS)
if err == nil {
return uint16(extport), nil
}
}
return 0, err
}
func (n *upnp) randomPort() int {

View file

@ -150,14 +150,9 @@ func (srv *Server) portMappingLoop() {
continue
}
external := m.port
if m.extPort != 0 {
external = m.extPort
}
log := newLogger(m.protocol, external, m.port)
log := newLogger(m.protocol, m.extPort, m.port)
log.Trace("Attempting port mapping")
p, err := srv.NAT.AddMapping(m.protocol, external, m.port, m.name, portMapDuration)
p, err := srv.NAT.AddMapping(m.protocol, m.extPort, m.port, m.name, portMapDuration)
if err != nil {
log.Debug("Couldn't add port mapping", "err", err)
m.extPort = 0
@ -167,8 +162,8 @@ func (srv *Server) portMappingLoop() {
// It was mapped!
m.extPort = int(p)
m.nextTime = srv.clock.Now().Add(portMapRefreshInterval)
if external != m.extPort {
log = newLogger(m.protocol, m.extPort, m.port)
log = newLogger(m.protocol, m.extPort, m.port)
if m.port != m.extPort {
log.Info("NAT mapped alternative port")
} else {
log.Info("NAT mapped port")