wgengine: separately dedupe wireguard configs and router configs.

Otherwise iOS/macOS will reconfigure their routing every time anything
minor changes in the netmap (in particular, endpoints and DERP homes),
which is way too often.

Some users reported "network reconfigured" errors from Chrome when this
happens.

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
reviewable/pr427/r1
Avery Pennarun 2020-05-31 02:37:58 -04:00
parent f0b6ba78e8
commit e46238a2af
1 changed files with 35 additions and 26 deletions

View File

@ -57,9 +57,10 @@ type userspaceEngine struct {
magicConn *magicsock.Conn magicConn *magicsock.Conn
linkMon *monitor.Mon linkMon *monitor.Mon
wgLock sync.Mutex // serializes all wgdev operations; see lock order comment below wgLock sync.Mutex // serializes all wgdev operations; see lock order comment below
lastReconfig string lastEngineSig string
lastCfg wgcfg.Config lastRouterSig string
lastCfg wgcfg.Config
mu sync.Mutex // guards following; see lock order comment below mu sync.Mutex // guards following; see lock order comment below
closing bool // Close was called (even if we're still closing) closing bool // Close was called (even if we're still closing)
@ -359,17 +360,18 @@ func (e *userspaceEngine) pinger(peerKey wgcfg.Key, ips []wgcfg.IP) {
p.run(ctx, peerKey, ips, srcIP) p.run(ctx, peerKey, ips, srcIP)
} }
func configSignature(cfg *wgcfg.Config, routerCfg *router.Config) (string, error) { func configSignatures(cfg *wgcfg.Config, routerCfg *router.Config) (string, string, error) {
// TODO(apenwarr): get rid of uapi stuff for in-process comms // TODO(apenwarr): get rid of uapi stuff for in-process comms
uapi, err := cfg.ToUAPI() uapi, err := cfg.ToUAPI()
if err != nil { if err != nil {
return "", err return "", "", err
} }
return fmt.Sprintf("%s %v", uapi, routerCfg), nil return uapi, fmt.Sprintf("%v", routerCfg), nil
} }
func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config) error { func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config) error {
e.logf("Reconfig: router.Set: %p %p", cfg, routerCfg)
if routerCfg == nil { if routerCfg == nil {
panic("routerCfg must not be nil") panic("routerCfg must not be nil")
} }
@ -386,35 +388,42 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
} }
e.mu.Unlock() e.mu.Unlock()
rc, err := configSignature(cfg, routerCfg) ec, rc, err := configSignatures(cfg, routerCfg)
if err != nil { if err != nil {
return err return err
} }
if rc == e.lastReconfig { engineChanged := ec != e.lastEngineSig
routerChanged := rc != e.lastRouterSig
if !engineChanged && !routerChanged {
return ErrNoChanges return ErrNoChanges
} }
e.lastEngineSig = ec
e.logf("wgengine: Reconfig: configuring userspace wireguard engine") e.lastRouterSig = rc
e.lastReconfig = rc
e.lastCfg = cfg.Copy() e.lastCfg = cfg.Copy()
// Tell magicsock about the new (or initial) private key e.logf("wgengine: Reconfig: configuring userspace wireguard engine")
// (which is needed by DERP) before wgdev gets it, as wgdev
// will start trying to handshake, which we want to be able to if engineChanged {
// go over DERP. // Tell magicsock about the new (or initial) private key
if err := e.magicConn.SetPrivateKey(cfg.PrivateKey); err != nil { // (which is needed by DERP) before wgdev gets it, as wgdev
e.logf("wgengine: Reconfig: SetPrivateKey: %v", err) // will start trying to handshake, which we want to be able to
// go over DERP.
if err := e.magicConn.SetPrivateKey(cfg.PrivateKey); err != nil {
e.logf("wgengine: Reconfig: SetPrivateKey: %v", err)
}
if err := e.wgdev.Reconfig(cfg); err != nil {
e.logf("wgdev.Reconfig: %v", err)
return err
}
e.magicConn.UpdatePeers(peerSet)
} }
if err := e.wgdev.Reconfig(cfg); err != nil { if routerChanged {
e.logf("wgdev.Reconfig: %v", err) if err := e.router.Set(routerCfg); err != nil {
return err return err
} }
e.magicConn.UpdatePeers(peerSet)
if err := e.router.Set(routerCfg); err != nil {
return err
} }
e.logf("wgengine: Reconfig done") e.logf("wgengine: Reconfig done")