db: wrap the sequential IP allocator so freed addresses are reused

hscontrol/db/ip.go

@@ -226,9 +226,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
@@ -236,24 +240,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
 		}
 	}
 }

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