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

[nexus] Multiple network interfaces must be in distinct VPC Subnets #1162

Closed
bnaecker opened this issue Jun 6, 2022 · 1 comment
Closed
Assignees
Labels
database Related to database access networking Related to the networking. nexus Related to nexus

Comments

@bnaecker
Copy link
Collaborator

bnaecker commented Jun 6, 2022

Currently we constrain multiple NICs for an instance in various ways. For example, they must all be in the same VPC. An additional constraint that's not yet enforced is that an instance may not have multiple NICs in the same VPC Subnet. The main reason for this is that it will likely result in problems for the guest networking stack. Should they create multiple such NICs, the routing tables in the guest would be confusing and probably wrong, since there's no obvious way for the guest kernel to pick one NIC over another for traffic destined for an address on that IP subnetwork.

This should be pretty straightforward, adding an additional constraint-checking subquery to the query in omicron_nexus::db::queries::network_interface::InsertQuery, and an error-handling branch in the corresponding network_interface::InsertError type.

EDIT: It's also possible this would be better implemented as a unique index, e.g., on (instance_id, subnet_id). I don't know if we'd use that index for searching, however, which means the space cost may not be worth it.

@bnaecker bnaecker added networking Related to the networking. database Related to database access nexus Related to nexus labels Jun 6, 2022
@bnaecker bnaecker self-assigned this Jun 6, 2022
@bnaecker
Copy link
Collaborator Author

bnaecker commented Jun 7, 2022

Resolved by #1167

@bnaecker bnaecker closed this as completed Jun 7, 2022
leftwo pushed a commit that referenced this issue Mar 4, 2024
Crucible changes:
Per client, queue-based backpressure (#1186)
A builder for the Downstairs Downstairs struct. (#1152)
Update Rust to v1.76.0 (#1153)
Deactivate the read only parent after a scrub (#1180)
Start byte-based backpressure earlier (#1179)
Tweak CI scripts to fix warnings (#1178)
Make `gw_ds_complete` less public (#1175)
Verify extent under repair is valid after copying files (#1159)
Remove individual panic setup, use global panic settings (#1174)
[smf] Use new zone network config service (#1096)
Move a few methods into downstairs (#1160)
Remove extra clone in upstairs read (#1163)
Make `crucible-downstairs` not depend on upstairs (#1165)
Update Rust crate rusqlite to 0.31 (#1171)
Update Rust crate reedline to 0.29.0 (#1170)
Update Rust crate clap to 4.5 (#1169)
Update Rust crate indicatif to 0.17.8 (#1168)
Update progenitor to bc0bb4b (#1164)
Do not 500 on snapshot delete for deleted region (#1162)
Drop jobs from Offline downstairs. (#1157)
`Mutex<Work>` → `Work` (#1156)
Added a contributing.md (#1158)
Remove ExtentFlushClose::source_downstairs (#1154)
Remove unnecessary mutexes from Downstairs (#1132)

Propolis changes:
PHD: improve Windows reliability (#651)
Update progenitor and omicron deps
Clean up VMM resource on server shutdown
Remove Inventory mechanism
Update vergen dependency
Properly handle pre/post illumos#16183 fixups
PHD: add `pfexec` to xtask phd-runner invocation (#647)
PHD: add Windows Server 2016 adapter & improve WS2016/2019 reliability (#646)
PHD: use `clap` for more `cargo xtask phd` args (#645)
PHD: several `cargo xtask phd` CLI fixes (#642)
PHD: Use ZFS clones for file-backed disks (#640)
PHD: improve ctrl-c handling (#634)
leftwo added a commit that referenced this issue Mar 4, 2024
Crucible changes:
Per client, queue-based backpressure (#1186)
A builder for the Downstairs Downstairs struct. (#1152) Update Rust to
v1.76.0 (#1153)
Deactivate the read only parent after a scrub (#1180) Start byte-based
backpressure earlier (#1179)
Tweak CI scripts to fix warnings (#1178)
Make `gw_ds_complete` less public (#1175)
Verify extent under repair is valid after copying files (#1159) Remove
individual panic setup, use global panic settings (#1174) [smf] Use new
zone network config service (#1096)
Move a few methods into downstairs (#1160)
Remove extra clone in upstairs read (#1163)
Make `crucible-downstairs` not depend on upstairs (#1165) Update Rust
crate rusqlite to 0.31 (#1171)
Update Rust crate reedline to 0.29.0 (#1170)
Update Rust crate clap to 4.5 (#1169)
Update Rust crate indicatif to 0.17.8 (#1168)
Update progenitor to bc0bb4b (#1164)
Do not 500 on snapshot delete for deleted region (#1162) Drop jobs from
Offline downstairs. (#1157)
`Mutex<Work>` → `Work` (#1156)
Added a contributing.md (#1158)
Remove ExtentFlushClose::source_downstairs (#1154) Remove unnecessary
mutexes from Downstairs (#1132)

Propolis changes:
PHD: improve Windows reliability (#651)
Update progenitor and omicron deps
Clean up VMM resource on server shutdown
Remove Inventory mechanism
Update vergen dependency
Properly handle pre/post illumos#16183 fixups
PHD: add `pfexec` to xtask phd-runner invocation (#647) PHD: add Windows
Server 2016 adapter & improve WS2016/2019 reliability (#646) PHD: use
`clap` for more `cargo xtask phd` args (#645) PHD: several `cargo xtask
phd` CLI fixes (#642)
PHD: Use ZFS clones for file-backed disks (#640)
PHD: improve ctrl-c handling (#634)

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
database Related to database access networking Related to the networking. nexus Related to nexus
Projects
None yet
Development

No branches or pull requests

1 participant