Remove mutexes from CKR and use router goroutine/doAdmin for update config

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 {

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,

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
 		}
 	}