From 4c978322979f00408c72b50931a8cdea2d5cdefc Mon Sep 17 00:00:00 2001
From: shuai-z <zs.broccoli@gmail.com>
Date: Wed, 22 Oct 2014 15:12:03 +0800
Subject: [PATCH] fixed the way of iterating over the range of map.

Fixed the following errors:
1. Request(0) causes a dead loop when the map is full and map.last == BEGIN.
2. When map.last is the only available port (or ip), Request(0) returns ErrAllPortsAllocated (or ErrNoAvailableIPs). Exception is when map.last == BEGIN.

Signed-off-by: shuai-z <zs.broccoli@gmail.com>
---
 daemon/networkdriver/ipallocator/allocator.go |  5 +-
 .../ipallocator/allocator_test.go             | 59 +++++++++++++++++++
 .../portallocator/portallocator.go            |  4 +-
 .../portallocator/portallocator_test.go       | 13 ++++
 4 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/daemon/networkdriver/ipallocator/allocator.go b/daemon/networkdriver/ipallocator/allocator.go
index 3f60d2d065..a8625c0300 100644
--- a/daemon/networkdriver/ipallocator/allocator.go
+++ b/daemon/networkdriver/ipallocator/allocator.go
@@ -129,7 +129,10 @@ func (allocated *allocatedMap) checkIP(ip net.IP) (net.IP, error) {
 // return an available ip if one is currently available.  If not,
 // return the next available ip for the nextwork
 func (allocated *allocatedMap) getNextIP() (net.IP, error) {
-	for pos := big.NewInt(0).Add(allocated.last, big.NewInt(1)); pos.Cmp(allocated.last) != 0; pos.Add(pos, big.NewInt(1)) {
+	pos := big.NewInt(0).Set(allocated.last)
+	allRange := big.NewInt(0).Sub(allocated.end, allocated.begin)
+	for i := big.NewInt(0); i.Cmp(allRange) <= 0; i.Add(i, big.NewInt(1)) {
+		pos.Add(pos, big.NewInt(1))
 		if pos.Cmp(allocated.end) == 1 {
 			pos.Set(allocated.begin)
 		}
diff --git a/daemon/networkdriver/ipallocator/allocator_test.go b/daemon/networkdriver/ipallocator/allocator_test.go
index c4ce40cd0a..8e0e853fac 100644
--- a/daemon/networkdriver/ipallocator/allocator_test.go
+++ b/daemon/networkdriver/ipallocator/allocator_test.go
@@ -432,6 +432,65 @@ func TestAllocateAllIps(t *testing.T) {
 	}
 
 	assertIPEquals(t, first, again)
+
+	// ensure that alloc.last == alloc.begin won't result in dead loop
+	if _, err := RequestIP(network, nil); err != ErrNoAvailableIPs {
+		t.Fatal(err)
+	}
+
+	// Test by making alloc.last the only free ip and ensure we get it back
+	// #1. first of the range, (alloc.last == ipToInt(first) already)
+	if err := ReleaseIP(network, first); err != nil {
+		t.Fatal(err)
+	}
+
+	ret, err := RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, first, ret)
+
+	// #2. last of the range, note that current is the last one
+	last := net.IPv4(192, 168, 0, 254)
+	setLastTo(t, network, last)
+
+	ret, err = RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, last, ret)
+
+	// #3. middle of the range
+	mid := net.IPv4(192, 168, 0, 7)
+	setLastTo(t, network, mid)
+
+	ret, err = RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, mid, ret)
+}
+
+// make sure the pool is full when calling setLastTo.
+// we don't cheat here
+func setLastTo(t *testing.T, network *net.IPNet, ip net.IP) {
+	if err := ReleaseIP(network, ip); err != nil {
+		t.Fatal(err)
+	}
+
+	ret, err := RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, ip, ret)
+
+	if err := ReleaseIP(network, ip); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestAllocateDifferentSubnets(t *testing.T) {
diff --git a/daemon/networkdriver/portallocator/portallocator.go b/daemon/networkdriver/portallocator/portallocator.go
index e5dd077a9e..3414d11e7a 100644
--- a/daemon/networkdriver/portallocator/portallocator.go
+++ b/daemon/networkdriver/portallocator/portallocator.go
@@ -136,7 +136,9 @@ func ReleaseAll() error {
 }
 
 func (pm *portMap) findPort() (int, error) {
-	for port := pm.last + 1; port != pm.last; port++ {
+	port := pm.last
+	for i := 0; i <= EndPortRange-BeginPortRange; i++ {
+		port++
 		if port > EndPortRange {
 			port = BeginPortRange
 		}
diff --git a/daemon/networkdriver/portallocator/portallocator_test.go b/daemon/networkdriver/portallocator/portallocator_test.go
index 3fb218502c..72581f1040 100644
--- a/daemon/networkdriver/portallocator/portallocator_test.go
+++ b/daemon/networkdriver/portallocator/portallocator_test.go
@@ -134,6 +134,19 @@ func TestAllocateAllPorts(t *testing.T) {
 	if newPort != port {
 		t.Fatalf("Expected port %d got %d", port, newPort)
 	}
+
+	// now pm.last == newPort, release it so that it's the only free port of
+	// the range, and ensure we get it back
+	if err := ReleasePort(defaultIP, "tcp", newPort); err != nil {
+		t.Fatal(err)
+	}
+	port, err = RequestPort(defaultIP, "tcp", 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if newPort != port {
+		t.Fatalf("Expected port %d got %d", newPort, port)
+	}
 }
 
 func BenchmarkAllocatePorts(b *testing.B) {