Merge pull request #95 from libp2p/feat/nat-permanent-retry

Retry NAT punching without duration on mapping failure

p2p/nat/mapping.go

+package nat
+
+import (
+	"fmt"
+	"sync"
+	"time"
+
+	ma "github.com/jbenet/go-multiaddr"
+	manet "github.com/jbenet/go-multiaddr-net"
+	"github.com/jbenet/goprocess"
+)
+
+// Mapping represents a port mapping in a NAT.
+type Mapping interface {
+	// NAT returns the NAT object this Mapping belongs to.
+	NAT() *NAT
+
+	// Protocol returns the protocol of this port mapping. This is either
+	// "tcp" or "udp" as no other protocols are likely to be NAT-supported.
+	Protocol() string
+
+	// InternalPort returns the internal device port. Mapping will continue to
+	// try to map InternalPort() to an external facing port.
+	InternalPort() int
+
+	// ExternalPort returns the external facing port. If the mapping is not
+	// established, port will be 0
+	ExternalPort() int
+
+	// InternalAddr returns the internal address.
+	InternalAddr() ma.Multiaddr
+
+	// ExternalAddr returns the external facing address. If the mapping is not
+	// established, addr will be nil, and and ErrNoMapping will be returned.
+	ExternalAddr() (addr ma.Multiaddr, err error)
+
+	// Close closes the port mapping
+	Close() error
+}
+
+// keeps republishing
+type mapping struct {
+	sync.Mutex // guards all fields
+
+	nat       *NAT
+	proto     string
+	intport   int
+	extport   int
+	permanent bool
+	intaddr   ma.Multiaddr
+	proc      goprocess.Process
+
+	comment string
+
+	cached    ma.Multiaddr
+	cacheTime time.Time
+	cacheLk   sync.Mutex
+}
+
+func (m *mapping) NAT() *NAT {
+	m.Lock()
+	defer m.Unlock()
+	return m.nat
+}
+
+func (m *mapping) Protocol() string {
+	m.Lock()
+	defer m.Unlock()
+	return m.proto
+}
+
+func (m *mapping) InternalPort() int {
+	m.Lock()
+	defer m.Unlock()
+	return m.intport
+}
+
+func (m *mapping) ExternalPort() int {
+	m.Lock()
+	defer m.Unlock()
+	return m.extport
+}
+
+func (m *mapping) setExternalPort(p int) {
+	m.Lock()
+	defer m.Unlock()
+	m.extport = p
+}
+
+func (m *mapping) InternalAddr() ma.Multiaddr {
+	m.Lock()
+	defer m.Unlock()
+	return m.intaddr
+}
+
+func (m *mapping) ExternalAddr() (ma.Multiaddr, error) {
+	m.cacheLk.Lock()
+	ctime := m.cacheTime
+	cval := m.cached
+	m.cacheLk.Unlock()
+	if time.Since(ctime) < CacheTime {
+		return cval, nil
+	}
+
+	if m.ExternalPort() == 0 { // dont even try right now.
+		return nil, ErrNoMapping
+	}
+
+	m.nat.natmu.Lock()
+	ip, err := m.nat.nat.GetExternalAddress()
+	m.nat.natmu.Unlock()
+	if err != nil {
+		return nil, err
+	}
+
+	ipmaddr, err := manet.FromIP(ip)
+	if err != nil {
+		return nil, fmt.Errorf("error parsing ip")
+	}
+
+	// call m.ExternalPort again, as mapping may have changed under our feet. (tocttou)
+	extport := m.ExternalPort()
+	if extport == 0 {
+		return nil, ErrNoMapping
+	}
+
+	tcp, err := ma.NewMultiaddr(fmt.Sprintf("/%s/%d", m.Protocol(), extport))
+	if err != nil {
+		return nil, err
+	}
+
+	maddr2 := ipmaddr.Encapsulate(tcp)
+
+	m.cacheLk.Lock()
+	m.cached = maddr2
+	m.cacheTime = time.Now()
+	m.cacheLk.Unlock()
+	return maddr2, nil
+}
+
+func (m *mapping) Close() error {
+	return m.proc.Close()
+}

p2p/nat/nat.go

@@ -14,7 +14,6 @@ import (
 	manet "github.com/jbenet/go-multiaddr-net"
 	goprocess "github.com/jbenet/goprocess"
 	periodic "github.com/jbenet/goprocess/periodic"
-	notifier "github.com/whyrusleeping/go-notifier"
 )
 
 var (
@@ -53,8 +52,9 @@ func DiscoverNAT() *NAT {
 // service that will periodically renew port mappings,
 // and keep an up-to-date list of all the external addresses.
 type NAT struct {
-	nat  nat.NAT
-	proc goprocess.Process // manages nat mappings lifecycle
+	natmu sync.Mutex
+	nat   nat.NAT
+	proc  goprocess.Process // manages nat mappings lifecycle
 
 	mappingmu sync.RWMutex // guards mappings
 	mappings  map[*mapping]struct{}
@@ -81,175 +81,6 @@ func (nat *NAT) Process() goprocess.Process {
 	return nat.proc
 }
 
-// Notifier is an object that assists NAT in notifying listeners.
-// It is implemented using thirdparty/notifier
-type Notifier struct {
-	n notifier.Notifier
-}
-
-func (n *Notifier) notifyAll(notify func(n Notifiee)) {
-	n.n.NotifyAll(func(n notifier.Notifiee) {
-		notify(n.(Notifiee))
-	})
-}
-
-// Notify signs up notifiee to listen to NAT events.
-func (n *Notifier) Notify(notifiee Notifiee) {
-	n.n.Notify(n)
-}
-
-// StopNotify stops signaling events to notifiee.
-func (n *Notifier) StopNotify(notifiee Notifiee) {
-	n.n.StopNotify(notifiee)
-}
-
-// Notifiee is an interface objects must implement to listen to NAT events.
-type Notifiee interface {
-
-	// Called every time a successful mapping happens
-	// Warning: the port mapping may have changed. If that is the
-	// case, both MappingSuccess and MappingChanged are called.
-	MappingSuccess(nat *NAT, m Mapping)
-
-	// Called when mapping a port succeeds, but the mapping is
-	// with a different port than an earlier success.
-	MappingChanged(nat *NAT, m Mapping, oldport int, newport int)
-
-	// Called when a port mapping fails. NAT will continue attempting after
-	// the next period. To stop trying, use: mapping.Close(). After this failure,
-	// mapping.ExternalPort() will be zero, and nat.ExternalAddrs() will not
-	// return the address for this mapping. With luck, the next attempt will
-	// succeed, without the client needing to do anything.
-	MappingFailed(nat *NAT, m Mapping, oldport int, err error)
-}
-
-// Mapping represents a port mapping in a NAT.
-type Mapping interface {
-	// NAT returns the NAT object this Mapping belongs to.
-	NAT() *NAT
-
-	// Protocol returns the protocol of this port mapping. This is either
-	// "tcp" or "udp" as no other protocols are likely to be NAT-supported.
-	Protocol() string
-
-	// InternalPort returns the internal device port. Mapping will continue to
-	// try to map InternalPort() to an external facing port.
-	InternalPort() int
-
-	// ExternalPort returns the external facing port. If the mapping is not
-	// established, port will be 0
-	ExternalPort() int
-
-	// InternalAddr returns the internal address.
-	InternalAddr() ma.Multiaddr
-
-	// ExternalAddr returns the external facing address. If the mapping is not
-	// established, addr will be nil, and and ErrNoMapping will be returned.
-	ExternalAddr() (addr ma.Multiaddr, err error)
-
-	// Close closes the port mapping
-	Close() error
-}
-
-// keeps republishing
-type mapping struct {
-	sync.Mutex // guards all fields
-
-	nat     *NAT
-	proto   string
-	intport int
-	extport int
-	intaddr ma.Multiaddr
-	proc    goprocess.Process
-
-	cached    ma.Multiaddr
-	cacheTime time.Time
-	cacheLk   sync.Mutex
-}
-
-func (m *mapping) NAT() *NAT {
-	m.Lock()
-	defer m.Unlock()
-	return m.nat
-}
-
-func (m *mapping) Protocol() string {
-	m.Lock()
-	defer m.Unlock()
-	return m.proto
-}
-
-func (m *mapping) InternalPort() int {
-	m.Lock()
-	defer m.Unlock()
-	return m.intport
-}
-
-func (m *mapping) ExternalPort() int {
-	m.Lock()
-	defer m.Unlock()
-	return m.extport
-}
-
-func (m *mapping) setExternalPort(p int) {
-	m.Lock()
-	defer m.Unlock()
-	m.extport = p
-}
-
-func (m *mapping) InternalAddr() ma.Multiaddr {
-	m.Lock()
-	defer m.Unlock()
-	return m.intaddr
-}
-
-func (m *mapping) ExternalAddr() (ma.Multiaddr, error) {
-	m.cacheLk.Lock()
-	ctime := m.cacheTime
-	cval := m.cached
-	m.cacheLk.Unlock()
-	if time.Since(ctime) < CacheTime {
-		return cval, nil
-	}
-
-	if m.ExternalPort() == 0 { // dont even try right now.
-		return nil, ErrNoMapping
-	}
-
-	ip, err := m.nat.nat.GetExternalAddress()
-	if err != nil {
-		return nil, err
-	}
-
-	ipmaddr, err := manet.FromIP(ip)
-	if err != nil {
-		return nil, fmt.Errorf("error parsing ip")
-	}
-
-	// call m.ExternalPort again, as mapping may have changed under our feet. (tocttou)
-	extport := m.ExternalPort()
-	if extport == 0 {
-		return nil, ErrNoMapping
-	}
-
-	tcp, err := ma.NewMultiaddr(fmt.Sprintf("/%s/%d", m.Protocol(), extport))
-	if err != nil {
-		return nil, err
-	}
-
-	maddr2 := ipmaddr.Encapsulate(tcp)
-
-	m.cacheLk.Lock()
-	m.cached = maddr2
-	m.cacheTime = time.Now()
-	m.cacheLk.Unlock()
-	return maddr2, nil
-}
-
-func (m *mapping) Close() error {
-	return m.proc.Close()
-}
-
 // Mappings returns a slice of all NAT mappings
 func (nat *NAT) Mappings() []Mapping {
 	nat.mappingmu.Lock()
@@ -333,13 +164,25 @@ func (nat *NAT) NewMapping(maddr ma.Multiaddr) (Mapping, error) {
 
 func (nat *NAT) establishMapping(m *mapping) {
 	oldport := m.ExternalPort()
+
 	log.Debugf("Attempting port map: %s/%d", m.Protocol(), m.InternalPort())
-	newport, err := nat.nat.AddPortMapping(m.Protocol(), m.InternalPort(), "http", MappingDuration)
+	comment := "libp2p"
+	if m.comment != "" {
+		comment = "libp2p-" + m.comment
+	}
+
+	nat.natmu.Lock()
+	newport, err := nat.nat.AddPortMapping(m.Protocol(), m.InternalPort(), comment, MappingDuration)
+	if err != nil {
+		// Some hardware does not support mappings with timeout, so try that
+		newport, err = nat.nat.AddPortMapping(m.Protocol(), m.InternalPort(), comment, 0)
+	}
+	nat.natmu.Lock()
 
 	failure := func() {
 		m.setExternalPort(0) // clear mapping
 		// TODO: log.Event
-		log.Debugf("failed to establish port mapping: %s", err)
+		log.Warningf("failed to establish port mapping: %s", err)
 		nat.Notifier.notifyAll(func(n Notifiee) {
 			n.MappingFailed(nat, m, oldport, err)
 		})

p2p/nat/notifier.go

+package nat
+
+import (
+	notifier "github.com/whyrusleeping/go-notifier"
+)
+
+// Notifier is an object that assists NAT in notifying listeners.
+// It is implemented using thirdparty/notifier
+type Notifier struct {
+	n notifier.Notifier
+}
+
+func (n *Notifier) notifyAll(notify func(n Notifiee)) {
+	n.n.NotifyAll(func(n notifier.Notifiee) {
+		notify(n.(Notifiee))
+	})
+}
+
+// Notify signs up notifiee to listen to NAT events.
+func (n *Notifier) Notify(notifiee Notifiee) {
+	n.n.Notify(n)
+}
+
+// StopNotify stops signaling events to notifiee.
+func (n *Notifier) StopNotify(notifiee Notifiee) {
+	n.n.StopNotify(notifiee)
+}
+
+// Notifiee is an interface objects must implement to listen to NAT events.
+type Notifiee interface {
+
+	// Called every time a successful mapping happens
+	// Warning: the port mapping may have changed. If that is the
+	// case, both MappingSuccess and MappingChanged are called.
+	MappingSuccess(nat *NAT, m Mapping)
+
+	// Called when mapping a port succeeds, but the mapping is
+	// with a different port than an earlier success.
+	MappingChanged(nat *NAT, m Mapping, oldport int, newport int)
+
+	// Called when a port mapping fails. NAT will continue attempting after
+	// the next period. To stop trying, use: mapping.Close(). After this failure,
+	// mapping.ExternalPort() will be zero, and nat.ExternalAddrs() will not
+	// return the address for this mapping. With luck, the next attempt will
+	// succeed, without the client needing to do anything.
+	MappingFailed(nat *NAT, m Mapping, oldport int, err error)
+}