Merge pull request #960 from fjl/nat-fixes

p2p/nat: fix UPnP auto discovery

1 files changed, 21 insertions, 24 deletions

diff --git a/p2p/nat/nat.go b/p2p/nat/nat.go
index fe00bdab0..9acb34398 100644
--- a/p2p/nat/nat.go
+++ b/p2p/nat/nat.go
@@ -86,13 +86,13 @@ func Map(m Interface, c chan struct{}, protocol string, extport, intport int, na
 	refresh := time.NewTimer(mapUpdateInterval)
 	defer func() {
 		refresh.Stop()
-		glog.V(logger.Debug).Infof("Deleting port mapping: %s %d -> %d (%s) using %s\n", protocol, extport, intport, name, m)
+		glog.V(logger.Debug).Infof("deleting port mapping: %s %d -> %d (%s) using %s\n", protocol, extport, intport, name, m)
 		m.DeleteMapping(protocol, extport, intport)
 	}()
-	glog.V(logger.Debug).Infof("add mapping: %s %d -> %d (%s) using %s\n", protocol, extport, intport, name, m)
 	if err := m.AddMapping(protocol, intport, extport, name, mapTimeout); err != nil {
-		glog.V(logger.Warn).Infof("network port %d could not be mapped: %v\n", intport, err)
-		glog.V(logger.Debug).Infof("mapping with %v returned %v\n", m, err)
+		glog.V(logger.Debug).Infof("network port %s:%d could not be mapped: %v\n", protocol, intport, err)
+	} else {
+		glog.V(logger.Info).Infof("mapped network port %s:%d -> %d (%s) using %s\n", protocol, extport, intport, name, m)
 	}
 	for {
 		select {
@@ -101,10 +101,9 @@ func Map(m Interface, c chan struct{}, protocol string, extport, intport int, na
 				return
 			}
 		case <-refresh.C:
-			glog.V(logger.Detail).Infof("refresh mapping: %s %d -> %d (%s) using %s\n", protocol, extport, intport, name, m)
+			glog.V(logger.Detail).Infof("refresh port mapping %s:%d -> %d (%s) using %s\n", protocol, extport, intport, name, m)
 			if err := m.AddMapping(protocol, intport, extport, name, mapTimeout); err != nil {
-				glog.V(logger.Warn).Infof("network port %d could not be mapped: %v\n", intport, err)
-				glog.V(logger.Debug).Infof("mapping with %v returned %v\n", m, err)
+				glog.V(logger.Debug).Infof("network port %s:%d could not be mapped: %v\n", protocol, intport, err)
 			}
 			refresh.Reset(mapUpdateInterval)
 		}
@@ -172,8 +171,9 @@ func PMP(gateway net.IP) Interface {
 // This type is useful because discovery can take a while but we
 // want return an Interface value from UPnP, PMP and Auto immediately.
 type autodisc struct {
-	what string
-	done <-chan Interface
+	what string // type of interface being autodiscovered
+	once sync.Once
+	doit func() Interface
 
 	mu    sync.Mutex
 	found Interface
@@ -181,9 +181,10 @@ type autodisc struct {
 
 func startautodisc(what string, doit func() Interface) Interface {
 	// TODO: monitor network configuration and rerun doit when it changes.
-	done := make(chan Interface)
-	ad := &autodisc{what: what, done: done}
-	go func() { done <- doit(); close(done) }()
+	ad := &autodisc{what: what, doit: doit}
+	// Start the auto discovery as early as possible so it is already
+	// in progress when the rest of the stack calls the methods.
+	go ad.wait()
 	return ad
 }
 
@@ -218,19 +219,15 @@ func (n *autodisc) String() string {
 	}
 }
 
+// wait blocks until auto-discovery has been performed.
 func (n *autodisc) wait() error {
-	n.mu.Lock()
-	found := n.found
-	n.mu.Unlock()
-	if found != nil {
-		// already discovered
-		return nil
-	}
-	if found = <-n.done; found == nil {
-		return errors.New("no UPnP or NAT-PMP router discovered")
+	n.once.Do(func() {
+		n.mu.Lock()
+		n.found = n.doit()
+		n.mu.Unlock()
+	})
+	if n.found == nil {
+		return fmt.Errorf("no %s router discovered", n.what)
 	}
-	n.mu.Lock()
-	n.found = found
-	n.mu.Unlock()
 	return nil
 }