Merge pull request #1910 from weaveworks/1909-forEachRoute-panic

cope with errors during route traversal

net/if.go

@@ -52,16 +52,25 @@ func EnsureInterfaceAndMcastRoute(ifaceName string) (*net.Interface, error) {
 		return nil, err
 	}
 	dest := net.IPv4(224, 0, 0, 0)
-	// check for currently-existing route after subscribing, to avoid race
-	if CheckRouteExists(ifaceName, dest) {
-		return iface, err
+	check := func(route netlink.Route) bool {
+		return route.LinkIndex == iface.Index && route.Dst.IP.Equal(dest)
 	}
-	for update := range ch {
-		link, _ := netlink.LinkByIndex(update.Route.LinkIndex)
-		if link.Attrs().Name == ifaceName && update.Route.Dst.IP.Equal(dest) {
-			break
+	// check for currently-existing route after subscribing, to avoid race
+	routes, err := netlink.RouteList(nil, netlink.FAMILY_V4)
+	if err != nil {
+		return nil, err
+	}
+	for _, route := range routes {
+		if check(route) {
+			return iface, nil
 		}
 	}
+	for update := range ch {
+		if check(update.Route) {
+			return iface, nil
+		}
+	}
+	// should never get here
 	return iface, nil
 }
 

net/route.go

@@ -10,7 +10,7 @@ import (
 // A network is considered free if it does not overlap any existing
 // routes on this host. This is the same approach taken by Docker.
 func CheckNetworkFree(subnet *net.IPNet, ignoreIfaceNames map[string]struct{}) error {
-	return forEachRoute(ignoreIfaceNames, func(name string, route netlink.Route) error {
+	return forEachRoute(ignoreIfaceNames, func(route netlink.Route) error {
 		if route.Dst != nil && overlaps(route.Dst, subnet) {
 			return fmt.Errorf("Network %s overlaps with existing route %s on host.", subnet, route.Dst)
 		}
@@ -26,7 +26,7 @@ func overlaps(n1, n2 *net.IPNet) bool {
 // For a specific address, we only care if it is actually *inside* an
 // existing route, because weave-local traffic never hits IP routing.
 func CheckAddressOverlap(addr net.IP, ignoreIfaceNames map[string]struct{}) error {
-	return forEachRoute(ignoreIfaceNames, func(name string, route netlink.Route) error {
+	return forEachRoute(ignoreIfaceNames, func(route netlink.Route) error {
 		if route.Dst != nil && route.Dst.Contains(addr) {
 			return fmt.Errorf("Address %s overlaps with existing route %s on host.", addr, route.Dst)
 		}
@@ -34,32 +34,24 @@ func CheckAddressOverlap(addr net.IP, ignoreIfaceNames map[string]struct{}) erro
 	})
 }
 
-func forEachRoute(ignoreIfaceNames map[string]struct{}, check func(name string, r netlink.Route) error) error {
+func forEachRoute(ignoreIfaceNames map[string]struct{}, check func(r netlink.Route) error) error {
+	ignoreIfaceIndices := make(map[int]struct{})
+	for ifaceName := range ignoreIfaceNames {
+		if iface, err := net.InterfaceByName(ifaceName); err == nil {
+			ignoreIfaceIndices[iface.Index] = struct{}{}
+		}
+	}
 	routes, err := netlink.RouteList(nil, netlink.FAMILY_V4)
 	if err != nil {
 		return err
 	}
 	for _, route := range routes {
-		link, err := netlink.LinkByIndex(route.LinkIndex)
-		if err == nil {
-			if _, found := ignoreIfaceNames[link.Attrs().Name]; found {
-				continue
-			}
+		if _, found := ignoreIfaceIndices[route.LinkIndex]; found {
+			continue
 		}
-		if err := check(link.Attrs().Name, route); err != nil {
+		if err := check(route); err != nil {
 			return err
 		}
 	}
 	return nil
 }
-
-func CheckRouteExists(ifaceName string, dest net.IP) bool {
-	found := false
-	forEachRoute(map[string]struct{}{}, func(name string, route netlink.Route) error {
-		if name == ifaceName && route.Dst.IP.Equal(dest) {
-			found = true
-		}
-		return nil
-	})
-	return found
-}