From e77729c2e081c5ce55aa1d4f316b7c7703e5de96 Mon Sep 17 00:00:00 2001 From: Erik Hollensbe Date: Thu, 26 Jun 2014 00:09:19 -0700 Subject: [PATCH] Use last allocated port logic in port allocator Docker-DCO-1.1-Signed-off-by: Erik Hollensbe (github: erikh) --- daemon/networkdriver/bridge/driver.go | 17 ++++--- .../portallocator/portallocator.go | 49 ++++++++++++------- daemon/networkdriver/portmapper/mapper.go | 24 ++++----- 3 files changed, 49 insertions(+), 41 deletions(-) diff --git a/daemon/networkdriver/bridge/driver.go b/daemon/networkdriver/bridge/driver.go index 16439d2aca..a843da0499 100644 --- a/daemon/networkdriver/bridge/driver.go +++ b/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 } diff --git a/daemon/networkdriver/portallocator/portallocator.go b/daemon/networkdriver/portallocator/portallocator.go index 3b01e28a30..c722ba98ba 100644 --- a/daemon/networkdriver/portallocator/portallocator.go +++ b/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 { diff --git a/daemon/networkdriver/portmapper/mapper.go b/daemon/networkdriver/portmapper/mapper.go index d4469cd6d0..1bd332271f 100644 --- a/daemon/networkdriver/portmapper/mapper.go +++ b/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