Merge pull request #116 from squeed/release-bump-go

release: bump golang version

.appveyor.yml

@@ -0,0 +1,28 @@
+clone_folder: c:\gopath\src\github.com\containernetworking\plugins
+
+environment:
+  GOPATH: c:\gopath
+
+install:
+  - echo %PATH%
+  - echo %GOPATH%
+  - set PATH=%GOPATH%\bin;c:\go\bin;%PATH%
+  - go version
+  - go env
+
+build: off
+
+test_script:
+  - ps: |
+      go list ./... | Select-String -Pattern (Get-Content "./plugins/linux_only.txt") -NotMatch > "to_test.txt"
+      echo "Will test:"
+      Get-Content "to_test.txt"
+      foreach ($pkg in Get-Content "to_test.txt") {
+        if ($pkg) {
+          echo $pkg
+          go test -v $pkg
+          if ($LastExitCode -ne 0) {
+            throw "test failed"
+          }
+        }
+      }

.travis.yml

@@ -3,12 +3,12 @@ sudo: required
 dist: trusty
 
 go:
-  - 1.7.x
   - 1.8.x
+  - 1.9.x
 
 env:
   global:
-  - PATH=$GOROOT/bin:$PATH
+  - PATH=$GOROOT/bin:$GOPATH/bin:$PATH
   matrix:
   - TARGET=amd64
   - TARGET=arm
@@ -19,6 +19,9 @@ env:
 matrix:
   fast_finish: true
 
+install:
+  - go get github.com/onsi/ginkgo/ginkgo
+
 script:
  - |
    if [ "${TARGET}" == "amd64" ]; then

Godeps/Godeps.json

@@ -6,6 +6,10 @@
 		"./..."
 	],
 	"Deps": [
+		{
+			"ImportPath": "github.com/alexflint/go-filemutex",
+			"Rev": "72bdc8eae2aef913234599b837f5dda445ca9bd9"
+		},
 		{
 			"ImportPath": "github.com/containernetworking/cni/libcni",
 			"Comment": "v0.6.0-rc1",

README.md

@@ -1,4 +1,5 @@
-[![Build Status](https://travis-ci.org/containernetworking/plugins.svg?branch=master)](https://travis-ci.org/containernetworking/plugins)
+[![Linux Build Status](https://travis-ci.org/containernetworking/plugins.svg?branch=master)](https://travis-ci.org/containernetworking/plugins)
+[![Windows Build Status](https://ci.appveyor.com/api/projects/status/kcuubx0chr76ev86/branch/master?svg=true)](https://ci.appveyor.com/project/cni-bot/plugins/branch/master)
 
 # plugins
 Some CNI network plugins, maintained by the containernetworking team. For more information, see the individual READMEs.

Vagrantfile

@@ -12,7 +12,7 @@ Vagrant.configure(2) do |config|
     apt-get update -y || (sleep 40 && apt-get update -y)
     apt-get install -y git
 
-    wget -qO- https://storage.googleapis.com/golang/go1.8.3.linux-amd64.tar.gz | tar -C /usr/local -xz
+    wget -qO- https://storage.googleapis.com/golang/go1.9.1.linux-amd64.tar.gz | tar -C /usr/local -xz
 
     echo 'export GOPATH=/go' >> /root/.bashrc
     echo 'export PATH=$PATH:/usr/local/go/bin:$GOPATH/bin' >> /root/.bashrc

pkg/ip/ipforward_linux.go

@@ -15,6 +15,7 @@
 package ip
 
 import (
+	"bytes"
 	"io/ioutil"
 
 	"github.com/containernetworking/cni/pkg/types/current"
@@ -51,5 +52,10 @@ func EnableForward(ips []*current.IPConfig) error {
 }
 
 func echo1(f string) error {
+	if content, err := ioutil.ReadFile(f); err == nil {
+		if bytes.Equal(bytes.TrimSpace(content), []byte("1")) {
+			return nil
+		}
+	}
 	return ioutil.WriteFile(f, []byte("1"), 0644)
 }

pkg/ip/ipforward_linux_test.go

@@ -0,0 +1,31 @@
+package ip
+
+import (
+	"io/ioutil"
+	"os"
+	"time"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("IpforwardLinux", func() {
+	It("echo1 must not write the file if content is 1", func() {
+		file, err := ioutil.TempFile(os.TempDir(), "containernetworking")
+		defer os.Remove(file.Name())
+		err = echo1(file.Name())
+		Expect(err).NotTo(HaveOccurred())
+		statBefore, err := file.Stat()
+		Expect(err).NotTo(HaveOccurred())
+
+		// take a duration here, otherwise next file modification operation time
+		// will be same as previous one.
+		time.Sleep(100 * time.Millisecond)
+
+		err = echo1(file.Name())
+		Expect(err).NotTo(HaveOccurred())
+		statAfter, err := file.Stat()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(statBefore.ModTime()).To(Equal(statAfter.ModTime()))
+	})
+})

pkg/ip/ipmasq_linux.go

@@ -75,7 +75,16 @@ func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error {
 
 // TeardownIPMasq undoes the effects of SetupIPMasq
 func TeardownIPMasq(ipn *net.IPNet, chain string, comment string) error {
-	ipt, err := iptables.New()
+	isV6 := ipn.IP.To4() == nil
+
+	var ipt *iptables.IPTables
+	var err error
+
+	if isV6 {
+		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6)
+	} else {
+		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	}
 	if err != nil {
 		return fmt.Errorf("failed to locate iptables: %v", err)
 	}

pkg/ip/link_linux.go

@@ -158,6 +158,9 @@ func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (net.Interface, ne
 func DelLinkByName(ifName string) error {
 	iface, err := netlink.LinkByName(ifName)
 	if err != nil {
+		if err.Error() == "Link not found" {
+			return ErrLinkNotFound
+		}
 		return fmt.Errorf("failed to lookup %q: %v", ifName, err)
 	}
 
@@ -168,9 +171,8 @@ func DelLinkByName(ifName string) error {
 	return nil
 }
 
-// DelLinkByNameAddr remove an interface returns its IP address
-// of the specified family
-func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) {
+// DelLinkByNameAddr remove an interface and returns its addresses
+func DelLinkByNameAddr(ifName string) ([]*net.IPNet, error) {
 	iface, err := netlink.LinkByName(ifName)
 	if err != nil {
 		if err != nil && err.Error() == "Link not found" {
@@ -179,8 +181,8 @@ func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) {
 		return nil, fmt.Errorf("failed to lookup %q: %v", ifName, err)
 	}
 
-	addrs, err := netlink.AddrList(iface, family)
-	if err != nil || len(addrs) == 0 {
+	addrs, err := netlink.AddrList(iface, netlink.FAMILY_ALL)
+	if err != nil {
 		return nil, fmt.Errorf("failed to get IP addresses for %q: %v", ifName, err)
 	}
 
@@ -188,7 +190,14 @@ func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) {
 		return nil, fmt.Errorf("failed to delete %q: %v", ifName, err)
 	}
 
-	return addrs[0].IPNet, nil
+	out := []*net.IPNet{}
+	for _, addr := range addrs {
+		if addr.IP.IsGlobalUnicast() {
+			out = append(out, addr.IPNet)
+		}
+	}
+
+	return out, nil
 }
 
 func SetHWAddrByIP(ifName string, ip4 net.IP, ip6 net.IP) error {

pkg/ip/link_linux_test.go

@@ -27,7 +27,6 @@ import (
 	"github.com/containernetworking/plugins/pkg/ns"
 
 	"github.com/vishvananda/netlink"
-	"github.com/vishvananda/netlink/nl"
 )
 
 func getHwAddr(linkname string) string {
@@ -133,7 +132,7 @@ var _ = Describe("Link", func() {
 				defer GinkgoRecover()
 
 				// This string should match the expected error codes in the cmdDel functions of some of the plugins
-				_, err := ip.DelLinkByNameAddr("THIS_DONT_EXIST", netlink.FAMILY_V4)
+				_, err := ip.DelLinkByNameAddr("THIS_DONT_EXIST")
 				Expect(err).To(Equal(ip.ErrLinkNotFound))
 
 				return nil
@@ -220,16 +219,14 @@ var _ = Describe("Link", func() {
 		})
 	})
 
-	It("DelLinkByNameAddr must throw an error for configured interfaces", func() {
+	It("DelLinkByNameAddr should return no IPs when no IPs are configured", func() {
 		_ = containerNetNS.Do(func(ns.NetNS) error {
 			defer GinkgoRecover()
 
 			// this will delete the host endpoint too
-			addr, err := ip.DelLinkByNameAddr(containerVethName, nl.FAMILY_V4)
-			Expect(err).To(HaveOccurred())
-
-			var ipNetNil *net.IPNet
-			Expect(addr).To(Equal(ipNetNil))
+			addr, err := ip.DelLinkByNameAddr(containerVethName)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(addr).To(HaveLen(0))
 			return nil
 		})
 	})

pkg/ip/route_linux.go

@@ -39,3 +39,9 @@ func AddHostRoute(ipn *net.IPNet, gw net.IP, dev netlink.Link) error {
 		Gw:        gw,
 	})
 }
+
+// AddDefaultRoute sets the default route on the given gateway.
+func AddDefaultRoute(gw net.IP, dev netlink.Link) error {
+	_, defNet, _ := net.ParseCIDR("0.0.0.0/0")
+	return AddRoute(defNet, gw, dev)
+}

pkg/ipam/ipam.go

@@ -15,16 +15,8 @@
 package ipam
 
 import (
-	"fmt"
-	"net"
-	"os"
-
 	"github.com/containernetworking/cni/pkg/invoke"
 	"github.com/containernetworking/cni/pkg/types"
-	"github.com/containernetworking/cni/pkg/types/current"
-	"github.com/containernetworking/plugins/pkg/ip"
-
-	"github.com/vishvananda/netlink"
 )
 
 func ExecAdd(plugin string, netconf []byte) (types.Result, error) {
@@ -34,66 +26,3 @@ func ExecAdd(plugin string, netconf []byte) (types.Result, error) {
 func ExecDel(plugin string, netconf []byte) error {
 	return invoke.DelegateDel(plugin, netconf)
 }
-
-// ConfigureIface takes the result of IPAM plugin and
-// applies to the ifName interface
-func ConfigureIface(ifName string, res *current.Result) error {
-	if len(res.Interfaces) == 0 {
-		return fmt.Errorf("no interfaces to configure")
-	}
-
-	link, err := netlink.LinkByName(ifName)
-	if err != nil {
-		return fmt.Errorf("failed to lookup %q: %v", ifName, err)
-	}
-
-	if err := netlink.LinkSetUp(link); err != nil {
-		return fmt.Errorf("failed to set %q UP: %v", ifName, err)
-	}
-
-	var v4gw, v6gw net.IP
-	for _, ipc := range res.IPs {
-		if ipc.Interface == nil {
-			continue
-		}
-		intIdx := *ipc.Interface
-		if intIdx < 0 || intIdx >= len(res.Interfaces) || res.Interfaces[intIdx].Name != ifName {
-			// IP address is for a different interface
-			return fmt.Errorf("failed to add IP addr %v to %q: invalid interface index", ipc, ifName)
-		}
-
-		addr := &netlink.Addr{IPNet: &ipc.Address, Label: ""}
-		if err = netlink.AddrAdd(link, addr); err != nil {
-			return fmt.Errorf("failed to add IP addr %v to %q: %v", ipc, ifName, err)
-		}
-
-		gwIsV4 := ipc.Gateway.To4() != nil
-		if gwIsV4 && v4gw == nil {
-			v4gw = ipc.Gateway
-		} else if !gwIsV4 && v6gw == nil {
-			v6gw = ipc.Gateway
-		}
-	}
-
-	ip.SettleAddresses(ifName, 10)
-
-	for _, r := range res.Routes {
-		routeIsV4 := r.Dst.IP.To4() != nil
-		gw := r.GW
-		if gw == nil {
-			if routeIsV4 && v4gw != nil {
-				gw = v4gw
-			} else if !routeIsV4 && v6gw != nil {
-				gw = v6gw
-			}
-		}
-		if err = ip.AddRoute(&r.Dst, gw, link); err != nil {
-			// we skip over duplicate routes as we assume the first one wins
-			if !os.IsExist(err) {
-				return fmt.Errorf("failed to add route '%v via %v dev %v': %v", r.Dst, gw, ifName, err)
-			}
-		}
-	}
-
-	return nil
-}

pkg/ipam/ipam_linux.go

@@ -0,0 +1,91 @@
+// Copyright 2015 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package ipam
+
+import (
+	"fmt"
+	"net"
+	"os"
+
+	"github.com/containernetworking/cni/pkg/types/current"
+	"github.com/containernetworking/plugins/pkg/ip"
+
+	"github.com/vishvananda/netlink"
+)
+
+// ConfigureIface takes the result of IPAM plugin and
+// applies to the ifName interface
+func ConfigureIface(ifName string, res *current.Result) error {
+	if len(res.Interfaces) == 0 {
+		return fmt.Errorf("no interfaces to configure")
+	}
+
+	link, err := netlink.LinkByName(ifName)
+	if err != nil {
+		return fmt.Errorf("failed to lookup %q: %v", ifName, err)
+	}
+
+	if err := netlink.LinkSetUp(link); err != nil {
+		return fmt.Errorf("failed to set %q UP: %v", ifName, err)
+	}
+
+	var v4gw, v6gw net.IP
+	for _, ipc := range res.IPs {
+		if ipc.Interface == nil {
+			continue
+		}
+		intIdx := *ipc.Interface
+		if intIdx < 0 || intIdx >= len(res.Interfaces) || res.Interfaces[intIdx].Name != ifName {
+			// IP address is for a different interface
+			return fmt.Errorf("failed to add IP addr %v to %q: invalid interface index", ipc, ifName)
+		}
+
+		addr := &netlink.Addr{IPNet: &ipc.Address, Label: ""}
+		if err = netlink.AddrAdd(link, addr); err != nil {
+			return fmt.Errorf("failed to add IP addr %v to %q: %v", ipc, ifName, err)
+		}
+
+		gwIsV4 := ipc.Gateway.To4() != nil
+		if gwIsV4 && v4gw == nil {
+			v4gw = ipc.Gateway
+		} else if !gwIsV4 && v6gw == nil {
+			v6gw = ipc.Gateway
+		}
+	}
+
+	if v6gw != nil {
+		ip.SettleAddresses(ifName, 10)
+	}
+
+	for _, r := range res.Routes {
+		routeIsV4 := r.Dst.IP.To4() != nil
+		gw := r.GW
+		if gw == nil {
+			if routeIsV4 && v4gw != nil {
+				gw = v4gw
+			} else if !routeIsV4 && v6gw != nil {
+				gw = v6gw
+			}
+		}
+		if err = ip.AddRoute(&r.Dst, gw, link); err != nil {
+			// we skip over duplicate routes as we assume the first one wins
+			if !os.IsExist(err) {
+				return fmt.Errorf("failed to add route '%v via %v dev %v': %v", r.Dst, gw, ifName, err)
+			}
+		}
+	}
+
+	return nil
+}

pkg/ipam/ipam_linux_test.go

@@ -39,7 +39,7 @@ func ipNetEqual(a, b *net.IPNet) bool {
 	return a.IP.Equal(b.IP)
 }
 
-var _ = Describe("IPAM Operations", func() {
+var _ = Describe("ConfigureIface", func() {
 	var originalNS ns.NetNS
 	var ipv4, ipv6, routev4, routev6 *net.IPNet
 	var ipgw4, ipgw6, routegwv4, routegwv6 net.IP

pkg/ns/ns.go

@@ -1,178 +0,0 @@
-// Copyright 2015 CNI authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package ns
-
-import (
-	"fmt"
-	"os"
-	"runtime"
-	"sync"
-	"syscall"
-)
-
-type NetNS interface {
-	// Executes the passed closure in this object's network namespace,
-	// attempting to restore the original namespace before returning.
-	// However, since each OS thread can have a different network namespace,
-	// and Go's thread scheduling is highly variable, callers cannot
-	// guarantee any specific namespace is set unless operations that
-	// require that namespace are wrapped with Do().  Also, no code called
-	// from Do() should call runtime.UnlockOSThread(), or the risk
-	// of executing code in an incorrect namespace will be greater.  See
-	// https://github.com/golang/go/wiki/LockOSThread for further details.
-	Do(toRun func(NetNS) error) error
-
-	// Sets the current network namespace to this object's network namespace.
-	// Note that since Go's thread scheduling is highly variable, callers
-	// cannot guarantee the requested namespace will be the current namespace
-	// after this function is called; to ensure this wrap operations that
-	// require the namespace with Do() instead.
-	Set() error
-
-	// Returns the filesystem path representing this object's network namespace
-	Path() string
-
-	// Returns a file descriptor representing this object's network namespace
-	Fd() uintptr
-
-	// Cleans up this instance of the network namespace; if this instance
-	// is the last user the namespace will be destroyed
-	Close() error
-}
-
-type netNS struct {
-	file    *os.File
-	mounted bool
-	closed  bool
-}
-
-// netNS implements the NetNS interface
-var _ NetNS = &netNS{}
-
-const (
-	// https://github.com/torvalds/linux/blob/master/include/uapi/linux/magic.h
-	NSFS_MAGIC   = 0x6e736673
-	PROCFS_MAGIC = 0x9fa0
-)
-
-type NSPathNotExistErr struct{ msg string }
-
-func (e NSPathNotExistErr) Error() string { return e.msg }
-
-type NSPathNotNSErr struct{ msg string }
-
-func (e NSPathNotNSErr) Error() string { return e.msg }
-
-func IsNSorErr(nspath string) error {
-	stat := syscall.Statfs_t{}
-	if err := syscall.Statfs(nspath, &stat); err != nil {
-		if os.IsNotExist(err) {
-			err = NSPathNotExistErr{msg: fmt.Sprintf("failed to Statfs %q: %v", nspath, err)}
-		} else {
-			err = fmt.Errorf("failed to Statfs %q: %v", nspath, err)
-		}
-		return err
-	}
-
-	switch stat.Type {
-	case PROCFS_MAGIC, NSFS_MAGIC:
-		return nil
-	default:
-		return NSPathNotNSErr{msg: fmt.Sprintf("unknown FS magic on %q: %x", nspath, stat.Type)}
-	}
-}
-
-// Returns an object representing the namespace referred to by @path
-func GetNS(nspath string) (NetNS, error) {
-	err := IsNSorErr(nspath)
-	if err != nil {
-		return nil, err
-	}
-
-	fd, err := os.Open(nspath)
-	if err != nil {
-		return nil, err
-	}
-
-	return &netNS{file: fd}, nil
-}
-
-func (ns *netNS) Path() string {
-	return ns.file.Name()
-}
-
-func (ns *netNS) Fd() uintptr {
-	return ns.file.Fd()
-}
-
-func (ns *netNS) errorIfClosed() error {
-	if ns.closed {
-		return fmt.Errorf("%q has already been closed", ns.file.Name())
-	}
-	return nil
-}
-
-func (ns *netNS) Do(toRun func(NetNS) error) error {
-	if err := ns.errorIfClosed(); err != nil {
-		return err
-	}
-
-	containedCall := func(hostNS NetNS) error {
-		threadNS, err := GetCurrentNS()
-		if err != nil {
-			return fmt.Errorf("failed to open current netns: %v", err)
-		}
-		defer threadNS.Close()
-
-		// switch to target namespace
-		if err = ns.Set(); err != nil {
-			return fmt.Errorf("error switching to ns %v: %v", ns.file.Name(), err)
-		}
-		defer threadNS.Set() // switch back
-
-		return toRun(hostNS)
-	}
-
-	// save a handle to current network namespace
-	hostNS, err := GetCurrentNS()
-	if err != nil {
-		return fmt.Errorf("Failed to open current namespace: %v", err)
-	}
-	defer hostNS.Close()
-
-	var wg sync.WaitGroup
-	wg.Add(1)
-
-	var innerError error
-	go func() {
-		defer wg.Done()
-		runtime.LockOSThread()
-		innerError = containedCall(hostNS)
-	}()
-	wg.Wait()
-
-	return innerError
-}
-
-// WithNetNSPath executes the passed closure under the given network
-// namespace, restoring the original namespace afterwards.
-func WithNetNSPath(nspath string, toRun func(NetNS) error) error {
-	ns, err := GetNS(nspath)
-	if err != nil {
-		return err
-	}
-	defer ns.Close()
-	return ns.Do(toRun)
-}

pkg/ns/ns_linux.go

@@ -21,6 +21,7 @@ import (
 	"path"
 	"runtime"
 	"sync"
+	"syscall"
 
 	"golang.org/x/sys/unix"
 )
@@ -147,3 +148,158 @@ func (ns *netNS) Set() error {
 
 	return nil
 }
+
+type NetNS interface {
+	// Executes the passed closure in this object's network namespace,
+	// attempting to restore the original namespace before returning.
+	// However, since each OS thread can have a different network namespace,
+	// and Go's thread scheduling is highly variable, callers cannot
+	// guarantee any specific namespace is set unless operations that
+	// require that namespace are wrapped with Do().  Also, no code called
+	// from Do() should call runtime.UnlockOSThread(), or the risk
+	// of executing code in an incorrect namespace will be greater.  See
+	// https://github.com/golang/go/wiki/LockOSThread for further details.
+	Do(toRun func(NetNS) error) error
+
+	// Sets the current network namespace to this object's network namespace.
+	// Note that since Go's thread scheduling is highly variable, callers
+	// cannot guarantee the requested namespace will be the current namespace
+	// after this function is called; to ensure this wrap operations that
+	// require the namespace with Do() instead.
+	Set() error
+
+	// Returns the filesystem path representing this object's network namespace
+	Path() string
+
+	// Returns a file descriptor representing this object's network namespace
+	Fd() uintptr
+
+	// Cleans up this instance of the network namespace; if this instance
+	// is the last user the namespace will be destroyed
+	Close() error
+}
+
+type netNS struct {
+	file    *os.File
+	mounted bool
+	closed  bool
+}
+
+// netNS implements the NetNS interface
+var _ NetNS = &netNS{}
+
+const (
+	// https://github.com/torvalds/linux/blob/master/include/uapi/linux/magic.h
+	NSFS_MAGIC   = 0x6e736673
+	PROCFS_MAGIC = 0x9fa0
+)
+
+type NSPathNotExistErr struct{ msg string }
+
+func (e NSPathNotExistErr) Error() string { return e.msg }
+
+type NSPathNotNSErr struct{ msg string }
+
+func (e NSPathNotNSErr) Error() string { return e.msg }
+
+func IsNSorErr(nspath string) error {
+	stat := syscall.Statfs_t{}
+	if err := syscall.Statfs(nspath, &stat); err != nil {
+		if os.IsNotExist(err) {
+			err = NSPathNotExistErr{msg: fmt.Sprintf("failed to Statfs %q: %v", nspath, err)}
+		} else {
+			err = fmt.Errorf("failed to Statfs %q: %v", nspath, err)
+		}
+		return err
+	}
+
+	switch stat.Type {
+	case PROCFS_MAGIC, NSFS_MAGIC:
+		return nil
+	default:
+		return NSPathNotNSErr{msg: fmt.Sprintf("unknown FS magic on %q: %x", nspath, stat.Type)}
+	}
+}
+
+// Returns an object representing the namespace referred to by @path
+func GetNS(nspath string) (NetNS, error) {
+	err := IsNSorErr(nspath)
+	if err != nil {
+		return nil, err
+	}
+
+	fd, err := os.Open(nspath)
+	if err != nil {
+		return nil, err
+	}
+
+	return &netNS{file: fd}, nil
+}
+
+func (ns *netNS) Path() string {
+	return ns.file.Name()
+}
+
+func (ns *netNS) Fd() uintptr {
+	return ns.file.Fd()
+}
+
+func (ns *netNS) errorIfClosed() error {
+	if ns.closed {
+		return fmt.Errorf("%q has already been closed", ns.file.Name())
+	}
+	return nil
+}
+
+func (ns *netNS) Do(toRun func(NetNS) error) error {
+	if err := ns.errorIfClosed(); err != nil {
+		return err
+	}
+
+	containedCall := func(hostNS NetNS) error {
+		threadNS, err := GetCurrentNS()
+		if err != nil {
+			return fmt.Errorf("failed to open current netns: %v", err)
+		}
+		defer threadNS.Close()
+
+		// switch to target namespace
+		if err = ns.Set(); err != nil {
+			return fmt.Errorf("error switching to ns %v: %v", ns.file.Name(), err)
+		}
+		defer threadNS.Set() // switch back
+
+		return toRun(hostNS)
+	}
+
+	// save a handle to current network namespace
+	hostNS, err := GetCurrentNS()
+	if err != nil {
+		return fmt.Errorf("Failed to open current namespace: %v", err)
+	}
+	defer hostNS.Close()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+
+	var innerError error
+	go func() {
+		defer wg.Done()
+		runtime.LockOSThread()
+		innerError = containedCall(hostNS)
+	}()
+	wg.Wait()
+
+	return innerError
+}
+
+// WithNetNSPath executes the passed closure under the given network
+// namespace, restoring the original namespace afterwards.
+func WithNetNSPath(nspath string, toRun func(NetNS) error) error {
+	ns, err := GetNS(nspath)
+	if err != nil {
+		return err
+	}
+	defer ns.Close()
+	return ns.Do(toRun)
+}

pkg/ns/ns_unspecified.go

@@ -1,36 +0,0 @@
-// Copyright 2015-2017 CNI authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// +build !linux
-
-package ns
-
-import "github.com/containernetworking/cni/pkg/types"
-
-// Returns an object representing the current OS thread's network namespace
-func GetCurrentNS() (NetNS, error) {
-	return nil, types.NotImplementedError
-}
-
-func NewNS() (NetNS, error) {
-	return nil, types.NotImplementedError
-}
-
-func (ns *netNS) Close() error {
-	return types.NotImplementedError
-}
-
-func (ns *netNS) Set() error {
-	return types.NotImplementedError
-}

pkg/testutils/echosvr/echosvr_test.go

@@ -0,0 +1,74 @@
+package main_test
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net"
+	"os/exec"
+	"strings"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gbytes"
+	"github.com/onsi/gomega/gexec"
+)
+
+var binaryPath string
+
+var _ = SynchronizedBeforeSuite(func() []byte {
+	binaryPath, err := gexec.Build("github.com/containernetworking/plugins/pkg/testutils/echosvr")
+	Expect(err).NotTo(HaveOccurred())
+	return []byte(binaryPath)
+}, func(data []byte) {
+	binaryPath = string(data)
+})
+
+var _ = SynchronizedAfterSuite(func() {}, func() {
+	gexec.CleanupBuildArtifacts()
+})
+
+var _ = Describe("Echosvr", func() {
+	var session *gexec.Session
+	BeforeEach(func() {
+		var err error
+		cmd := exec.Command(binaryPath)
+		session, err = gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		session.Kill().Wait()
+	})
+
+	It("starts and doesn't terminate immediately", func() {
+		Consistently(session).ShouldNot(gexec.Exit())
+	})
+
+	tryConnect := func() (net.Conn, error) {
+		programOutput := session.Out.Contents()
+		addr := strings.TrimSpace(string(programOutput))
+
+		conn, err := net.Dial("tcp", addr)
+		if err != nil {
+			return nil, err
+		}
+		return conn, err
+	}
+
+	It("prints its listening address to stdout", func() {
+		Eventually(session.Out).Should(gbytes.Say("\n"))
+		conn, err := tryConnect()
+		Expect(err).NotTo(HaveOccurred())
+		conn.Close()
+	})
+
+	It("will echo data back to us", func() {
+		Eventually(session.Out).Should(gbytes.Say("\n"))
+		conn, err := tryConnect()
+		Expect(err).NotTo(HaveOccurred())
+		defer conn.Close()
+
+		fmt.Fprintf(conn, "hello")
+		Expect(ioutil.ReadAll(conn)).To(Equal([]byte("hello")))
+	})
+})

pkg/testutils/echosvr/init_test.go

@@ -0,0 +1,13 @@
+package main_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"testing"
+)
+
+func TestEchosvr(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Testutils Echosvr Suite")
+}

pkg/testutils/echosvr/main.go

@@ -0,0 +1,38 @@
+// Echosvr is a simple TCP echo server
+//
+// It prints its listen address on stdout
+//    127.0.0.1:xxxxx
+//  A test should wait for this line, parse it
+//  and may then attempt to connect.
+package main
+
+import (
+	"fmt"
+	"net"
+)
+
+func main() {
+	listener, err := net.Listen("tcp", ":")
+	if err != nil {
+		panic(err)
+	}
+	_, port, err := net.SplitHostPort(listener.Addr().String())
+	if err != nil {
+		panic(err)
+	}
+	fmt.Printf("127.0.0.1:%s\n", port)
+	for {
+		conn, err := listener.Accept()
+		if err != nil {
+			panic(err)
+		}
+		go handleConnection(conn)
+	}
+}
+
+func handleConnection(conn net.Conn) {
+	buf := make([]byte, 512)
+	nBytesRead, _ := conn.Read(buf)
+	conn.Write(buf[0:nBytesRead])
+	conn.Close()
+}

plugins/ipam/dhcp/README.md

@@ -18,6 +18,7 @@ $ ./dhcp daemon
 
 If given `-pidfile <path>` arguments after 'daemon', the dhcp plugin will write
 its PID to the given file.
+If given `-hostprefix <prefix>` arguments after 'daemon', the dhcp plugin will use this prefix for netns as `<prefix>/<original netns>`. It could be used in case of running dhcp daemon as container.
 
 Alternatively, you can use systemd socket activation protocol.
 Be sure that the .socket file uses /run/cni/dhcp.sock as the socket path.

plugins/ipam/dhcp/daemon.go

@@ -39,8 +39,9 @@ const resendCount = 3
 var errNoMoreTries = errors.New("no more tries")
 
 type DHCP struct {
-	mux    sync.Mutex
-	leases map[string]*DHCPLease
+	mux             sync.Mutex
+	leases          map[string]*DHCPLease
+	hostNetnsPrefix string
 }
 
 func newDHCP() *DHCP {
@@ -58,7 +59,8 @@ func (d *DHCP) Allocate(args *skel.CmdArgs, result *current.Result) error {
 	}
 
 	clientID := args.ContainerID + "/" + conf.Name
-	l, err := AcquireLease(clientID, args.Netns, args.IfName)
+	hostNetns := d.hostNetnsPrefix + args.Netns
+	l, err := AcquireLease(clientID, hostNetns, args.IfName)
 	if err != nil {
 		return err
 	}
@@ -91,10 +93,9 @@ func (d *DHCP) Release(args *skel.CmdArgs, reply *struct{}) error {
 
 	if l := d.getLease(args.ContainerID, conf.Name); l != nil {
 		l.Stop()
-		return nil
 	}
 
-	return fmt.Errorf("lease not found: %v/%v", args.ContainerID, conf.Name)
+	return nil
 }
 
 func (d *DHCP) getLease(contID, netName string) *DHCPLease {
@@ -141,7 +142,7 @@ func getListener() (net.Listener, error) {
 	}
 }
 
-func runDaemon(pidfilePath string) error {
+func runDaemon(pidfilePath string, hostPrefix string) error {
 	// since other goroutines (on separate threads) will change namespaces,
 	// ensure the RPC server does not get scheduled onto those
 	runtime.LockOSThread()
@@ -162,6 +163,7 @@ func runDaemon(pidfilePath string) error {
 	}
 
 	dhcp := newDHCP()
+	dhcp.hostNetnsPrefix = hostPrefix
 	rpc.Register(dhcp)
 	rpc.HandleHTTP()
 	http.Serve(l, nil)

plugins/ipam/dhcp/lease.go

@@ -292,8 +292,26 @@ func (l *DHCPLease) Gateway() net.IP {
 }
 
 func (l *DHCPLease) Routes() []*types.Route {
-	routes := parseRoutes(l.opts)
-	return append(routes, parseCIDRRoutes(l.opts)...)
+	routes := []*types.Route{}
+
+	// RFC 3442 states that if Classless Static Routes (option 121)
+	// exist, we ignore Static Routes (option 33) and the Router/Gateway.
+	opt121_routes := parseCIDRRoutes(l.opts)
+	if len(opt121_routes) > 0 {
+		return append(routes, opt121_routes...)
+	}
+
+	// Append Static Routes
+	routes = append(routes, parseRoutes(l.opts)...)
+
+	// The CNI spec says even if there is a gateway specified, we must
+	// add a default route in the routes section.
+	if gw := l.Gateway(); gw != nil {
+		_, defaultRoute, _ := net.ParseCIDR("0.0.0.0/0")
+		routes = append(routes, &types.Route{Dst: *defaultRoute, GW: gw})
+	}
+
+	return routes
 }
 
 // jitter returns a random value within [-span, span) range

plugins/ipam/dhcp/main.go

@@ -33,11 +33,13 @@ const socketPath = "/run/cni/dhcp.sock"
 func main() {
 	if len(os.Args) > 1 && os.Args[1] == "daemon" {
 		var pidfilePath string
+		var hostPrefix string
 		daemonFlags := flag.NewFlagSet("daemon", flag.ExitOnError)
 		daemonFlags.StringVar(&pidfilePath, "pidfile", "", "optional path to write daemon PID to")
+		daemonFlags.StringVar(&hostPrefix, "hostprefix", "", "optional prefix to netns")
 		daemonFlags.Parse(os.Args[2:])
 
-		if err := runDaemon(pidfilePath); err != nil {
+		if err := runDaemon(pidfilePath, hostPrefix); err != nil {
 			log.Printf(err.Error())
 			os.Exit(1)
 		}

plugins/ipam/host-local/README.md

@@ -120,6 +120,10 @@ The following [args conventions](https://github.com/containernetworking/cni/blob
 
 * `ips` (array of strings): A list of custom IPs to attempt to allocate
 
+The following [Capability Args](https://github.com/containernetworking/cni/blob/master/CONVENTIONS.md) are supported:
+
+* `ipRanges`: The exact same as the `ranges` array - a list of address pools
+
 ### Custom IP allocation
 For every requested custom IP, the `host-local` allocator will request that IP
 if it falls within one of the `range` objects. Thus it is possible to specify

plugins/ipam/host-local/backend/allocator/config.go

@@ -23,12 +23,16 @@ import (
 	types020 "github.com/containernetworking/cni/pkg/types/020"
 )
 
-// The top-level network config, just so we can get the IPAM block
+// The top-level network config - IPAM plugins are passed the full configuration
+// of the calling plugin, not just the IPAM section.
 type Net struct {
-	Name       string      `json:"name"`
-	CNIVersion string      `json:"cniVersion"`
-	IPAM       *IPAMConfig `json:"ipam"`
-	Args       *struct {
+	Name          string      `json:"name"`
+	CNIVersion    string      `json:"cniVersion"`
+	IPAM          *IPAMConfig `json:"ipam"`
+	RuntimeConfig struct {    // The capability arg
+		IPRanges []RangeSet `json:"ipRanges,omitempty"`
+	} `json:"runtimeConfig,omitempty"`
+	Args *struct {
 		A *IPAMArgs `json:"cni"`
 	} `json:"args"`
 }
@@ -106,6 +110,11 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
 	}
 	n.IPAM.Range = nil
 
+	// If a range is supplied as a runtime config, prepend it to the Ranges
+	if len(n.RuntimeConfig.IPRanges) > 0 {
+		n.IPAM.Ranges = append(n.RuntimeConfig.IPRanges, n.IPAM.Ranges...)
+	}
+
 	if len(n.IPAM.Ranges) == 0 {
 		return nil, "", fmt.Errorf("no IP ranges specified")
 	}

plugins/ipam/host-local/backend/allocator/config_test.go

@@ -132,12 +132,18 @@ var _ = Describe("IPAM config", func() {
 		}))
 	})
 
-	It("Should parse a mixed config", func() {
+	It("Should parse a mixed config with runtime args", func() {
 		input := `{
 			"cniVersion": "0.3.1",
 			"name": "mynet",
 			"type": "ipvlan",
 			"master": "foo0",
+			"runtimeConfig": {
+				"irrelevant": "a",
+				"ipRanges": [
+					[{ "subnet": "12.1.3.0/24" }]
+				]
+			},
 			"ipam": {
 				"type": "host-local",
 				"subnet": "10.1.2.0/24",
@@ -162,6 +168,17 @@ var _ = Describe("IPAM config", func() {
 			Name: "mynet",
 			Type: "host-local",
 			Ranges: []RangeSet{
+				{ // The RuntimeConfig should always be first
+					{
+						RangeStart: net.IP{12, 1, 3, 1},
+						RangeEnd:   net.IP{12, 1, 3, 254},
+						Gateway:    net.IP{12, 1, 3, 1},
+						Subnet: types.IPNet{
+							IP:   net.IP{12, 1, 3, 0},
+							Mask: net.CIDRMask(24, 32),
+						},
+					},
+				},
 				{
 					{
 						RangeStart: net.IP{10, 1, 2, 9},

plugins/ipam/host-local/backend/disk/backend.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"github.com/containernetworking/plugins/plugins/ipam/host-local/backend"
+	"runtime"
 )
 
 const lastIPFilePrefix = "last_reserved_ip."
@@ -31,7 +32,7 @@ var defaultDataDir = "/var/lib/cni/networks"
 // Store is a simple disk-backed store that creates one file per IP
 // address in a given directory. The contents of the file are the container ID.
 type Store struct {
-	FileLock
+	*FileLock
 	dataDir string
 }
 
@@ -51,11 +52,12 @@ func New(network, dataDir string) (*Store, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &Store{*lk, dir}, nil
+	return &Store{lk, dir}, nil
 }
 
 func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) {
-	fname := filepath.Join(s.dataDir, ip.String())
+	fname := GetEscapedPath(s.dataDir, ip.String())
+
 	f, err := os.OpenFile(fname, os.O_RDWR|os.O_EXCL|os.O_CREATE, 0644)
 	if os.IsExist(err) {
 		return false, nil
@@ -73,7 +75,7 @@ func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) {
 		return false, err
 	}
 	// store the reserved ip in lastIPFile
-	ipfile := filepath.Join(s.dataDir, lastIPFilePrefix+rangeID)
+	ipfile := GetEscapedPath(s.dataDir, lastIPFilePrefix+rangeID)
 	err = ioutil.WriteFile(ipfile, []byte(ip.String()), 0644)
 	if err != nil {
 		return false, err
@@ -83,7 +85,7 @@ func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) {
 
 // LastReservedIP returns the last reserved IP if exists
 func (s *Store) LastReservedIP(rangeID string) (net.IP, error) {
-	ipfile := filepath.Join(s.dataDir, lastIPFilePrefix+rangeID)
+	ipfile := GetEscapedPath(s.dataDir, lastIPFilePrefix+rangeID)
 	data, err := ioutil.ReadFile(ipfile)
 	if err != nil {
 		return nil, err
@@ -92,7 +94,7 @@ func (s *Store) LastReservedIP(rangeID string) (net.IP, error) {
 }
 
 func (s *Store) Release(ip net.IP) error {
-	return os.Remove(filepath.Join(s.dataDir, ip.String()))
+	return os.Remove(GetEscapedPath(s.dataDir, ip.String()))
 }
 
 // N.B. This function eats errors to be tolerant and
@@ -115,3 +117,10 @@ func (s *Store) ReleaseByID(id string) error {
 	})
 	return err
 }
+
+func GetEscapedPath(dataDir string, fname string) string {
+	if runtime.GOOS == "windows" {
+		fname = strings.Replace(fname, ":", "_", -1)
+	}
+	return filepath.Join(dataDir, fname)
+}

plugins/ipam/host-local/backend/disk/disk_suite_test.go

@@ -1,4 +1,4 @@
-// Copyright 2015 CNI authors
+// Copyright 2016 CNI authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,16 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package ip
+package disk
 
 import (
-	"net"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
 
-	"github.com/vishvananda/netlink"
+	"testing"
 )
 
-// AddDefaultRoute sets the default route on the given gateway.
-func AddDefaultRoute(gw net.IP, dev netlink.Link) error {
-	_, defNet, _ := net.ParseCIDR("0.0.0.0/0")
-	return AddRoute(defNet, gw, dev)
+func TestLock(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Disk Suite")
 }

plugins/ipam/host-local/backend/disk/lock.go

@@ -15,18 +15,28 @@
 package disk
 
 import (
+	"github.com/alexflint/go-filemutex"
 	"os"
-	"syscall"
+	"path"
 )
 
 // FileLock wraps os.File to be used as a lock using flock
 type FileLock struct {
-	f *os.File
+	f *filemutex.FileMutex
 }
 
 // NewFileLock opens file/dir at path and returns unlocked FileLock object
-func NewFileLock(path string) (*FileLock, error) {
-	f, err := os.Open(path)
+func NewFileLock(lockPath string) (*FileLock, error) {
+	fi, err := os.Stat(lockPath)
+	if err != nil {
+		return nil, err
+	}
+
+	if fi.IsDir() {
+		lockPath = path.Join(lockPath, "lock")
+	}
+
+	f, err := filemutex.New(lockPath)
 	if err != nil {
 		return nil, err
 	}
@@ -34,17 +44,16 @@ func NewFileLock(path string) (*FileLock, error) {
 	return &FileLock{f}, nil
 }
 
-// Close closes underlying file
 func (l *FileLock) Close() error {
 	return l.f.Close()
 }
 
 // Lock acquires an exclusive lock
 func (l *FileLock) Lock() error {
-	return syscall.Flock(int(l.f.Fd()), syscall.LOCK_EX)
+	return l.f.Lock()
 }
 
 // Unlock releases the lock
 func (l *FileLock) Unlock() error {
-	return syscall.Flock(int(l.f.Fd()), syscall.LOCK_UN)
+	return l.f.Unlock()
 }

plugins/ipam/host-local/backend/disk/lock_test.go

@@ -0,0 +1,63 @@
+// Copyright 2016 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package disk
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Lock Operations", func() {
+	It("locks a file path", func() {
+		dir, err := ioutil.TempDir("", "")
+		Expect(err).ToNot(HaveOccurred())
+		defer os.RemoveAll(dir)
+
+		// create a dummy file to lock
+		path := filepath.Join(dir, "x")
+		f, err := os.OpenFile(path, os.O_RDONLY|os.O_CREATE, 0666)
+		Expect(err).ToNot(HaveOccurred())
+		err = f.Close()
+		Expect(err).ToNot(HaveOccurred())
+
+		// now use it to lock
+		m, err := NewFileLock(path)
+		Expect(err).ToNot(HaveOccurred())
+
+		err = m.Lock()
+		Expect(err).ToNot(HaveOccurred())
+		err = m.Unlock()
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("locks a folder path", func() {
+		dir, err := ioutil.TempDir("", "")
+		Expect(err).ToNot(HaveOccurred())
+		defer os.RemoveAll(dir)
+
+		// use the folder to lock
+		m, err := NewFileLock(dir)
+		Expect(err).ToNot(HaveOccurred())
+
+		err = m.Lock()
+		Expect(err).ToNot(HaveOccurred())
+		err = m.Unlock()
+		Expect(err).ToNot(HaveOccurred())
+	})
+})

plugins/ipam/host-local/host_local_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/containernetworking/cni/pkg/types/current"
 	"github.com/containernetworking/plugins/pkg/testutils"
 
+	"github.com/containernetworking/plugins/plugins/ipam/host-local/backend/disk"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -37,7 +38,7 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
@@ -45,26 +46,26 @@ var _ = Describe("host-local Operations", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		conf := fmt.Sprintf(`{
-"cniVersion": "0.3.1",
-"name": "mynet",
-"type": "ipvlan",
-"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"dataDir": "%s",
-		"resolvConf": "%s/resolv.conf",
-		"ranges": [
-			[{ "subnet": "10.1.2.0/24" }, {"subnet": "10.2.2.0/24"}],
-			[{ "subnet": "2001:db8:1::0/64" }]
-		],
-		"routes": [
-			{"dst": "0.0.0.0/0"},
-			{"dst": "::/0"},
-			{"dst": "192.168.0.0/16", "gw": "1.1.1.1"},
-			{"dst": "2001:db8:2::0/64", "gw": "2001:db8:3::1"}
-		]
-	}
-}`, tmpDir, tmpDir)
+		"cniVersion": "0.3.1",
+		"name": "mynet",
+		"type": "ipvlan",
+		"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"dataDir": "%s",
+				"resolvConf": "%s/resolv.conf",
+				"ranges": [
+					[{ "subnet": "10.1.2.0/24" }, {"subnet": "10.2.2.0/24"}],
+					[{ "subnet": "2001:db8:1::0/64" }]
+				],
+				"routes": [
+					{"dst": "0.0.0.0/0"},
+					{"dst": "::/0"},
+					{"dst": "192.168.0.0/16", "gw": "1.1.1.1"},
+					{"dst": "2001:db8:2::0/64", "gw": "2001:db8:3::1"}
+				]
+			}
+		}`, tmpDir, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "dummy",
@@ -112,7 +113,7 @@ var _ = Describe("host-local Operations", func() {
 		Expect(err).NotTo(HaveOccurred())
 		Expect(string(contents)).To(Equal("dummy"))
 
-		ipFilePath2 := filepath.Join(tmpDir, "mynet", "2001:db8:1::2")
+		ipFilePath2 := filepath.Join(tmpDir, disk.GetEscapedPath("mynet", "2001:db8:1::2"))
 		contents, err = ioutil.ReadFile(ipFilePath2)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(string(contents)).To(Equal("dummy"))
@@ -142,21 +143,21 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
 		conf := fmt.Sprintf(`{
-	"cniVersion": "0.3.0",
-	"name": "mynet",
-	"type": "ipvlan",
-	"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"subnet": "10.1.2.0/24",
-		"dataDir": "%s"
-	}
-}`, tmpDir)
+			"cniVersion": "0.3.0",
+			"name": "mynet",
+			"type": "ipvlan",
+			"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"subnet": "10.1.2.0/24",
+				"dataDir": "%s"
+			}
+		}`, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "dummy",
@@ -176,7 +177,7 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
@@ -184,17 +185,17 @@ var _ = Describe("host-local Operations", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		conf := fmt.Sprintf(`{
-	"cniVersion": "0.1.0",
-	"name": "mynet",
-	"type": "ipvlan",
-	"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"subnet": "10.1.2.0/24",
-		"dataDir": "%s",
-		"resolvConf": "%s/resolv.conf"
-	}
-}`, tmpDir, tmpDir)
+			"cniVersion": "0.1.0",
+			"name": "mynet",
+			"type": "ipvlan",
+			"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"subnet": "10.1.2.0/24",
+				"dataDir": "%s",
+				"resolvConf": "%s/resolv.conf"
+			}
+		}`, tmpDir, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "dummy",
@@ -245,21 +246,21 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
 		conf := fmt.Sprintf(`{
-	"cniVersion": "0.3.1",
-	"name": "mynet",
-	"type": "ipvlan",
-	"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"subnet": "10.1.2.0/24",
-		"dataDir": "%s"
-	}
-}`, tmpDir)
+			"cniVersion": "0.3.1",
+			"name": "mynet",
+			"type": "ipvlan",
+			"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"subnet": "10.1.2.0/24",
+				"dataDir": "%s"
+			}
+		}`, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "   dummy\n ",
@@ -296,21 +297,21 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
 		conf := fmt.Sprintf(`{
-	"cniVersion": "0.2.0",
-	"name": "mynet",
-	"type": "ipvlan",
-	"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"subnet": "10.1.2.0/24",
-		"dataDir": "%s"
-	}
-}`, tmpDir)
+			"cniVersion": "0.2.0",
+			"name": "mynet",
+			"type": "ipvlan",
+			"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"subnet": "10.1.2.0/24",
+				"dataDir": "%s"
+			}
+		}`, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "testing",
@@ -331,28 +332,28 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
 		conf := fmt.Sprintf(`{
-	"cniVersion": "0.3.1",
-	"name": "mynet",
-	"type": "ipvlan",
-	"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"dataDir": "%s",
-		"ranges": [
-			[{ "subnet": "10.1.2.0/24" }]
-		]
-	},
-	"args": {
-		"cni": {
-			"ips": ["10.1.2.88"]
-		}
-	}
-}`, tmpDir)
+			"cniVersion": "0.3.1",
+			"name": "mynet",
+			"type": "ipvlan",
+			"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"dataDir": "%s",
+				"ranges": [
+					[{ "subnet": "10.1.2.0/24" }]
+				]
+			},
+			"args": {
+				"cni": {
+					"ips": ["10.1.2.88"]
+				}
+			}
+		}`, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "dummy",
@@ -376,7 +377,7 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
@@ -384,24 +385,24 @@ var _ = Describe("host-local Operations", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		conf := fmt.Sprintf(`{
-	"cniVersion": "0.3.1",
-	"name": "mynet",
-	"type": "ipvlan",
-	"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"dataDir": "%s",
-		"ranges": [
-			[{ "subnet": "10.1.2.0/24" }],
-			[{ "subnet": "10.1.3.0/24" }]
-		]
-	},
-	"args": {
-		"cni": {
-			"ips": ["10.1.2.88", "10.1.3.77"]
-		}
-	}
-}`, tmpDir)
+			"cniVersion": "0.3.1",
+			"name": "mynet",
+			"type": "ipvlan",
+			"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"dataDir": "%s",
+				"ranges": [
+					[{ "subnet": "10.1.2.0/24" }],
+					[{ "subnet": "10.1.3.0/24" }]
+				]
+			},
+			"args": {
+				"cni": {
+					"ips": ["10.1.2.88", "10.1.3.77"]
+				}
+			}
+		}`, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "dummy",
@@ -426,7 +427,7 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
@@ -434,24 +435,24 @@ var _ = Describe("host-local Operations", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		conf := fmt.Sprintf(`{
-	"cniVersion": "0.3.1",
-	"name": "mynet",
-	"type": "ipvlan",
-	"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"dataDir": "%s",
-		"ranges": [
-			[{"subnet":"172.16.1.0/24"}, { "subnet": "10.1.2.0/24" }],
-			[{ "subnet": "2001:db8:1::/24" }]
-		]
-	},
-	"args": {
-		"cni": {
-			"ips": ["10.1.2.88", "2001:db8:1::999"]
-		}
-	}
-}`, tmpDir)
+			"cniVersion": "0.3.1",
+			"name": "mynet",
+			"type": "ipvlan",
+			"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"dataDir": "%s",
+				"ranges": [
+					[{"subnet":"172.16.1.0/24"}, { "subnet": "10.1.2.0/24" }],
+					[{ "subnet": "2001:db8:1::/24" }]
+				]
+			},
+			"args": {
+				"cni": {
+					"ips": ["10.1.2.88", "2001:db8:1::999"]
+				}
+			}
+		}`, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "dummy",
@@ -476,29 +477,29 @@ var _ = Describe("host-local Operations", func() {
 		const ifname string = "eth0"
 		const nspath string = "/some/where"
 
-		tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+		tmpDir, err := getTmpDir()
 		Expect(err).NotTo(HaveOccurred())
 		defer os.RemoveAll(tmpDir)
 
 		conf := fmt.Sprintf(`{
-	"cniVersion": "0.3.1",
-	"name": "mynet",
-	"type": "ipvlan",
-	"master": "foo0",
-	"ipam": {
-		"type": "host-local",
-		"dataDir": "%s",
-		"ranges": [
-			[{ "subnet": "10.1.2.0/24" }],
-			[{ "subnet": "10.1.3.0/24" }]
-		]
-	},
-	"args": {
-		"cni": {
-			"ips": ["10.1.2.88", "10.1.2.77"]
-		}
-	}
-}`, tmpDir)
+			"cniVersion": "0.3.1",
+			"name": "mynet",
+			"type": "ipvlan",
+			"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"dataDir": "%s",
+				"ranges": [
+					[{ "subnet": "10.1.2.0/24" }],
+					[{ "subnet": "10.1.3.0/24" }]
+				]
+			},
+			"args": {
+				"cni": {
+					"ips": ["10.1.2.88", "10.1.2.77"]
+				}
+			}
+		}`, tmpDir)
 
 		args := &skel.CmdArgs{
 			ContainerID: "dummy",
@@ -517,6 +518,15 @@ var _ = Describe("host-local Operations", func() {
 	})
 })
 
+func getTmpDir() (string, error) {
+	tmpDir, err := ioutil.TempDir("", "host_local_artifacts")
+	if err == nil {
+		tmpDir = filepath.ToSlash(tmpDir)
+	}
+
+	return tmpDir, err
+}
+
 func mustCIDR(s string) net.IPNet {
 	ip, n, err := net.ParseCIDR(s)
 	n.IP = ip

plugins/linux_only.txt

@@ -0,0 +1,10 @@
+plugins/ipam/dhcp
+plugins/main/bridge
+plugins/main/host-device
+plugins/main/ipvlan
+plugins/main/loopback
+plugins/main/macvlan
+plugins/main/ptp
+plugins/main/vlan
+plugins/meta/portmap
+plugins/meta/tuning

plugins/main/bridge/bridge.go

@@ -32,6 +32,7 @@ import (
 	"github.com/containernetworking/plugins/pkg/ipam"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/containernetworking/plugins/pkg/utils"
+	"github.com/j-keck/arping"
 	"github.com/vishvananda/netlink"
 )
 
@@ -172,6 +173,13 @@ func ensureBridgeAddr(br *netlink.Bridge, family int, ipn *net.IPNet, forceAddre
 	if err := netlink.AddrAdd(br, addr); err != nil {
 		return fmt.Errorf("could not add IP address to %q: %v", br.Name, err)
 	}
+
+	// Set the bridge's MAC to itself. Otherwise, the bridge will take the
+	// lowest-numbered mac on the bridge, and will change as ifs churn
+	if err := netlink.LinkSetHardwareAddr(br, br.HardwareAddr); err != nil {
+		return fmt.Errorf("could not set bridge's mac: %v", err)
+	}
+
 	return nil
 }
 
@@ -294,8 +302,15 @@ func setupBridge(n *NetConf) (*netlink.Bridge, *current.Interface, error) {
 }
 
 // disableIPV6DAD disables IPv6 Duplicate Address Detection (DAD)
-// for an interface.
+// for an interface, if the interface does not support enhanced_dad.
+// We do this because interfaces with hairpin mode will see their own DAD packets
 func disableIPV6DAD(ifName string) error {
+	// ehanced_dad sends a nonce with the DAD packets, so that we can safely
+	// ignore ourselves
+	enh, err := ioutil.ReadFile(fmt.Sprintf("/proc/sys/net/ipv6/conf/%s/enhanced_dad", ifName))
+	if err == nil && string(enh) == "1\n" {
+		return nil
+	}
 	f := fmt.Sprintf("/proc/sys/net/ipv6/conf/%s/accept_dad", ifName)
 	return ioutil.WriteFile(f, []byte("0"), 0644)
 }
@@ -363,32 +378,34 @@ func cmdAdd(args *skel.CmdArgs) error {
 
 	// Configure the container hardware address and IP address(es)
 	if err := netns.Do(func(_ ns.NetNS) error {
-		// Disable IPv6 DAD just in case hairpin mode is enabled on the
-		// bridge. Hairpin mode causes echos of neighbor solicitation
-		// packets, which causes DAD failures.
-		// TODO: (short term) Disable DAD conditional on actual hairpin mode
-		// TODO: (long term) Use enhanced DAD when that becomes available in kernels.
-		if err := disableIPV6DAD(args.IfName); err != nil {
+		contVeth, err := net.InterfaceByName(args.IfName)
+		if err != nil {
 			return err
 		}
 
+		// Disable IPv6 DAD just in case hairpin mode is enabled on the
+		// bridge. Hairpin mode causes echos of neighbor solicitation
+		// packets, which causes DAD failures.
+		for _, ipc := range result.IPs {
+			if ipc.Version == "6" && (n.HairpinMode || n.PromiscMode) {
+				if err := disableIPV6DAD(args.IfName); err != nil {
+					return err
+				}
+				break
+			}
+		}
+
+		// Add the IP to the interface
 		if err := ipam.ConfigureIface(args.IfName, result); err != nil {
 			return err
 		}
 
-		if result.IPs[0].Address.IP.To4() != nil {
-			if err := ip.SetHWAddrByIP(args.IfName, result.IPs[0].Address.IP, nil /* TODO IPv6 */); err != nil {
-				return err
+		// Send a gratuitous arp
+		for _, ipc := range result.IPs {
+			if ipc.Version == "4" {
+				_ = arping.GratuitousArpOverIface(ipc.Address.IP, *contVeth)
 			}
 		}
-
-		// Refetch the veth since its MAC address may changed
-		link, err := netlink.LinkByName(args.IfName)
-		if err != nil {
-			return fmt.Errorf("could not lookup %q: %v", args.IfName, err)
-		}
-		containerInterface.Mac = link.Attrs().HardwareAddr.String()
-
 		return nil
 	}); err != nil {
 		return err
@@ -415,12 +432,6 @@ func cmdAdd(args *skel.CmdArgs) error {
 				}
 			}
 		}
-
-		if firstV4Addr != nil {
-			if err := ip.SetHWAddrByIP(n.BrName, firstV4Addr, nil /* TODO IPv6 */); err != nil {
-				return err
-			}
-		}
 	}
 
 	if n.IPMasq {
@@ -463,10 +474,10 @@ func cmdDel(args *skel.CmdArgs) error {
 	// There is a netns so try to clean up. Delete can be called multiple times
 	// so don't return an error if the device is already removed.
 	// If the device isn't there then don't try to clean up IP masq either.
-	var ipn *net.IPNet
+	var ipnets []*net.IPNet
 	err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		var err error
-		ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_ALL)
+		ipnets, err = ip.DelLinkByNameAddr(args.IfName)
 		if err != nil && err == ip.ErrLinkNotFound {
 			return nil
 		}
@@ -477,10 +488,14 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
-	if ipn != nil && n.IPMasq {
+	if n.IPMasq {
 		chain := utils.FormatChainName(n.Name, args.ContainerID)
 		comment := utils.FormatComment(n.Name, args.ContainerID)
-		err = ip.TeardownIPMasq(ipn, chain, comment)
+		for _, ipn := range ipnets {
+			if err := ip.TeardownIPMasq(ipn, chain, comment); err != nil {
+				return err
+			}
+		}
 	}
 
 	return err

plugins/main/bridge/bridge_test.go

@@ -25,7 +25,6 @@ import (
 	"github.com/containernetworking/cni/pkg/types/current"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/containernetworking/plugins/pkg/testutils"
-	"github.com/containernetworking/plugins/pkg/utils/hwaddr"
 
 	"github.com/vishvananda/netlink"
 
@@ -266,6 +265,7 @@ func (tester *testerV03x) cmdAddTest(tc testCase) {
 		Expect(link.Attrs().Name).To(Equal(BRNAME))
 		Expect(link).To(BeAssignableToTypeOf(&netlink.Bridge{}))
 		Expect(link.Attrs().HardwareAddr.String()).To(Equal(result.Interfaces[0].Mac))
+		bridgeMAC := link.Attrs().HardwareAddr.String()
 
 		// Ensure bridge has expected gateway address(es)
 		addrs, err := netlink.AddrList(link, netlink.FAMILY_ALL)
@@ -274,10 +274,6 @@ func (tester *testerV03x) cmdAddTest(tc testCase) {
 		for _, cidr := range tc.expGWCIDRs {
 			ip, subnet, err := net.ParseCIDR(cidr)
 			Expect(err).NotTo(HaveOccurred())
-			if ip.To4() != nil {
-				hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr)
-				Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString))
-			}
 
 			found := false
 			subnetPrefix, subnetBits := subnet.Mask.Size()
@@ -300,6 +296,12 @@ func (tester *testerV03x) cmdAddTest(tc testCase) {
 		Expect(err).NotTo(HaveOccurred())
 		Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{}))
 		tester.vethName = result.Interfaces[1].Name
+
+		// Check that the bridge has a different mac from the veth
+		// If not, it means the bridge has an unstable mac and will change
+		// as ifs are added and removed
+		Expect(link.Attrs().HardwareAddr.String()).NotTo(Equal(bridgeMAC))
+
 		return nil
 	})
 	Expect(err).NotTo(HaveOccurred())
@@ -314,14 +316,11 @@ func (tester *testerV03x) cmdAddTest(tc testCase) {
 		Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{}))
 
 		expCIDRsV4, expCIDRsV6 := tc.expectedCIDRs()
-		if expCIDRsV4 != nil {
-			hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr)
-			Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString))
-		}
 		addrs, err := netlink.AddrList(link, netlink.FAMILY_V4)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(addrs)).To(Equal(len(expCIDRsV4)))
 		addrs, err = netlink.AddrList(link, netlink.FAMILY_V6)
+		Expect(len(addrs)).To(Equal(len(expCIDRsV6) + 1)) //add one for the link-local
 		Expect(err).NotTo(HaveOccurred())
 		// Ignore link local address which may or may not be
 		// ready when we read addresses.
@@ -442,10 +441,6 @@ func (tester *testerV01xOr02x) cmdAddTest(tc testCase) {
 		for _, cidr := range tc.expGWCIDRs {
 			ip, subnet, err := net.ParseCIDR(cidr)
 			Expect(err).NotTo(HaveOccurred())
-			if ip.To4() != nil {
-				hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr)
-				Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString))
-			}
 
 			found := false
 			subnetPrefix, subnetBits := subnet.Mask.Size()
@@ -479,10 +474,6 @@ func (tester *testerV01xOr02x) cmdAddTest(tc testCase) {
 		Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{}))
 
 		expCIDRsV4, expCIDRsV6 := tc.expectedCIDRs()
-		if expCIDRsV4 != nil {
-			hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr)
-			Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString))
-		}
 		addrs, err := netlink.AddrList(link, netlink.FAMILY_V4)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(addrs)).To(Equal(len(expCIDRsV4)))
@@ -892,4 +883,28 @@ var _ = Describe("bridge Operations", func() {
 		})
 		Expect(err).NotTo(HaveOccurred())
 	})
+	It("creates a bridge with a stable MAC addresses", func() {
+		testCases := []testCase{
+			{
+				subnet: "10.1.2.0/24",
+			},
+			{
+				subnet: "2001:db8:42::/64",
+			},
+		}
+
+		for _, tc := range testCases {
+			tc.cniVersion = "0.3.1"
+			_, _, err := setupBridge(tc.netConf())
+			link, err := netlink.LinkByName(BRNAME)
+			Expect(err).NotTo(HaveOccurred())
+			origMac := link.Attrs().HardwareAddr
+
+			cmdAddDelTest(originalNS, tc)
+
+			link, err = netlink.LinkByName(BRNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().HardwareAddr).To(Equal(origMac))
+		}
+	})
 })

plugins/main/host-device/README.md

@@ -0,0 +1,21 @@
+# host-device
+Move an already-existing device in to a container.
+
+This simple plugin will move the requested device from the host's network namespace
+to the container's. Nothing else will be done - no IPAM, no addresses.
+
+The device can be specified with any one of three properties:
+* `device`: The device name, e.g. `eth0`, `can0`
+* `hwaddr`: A MAC address
+* `kernelpath`: The kernel device kobj, e.g. `/sys/devices/pci0000:00/0000:00:1f.6`
+
+For this plugin, `CNI_IFNAME` will be ignored. Upon DEL, the device will be moved back.
+
+A sample configuration might look like:
+
+```json
+{
+	"cniVersion": "0.3.1",
+	"device": "enp0s1"
+}
+```

plugins/main/host-device/host-device.go

@@ -0,0 +1,216 @@
+// Copyright 2015 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net"
+	"path/filepath"
+	"runtime"
+	"strings"
+
+	"github.com/containernetworking/cni/pkg/skel"
+	"github.com/containernetworking/cni/pkg/types"
+	"github.com/containernetworking/cni/pkg/types/current"
+	"github.com/containernetworking/cni/pkg/version"
+	"github.com/containernetworking/plugins/pkg/ns"
+	"github.com/vishvananda/netlink"
+)
+
+type NetConf struct {
+	types.NetConf
+	Device     string `json:"device"`     // Device-Name, something like eth0 or can0 etc.
+	HWAddr     string `json:"hwaddr"`     // MAC Address of target network interface
+	KernelPath string `json:"kernelpath"` // Kernelpath of the device
+}
+
+func init() {
+	// this ensures that main runs only on main thread (thread group leader).
+	// since namespace ops (unshare, setns) are done for a single thread, we
+	// must ensure that the goroutine does not jump from OS thread to thread
+	runtime.LockOSThread()
+}
+
+func loadConf(bytes []byte) (*NetConf, error) {
+	n := &NetConf{}
+	if err := json.Unmarshal(bytes, n); err != nil {
+		return nil, fmt.Errorf("failed to load netconf: %v", err)
+	}
+	if n.Device == "" && n.HWAddr == "" && n.KernelPath == "" {
+		return nil, fmt.Errorf(`specify either "device", "hwaddr" or "kernelpath"`)
+	}
+	return n, nil
+}
+
+func cmdAdd(args *skel.CmdArgs) error {
+	cfg, err := loadConf(args.StdinData)
+	if err != nil {
+		return err
+	}
+	containerNs, err := ns.GetNS(args.Netns)
+	if err != nil {
+		return fmt.Errorf("failed to open netns %q: %v", args.Netns, err)
+	}
+	defer containerNs.Close()
+
+	hostDev, err := getLink(cfg.Device, cfg.HWAddr, cfg.KernelPath)
+	if err != nil {
+		return fmt.Errorf("failed to find host device: %v", err)
+	}
+
+	contDev, err := moveLinkIn(hostDev, containerNs, args.IfName)
+	if err != nil {
+		return fmt.Errorf("failed to move link %v", err)
+	}
+	return printLink(contDev, cfg.CNIVersion, containerNs)
+}
+
+func cmdDel(args *skel.CmdArgs) error {
+	_, err := loadConf(args.StdinData)
+	if err != nil {
+		return err
+	}
+	containerNs, err := ns.GetNS(args.Netns)
+	if err != nil {
+		return fmt.Errorf("failed to open netns %q: %v", args.Netns, err)
+	}
+	defer containerNs.Close()
+
+	if err := moveLinkOut(containerNs, args.IfName); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, ifName string) (netlink.Link, error) {
+	if err := netlink.LinkSetNsFd(hostDev, int(containerNs.Fd())); err != nil {
+		return nil, err
+	}
+
+	var contDev netlink.Link
+	if err := containerNs.Do(func(_ ns.NetNS) error {
+		var err error
+		contDev, err = netlink.LinkByName(hostDev.Attrs().Name)
+		if err != nil {
+			return fmt.Errorf("failed to find %q: %v", hostDev.Attrs().Name, err)
+		}
+		// Save host device name into the container device's alias property
+		if err := netlink.LinkSetAlias(contDev, hostDev.Attrs().Name); err != nil {
+			return fmt.Errorf("failed to set alias to %q: %v", hostDev.Attrs().Name, err)
+		}
+		// Rename container device to respect args.IfName
+		if err := netlink.LinkSetName(contDev, ifName); err != nil {
+			return fmt.Errorf("failed to rename device %q to %q: %v", hostDev.Attrs().Name, ifName, err)
+		}
+		// Retrieve link again to get up-to-date name and attributes
+		contDev, err = netlink.LinkByName(ifName)
+		if err != nil {
+			return fmt.Errorf("failed to find %q: %v", ifName, err)
+		}
+		return nil
+	}); err != nil {
+		return nil, err
+	}
+
+	return contDev, nil
+}
+
+func moveLinkOut(containerNs ns.NetNS, ifName string) error {
+	defaultNs, err := ns.GetCurrentNS()
+	if err != nil {
+		return err
+	}
+	defer defaultNs.Close()
+
+	return containerNs.Do(func(_ ns.NetNS) error {
+		dev, err := netlink.LinkByName(ifName)
+		if err != nil {
+			return fmt.Errorf("failed to find %q: %v", ifName, err)
+		}
+		// Rename device to it's original name
+		if err := netlink.LinkSetName(dev, dev.Attrs().Alias); err != nil {
+			return fmt.Errorf("failed to restore %q to original name %q: %v", ifName, dev.Attrs().Alias, err)
+		}
+		if err := netlink.LinkSetNsFd(dev, int(defaultNs.Fd())); err != nil {
+			return fmt.Errorf("failed to move %q to host netns: %v", dev.Attrs().Alias, err)
+		}
+		return nil
+	})
+}
+
+func printLink(dev netlink.Link, cniVersion string, containerNs ns.NetNS) error {
+	result := current.Result{
+		CNIVersion: current.ImplementedSpecVersion,
+		Interfaces: []*current.Interface{
+			{
+				Name:    dev.Attrs().Name,
+				Mac:     dev.Attrs().HardwareAddr.String(),
+				Sandbox: containerNs.Path(),
+			},
+		},
+	}
+	return types.PrintResult(&result, cniVersion)
+}
+
+func getLink(devname, hwaddr, kernelpath string) (netlink.Link, error) {
+	links, err := netlink.LinkList()
+	if err != nil {
+		return nil, fmt.Errorf("failed to list node links: %v", err)
+	}
+
+	if len(devname) > 0 {
+		return netlink.LinkByName(devname)
+	} else if len(hwaddr) > 0 {
+		hwAddr, err := net.ParseMAC(hwaddr)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse MAC address %q: %v", hwaddr, err)
+		}
+
+		for _, link := range links {
+			if bytes.Equal(link.Attrs().HardwareAddr, hwAddr) {
+				return link, nil
+			}
+		}
+	} else if len(kernelpath) > 0 {
+		if !filepath.IsAbs(kernelpath) || !strings.HasPrefix(kernelpath, "/sys/devices/") {
+			return nil, fmt.Errorf("kernel device path %q must be absolute and begin with /sys/devices/", kernelpath)
+		}
+		netDir := filepath.Join(kernelpath, "net")
+		files, err := ioutil.ReadDir(netDir)
+		if err != nil {
+			return nil, fmt.Errorf("failed to find network devices at %q", netDir)
+		}
+
+		// Grab the first device from eg /sys/devices/pci0000:00/0000:00:19.0/net
+		for _, file := range files {
+			// Make sure it's really an interface
+			for _, l := range links {
+				if file.Name() == l.Attrs().Name {
+					return l, nil
+				}
+			}
+		}
+	}
+
+	return nil, fmt.Errorf("failed to find physical interface")
+}
+
+func main() {
+	skel.PluginMain(cmdAdd, cmdDel, version.All)
+}

plugins/main/host-device/host-device_suite_test.go

@@ -1,4 +1,4 @@
-// Copyright 2015-2017 CNI authors
+// Copyright 2016 CNI authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,23 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// +build !linux
-
-package ip
+package main
 
 import (
-	"net"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
 
-	"github.com/containernetworking/cni/pkg/types"
-	"github.com/vishvananda/netlink"
+	"testing"
 )
 
-// AddRoute adds a universally-scoped route to a device.
-func AddRoute(ipn *net.IPNet, gw net.IP, dev netlink.Link) error {
-	return types.NotImplementedError
-}
-
-// AddHostRoute adds a host-scoped route to a device.
-func AddHostRoute(ipn *net.IPNet, gw net.IP, dev netlink.Link) error {
-	return types.NotImplementedError
+func TestVlan(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "host-device Suite")
 }

plugins/main/host-device/host-device_test.go

@@ -0,0 +1,155 @@
+// Copyright 2017 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"fmt"
+	"math/rand"
+
+	"github.com/containernetworking/cni/pkg/skel"
+	"github.com/containernetworking/cni/pkg/types"
+	"github.com/containernetworking/cni/pkg/types/current"
+	"github.com/containernetworking/plugins/pkg/ns"
+	"github.com/containernetworking/plugins/pkg/testutils"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/vishvananda/netlink"
+)
+
+var _ = Describe("base functionality", func() {
+	var originalNS ns.NetNS
+	var ifname string
+
+	BeforeEach(func() {
+		var err error
+		originalNS, err = ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+
+		ifname = fmt.Sprintf("dummy-%x", rand.Int31())
+	})
+
+	AfterEach(func() {
+		originalNS.Close()
+	})
+
+	It("Works with a valid config", func() {
+		var origLink netlink.Link
+
+		// prepare ifname in original namespace
+		err := originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+			err := netlink.LinkAdd(&netlink.Dummy{
+				LinkAttrs: netlink.LinkAttrs{
+					Name: ifname,
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+			origLink, err = netlink.LinkByName(ifname)
+			Expect(err).NotTo(HaveOccurred())
+			err = netlink.LinkSetUp(origLink)
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// call CmdAdd
+		targetNS, err := ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+
+		CNI_IFNAME := "eth0"
+		conf := fmt.Sprintf(`{
+			"cniVersion": "0.3.0",
+			"name": "cni-plugin-host-device-test",
+			"type": "host-device",
+			"device": %q
+		}`, ifname)
+		args := &skel.CmdArgs{
+			ContainerID: "dummy",
+			Netns:       targetNS.Path(),
+			IfName:      CNI_IFNAME,
+			StdinData:   []byte(conf),
+		}
+		var resI types.Result
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+			var err error
+			resI, _, err = testutils.CmdAddWithResult(targetNS.Path(), CNI_IFNAME, []byte(conf), func() error { return cmdAdd(args) })
+			return err
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// check that the result was sane
+		res, err := current.NewResultFromResult(resI)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(res.Interfaces).To(Equal([]*current.Interface{
+			{
+				Name:    CNI_IFNAME,
+				Mac:     origLink.Attrs().HardwareAddr.String(),
+				Sandbox: targetNS.Path(),
+			},
+		}))
+
+		// assert that dummy0 is now in the target namespace
+		err = targetNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+			link, err := netlink.LinkByName(CNI_IFNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr))
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// assert that dummy0 is now NOT in the original namespace anymore
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+			_, err := netlink.LinkByName(ifname)
+			Expect(err).To(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Check that deleting the device moves it back and restores the name
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+			err = testutils.CmdDelWithResult(targetNS.Path(), CNI_IFNAME, func() error { return cmdDel(args) })
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err := netlink.LinkByName(ifname)
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+	})
+
+	It("fails an invalid config", func() {
+		conf := `{
+			"cniVersion": "0.3.0",
+			"name": "cni-plugin-host-device-test",
+			"type": "host-device"
+		}`
+
+		args := &skel.CmdArgs{
+			ContainerID: "dummy",
+			Netns:       originalNS.Path(),
+			IfName:      ifname,
+			StdinData:   []byte(conf),
+		}
+		_, _, err := testutils.CmdAddWithResult(originalNS.Path(), ifname, []byte(conf), func() error { return cmdAdd(args) })
+		Expect(err).To(MatchError(`specify either "device", "hwaddr" or "kernelpath"`))
+
+	})
+
+})

plugins/main/ipvlan/README.md

@@ -27,10 +27,10 @@ Because all ipvlan interfaces share the MAC address with the host interface, DHC
 
 * `name` (string, required): the name of the network.
 * `type` (string, required): "ipvlan".
-* `master` (string, required): name of the host interface to enslave.
-* `mode` (string, optional): one of "l2", "l3". Defaults to "l2".
+* `master` (string, required unless chained): name of the host interface to enslave.
+* `mode` (string, optional): one of "l2", "l3", "l3s". Defaults to "l2".
 * `mtu` (integer, optional): explicitly set MTU to the specified value. Defaults to the value chosen by the kernel.
-* `ipam` (dictionary, required): IPAM configuration to be used for this network.
+* `ipam` (dictionary, required unless chained): IPAM configuration to be used for this network.
 
 ## Notes
 
@@ -38,3 +38,8 @@ Because all ipvlan interfaces share the MAC address with the host interface, DHC
 Therefore the container will not be able to reach the host via `ipvlan` interface.
 Be sure to also have container join a network that provides connectivity to the host (e.g. `ptp`).
 * A single master interface can not be enslaved by both `macvlan` and `ipvlan`.
+* For IP allocation schemes that cannot be interface agnostic, the ipvlan plugin
+can be chained with an earlier plugin that handles this logic. If `master` is
+omitted, then the previous Result must contain a single interface name for the
+ipvlan plugin to enslave. If `ipam` is omitted, then the previous Result is used
+to configure the ipvlan interface.

plugins/main/ipvlan/ipvlan.go

@@ -32,6 +32,12 @@ import (
 
 type NetConf struct {
 	types.NetConf
+
+	// support chaining for master interface and IP decisions
+	// occurring prior to running ipvlan plugin
+	RawPrevResult *map[string]interface{} `json:"prevResult"`
+	PrevResult    *current.Result         `json:"-"`
+
 	Master string `json:"master"`
 	Mode   string `json:"mode"`
 	MTU    int    `json:"mtu"`
@@ -49,8 +55,31 @@ func loadConf(bytes []byte) (*NetConf, string, error) {
 	if err := json.Unmarshal(bytes, n); err != nil {
 		return nil, "", fmt.Errorf("failed to load netconf: %v", err)
 	}
+	// Parse previous result
+	if n.RawPrevResult != nil {
+		resultBytes, err := json.Marshal(n.RawPrevResult)
+		if err != nil {
+			return nil, "", fmt.Errorf("could not serialize prevResult: %v", err)
+		}
+		res, err := version.NewResult(n.CNIVersion, resultBytes)
+		if err != nil {
+			return nil, "", fmt.Errorf("could not parse prevResult: %v", err)
+		}
+		n.RawPrevResult = nil
+		n.PrevResult, err = current.NewResultFromResult(res)
+		if err != nil {
+			return nil, "", fmt.Errorf("could not convert result to current version: %v", err)
+		}
+	}
 	if n.Master == "" {
-		return nil, "", fmt.Errorf(`"master" field is required. It specifies the host interface name to virtualize`)
+		if n.PrevResult == nil {
+			return nil, "", fmt.Errorf(`"master" field is required. It specifies the host interface name to virtualize`)
+		}
+		if len(n.PrevResult.Interfaces) == 1 && n.PrevResult.Interfaces[0].Name != "" {
+			n.Master = n.PrevResult.Interfaces[0].Name
+		} else {
+			return nil, "", fmt.Errorf("chained master failure. PrevResult lacks a single named interface")
+		}
 	}
 	return n, n.CNIVersion, nil
 }
@@ -143,19 +172,26 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
-	// run the IPAM plugin and get back the config to apply
-	r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData)
-	if err != nil {
-		return err
-	}
-	// Convert whatever the IPAM result was into the current Result type
-	result, err := current.NewResultFromResult(r)
-	if err != nil {
-		return err
-	}
+	var result *current.Result
+	// Configure iface from PrevResult if we have IPs and an IPAM
+	// block has not been configured
+	if n.IPAM.Type == "" && n.PrevResult != nil && len(n.PrevResult.IPs) > 0 {
+		result = n.PrevResult
+	} else {
+		// run the IPAM plugin and get back the config to apply
+		r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData)
+		if err != nil {
+			return err
+		}
+		// Convert whatever the IPAM result was into the current Result type
+		result, err = current.NewResultFromResult(r)
+		if err != nil {
+			return err
+		}
 
-	if len(result.IPs) == 0 {
-		return errors.New("IPAM plugin returned missing IP config")
+		if len(result.IPs) == 0 {
+			return errors.New("IPAM plugin returned missing IP config")
+		}
 	}
 	for _, ipc := range result.IPs {
 		// All addresses belong to the ipvlan interface
@@ -182,9 +218,12 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
-	err = ipam.ExecDel(n.IPAM.Type, args.StdinData)
-	if err != nil {
-		return err
+	// On chained invocation, IPAM block can be empty
+	if n.IPAM.Type != "" {
+		err = ipam.ExecDel(n.IPAM.Type, args.StdinData)
+		if err != nil {
+			return err
+		}
 	}
 
 	if args.Netns == "" {
@@ -194,7 +233,7 @@ func cmdDel(args *skel.CmdArgs) error {
 	// There is a netns so try to clean up. Delete can be called multiple times
 	// so don't return an error if the device is already removed.
 	err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
-		if _, err := ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4); err != nil {
+		if err := ip.DelLinkByName(args.IfName); err != nil {
 			if err != ip.ErrLinkNotFound {
 				return err
 			}

plugins/main/ipvlan/ipvlan_test.go

@@ -33,6 +33,79 @@ import (
 
 const MASTER_NAME = "eth0"
 
+func ipvlanAddDelTest(conf, IFNAME string, originalNS ns.NetNS) {
+	targetNs, err := ns.NewNS()
+	Expect(err).NotTo(HaveOccurred())
+	defer targetNs.Close()
+
+	args := &skel.CmdArgs{
+		ContainerID: "dummy",
+		Netns:       targetNs.Path(),
+		IfName:      IFNAME,
+		StdinData:   []byte(conf),
+	}
+
+	var result *current.Result
+	err = originalNS.Do(func(ns.NetNS) error {
+		defer GinkgoRecover()
+
+		r, _, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, []byte(conf), func() error {
+			return cmdAdd(args)
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		result, err = current.GetResult(r)
+		Expect(err).NotTo(HaveOccurred())
+
+		Expect(len(result.Interfaces)).To(Equal(1))
+		Expect(result.Interfaces[0].Name).To(Equal(IFNAME))
+		Expect(len(result.IPs)).To(Equal(1))
+		return nil
+	})
+	Expect(err).NotTo(HaveOccurred())
+
+	// Make sure ipvlan link exists in the target namespace
+	err = targetNs.Do(func(ns.NetNS) error {
+		defer GinkgoRecover()
+
+		link, err := netlink.LinkByName(IFNAME)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(link.Attrs().Name).To(Equal(IFNAME))
+
+		hwaddr, err := net.ParseMAC(result.Interfaces[0].Mac)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(link.Attrs().HardwareAddr).To(Equal(hwaddr))
+
+		addrs, err := netlink.AddrList(link, syscall.AF_INET)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(addrs)).To(Equal(1))
+		return nil
+	})
+	Expect(err).NotTo(HaveOccurred())
+
+	err = originalNS.Do(func(ns.NetNS) error {
+		defer GinkgoRecover()
+
+		err = testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error {
+			return cmdDel(args)
+		})
+		Expect(err).NotTo(HaveOccurred())
+		return nil
+	})
+	Expect(err).NotTo(HaveOccurred())
+
+	// Make sure ipvlan link has been deleted
+	err = targetNs.Do(func(ns.NetNS) error {
+		defer GinkgoRecover()
+
+		link, err := netlink.LinkByName(IFNAME)
+		Expect(err).To(HaveOccurred())
+		Expect(link).To(BeNil())
+		return nil
+	})
+	Expect(err).NotTo(HaveOccurred())
+}
+
 var _ = Describe("ipvlan Operations", func() {
 	var originalNS ns.NetNS
 
@@ -116,76 +189,35 @@ var _ = Describe("ipvlan Operations", func() {
     }
 }`, MASTER_NAME)
 
-		targetNs, err := ns.NewNS()
-		Expect(err).NotTo(HaveOccurred())
-		defer targetNs.Close()
+		ipvlanAddDelTest(conf, IFNAME, originalNS)
+	})
 
-		args := &skel.CmdArgs{
-			ContainerID: "dummy",
-			Netns:       targetNs.Path(),
-			IfName:      IFNAME,
-			StdinData:   []byte(conf),
-		}
+	It("configures and deconfigures an iplvan link with ADD/DEL when chained", func() {
+		const IFNAME = "ipvl0"
 
-		var result *current.Result
-		err = originalNS.Do(func(ns.NetNS) error {
-			defer GinkgoRecover()
+		conf := fmt.Sprintf(`{
+    "cniVersion": "0.3.1",
+    "name": "mynet",
+    "type": "ipvlan",
+    "prevResult": {
+            "interfaces": [
+                    {
+                            "name": "%s"
+                    }
+            ],
+            "ips": [
+                    {
+                            "version": "4",
+                            "address": "10.1.2.2/24",
+                            "gateway": "10.1.2.1",
+                            "interface": 0
+                    }
+            ],
+            "routes": []
+    }
+}`, MASTER_NAME)
 
-			r, _, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, []byte(conf), func() error {
-				return cmdAdd(args)
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			result, err = current.GetResult(r)
-			Expect(err).NotTo(HaveOccurred())
-
-			Expect(len(result.Interfaces)).To(Equal(1))
-			Expect(result.Interfaces[0].Name).To(Equal(IFNAME))
-			Expect(len(result.IPs)).To(Equal(1))
-			return nil
-		})
-		Expect(err).NotTo(HaveOccurred())
-
-		// Make sure ipvlan link exists in the target namespace
-		err = targetNs.Do(func(ns.NetNS) error {
-			defer GinkgoRecover()
-
-			link, err := netlink.LinkByName(IFNAME)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(link.Attrs().Name).To(Equal(IFNAME))
-
-			hwaddr, err := net.ParseMAC(result.Interfaces[0].Mac)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(link.Attrs().HardwareAddr).To(Equal(hwaddr))
-
-			addrs, err := netlink.AddrList(link, syscall.AF_INET)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(len(addrs)).To(Equal(1))
-			return nil
-		})
-		Expect(err).NotTo(HaveOccurred())
-
-		err = originalNS.Do(func(ns.NetNS) error {
-			defer GinkgoRecover()
-
-			err = testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error {
-				return cmdDel(args)
-			})
-			Expect(err).NotTo(HaveOccurred())
-			return nil
-		})
-		Expect(err).NotTo(HaveOccurred())
-
-		// Make sure ipvlan link has been deleted
-		err = targetNs.Do(func(ns.NetNS) error {
-			defer GinkgoRecover()
-
-			link, err := netlink.LinkByName(IFNAME)
-			Expect(err).To(HaveOccurred())
-			Expect(link).To(BeNil())
-			return nil
-		})
-		Expect(err).NotTo(HaveOccurred())
+		ipvlanAddDelTest(conf, IFNAME, originalNS)
 	})
 
 	It("deconfigures an unconfigured ipvlan link with DEL", func() {

plugins/main/macvlan/macvlan.go

@@ -161,11 +161,28 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
+	// Delete link if err to avoid link leak in this ns
+	defer func() {
+		if err != nil {
+			netns.Do(func(_ ns.NetNS) error {
+				return ip.DelLinkByName(args.IfName)
+			})
+		}
+	}()
+
 	// run the IPAM plugin and get back the config to apply
 	r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData)
 	if err != nil {
 		return err
 	}
+
+	// Invoke ipam del if err to avoid ip leak
+	defer func() {
+		if err != nil {
+			ipam.ExecDel(n.IPAM.Type, args.StdinData)
+		}
+	}()
+
 	// Convert whatever the IPAM result was into the current Result type
 	result, err := current.NewResultFromResult(r)
 	if err != nil {
@@ -226,7 +243,7 @@ func cmdDel(args *skel.CmdArgs) error {
 	// There is a netns so try to clean up. Delete can be called multiple times
 	// so don't return an error if the device is already removed.
 	err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
-		if _, err := ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4); err != nil {
+		if err := ip.DelLinkByName(args.IfName); err != nil {
 			if err != ip.ErrLinkNotFound {
 				return err
 			}

plugins/main/ptp/ptp.go

@@ -259,10 +259,10 @@ func cmdDel(args *skel.CmdArgs) error {
 	// There is a netns so try to clean up. Delete can be called multiple times
 	// so don't return an error if the device is already removed.
 	// If the device isn't there then don't try to clean up IP masq either.
-	var ipn *net.IPNet
+	var ipnets []*net.IPNet
 	err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		var err error
-		ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4)
+		ipnets, err = ip.DelLinkByNameAddr(args.IfName)
 		if err != nil && err == ip.ErrLinkNotFound {
 			return nil
 		}
@@ -273,10 +273,12 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
-	if ipn != nil && conf.IPMasq {
+	if len(ipnets) != 0 && conf.IPMasq {
 		chain := utils.FormatChainName(conf.Name, args.ContainerID)
 		comment := utils.FormatComment(conf.Name, args.ContainerID)
-		err = ip.TeardownIPMasq(ipn, chain, comment)
+		for _, ipn := range ipnets {
+			err = ip.TeardownIPMasq(ipn, chain, comment)
+		}
 	}
 
 	return err

plugins/main/vlan/vlan.go

@@ -181,9 +181,8 @@ func cmdDel(args *skel.CmdArgs) error {
 	}
 
 	err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
-		_, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4)
-		// FIXME: use ip.ErrLinkNotFound when cni is revendored
-		if err != nil && err.Error() == "Link not found" {
+		err = ip.DelLinkByName(args.IfName)
+		if err != nil && err != ip.ErrLinkNotFound {
 			return nil
 		}
 		return err

plugins/meta/portmap/README.md

@@ -8,6 +8,8 @@ You should use this plugin as part of a network configuration list. It accepts
 the following configuration options:
 
 * `snat` - boolean, default true. If true or omitted, set up the SNAT chains
+* `markMasqBit` - int, (0-31), default 13. The mark bit to use for masquerading (see section SNAT). Cannot be set when `externalSetMarkChain` is used.
+* `externalSetMarkChain` - string, default nil. If you already have a Masquerade mark chain (e.g. Kubernetes), specify it here. This will use that instead of creating a separate chain. When this is set, `markMasqBit` must be unspecified.
 * `conditionsV4`, `conditionsV6` - array of strings. A list of arbitrary `iptables` 
 matches to add to the per-container rule. This may be useful if you wish to 
 exclude specific IPs from port-mapping
@@ -15,7 +17,7 @@ exclude specific IPs from port-mapping
 The plugin expects to receive the actual list of port mappings via the 
 `portMappings` [capability argument](https://github.com/containernetworking/cni/blob/master/CONVENTIONS.md)
 
-So a sample standalone config list (with the file extension .conflist) might
+A sample standalone config list for Kubernetes (with the file extension .conflist) might
 look like:
 
 ```json
@@ -39,21 +41,31 @@ look like:
                 {
                         "type": "portmap",
                         "capabilities": {"portMappings": true},
-                        "snat": false,
-                        "conditionsV4": ["!", "-d", "192.0.2.0/24"],
-                        "conditionsV6": ["!", "-d", "fc00::/7"]
+                        "externalSetMarkChain": "KUBE-MARK-MASQ"
                 }
         ]
 }
 ```
 
+A configuration file with all options set:
+```json
+{
+        "type": "portmap",
+        "capabilities": {"portMappings": true},
+        "snat": true,
+        "markMasqBit": 13,
+        "externalSetMarkChain": "CNI-HOSTPORT-SETMARK",
+        "conditionsV4": ["!", "-d", "192.0.2.0/24"],
+        "conditionsV6": ["!", "-d", "fc00::/7"]
+}
+```
+
 
 
 ## Rule structure
 The plugin sets up two sequences of chains and rules - one "primary" DNAT
 sequence to rewrite the destination, and one additional SNAT sequence that
-rewrites the source address for packets from localhost. The sequence is somewhat
-complex to minimize the number of rules non-forwarded packets must traverse.
+will masquerade traffic as needed.
 
 
 ### DNAT
@@ -68,50 +80,54 @@ rules look like this:
 - `--dst-type LOCAL -j CNI-HOSTPORT-DNAT`
 
 `CNI-HOSTPORT-DNAT` chain:
-- `${ConditionsV4/6} -j CNI-DN-xxxxxx` (where xxxxxx is a function of the ContainerID and network name)
+- `${ConditionsV4/6} -p tcp --destination-ports 8080,8043 -j CNI-DN-xxxxxx` (where xxxxxx is a function of the ContainerID and network name)
 
-`CNI-DN-xxxxxx` chain: 
-- `-p tcp --dport 8080 -j DNAT --to-destination 172.16.30.2:80`
+`CNI-HOSTPORT-SETMARK` chain:
+- `-j MARK --set-xmark 0x2000/0x2000`
+
+`CNI-DN-xxxxxx` chain:
+- `-p tcp -s 172.16.30.2 --dport 8080 -j CNI-HOSTPORT-SETMARK` (masquerade hairpin traffic)
+- `-p tcp -s 127.0.0.1 --dport 8080 -j CNI-HOSTPORT-SETMARK` (masquerade localhost traffic)
+- `-p tcp --dport 8080 -j DNAT --to-destination 172.16.30.2:80` (rewrite destination)
+- `-p tcp -s 172.16.30.2 --dport 8043 -j CNI-HOSTPORT-SETMARK`
+- `-p tcp -s 127.0.0.1 --dport 8043 -j CNI-HOSTPORT-SETMARK`
 - `-p tcp --dport 8043 -j DNAT --to-destination 172.16.30.2:443`
 
 New connections to the host will have to traverse every rule, so large numbers
 of port forwards may have a performance impact. This won't affect established
 connections, just the first packet.
 
-### SNAT 
-The SNAT rule enables port-forwarding from the localhost IP on the host.
-This rule rewrites (masquerades) the source address for connections from
-localhost. If this rule did not exist, a connection to `localhost:80` would
-still have a source IP of 127.0.0.1 when received by the container, so no 
-packets would respond. Again, it is a sequence of 3 chains. Because SNAT has to
-occur in the `POSTROUTING` chain, the packet has already been through the DNAT
-chain.
+### SNAT (Masquerade)
+Some packets also need to have the source address rewritten:
+* connections from localhost
+* Hairpin traffic back to the container.
+
+In the DNAT chain, a bit is set on the mark for packets that need snat. This
+chain performs that masquerading. By default, bit 13 is set, but this is
+configurable. If you are using other tools that also use the iptables mark,
+you should make sure this doesn't conflict.
+
+Some container runtimes, most notably Kubernetes, already have a set of rules
+for masquerading when a specific mark bit is set. If so enabled, the plugin
+will use that chain instead.
 
 `POSTROUTING`:
-- `-s 127.0.0.1 ! -d 127.0.0.1 -j CNI-HOSTPORT-SNAT`
+- `-j CNI-HOSTPORT-MASQ`
 
-`CNI-HOSTPORT-SNAT`:
-- `-j CNI-SN-xxxxx`
-
-`CNI-SN-xxxxx`:
-- `-p tcp -s 127.0.0.1 -d 172.16.30.2 --dport 80 -j MASQUERADE`
-- `-p tcp -s 127.0.0.1 -d 172.16.30.2 --dport 443 -j MASQUERADE`
-
-Only new connections from the host, where the source address is 127.0.0.1 but
-not the destination will traverse this chain. It is unlikely that any packets
-will reach these rules without being SNATted, so the cost should be minimal.
+`CNI-HOSTPORT-MASQ`:
+- `--mark 0x2000 -j MASQUERADE`
 
 Because MASQUERADE happens in POSTROUTING, it means that packets with source ip
-127.0.0.1 need to pass a routing boundary. By default, that is not allowed
-in Linux. So, need to enable the sysctl `net.ipv4.conf.IFNAME.route_localnet`,
-where IFNAME is the name of the host-side interface that routes traffic to the
-container.
+127.0.0.1 need to first pass a routing boundary before being masqueraded. By
+default, that is not allowed in Linux. So, the plugin needs to enable the sysctl
+`net.ipv4.conf.IFNAME.route_localnet`, where IFNAME is the name of the host-side
+interface that routes traffic to the container.
 
-There is no equivalent to `route_localnet` for ipv6, so SNAT does not work
-for ipv6. If you need port forwarding from localhost, your container must have
-an ipv4 address.
+There is no equivalent to `route_localnet` for ipv6, so connections to ::1
+will not be portmapped for ipv6. If you need port forwarding from localhost,
+your container must have an ipv4 address.
 
 
 ## Known issues
 - ipsets could improve efficiency
-- SNAT does not work with ipv6.
+- forwarding from localhost does not work with ipv6.

plugins/meta/portmap/chain.go

@@ -25,12 +25,14 @@ import (
 type chain struct {
 	table       string
 	name        string
-	entryRule   []string // the rule that enters this chain
 	entryChains []string // the chains to add the entry rule
+
+	entryRules [][]string // the rules that "point" to this chain
+	rules      [][]string // the rules this chain contains
 }
 
 // setup idempotently creates the chain. It will not error if the chain exists.
-func (c *chain) setup(ipt *iptables.IPTables, rules [][]string) error {
+func (c *chain) setup(ipt *iptables.IPTables) error {
 	// create the chain
 	exists, err := chainExists(ipt, c.table, c.name)
 	if err != nil {
@@ -43,17 +45,21 @@ func (c *chain) setup(ipt *iptables.IPTables, rules [][]string) error {
 	}
 
 	// Add the rules to the chain
-	for i := len(rules) - 1; i >= 0; i-- {
-		if err := prependUnique(ipt, c.table, c.name, rules[i]); err != nil {
+	for i := len(c.rules) - 1; i >= 0; i-- {
+		if err := prependUnique(ipt, c.table, c.name, c.rules[i]); err != nil {
 			return err
 		}
 	}
 
-	// Add the entry rules
-	entryRule := append(c.entryRule, "-j", c.name)
+	// Add the entry rules to the entry chains
 	for _, entryChain := range c.entryChains {
-		if err := prependUnique(ipt, c.table, entryChain, entryRule); err != nil {
-			return err
+		for i := len(c.entryRules) - 1; i >= 0; i-- {
+			r := []string{}
+			r = append(r, c.entryRules[i]...)
+			r = append(r, "-j", c.name)
+			if err := prependUnique(ipt, c.table, entryChain, r); err != nil {
+				return err
+			}
 		}
 	}
 

plugins/meta/portmap/chain_test.go

@@ -49,8 +49,12 @@ var _ = Describe("chain tests", func() {
 		testChain = chain{
 			table:       TABLE,
 			name:        chainName,
-			entryRule:   []string{"-d", "203.0.113.1"},
 			entryChains: []string{tlChainName},
+			entryRules:  [][]string{{"-d", "203.0.113.1"}},
+			rules: [][]string{
+				{"-m", "comment", "--comment", "test 1", "-j", "RETURN"},
+				{"-m", "comment", "--comment", "test 2", "-j", "RETURN"},
+			},
 		}
 
 		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4)
@@ -90,11 +94,7 @@ var _ = Describe("chain tests", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		// Create the chain
-		chainRules := [][]string{
-			{"-m", "comment", "--comment", "test 1", "-j", "RETURN"},
-			{"-m", "comment", "--comment", "test 2", "-j", "RETURN"},
-		}
-		err = testChain.setup(ipt, chainRules)
+		err = testChain.setup(ipt)
 		Expect(err).NotTo(HaveOccurred())
 
 		// Verify the chain exists
@@ -151,15 +151,11 @@ var _ = Describe("chain tests", func() {
 	It("creates chains idempotently", func() {
 		defer cleanup()
 
-		// Create the chain
-		chainRules := [][]string{
-			{"-m", "comment", "--comment", "test", "-j", "RETURN"},
-		}
-		err := testChain.setup(ipt, chainRules)
+		err := testChain.setup(ipt)
 		Expect(err).NotTo(HaveOccurred())
 
 		// Create it again!
-		err = testChain.setup(ipt, chainRules)
+		err = testChain.setup(ipt)
 		Expect(err).NotTo(HaveOccurred())
 
 		// Make sure there are only two rules
@@ -167,18 +163,14 @@ var _ = Describe("chain tests", func() {
 		rules, err := ipt.List(TABLE, testChain.name)
 		Expect(err).NotTo(HaveOccurred())
 
-		Expect(len(rules)).To(Equal(2))
+		Expect(len(rules)).To(Equal(3))
 
 	})
 
 	It("deletes chains idempotently", func() {
 		defer cleanup()
 
-		// Create the chain
-		chainRules := [][]string{
-			{"-m", "comment", "--comment", "test", "-j", "RETURN"},
-		}
-		err := testChain.setup(ipt, chainRules)
+		err := testChain.setup(ipt)
 		Expect(err).NotTo(HaveOccurred())
 
 		err = testChain.teardown(ipt)

plugins/meta/portmap/main.go

@@ -47,10 +47,12 @@ type PortMapEntry struct {
 
 type PortMapConf struct {
 	types.NetConf
-	SNAT          *bool     `json:"snat,omitempty"`
-	ConditionsV4  *[]string `json:"conditionsV4"`
-	ConditionsV6  *[]string `json:"conditionsV6"`
-	RuntimeConfig struct {
+	SNAT                 *bool     `json:"snat,omitempty"`
+	ConditionsV4         *[]string `json:"conditionsV4"`
+	ConditionsV6         *[]string `json:"conditionsV6"`
+	MarkMasqBit          *int      `json:"markMasqBit"`
+	ExternalSetMarkChain *string   `json:"externalSetMarkChain"`
+	RuntimeConfig        struct {
 		PortMaps []PortMapEntry `json:"portMappings,omitempty"`
 	} `json:"runtimeConfig,omitempty"`
 	RawPrevResult map[string]interface{} `json:"prevResult,omitempty"`
@@ -63,6 +65,10 @@ type PortMapConf struct {
 	ContIPv6    net.IP `json:"-"`
 }
 
+// The default mark bit to signal that masquerading is required
+// Kubernetes uses 14 and 15, Calico uses 20-31.
+const DefaultMarkBit = 13
+
 func cmdAdd(args *skel.CmdArgs) error {
 	netConf, err := parseConfig(args.StdinData, args.IfName)
 	if err != nil {
@@ -145,6 +151,19 @@ func parseConfig(stdin []byte, ifName string) (*PortMapConf, error) {
 		conf.SNAT = &tvar
 	}
 
+	if conf.MarkMasqBit != nil && conf.ExternalSetMarkChain != nil {
+		return nil, fmt.Errorf("Cannot specify externalSetMarkChain and markMasqBit")
+	}
+
+	if conf.MarkMasqBit == nil {
+		bvar := DefaultMarkBit // go constants are "special"
+		conf.MarkMasqBit = &bvar
+	}
+
+	if *conf.MarkMasqBit < 0 || *conf.MarkMasqBit > 31 {
+		return nil, fmt.Errorf("MasqMarkBit must be between 0 and 31")
+	}
+
 	// Reject invalid port numbers
 	for _, pm := range conf.RuntimeConfig.PortMaps {
 		if pm.ContainerPort <= 0 {

plugins/meta/portmap/portmap.go

@@ -17,6 +17,7 @@ package main
 import (
 	"fmt"
 	"net"
+	"sort"
 	"strconv"
 
 	"github.com/containernetworking/plugins/pkg/utils/sysctl"
@@ -24,33 +25,26 @@ import (
 )
 
 // This creates the chains to be added to iptables. The basic structure is
-// a bit complex for efficiencies sake. We create 2 chains: a summary chain
+// a bit complex for efficiency's sake. We create 2 chains: a summary chain
 // that is shared between invocations, and an invocation (container)-specific
 // chain. This minimizes the number of operations on the top level, but allows
 // for easy cleanup.
 //
-// We also create DNAT chains to rewrite destinations, and SNAT chains so that
-// connections to localhost work.
-//
 // The basic setup (all operations are on the nat table) is:
 //
 // DNAT case (rewrite destination IP and port):
-// PREROUTING, OUTPUT: --dst-type local -j CNI-HOSTPORT_DNAT
-// CNI-HOSTPORT-DNAT: -j CNI-DN-abcd123
+// PREROUTING, OUTPUT: --dst-type local -j CNI-HOSTPORT-DNAT
+// CNI-HOSTPORT-DNAT: --destination-ports 8080,8081 -j CNI-DN-abcd123
 // CNI-DN-abcd123: -p tcp --dport 8080 -j DNAT --to-destination 192.0.2.33:80
 // CNI-DN-abcd123: -p tcp --dport 8081 -j DNAT ...
-//
-// SNAT case (rewrite source IP from localhost after dnat):
-// POSTROUTING: -s 127.0.0.1 ! -d 127.0.0.1 -j CNI-HOSTPORT-SNAT
-// CNI-HOSTPORT-SNAT: -j CNI-SN-abcd123
-// CNI-SN-abcd123: -p tcp -s 127.0.0.1 -d 192.0.2.33 --dport 80 -j MASQUERADE
-// CNI-SN-abcd123: -p tcp -s 127.0.0.1 -d 192.0.2.33 --dport 90 -j MASQUERADE
 
 // The names of the top-level summary chains.
 // These should never be changed, or else upgrading will require manual
 // intervention.
 const TopLevelDNATChainName = "CNI-HOSTPORT-DNAT"
-const TopLevelSNATChainName = "CNI-HOSTPORT-SNAT"
+const SetMarkChainName = "CNI-HOSTPORT-SETMARK"
+const MarkMasqChainName = "CNI-HOSTPORT-MASQ"
+const OldTopLevelSNATChainName = "CNI-HOSTPORT-SNAT"
 
 // forwardPorts establishes port forwarding to a given container IP.
 // containerIP can be either v4 or v6.
@@ -59,48 +53,35 @@ func forwardPorts(config *PortMapConf, containerIP net.IP) error {
 
 	var ipt *iptables.IPTables
 	var err error
-	var conditions *[]string
 
 	if isV6 {
 		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6)
-		conditions = config.ConditionsV6
 	} else {
 		ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4)
-		conditions = config.ConditionsV4
 	}
 	if err != nil {
 		return fmt.Errorf("failed to open iptables: %v", err)
 	}
 
-	toplevelDnatChain := genToplevelDnatChain()
-	if err := toplevelDnatChain.setup(ipt, nil); err != nil {
-		return fmt.Errorf("failed to create top-level DNAT chain: %v", err)
-	}
+	// Enable masquerading for traffic as necessary.
+	// The DNAT chain sets a mark bit for traffic that needs masq:
+	// - connections from localhost
+	// - hairpin traffic back to the container
+	// Idempotently create the rule that masquerades traffic with this mark.
+	// Need to do this first; the DNAT rules reference these chains
+	if *config.SNAT {
+		if config.ExternalSetMarkChain == nil {
+			setMarkChain := genSetMarkChain(*config.MarkMasqBit)
+			if err := setMarkChain.setup(ipt); err != nil {
+				return fmt.Errorf("unable to create chain %s: %v", setMarkChain.name, err)
+			}
 
-	dnatChain := genDnatChain(config.Name, config.ContainerID, conditions)
-	_ = dnatChain.teardown(ipt) // If we somehow collide on this container ID + network, cleanup
-
-	dnatRules := dnatRules(config.RuntimeConfig.PortMaps, containerIP)
-	if err := dnatChain.setup(ipt, dnatRules); err != nil {
-		return fmt.Errorf("unable to setup DNAT: %v", err)
-	}
-
-	// Enable SNAT for connections to localhost.
-	// This won't work for ipv6, since the kernel doesn't have the equvalent
-	// route_localnet sysctl.
-	if *config.SNAT && !isV6 {
-		toplevelSnatChain := genToplevelSnatChain(isV6)
-		if err := toplevelSnatChain.setup(ipt, nil); err != nil {
-			return fmt.Errorf("failed to create top-level SNAT chain: %v", err)
+			masqChain := genMarkMasqChain(*config.MarkMasqBit)
+			if err := masqChain.setup(ipt); err != nil {
+				return fmt.Errorf("unable to create chain %s: %v", setMarkChain.name, err)
+			}
 		}
 
-		snatChain := genSnatChain(config.Name, config.ContainerID)
-		_ = snatChain.teardown(ipt)
-
-		snatRules := snatRules(config.RuntimeConfig.PortMaps, containerIP)
-		if err := snatChain.setup(ipt, snatRules); err != nil {
-			return fmt.Errorf("unable to setup SNAT: %v", err)
-		}
 		if !isV6 {
 			// Set the route_localnet bit on the host interface, so that
 			// 127/8 can cross a routing boundary.
@@ -113,6 +94,20 @@ func forwardPorts(config *PortMapConf, containerIP net.IP) error {
 		}
 	}
 
+	// Generate the DNAT (actual port forwarding) rules
+	toplevelDnatChain := genToplevelDnatChain()
+	if err := toplevelDnatChain.setup(ipt); err != nil {
+		return fmt.Errorf("failed to create top-level DNAT chain: %v", err)
+	}
+
+	dnatChain := genDnatChain(config.Name, config.ContainerID)
+	// First, idempotently tear down this chain in case there was some
+	// sort of collision or bad state.
+	fillDnatRules(&dnatChain, config, containerIP)
+	if err := dnatChain.setup(ipt); err != nil {
+		return fmt.Errorf("unable to setup DNAT: %v", err)
+	}
+
 	return nil
 }
 
@@ -124,106 +119,153 @@ func genToplevelDnatChain() chain {
 	return chain{
 		table: "nat",
 		name:  TopLevelDNATChainName,
-		entryRule: []string{
+		entryRules: [][]string{{
 			"-m", "addrtype",
 			"--dst-type", "LOCAL",
-		},
+		}},
 		entryChains: []string{"PREROUTING", "OUTPUT"},
 	}
 }
 
 // genDnatChain creates the per-container chain.
 // Conditions are any static entry conditions for the chain.
-func genDnatChain(netName, containerID string, conditions *[]string) chain {
-	name := formatChainName("DN-", netName, containerID)
-	comment := fmt.Sprintf(`dnat name: "%s" id: "%s"`, netName, containerID)
-
-	ch := chain{
-		table: "nat",
-		name:  name,
-		entryRule: []string{
-			"-m", "comment",
-			"--comment", comment,
-		},
+func genDnatChain(netName, containerID string) chain {
+	return chain{
+		table:       "nat",
+		name:        formatChainName("DN-", netName, containerID),
 		entryChains: []string{TopLevelDNATChainName},
 	}
-	if conditions != nil && len(*conditions) != 0 {
-		ch.entryRule = append(ch.entryRule, *conditions...)
-	}
-
-	return ch
 }
 
 // dnatRules generates the destination NAT rules, one per port, to direct
 // traffic from hostip:hostport to podip:podport
-func dnatRules(entries []PortMapEntry, containerIP net.IP) [][]string {
-	out := make([][]string, 0, len(entries))
+func fillDnatRules(c *chain, config *PortMapConf, containerIP net.IP) {
+	isV6 := (containerIP.To4() == nil)
+	comment := trimComment(fmt.Sprintf(`dnat name: "%s" id: "%s"`, config.Name, config.ContainerID))
+	entries := config.RuntimeConfig.PortMaps
+	setMarkChainName := SetMarkChainName
+	if config.ExternalSetMarkChain != nil {
+		setMarkChainName = *config.ExternalSetMarkChain
+	}
+
+	//Generate the dnat entry rules. We'll use multiport, but it ony accepts
+	// up to 15 rules, so partition the list if needed.
+	// Do it in a stable order for testing
+	protoPorts := groupByProto(entries)
+	protos := []string{}
+	for proto := range protoPorts {
+		protos = append(protos, proto)
+	}
+	sort.Strings(protos)
+	for _, proto := range protos {
+		for _, portSpec := range splitPortList(protoPorts[proto]) {
+			r := []string{
+				"-m", "comment",
+				"--comment", comment,
+				"-m", "multiport",
+				"-p", proto,
+				"--destination-ports", portSpec,
+			}
+
+			if isV6 && config.ConditionsV6 != nil && len(*config.ConditionsV6) > 0 {
+				r = append(r, *config.ConditionsV6...)
+			} else if !isV6 && config.ConditionsV4 != nil && len(*config.ConditionsV4) > 0 {
+				r = append(r, *config.ConditionsV4...)
+			}
+			c.entryRules = append(c.entryRules, r)
+		}
+	}
+
+	// For every entry, generate 3 rules:
+	// - mark hairpin for masq
+	// - mark localhost for masq (for v4)
+	// - do dnat
+	// the ordering is important here; the mark rules must be first.
+	c.rules = make([][]string, 0, 3*len(entries))
 	for _, entry := range entries {
-		rule := []string{
+		ruleBase := []string{
 			"-p", entry.Protocol,
 			"--dport", strconv.Itoa(entry.HostPort)}
-
 		if entry.HostIP != "" {
-			rule = append(rule,
+			ruleBase = append(ruleBase,
 				"-d", entry.HostIP)
 		}
 
-		rule = append(rule,
+		// Add mark-to-masquerade rules for hairpin and localhost
+		if *config.SNAT {
+			// hairpin
+			hpRule := make([]string, len(ruleBase), len(ruleBase)+4)
+			copy(hpRule, ruleBase)
+
+			hpRule = append(hpRule,
+				"-s", containerIP.String(),
+				"-j", setMarkChainName,
+			)
+			c.rules = append(c.rules, hpRule)
+
+			if !isV6 {
+				// localhost
+				localRule := make([]string, len(ruleBase), len(ruleBase)+4)
+				copy(localRule, ruleBase)
+
+				localRule = append(localRule,
+					"-s", "127.0.0.1",
+					"-j", setMarkChainName,
+				)
+				c.rules = append(c.rules, localRule)
+			}
+		}
+
+		// The actual dnat rule
+		dnatRule := make([]string, len(ruleBase), len(ruleBase)+4)
+		copy(dnatRule, ruleBase)
+		dnatRule = append(dnatRule,
 			"-j", "DNAT",
-			"--to-destination", fmtIpPort(containerIP, entry.ContainerPort))
-
-		out = append(out, rule)
-	}
-	return out
-}
-
-// genToplevelSnatChain creates the top-level summary snat chain.
-// IMPORTANT: do not change this, or else upgrading plugins will require
-// manual intervention
-func genToplevelSnatChain(isV6 bool) chain {
-	return chain{
-		table: "nat",
-		name:  TopLevelSNATChainName,
-		entryRule: []string{
-			"-s", localhostIP(isV6),
-			"!", "-d", localhostIP(isV6),
-		},
-		entryChains: []string{"POSTROUTING"},
+			"--to-destination", fmtIpPort(containerIP, entry.ContainerPort),
+		)
+		c.rules = append(c.rules, dnatRule)
 	}
 }
 
-// genSnatChain creates the snat (localhost) chain for this container.
-func genSnatChain(netName, containerID string) chain {
-	name := formatChainName("SN-", netName, containerID)
-	comment := fmt.Sprintf(`snat name: "%s" id: "%s"`, netName, containerID)
-
-	return chain{
+// genSetMarkChain creates the SETMARK chain - the chain that sets the
+// "to-be-masqueraded" mark and returns.
+// Chains are idempotent, so we'll always create this.
+func genSetMarkChain(markBit int) chain {
+	markValue := 1 << uint(markBit)
+	markDef := fmt.Sprintf("%#x/%#x", markValue, markValue)
+	ch := chain{
 		table: "nat",
-		name:  name,
-		entryRule: []string{
+		name:  SetMarkChainName,
+		rules: [][]string{{
 			"-m", "comment",
-			"--comment", comment,
-		},
-		entryChains: []string{TopLevelSNATChainName},
+			"--comment", "CNI portfwd masquerade mark",
+			"-j", "MARK",
+			"--set-xmark", markDef,
+		}},
 	}
+	return ch
 }
 
-// snatRules sets up masquerading for connections to localhost:hostport,
-// rewriting the source so that returning packets are correct.
-func snatRules(entries []PortMapEntry, containerIP net.IP) [][]string {
-	isV6 := (containerIP.To4() == nil)
-
-	out := make([][]string, 0, len(entries))
-	for _, entry := range entries {
-		out = append(out, []string{
-			"-p", entry.Protocol,
-			"-s", localhostIP(isV6),
-			"-d", containerIP.String(),
-			"--dport", strconv.Itoa(entry.ContainerPort),
+// genMarkMasqChain creates the chain that masquerades all packets marked
+// in the SETMARK chain
+func genMarkMasqChain(markBit int) chain {
+	markValue := 1 << uint(markBit)
+	markDef := fmt.Sprintf("%#x/%#x", markValue, markValue)
+	ch := chain{
+		table:       "nat",
+		name:        MarkMasqChainName,
+		entryChains: []string{"POSTROUTING"},
+		entryRules: [][]string{{
+			"-m", "comment",
+			"--comment", "CNI portfwd requiring masquerade",
+		}},
+		rules: [][]string{{
+			"-m", "mark",
+			"--mark", markDef,
 			"-j", "MASQUERADE",
-		})
+		}},
 	}
-	return out
+	return ch
 }
 
 // enableLocalnetRouting tells the kernel not to treat 127/8 as a martian,
@@ -234,6 +276,18 @@ func enableLocalnetRouting(ifName string) error {
 	return err
 }
 
+// genOldSnatChain is no longer used, but used to be created. We'll try and
+// tear it down in case the plugin version changed between ADD and DEL
+func genOldSnatChain(netName, containerID string) chain {
+	name := formatChainName("SN-", netName, containerID)
+
+	return chain{
+		table:       "nat",
+		name:        name,
+		entryChains: []string{OldTopLevelSNATChainName},
+	}
+}
+
 // unforwardPorts deletes any iptables rules created by this plugin.
 // It should be idempotent - it will not error if the chain does not exist.
 //
@@ -245,8 +299,10 @@ func enableLocalnetRouting(ifName string) error {
 // So, we first check that iptables is "generally OK" by doing a check. If
 // not, we ignore the error, unless neither v4 nor v6 are OK.
 func unforwardPorts(config *PortMapConf) error {
-	dnatChain := genDnatChain(config.Name, config.ContainerID, nil)
-	snatChain := genSnatChain(config.Name, config.ContainerID)
+	dnatChain := genDnatChain(config.Name, config.ContainerID)
+
+	// Might be lying around from old versions
+	oldSnatChain := genOldSnatChain(config.Name, config.ContainerID)
 
 	ip4t := maybeGetIptables(false)
 	ip6t := maybeGetIptables(true)
@@ -258,16 +314,14 @@ func unforwardPorts(config *PortMapConf) error {
 		if err := dnatChain.teardown(ip4t); err != nil {
 			return fmt.Errorf("could not teardown ipv4 dnat: %v", err)
 		}
-		if err := snatChain.teardown(ip4t); err != nil {
-			return fmt.Errorf("could not teardown ipv4 snat: %v", err)
-		}
+		oldSnatChain.teardown(ip4t)
 	}
 
 	if ip6t != nil {
 		if err := dnatChain.teardown(ip6t); err != nil {
 			return fmt.Errorf("could not teardown ipv6 dnat: %v", err)
 		}
-		// no SNAT teardown because it doesn't work for v6
+		oldSnatChain.teardown(ip6t)
 	}
 	return nil
 }

plugins/meta/portmap/portmap_integ_test.go

@@ -15,12 +15,14 @@
 package main
 
 import (
+	"bytes"
 	"fmt"
 	"math/rand"
 	"net"
 	"os"
+	"os/exec"
 	"path/filepath"
-	"time"
+	"strconv"
 
 	"github.com/containernetworking/cni/libcni"
 	"github.com/containernetworking/cni/pkg/types/current"
@@ -28,19 +30,20 @@ import (
 	"github.com/coreos/go-iptables/iptables"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gexec"
 	"github.com/vishvananda/netlink"
 )
 
 const TIMEOUT = 90
 
 var _ = Describe("portmap integration tests", func() {
-	rand.Seed(time.Now().UTC().UnixNano())
-
-	var configList *libcni.NetworkConfigList
-	var cniConf *libcni.CNIConfig
-	var targetNS ns.NetNS
-	var containerPort int
-	var closeChan chan interface{}
+	var (
+		configList    *libcni.NetworkConfigList
+		cniConf       *libcni.CNIConfig
+		targetNS      ns.NetNS
+		containerPort int
+		session       *gexec.Session
+	)
 
 	BeforeEach(func() {
 		var err error
@@ -80,12 +83,12 @@ var _ = Describe("portmap integration tests", func() {
 		fmt.Fprintln(GinkgoWriter, "namespace:", targetNS.Path())
 
 		// Start an echo server and get the port
-		containerPort, closeChan, err = RunEchoServerInNS(targetNS)
+		containerPort, session, err = StartEchoServerInNamespace(targetNS)
 		Expect(err).NotTo(HaveOccurred())
-
 	})
 
 	AfterEach(func() {
+		session.Terminate().Wait()
 		if targetNS != nil {
 			targetNS.Close()
 		}
@@ -123,13 +126,20 @@ var _ = Describe("portmap integration tests", func() {
 		// we'll also manually check the iptables chains
 		ipt, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 		Expect(err).NotTo(HaveOccurred())
-		dnatChainName := genDnatChain("cni-portmap-unit-test", runtimeConfig.ContainerID, nil).name
+		dnatChainName := genDnatChain("cni-portmap-unit-test", runtimeConfig.ContainerID).name
 
 		// Create the network
 		resI, err := cniConf.AddNetworkList(configList, &runtimeConfig)
 		Expect(err).NotTo(HaveOccurred())
 		defer deleteNetwork()
 
+		// Undo Docker's forwarding policy
+		cmd := exec.Command("iptables", "-t", "filter",
+			"-P", "FORWARD", "ACCEPT")
+		cmd.Stderr = GinkgoWriter
+		err = cmd.Run()
+		Expect(err).NotTo(HaveOccurred())
+
 		// Check the chain exists
 		_, err = ipt.List("nat", dnatChainName)
 		Expect(err).NotTo(HaveOccurred())
@@ -154,16 +164,19 @@ var _ = Describe("portmap integration tests", func() {
 			hostIP, hostPort, contIP, containerPort)
 
 		// Sanity check: verify that the container is reachable directly
-		contOK := testEchoServer(fmt.Sprintf("%s:%d", contIP.String(), containerPort))
+		contOK := testEchoServer(contIP.String(), containerPort, "")
 
 		// Verify that a connection to the forwarded port works
-		dnatOK := testEchoServer(fmt.Sprintf("%s:%d", hostIP, hostPort))
+		dnatOK := testEchoServer(hostIP, hostPort, "")
 
 		// Verify that a connection to localhost works
-		snatOK := testEchoServer(fmt.Sprintf("%s:%d", "127.0.0.1", hostPort))
+		snatOK := testEchoServer("127.0.0.1", hostPort, "")
+
+		// verify that hairpin works
+		hairpinOK := testEchoServer(hostIP, hostPort, targetNS.Path())
 
 		// Cleanup
-		close(closeChan)
+		session.Terminate()
 		err = deleteNetwork()
 		Expect(err).NotTo(HaveOccurred())
 
@@ -181,6 +194,9 @@ var _ = Describe("portmap integration tests", func() {
 		if !snatOK {
 			Fail("connection to 127.0.0.1 was not forwarded")
 		}
+		if !hairpinOK {
+			Fail("Hairpin connection failed")
+		}
 
 		close(done)
 
@@ -188,40 +204,33 @@ var _ = Describe("portmap integration tests", func() {
 })
 
 // testEchoServer returns true if we found an echo server on the port
-func testEchoServer(address string) bool {
-	fmt.Fprintln(GinkgoWriter, "dialing", address)
-	conn, err := net.Dial("tcp", address)
-	if err != nil {
-		fmt.Fprintln(GinkgoWriter, "connection to", address, "failed:", err)
-		return false
-	}
-	defer conn.Close()
-
-	conn.SetDeadline(time.Now().Add(TIMEOUT * time.Second))
-	fmt.Fprintln(GinkgoWriter, "connected to", address)
-
+func testEchoServer(address string, port int, netns string) bool {
 	message := "Aliquid melius quam pessimum optimum non est."
-	_, err = fmt.Fprint(conn, message)
+
+	bin, err := exec.LookPath("nc")
+	Expect(err).NotTo(HaveOccurred())
+	var cmd *exec.Cmd
+	if netns != "" {
+		netns = filepath.Base(netns)
+		cmd = exec.Command("ip", "netns", "exec", netns, bin, "-v", address, strconv.Itoa(port))
+	} else {
+		cmd = exec.Command("nc", address, strconv.Itoa(port))
+	}
+	cmd.Stdin = bytes.NewBufferString(message)
+	cmd.Stderr = GinkgoWriter
+	out, err := cmd.Output()
 	if err != nil {
-		fmt.Fprintln(GinkgoWriter, "sending message to", address, " failed:", err)
+		fmt.Fprintln(GinkgoWriter, "got non-zero exit from ", cmd.Args)
 		return false
 	}
 
-	conn.SetDeadline(time.Now().Add(TIMEOUT * time.Second))
-	fmt.Fprintln(GinkgoWriter, "reading...")
-	response := make([]byte, len(message))
-	_, err = conn.Read(response)
-	if err != nil {
-		fmt.Fprintln(GinkgoWriter, "receiving message from", address, " failed:", err)
+	if string(out) != message {
+		fmt.Fprintln(GinkgoWriter, "returned message didn't match?")
+		fmt.Fprintln(GinkgoWriter, string(out))
 		return false
 	}
 
-	fmt.Fprintln(GinkgoWriter, "read...")
-	if string(response) == message {
-		return true
-	}
-	fmt.Fprintln(GinkgoWriter, "returned message didn't match?")
-	return false
+	return true
 }
 
 func getLocalIP() string {

plugins/meta/portmap/portmap_suite_test.go

@@ -15,89 +15,64 @@
 package main
 
 import (
-	"fmt"
+	"math/rand"
 	"net"
-	"time"
+	"os/exec"
+	"path/filepath"
+	"strconv"
+	"strings"
 
 	"github.com/containernetworking/plugins/pkg/ns"
 
 	. "github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/config"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gbytes"
+	"github.com/onsi/gomega/gexec"
 
 	"testing"
 )
 
 func TestPortmap(t *testing.T) {
+	rand.Seed(config.GinkgoConfig.RandomSeed)
+
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "portmap Suite")
 }
 
-// OpenEchoServer opens a server that listens until closeChan is closed.
-// It opens on a random port and sends the port number on portChan when
-// the server is up and running. If an error is encountered, closes portChan.
-// If closeChan is closed, closes the socket.
-func OpenEchoServer(portChan chan<- int, closeChan <-chan interface{}) error {
-	laddr, err := net.ResolveTCPAddr("tcp", "0.0.0.0:0")
-	if err != nil {
-		close(portChan)
-		return err
-	}
-	sock, err := net.ListenTCP("tcp", laddr)
-	if err != nil {
-		close(portChan)
-		return err
-	}
-	defer sock.Close()
+var echoServerBinaryPath string
 
-	switch addr := sock.Addr().(type) {
-	case *net.TCPAddr:
-		portChan <- addr.Port
-	default:
-		close(portChan)
-		return fmt.Errorf("addr cast failed!")
-	}
-	for {
-		select {
-		case <-closeChan:
-			break
-		default:
-		}
+var _ = SynchronizedBeforeSuite(func() []byte {
+	binaryPath, err := gexec.Build("github.com/containernetworking/plugins/pkg/testutils/echosvr")
+	Expect(err).NotTo(HaveOccurred())
+	return []byte(binaryPath)
+}, func(data []byte) {
+	echoServerBinaryPath = string(data)
+})
 
-		sock.SetDeadline(time.Now().Add(time.Second))
-		con, err := sock.AcceptTCP()
-		if err != nil {
-			if opErr, ok := err.(*net.OpError); ok && opErr.Timeout() {
-				continue
-			}
-			continue
-		}
+var _ = SynchronizedAfterSuite(func() {}, func() {
+	gexec.CleanupBuildArtifacts()
+})
 
-		buf := make([]byte, 512)
-		con.Read(buf)
-		con.Write(buf)
-		con.Close()
-	}
+func startInNetNS(binPath string, netNS ns.NetNS) (*gexec.Session, error) {
+	baseName := filepath.Base(netNS.Path())
+	// we are relying on the netNS path living in /var/run/netns
+	// where `ip netns exec` can find it
+	cmd := exec.Command("ip", "netns", "exec", baseName, binPath)
+	session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
+	return session, err
 }
 
-func RunEchoServerInNS(netNS ns.NetNS) (int, chan interface{}, error) {
-	portChan := make(chan int)
-	closeChan := make(chan interface{})
+func StartEchoServerInNamespace(netNS ns.NetNS) (int, *gexec.Session, error) {
+	session, err := startInNetNS(echoServerBinaryPath, netNS)
+	Expect(err).NotTo(HaveOccurred())
 
-	go func() {
-		err := netNS.Do(func(ns.NetNS) error {
-			OpenEchoServer(portChan, closeChan)
-			return nil
-		})
-		// Somehow the ns.Do failed
-		if err != nil {
-			close(portChan)
-		}
-	}()
+	// wait for it to print it's address on stdout
+	Eventually(session.Out).Should(gbytes.Say("\n"))
+	_, portString, err := net.SplitHostPort(strings.TrimSpace(string(session.Out.Contents())))
+	Expect(err).NotTo(HaveOccurred())
 
-	portNum := <-portChan
-	if portNum == 0 {
-		return 0, nil, fmt.Errorf("failed to execute server")
-	}
-
-	return portNum, closeChan, nil
+	port, err := strconv.Atoi(portString)
+	Expect(err).NotTo(HaveOccurred())
+	return port, session, nil
 }

plugins/meta/portmap/portmap_test.go

@@ -15,6 +15,7 @@
 package main
 
 import (
+	"fmt"
 	"net"
 
 	. "github.com/onsi/ginkgo"
@@ -25,13 +26,6 @@ var _ = Describe("portmapping configuration", func() {
 	netName := "testNetName"
 	containerID := "icee6giejonei6sohng6ahngee7laquohquee9shiGo7fohferakah3Feiyoolu2pei7ciPhoh7shaoX6vai3vuf0ahfaeng8yohb9ceu0daez5hashee8ooYai5wa3y"
 
-	mappings := []PortMapEntry{
-		{80, 90, "tcp", ""},
-		{1000, 2000, "udp", ""},
-	}
-	ipv4addr := net.ParseIP("192.2.0.1")
-	ipv6addr := net.ParseIP("2001:db8::1")
-
 	Context("config parsing", func() {
 		It("Correctly parses an ADD config", func() {
 			configBytes := []byte(`{
@@ -156,101 +150,179 @@ var _ = Describe("portmapping configuration", func() {
 
 	Describe("Generating chains", func() {
 		Context("for DNAT", func() {
-			It("generates a correct container chain", func() {
-				ch := genDnatChain(netName, containerID, &[]string{"-m", "hello"})
+			It("generates a correct standard container chain", func() {
+				ch := genDnatChain(netName, containerID)
 
 				Expect(ch).To(Equal(chain{
-					table: "nat",
-					name:  "CNI-DN-bfd599665540dd91d5d28",
-					entryRule: []string{
-						"-m", "comment",
-						"--comment", `dnat name: "testNetName" id: "` + containerID + `"`,
-						"-m", "hello",
-					},
+					table:       "nat",
+					name:        "CNI-DN-bfd599665540dd91d5d28",
 					entryChains: []string{TopLevelDNATChainName},
 				}))
+				configBytes := []byte(`{
+	"name": "test",
+	"type": "portmap",
+	"cniVersion": "0.3.1",
+	"runtimeConfig": {
+		"portMappings": [
+			{ "hostPort": 8080, "containerPort": 80, "protocol": "tcp"},
+			{ "hostPort": 8081, "containerPort": 80, "protocol": "tcp"},
+			{ "hostPort": 8080, "containerPort": 81, "protocol": "udp"},
+			{ "hostPort": 8082, "containerPort": 82, "protocol": "udp"}
+		]
+	},
+	"snat": true,
+	"conditionsV4": ["a", "b"],
+	"conditionsV6": ["c", "d"]
+}`)
+
+				conf, err := parseConfig(configBytes, "foo")
+				Expect(err).NotTo(HaveOccurred())
+				conf.ContainerID = containerID
+
+				ch = genDnatChain(conf.Name, containerID)
+				Expect(ch).To(Equal(chain{
+					table:       "nat",
+					name:        "CNI-DN-67e92b96e692a494b6b85",
+					entryChains: []string{"CNI-HOSTPORT-DNAT"},
+				}))
+
+				fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2"))
+
+				Expect(ch.entryRules).To(Equal([][]string{
+					{"-m", "comment", "--comment",
+						fmt.Sprintf("dnat name: \"test\" id: \"%s\"", containerID),
+						"-m", "multiport",
+						"-p", "tcp",
+						"--destination-ports", "8080,8081",
+						"a", "b"},
+					{"-m", "comment", "--comment",
+						fmt.Sprintf("dnat name: \"test\" id: \"%s\"", containerID),
+						"-m", "multiport",
+						"-p", "udp",
+						"--destination-ports", "8080,8082",
+						"a", "b"},
+				}))
+
+				Expect(ch.rules).To(Equal([][]string{
+					{"-p", "tcp", "--dport", "8080", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "tcp", "--dport", "8080", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"},
+					{"-p", "tcp", "--dport", "8081", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "tcp", "--dport", "8081", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "10.0.0.2:80"},
+					{"-p", "udp", "--dport", "8080", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "udp", "--dport", "8080", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:81"},
+					{"-p", "udp", "--dport", "8082", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "udp", "--dport", "8082", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "10.0.0.2:82"},
+				}))
+
+				ch.rules = nil
+				ch.entryRules = nil
+
+				fillDnatRules(&ch, conf, net.ParseIP("2001:db8::2"))
+
+				Expect(ch.rules).To(Equal([][]string{
+					{"-p", "tcp", "--dport", "8080", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "[2001:db8::2]:80"},
+					{"-p", "tcp", "--dport", "8081", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "[2001:db8::2]:80"},
+					{"-p", "udp", "--dport", "8080", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "[2001:db8::2]:81"},
+					{"-p", "udp", "--dport", "8082", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"},
+					{"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "[2001:db8::2]:82"},
+				}))
+
+				// Disable snat, generate rules
+				ch.rules = nil
+				ch.entryRules = nil
+				fvar := false
+				conf.SNAT = &fvar
+
+				fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2"))
+				Expect(ch.rules).To(Equal([][]string{
+					{"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"},
+					{"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "10.0.0.2:80"},
+					{"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:81"},
+					{"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "10.0.0.2:82"},
+				}))
+			})
+
+			It("generates a correct chain with external mark", func() {
+				ch := genDnatChain(netName, containerID)
+
+				Expect(ch).To(Equal(chain{
+					table:       "nat",
+					name:        "CNI-DN-bfd599665540dd91d5d28",
+					entryChains: []string{TopLevelDNATChainName},
+				}))
+				configBytes := []byte(`{
+	"name": "test",
+	"type": "portmap",
+	"cniVersion": "0.3.1",
+	"runtimeConfig": {
+		"portMappings": [
+			{ "hostPort": 8080, "containerPort": 80, "protocol": "tcp"}
+		]
+	},
+	"externalSetMarkChain": "PLZ-SET-MARK",
+	"conditionsV4": ["a", "b"],
+	"conditionsV6": ["c", "d"]
+}`)
+
+				conf, err := parseConfig(configBytes, "foo")
+				Expect(err).NotTo(HaveOccurred())
+				conf.ContainerID = containerID
+
+				ch = genDnatChain(conf.Name, containerID)
+				fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2"))
+				Expect(ch.rules).To(Equal([][]string{
+					{"-p", "tcp", "--dport", "8080", "-s", "10.0.0.2", "-j", "PLZ-SET-MARK"},
+					{"-p", "tcp", "--dport", "8080", "-s", "127.0.0.1", "-j", "PLZ-SET-MARK"},
+					{"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"},
+				}))
 			})
 
 			It("generates a correct top-level chain", func() {
 				ch := genToplevelDnatChain()
 
 				Expect(ch).To(Equal(chain{
-					table: "nat",
-					name:  "CNI-HOSTPORT-DNAT",
-					entryRule: []string{
-						"-m", "addrtype",
-						"--dst-type", "LOCAL",
-					},
+					table:       "nat",
+					name:        "CNI-HOSTPORT-DNAT",
 					entryChains: []string{"PREROUTING", "OUTPUT"},
+					entryRules:  [][]string{{"-m", "addrtype", "--dst-type", "LOCAL"}},
 				}))
 			})
-		})
-
-		Context("for SNAT", func() {
-			It("generates a correct container chain", func() {
-				ch := genSnatChain(netName, containerID)
 
+			It("generates the correct mark chains", func() {
+				masqBit := 5
+				ch := genSetMarkChain(masqBit)
 				Expect(ch).To(Equal(chain{
 					table: "nat",
-					name:  "CNI-SN-bfd599665540dd91d5d28",
-					entryRule: []string{
+					name:  "CNI-HOSTPORT-SETMARK",
+					rules: [][]string{{
 						"-m", "comment",
-						"--comment", `snat name: "testNetName" id: "` + containerID + `"`,
-					},
-					entryChains: []string{TopLevelSNATChainName},
+						"--comment", "CNI portfwd masquerade mark",
+						"-j", "MARK",
+						"--set-xmark", "0x20/0x20",
+					}},
 				}))
-			})
 
-			It("generates a correct top-level chain", func() {
-				Context("for ipv4", func() {
-					ch := genToplevelSnatChain(false)
-					Expect(ch).To(Equal(chain{
-						table: "nat",
-						name:  "CNI-HOSTPORT-SNAT",
-						entryRule: []string{
-							"-s", "127.0.0.1",
-							"!", "-d", "127.0.0.1",
-						},
-						entryChains: []string{"POSTROUTING"},
-					}))
-				})
-			})
-		})
-	})
-
-	Describe("Forwarding rules", func() {
-		Context("for DNAT", func() {
-			It("generates correct ipv4 rules", func() {
-				rules := dnatRules(mappings, ipv4addr)
-				Expect(rules).To(Equal([][]string{
-					{"-p", "tcp", "--dport", "80", "-j", "DNAT", "--to-destination", "192.2.0.1:90"},
-					{"-p", "udp", "--dport", "1000", "-j", "DNAT", "--to-destination", "192.2.0.1:2000"},
-				}))
-			})
-			It("generates correct ipv6 rules", func() {
-				rules := dnatRules(mappings, ipv6addr)
-				Expect(rules).To(Equal([][]string{
-					{"-p", "tcp", "--dport", "80", "-j", "DNAT", "--to-destination", "[2001:db8::1]:90"},
-					{"-p", "udp", "--dport", "1000", "-j", "DNAT", "--to-destination", "[2001:db8::1]:2000"},
-				}))
-			})
-		})
-
-		Context("for SNAT", func() {
-
-			It("generates correct ipv4 rules", func() {
-				rules := snatRules(mappings, ipv4addr)
-				Expect(rules).To(Equal([][]string{
-					{"-p", "tcp", "-s", "127.0.0.1", "-d", "192.2.0.1", "--dport", "90", "-j", "MASQUERADE"},
-					{"-p", "udp", "-s", "127.0.0.1", "-d", "192.2.0.1", "--dport", "2000", "-j", "MASQUERADE"},
-				}))
-			})
-
-			It("generates correct ipv6 rules", func() {
-				rules := snatRules(mappings, ipv6addr)
-				Expect(rules).To(Equal([][]string{
-					{"-p", "tcp", "-s", "::1", "-d", "2001:db8::1", "--dport", "90", "-j", "MASQUERADE"},
-					{"-p", "udp", "-s", "::1", "-d", "2001:db8::1", "--dport", "2000", "-j", "MASQUERADE"},
+				ch = genMarkMasqChain(masqBit)
+				Expect(ch).To(Equal(chain{
+					table:       "nat",
+					name:        "CNI-HOSTPORT-MASQ",
+					entryChains: []string{"POSTROUTING"},
+					entryRules: [][]string{{
+						"-m", "comment",
+						"--comment", "CNI portfwd requiring masquerade",
+					}},
+					rules: [][]string{{
+						"-m", "mark",
+						"--mark", "0x20/0x20",
+						"-j", "MASQUERADE",
+					}},
 				}))
 			})
 		})

plugins/meta/portmap/utils.go

@@ -18,6 +18,8 @@ import (
 	"crypto/sha512"
 	"fmt"
 	"net"
+	"strconv"
+	"strings"
 
 	"github.com/vishvananda/netlink"
 )
@@ -65,3 +67,51 @@ func formatChainName(prefix, name, id string) string {
 	chain := fmt.Sprintf("CNI-%s%x", prefix, chainBytes)
 	return chain[:maxChainNameLength]
 }
+
+// groupByProto groups port numbers by protocol
+func groupByProto(entries []PortMapEntry) map[string][]int {
+	if len(entries) == 0 {
+		return map[string][]int{}
+	}
+	out := map[string][]int{}
+	for _, e := range entries {
+		_, ok := out[e.Protocol]
+		if ok {
+			out[e.Protocol] = append(out[e.Protocol], e.HostPort)
+		} else {
+			out[e.Protocol] = []int{e.HostPort}
+		}
+	}
+
+	return out
+}
+
+// splitPortList splits a list of integers in to one or more comma-separated
+// string values, for use by multiport. Multiport only allows up to 15 ports
+// per entry.
+func splitPortList(l []int) []string {
+	out := []string{}
+
+	acc := []string{}
+	for _, i := range l {
+		acc = append(acc, strconv.Itoa(i))
+		if len(acc) == 15 {
+			out = append(out, strings.Join(acc, ","))
+			acc = []string{}
+		}
+	}
+
+	if len(acc) > 0 {
+		out = append(out, strings.Join(acc, ","))
+	}
+	return out
+}
+
+// trimComment makes sure no comment is over the iptables limit of 255 chars
+func trimComment(val string) string {
+	if len(val) <= 255 {
+		return val
+	}
+
+	return val[0:253] + "..."
+}

scripts/release.sh

@@ -12,12 +12,14 @@ OUTPUT_DIR=bin
 # Always clean first
 rm -Rf ${SRC_DIR}/${RELEASE_DIR}
 mkdir -p ${SRC_DIR}/${RELEASE_DIR}
+mkdir -p ${OUTPUT_DIR}
 
-docker run -i -v ${SRC_DIR}:/opt/src --rm golang:1.8-alpine \
+docker run -i -v ${SRC_DIR}:/opt/src --rm golang:1.9-alpine \
 /bin/sh -xe -c "\
     apk --no-cache add bash tar;
     cd /opt/src; umask 0022;
     for arch in amd64 arm arm64 ppc64le s390x; do \
+        rm -f ${OUTPUT_DIR}/*; \
         CGO_ENABLED=0 GOARCH=\$arch ./build.sh ${BUILDFLAGS}; \
         for format in tgz; do \
             FILENAME=cni-plugins-\$arch-${TAG}.\$format; \

test.sh

@@ -10,36 +10,36 @@ source ./build.sh
 
 echo "Running tests"
 
-TESTABLE="plugins/ipam/dhcp plugins/ipam/host-local plugins/ipam/host-local/backend/allocator plugins/main/loopback plugins/main/ipvlan plugins/main/macvlan plugins/main/bridge plugins/main/ptp plugins/meta/flannel plugins/main/vlan plugins/sample pkg/ip pkg/ipam pkg/ns pkg/utils pkg/utils/hwaddr pkg/utils/sysctl plugins/meta/portmap"
+# test everything that's not in vendor
+pushd "$GOPATH/src/$REPO_PATH" >/dev/null
+  ALL_PKGS="$(go list ./... | grep -v vendor | xargs echo)"
+popd >/dev/null
+
+GINKGO_FLAGS="-p --randomizeAllSpecs --randomizeSuites --failOnPending --progress"
 
 # user has not provided PKG override
 if [ -z "$PKG" ]; then
-	TEST=$TESTABLE
-	FMT=$TESTABLE
+  GINKGO_FLAGS="$GINKGO_FLAGS -r ."
+  LINT_TARGETS="$ALL_PKGS"
 
 # user has provided PKG override
 else
-	TEST=$PKG
-
-	# only run gofmt on packages provided by user
-	FMT="$TEST"
+  GINKGO_FLAGS="$GINKGO_FLAGS $PKG"
+  LINT_TARGETS="$PKG"
 fi
 
-# split TEST into an array and prepend REPO_PATH to each local package
-split=(${TEST// / })
-TEST=${split[@]/#/${REPO_PATH}/}
-
-sudo -E bash -c "umask 0; PATH=${GOROOT}/bin:$(pwd)/bin:${PATH} go test ${TEST}"
+cd "$GOPATH/src/$REPO_PATH"
+sudo -E bash -c "umask 0; PATH=${GOROOT}/bin:$(pwd)/bin:${PATH} ginkgo ${GINKGO_FLAGS}"
 
 echo "Checking gofmt..."
-fmtRes=$(gofmt -l $FMT)
+fmtRes=$(go fmt $LINT_TARGETS)
 if [ -n "${fmtRes}" ]; then
-	echo -e "gofmt checking failed:\n${fmtRes}"
+	echo -e "go fmt checking failed:\n${fmtRes}"
 	exit 255
 fi
 
 echo "Checking govet..."
-vetRes=$(go vet $TEST)
+vetRes=$(go vet $LINT_TARGETS)
 if [ -n "${vetRes}" ]; then
 	echo -e "govet checking failed:\n${vetRes}"
 	exit 255

vendor/github.com/alexflint/go-filemutex/LICENSE

@@ -0,0 +1,21 @@
+The MIT License
+
+Copyright (c) 2010-2017 Alex Flint.
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.

vendor/github.com/alexflint/go-filemutex/README.md

@@ -0,0 +1,31 @@
+# FileMutex
+
+FileMutex is similar to `sync.RWMutex`, but also synchronizes across processes.
+On Linux, OSX, and other POSIX systems it uses the flock system call. On windows
+it uses the LockFileEx and UnlockFileEx system calls.
+
+```go
+import (
+	"log"
+	"github.com/alexflint/go-filemutex"
+)
+
+func main() {
+	m, err := filemutex.New("/tmp/foo.lock")
+	if err != nil {
+		log.Fatalln("Directory did not exist or file could not created")
+	}
+
+	m.Lock()  // Will block until lock can be acquired
+
+	// Code here is protected by the mutex
+
+	m.Unlock()
+}
+```
+
+### Installation
+
+    go get github.com/alexflint/go-filemutex
+
+Forked from https://github.com/golang/build/tree/master/cmd/builder/filemutex_*.go

vendor/github.com/alexflint/go-filemutex/filemutex_flock.go

@@ -0,0 +1,67 @@
+// Copyright 2013 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build darwin dragonfly freebsd linux netbsd openbsd
+
+package filemutex
+
+import (
+	"syscall"
+)
+
+const (
+	mkdirPerm = 0750
+)
+
+// FileMutex is similar to sync.RWMutex, but also synchronizes across processes.
+// This implementation is based on flock syscall.
+type FileMutex struct {
+	fd int
+}
+
+func New(filename string) (*FileMutex, error) {
+	fd, err := syscall.Open(filename, syscall.O_CREAT|syscall.O_RDONLY, mkdirPerm)
+	if err != nil {
+		return nil, err
+	}
+	return &FileMutex{fd: fd}, nil
+}
+
+func (m *FileMutex) Lock() error {
+	if err := syscall.Flock(m.fd, syscall.LOCK_EX); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (m *FileMutex) Unlock() error {
+	if err := syscall.Flock(m.fd, syscall.LOCK_UN); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (m *FileMutex) RLock() error {
+	if err := syscall.Flock(m.fd, syscall.LOCK_SH); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (m *FileMutex) RUnlock() error {
+	if err := syscall.Flock(m.fd, syscall.LOCK_UN); err != nil {
+		return err
+	}
+	return nil
+}
+
+// Close does an Unlock() combined with closing and unlinking the associated
+// lock file. You should create a New() FileMutex for every Lock() attempt if
+// using Close().
+func (m *FileMutex) Close() error {
+	if err := syscall.Flock(m.fd, syscall.LOCK_UN); err != nil {
+		return err
+	}
+	return syscall.Close(m.fd)
+}

vendor/github.com/alexflint/go-filemutex/filemutex_windows.go

@@ -0,0 +1,102 @@
+// Copyright 2013 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package filemutex
+
+import (
+	"syscall"
+	"unsafe"
+)
+
+var (
+	modkernel32      = syscall.NewLazyDLL("kernel32.dll")
+	procLockFileEx   = modkernel32.NewProc("LockFileEx")
+	procUnlockFileEx = modkernel32.NewProc("UnlockFileEx")
+)
+
+const (
+	lockfileExclusiveLock = 2
+)
+
+func lockFileEx(h syscall.Handle, flags, reserved, locklow, lockhigh uint32, ol *syscall.Overlapped) (err error) {
+	r1, _, e1 := syscall.Syscall6(procLockFileEx.Addr(), 6, uintptr(h), uintptr(flags), uintptr(reserved), uintptr(locklow), uintptr(lockhigh), uintptr(unsafe.Pointer(ol)))
+	if r1 == 0 {
+		if e1 != 0 {
+			err = error(e1)
+		} else {
+			err = syscall.EINVAL
+		}
+	}
+	return
+}
+
+func unlockFileEx(h syscall.Handle, reserved, locklow, lockhigh uint32, ol *syscall.Overlapped) (err error) {
+	r1, _, e1 := syscall.Syscall6(procUnlockFileEx.Addr(), 5, uintptr(h), uintptr(reserved), uintptr(locklow), uintptr(lockhigh), uintptr(unsafe.Pointer(ol)), 0)
+	if r1 == 0 {
+		if e1 != 0 {
+			err = error(e1)
+		} else {
+			err = syscall.EINVAL
+		}
+	}
+	return
+}
+
+// FileMutex is similar to sync.RWMutex, but also synchronizes across processes.
+// This implementation is based on flock syscall.
+type FileMutex struct {
+	fd syscall.Handle
+}
+
+func New(filename string) (*FileMutex, error) {
+	fd, err := syscall.CreateFile(&(syscall.StringToUTF16(filename)[0]), syscall.GENERIC_READ|syscall.GENERIC_WRITE,
+		syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE, nil, syscall.OPEN_ALWAYS, syscall.FILE_ATTRIBUTE_NORMAL, 0)
+	if err != nil {
+		return nil, err
+	}
+	return &FileMutex{fd: fd}, nil
+}
+
+func (m *FileMutex) Lock() error {
+	var ol syscall.Overlapped
+	if err := lockFileEx(m.fd, lockfileExclusiveLock, 0, 1, 0, &ol); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (m *FileMutex) Unlock() error {
+	var ol syscall.Overlapped
+	if err := unlockFileEx(m.fd, 0, 1, 0, &ol); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (m *FileMutex) RLock() error {
+	var ol syscall.Overlapped
+	if err := lockFileEx(m.fd, 0, 0, 1, 0, &ol); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (m *FileMutex) RUnlock() error {
+	var ol syscall.Overlapped
+	if err := unlockFileEx(m.fd, 0, 1, 0, &ol); err != nil {
+		return err
+	}
+	return nil
+}
+
+// Close does an Unlock() combined with closing and unlinking the associated
+// lock file. You should create a New() FileMutex for every Lock() attempt if
+// using Close().
+func (m *FileMutex) Close() error {
+	var ol syscall.Overlapped
+	if err := unlockFileEx(m.fd, 0, 1, 0, &ol); err != nil {
+		return err
+	}
+	return syscall.Close(m.fd)
+}