lib/config, lib/connections: Configurable protocol priority (ref #8626) (#8868)

This makes the various protocol priorities configurable among the other
options. With this, it's possible to prefer QUIC over TCP for WAN
connections, for example. Both sides need to be similarly configured for
this to work properly.

The default priority order remains the same as previously (TCP, QUIC,
Relay, with LAN better than WAN).

To make this happen I made each dialer & listener more priority aware,
and moved the check for whether a connection is LAN or not into the
dialer / listener -- this is the new "lanChecker" type that's passed
around.

lib/connections/connections_test.go

@@ -439,7 +439,8 @@ func withConnectionPair(b interface{ Fatal(...interface{}) }, connUri string, h
 	}
 	natSvc := nat.NewService(deviceId, wcfg)
 	conns := make(chan internalConn, 1)
-	listenSvc := lf.New(uri, wcfg, tlsCfg, conns, natSvc, registry.New())
+	lanChecker := &lanChecker{wcfg}
+	listenSvc := lf.New(uri, wcfg, tlsCfg, conns, natSvc, registry.New(), lanChecker)
 	supervisor.Add(listenSvc)
 
 	var addr *url.URL
@@ -459,7 +460,7 @@ func withConnectionPair(b interface{ Fatal(...interface{}) }, connUri string, h
 		b.Fatal(err)
 	}
 	// Purposely using a different registry: Don't want to reuse port between dialer and listener on the same device
-	dialer := df.New(cfg.Options, tlsCfg, registry.New())
+	dialer := df.New(cfg.Options, tlsCfg, registry.New(), lanChecker)
 
 	// Relays might take some time to register the device, so dial multiple times
 	clientConn, err := dialer.Dial(ctx, deviceId, addr)

lib/connections/lan_test.go

@@ -38,7 +38,7 @@ func TestIsLANHost(t *testing.T) {
 			AlwaysLocalNets: []string{"10.20.30.0/24"},
 		},
 	}, protocol.LocalDeviceID, events.NoopLogger)
-	s := &service{cfg: cfg}
+	s := &lanChecker{cfg: cfg}
 
 	for _, tc := range cases {
 		res := s.isLANHost(tc.addr)

lib/connections/quic_dial.go

@@ -25,8 +25,6 @@ import (
 )
 
 const (
-	quicPriority = 100
-
 	// The timeout for connecting, accepting and creating the various
 	// streams.
 	quicOperationTimeout = 10 * time.Second
@@ -92,12 +90,17 @@ func (d *quicDialer) Dial(ctx context.Context, _ protocol.DeviceID, uri *url.URL
 		return internalConn{}, fmt.Errorf("open stream: %w", err)
 	}
 
-	return newInternalConn(&quicTlsConn{session, stream, createdConn}, connTypeQUICClient, quicPriority), nil
+	priority := d.wanPriority
+	isLocal := d.lanChecker.isLAN(session.RemoteAddr())
+	if isLocal {
+		priority = d.lanPriority
+	}
+	return newInternalConn(&quicTlsConn{session, stream, createdConn}, connTypeQUICClient, isLocal, priority), nil
 }
 
 type quicDialerFactory struct{}
 
-func (quicDialerFactory) New(opts config.OptionsConfiguration, tlsCfg *tls.Config, registry *registry.Registry) genericDialer {
+func (quicDialerFactory) New(opts config.OptionsConfiguration, tlsCfg *tls.Config, registry *registry.Registry, lanChecker *lanChecker) genericDialer {
 	// So the idea is that we should probably try dialing every 20 seconds.
 	// However it would still be nice if this was adjustable/proportional to ReconnectIntervalS
 	// But prevent something silly like 1/3 = 0 etc.
@@ -109,15 +112,14 @@ func (quicDialerFactory) New(opts config.OptionsConfiguration, tlsCfg *tls.Confi
 		commonDialer: commonDialer{
 			reconnectInterval: time.Duration(quicInterval) * time.Second,
 			tlsCfg:            tlsCfg,
+			lanPriority:       opts.ConnectionPriorityQUICLAN,
+			wanPriority:       opts.ConnectionPriorityQUICWAN,
+			lanChecker:        lanChecker,
 		},
 		registry: registry,
 	}
 }
 
-func (quicDialerFactory) Priority() int {
-	return quicPriority
-}
-
 func (quicDialerFactory) AlwaysWAN() bool {
 	return false
 }

lib/connections/quic_listen.go

@@ -40,12 +40,13 @@ type quicListener struct {
 
 	onAddressesChangedNotifier
 
-	uri      *url.URL
-	cfg      config.Wrapper
-	tlsCfg   *tls.Config
-	conns    chan internalConn
-	factory  listenerFactory
-	registry *registry.Registry
+	uri        *url.URL
+	cfg        config.Wrapper
+	tlsCfg     *tls.Config
+	conns      chan internalConn
+	factory    listenerFactory
+	registry   *registry.Registry
+	lanChecker *lanChecker
 
 	address *url.URL
 	laddr   net.Addr
@@ -168,7 +169,12 @@ func (t *quicListener) serve(ctx context.Context) error {
 			continue
 		}
 
-		t.conns <- newInternalConn(&quicTlsConn{session, stream, nil}, connTypeQUICServer, quicPriority)
+		priority := t.cfg.Options().ConnectionPriorityQUICWAN
+		isLocal := t.lanChecker.isLAN(session.RemoteAddr())
+		if isLocal {
+			priority = t.cfg.Options().ConnectionPriorityQUICLAN
+		}
+		t.conns <- newInternalConn(&quicTlsConn{session, stream, nil}, connTypeQUICServer, isLocal, priority)
 	}
 }
 
@@ -218,14 +224,15 @@ func (*quicListenerFactory) Valid(config.Configuration) error {
 	return nil
 }
 
-func (f *quicListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.Config, conns chan internalConn, _ *nat.Service, registry *registry.Registry) genericListener {
+func (f *quicListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.Config, conns chan internalConn, _ *nat.Service, registry *registry.Registry, lanChecker *lanChecker) genericListener {
 	l := &quicListener{
-		uri:      fixupPort(uri, config.DefaultQUICPort),
-		cfg:      cfg,
-		tlsCfg:   tlsCfg,
-		conns:    conns,
-		factory:  f,
-		registry: registry,
+		uri:        fixupPort(uri, config.DefaultQUICPort),
+		cfg:        cfg,
+		tlsCfg:     tlsCfg,
+		conns:      conns,
+		factory:    f,
+		registry:   registry,
+		lanChecker: lanChecker,
 	}
 	l.ServiceWithError = svcutil.AsService(l.serve, l.String())
 	l.nat.Store(uint64(stun.NATUnknown))

lib/connections/relay_dial.go

@@ -19,8 +19,6 @@ import (
 	"github.com/syncthing/syncthing/lib/relay/client"
 )
 
-const relayPriority = 200
-
 func init() {
 	dialers["relay"] = relayDialerFactory{}
 }
@@ -64,23 +62,21 @@ func (d *relayDialer) Dial(ctx context.Context, id protocol.DeviceID, uri *url.U
 		return internalConn{}, err
 	}
 
-	return newInternalConn(tc, connTypeRelayClient, relayPriority), nil
+	return newInternalConn(tc, connTypeRelayClient, false, d.wanPriority), nil
 }
 
 type relayDialerFactory struct{}
 
-func (relayDialerFactory) New(opts config.OptionsConfiguration, tlsCfg *tls.Config, _ *registry.Registry) genericDialer {
+func (relayDialerFactory) New(opts config.OptionsConfiguration, tlsCfg *tls.Config, _ *registry.Registry, _ *lanChecker) genericDialer {
 	return &relayDialer{commonDialer{
 		trafficClass:      opts.TrafficClass,
 		reconnectInterval: time.Duration(opts.RelayReconnectIntervalM) * time.Minute,
 		tlsCfg:            tlsCfg,
+		wanPriority:       opts.ConnectionPriorityRelay,
+		lanPriority:       opts.ConnectionPriorityRelay,
 	}}
 }
 
-func (relayDialerFactory) Priority() int {
-	return relayPriority
-}
-
 func (relayDialerFactory) AlwaysWAN() bool {
 	return true
 }

lib/connections/relay_listen.go

@@ -106,7 +106,7 @@ func (t *relayListener) handleInvitations(ctx context.Context, clnt client.Relay
 				continue
 			}
 
-			t.conns <- newInternalConn(tc, connTypeRelayServer, relayPriority)
+			t.conns <- newInternalConn(tc, connTypeRelayServer, false, t.cfg.Options().ConnectionPriorityRelay)
 
 		// Poor mans notifier that informs the connection service that the
 		// relay URI has changed. This can only happen when we connect to a
@@ -177,7 +177,7 @@ func (*relayListener) NATType() string {
 
 type relayListenerFactory struct{}
 
-func (f *relayListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.Config, conns chan internalConn, _ *nat.Service, _ *registry.Registry) genericListener {
+func (f *relayListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.Config, conns chan internalConn, _ *nat.Service, _ *registry.Registry, _ *lanChecker) genericListener {
 	t := &relayListener{
 		uri:     uri,
 		cfg:     cfg,

lib/connections/service.go

@@ -162,6 +162,7 @@ type service struct {
 	evLogger             events.Logger
 	registry             *registry.Registry
 	keyGen               *protocol.KeyGenerator
+	lanChecker           *lanChecker
 
 	dialNow           chan struct{}
 	dialNowDevices    map[protocol.DeviceID]struct{}
@@ -192,6 +193,7 @@ func NewService(cfg config.Wrapper, myID protocol.DeviceID, mdl Model, tlsCfg *t
 		evLogger:             evLogger,
 		registry:             registry,
 		keyGen:               keyGen,
+		lanChecker:           &lanChecker{cfg},
 
 		dialNowDevicesMut: sync.NewMutex(),
 		dialNow:           make(chan struct{}, 1),
@@ -405,9 +407,6 @@ func (s *service) handleHellos(ctx context.Context) error {
 			continue
 		}
 
-		// Determine only once whether a connection is considered local
-		// according to our configuration, then cache the decision.
-		c.isLocal = s.isLAN(c.RemoteAddr())
 		// Wrap the connection in rate limiters. The limiter itself will
 		// keep up with config changes to the rate and whether or not LAN
 		// connections are limited.
@@ -496,13 +495,14 @@ func (s *service) connect(ctx context.Context) error {
 	}
 }
 
-func (*service) bestDialerPriority(cfg config.Configuration) int {
+func (s *service) bestDialerPriority(cfg config.Configuration) int {
 	bestDialerPriority := worstDialerPriority
 	for _, df := range dialers {
 		if df.Valid(cfg) != nil {
 			continue
 		}
-		if prio := df.Priority(); prio < bestDialerPriority {
+		prio := df.New(cfg.Options, s.tlsCfg, s.registry, s.lanChecker).Priority("127.0.0.1")
+		if prio < bestDialerPriority {
 			bestDialerPriority = prio
 		}
 	}
@@ -544,7 +544,14 @@ func (s *service) dialDevices(ctx context.Context, now time.Time, cfg config.Con
 		priorityCutoff := worstDialerPriority
 		connection, connected := s.model.Connection(deviceCfg.DeviceID)
 		if connected {
+			// Set the priority cutoff to the current connection's priority,
+			// so that we don't attempt any dialers with worse priority.
 			priorityCutoff = connection.Priority()
+
+			// Reduce the priority cutoff by the upgrade threshold, so that
+			// we don't attempt dialers that aren't considered a worthy upgrade.
+			priorityCutoff -= cfg.Options.ConnectionPriorityUpgradeThreshold
+
 			if bestDialerPriority >= priorityCutoff {
 				// Our best dialer is not any better than what we already
 				// have, so nothing to do here.
@@ -564,7 +571,7 @@ func (s *service) dialDevices(ctx context.Context, now time.Time, cfg config.Con
 	}
 
 	// Sort the queue in an order we think will be useful (most recent
-	// first, deprioriting unstable devices, randomizing those we haven't
+	// first, deprioritising unstable devices, randomizing those we haven't
 	// seen in a long while). If we don't do connection limiting the sorting
 	// doesn't have much effect, but it may result in getting up and running
 	// quicker if only a subset of configured devices are actually reachable
@@ -657,24 +664,15 @@ func (s *service) resolveDialTargets(ctx context.Context, now time.Time, cfg con
 			continue
 		}
 
-		priority := dialerFactory.Priority()
+		dialer := dialerFactory.New(s.cfg.Options(), s.tlsCfg, s.registry, s.lanChecker)
+		priority := dialer.Priority(uri.Host)
 		if priority >= priorityCutoff {
-			l.Debugf("Not dialing using %s as priority is not better than current connection (%d >= %d)", dialerFactory, dialerFactory.Priority(), priorityCutoff)
+			l.Debugf("Not dialing using %s as priority is not better than current connection (%d >= %d)", dialerFactory, priority, priorityCutoff)
 			continue
 		}
 
-		dialer := dialerFactory.New(s.cfg.Options(), s.tlsCfg, s.registry)
 		nextDialAt.set(deviceID, addr, now.Add(dialer.RedialFrequency()))
 
-		// For LAN addresses, increase the priority so that we
-		// try these first.
-		switch {
-		case dialerFactory.AlwaysWAN():
-			// Do nothing.
-		case s.isLANHost(uri.Host):
-			priority--
-		}
-
 		dialTargets = append(dialTargets, dialTarget{
 			addr:     addr,
 			dialer:   dialer,
@@ -703,7 +701,11 @@ func (s *service) resolveDeviceAddrs(ctx context.Context, cfg config.DeviceConfi
 	return util.UniqueTrimmedStrings(addrs)
 }
 
-func (s *service) isLANHost(host string) bool {
+type lanChecker struct {
+	cfg config.Wrapper
+}
+
+func (s *lanChecker) isLANHost(host string) bool {
 	// Probably we are called with an ip:port combo which we can resolve as
 	// a TCP address.
 	if addr, err := net.ResolveTCPAddr("tcp", host); err == nil {
@@ -717,7 +719,7 @@ func (s *service) isLANHost(host string) bool {
 	return false
 }
 
-func (s *service) isLAN(addr net.Addr) bool {
+func (s *lanChecker) isLAN(addr net.Addr) bool {
 	var ip net.IP
 
 	switch addr := addr.(type) {
@@ -763,7 +765,7 @@ func (s *service) createListener(factory listenerFactory, uri *url.URL) bool {
 
 	l.Debugln("Starting listener", uri)
 
-	listener := factory.New(uri, s.cfg, s.tlsCfg, s.conns, s.natService, s.registry)
+	listener := factory.New(uri, s.cfg, s.tlsCfg, s.conns, s.natService, s.registry, s.lanChecker)
 	listener.OnAddressesChanged(s.logListenAddressesChangedEvent)
 
 	// Retrying a listener many times in rapid succession is unlikely to help,

lib/connections/structs.go

@@ -87,10 +87,11 @@ func (t connType) Transport() string {
 	}
 }
 
-func newInternalConn(tc tlsConn, connType connType, priority int) internalConn {
+func newInternalConn(tc tlsConn, connType connType, isLocal bool, priority int) internalConn {
 	return internalConn{
 		tlsConn:       tc,
 		connType:      connType,
+		isLocal:       isLocal,
 		priority:      priority,
 		establishedAt: time.Now().Truncate(time.Second),
 	}
@@ -138,12 +139,15 @@ func (c internalConn) EstablishedAt() time.Time {
 }
 
 func (c internalConn) String() string {
-	return fmt.Sprintf("%s-%s/%s/%s", c.LocalAddr(), c.RemoteAddr(), c.Type(), c.Crypto())
+	t := "WAN"
+	if c.isLocal {
+		t = "LAN"
+	}
+	return fmt.Sprintf("%s-%s/%s/%s/%s-P%d", c.LocalAddr(), c.RemoteAddr(), c.Type(), c.Crypto(), t, c.Priority())
 }
 
 type dialerFactory interface {
-	New(config.OptionsConfiguration, *tls.Config, *registry.Registry) genericDialer
-	Priority() int
+	New(config.OptionsConfiguration, *tls.Config, *registry.Registry, *lanChecker) genericDialer
 	AlwaysWAN() bool
 	Valid(config.Configuration) error
 	String() string
@@ -153,19 +157,30 @@ type commonDialer struct {
 	trafficClass      int
 	reconnectInterval time.Duration
 	tlsCfg            *tls.Config
+	lanChecker        *lanChecker
+	lanPriority       int
+	wanPriority       int
 }
 
 func (d *commonDialer) RedialFrequency() time.Duration {
 	return d.reconnectInterval
 }
 
+func (d *commonDialer) Priority(host string) int {
+	if d.lanChecker.isLANHost(host) {
+		return d.lanPriority
+	}
+	return d.wanPriority
+}
+
 type genericDialer interface {
 	Dial(context.Context, protocol.DeviceID, *url.URL) (internalConn, error)
 	RedialFrequency() time.Duration
+	Priority(host string) int
 }
 
 type listenerFactory interface {
-	New(*url.URL, config.Wrapper, *tls.Config, chan internalConn, *nat.Service, *registry.Registry) genericListener
+	New(*url.URL, config.Wrapper, *tls.Config, chan internalConn, *nat.Service, *registry.Registry, *lanChecker) genericListener
 	Valid(config.Configuration) error
 }
 

lib/connections/tcp_dial.go

@@ -18,8 +18,6 @@ import (
 	"github.com/syncthing/syncthing/lib/protocol"
 )
 
-const tcpPriority = 10
-
 func init() {
 	factory := &tcpDialerFactory{}
 	for _, scheme := range []string{"tcp", "tcp4", "tcp6"} {
@@ -59,26 +57,30 @@ func (d *tcpDialer) Dial(ctx context.Context, _ protocol.DeviceID, uri *url.URL)
 		return internalConn{}, err
 	}
 
-	return newInternalConn(tc, connTypeTCPClient, tcpPriority), nil
+	priority := d.wanPriority
+	isLocal := d.lanChecker.isLAN(conn.RemoteAddr())
+	if isLocal {
+		priority = d.lanPriority
+	}
+	return newInternalConn(tc, connTypeTCPClient, isLocal, priority), nil
 }
 
 type tcpDialerFactory struct{}
 
-func (tcpDialerFactory) New(opts config.OptionsConfiguration, tlsCfg *tls.Config, registry *registry.Registry) genericDialer {
+func (tcpDialerFactory) New(opts config.OptionsConfiguration, tlsCfg *tls.Config, registry *registry.Registry, lanChecker *lanChecker) genericDialer {
 	return &tcpDialer{
 		commonDialer: commonDialer{
 			trafficClass:      opts.TrafficClass,
 			reconnectInterval: time.Duration(opts.ReconnectIntervalS) * time.Second,
 			tlsCfg:            tlsCfg,
+			lanPriority:       opts.ConnectionPriorityTCPLAN,
+			wanPriority:       opts.ConnectionPriorityTCPWAN,
+			lanChecker:        lanChecker,
 		},
 		registry: registry,
 	}
 }
 
-func (tcpDialerFactory) Priority() int {
-	return tcpPriority
-}
-
 func (tcpDialerFactory) AlwaysWAN() bool {
 	return false
 }

lib/connections/tcp_listen.go

@@ -32,12 +32,13 @@ type tcpListener struct {
 	svcutil.ServiceWithError
 	onAddressesChangedNotifier
 
-	uri      *url.URL
-	cfg      config.Wrapper
-	tlsCfg   *tls.Config
-	conns    chan internalConn
-	factory  listenerFactory
-	registry *registry.Registry
+	uri        *url.URL
+	cfg        config.Wrapper
+	tlsCfg     *tls.Config
+	conns      chan internalConn
+	factory    listenerFactory
+	registry   *registry.Registry
+	lanChecker *lanChecker
 
 	natService *nat.Service
 	mapping    *nat.Mapping
@@ -148,7 +149,12 @@ func (t *tcpListener) serve(ctx context.Context) error {
 			continue
 		}
 
-		t.conns <- newInternalConn(tc, connTypeTCPServer, tcpPriority)
+		priority := t.cfg.Options().ConnectionPriorityTCPWAN
+		isLocal := t.lanChecker.isLAN(conn.RemoteAddr())
+		if isLocal {
+			priority = t.cfg.Options().ConnectionPriorityTCPLAN
+		}
+		t.conns <- newInternalConn(tc, connTypeTCPServer, isLocal, priority)
 	}
 }
 
@@ -214,7 +220,7 @@ func (t *tcpListener) NATType() string {
 
 type tcpListenerFactory struct{}
 
-func (f *tcpListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.Config, conns chan internalConn, natService *nat.Service, registry *registry.Registry) genericListener {
+func (f *tcpListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.Config, conns chan internalConn, natService *nat.Service, registry *registry.Registry, lanChecker *lanChecker) genericListener {
 	l := &tcpListener{
 		uri:        fixupPort(uri, config.DefaultTCPPort),
 		cfg:        cfg,
@@ -223,6 +229,7 @@ func (f *tcpListenerFactory) New(uri *url.URL, cfg config.Wrapper, tlsCfg *tls.C
 		natService: natService,
 		factory:    f,
 		registry:   registry,
+		lanChecker: lanChecker,
 	}
 	l.ServiceWithError = svcutil.AsService(l.serve, l.String())
 	return l