diff --git a/ipam/allocator.go b/ipam/allocator.go index 0949658a..809605e0 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -642,15 +642,14 @@ func (alloc *Allocator) donateSpace(subnet address.CIDR, to router.PeerName) { defer alloc.sendRingUpdate(to) alloc.debugln("Peer", to, "asked me for space") - start, size, ok := alloc.space.Donate(subnet.Start, subnet.End()) + start, end, ok := alloc.space.Donate(subnet.Start, subnet.End()) if !ok { free := alloc.space.NumFreeAddressesInRange(subnet.Start, subnet.End()) common.Assert(free == 0) alloc.debugln("No space to give to peer", to) return } - end := address.Add(start, size) - alloc.debugln("Giving range", start, end, size, "to", to) + alloc.debugln("Giving range", start, end, "to", to) alloc.ring.GrantRangeToHost(start, end, to) } diff --git a/ipam/space/space.go b/ipam/space/space.go index c0ddcb57..b456e0df 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -39,7 +39,7 @@ func (s *Space) Clear() { // Walk down the free list calling f() on the in-range portions, until // f() returns true or we run out of free space. Return true iff f() returned true -func (s *Space) walkFree(rangeStart, rangeEnd address.Address, f func(pos int, start, end address.Address) bool) bool { +func (s *Space) walkFree(rangeStart, rangeEnd address.Address, f func(start, end address.Address) bool) bool { if rangeStart >= rangeEnd { // degenerate case return false } @@ -62,7 +62,7 @@ func (s *Space) walkFree(rangeStart, rangeEnd address.Address, f func(pos int, s end = rangeEnd } // at this point we know start= biggest { - pos = i - biggest = size + if size >= biggestSize { + biggestStart, biggestEnd = start, end + biggestSize = size } return false }) - return pos, biggest + return } -func (s *Space) Donate(rangeStart, rangeEnd address.Address) (address.Address, address.Offset, bool) { - if len(s.free) == 0 { +func (s *Space) Donate(rangeStart, rangeEnd address.Address) (address.Address, address.Address, bool) { + start, end := s.biggestFreeRange(rangeStart, rangeEnd) + size := address.Subtract(end, start) + + if size == 0 { return 0, 0, false } - pos, biggest := s.biggestFreeRange(rangeStart, rangeEnd) - - // Donate half of that biggest free range, rounding up so - // that the donation can't be empty - end := s.free[pos+1] - start := end - address.Address((biggest+1)/2) + // Donate half of that biggest free range. Note size/2 rounds down, so + // the resulting donation size rounds up, and in particular can't be empty. + start = address.Add(start, size/2) s.ours = subtract(s.ours, start, end) s.free = subtract(s.free, start, end) - return start, address.Subtract(end, start), true + return start, end, true } func firstGreater(a []address.Address, x address.Address) int { diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 12ede1e4..785b17f5 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -77,19 +77,19 @@ func TestLowlevel(t *testing.T) { wt.AssertErrorInterface(t, s.Free(0), (*error)(nil), "free not allocated") wt.AssertErrorInterface(t, s.Free(100), (*error)(nil), "double free") - start, size, ok := s.Donate(0, 1000) - wt.AssertTrue(t, ok && start == 125 && size == 25, "donate") + start, end, ok := s.Donate(0, 1000) + wt.AssertTrue(t, ok && start == 125 && address.Subtract(end, start) == 25, "donate") // test Donate when addresses are scarce s = New() - start, size, ok = s.Donate(0, 1000) + start, end, ok = s.Donate(0, 1000) wt.AssertTrue(t, !ok, "donate on empty space should fail") s.Add(0, 3) wt.AssertNoErr(t, s.Claim(0)) wt.AssertNoErr(t, s.Claim(2)) - start, size, ok = s.Donate(0, 1000) - wt.AssertTrue(t, ok && start == 1 && size == 1, "donate") - start, size, ok = s.Donate(0, 1000) + start, end, ok = s.Donate(0, 1000) + wt.AssertTrue(t, ok && start == 1 && end == 2, "donate") + start, end, ok = s.Donate(0, 1000) wt.AssertTrue(t, !ok, "donate should fail") } @@ -144,8 +144,8 @@ func TestSpaceFree(t *testing.T) { space := makeSpace(ip(testAddr1), 20) // Check we are prepared to give up the entire space - pos, size := space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip(testAddr1) && size == 20, "Wrong space") + start, end := space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip(testAddr1) && address.Subtract(end, start) == 20, "Wrong space") for i := 0; i < 20; i++ { ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) @@ -155,18 +155,18 @@ func TestSpaceFree(t *testing.T) { // Check we are full ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, !ok, "Should have failed to get address") - _, size, ok = space.Donate(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, size == 0, "Wrong space") + start, end, ok = space.Donate(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, address.Subtract(end, start) == 0, "Wrong space") // Free in the middle wt.AssertSuccess(t, space.Free(ip("10.0.3.13"))) - pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip("10.0.3.13") && size == 1, "Wrong space") + start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip("10.0.3.13") && address.Subtract(end, start) == 1, "Wrong space") // Free one at the end wt.AssertSuccess(t, space.Free(ip("10.0.3.23"))) - pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip("10.0.3.23") && size == 1, "Wrong space") + start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip("10.0.3.23") && address.Subtract(end, start) == 1, "Wrong space") // Now free a few at the end wt.AssertSuccess(t, space.Free(ip("10.0.3.22"))) @@ -175,8 +175,8 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.NumFreeAddresses(), address.Offset(4)) // Now get the biggest free space; should be 3.21 - pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip("10.0.3.21") && size == 3, "Wrong space") + start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip("10.0.3.21") && address.Subtract(end, start) == 3, "Wrong space") // Now free a few in the middle wt.AssertSuccess(t, space.Free(ip("10.0.3.12"))) @@ -186,8 +186,8 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.NumFreeAddresses(), address.Offset(7)) // Now get the biggest free space; should be 3.21 - pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip("10.0.3.10") && size == 4, "Wrong space") + start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip("10.0.3.10") && address.Subtract(end, start) == 4, "Wrong space") wt.AssertEquals(t, space.OwnedRanges(), []address.Range{address.Range{Start: ip("10.0.3.4"), End: ip("10.0.3.24")}}) } @@ -210,7 +210,8 @@ func TestDonateSimple(t *testing.T) { ps1 := makeSpace(ipAddr1, size) // Empty space set should split in two and give me the second half - start, numGivenUp, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) + start, end, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) + numGivenUp := address.Subtract(end, start) wt.AssertTrue(t, ok, "Donate result") wt.AssertEqualString(t, start.String(), "10.0.1.24", "Invalid start") wt.AssertEquals(t, numGivenUp, address.Offset(size/2)) @@ -219,11 +220,11 @@ func TestDonateSimple(t *testing.T) { // Now check we can give the rest up. count := 0 // count to avoid infinite loop for ; count < 1000; count++ { - _, size, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) + start, end, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) if !ok { break } - numGivenUp += size + numGivenUp += address.Subtract(end, start) } wt.AssertEquals(t, ps1.NumFreeAddresses(), address.Offset(0)) wt.AssertEquals(t, numGivenUp, address.Offset(size)) @@ -253,10 +254,10 @@ func TestDonateHard(t *testing.T) { } // Now split - newRange, numGivenUp, ok := spaceset.Donate(start, end) + newRange, end, ok := spaceset.Donate(start, end) wt.AssertTrue(t, ok, "GiveUpSpace result") wt.AssertEquals(t, newRange, ip("10.0.1.23")) - wt.AssertEquals(t, numGivenUp, address.Offset(24)) + wt.AssertEquals(t, address.Subtract(end, newRange), address.Offset(24)) wt.AssertEquals(t, spaceset.NumFreeAddresses(), address.Offset(23)) //Space set should now have 2 spaces