From 9e186bdd6730d6c55c61858da429e9bf5bca410e Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Mon, 14 Jan 2019 18:34:15 +0000 Subject: [PATCH] Remove mutexes from CKR and use router goroutine/doAdmin for update config --- src/yggdrasil/ckr.go | 64 +++++++++++------------------------------ src/yggdrasil/core.go | 1 + src/yggdrasil/router.go | 8 ------ 3 files changed, 17 insertions(+), 56 deletions(-) diff --git a/src/yggdrasil/ckr.go b/src/yggdrasil/ckr.go index bf569fbb..14464f64 100644 --- a/src/yggdrasil/ckr.go +++ b/src/yggdrasil/ckr.go @@ -7,7 +7,6 @@ import ( "fmt" "net" "sort" - "sync" "github.com/yggdrasil-network/yggdrasil-go/src/address" "github.com/yggdrasil-network/yggdrasil-go/src/crypto" @@ -17,19 +16,15 @@ import ( // allow traffic for non-Yggdrasil ranges to be routed over Yggdrasil. type cryptokey struct { - core *Core - enabled bool - reconfigure chan chan error - ipv4routes []cryptokey_route - ipv6routes []cryptokey_route - ipv4cache map[address.Address]cryptokey_route - ipv6cache map[address.Address]cryptokey_route - ipv4sources []net.IPNet - ipv6sources []net.IPNet - mutexenabled sync.RWMutex // protects enabled - mutexroutes sync.RWMutex // protects ipv4routes, ipv6routes - mutexcache sync.RWMutex // protects ipv4cache, ipv6cache - mutexsources sync.RWMutex // protects ipv4sources, ipv6sources + core *Core + enabled bool + reconfigure chan chan error + ipv4routes []cryptokey_route + ipv6routes []cryptokey_route + ipv4cache map[address.Address]cryptokey_route + ipv6cache map[address.Address]cryptokey_route + ipv4sources []net.IPNet + ipv6sources []net.IPNet } type cryptokey_route struct { @@ -45,7 +40,11 @@ func (c *cryptokey) init(core *Core) { for { select { case e := <-c.reconfigure: - e <- c.configure() + var err error + c.core.router.doAdmin(func() { + err = c.core.router.cryptokey.configure() + }) + e <- err } } }() @@ -55,7 +54,8 @@ func (c *cryptokey) init(core *Core) { } } -// Configure the CKR routes +// Configure the CKR routes - this must only ever be called from the router +// goroutine, e.g. through router.doAdmin func (c *cryptokey) configure() error { c.core.configMutex.RLock() defer c.core.configMutex.RUnlock() @@ -64,10 +64,8 @@ func (c *cryptokey) configure() error { c.setEnabled(c.core.config.TunnelRouting.Enable) // Clear out existing routes - c.mutexroutes.Lock() c.ipv6routes = make([]cryptokey_route, 0) c.ipv4routes = make([]cryptokey_route, 0) - c.mutexroutes.Unlock() // Add IPv6 routes for ipv6, pubkey := range c.core.config.TunnelRouting.IPv6Destinations { @@ -84,10 +82,8 @@ func (c *cryptokey) configure() error { } // Clear out existing sources - c.mutexsources.Lock() c.ipv6sources = make([]net.IPNet, 0) c.ipv4sources = make([]net.IPNet, 0) - c.mutexsources.Unlock() // Add IPv6 sources c.ipv6sources = make([]net.IPNet, 0) @@ -106,25 +102,19 @@ func (c *cryptokey) configure() error { } // Wipe the caches - c.mutexcache.Lock() c.ipv4cache = make(map[address.Address]cryptokey_route, 0) c.ipv6cache = make(map[address.Address]cryptokey_route, 0) - c.mutexcache.Unlock() return nil } // Enable or disable crypto-key routing. func (c *cryptokey) setEnabled(enabled bool) { - c.mutexenabled.Lock() - defer c.mutexenabled.Unlock() c.enabled = enabled } // Check if crypto-key routing is enabled. func (c *cryptokey) isEnabled() bool { - c.mutexenabled.RLock() - defer c.mutexenabled.RUnlock() return c.enabled } @@ -148,9 +138,6 @@ func (c *cryptokey) isValidSource(addr address.Address, addrlen int) bool { // Does it match a configured CKR source? if c.isEnabled() { - c.mutexsources.RLock() - defer c.mutexsources.RUnlock() - // Build our references to the routing sources var routingsources *[]net.IPNet @@ -177,9 +164,6 @@ func (c *cryptokey) isValidSource(addr address.Address, addrlen int) bool { // Adds a source subnet, which allows traffic with these source addresses to // be tunnelled using crypto-key routing. func (c *cryptokey) addSourceSubnet(cidr string) error { - c.mutexsources.Lock() - defer c.mutexsources.Unlock() - // Is the CIDR we've been given valid? _, ipnet, err := net.ParseCIDR(cidr) if err != nil { @@ -217,9 +201,6 @@ func (c *cryptokey) addSourceSubnet(cidr string) error { // Adds a destination route for the given CIDR to be tunnelled to the node // with the given BoxPubKey. func (c *cryptokey) addRoute(cidr string, dest string) error { - c.mutexroutes.Lock() - defer c.mutexroutes.Unlock() - // Is the CIDR we've been given valid? ipaddr, ipnet, err := net.ParseCIDR(cidr) if err != nil { @@ -294,11 +275,6 @@ func (c *cryptokey) addRoute(cidr string, dest string) error { // length specified in bytes) from the crypto-key routing table. An error is // returned if the address is not suitable or no route was found. func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (crypto.BoxPubKey, error) { - c.mutexroutes.RLock() - c.mutexcache.RLock() - defer c.mutexroutes.RUnlock() - defer c.mutexcache.RUnlock() - // Check if the address is a valid Yggdrasil address - if so it // is exempt from all CKR checking if addr.IsValid() { @@ -359,9 +335,6 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c // Removes a source subnet, which allows traffic with these source addresses to // be tunnelled using crypto-key routing. func (c *cryptokey) removeSourceSubnet(cidr string) error { - c.mutexsources.Lock() - defer c.mutexsources.Unlock() - // Is the CIDR we've been given valid? _, ipnet, err := net.ParseCIDR(cidr) if err != nil { @@ -397,11 +370,6 @@ func (c *cryptokey) removeSourceSubnet(cidr string) error { // Removes a destination route for the given CIDR to be tunnelled to the node // with the given BoxPubKey. func (c *cryptokey) removeRoute(cidr string, dest string) error { - c.mutexroutes.Lock() - c.mutexcache.Lock() - defer c.mutexroutes.Unlock() - defer c.mutexcache.Unlock() - // Is the CIDR we've been given valid? _, ipnet, err := net.ParseCIDR(cidr) if err != nil { diff --git a/src/yggdrasil/core.go b/src/yggdrasil/core.go index 4b00fc37..c6d6f4a6 100644 --- a/src/yggdrasil/core.go +++ b/src/yggdrasil/core.go @@ -141,6 +141,7 @@ func (c *Core) UpdateConfig(config *config.NodeConfig) { c.peers.reconfigure, c.router.reconfigure, c.router.tun.reconfigure, + c.router.cryptokey.reconfigure, c.switchTable.reconfigure, c.tcp.reconfigure, c.multicast.reconfigure, diff --git a/src/yggdrasil/router.go b/src/yggdrasil/router.go index 74fff3fd..68fb025a 100644 --- a/src/yggdrasil/router.go +++ b/src/yggdrasil/router.go @@ -127,14 +127,6 @@ func (r *router) mainLoop() { case f := <-r.admin: f() case e := <-r.reconfigure: - // Send reconfigure notification to cryptokey - response := make(chan error) - r.cryptokey.reconfigure <- response - if err := <-response; err != nil { - e <- err - } - - // Anything else to do? e <- nil } }