Use last allocated port logic in port allocator

Docker-DCO-1.1-Signed-off-by: Erik Hollensbe <github@hollensbe.org> (github: erikh)

daemon/networkdriver/bridge/driver.go

@@ -20,7 +20,8 @@ import (
 )
 
 const (
-	DefaultNetworkBridge = "docker0"
+	DefaultNetworkBridge     = "docker0"
+	MaxAllocatedPortAttempts = 10
 )
 
 // Network interface represents the networking stack of a container
@@ -401,15 +402,15 @@ func AllocatePort(job *engine.Job) engine.Status {
 		return job.Errorf("unsupported address type %s", proto)
 	}
 
-	/*
-	 Try up to 10 times to get a port that's not already allocated.
-
-	 In the event of failure to bind, return the error that portmapper.Map
-	 yields.
-	*/
+	//
+	// Try up to 10 times to get a port that's not already allocated.
+	//
+	// In the event of failure to bind, return the error that portmapper.Map
+	// yields.
+	//
 
 	var host net.Addr
-	for i := 0; i < 10; i++ {
+	for i := 0; i < MaxAllocatedPortAttempts; i++ {
 		if host, err = portmapper.Map(container, ip, hostPort); err == nil {
 			break
 		}

daemon/networkdriver/portallocator/portallocator.go

@@ -7,9 +7,13 @@ import (
 	"sync"
 )
 
+type portMap struct {
+	p    map[int]struct{}
+	last int
+}
+
 type (
-	portMap     map[int]bool
-	protocolMap map[string]portMap
+	protocolMap map[string]*portMap
 	ipMapping   map[string]protocolMap
 )
 
@@ -71,8 +75,8 @@ func RequestPort(ip net.IP, proto string, port int) (int, error) {
 	mapping := getOrCreate(ip)
 
 	if port > 0 {
-		if !mapping[proto][port] {
-			mapping[proto][port] = true
+		if _, ok := mapping[proto].p[port]; !ok {
+			mapping[proto].p[port] = struct{}{}
 			return port, nil
 		} else {
 			return 0, NewErrPortAlreadyAllocated(ip.String(), port)
@@ -94,8 +98,8 @@ func ReleasePort(ip net.IP, proto string, port int) error {
 
 	ip = getDefault(ip)
 
-	mapping := getOrCreate(ip)
-	delete(mapping[proto], port)
+	mapping := getOrCreate(ip)[proto]
+	delete(mapping.p, port)
 
 	return nil
 }
@@ -114,8 +118,8 @@ func getOrCreate(ip net.IP) protocolMap {
 
 	if _, ok := globalMap[ipstr]; !ok {
 		globalMap[ipstr] = protocolMap{
-			"tcp": portMap{},
-			"udp": portMap{},
+			"tcp": &portMap{p: map[int]struct{}{}, last: 0},
+			"udp": &portMap{p: map[int]struct{}{}, last: 0},
 		}
 	}
 
@@ -123,21 +127,28 @@ func getOrCreate(ip net.IP) protocolMap {
 }
 
 func findPort(ip net.IP, proto string) (int, error) {
-	port := BeginPortRange
+	mapping := getOrCreate(ip)[proto]
 
-	mapping := getOrCreate(ip)
-
-	for mapping[proto][port] {
-		port++
-
-		if port > EndPortRange {
-			return 0, ErrAllPortsAllocated
-		}
+	if mapping.last == 0 {
+		mapping.p[BeginPortRange] = struct{}{}
+		mapping.last = BeginPortRange
+		return BeginPortRange, nil
 	}
 
-	mapping[proto][port] = true
+	for port := mapping.last + 1; port != mapping.last; port++ {
+		if port > EndPortRange {
+			port = BeginPortRange
+		}
 
-	return port, nil
+		if _, ok := mapping.p[port]; !ok {
+			mapping.p[port] = struct{}{}
+			mapping.last = port
+			return port, nil
+		}
+
+	}
+
+	return 0, ErrAllPortsAllocated
 }
 
 func getDefault(ip net.IP) net.IP {

daemon/networkdriver/portmapper/mapper.go

@@ -48,6 +48,13 @@ func Map(container net.Addr, hostIP net.IP, hostPort int) (net.Addr, error) {
 		allocatedHostPort int
 	)
 
+	// release the port on any error during return.
+	defer func() {
+		if err != nil {
+			portallocator.ReleasePort(hostIP, proto, allocatedHostPort)
+		}
+	}()
+
 	switch container.(type) {
 	case *net.TCPAddr:
 		proto = "tcp"
@@ -74,33 +81,22 @@ func Map(container net.Addr, hostIP net.IP, hostPort int) (net.Addr, error) {
 		return nil, err
 	}
 
-	// When binding fails:
-	//  - for a specifically requested port: it might be locked by some other
-	//    process, so we want to allow for an ulterior retry
-	//  - for an automatically allocated port: it falls in the Docker range of
-	//    ports, so we'll just remember it as used and try the next free one
-	defer func() {
-		if err != nil && hostPort != 0 {
-			portallocator.ReleasePort(hostIP, proto, allocatedHostPort)
-		}
-	}()
-
 	key := getKey(m.host)
 	if _, exists := currentMappings[key]; exists {
 		err = ErrPortMappedForIP
-		return m.host, err
+		return nil, err
 	}
 
 	containerIP, containerPort := getIPAndPort(m.container)
 	if err := forward(iptables.Add, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort); err != nil {
-		return m.host, err
+		return nil, err
 	}
 
 	p, err := newProxy(m.host, m.container)
 	if err != nil {
 		// need to undo the iptables rules before we return
 		forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
-		return m.host, err
+		return nil, err
 	}
 
 	m.userlandProxy = p