diff --git a/net/arp.go b/net/arp.go index bbd6dd7b..7d9418f8 100644 --- a/net/arp.go +++ b/net/arp.go @@ -10,7 +10,7 @@ import ( // Configure the ARP cache parameters for the given interface. This // makes containers react more quickly to a change in the MAC address // associated with an IP address. -func ConfigureARPCache(procPath, name string) error { +func configureARPCache(procPath, name string) error { if err := sysctl(procPath, fmt.Sprintf("net/ipv4/neigh/%s/base_reachable_time", name), "5"); err != nil { return err } diff --git a/net/bridge.go b/net/bridge.go index 3be330fb..504a5d12 100644 --- a/net/bridge.go +++ b/net/bridge.go @@ -275,7 +275,7 @@ func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger) (Br return bridgeType, err } - if err := ConfigureARPCache(procPath, config.WeaveBridgeName); err != nil { + if err := configureARPCache(procPath, config.WeaveBridgeName); err != nil { return bridgeType, errors.Wrapf(err, "configuring ARP cache on bridge %q", config.WeaveBridgeName) } diff --git a/net/netdev.go b/net/netdev.go index 35480cbf..33e9b8f2 100644 --- a/net/netdev.go +++ b/net/netdev.go @@ -1,11 +1,9 @@ package net import ( - "encoding/json" "fmt" "net" "os" - "strconv" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" @@ -17,7 +15,7 @@ type Dev struct { CIDRs []*net.IPNet `json:"CIDRs,omitempty"` } -func LinkToNetDev(link netlink.Link) (Dev, error) { +func linkToNetDev(link netlink.Link) (Dev, error) { addrs, err := netlink.AddrList(link, netlink.FAMILY_V4) if err != nil { return Dev{}, err @@ -90,16 +88,30 @@ func GetNetDevsByVethPeerIds(processID int, peerIDs []int) ([]Dev, error) { return nil, nil } + // convert list of peerIDs into a map for faster lookup + indexes := make(map[int]struct{}) + for _, id := range peerIDs { + indexes[id] = struct{}{} + } + var netdevs []Dev - peersStr := make([]string, len(peerIDs)) - for i, id := range peerIDs { - peersStr[i] = strconv.Itoa(id) - } - nds, err := WithNetNSByPid(processID, "list-netdevs", peersStr...) - if err != nil { - return nil, fmt.Errorf("list-netdevs failed: %s", err) - } - err = json.Unmarshal(nds, &netdevs) + err = WithNetNS(netnsContainer, func() error { + links, err := netlink.LinkList() + if err != nil { + return err + } + + for _, link := range links { + if _, found := indexes[link.Attrs().Index]; found { + netdev, err := linkToNetDev(link) + if err != nil { + return err + } + netdevs = append(netdevs, netdev) + } + } + return nil + }) return netdevs, err } @@ -112,5 +124,5 @@ func GetBridgeNetDev(bridgeName string) (Dev, error) { return Dev{}, err } - return LinkToNetDev(link) + return linkToNetDev(link) } diff --git a/net/netns.go b/net/netns.go index 3bd4a1c2..0a5c2249 100644 --- a/net/netns.go +++ b/net/netns.go @@ -1,10 +1,10 @@ +// +build go1.10 + package net import ( - "bytes" "errors" "fmt" - "os/exec" "path/filepath" "runtime" @@ -14,19 +14,7 @@ import ( var ErrLinkNotFound = errors.New("Link not found") -// NB: The following function is unsafe, because: -// - It changes a network namespace (netns) of an OS thread which runs -// the function. During execution, the Go runtime might clone a new OS thread -// for scheduling other go-routines, thus they might end up running in -// a "wrong" netns. -// - runtime.LockOSThread does not guarantee that a spawned go-routine on -// the locked thread will be run by it. Thus, the work function is -// not allowed to spawn any go-routine which is dependent on the given netns. - -// Please see https://github.com/weaveworks/weave/issues/2388#issuecomment-228365069 -// for more details and make sure that you understand the implications before -// using the function! -func WithNetNSUnsafe(ns netns.NsHandle, work func() error) error { +func WithNetNS(ns netns.NsHandle, work func() error) error { runtime.LockOSThread() defer runtime.UnlockOSThread() @@ -45,8 +33,8 @@ func WithNetNSUnsafe(ns netns.NsHandle, work func() error) error { return err } -func WithNetNSLinkUnsafe(ns netns.NsHandle, ifName string, work func(link netlink.Link) error) error { - return WithNetNSUnsafe(ns, func() error { +func WithNetNSLink(ns netns.NsHandle, ifName string, work func(link netlink.Link) error) error { + return WithNetNS(ns, func() error { link, err := netlink.LinkByName(ifName) if err != nil { if err.Error() == errors.New("Link not found").Error() { @@ -58,26 +46,12 @@ func WithNetNSLinkUnsafe(ns netns.NsHandle, ifName string, work func(link netlin }) } -var WeaveUtilCmd = "weaveutil" - -// A safe version of WithNetNS* which creates a process executing -// "nsenter --net= weaveutil [args]". -func WithNetNS(nsPath string, cmd string, args ...string) ([]byte, error) { - var stdout, stderr bytes.Buffer - - args = append([]string{"--net=" + nsPath, WeaveUtilCmd, cmd}, args...) - c := exec.Command("nsenter", args...) - c.Stdout = &stdout - c.Stderr = &stderr - if err := c.Run(); err != nil { - return nil, fmt.Errorf("%s: %s", string(stderr.Bytes()), err) +func WithNetNSByPath(path string, work func() error) error { + ns, err := netns.GetFromPath(path) + if err != nil { + return err } - - return stdout.Bytes(), nil -} - -func WithNetNSByPid(pid int, cmd string, args ...string) ([]byte, error) { - return WithNetNS(NSPathByPid(pid), cmd, args...) + return WithNetNS(ns, work) } func NSPathByPid(pid int) string { diff --git a/net/veth.go b/net/veth.go index 6c188fb5..dd82abef 100644 --- a/net/veth.go +++ b/net/veth.go @@ -104,7 +104,10 @@ const ( ) func interfaceExistsInNamespace(netNSPath string, ifName string) bool { - _, err := WithNetNS(netNSPath, "check-iface", ifName) + err := WithNetNSByPath(netNSPath, func() error { + _, err := netlink.LinkByName(ifName) + return err + }) return err == nil } @@ -125,7 +128,9 @@ func AttachContainer(netNSPath, id, ifName, bridgeName string, mtu int, withMult if err := netlink.LinkSetNsFd(veth, int(ns)); err != nil { return fmt.Errorf("failed to move veth to container netns: %s", err) } - if _, err := WithNetNS(netNSPath, "setup-iface", peerName, ifName); err != nil { + if err := WithNetNS(ns, func() error { + return setupIface(peerName, ifName) + }); err != nil { return fmt.Errorf("error setting up interface: %s", err) } return nil @@ -139,19 +144,16 @@ func AttachContainer(netNSPath, id, ifName, bridgeName string, mtu int, withMult } - 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 { + if err := WithNetNSLink(ns, ifName, func(veth netlink.Link) error { + return setupIfaceAddrs(veth, withMulticastRoute, cidrs) + }); 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 { +// setupIfaceAddrs expects to be called 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 @@ -203,9 +205,8 @@ func SetupIfaceAddrs(veth netlink.Link, withMulticastRoute bool, cidrs []*net.IP return nil } -// SetupIface is the implementation of the 'setup-iface' call above, -// running in another process in the container's netns -func SetupIface(ifaceName, newIfName string) error { +// setupIface expects to be called in the container's netns +func setupIface(ifaceName, newIfName string) error { ipt, err := iptables.New() if err != nil { return err @@ -219,14 +220,32 @@ func SetupIface(ifaceName, newIfName string) error { return err } // This is only called by AttachContainer which is only called in host pid namespace - if err := ConfigureARPCache("/proc", newIfName); err != nil { + if err := configureARPCache("/proc", newIfName); err != nil { return err } return ipt.Append("filter", "INPUT", "-i", newIfName, "-d", "224.0.0.0/4", "-j", "DROP") } -// NB: This function can be used only by a process that terminates immediately -// after calling the function as it changes netns via WithNetNSLinkUnsafe. +// configureARP is a helper for the Docker plugin which doesn't set the addresses itself +func ConfigureARP(prefix, rootPath string) error { + links, err := netlink.LinkList() + if err != nil { + return err + } + for _, link := range links { + ifName := link.Attrs().Name + if strings.HasPrefix(ifName, prefix) { + configureARPCache(rootPath+"/proc", ifName) + if addrs, err := netlink.AddrList(link, netlink.FAMILY_V4); err == nil { + for _, addr := range addrs { + arping.GratuitousArpOverIfaceByName(addr.IPNet.IP, ifName) + } + } + } + } + return nil +} + func DetachContainer(netNSPath, id, ifName string, cidrs []*net.IPNet) error { ns, err := netns.GetFromPath(netNSPath) if err != nil { @@ -239,7 +258,7 @@ func DetachContainer(netNSPath, id, ifName string, cidrs []*net.IPNet) error { return err } - return WithNetNSLinkUnsafe(ns, ifName, func(veth netlink.Link) error { + return WithNetNSLink(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 index c23041ba..363971a3 100644 --- a/plugin/net/cni.go +++ b/plugin/net/cni.go @@ -130,7 +130,7 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { if err := weavenet.AttachContainer(args.Netns, id, args.IfName, conf.BrName, conf.MTU, false, []*net.IPNet{&ip.Address}, false, conf.HairpinMode); err != nil { return err } - if err := weavenet.WithNetNSLinkUnsafe(ns, args.IfName, func(link netlink.Link) error { + if err := weavenet.WithNetNSLink(ns, args.IfName, func(link netlink.Link) error { return setupRoutes(link, args.IfName, ip.Address, ip.Gateway, result.Routes) }); err != nil { return fmt.Errorf("error setting up routes: %s", err) @@ -177,7 +177,13 @@ func (c *CNIPlugin) CmdDel(args *skel.CmdArgs) error { // As of CNI 0.3 spec, runtimes can send blank if they just want the address deallocated if args.Netns != "" { - if _, err = weavenet.WithNetNS(args.Netns, "del-iface", args.IfName); err != nil { + if err = weavenet.WithNetNSByPath(args.Netns, func() error { + link, err := netlink.LinkByName(args.IfName) + if err != nil { + return err + } + return netlink.LinkDel(link) + }); err != nil { // We log the error and carry on instead of returning nil, as there may still be resources to free in IPAM. logOnStderr(fmt.Errorf("error removing interface %q: %s", args.IfName, err)) } diff --git a/plugin/net/watcher.go b/plugin/net/watcher.go index d31ad71f..2c386a8c 100644 --- a/plugin/net/watcher.go +++ b/plugin/net/watcher.go @@ -49,7 +49,9 @@ func (w *watcher) ContainerStarted(id string) { rootDir = "/host" } netNSPath := weavenet.NSPathByPidWithRoot(rootDir, info.State.Pid) - if _, err := weavenet.WithNetNS(netNSPath, "configure-arp", weavenet.VethName, rootDir); err != nil { + if err := weavenet.WithNetNSByPath(netNSPath, func() error { + return weavenet.ConfigureARP(weavenet.VethName, rootDir) + }); err != nil { w.driver.warn("ContainerStarted", "unable to configure interfaces: %s", err) } } diff --git a/prog/weaveutil/main.go b/prog/weaveutil/main.go index 45592e7d..354b8c49 100644 --- a/prog/weaveutil/main.go +++ b/prog/weaveutil/main.go @@ -4,10 +4,7 @@ package main import ( "fmt" "os" - "path/filepath" "strings" - - weavenet "github.com/weaveworks/weave/net" ) var commands map[string]func([]string) error @@ -36,12 +33,6 @@ func init() { "create-volume-container": createVolumeContainer, "attach-container": attach, "detach-container": detach, - "configure-arp": configureARP, - "check-iface": checkIface, - "del-iface": delIface, - "setup-iface": setupIface, - "setup-iface-addrs": setupIfaceAddrs, - "list-netdevs": listNetDevs, "cni-net": cniNet, "cni-ipam": cniIPAM, "bridge-ip": bridgeIP, @@ -55,14 +46,6 @@ func init() { } func main() { - // force re-exec of this binary - selfPath, err := filepath.EvalSymlinks("/proc/self/exe") - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - weavenet.WeaveUtilCmd = selfPath - // If no args passed, act as CNI plugin based on executable name switch { case len(os.Args) == 1 && strings.HasSuffix(os.Args[0], "weave-ipam"): diff --git a/prog/weaveutil/netdev.go b/prog/weaveutil/netdev.go deleted file mode 100644 index af4e63a5..00000000 --- a/prog/weaveutil/netdev.go +++ /dev/null @@ -1,141 +0,0 @@ -package main - -import ( - "encoding/json" - "fmt" - "strconv" - "strings" - - "github.com/j-keck/arping" - "github.com/vishvananda/netlink" - - weavenet "github.com/weaveworks/weave/net" -) - -// checkIface returns an error if the given interface cannot be found. -func checkIface(args []string) error { - if len(args) != 1 { - cmdUsage("check-iface", "") - } - ifaceName := args[0] - - if _, err := netlink.LinkByName(ifaceName); err != nil { - return err - } - - return nil -} - -func delIface(args []string) error { - if len(args) != 1 { - cmdUsage("del-iface", "") - } - ifName := args[0] - - link, err := netlink.LinkByName(ifName) - if err != nil { - return err - } - return netlink.LinkDel(link) -} - -// setupIface renames the given iface and configures its ARP cache settings. -func setupIface(args []string) error { - if len(args) != 2 { - cmdUsage("setup-iface", " ") - } - ifaceName := args[0] - newIfName := args[1] - - 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", " ...") - } - 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", " ") - } - prefix := args[0] - rootPath := args[1] - - links, err := netlink.LinkList() - if err != nil { - return err - } - for _, link := range links { - ifName := link.Attrs().Name - if strings.HasPrefix(ifName, prefix) { - weavenet.ConfigureARPCache(rootPath+"/proc", ifName) - if addrs, err := netlink.AddrList(link, netlink.FAMILY_V4); err == nil { - for _, addr := range addrs { - arping.GratuitousArpOverIfaceByName(addr.IPNet.IP, ifName) - } - } - } - } - - return nil -} - -// listNetDevs outputs network ifaces identified by the given indexes -// in the format of weavenet.Dev. -func listNetDevs(args []string) error { - if len(args) == 0 { - cmdUsage("list-netdevs", "[ ]") - } - - indexes := make(map[int]struct{}) - for _, index := range args { - if index != "" { - id, err := strconv.Atoi(index) - if err != nil { - return err - } - indexes[id] = struct{}{} - } - } - - links, err := netlink.LinkList() - if err != nil { - return err - } - - var netdevs []weavenet.Dev - - for _, link := range links { - if _, found := indexes[link.Attrs().Index]; found { - netdev, err := weavenet.LinkToNetDev(link) - if err != nil { - return err - } - netdevs = append(netdevs, netdev) - } - } - - nds, err := json.Marshal(netdevs) - if err != nil { - return err - } - fmt.Println(string(nds)) - - return nil -}