Simplify reconfiguration

src/multicast/multicast.go

@@ -83,7 +83,6 @@ func (m *Multicast) Stop() error {
 func (m *Multicast) UpdateConfig(config *config.NodeConfig) {
 	m.log.Debugln("Reloading multicast configuration...")
 	m.config.Replace(*config)
-	m.log.Infoln("Multicast configuration reloaded successfully")
 }
 
 // GetInterfaces returns the currently known/enabled multicast interfaces. It is

src/tuntap/ckr.go

@@ -20,7 +20,6 @@ import (
 type cryptokey struct {
 	tun          *TunAdapter
 	enabled      atomic.Value // bool
-	reconfigure  chan chan error
 	ipv4remotes  []cryptokey_route
 	ipv6remotes  []cryptokey_route
 	ipv4cache    map[address.Address]cryptokey_route
@@ -40,24 +39,11 @@ type cryptokey_route struct {
 // Initialise crypto-key routing. This must be done before any other CKR calls.
 func (c *cryptokey) init(tun *TunAdapter) {
 	c.tun = tun
-	c.reconfigure = make(chan chan error, 1)
-	go func() {
-		for {
-			e := <-c.reconfigure
-			e <- nil
-		}
-	}()
-
-	c.tun.log.Debugln("Configuring CKR...")
-	if err := c.configure(); err != nil {
-		c.tun.log.Errorln("CKR configuration failed:", err)
-	} else {
-		c.tun.log.Debugln("CKR configured")
-	}
+	c.configure()
 }
 
-// Configure the CKR routes.
-func (c *cryptokey) configure() error {
+// Configure the CKR routes. This should only ever be ran by the TUN/TAP actor.
+func (c *cryptokey) configure() {
 	current := c.tun.config.GetCurrent()
 
 	// Set enabled/disabled state
@@ -72,14 +58,14 @@ func (c *cryptokey) configure() error {
 	// Add IPv6 routes
 	for ipv6, pubkey := range current.TunnelRouting.IPv6RemoteSubnets {
 		if err := c.addRemoteSubnet(ipv6, pubkey); err != nil {
-			return err
+			c.tun.log.Errorln("Error adding CKR IPv6 remote subnet:", err)
 		}
 	}
 
 	// Add IPv4 routes
 	for ipv4, pubkey := range current.TunnelRouting.IPv4RemoteSubnets {
 		if err := c.addRemoteSubnet(ipv4, pubkey); err != nil {
-			return err
+			c.tun.log.Errorln("Error adding CKR IPv4 remote subnet:", err)
 		}
 	}
 
@@ -93,7 +79,7 @@ func (c *cryptokey) configure() error {
 	c.ipv6locals = make([]net.IPNet, 0)
 	for _, source := range current.TunnelRouting.IPv6LocalSubnets {
 		if err := c.addLocalSubnet(source); err != nil {
-			return err
+			c.tun.log.Errorln("Error adding CKR IPv6 local subnet:", err)
 		}
 	}
 
@@ -101,7 +87,7 @@ func (c *cryptokey) configure() error {
 	c.ipv4locals = make([]net.IPNet, 0)
 	for _, source := range current.TunnelRouting.IPv4LocalSubnets {
 		if err := c.addLocalSubnet(source); err != nil {
-			return err
+			c.tun.log.Errorln("Error adding CKR IPv4 local subnet:", err)
 		}
 	}
 
@@ -110,8 +96,6 @@ func (c *cryptokey) configure() error {
 	c.ipv4cache = make(map[address.Address]cryptokey_route, 0)
 	c.ipv6cache = make(map[address.Address]cryptokey_route, 0)
 	c.mutexcaches.Unlock()
-
-	return nil
 }
 
 // Enable or disable crypto-key routing.
@@ -181,19 +165,19 @@ func (c *cryptokey) addLocalSubnet(cidr string) error {
 	} else if prefixsize == net.IPv4len*8 {
 		routingsources = &c.ipv4locals
 	} else {
-		return errors.New("Unexpected prefix size")
+		return errors.New("unexpected prefix size")
 	}
 
 	// Check if we already have this CIDR
 	for _, subnet := range *routingsources {
 		if subnet.String() == ipnet.String() {
-			return errors.New("Source subnet already configured")
+			return errors.New("local subnet already configured")
 		}
 	}
 
 	// Add the source subnet
 	*routingsources = append(*routingsources, *ipnet)
-	c.tun.log.Infoln("Added CKR source subnet", cidr)
+	c.tun.log.Infoln("Added CKR local subnet", cidr)
 	return nil
 }
 
@@ -226,7 +210,7 @@ func (c *cryptokey) addRemoteSubnet(cidr string, dest string) error {
 		routingtable = &c.ipv4remotes
 		routingcache = &c.ipv4cache
 	} else {
-		return errors.New("Unexpected prefix size")
+		return errors.New("unexpected prefix size")
 	}
 
 	// Is the route an Yggdrasil destination?
@@ -235,19 +219,19 @@ func (c *cryptokey) addRemoteSubnet(cidr string, dest string) error {
 	copy(addr[:], ipaddr)
 	copy(snet[:], ipnet.IP)
 	if addr.IsValid() || snet.IsValid() {
-		return errors.New("Can't specify Yggdrasil destination as crypto-key route")
+		return errors.New("can't specify Yggdrasil destination as crypto-key route")
 	}
 	// Do we already have a route for this subnet?
 	for _, route := range *routingtable {
 		if route.subnet.String() == ipnet.String() {
-			return errors.New(fmt.Sprintf("Route already exists for %s", cidr))
+			return fmt.Errorf("remote subnet already exists for %s", cidr)
 		}
 	}
 	// Decode the public key
 	if bpk, err := hex.DecodeString(dest); err != nil {
 		return err
 	} else if len(bpk) != crypto.BoxPubKeyLen {
-		return errors.New(fmt.Sprintf("Incorrect key length for %s", dest))
+		return fmt.Errorf("incorrect key length for %s", dest)
 	} else {
 		// Add the new crypto-key route
 		var key crypto.BoxPubKey
@@ -270,7 +254,7 @@ func (c *cryptokey) addRemoteSubnet(cidr string, dest string) error {
 			delete(*routingcache, k)
 		}
 
-		c.tun.log.Infoln("Added CKR destination subnet", cidr)
+		c.tun.log.Infoln("Added CKR remote subnet", cidr)
 		return nil
 	}
 }
@@ -284,7 +268,7 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c
 	// Check if the address is a valid Yggdrasil address - if so it
 	// is exempt from all CKR checking
 	if addr.IsValid() {
-		return crypto.BoxPubKey{}, errors.New("Cannot look up CKR for Yggdrasil addresses")
+		return crypto.BoxPubKey{}, errors.New("cannot look up CKR for Yggdrasil addresses")
 	}
 
 	// Build our references to the routing table and cache
@@ -297,7 +281,7 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c
 	} else if addrlen == net.IPv4len {
 		routingcache = &c.ipv4cache
 	} else {
-		return crypto.BoxPubKey{}, errors.New("Unexpected prefix size")
+		return crypto.BoxPubKey{}, errors.New("unexpected prefix size")
 	}
 
 	// Check if there's a cache entry for this addr
@@ -317,7 +301,7 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c
 	} else if addrlen == net.IPv4len {
 		routingtable = &c.ipv4remotes
 	} else {
-		return crypto.BoxPubKey{}, errors.New("Unexpected prefix size")
+		return crypto.BoxPubKey{}, errors.New("unexpected prefix size")
 	}
 
 	// No cache was found - start by converting the address into a net.IP
@@ -378,18 +362,18 @@ func (c *cryptokey) removeLocalSubnet(cidr string) error {
 	} else if prefixsize == net.IPv4len*8 {
 		routingsources = &c.ipv4locals
 	} else {
-		return errors.New("Unexpected prefix size")
+		return errors.New("unexpected prefix size")
 	}
 
 	// Check if we already have this CIDR
 	for idx, subnet := range *routingsources {
 		if subnet.String() == ipnet.String() {
 			*routingsources = append((*routingsources)[:idx], (*routingsources)[idx+1:]...)
-			c.tun.log.Infoln("Removed CKR source subnet", cidr)
+			c.tun.log.Infoln("Removed CKR local subnet", cidr)
 			return nil
 		}
 	}
-	return errors.New("Source subnet not found")
+	return errors.New("local subnet not found")
 }
 
 // Removes a destination route for the given CIDR to be tunnelled to the node
@@ -421,7 +405,7 @@ func (c *cryptokey) removeRemoteSubnet(cidr string, dest string) error {
 		routingtable = &c.ipv4remotes
 		routingcache = &c.ipv4cache
 	} else {
-		return errors.New("Unexpected prefix size")
+		return errors.New("unexpected prefix size")
 	}
 
 	// Decode the public key
@@ -429,7 +413,7 @@ func (c *cryptokey) removeRemoteSubnet(cidr string, dest string) error {
 	if err != nil {
 		return err
 	} else if len(bpk) != crypto.BoxPubKeyLen {
-		return errors.New(fmt.Sprintf("Incorrect key length for %s", dest))
+		return fmt.Errorf("incorrect key length for %s", dest)
 	}
 	netStr := ipnet.String()
 
@@ -439,9 +423,9 @@ func (c *cryptokey) removeRemoteSubnet(cidr string, dest string) error {
 			for k := range *routingcache {
 				delete(*routingcache, k)
 			}
-			c.tun.log.Infof("Removed CKR destination subnet %s via %s\n", cidr, dest)
+			c.tun.log.Infof("Removed CKR remote subnet %s via %s\n", cidr, dest)
 			return nil
 		}
 	}
-	return errors.New(fmt.Sprintf("Route does not exists for %s", cidr))
+	return fmt.Errorf("route does not exists for %s", cidr)
 }

src/tuntap/tun.go

@@ -13,6 +13,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+
 	//"sync"
 
 	"github.com/Arceliar/phony"
@@ -200,29 +201,11 @@ func (tun *TunAdapter) _stop() error {
 func (tun *TunAdapter) UpdateConfig(config *config.NodeConfig) {
 	tun.log.Debugln("Reloading TUN/TAP configuration...")
 
+	// Replace the active configuration with the supplied one
 	tun.config.Replace(*config)
 
-	errors := 0
-
-	components := []chan chan error{
-		tun.reconfigure,
-		tun.ckr.reconfigure,
-	}
-
-	for _, component := range components {
-		response := make(chan error)
-		component <- response
-		if err := <-response; err != nil {
-			tun.log.Errorln(err)
-			errors++
-		}
-	}
-
-	if errors > 0 {
-		tun.log.Warnln(errors, "TUN/TAP module(s) reported errors during configuration reload")
-	} else {
-		tun.log.Infoln("TUN/TAP configuration reloaded successfully")
-	}
+	// Notify children about the configuration change
+	tun.Act(nil, tun.ckr.configure)
 }
 
 func (tun *TunAdapter) handler() error {

src/yggdrasil/core.go

@@ -20,6 +20,7 @@ type Core struct {
 	// This is the main data structure that holds everything else for a node
 	// We're going to keep our own copy of the provided config - that way we can
 	// guarantee that it will be covered by the mutex
+	phony.Inbox
 	config      config.NodeState // Config
 	boxPub      crypto.BoxPubKey
 	boxPriv     crypto.BoxPrivKey
@@ -112,47 +113,14 @@ func (c *Core) addPeerLoop() {
 // config.NodeConfig and then signals the various module goroutines to
 // reconfigure themselves if needed.
 func (c *Core) UpdateConfig(config *config.NodeConfig) {
-	c.log.Infoln("Reloading node configuration...")
+	c.log.Debugln("Reloading node configuration...")
 
+	// Replace the active configuration with the supplied one
 	c.config.Replace(*config)
-	errors := 0
 
-	// Each reconfigure function should pass any errors to the channel, then close it
-	components := map[phony.Actor][]func(chan error){
-		&c.router: []func(chan error){
-			c.router.reconfigure,
-			c.router.dht.reconfigure,
-			c.router.searches.reconfigure,
-			c.router.sessions.reconfigure,
-		},
-		&c.switchTable: []func(chan error){
-			c.switchTable.reconfigure,
-			c.link.reconfigure,
-			c.peers.reconfigure,
-		},
-	}
-
-	// TODO: We count errors here but honestly that provides us with absolutely no
-	// benefit over components reporting errors themselves, so maybe we can use
-	// actor.Act() here instead and stop counting errors
-	for actor, functions := range components {
-		for _, function := range functions {
-			response := make(chan error)
-			phony.Block(actor, func() {
-				function(response)
-			})
-			for err := range response {
-				c.log.Errorln(err)
-				errors++
-			}
-		}
-	}
-
-	if errors > 0 {
-		c.log.Warnln(errors, "node module(s) reported errors during configuration reload")
-	} else {
-		c.log.Infoln("Node configuration reloaded successfully")
-	}
+	// Notify the router and switch about the new configuration
+	c.router.Act(c, c.router.reconfigure)
+	c.switchTable.Act(c, c.switchTable.reconfigure)
 }
 
 // Start starts up Yggdrasil using the provided config.NodeConfig, and outputs

src/yggdrasil/dht.go

@@ -82,8 +82,7 @@ func (t *dht) init(r *router) {
 	t.reset()
 }
 
-func (t *dht) reconfigure(e chan error) {
-	defer close(e)
+func (t *dht) reconfigure() {
 	// This is where reconfiguration would go, if we had anything to do
 }
 

src/yggdrasil/link.go

@@ -79,13 +79,8 @@ func (l *link) init(c *Core) error {
 	return nil
 }
 
-func (l *link) reconfigure(e chan error) {
-	defer close(e)
-	tcpResponse := make(chan error)
-	l.tcp.reconfigure(tcpResponse)
-	for err := range tcpResponse {
-		e <- err
-	}
+func (l *link) reconfigure() {
+	l.tcp.reconfigure()
 }
 
 func (l *link) call(uri string, sintf string) error {

src/yggdrasil/peer.go

@@ -34,8 +34,7 @@ func (ps *peers) init(c *Core) {
 	ps.core = c
 }
 
-func (ps *peers) reconfigure(e chan error) {
-	defer close(e)
+func (ps *peers) reconfigure() {
 	// This is where reconfiguration would go, if we had anything to do
 }
 

src/yggdrasil/router.go

@@ -73,18 +73,20 @@ func (r *router) init(core *Core) {
 	r.sessions.init(r)
 }
 
-func (r *router) reconfigure(e chan error) {
-	defer close(e)
-	var errs []error
+// Reconfigures the router and any child modules. This should only ever be run
+// by the router actor.
+func (r *router) reconfigure() {
 	// Reconfigure the router
 	current := r.core.config.GetCurrent()
-	err := r.nodeinfo.setNodeInfo(current.NodeInfo, current.NodeInfoPrivacy)
-	if err != nil {
-		errs = append(errs, err)
-	}
-	for _, err := range errs {
-		e <- err
+	if err := r.nodeinfo.setNodeInfo(current.NodeInfo, current.NodeInfoPrivacy); err != nil {
+		r.core.log.Errorln("Error reloading NodeInfo:", err)
+	} else {
+		r.core.log.Infoln("NodeInfo updated")
 	}
+	// Reconfigure children
+	r.dht.reconfigure()
+	r.searches.reconfigure()
+	r.sessions.reconfigure()
 }
 
 // Starts the tickerLoop goroutine.

src/yggdrasil/search.go

@@ -55,8 +55,7 @@ func (s *searches) init(r *router) {
 	s.searches = make(map[crypto.NodeID]*searchInfo)
 }
 
-func (s *searches) reconfigure(e chan error) {
-	defer close(e)
+func (s *searches) reconfigure() {
 	// This is where reconfiguration would go, if we had anything to do
 }
 

src/yggdrasil/session.go

@@ -73,8 +73,7 @@ type sessionInfo struct {
 	callbacks      []chan func()                 // Finished work from crypto workers
 }
 
-func (sinfo *sessionInfo) reconfigure(e chan error) {
-	defer close(e)
+func (sinfo *sessionInfo) reconfigure() {
 	// This is where reconfiguration would go, if we had anything to do
 }
 
@@ -161,17 +160,9 @@ func (ss *sessions) init(r *router) {
 	ss.lastCleanup = time.Now()
 }
 
-func (ss *sessions) reconfigure(e chan error) {
-	defer close(e)
-	responses := make(map[crypto.Handle]chan error)
-	for index, session := range ss.sinfos {
-		responses[index] = make(chan error)
-		session.reconfigure(responses[index])
-	}
-	for _, response := range responses {
-		for err := range response {
-			e <- err
-		}
+func (ss *sessions) reconfigure() {
+	for _, session := range ss.sinfos {
+		session.reconfigure()
 	}
 }
 

src/yggdrasil/switch.go

@@ -199,9 +199,10 @@ func (t *switchTable) init(core *Core) {
 	})
 }
 
-func (t *switchTable) reconfigure(e chan error) {
-	defer close(e)
+func (t *switchTable) reconfigure() {
 	// This is where reconfiguration would go, if we had anything useful to do.
+	t.core.link.reconfigure()
+	t.core.peers.reconfigure()
 }
 
 // Safely gets a copy of this node's locator.

src/yggdrasil/tcp.go

@@ -95,8 +95,7 @@ func (t *tcp) init(l *link) error {
 	return nil
 }
 
-func (t *tcp) reconfigure(e chan error) {
-	defer close(e)
+func (t *tcp) reconfigure() {
 	t.link.core.config.Mutex.RLock()
 	added := util.Difference(t.link.core.config.Current.Listen, t.link.core.config.Previous.Listen)
 	deleted := util.Difference(t.link.core.config.Previous.Listen, t.link.core.config.Current.Listen)
@@ -107,7 +106,9 @@ func (t *tcp) reconfigure(e chan error) {
 				continue
 			}
 			if _, err := t.listen(a[6:]); err != nil {
-				e <- err
+				t.link.core.log.Errorln("Error adding TCP", a[6:], "listener:", err)
+			} else {
+				t.link.core.log.Infoln("Started TCP listener:", a[6:])
 			}
 		}
 		for _, d := range deleted {
@@ -118,6 +119,7 @@ func (t *tcp) reconfigure(e chan error) {
 			if listener, ok := t.listeners[d[6:]]; ok {
 				t.mutex.Unlock()
 				listener.Stop <- true
+				t.link.core.log.Infoln("Stopped TCP listener:", d[6:])
 			} else {
 				t.mutex.Unlock()
 			}