[common] be less confusing about what a "prefix" is#10872
Merged
Conversation
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
force-pushed
the
eliza/what-even-is-a-prefix-anyway
branch
from
July 21, 2026 00:51
a0c4679 to
f2359d1
Compare
jgallagher
reviewed
Jul 21, 2026
jgallagher
left a comment
Contributor
There was a problem hiding this comment.
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.
| /// See [RFC 5771] for IPv4 multicast address assignments. | ||
| /// | ||
| /// [RFC 5771]: https://www.rfc-editor.org/rfc/rfc5771 | ||
| pub const IPV4_MULTICAST_RANGE: Ipv4Net = |
Contributor
There was a problem hiding this comment.
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)
Member
Author
There was a problem hiding this comment.
Yeah, I think we may want to rename some of these too.
Co-authored-by: John Gallagher <john@oxidecomputer.com>
jgallagher
approved these changes
Jul 21, 2026
hawkw
enabled auto-merge (squash)
July 21, 2026 17:55
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
omicron_common::addressmodule 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 asBOOTSTRAP_SUBNET_PREFIX, which describes a /40 network, and therefore unsurprisingly is the number 40, orRACK_PREFIXandAZ_PREFIX, which are 56 and 48, respectively. These constants are typically used as const generic parameters to theIpv6Subnettype, 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 asIPV6_MULTICAST_PREFIXandIPV6_ADMIN_SCOPED_MULTICAST_PREFIX, which areff00andff04, 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
_PREFIXconstant 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 beingu8s and the ones that represent IPv6 octet pairs being, naturally, octet pairs, i.e.u16s. Luckily, one cannot accidentally insert au16into au8-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, fromsled_hardware_types::underlaytoomicron_common::address.1 I'd like to be able to reference it in Nexus in #10835. Sadly, as I mentioned above, there is already aBOOTSTRAP_SUBNET_PREFIXhere, which represents the length of a prefix, rather than the actual octets. Having bothBOOTSTRAP_PREFIXandBOOTSTRAP_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 letterf, are namedFOO_PREFIX. Doing this touches a lot of files, but reviewers should really only need to look closely atcommon/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 Symbolbutton" to assist in making this change.Footnotes
Which it's only currently referenced in
sled-hardware, which depends onomicron-commonas well as onsled-hardware-types. ↩