diff --git a/hscontrol/db/ip.go b/hscontrol/db/ip.go index fede11b7f..0937faae3 100644 --- a/hscontrol/db/ip.go +++ b/hscontrol/db/ip.go @@ -209,9 +209,13 @@ func (i *IPAllocator) next(prev netip.Addr, prefix *netip.Prefix) (*netip.Addr, // starting point at random and then scans deterministically: this keeps // the loop finite, so an exhausted prefix returns ErrCouldNotAllocateIP // instead of re-drawing in-prefix addresses forever under i.mu. + if !prefix.Contains(ip) { + ip = prefix.Masked().Addr() + } + start := ip for { - if prefix.Contains(ip) && !set.Contains(ip) && !isTailscaleReservedIP(ip) { + if !set.Contains(ip) && !isTailscaleReservedIP(ip) { i.usedIPs.Add(ip) return &ip, nil @@ -219,24 +223,13 @@ func (i *IPAllocator) next(prev netip.Addr, prefix *netip.Prefix) (*netip.Addr, ip = ip.Next() - switch i.strategy { - case types.IPAllocationStrategySequential: - // Sequential allocation never wraps: walking past the end of - // the prefix means the pool is exhausted. - if !prefix.Contains(ip) { - return nil, ErrCouldNotAllocateIP - } - case types.IPAllocationStrategyRandom: - // Random allocation wraps within the prefix so every address is - // examined exactly once; returning to the start means the prefix - // is exhausted. - if !prefix.Contains(ip) { - ip = prefix.Masked().Addr() - } + // Wrap so addresses freed behind the cursor can be reused. + if !prefix.Contains(ip) { + ip = prefix.Masked().Addr() + } - if ip == start { - return nil, ErrCouldNotAllocateIP - } + if ip == start { + return nil, ErrCouldNotAllocateIP } } } diff --git a/hscontrol/db/ip_test.go b/hscontrol/db/ip_test.go index cf450e38d..125177c84 100644 --- a/hscontrol/db/ip_test.go +++ b/hscontrol/db/ip_test.go @@ -27,6 +27,53 @@ var nap = func(pref string) *netip.Addr { return &n } +func TestIPAllocatorSequentialReusesFreedIPs(t *testing.T) { + prefix := netip.MustParsePrefix("100.64.0.0/29") + alloc, err := NewIPAllocator( + nil, &prefix, nil, types.IPAllocationStrategySequential, + ) + require.NoError(t, err) + + issued := make([]netip.Addr, 0, 6) + + for range 6 { + ip, _, err := alloc.Next() + require.NoError(t, err) + + issued = append(issued, *ip) + } + + _, _, err = alloc.Next() + require.ErrorIs(t, err, ErrCouldNotAllocateIP) + + alloc.FreeIPs([]netip.Addr{issued[1]}) + ip, _, err := alloc.Next() + require.NoError(t, err) + require.Equal(t, issued[1], *ip) + alloc.FreeIPs([]netip.Addr{issued[0], issued[4]}) + ip, _, err = alloc.Next() + require.NoError(t, err) + require.Equal(t, issued[4], *ip, "advance before wrapping to an earlier hole") + ip, _, err = alloc.Next() + require.NoError(t, err) + require.Equal(t, issued[0], *ip) +} + +func TestIPAllocatorSequentialChurn(t *testing.T) { + prefix4 := netip.MustParsePrefix("100.64.0.0/30") + prefix6 := netip.MustParsePrefix("fd7a:115c:a1e0::/126") + alloc, err := NewIPAllocator( + nil, &prefix4, &prefix6, types.IPAllocationStrategySequential, + ) + require.NoError(t, err) + + for range 20 { + ip4, ip6, err := alloc.Next() + require.NoError(t, err) + alloc.FreeIPs([]netip.Addr{*ip4, *ip6}) + } +} + func TestIPAllocatorSequential(t *testing.T) { tests := []struct { name string