From 06424024499fe4eae68776bfa397ba3ecf69a38a Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sat, 25 Apr 2015 20:52:07 +0900 Subject: [PATCH] Break out UPnP port mapping into a service --- cmd/syncthing/main.go | 123 +++------------------------------ cmd/syncthing/upnpsvc.go | 111 +++++++++++++++++++++++++++++ internal/config/config.go | 2 +- internal/config/config_test.go | 2 +- internal/upnp/upnp.go | 8 --- test/h2/config.xml | 4 +- 6 files changed, 124 insertions(+), 126 deletions(-) create mode 100644 cmd/syncthing/upnpsvc.go diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 1b035da61..3153fc0ec 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 000000000..df71a69bd --- /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 6291835ef..caca77369 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 7cd7f7bdb..048a85dfc 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 e3e3f4742..fc53f9209 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 d7740200f..5bf45a9f4 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