Move setting up addresses for a container, into weaveutil-invoked fun…

…ction

This means we only change netns by execing nsenter: changing netns in
Go is unsafe for long-running processes like the proxy.

net/veth.go

@@ -104,20 +104,13 @@ func interfaceExistsInNamespace(netNSPath string, ifName string) bool {
 	return err == nil
 }
 
-// NB: This function can be used only by a process that terminates immediately
-//     after calling the function as it changes netns via WithNetNSLinkUnsafe.
 func AttachContainer(netNSPath, id, ifName, bridgeName string, mtu int, withMulticastRoute bool, cidrs []*net.IPNet, keepTXOn bool) error {
 	ns, err := netns.GetFromPath(netNSPath)
 	if err != nil {
 		return err
 	}
 	defer ns.Close()
 
-	ipt, err := iptables.New()
-	if err != nil {
-		return err
-	}
-
 	if !interfaceExistsInNamespace(netNSPath, ifName) {
 		maxIDLen := IFNAMSIZ - 1 - len(vethPrefix+"pl")
 		if len(id) > maxIDLen {
@@ -138,54 +131,67 @@ func AttachContainer(netNSPath, id, ifName, bridgeName string, mtu int, withMult
 		}
 	}
 
-	if err := WithNetNSLinkUnsafe(ns, ifName, func(veth netlink.Link) error {
-		newAddresses, err := AddAddresses(veth, cidrs)
+	args := []string{ifName, fmt.Sprintf("%t", withMulticastRoute)}
+	for _, cidr := range cidrs {
+		args = append(args, cidr.String())
+	}
+	if _, err := WithNetNS(netNSPath, "setup-iface-addrs", args...); err != nil {
+		return fmt.Errorf("error setting up interface addresses: %s", err)
+	}
+	return nil
+}
+
+// SetupIfaceAddrs is the implementation of the 'setup-iface-addrs' call above,
+// running in another process in the container's netns
+func SetupIfaceAddrs(veth netlink.Link, withMulticastRoute bool, cidrs []*net.IPNet) error {
+	newAddresses, err := AddAddresses(veth, cidrs)
+	if err != nil {
+		return err
+	}
+
+	ifName := veth.Attrs().Name
+	ipt, err := iptables.New()
+	if err != nil {
+		return err
+	}
+
+	// Add multicast ACCEPT rules for new subnets
+	for _, ipnet := range newAddresses {
+		acceptRule := []string{"-i", ifName, "-s", subnet(ipnet), "-d", "224.0.0.0/4", "-j", "ACCEPT"}
+		exists, err := ipt.Exists("filter", "INPUT", acceptRule...)
 		if err != nil {
 			return err
 		}
-
-		// Add multicast ACCEPT rules for new subnets
-		for _, ipnet := range newAddresses {
-			acceptRule := []string{"-i", ifName, "-s", subnet(ipnet), "-d", "224.0.0.0/4", "-j", "ACCEPT"}
-			exists, err := ipt.Exists("filter", "INPUT", acceptRule...)
-			if err != nil {
+		if !exists {
+			if err := ipt.Insert("filter", "INPUT", 1, acceptRule...); err != nil {
 				return err
 			}
-			if !exists {
-				if err := ipt.Insert("filter", "INPUT", 1, acceptRule...); err != nil {
-					return err
-				}
-			}
 		}
+	}
 
-		if err := netlink.LinkSetUp(veth); err != nil {
+	if err := netlink.LinkSetUp(veth); err != nil {
+		return err
+	}
+	for _, ipnet := range newAddresses {
+		// If we don't wait for a bit here, we see the arp fail to reach the bridge.
+		time.Sleep(1 * time.Millisecond)
+		arping.GratuitousArpOverIfaceByName(ipnet.IP, ifName)
+	}
+	if withMulticastRoute {
+		/* Route multicast packets across the weave network.
+		This must come last in 'attach'. If you change this, change weavewait to match.
+
+		TODO: Add the MTU lock to prevent PMTU discovery for multicast
+		destinations. Without that, the kernel sets the DF flag on
+		multicast packets. Since RFC1122 prohibits sending of ICMP
+		errors for packets with multicast destinations, that causes
+		packets larger than the PMTU to be dropped silently.  */
+
+		_, multicast, _ := net.ParseCIDR("224.0.0.0/4")
+		if err := AddRoute(veth, netlink.SCOPE_LINK, multicast, nil); err != nil {
 			return err
 		}
-		for _, ipnet := range newAddresses {
-			// If we don't wait for a bit here, we see the arp fail to reach the bridge.
-			time.Sleep(1 * time.Millisecond)
-			arping.GratuitousArpOverIfaceByName(ipnet.IP, ifName)
-		}
-		if withMulticastRoute {
-			/* Route multicast packets across the weave network.
-			This must come last in 'attach'. If you change this, change weavewait to match.
-
-			TODO: Add the MTU lock to prevent PMTU discovery for multicast
-			destinations. Without that, the kernel sets the DF flag on
-			multicast packets. Since RFC1122 prohibits sending of ICMP
-			errors for packets with multicast destinations, that causes
-			packets larger than the PMTU to be dropped silently.  */
-
-			_, multicast, _ := net.ParseCIDR("224.0.0.0/4")
-			if err := AddRoute(veth, netlink.SCOPE_LINK, multicast, nil); err != nil {
-				return err
-			}
-		}
-		return nil
-	}); err != nil {
-		return err
 	}
-
 	return nil
 }
 

prog/weaveutil/main.go

@@ -32,6 +32,7 @@ func init() {
 		"check-iface":              checkIface,
 		"del-iface":                delIface,
 		"setup-iface":              setupIface,
+		"setup-iface-addrs":        setupIfaceAddrs,
 		"list-netdevs":             listNetDevs,
 		"cni-net":                  cniNet,
 		"cni-ipam":                 cniIPAM,

prog/weaveutil/netdev.go

@@ -50,6 +50,26 @@ func setupIface(args []string) error {
 	return weavenet.SetupIface(ifaceName, newIfName)
 }
 
+// setupIfaceAddrs sets up addresses on an interface. It expects to be called inside the container's netns.
+func setupIfaceAddrs(args []string) error {
+	if len(args) < 1 {
+		cmdUsage("setup-iface-addrs", "<iface-name> <with-multicast> <cidr>...")
+	}
+	link, err := netlink.LinkByName(args[0])
+	if err != nil {
+		return err
+	}
+	withMulticastRoute, err := strconv.ParseBool(args[1])
+	if err != nil {
+		return err
+	}
+	cidrs, err := parseCIDRs(args[2:])
+	if err != nil {
+		return err
+	}
+	return weavenet.SetupIfaceAddrs(link, withMulticastRoute, cidrs)
+}
+
 func configureARP(args []string) error {
 	if len(args) != 2 {
 		cmdUsage("configure-arp", "<iface-name-prefix> <root-path>")