Skip to content

[common] be less confusing about what a "prefix" is#10872

Merged
hawkw merged 7 commits into
mainfrom
eliza/what-even-is-a-prefix-anyway
Jul 21, 2026
Merged

[common] be less confusing about what a "prefix" is#10872
hawkw merged 7 commits into
mainfrom
eliza/what-even-is-a-prefix-anyway

Conversation

@hawkw

@hawkw hawkw commented Jul 21, 2026

Copy link
Copy Markdown
Member

The omicron_common::address module contains a whole bunch of constants whose names end with or otherwise contain the word "prefix" (or, in this case, the word "PREFIX"). Unfortunately, this word is used somewhat inconsistently. In some cases, "prefix" means "the length of a prefix of a subnet", such as BOOTSTRAP_SUBNET_PREFIX, which describes a /40 network, and therefore unsurprisingly is the number 40, or RACK_PREFIX and AZ_PREFIX, which are 56 and 48, respectively. These constants are typically used as const generic parameters to the Ipv6Subnet type, which is generic over the length of a prefix. In other cases, we use the word "prefix" to refer to the "first octets in a subnet", such as IPV6_MULTICAST_PREFIX and
IPV6_ADMIN_SCOPED_MULTICAST_PREFIX, which are ff00 and ff04, and respectivefly describe the first octet or first two octets in subnets which are /8 and /16, respectively.

This was a bit confusing for me to discover that sometimes ending with "_PREFIX" means a constant contains one thing, and sometimes it means another thing. If one were to use one kind of _PREFIX constant in a place where the other kind is expected, I can imagine things going quite wrong, although this is helped a bit by the ones that represent prefix lengths typically being u8s and the ones that represent IPv6 octet pairs being, naturally, octet pairs, i.e. u16s. Luckily, one cannot accidentally insert a u16 into a u8-shaped hole, or vice versa, so we haven't messed this up yet, at least as far as I can tell.

However, there is another annoying issue here, which is that I would quite like to move the constant BOOTSTRAP_PREFIX, which represents the first two octets of the bootstrap network, from sled_hardware_types::underlay to omicron_common::address.1 I'd like to be able to reference it in Nexus in #10835. Sadly, as I mentioned above, there is already a BOOTSTRAP_SUBNET_PREFIX here, which represents the length of a prefix, rather than the actual octets. Having both BOOTSTRAP_PREFIX and BOOTSTRAP_SUBNET_PREFIX, with different meanings, seems bad and unfortunate.

Therefore, this branch adopts a more consistent naming scheme, where constants that represent the length of a prefix are always named FOO_PREFIX_LENGTH, while constants which represent the first octet(s) of a subnet, often beginning with the letter f, are named FOO_PREFIX. Doing this touches a lot of files, but reviewers should really only need to look closely at common/src/address.rs, as the rest are just places where these various constants are referenced. I would love to have a network guy or two fact-check whether I am using these terms correctly. Thanks!

AI Usage Disclosure: This was a large, mechanical change, so I used an AI tool called "the rust-analyzer Rename Symbol button" to assist in making this change.

Footnotes

  1. Which it's only currently referenced in sled-hardware, which depends on omicron-common as well as on sled-hardware-types.

@hawkw hawkw self-assigned this Jul 21, 2026
@hawkw hawkw changed the title [common] be less confusing about what a "prefix" [common] be less confusing about what a "prefix" is Jul 21, 2026
hawkw added 2 commits July 20, 2026 17:51
The `omicron_common::address` module contains a whole bunch of constants
whose names end with or otherwise contain the word "prefix" (or, in this
case, the word "PREFIX"). Unfortunately, this word is used somewhat
inconsistently. In some cases, "prefix" means "the _length_ of a prefix
of a subnet", such as [`BOOTSTRAP_SUBNET_PREFIX`], which describes a /40
network, and therefore unsurprisingly is the number 40, or
[`RACK_PREFIX`] and [`AZ_PREFIX`], which are 56 and 48, respectively.
These constants are typically used as const generic parameters to the
`Ipv6Subnet` type, which is generic over the length of a prefix. In
other cases, we use the word "prefix" to refer to the "first octets in a
subnet", such as [`IPV6_MULTICAST_PREFIX`] and
`IPV6_ADMIN_SCOPED_MULTICAST_PREFIX`, which are `ff00` and `ff04`, and
respectivefly describe the first octet or frist two octets in subnets
which are /8 and /16, respectively.

This was a bit confusing for me to discover that sometimes ending with
"_PREFIX" means a constant contains one thing, and sometimes it means
another thing. If one were to use one kind of `_PREFIX` constant in a
place where the other kind is expected, I can imagine things going quite
wrong, although this is helped a bit by the ones that represent prefix
lengths typically being `u8`s and the ones that represent IPv6 octet
pairs being, naturally, octet _pairs_, i.e. `u16`s. Luckily, one cannot
accidentally insert a `u16` into a `u8`-shaped hole, or vice versa, so
we haven't messed this up _yet_, at least as far as I can tell.

However, there is another annoying issue here, which is that I would
quite like to move the constant [`BOOTSTRAP_PREFIX`], which represents
the first two octets of the bootstrap network, from
`sled_hardware_types::underlay` to `omicron_common::address`.[^1] I'd
like to be able to reference it in Nexus in #10835. Sadly, as I
mentioned above, there is already a `BOOTSTRAP_SUBNET_PREFIX` here,
which represents the _length_ of a prefix, rather than the actual
octets. Having both `BOOTSTRAP_PREFIX` and `BOOTSTRAP_SUBNET_PREFIX`,
with different meanings, seems bad and unfortunate.

Therefore, this branch adopts a more consistent naming scheme, where
constants that represent the length of a prefix are always named
`FOO_PREFIX_LENGTH`, and constants which represent the first octet(s) of
a subnet, often beginning with the letter `f`, are named `FOO_PREFIX`.
Doing this touches a lot of files, but reviewers should really only need
to look closely at `common/src/address.rs`, as the rest are just places
where these various constants are referenced. I would love to have a
network guy or two fact-check whether I am using these terms correctly.
Thanks!

**AI Usage Disclosure**: This was a large, mechanical change, so I used
an AI tool called "the rust-analyzer `Rename Symbol` button" to assist
in making this change.

[^1]: Which it's only currently referenced in `sled-hardware`, which
depends on `omicron-common` as well as on `sled-hardware-types`.

[`BOOTSTRAP_SUBNET_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L22
[`AZ_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L23
[`RACK_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L24
[`IPV6_MULTICAST_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L120
[`IPV6_ADMIN_SCOPED_MULTICAST_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L130
[`BOOTSTRAP_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/sled-hardware/types/src/underlay.rs#L6
@hawkw
hawkw force-pushed the eliza/what-even-is-a-prefix-anyway branch from a0c4679 to f2359d1 Compare July 21, 2026 00:51

@jgallagher jgallagher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up! I think Levon and I had a similar conversation not too long ago where it wasn't clear which name to pick because of the same inconsistencies you noted. As requested I really just looked at common/src/address.rs - changes LGTM, with a few questions about whether there are a couple more spots to rename.

Comment thread common/src/address.rs
/// See [RFC 5771] for IPv4 multicast address assignments.
///
/// [RFC 5771]: https://www.rfc-editor.org/rfc/rfc5771
pub const IPV4_MULTICAST_RANGE: Ipv4Net =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily for this PR, but while we're asking about naming consistency of these constants - what's the difference between *_RANGE and *_SUBNET? (Both are of type Ipv*Net)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think we may want to rename some of these too.

Comment thread common/src/address.rs Outdated
Comment thread common/src/address.rs Outdated
@hawkw
hawkw enabled auto-merge (squash) July 21, 2026 17:55
@hawkw
hawkw merged commit a3012e0 into main Jul 21, 2026
19 checks passed
@hawkw
hawkw deleted the eliza/what-even-is-a-prefix-anyway branch July 21, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants