Denote WithNetNS* functions with unsafe

Please see #2388 (comment) for more details.
weaveworks · Jul 4, 2016 · c2d3022 · c2d3022
1 parent ea45822
commit c2d3022
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 10 deletions.
diff --git a/common/utils.go b/common/utils.go
@@ -48,7 +48,7 @@ func FindNetDevs(processID int, match func(link netlink.Link) bool) ([]NetDev, e
 	}
 	defer ns.Close()
 
-	err = weavenet.WithNetNS(ns, func() error {
+	err = weavenet.WithNetNSUnsafe(ns, func() error {
 		return forEachLink(func(link netlink.Link) error {
 			if match(link) {
 				netDev, err := linkToNetDev(link)

diff --git a/net/netns.go b/net/netns.go
@@ -7,7 +7,17 @@ import (
 	"github.com/vishvananda/netns"
 )
 
-func WithNetNS(ns netns.NsHandle, work func() error) error {
+// NB: The following function is unsafe, because it changes a network namespace
+//     of an OS thread which executes it. During the execution, the Go runtime
+//     might clone a new thread which is going to run in the given ns and might
+//     schedule other go-routines which suppose to run in the host network ns.
+//     Also, the work function cannot create any go-routine, because it might
+//     be run by other threads running in any non-given network namespace.
+//     Please see https://github.com/weaveworks/weave/issues/2388#issuecomment-228365069
+//     for more details.
+//
+//     Before using, make sure that you understand the implications!
+func WithNetNSUnsafe(ns netns.NsHandle, work func() error) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
@@ -26,8 +36,8 @@ func WithNetNS(ns netns.NsHandle, work func() error) error {
 	return err
 }
 
-func WithNetNSLink(ns netns.NsHandle, ifName string, work func(link netlink.Link) error) error {
-	return WithNetNS(ns, func() error {
+func WithNetNSLinkUnsafe(ns netns.NsHandle, ifName string, work func(link netlink.Link) error) error {
+	return WithNetNSUnsafe(ns, func() error {
 		link, err := netlink.LinkByName(ifName)
 		if err != nil {
 			return err

diff --git a/net/veth.go b/net/veth.go
@@ -104,7 +104,7 @@ const (
 )
 
 func interfaceExistsInNamespace(ns netns.NsHandle, ifName string) bool {
-	err := WithNetNS(ns, func() error {
+	err := WithNetNSUnsafe(ns, func() error {
 		_, err := netlink.LinkByName(ifName)
 		return err
 	})
@@ -122,7 +122,7 @@ func AttachContainer(ns netns.NsHandle, id, ifName, bridgeName string, mtu int,
 			if err := netlink.LinkSetNsFd(veth, int(ns)); err != nil {
 				return fmt.Errorf("failed to move veth to container netns: %s", err)
 			}
-			if err := WithNetNS(ns, func() error {
+			if err := WithNetNSUnsafe(ns, func() error {
 				if err := netlink.LinkSetName(veth, ifName); err != nil {
 					return err
 				}
@@ -140,7 +140,7 @@ func AttachContainer(ns netns.NsHandle, id, ifName, bridgeName string, mtu int,
 		}
 	}
 
-	if err := WithNetNSLink(ns, ifName, func(veth netlink.Link) error {
+	if err := WithNetNSLinkUnsafe(ns, ifName, func(veth netlink.Link) error {
 		newAddresses, err := AddAddresses(veth, cidrs)
 		if err != nil {
 			return err
@@ -177,7 +177,7 @@ func AttachContainer(ns netns.NsHandle, id, ifName, bridgeName string, mtu int,
 }
 
 func DetachContainer(ns netns.NsHandle, id, ifName string, cidrs []*net.IPNet) error {
-	return WithNetNSLink(ns, ifName, func(veth netlink.Link) error {
+	return WithNetNSLinkUnsafe(ns, ifName, func(veth netlink.Link) error {
 		existingAddrs, err := netlink.AddrList(veth, netlink.FAMILY_V4)
 		if err != nil {
 			return fmt.Errorf("failed to get IP address for %q: %v", veth.Attrs().Name, err)

diff --git a/plugin/net/cni.go b/plugin/net/cni.go
@@ -95,7 +95,7 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error {
 	if err := weavenet.AttachContainer(ns, id, args.IfName, conf.BrName, conf.MTU, false, []*net.IPNet{&result.IP4.IP}, false); err != nil {
 		return err
 	}
-	if err := weavenet.WithNetNSLink(ns, args.IfName, func(link netlink.Link) error {
+	if err := weavenet.WithNetNSLinkUnsafe(ns, args.IfName, func(link netlink.Link) error {
 		return setupRoutes(link, args.IfName, result.IP4.IP, result.IP4.Gateway, result.IP4.Routes)
 	}); err != nil {
 		return fmt.Errorf("error setting up routes: %s", err)
@@ -158,7 +158,7 @@ func (c *CNIPlugin) CmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 	defer ns.Close()
-	err = weavenet.WithNetNS(ns, func() error {
+	err = weavenet.WithNetNSUnsafe(ns, func() error {
 		link, err := netlink.LinkByName(args.IfName)
 		if err != nil {
 			return err