diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go
index 1b035da6..3153fc0e 100644
--- a/cmd/syncthing/main.go
+++ b/cmd/syncthing/main.go
@@ -35,7 +35,6 @@ import (
"github.com/syncthing/syncthing/internal/osutil"
"github.com/syncthing/syncthing/internal/symlinks"
"github.com/syncthing/syncthing/internal/upgrade"
- "github.com/syncthing/syncthing/internal/upnp"
"github.com/syndtr/goleveldb/leveldb"
"github.com/syndtr/goleveldb/leveldb/errors"
"github.com/syndtr/goleveldb/leveldb/opt"
@@ -109,8 +108,6 @@ var (
readRateLimit *ratelimit.Bucket
stop = make(chan int)
discoverer *discover.Discoverer
- externalPort int
- igd *upnp.IGD
cert tls.Certificate
lans []*net.IPNet
)
@@ -573,18 +570,20 @@ func syncthingMain() {
if err != nil {
l.Fatalln("Bad listen address:", err)
}
- externalPort = addr.Port
- // UPnP
- igd = nil
+ // Start discovery
+
+ localPort := addr.Port
+ discoverer = discovery(localPort)
+
+ // Start UPnP. The UPnP service will restart global discovery if the
+ // external port changes.
if opts.UPnPEnabled {
- setupUPnP()
+ upnpSvc := newUPnPSvc(cfg, localPort)
+ mainSvc.Add(upnpSvc)
}
- // Routine to connect out to configured devices
- discoverer = discovery(externalPort)
-
connectionSvc := newConnectionSvc(cfg, myID, m, tlsCfg)
mainSvc.Add(connectionSvc)
@@ -765,110 +764,6 @@ func generatePingEvents() {
}
}
-func setupUPnP() {
- if opts := cfg.Options(); len(opts.ListenAddress) == 1 {
- _, portStr, err := net.SplitHostPort(opts.ListenAddress[0])
- if err != nil {
- l.Warnln("Bad listen address:", err)
- } else {
- // Set up incoming port forwarding, if necessary and possible
- port, _ := strconv.Atoi(portStr)
- igds := upnp.Discover(time.Duration(cfg.Options().UPnPTimeoutS) * time.Second)
- if len(igds) > 0 {
- // Configure the first discovered IGD only. This is a work-around until we have a better mechanism
- // for handling multiple IGDs, which will require changes to the global discovery service
- igd = &igds[0]
-
- externalPort = setupExternalPort(igd, port)
- if externalPort == 0 {
- l.Warnln("Failed to create UPnP port mapping")
- } else {
- l.Infof("Created UPnP port mapping for external port %d on UPnP device %s.", externalPort, igd.FriendlyIdentifier())
-
- if opts.UPnPRenewalM > 0 {
- go renewUPnP(port)
- }
- }
- }
- }
- } else {
- l.Warnln("Multiple listening addresses; not attempting UPnP port mapping")
- }
-}
-
-func setupExternalPort(igd *upnp.IGD, port int) int {
- if igd == nil {
- return 0
- }
-
- for i := 0; i < 10; i++ {
- r := 1024 + predictableRandom.Intn(65535-1024)
- err := igd.AddPortMapping(upnp.TCP, r, port, fmt.Sprintf("syncthing-%d", r), cfg.Options().UPnPLeaseM*60)
- if err == nil {
- return r
- }
- }
- return 0
-}
-
-func renewUPnP(port int) {
- for {
- opts := cfg.Options()
- time.Sleep(time.Duration(opts.UPnPRenewalM) * time.Minute)
- // Some values might have changed while we were sleeping
- opts = cfg.Options()
-
- // Make sure our IGD reference isn't nil
- if igd == nil {
- if debugNet {
- l.Debugln("Undefined IGD during UPnP port renewal. Re-discovering...")
- }
- igds := upnp.Discover(time.Duration(opts.UPnPTimeoutS) * time.Second)
- if len(igds) > 0 {
- // Configure the first discovered IGD only. This is a work-around until we have a better mechanism
- // for handling multiple IGDs, which will require changes to the global discovery service
- igd = &igds[0]
- } else {
- if debugNet {
- l.Debugln("Failed to discover IGD during UPnP port mapping renewal.")
- }
- continue
- }
- }
-
- // Just renew the same port that we already have
- if externalPort != 0 {
- err := igd.AddPortMapping(upnp.TCP, externalPort, port, "syncthing", opts.UPnPLeaseM*60)
- if err != nil {
- l.Warnf("Error renewing UPnP port mapping for external port %d on device %s: %s", externalPort, igd.FriendlyIdentifier(), err.Error())
- } else if debugNet {
- l.Debugf("Renewed UPnP port mapping for external port %d on device %s.", externalPort, igd.FriendlyIdentifier())
- }
-
- continue
- }
-
- // Something strange has happened. We didn't have an external port before?
- // Or perhaps the gateway has changed?
- // Retry the same port sequence from the beginning.
- if debugNet {
- l.Debugln("No UPnP port mapping defined, updating...")
- }
-
- forwardedPort := setupExternalPort(igd, port)
- if forwardedPort != 0 {
- externalPort = forwardedPort
- discoverer.StopGlobal()
- discoverer.StartGlobal(opts.GlobalAnnServers, uint16(forwardedPort))
- if debugNet {
- l.Debugf("Updated UPnP port mapping for external port %d on device %s.", forwardedPort, igd.FriendlyIdentifier())
- }
- } else {
- l.Warnf("Failed to update UPnP port mapping for external port on device " + igd.FriendlyIdentifier() + ".")
- }
- }
-}
-
func resetDB() error {
return os.RemoveAll(locations[locDatabase])
}
diff --git a/cmd/syncthing/upnpsvc.go b/cmd/syncthing/upnpsvc.go
new file mode 100644
index 00000000..df71a69b
--- /dev/null
+++ b/cmd/syncthing/upnpsvc.go
@@ -0,0 +1,111 @@
+// Copyright (C) 2015 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package main
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/syncthing/syncthing/internal/config"
+ "github.com/syncthing/syncthing/internal/upnp"
+)
+
+// The UPnP service runs a loop for discovery of IGDs (Internet Gateway
+// Devices) and setup/renewal of a port mapping.
+type upnpSvc struct {
+ cfg *config.Wrapper
+ localPort int
+}
+
+func newUPnPSvc(cfg *config.Wrapper, localPort int) *upnpSvc {
+ return &upnpSvc{
+ cfg: cfg,
+ localPort: localPort,
+ }
+}
+
+func (s *upnpSvc) Serve() {
+ extPort := 0
+ foundIGD := true
+
+ for {
+ igds := upnp.Discover(time.Duration(s.cfg.Options().UPnPTimeoutS) * time.Second)
+ if len(igds) > 0 {
+ foundIGD = true
+ extPort = s.tryIGDs(igds, extPort)
+ } else if foundIGD {
+ // Only print a notice if we've previously found an IGD or this is
+ // the first time around.
+ foundIGD = false
+ l.Infof("No UPnP device detected")
+ }
+
+ d := time.Duration(s.cfg.Options().UPnPRenewalM) * time.Minute
+ if d == 0 {
+ // We always want to do renewal so lets just pick a nice sane number.
+ d = 30 * time.Minute
+ }
+ time.Sleep(d)
+ }
+}
+
+func (s *upnpSvc) Stop() {
+ panic("upnpSvc cannot stop")
+}
+
+func (s *upnpSvc) tryIGDs(igds []upnp.IGD, prevExtPort int) int {
+ // Lets try all the IGDs we found and use the first one that works.
+ // TODO: Use all of them, and sort out the resulting mess to the
+ // discovery announcement code...
+ for _, igd := range igds {
+ extPort, err := s.tryIGD(igd, prevExtPort)
+ if err != nil {
+ l.Warnf("Failed to set UPnP port mapping: external port %d on device %s.", extPort, igd.FriendlyIdentifier())
+ continue
+ }
+
+ if extPort != prevExtPort {
+ // External port changed; refresh the discovery announcement.
+ // TODO: Don't reach out to some magic global here?
+ l.Infof("New UPnP port mapping: external port %d to local port %d.", extPort, s.localPort)
+ discoverer.StopGlobal()
+ discoverer.StartGlobal(s.cfg.Options().GlobalAnnServers, uint16(extPort))
+ }
+ if debugNet {
+ l.Debugf("Created/updated UPnP port mapping for external port %d on device %s.", extPort, igd.FriendlyIdentifier())
+ }
+ return extPort
+ }
+
+ return 0
+}
+
+func (s *upnpSvc) tryIGD(igd upnp.IGD, suggestedPort int) (int, error) {
+ var err error
+ leaseTime := s.cfg.Options().UPnPLeaseM * 60
+
+ if suggestedPort != 0 {
+ // First try renewing our existing mapping.
+ name := fmt.Sprintf("syncthing-%d", suggestedPort)
+ err = igd.AddPortMapping(upnp.TCP, suggestedPort, s.localPort, name, leaseTime)
+ if err == nil {
+ return suggestedPort, nil
+ }
+ }
+
+ for i := 0; i < 10; i++ {
+ // Then try up to ten random ports.
+ extPort := 1024 + predictableRandom.Intn(65535-1024)
+ name := fmt.Sprintf("syncthing-%d", suggestedPort)
+ err = igd.AddPortMapping(upnp.TCP, extPort, s.localPort, name, leaseTime)
+ if err == nil {
+ return extPort, nil
+ }
+ }
+
+ return 0, err
+}
diff --git a/internal/config/config.go b/internal/config/config.go
index 6291835e..caca7736 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -230,7 +230,7 @@ type OptionsConfiguration struct {
UPnPEnabled bool `xml:"upnpEnabled" json:"upnpEnabled" default:"true"`
UPnPLeaseM int `xml:"upnpLeaseMinutes" json:"upnpLeaseMinutes" default:"0"`
UPnPRenewalM int `xml:"upnpRenewalMinutes" json:"upnpRenewalMinutes" default:"30"`
- UPnPTimeoutS int `xml:"upnpTimeoutSeconds" json:"upnpTimeoutSeconds" default:"3"`
+ UPnPTimeoutS int `xml:"upnpTimeoutSeconds" json:"upnpTimeoutSeconds" default:"10"`
URAccepted int `xml:"urAccepted" json:"urAccepted"` // Accepted usage reporting version; 0 for off (undecided), -1 for off (permanently)
URUniqueID string `xml:"urUniqueID" json:"urUniqueId"` // Unique ID for reporting purposes, regenerated when UR is turned on.
RestartOnWakeup bool `xml:"restartOnWakeup" json:"restartOnWakeup" default:"true"`
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 7cd7f7bd..048a85df 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -44,7 +44,7 @@ func TestDefaultValues(t *testing.T) {
UPnPEnabled: true,
UPnPLeaseM: 0,
UPnPRenewalM: 30,
- UPnPTimeoutS: 3,
+ UPnPTimeoutS: 10,
RestartOnWakeup: true,
AutoUpgradeIntervalH: 12,
KeepTemporariesH: 24,
diff --git a/internal/upnp/upnp.go b/internal/upnp/upnp.go
index e3e3f474..fc53f920 100644
--- a/internal/upnp/upnp.go
+++ b/internal/upnp/upnp.go
@@ -95,7 +95,6 @@ type upnpRoot struct {
// The order in which the devices appear in the results list is not deterministic.
func Discover(timeout time.Duration) []IGD {
var results []IGD
- l.Infoln("Starting UPnP discovery...")
interfaces, err := net.Interfaces()
if err != nil {
@@ -144,13 +143,6 @@ func Discover(timeout time.Duration) []IGD {
wg.Wait()
close(resultChan)
- suffix := "devices"
- if len(results) == 1 {
- suffix = "device"
- }
-
- l.Infof("UPnP discovery complete (found %d %s).", len(results), suffix)
-
return results
}
diff --git a/test/h2/config.xml b/test/h2/config.xml
index d7740200..5bf45a9f 100644
--- a/test/h2/config.xml
+++ b/test/h2/config.xml
@@ -54,9 +54,9 @@
0
5
false
- false
+ true
0
- 30
+ 1
-1
true