From 64dc7a1f69613eb21a8de5a02a0507d78eefcd01 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 11 May 2016 12:05:45 +0100 Subject: [PATCH] Donate power-of-2-aligned ranges instead of just slicing at the mid-point. Also fix up space tests to match. --- ipam/allocator_test.go | 2 +- ipam/space/space.go | 16 +++++++--- ipam/space/space_test.go | 63 +++++++++++++++++++------------------ net/address/address.go | 29 +++++++++++++++++ net/address/address_test.go | 26 +++++++++++++++ 5 files changed, 99 insertions(+), 37 deletions(-) create mode 100644 net/address/address_test.go diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index af05530a..40401052 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -343,7 +343,7 @@ func TestNoFrag(t *testing.T) { allocs[2].actionChan <- func() { resultChan <- len(allocs[2].ring.Entries) } - require.True(t, <-resultChan < 5, "excessive ring fragmentation") + require.True(t, <-resultChan < 7, "excessive ring fragmentation") stopNetworkOfAllocators(allocs, router) } } diff --git a/ipam/space/space.go b/ipam/space/space.go index 80d99c97..41594898 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -122,8 +122,11 @@ func (s *Space) biggestFreeRange(r address.Range) (biggest address.Range) { biggestSize := address.Count(0) s.walkFree(r, func(chunk address.Range) bool { if size := chunk.Size(); size >= biggestSize { - biggest = chunk - biggestSize = size + chunk = chunk.BiggestPow2AlignedRange() + if size = chunk.Size(); size >= biggestSize { + biggest = chunk + biggestSize = size + } } return false }) @@ -137,9 +140,12 @@ func (s *Space) Donate(r address.Range) (address.Range, bool) { return address.Range{}, false } - // 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. - biggest.Start = address.Add(biggest.Start, address.Offset(biggest.Size()/2)) + // Donate no more than half what we have available + if biggest.Size() > s.NumFreeAddressesInRange(r)/2 { + // Note size/2 rounds down, so the resulting donation size + // rounds up, and in particular can't be empty. + biggest.Start = address.Add(biggest.Start, address.Offset(biggest.Size()/2)) + } s.ours = subtract(s.ours, biggest.Start, biggest.End) s.free = subtract(s.free, biggest.Start, biggest.End) diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 295cfd57..486410f5 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -68,7 +68,7 @@ func TestLowlevel(t *testing.T) { wt.AssertErrorInterface(t, (*error)(nil), s.Free(100), "double free") r, ok := s.Donate(address.NewRange(0, 1000)) - require.True(t, ok && r.Start == 125 && r.Size() == 25, "donate") + require.Equal(t, address.NewRange(0xa0, 0x20), r, "donate") // test Donate when addresses are scarce s = New() @@ -78,7 +78,7 @@ func TestLowlevel(t *testing.T) { require.NoError(t, s.Claim(0)) require.NoError(t, s.Claim(2)) r, ok = s.Donate(address.NewRange(0, 1000)) - require.True(t, ok && r.Start == 1 && r.End == 2, "donate") + require.Equal(t, address.NewRange(1, 1), r, "donate") r, ok = s.Donate(address.NewRange(0, 1000)) require.True(t, !ok, "donate should fail") } @@ -124,20 +124,21 @@ func TestSpaceAllocate(t *testing.T) { func TestSpaceFree(t *testing.T) { const ( - testAddr1 = "10.0.3.4" + testAddr1 = "10.0.3.16" testAddrx = "10.0.3.19" testAddry = "10.0.9.19" containerID = "deadbeef" + size = 16 ) - entireRange := address.NewRange(ip(testAddr1), 20) - space := makeSpace(ip(testAddr1), 20) + entireRange := address.NewRange(ip(testAddr1), size) + space := makeSpace(ip(testAddr1), size) // Check we are prepared to give up the entire space r := space.biggestFreeRange(entireRange) - require.True(t, r.Start == ip(testAddr1) && r.Size() == 20, "Wrong space") + require.Equal(t, address.NewRange(ip(testAddr1), size), r, "Wrong space") - for i := 0; i < 20; i++ { + for i := 0; i < size; i++ { ok, _ := space.Allocate(entireRange) require.True(t, ok, "Failed to get address") } @@ -149,44 +150,44 @@ func TestSpaceFree(t *testing.T) { require.True(t, r.Size() == 0, "Wrong space") // Free in the middle - require.NoError(t, space.Free(ip("10.0.3.13"))) - r = space.biggestFreeRange(entireRange) - require.True(t, r.Start == ip("10.0.3.13") && r.Size() == 1, "Wrong space") - - // Free one at the end require.NoError(t, space.Free(ip("10.0.3.23"))) r = space.biggestFreeRange(entireRange) require.True(t, r.Start == ip("10.0.3.23") && r.Size() == 1, "Wrong space") + // Free one at the end + require.NoError(t, space.Free(ip("10.0.3.31"))) + r = space.biggestFreeRange(entireRange) + require.True(t, r.Start == ip("10.0.3.31") && r.Size() == 1, "Wrong space") + // Now free a few at the end - require.NoError(t, space.Free(ip("10.0.3.22"))) - require.NoError(t, space.Free(ip("10.0.3.21"))) + require.NoError(t, space.Free(ip("10.0.3.30"))) + require.NoError(t, space.Free(ip("10.0.3.29"))) require.Equal(t, address.Count(4), space.NumFreeAddresses()) - // Now get the biggest free space; should be 3.21 + // Now get the biggest free space; should be 3.30 r = space.biggestFreeRange(entireRange) - require.True(t, r.Start == ip("10.0.3.21") && r.Size() == 3, "Wrong space") + require.Equal(t, address.NewRange(ip("10.0.3.30"), 2), r, "Wrong space") // Now free a few in the middle - require.NoError(t, space.Free(ip("10.0.3.12"))) - require.NoError(t, space.Free(ip("10.0.3.11"))) - require.NoError(t, space.Free(ip("10.0.3.10"))) + require.NoError(t, space.Free(ip("10.0.3.24"))) + require.NoError(t, space.Free(ip("10.0.3.22"))) + require.NoError(t, space.Free(ip("10.0.3.21"))) require.Equal(t, address.Count(7), space.NumFreeAddresses()) - // Now get the biggest free space; should be 3.21 + // Now get the biggest free space; should be 3.30 r = space.biggestFreeRange(entireRange) - require.True(t, r.Start == ip("10.0.3.10") && r.Size() == 4, "Wrong space") + require.Equal(t, address.NewRange(ip("10.0.3.30"), 2), r, "Wrong space") - require.Equal(t, []address.Range{{Start: ip("10.0.3.4"), End: ip("10.0.3.24")}}, space.OwnedRanges()) + require.Equal(t, []address.Range{{Start: ip("10.0.3.16"), End: ip("10.0.3.32")}}, space.OwnedRanges()) } func TestDonateSimple(t *testing.T) { const ( testAddr1 = "10.0.1.0" testAddr2 = "10.0.1.32" - size = 48 + size = 64 ) var ( @@ -199,7 +200,7 @@ func TestDonateSimple(t *testing.T) { r, ok := ps1.Donate(address.NewRange(ip(testAddr1), size)) numGivenUp := r.Size() require.True(t, ok, "Donate result") - require.Equal(t, "10.0.1.24", r.Start.String(), "Invalid start") + require.Equal(t, "10.0.1.32", r.Start.String(), "Invalid start") require.Equal(t, address.Count(size/2), numGivenUp) require.Equal(t, address.Count(size/2), ps1.NumFreeAddresses()) @@ -241,13 +242,13 @@ func TestDonateHard(t *testing.T) { // Now split newRange, ok := spaceset.Donate(address.NewRange(start, size)) require.True(t, ok, "GiveUpSpace result") - require.Equal(t, ip("10.0.1.23"), newRange.Start) - require.Equal(t, address.Count(24), newRange.Size()) - require.Equal(t, address.Count(23), spaceset.NumFreeAddresses()) + require.Equal(t, address.NewRange(ip("10.0.1.16"), 16), newRange, "Wrong space") + require.Equal(t, address.Count(31), spaceset.NumFreeAddresses()) - //Space set should now have 2 spaces - expected := New() - expected.Add(start, 23) - expected.ours = add(nil, ip("10.0.1.47"), ip("10.0.1.48")) + //Space set should now have 3 spaces + expected := &Space{ + ours: add(nil, ip("10.0.1.47"), ip("10.0.1.48")), + free: add(add(nil, ip("10.0.1.0"), ip("10.0.1.16")), ip("10.0.1.32"), ip("10.0.1.47")), + } require.Equal(t, expected, spaceset) } diff --git a/net/address/address.go b/net/address/address.go index f3a90025..f3f54926 100644 --- a/net/address/address.go +++ b/net/address/address.go @@ -35,6 +35,35 @@ func (r Range) AsCIDRString() string { return CIDR{Addr: r.Start, PrefixLen: prefixLen}.String() } +// return the highest bit set in a +// algorithm from http://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2 +func msb(v Count) Count { + v |= v >> 1 + v |= v >> 2 + v |= v >> 4 + v |= v >> 8 + v |= v >> 16 + // Cast to 64-bit to avoid overflow when original v >= 0x80000000 + return Count((uint64(v) + 1) / 2) +} + +func (r Range) BiggestPow2AlignedRange() Range { + sizeMsb := Offset(msb(r.Size())) + maskedSize := Offset(r.Size()) & (sizeMsb - 1) + maskedStart := Offset(r.Start) & (sizeMsb - 1) + if maskedStart == 0 { + return NewRange(r.Start, sizeMsb) + } + if sizeMsb-maskedStart <= maskedSize { + return NewRange(Add(r.Start, sizeMsb-maskedStart), sizeMsb) + } + size := sizeMsb / 2 + if size >= maskedStart { + return NewRange(Add(r.Start, size-maskedStart), size) + } + return NewRange(Add(r.Start, sizeMsb-maskedStart), size) +} + func MakeCIDR(subnet CIDR, addr Address) CIDR { return CIDR{Addr: addr, PrefixLen: subnet.PrefixLen} } diff --git a/net/address/address_test.go b/net/address/address_test.go new file mode 100644 index 00000000..78cdceef --- /dev/null +++ b/net/address/address_test.go @@ -0,0 +1,26 @@ +package address + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBiggestPow2AlignedRange(t *testing.T) { + require.Equal(t, NewRange(0, 1), NewRange(0, 1).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(0, 2), NewRange(0, 2).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(0, 2), NewRange(0, 3).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(0, 4), NewRange(0, 7).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(1, 1), NewRange(1, 1).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(1, 1), NewRange(1, 2).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(2, 2), NewRange(1, 3).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(2, 2), NewRange(1, 4).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(4, 4), NewRange(1, 8).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(4, 4), NewRange(2, 8).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(4, 4), NewRange(4, 8).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(8, 4), NewRange(8, 7).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(8, 8), NewRange(8, 8).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(0, 0x4000), NewRange(0, 0x7fff).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(0x2000, 2), NewRange(0x2000, 3).BiggestPow2AlignedRange()) + require.Equal(t, NewRange(0xffff, 1), NewRange(0xffff, 1).BiggestPow2AlignedRange()) +}