From f11314ed3ea8e309f82858bf26a09fc5ed723cd3 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 18 Mar 2016 20:09:47 +0000 Subject: [PATCH] Introduce 'Count' type to avoid abusing 'Offset' --- ipam/allocator.go | 2 +- ipam/allocator_test.go | 4 ++-- ipam/ring/entry.go | 1 + ipam/ring/ring.go | 8 ++++---- ipam/ring/ring_test.go | 4 ++-- ipam/space/space.go | 8 ++++---- ipam/space/space_test.go | 44 ++++++++++++++++++++-------------------- ipam/testutils_test.go | 4 ++-- net/address/address.go | 8 +++++++- 9 files changed, 45 insertions(+), 38 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index 819b1a49..a3ae4a90 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -795,7 +795,7 @@ func (alloc *Allocator) reportFreeSpace() { return } - freespace := make(map[address.Address]address.Offset) + freespace := make(map[address.Address]address.Count) for _, r := range ranges { freespace[r.Start] = alloc.space.NumFreeAddressesInRange(r) } diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 6845756f..de5f6c62 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -84,7 +84,7 @@ func TestAllocFree(t *testing.T) { // Move the clock forward and clear out the dead container alloc.actionChan <- func() { alloc.now = func() time.Time { return time.Now().Add(containerDiedTimeout * 2) } } alloc.actionChan <- func() { alloc.removeDeadContainers() } - require.Equal(t, address.Offset(spaceSize+1), alloc.NumFreeAddresses(subnet.Range())) + require.Equal(t, address.Count(spaceSize+1), alloc.NumFreeAddresses(subnet.Range())) } func TestBootstrap(t *testing.T) { @@ -345,7 +345,7 @@ func TestTransfer(t *testing.T) { require.NoError(t, alloc1.AdminTakeoverRanges(alloc3.ourName.String())) router.Flush() - require.Equal(t, address.Offset(1024), alloc1.NumFreeAddresses(subnet.Range())) + require.Equal(t, address.Count(1024), alloc1.NumFreeAddresses(subnet.Range())) _, err = alloc1.Allocate("foo", subnet, returnFalse) require.True(t, err == nil, "Failed to get address") diff --git a/ipam/ring/entry.go b/ipam/ring/entry.go index d7ca53e5..b4774f8f 100644 --- a/ipam/ring/entry.go +++ b/ipam/ring/entry.go @@ -15,6 +15,7 @@ type entry struct { Peer mesh.PeerName // Who owns this range Version uint32 // Version of this range Free address.Offset // Number of free IPs in this range + // Note: Free should perhaps be an address.Count, but we can't change the wire protocol } func (e *entry) Equal(e2 *entry) bool { diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index a054a368..58e320ba 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -393,7 +393,7 @@ func (r *Ring) String() string { // ReportFree is used by the allocator to tell the ring how many free // ips are in a given range, so that ChoosePeersToAskForSpace can make // more intelligent decisions. -func (r *Ring) ReportFree(freespace map[address.Address]address.Offset) { +func (r *Ring) ReportFree(freespace map[address.Address]address.Count) { r.assertInvariants() defer r.assertInvariants() defer r.updateExportedVariables() @@ -423,13 +423,13 @@ func (r *Ring) ReportFree(freespace map[address.Address]address.Offset) { // Check we're not reporting more space than the range entry, next := entries.entry(i), entries.entry(i+1) maxSize := r.distance(entry.Token, next.Token) - common.Assert(free <= maxSize) + common.Assert(free <= address.Count(maxSize)) - if entries[i].Free == free { + if address.Count(entries[i].Free) == free { return } - entries[i].Free = free + entries[i].Free = address.Offset(free) entries[i].Version++ } } diff --git a/ipam/ring/ring_test.go b/ipam/ring/ring_test.go index fa79c9d4..bdf3bc73 100644 --- a/ipam/ring/ring_test.go +++ b/ipam/ring/ring_test.go @@ -374,7 +374,7 @@ func TestFindFree(t *testing.T) { assertPeersWithSpace(t, ring1, start, end, 0) // We shouldn't return outselves - ring1.ReportFree(map[address.Address]address.Offset{start: 10}) + ring1.ReportFree(map[address.Address]address.Count{start: 10}) assertPeersWithSpace(t, ring1, start, end, 0) ring1.Entries = []*entry{{Token: start, Peer: peer1name, Free: 1}, @@ -409,7 +409,7 @@ func TestReportFree(t *testing.T) { ring1.GrantRangeToHost(middle, end, peer2name) require.NoError(t, ring2.Merge(*ring1)) - freespace := make(map[address.Address]address.Offset) + freespace := make(map[address.Address]address.Count) for _, r := range ring2.OwnedRanges() { freespace[r.Start] = 0 } diff --git a/ipam/space/space.go b/ipam/space/space.go index aba25b46..de7bc36b 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -88,8 +88,8 @@ func (s *Space) Claim(addr address.Address) error { return nil } -func (s *Space) NumFreeAddressesInRange(r address.Range) address.Offset { - res := address.Offset(0) +func (s *Space) NumFreeAddressesInRange(r address.Range) address.Count { + res := address.Count(0) s.walkFree(r, func(chunk address.Range) bool { res += chunk.Size() return false @@ -111,7 +111,7 @@ func (s *Space) Free(addr address.Address) error { } func (s *Space) biggestFreeRange(r address.Range) (biggest address.Range) { - biggestSize := address.Offset(0) + biggestSize := address.Count(0) s.walkFree(r, func(chunk address.Range) bool { if size := chunk.Size(); size >= biggestSize { biggest = chunk @@ -131,7 +131,7 @@ func (s *Space) Donate(r address.Range) (address.Range, bool) { // 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, biggest.Size()/2) + 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 7897a7f4..3b3290ac 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -21,10 +21,10 @@ func ip(s string) address.Address { // Helper function to avoid 'NumFreeAddressesInRange(start, end)' // dozens of times in tests -func (s *Space) NumFreeAddresses() address.Offset { - res := address.Offset(0) +func (s *Space) NumFreeAddresses() address.Count { + res := address.Count(0) for i := 0; i < len(s.free); i += 2 { - res += address.Subtract(s.free[i+1], s.free[i]) + res += address.Length(s.free[i+1], s.free[i]) } return res } @@ -61,19 +61,19 @@ func TestLowlevel(t *testing.T) { require.Equal(t, []address.Address{}, a) s := New() - require.Equal(t, address.Offset(0), s.NumFreeAddresses()) + require.Equal(t, address.Count(0), s.NumFreeAddresses()) ok, got := s.Allocate(address.NewRange(0, 1000)) require.False(t, ok, "allocate in empty space should fail") s.Add(100, 100) - require.Equal(t, address.Offset(100), s.NumFreeAddresses()) + require.Equal(t, address.Count(100), s.NumFreeAddresses()) ok, got = s.Allocate(address.NewRange(0, 1000)) require.True(t, ok && got == 100, "allocate") - require.Equal(t, address.Offset(99), s.NumFreeAddresses()) + require.Equal(t, address.Count(99), s.NumFreeAddresses()) require.NoError(t, s.Claim(150)) - require.Equal(t, address.Offset(98), s.NumFreeAddresses()) + require.Equal(t, address.Count(98), s.NumFreeAddresses()) require.NoError(t, s.Free(100)) - require.Equal(t, address.Offset(99), s.NumFreeAddresses()) + require.Equal(t, address.Count(99), s.NumFreeAddresses()) wt.AssertErrorInterface(t, (*error)(nil), s.Free(0), "free not allocated") wt.AssertErrorInterface(t, (*error)(nil), s.Free(100), "double free") @@ -107,19 +107,19 @@ func TestSpaceAllocate(t *testing.T) { ) space1 := makeSpace(start, size) - require.Equal(t, address.Offset(20), space1.NumFreeAddresses()) + require.Equal(t, address.Count(20), space1.NumFreeAddresses()) space1.assertInvariants() _, addr1 := space1.Allocate(address.NewRange(start, size)) require.Equal(t, testAddr1, addr1.String(), "address") - require.Equal(t, address.Offset(19), space1.NumFreeAddresses()) + require.Equal(t, address.Count(19), space1.NumFreeAddresses()) space1.assertInvariants() _, addr2 := space1.Allocate(address.NewRange(start, size)) require.False(t, addr2.String() == testAddr1, "address") - require.Equal(t, address.Offset(18), space1.NumFreeAddresses()) - require.Equal(t, address.Offset(13), space1.NumFreeAddressesInRange(address.Range{Start: ip(testAddr1), End: ip(testAddrx)})) - require.Equal(t, address.Offset(18), space1.NumFreeAddressesInRange(address.Range{Start: ip(testAddr1), End: ip(testAddry)})) + require.Equal(t, address.Count(18), space1.NumFreeAddresses()) + require.Equal(t, address.Count(13), space1.NumFreeAddressesInRange(address.Range{Start: ip(testAddr1), End: ip(testAddrx)})) + require.Equal(t, address.Count(18), space1.NumFreeAddressesInRange(address.Range{Start: ip(testAddr1), End: ip(testAddry)})) space1.assertInvariants() space1.Free(addr2) @@ -172,7 +172,7 @@ func TestSpaceFree(t *testing.T) { require.NoError(t, space.Free(ip("10.0.3.22"))) require.NoError(t, space.Free(ip("10.0.3.21"))) - require.Equal(t, address.Offset(4), space.NumFreeAddresses()) + require.Equal(t, address.Count(4), space.NumFreeAddresses()) // Now get the biggest free space; should be 3.21 r = space.biggestFreeRange(entireRange) @@ -183,7 +183,7 @@ func TestSpaceFree(t *testing.T) { require.NoError(t, space.Free(ip("10.0.3.11"))) require.NoError(t, space.Free(ip("10.0.3.10"))) - require.Equal(t, address.Offset(7), space.NumFreeAddresses()) + require.Equal(t, address.Count(7), space.NumFreeAddresses()) // Now get the biggest free space; should be 3.21 r = space.biggestFreeRange(entireRange) @@ -210,8 +210,8 @@ func TestDonateSimple(t *testing.T) { numGivenUp := r.Size() require.True(t, ok, "Donate result") require.Equal(t, "10.0.1.24", r.Start.String(), "Invalid start") - require.Equal(t, address.Offset(size/2), numGivenUp) - require.Equal(t, address.Offset(size/2), ps1.NumFreeAddresses()) + require.Equal(t, address.Count(size/2), numGivenUp) + require.Equal(t, address.Count(size/2), ps1.NumFreeAddresses()) // Now check we can give the rest up. count := 0 // count to avoid infinite loop @@ -222,8 +222,8 @@ func TestDonateSimple(t *testing.T) { } numGivenUp += r.Size() } - require.Equal(t, address.Offset(0), ps1.NumFreeAddresses()) - require.Equal(t, address.Offset(size), numGivenUp) + require.Equal(t, address.Count(0), ps1.NumFreeAddresses()) + require.Equal(t, address.Count(size), numGivenUp) } func TestDonateHard(t *testing.T) { @@ -240,7 +240,7 @@ func TestDonateHard(t *testing.T) { require.True(t, ok, "Failed to get IP!") } - require.Equal(t, address.Offset(0), spaceset.NumFreeAddresses()) + require.Equal(t, address.Count(0), spaceset.NumFreeAddresses()) // Now free all but the last address // this will force us to split the free list @@ -252,8 +252,8 @@ func TestDonateHard(t *testing.T) { 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.Offset(24), newRange.Size()) - require.Equal(t, address.Offset(23), spaceset.NumFreeAddresses()) + require.Equal(t, address.Count(24), newRange.Size()) + require.Equal(t, address.Count(23), spaceset.NumFreeAddresses()) //Space set should now have 2 spaces expected := New() diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 32de500b..87b8f918 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -167,8 +167,8 @@ func (alloc *Allocator) claimRingForTesting(allocs ...*Allocator) { alloc.space.AddRanges(alloc.ring.OwnedRanges()) } -func (alloc *Allocator) NumFreeAddresses(r address.Range) address.Offset { - resultChan := make(chan address.Offset) +func (alloc *Allocator) NumFreeAddresses(r address.Range) address.Count { + resultChan := make(chan address.Count) alloc.actionChan <- func() { resultChan <- alloc.space.NumFreeAddressesInRange(r) } diff --git a/net/address/address.go b/net/address/address.go index 67cfee45..f3a90025 100644 --- a/net/address/address.go +++ b/net/address/address.go @@ -10,6 +10,7 @@ import ( // Using 32-bit integer to represent IPv4 address type Address uint32 type Offset uint32 +type Count uint32 type Range struct { Start, End Address // [Start, End); Start <= End @@ -18,7 +19,7 @@ type Range struct { func NewRange(start Address, size Offset) Range { return Range{Start: start, End: Add(start, size)} } -func (r Range) Size() Offset { return Subtract(r.End, r.Start) } +func (r Range) Size() Count { return Length(r.End, r.Start) } func (r Range) String() string { return fmt.Sprintf("%s-%s", r.Start, r.End-1) } func (r Range) Overlaps(or Range) bool { return !(r.Start >= or.End || r.End <= or.Start) } func (r Range) Contains(addr Address) bool { return addr >= r.Start && addr < r.End } @@ -116,6 +117,11 @@ func Subtract(a, b Address) Offset { return Offset(a - b) } +func Length(a, b Address) Count { + common.Assert(a >= b) + return Count(a - b) +} + func Min(a, b Offset) Offset { if a > b { return b