From 023df9239e46892d8e0e3fcadac1da736c7f9655 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 10 Mar 2020 12:25:42 -0700 Subject: [PATCH] Move linkstate boring change filtering to magicsock So we can at least re-STUN on boring updates. Signed-off-by: Brad Fitzpatrick --- net/interfaces/interfaces.go | 34 +++++++++++++++++++++++++++++++++ wgengine/magicsock/magicsock.go | 31 +++++++++++++++++++++++++++++- wgengine/monitor/monitor.go | 28 --------------------------- 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/net/interfaces/interfaces.go b/net/interfaces/interfaces.go index 0d37144d6..11804dd61 100644 --- a/net/interfaces/interfaces.go +++ b/net/interfaces/interfaces.go @@ -7,6 +7,7 @@ package interfaces import ( "net" + "reflect" "strings" ) @@ -182,3 +183,36 @@ var linkLocalIPv4 = func() *net.IPNet { } return ipNet }() + +// State is intended to store the state of the machine's network interfaces, +// routing table, and other network configuration. +// For now it's pretty basic. +type State struct { + InterfaceIPs map[string][]net.IP +} + +func (s *State) Equal(s2 *State) bool { + return reflect.DeepEqual(s, s2) +} + +// RemoveTailscaleInterfaces modifes s to remove any interfaces that +// are owned by this process. (TODO: make this true; currently it +// makes the Linux-only assumption that the interface is named +// /^tailscale/) +func (s *State) RemoveTailscaleInterfaces() { + for name := range s.InterfaceIPs { + if strings.HasPrefix(name, "tailscale") { // TODO: use --tun flag value, etc; see TODO in method doc + delete(s.InterfaceIPs, name) + } + } +} + +func GetState() (*State, error) { + s := &State{InterfaceIPs: make(map[string][]net.IP)} + if err := ForeachInterfaceAddress(func(ni Interface, ip net.IP) { + s.InterfaceIPs[ni.Name] = append(s.InterfaceIPs[ni.Name], ip) + }); err != nil { + return nil, err + } + return s, nil +} diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index de60bec9b..50160c877 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -63,6 +63,9 @@ type Conn struct { connCtx context.Context // closed on Conn.Close connCtxCancel func() // closes connCtx + linkChangeMu sync.Mutex + linkState *interfaces.State + // addrsByUDP is a map of every remote ip:port to a priority // list of endpoint addresses for a peer. // The priority list is provided by wgengine configuration. @@ -201,6 +204,7 @@ func Listen(opts Options) (*Conn, error) { derpTLSConfig: opts.derpTLSConfig, derps: derpmap.Prod(), } + c.linkState, _ = getLinkState() if len(opts.STUN) > 0 { c.derps = derpmap.NewTestWorld(opts.STUN...) } @@ -1080,9 +1084,35 @@ func (c *Conn) reSTUN() { } } +// LinkChange should be called whenever something changed with the +// network, no matter how minor. The LinkChange method then looks +// at the state of the network and decides whether the change from +// before is interesting enough to warrant taking action on. func (c *Conn) LinkChange() { defer c.reSTUN() + c.linkChangeMu.Lock() + defer c.linkChangeMu.Unlock() + + cur, err := getLinkState() + if err != nil { + return + } + if c.linkState != nil && !cur.Equal(c.linkState) { + c.linkState = cur + c.rebind() + } +} + +func getLinkState() (*interfaces.State, error) { + s, err := interfaces.GetState() + if s != nil { + s.RemoveTailscaleInterfaces() + } + return s, err +} + +func (c *Conn) rebind() { if c.pconnPort != 0 { c.pconn.mu.Lock() if err := c.pconn.pconn.Close(); err != nil { @@ -1098,7 +1128,6 @@ func (c *Conn) LinkChange() { c.logf("magicsock: link change unable to bind fixed port %d: %v, falling back to random port", c.pconnPort, err) c.pconn.mu.Unlock() } - c.logf("magicsock: link change, binding new port") packetConn, err := net.ListenPacket("udp4", ":0") if err != nil { diff --git a/wgengine/monitor/monitor.go b/wgengine/monitor/monitor.go index 2abbcd244..c644b5229 100644 --- a/wgengine/monitor/monitor.go +++ b/wgengine/monitor/monitor.go @@ -7,14 +7,9 @@ package monitor import ( - "fmt" - "net" - "runtime" - "strings" "sync" "time" - "tailscale.com/net/interfaces" "tailscale.com/types/logger" ) @@ -104,7 +99,6 @@ func (m *Mon) Close() error { // the change channel of changes, and stopping when a stop is issued. func (m *Mon) pump() { defer m.goroutines.Done() - last := interfaceSummary() for { _, err := m.om.Receive() if err != nil { @@ -118,14 +112,6 @@ func (m *Mon) pump() { time.Sleep(time.Second) continue } - - cur := interfaceSummary() - if cur == last { - continue - } - m.logf("wgengine/monitor: now %v (was %v)", cur, last) - last = cur - select { case m.change <- struct{}{}: case <-m.stop: @@ -154,17 +140,3 @@ func (m *Mon) debounce() { } } } - -func interfaceSummary() string { - var sb strings.Builder - _ = interfaces.ForeachInterfaceAddress(func(ni interfaces.Interface, addr net.IP) { - if runtime.GOOS == "linux" && strings.HasPrefix(ni.Name, "tailscale") { - // Skip tailscale0, etc on Linux. - return - } - if ni.IsUp() { - fmt.Fprintf(&sb, "%s=%s ", ni.Name, addr) - } - }) - return strings.TrimSpace(sb.String()) -}