Do not use netns as ID or for entropy

ContainerID is now required so use that
or generate random bytes.

Fixes #5

pkg/ip/link.go

@@ -15,7 +15,7 @@
 package ip
 
 import (
-	"crypto/sha512"
+	"crypto/rand"
 	"fmt"
 	"net"
 	"os"
@@ -23,7 +23,7 @@ import (
 	"github.com/appc/cni/Godeps/_workspace/src/github.com/vishvananda/netlink"
 )
 
-func makeVeth(name, peer string, mtu int) (netlink.Link, error) {
+func makeVethPair(name, peer string, mtu int) (netlink.Link, error) {
 	veth := &netlink.Veth{
 		LinkAttrs: netlink.LinkAttrs{
 			Name:  name,
@@ -39,33 +39,50 @@ func makeVeth(name, peer string, mtu int) (netlink.Link, error) {
 	return veth, nil
 }
 
+func makeVeth(name string, mtu int) (peerName string, veth netlink.Link, err error) {
+	for i := 0; i < 10; i++ {
+		peerName, err = RandomVethName()
+		if err != nil {
+			return
+		}
+
+		veth, err = makeVethPair(name, peerName, mtu)
+		switch {
+		case err == nil:
+			return
+
+		case os.IsExist(err):
+			continue
+
+		default:
+			err = fmt.Errorf("failed to make veth pair: %v", err)
+			return
+		}
+	}
+
+	// should really never be hit
+	err = fmt.Errorf("failed to find a unique veth name")
+	return
+}
+
 // RandomVethName returns string "veth" with random prefix (hashed from entropy)
-func RandomVethName(entropy string) string {
+func RandomVethName() (string, error) {
-	h := sha512.New()
+	entropy := make([]byte, 4)
-	h.Write([]byte(entropy))
+	_, err := rand.Reader.Read(entropy)
-	return fmt.Sprintf("veth%x", h.Sum(nil)[:5])
+	if err != nil {
+		return "", fmt.Errorf("failed to generate random veth name: %v", err)
+	}
+
+	// NetworkManager (recent versions) will ignore veth devices that start with "veth"
+	return fmt.Sprintf("veth%x", entropy), nil
 }
 
 // SetupVeth sets up a virtual ethernet link.
 // Should be in container netns.
-// TODO(eyakubovich): get rid of entropy and ask kernel to pick name via pattern
+func SetupVeth(contVethName string, mtu int, hostNS *os.File) (hostVeth, contVeth netlink.Link, err error) {
-func SetupVeth(entropy, contVethName string, mtu int, hostNS *os.File) (hostVeth, contVeth netlink.Link, err error) {
+	var hostVethName string
-	// NetworkManager (recent versions) will ignore veth devices that start with "veth"
+	hostVethName, contVeth, err = makeVeth(contVethName, mtu)
-	hostVethName := RandomVethName(entropy)
-	hostVeth, err = makeVeth(hostVethName, contVethName, mtu)
 	if err != nil {
-		err = fmt.Errorf("failed to make veth pair: %v", err)
-		return
-	}
-
-	if err = netlink.LinkSetUp(hostVeth); err != nil {
-		err = fmt.Errorf("failed to set %q up: %v", hostVethName, err)
-		return
-	}
-
-	contVeth, err = netlink.LinkByName(contVethName)
-	if err != nil {
-		err = fmt.Errorf("failed to lookup %q: %v", contVethName, err)
 		return
 	}
 
@@ -74,6 +91,17 @@ func SetupVeth(entropy, contVethName string, mtu int, hostNS *os.File) (hostVeth
 		return
 	}
 
+	hostVeth, err = netlink.LinkByName(hostVethName)
+	if err != nil {
+		err = fmt.Errorf("failed to lookup %q: %v", hostVethName, err)
+		return
+	}
+
+	if err = netlink.LinkSetUp(hostVeth); err != nil {
+		err = fmt.Errorf("failed to set %q up: %v", contVethName, err)
+		return
+	}
+
 	if err = netlink.LinkSetNsFd(hostVeth, int(hostNS.Fd())); err != nil {
 		err = fmt.Errorf("failed to move veth to host netns: %v", err)
 		return

plugins/ipam/host-local/main.go

@@ -48,9 +48,9 @@ func cmdAdd(args *skel.CmdArgs) error {
 
 	switch ipamConf.Type {
 	case "host-local":
-		ipConf, err = allocator.Get(args.Netns)
+		ipConf, err = allocator.Get(args.ContainerID)
 	case "host-local-ptp":
-		ipConf, err = allocator.GetPtP(args.Netns)
+		ipConf, err = allocator.GetPtP(args.ContainerID)
 	default:
 		return errors.New("Unsupported IPAM plugin type")
 	}
@@ -81,5 +81,5 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
-	return allocator.Release(args.Netns)
+	return allocator.Release(args.ContainerID)
 }

plugins/main/bridge/bridge.go

@@ -130,7 +130,7 @@ func setupVeth(netns string, br *netlink.Bridge, ifName string, mtu int, pr *plu
 
 	err := ns.WithNetNSPath(netns, func(hostNS *os.File) error {
 		// create the veth pair in the container and move host end into host netns
-		hostVeth, _, err := ip.SetupVeth(netns, ifName, mtu, hostNS)
+		hostVeth, _, err := ip.SetupVeth(ifName, mtu, hostNS)
 		if err != nil {
 			return err
 		}

plugins/main/ipvlan/ipvlan.go

@@ -107,7 +107,11 @@ func cmdAdd(args *skel.CmdArgs) error {
 	}
 	defer netns.Close()
 
-	tmpName := ip.RandomVethName(args.Netns)
+	tmpName, err := ip.RandomVethName()
+	if err != nil {
+		return err
+	}
+
 	if err = createIpvlan(n, tmpName, netns); err != nil {
 		return err
 	}

plugins/main/macvlan/macvlan.go

@@ -111,7 +111,11 @@ func cmdAdd(args *skel.CmdArgs) error {
 	}
 	defer netns.Close()
 
-	tmpName := ip.RandomVethName(args.Netns)
+	tmpName, err := ip.RandomVethName()
+	if err != nil {
+		return err
+	}
+
 	if err = createMacvlan(n, tmpName, netns); err != nil {
 		return err
 	}

plugins/main/veth/veth.go

@@ -47,9 +47,7 @@ type NetConf struct {
 func setupContainerVeth(netns, ifName string, mtu int, pr *plugin.Result) (string, error) {
 	var hostVethName string
 	err := ns.WithNetNSPath(netns, func(hostNS *os.File) error {
-		entropy := netns + ifName
+		hostVeth, _, err := ip.SetupVeth(ifName, mtu, hostNS)
-
-		hostVeth, _, err := ip.SetupVeth(entropy, ifName, mtu, hostNS)
 		if err != nil {
 			return err
 		}
@@ -116,7 +114,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 	}
 
 	if conf.IPMasq {
-		h := sha512.Sum512([]byte(args.Netns))
+		h := sha512.Sum512([]byte(args.ContainerID))
 		chain := fmt.Sprintf("CNI-%s-%x", conf.Name, h[:8])
 		if err = ip.SetupIPMasq(&result.IP4.IP, chain); err != nil {
 			return err
@@ -143,7 +141,7 @@ func cmdDel(args *skel.CmdArgs) error {
 	}
 
 	if conf.IPMasq {
-		h := sha512.Sum512([]byte(args.Netns))
+		h := sha512.Sum512([]byte(args.ContainerID))
 		chain := fmt.Sprintf("CNI-%s-%x", conf.Name, h[:8])
 		if err = ip.TeardownIPMasq(ipn, chain); err != nil {
 			return err

scripts/docker-run.sh

@@ -9,10 +9,10 @@ contid=$(docker run -d --net=none busybox:latest /bin/sleep 10000000)
 pid=$(docker inspect -f '{{ .State.Pid }}' $contid)
 netnspath=/proc/$pid/ns/net
 
-./exec-plugins.sh add $netnspath
+./exec-plugins.sh add $contid $netnspath
 
 function cleanup() {
-	./exec-plugins.sh del $netnspath
+	./exec-plugins.sh del $contid $netnspath
 	docker kill $contid >/dev/null
 }
 trap cleanup EXIT

scripts/exec-plugins.sh

@@ -1,29 +1,36 @@
-#!/bin/bash -e
+#!/bin/bash
 
 NETCONFPATH=${NETCONFPATH-/etc/cni/net.d}
 
 function exec_plugins() {
 	i=0
-	netns=$2
+	contid=$2
+	netns=$3
 	export CNI_COMMAND=$(echo $1 | tr '[:lower:]' '[:upper:]')
 	export PATH=$CNI_PATH:$PATH
+	export CNI_CONTAINERID=$contid
 	export CNI_NETNS=$netns
 
 	for netconf in $(echo $NETCONFPATH/*.conf | sort); do
+		name=$(jq -r '.name' <$netconf)
 		plugin=$(jq -r '.type' <$netconf)
 		export CNI_IFNAME=$(printf eth%d $i)
 
 		$plugin <$netconf >/dev/null
+		if [ $? -ne 0 ]; then
+			echo "${name} : error executing $CNI_COMMAND"
+			exit 1
+		fi
 
 		let "i=i+1"
 	done
 }
 
-if [ $# -ne 2 ]; then
+if [ $# -ne 3 ]; then
-	echo "Usage: $0 add|del NETNS-PATH"
+	echo "Usage: $0 add|del CONTAINER-ID NETNS-PATH"
 	echo "  Adds or deletes the container specified by NETNS-PATH to the networks"
 	echo "  specified in \$NETCONFPATH directory"
 	exit 1
 fi
 
-exec_plugins $1 $2
+exec_plugins $1 $2 $3

scripts/priv-net-run.sh

@@ -3,18 +3,18 @@
 # Run a command in a private network namespace
 # set up by CNI plugins
 
-netnsname=$(printf '%x%x' $RANDOM $RANDOM)
+contid=$(printf '%x%x%x%x' $RANDOM $RANDOM $RANDOM $RANDOM)
-netnspath=/var/run/netns/$netnsname
+netnspath=/var/run/netns/$contid
 
-ip netns add $netnsname
+ip netns add $contid
-ip netns exec $netnsname ip link set lo up
+ip netns exec $contid ip link set lo up
-./exec-plugins.sh add $netnspath
+./exec-plugins.sh add $contid $netnspath
 
 
 function cleanup() {
-	./exec-plugins.sh del $netnspath
+	./exec-plugins.sh del $contid $netnspath
-	ip netns delete $netnsname
+	ip netns delete $contid
 }
 trap cleanup EXIT
 
-ip netns exec $netnsname $@
+ip netns exec $contid $@