Store "last allocated IP" in `BlueprintSledConfig` #9550

jgallagher · 2025-12-19T18:27:31Z

This PR is large-ish but very boilerplatey. This is a prerequisite for the planner work to avoid reading expunged zones @smklein pointed out in #9521 (comment).

We add a last_allocated_ip_subnet_offset field to BlueprintSledConfig, along with a last_allocated_ip() method that adds the offset to the sled's subnet. The rest is fallout of using that field:

Many expectorate changes to add a new "last allocated IP" line
New blippy check that no sled has a zone with an IP above the last allocated IP
The planner now uses this field when creating each sled's underlay IP allocator instead of scraping "all in-service and expunged zones" to find all the ever-used-on-this-sled IPs

Staged on top of #9549.

…llocator

jgallagher · 2025-12-19T18:28:14Z

schema/crdb/blueprint-sled-last-used-ip/up02.sql

@@ -0,0 +1,59 @@
+-- Working with INET values in SQL is "fun".


This migration query is quite horrifying IMO. I would love suggestions to make it less so.

davepacheco · 2025-12-19T18:59:01Z

common/src/address.rs

 // to assume that addresses in this subnet are available.
 pub const SLED_RESERVED_ADDRESSES: u16 = 32;

+static_assertions::const_assert_eq!(


Document this?

I added this because I was concerned some places for the new field semantically wanted to use RSS_RESERVED_ADDRESSES and others were using SLED_RESERVED_ADDRESSES, and those mixed uses only worked if they both had the same value. With the switch to the newtype, I don't think that's true anymore (because it enforces a minimum, and if RSS_RESERVED_ADDRESSES went higher that would be fine), so I just took this back out.

davepacheco · 2025-12-19T19:02:23Z

nexus/reconfigurator/execution/src/database.rs

            BlueprintSledConfig {
                state: SledState::Active,
                subnet: Ipv6Subnet::new(Ipv6Addr::LOCALHOST),
+                last_allocated_ip_subnet_offset: RSS_RESERVED_ADDRESSES,


Since this is used in a lot of tests and the connection feels pretty non-obvious, I wonder if we could document this better somehow? Maybe another constant (TESTING_INITIAL_LAST_ALLOCATED_IP_SUBNET_OFFSET is maybe long...) with a comment about it?

I really don't love another constant; we already have both RSS_RESERVED_ADDRESSES and SLED_RESERVED_ADDRESSES that both mean "the starting point of what Reconfigurator can use". I think we only have two because SLED_RESERVED_ADDRESSES is supposed to be for special IPs like "gz" and "switch zone", except RSS stomps all over it when assigning zone IPs.

I did briefly consider making this a newtype instead of a raw u16; then these could all be something like LastAllocatedIpOffset::initial(). Would that be better?

I don't think it's too bad to have a third constant, in that they're all right near each other, documented together, and have static asserts. In fact, I wouldn't define this one to be 32 -- I'd define it as the value of that other constant explicitly. More like an alias.

A different type with a named constructor would also work. You don't need a separate type to make it a free function... but then it may as well be a constant.

I went with a newtype in 27a7e0d - what do you think?

davepacheco · 2025-12-19T19:18:50Z

schema/crdb/dbinit.sql

    subnet INET NOT NULL,

+    -- the last allocated IP within `subnet` used by the blueprint
+    last_allocated_ip_subnet_offset INT4 NOT NULL,


What do you think about a constraint between 0 and 65K?

Added in 61502ee

davepacheco · 2025-12-19T19:32:34Z

schema/crdb/blueprint-sled-last-used-ip/up02.sql

+                    -- Final arg to `substr()`: this trims off the leading
+                    -- `::` in the `::NNNN` we produced above.
+                    3),
+                -- Final args to `lpad()`: this ensure we always have
+                -- exactly 4 hex digits, left padding with 0 if needed.
+                4, '0')


These seem aligned weirdly for the functions that they're arguments to?

davepacheco · 2025-12-19T19:48:34Z

schema/crdb/blueprint-sled-last-used-ip/up02.sql

+            SELECT ('x' ||
+                lpad(
+                    substr(
+                        -- bitwise & the IP with a /112 hostmask, which
+                        -- squishes this IP down to the string '::NNNN',
+                        -- where there will be 1-4 hex-valued X characters.
+                        host(primary_service_ip & hostmask('::/112')),
+                    -- Final arg to `substr()`: this trims off the leading
+                    -- `::` in the `::NNNN` we produced above.
+                    3),
+                -- Final args to `lpad()`: this ensure we always have
+                -- exactly 4 hex digits, left padding with 0 if needed.
+                4, '0')
+            -- We concatenated a literal 'x' prefix onto the front of the
+            -- `NNNN` value we just produced, which causes `bit(16)` to
+            -- interpret it as a 16-bit hex value (which it is!). Then
+            -- convert that bitstring into an integer.
+            )::bit(16)::int AS final_hextet


ChatGPT tells me and I've casually confirmed that you can treat inet values as integers. So this works in dogfood to compute the offset for a given row:

root@[fd00:1122:3344:10b::3]:32221/omicron> select sled_id,primary_service_ip,((primary_service_ip & hostmask('::/112')) - '::'::inet) as offset from bp_omicron_zone where blueprint_id = '01155c9f-f124-41b1-aafd-0bc132d22598'; sled_id | primary_service_ip | offset ---------------------------------------+------------------------+--------- b886b58a-1e3f-4be1-b9f2-0c2e66c6bc88 | fd00:1122:3344:106::c | 12 0c7011f7-a4bf-4daf-90cc-1c2410103300 | fd00:1122:3344:2::1 | 1 f15774c1-b8e5-434f-a493-ec43f96cba06 | fd00:1122:3344:105::28 | 40 ... 7b862eb6-7f50-4c2f-b9a6-0d12ac913d3c | fd00:1122:3344:101::45 | 69 f15774c1-b8e5-434f-a493-ec43f96cba06 | fd00:1122:3344:105::22 | 34 b886b58a-1e3f-4be1-b9f2-0c2e66c6bc88 | fd00:1122:3344:106::d | 13 (698 rows)

Assuming I did the right thing, maybe that could clean a bunch of this up?

... yeah that's way better. Thanks

Changed in 8d55a7d

jgallagher added 5 commits December 19, 2025 13:22

initial: add last_allocated_ip_subnet_offset to blueprint

82cf685

use last_allocated_ip_subnet_offset when constructing SledUnderlayIpA…

ce625ff

…llocator

schema migration

5b9dd05

openapi

9450b58

add blippy check for IP above last allocated

96e1936

jgallagher requested review from davepacheco and smklein December 19, 2025 18:27

jgallagher commented Dec 19, 2025

View reviewed changes

davepacheco reviewed Dec 19, 2025

View reviewed changes

jgallagher added 3 commits December 19, 2025 15:14

Dave saves ~~Christmas~~ data migrations

8d55a7d

add check constraint for u16 column

61502ee

add LastAllocatedSubnetIpOffset newtype

27a7e0d

jgallagher requested a review from davepacheco December 19, 2025 21:14

		@@ -0,0 +1,59 @@
		-- Working with INET values in SQL is "fun".

Store "last allocated IP" in BlueprintSledConfig #9550

Are you sure you want to change the base?

Store "last allocated IP" in BlueprintSledConfig #9550

Uh oh!

Conversation

jgallagher commented Dec 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Store "last allocated IP" in `BlueprintSledConfig` #9550

Store "last allocated IP" in `BlueprintSledConfig` #9550