diff --git a/net/netns.go b/net/netns.go index a501f15892..5e9e205f3b 100644 --- a/net/netns.go +++ b/net/netns.go @@ -57,16 +57,6 @@ func WithNetNSLinkUnsafe(ns netns.NsHandle, ifName string, work func(link netlin }) } -func WithNetNSLinkByPidUnsafe(pid int, ifName string, work func(link netlink.Link) error) error { - ns, err := netns.GetFromPid(pid) - if err != nil { - return err - } - defer ns.Close() - - return WithNetNSLinkUnsafe(ns, ifName, work) -} - // A safe version of WithNetNS* which creates a process executing // "nsenter --net= weaveutil [args]". func WithNetNS(nsPath string, cmd string, args ...string) (string, error) { diff --git a/net/veth.go b/net/veth.go index e88be9edaa..f33d7b589d 100644 --- a/net/veth.go +++ b/net/veth.go @@ -105,21 +105,26 @@ const ( vethPrefix = "v" + VethName // starts with "veth" to suppress UI notifications ) -func interfaceExistsInNamespace(ns netns.NsHandle, ifName string) bool { - err := WithNetNSUnsafe(ns, func() error { - _, err := netlink.LinkByName(ifName) - return err - }) +func interfaceExistsInNamespace(netNSPath string, ifName string) bool { + _, err := WithNetNS(netNSPath, "check-iface", ifName) return err == nil } -func AttachContainer(ns netns.NsHandle, id, ifName, bridgeName string, mtu int, withMulticastRoute bool, cidrs []*net.IPNet, keepTXOn bool) error { +// 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(ns, ifName) { + if !interfaceExistsInNamespace(netNSPath, ifName) { maxIDLen := IFNAMSIZ - 1 - len(vethPrefix+"pl") if len(id) > maxIDLen { id = id[:maxIDLen] // trim passed ID if too long @@ -129,18 +134,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 := WithNetNSUnsafe(ns, func() error { - if err := netlink.LinkSetName(veth, ifName); err != nil { - return err - } - if err := ConfigureARPCache(ifName); err != nil { - return err - } - if err := ipt.Append("filter", "INPUT", "-i", ifName, "-d", "224.0.0.0/4", "-j", "DROP"); err != nil { - return err - } - return nil - }); err != nil { + if _, err := WithNetNS(netNSPath, "setup-iface", peerName, ifName); err != nil { return fmt.Errorf("error setting up interface: %s", err) } return nil @@ -201,7 +195,15 @@ func AttachContainer(ns netns.NsHandle, id, ifName, bridgeName string, mtu int, return nil } -func DetachContainer(ns netns.NsHandle, id, ifName string, cidrs []*net.IPNet) error { +// NB: This function can be used only by a process that terminates immediately +// after calling the function as it changes netns via WithNetNSLinkUnsafe. +func DetachContainer(netNSPath, id, ifName string, cidrs []*net.IPNet) error { + ns, err := netns.GetFromPath(netNSPath) + if err != nil { + return err + } + defer ns.Close() + ipt, err := iptables.New() if err != nil { return err diff --git a/plugin/net/cni.go b/plugin/net/cni.go index 0fdd5b419b..37ae566349 100644 --- a/plugin/net/cni.go +++ b/plugin/net/cni.go @@ -107,7 +107,7 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { id = fmt.Sprintf("%x", data) } - if err := weavenet.AttachContainer(ns, id, args.IfName, conf.BrName, conf.MTU, false, []*net.IPNet{&result.IP4.IP}, false); err != nil { + if err := weavenet.AttachContainer(args.Netns, id, args.IfName, conf.BrName, conf.MTU, false, []*net.IPNet{&result.IP4.IP}, false); err != nil { return err } if err := weavenet.WithNetNSLinkUnsafe(ns, args.IfName, func(link netlink.Link) error { @@ -181,19 +181,7 @@ func (c *CNIPlugin) CmdDel(args *skel.CmdArgs) error { return err } - ns, err := netns.GetFromPath(args.Netns) - if err != nil { - return fmt.Errorf("error accessing namespace %q: %s", args.Netns, err) - } - defer ns.Close() - err = weavenet.WithNetNSUnsafe(ns, func() error { - link, err := netlink.LinkByName(args.IfName) - if err != nil { - return err - } - return netlink.LinkDel(link) - }) - if err != nil { + if _, err = weavenet.WithNetNS(args.Netns, "del-iface", args.IfName); err != nil { return fmt.Errorf("error removing interface %q: %s", args.IfName, err) } diff --git a/prog/weaveutil/attach.go b/prog/weaveutil/attach.go index c3c469deeb..fd45810e3d 100644 --- a/prog/weaveutil/attach.go +++ b/prog/weaveutil/attach.go @@ -32,10 +32,15 @@ func attach(args []string) error { } } - pid, nsContainer, err := containerPidAndNs(args[0]) + pid, err := containerPid(args[0]) if err != nil { return err } + nsContainer, err := netns.GetFromPid(pid) + if err != nil { + return fmt.Errorf("unable to open namespace for container %s: %s", args[0], err) + } + if nsHost, err := netns.GetFromPid(1); err != nil { return fmt.Errorf("unable to open host namespace: %s", err) } else if nsHost.Equal(nsContainer) { @@ -50,7 +55,7 @@ func attach(args []string) error { return err } - err = weavenet.AttachContainer(nsContainer, fmt.Sprint(pid), weavenet.VethName, args[1], mtu, withMulticastRoute, cidrs, keepTXOn) + err = weavenet.AttachContainer(weavenet.NSPathByPid(pid), fmt.Sprint(pid), weavenet.VethName, args[1], mtu, withMulticastRoute, cidrs, keepTXOn) // If we detected an error but the container has died, tell the user that instead. if err != nil && !processExists(pid) { err = fmt.Errorf("Container %s died", args[0]) @@ -58,23 +63,19 @@ func attach(args []string) error { return err } -func containerPidAndNs(containerID string) (int, netns.NsHandle, error) { +func containerPid(containerID string) (int, error) { c, err := docker.NewVersionedClientFromEnv("1.18") if err != nil { - return 0, 0, fmt.Errorf("unable to connect to docker: %s", err) + return 0, fmt.Errorf("unable to connect to docker: %s", err) } container, err := c.InspectContainer(containerID) if err != nil { - return 0, 0, fmt.Errorf("unable to inspect container %s: %s", containerID, err) + return 0, fmt.Errorf("unable to inspect container %s: %s", containerID, err) } if container.State.Pid == 0 { - return 0, 0, fmt.Errorf("container %s not running", containerID) - } - ns, err := netns.GetFromPid(container.State.Pid) - if err != nil { - return 0, 0, fmt.Errorf("unable to open namespace for container %s: %s", containerID, err) + return 0, fmt.Errorf("container %s not running", containerID) } - return container.State.Pid, ns, nil + return container.State.Pid, nil } func processExists(pid int) bool { @@ -99,7 +100,7 @@ func detach(args []string) error { cmdUsage("detach-container", " ...") } - _, ns, err := containerPidAndNs(args[0]) + pid, err := containerPid(args[0]) if err != nil { return err } @@ -107,5 +108,5 @@ func detach(args []string) error { if err != nil { return err } - return weavenet.DetachContainer(ns, args[0], weavenet.VethName, cidrs) + return weavenet.DetachContainer(weavenet.NSPathByPid(pid), args[0], weavenet.VethName, cidrs) }