Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allocates guest MAC addresses via a next item query #1135

Merged
merged 2 commits into from
Jun 1, 2022

Conversation

bnaecker
Copy link
Collaborator

  • Provides random base address within the guest-visible range of
    A8:40:25:F0:00:00-A8:40:25:FE:FF:FF.
  • Switch the database representation of MAC addresses in the
    network_interface table from a string to INT8. The string
    representation is human-readable, but doesn't allow any other
    operations on it (such as addition). There's also no native
    MACADDR support in CRDB, so we don't lose much by switching to a new
    repr. Any client code needs to parse/interpret the value anyway, and a
    byte-representation in some ways is more natural or obvious.
  • Adds a NextItem query for generating a random guest MAC address. This
    is the upshot of the change to an integer, that we can add to it and
    thus use the NextItem query machinery.
  • Adds a variant of the NetworkInterfaceError for detecting MAC
    address exhaustion.

@bnaecker
Copy link
Collaborator Author

bnaecker commented May 28, 2022

This PR should resolve both #959 and #963. It uses the new NextItem machinery to select an available, random MAC address for guest NICs within the reserved portion of the range. This allows us to both preclude selection of the MAC reserved for the OPTE virtual gateway (or other system services), and avoid conflict, without a retry loop.

The cost we pay for this is a change in the database representation. Previously we stored addresses as strings, e.g., a8:25:40:f4:01:02. That's nice and human-friendly, but it doesn't work in a NextItem query, which requires a type we can add integers to. This PR changes that to a INT8, using the 6 least-significant bytes to store the octets of the MAC. (Note that it's always big-endian.)

I've also added a new variant of the NetworkInterfaceError that detects MAC address exhaustion. I tested that by artificially restricting the range of possible MACs and seeing this when provisioning:

bnaecker@feldspar : ~/cli $ ./target/debug/oxide instance create -o o -p p -D "vm0" --hostname vm0 -m $((1024 * 1024 * 1024)) -c 4 vm4
✘ Invalid request: No available MAC addresses for interface

It's likely we'll hit other limits before this one, but I'd like to have an explicit check in any case.

@bnaecker bnaecker requested a review from smklein May 28, 2022 05:23
- Provides random base address within the guest-visible range of
  A8:40:25:F0:00:00-A8:40:25:FE:FF:FF.
- Switch the database representation of MAC addresses in the
  `network_interface` table from a string to INT8. The string
  representation is human-readable, but doesn't allow any other
  operations on it (such as addition). There's also no native
  MACADDR support in CRDB, so we don't lose much by switching to a new
  repr. Any client code needs to parse/interpret the value anyway, and a
  byte-representation in some ways is more natural or obvious.
- Adds a NextItem query for generating a random guest MAC address. This
  is the upshot of the change to an integer, that we can add to it and
  thus use the NextItem query machinery.
- Adds a variant of the `NetworkInterfaceError` for detecting MAC
  address exhaustion.
@bnaecker bnaecker force-pushed the split-mac-addr-space branch from 3e272db to 80092af Compare May 31, 2022 19:32
@smklein smklein self-assigned this Jun 1, 2022
nexus/src/db/model/macaddr.rs Show resolved Hide resolved
nexus/src/db/queries/network_interface.rs Outdated Show resolved Hide resolved
@smklein smklein assigned bnaecker and unassigned smklein Jun 1, 2022
@bnaecker bnaecker enabled auto-merge (squash) June 1, 2022 16:17
@bnaecker bnaecker merged commit 505d3df into main Jun 1, 2022
@bnaecker bnaecker deleted the split-mac-addr-space branch June 1, 2022 17:18
leftwo pushed a commit that referenced this pull request Feb 9, 2024
Crucible changes:
Remove unused fields in IOop (#1149)
New downstairs clone subcommand. (#1129)
Simplify the do_work_task loop (#1150)
Move `Guest` stuff into a module (#1125)
Bump nix to 0.27.1 and use new safer Fd APIs (#1110)
Move `FramedWrite` work to a separate task (#1145)
Use fewer borrows in ExtentInner API (#1147)
Update Rust crate reedline to 0.28.0 (#1141)
Update Rust crate tokio to 1.36 (#1143)
Update Rust crate slog-bunyan to 2.5.0 (#1139)
Update Rust crate rayon to 1.8.1 (#1138)
Update Rust crate itertools to 0.12.1 (#1137)
Update Rust crate byte-unit to 5.1.4 (#1136)
Update Rust crate base64 to 0.21.7 (#1135)
Update Rust crate async-trait to 0.1.77 (#1134)
Discard deferred msgs (#1131)
Minor Downstairs cleanup (#1127)
Update test_fail_live_repair to support pstop (#1128)
Ignore client messages after stopping the IO task (#1126)
Move client IO task into a struct (#1124)
Bump Rust to 1.75 and fix new Clippy lints (#1123)

Propolis changes:
PHD: convert to async (#633)
PHD: assume specialized Windows images (#636)
propolis-standalone-config needn't be a crate
standalone: Use tar for snapshot/restore
phd: use latest "lab-2.0-opte" target, not a specific version (#637)
PHD: add tests for migration of running processes (#623)
PHD: fix `cargo xtask phd` tidy not doing anything (#630)
PHD: add documentation for `cargo xtask phd` (#629)
standalone: improve virtual device creation errors (#632)
phd: add Windows Server 2019 guest adapter (#627)
PHD: add `cargo xtask phd` to make using PHD nicer (#619)
leftwo added a commit that referenced this pull request Feb 9, 2024
Crucible changes:
Remove unused fields in IOop (#1149)
New downstairs clone subcommand. (#1129)
Simplify the do_work_task loop (#1150)
Move `Guest` stuff into a module (#1125)
Bump nix to 0.27.1 and use new safer Fd APIs (#1110) Move `FramedWrite`
work to a separate task (#1145) Use fewer borrows in ExtentInner API
(#1147)
Update Rust crate reedline to 0.28.0 (#1141)
Update Rust crate tokio to 1.36 (#1143)
Update Rust crate slog-bunyan to 2.5.0 (#1139)
Update Rust crate rayon to 1.8.1 (#1138)
Update Rust crate itertools to 0.12.1 (#1137)
Update Rust crate byte-unit to 5.1.4 (#1136)
Update Rust crate base64 to 0.21.7 (#1135)
Update Rust crate async-trait to 0.1.77 (#1134)
Discard deferred msgs (#1131)
Minor Downstairs cleanup (#1127)
Update test_fail_live_repair to support pstop (#1128) Ignore client
messages after stopping the IO task (#1126) Move client IO task into a
struct (#1124)
Bump Rust to 1.75 and fix new Clippy lints (#1123)

Propolis changes:
PHD: convert to async (#633)
PHD: assume specialized Windows images (#636)
propolis-standalone-config needn't be a crate
standalone: Use tar for snapshot/restore
phd: use latest "lab-2.0-opte" target, not a specific version (#637)
PHD: add tests for migration of running processes (#623) PHD: fix `cargo
xtask phd` tidy not doing anything (#630) PHD: add documentation for
`cargo xtask phd` (#629) standalone: improve virtual device creation
errors (#632) phd: add Windows Server 2019 guest adapter (#627)
PHD: add `cargo xtask phd` to make using PHD nicer (#619)

Co-authored-by: Alan Hanson <[email protected]>
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