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] Need API to update network interfaces #1153

Closed
bnaecker opened this issue Jun 3, 2022 · 7 comments
Closed

[nexus] Need API to update network interfaces #1153

bnaecker opened this issue Jun 3, 2022 · 7 comments
Assignees

Comments

@bnaecker
Copy link
Collaborator

bnaecker commented Jun 3, 2022

Add an HTTP entrypoint and attendant queries for updating a network interface. Parameters that can be changed are:

  • name
  • description
  • if NIC is primary

The last one is a bit funny. I think we only want to support designating a new primary NIC, not designating a NIC as a secondary. Instances always have a primary, so we can't support the latter.

@rmustacc
Copy link

rmustacc commented Jun 3, 2022

Note as we had additional support for multiple IPs or controlling v4/v6 I'd expect this to also go here.

@bnaecker
Copy link
Collaborator Author

bnaecker commented Jun 3, 2022

Thanks Robert -- I had assumed, incorrectly it seems, that modifying IPs would mean adding a new NIC, not modifying an existing one. I'll keep that in mind, thanks!

@bnaecker
Copy link
Collaborator Author

bnaecker commented Jun 6, 2022

@rmustacc I believe I misread this. You're specifically saying that adding an IP to an interface should be supported, right? For example, we could support adding a V4 address to an existing NIC with only a V6 address. Or does this also include changing a NIC's address from V6 to V4?

@rmustacc
Copy link

rmustacc commented Jun 6, 2022

I'm saying in the future (not now per se) there are basically two different axis:

  • Whether the NIC uses one or both of the v4 and v6 part of its VPC Subnet
  • The number and specific IP addresses it has from the corresponding v4/v6 side of the VPC Subnet

@bnaecker
Copy link
Collaborator Author

bnaecker commented Jun 6, 2022

I must be missing something, since I see those as redundant, not independent axes. If the NIC has zero addresses from the V6 subnet of its VPC Subnet, then it seems the answer to the first question is, "no, it doesn't use the V6 part." If it has one or more, then the answer is "yes".

Is there a practical difference between having a version of the protocol "available" to a NIC and it actually having an address from that version?

@bnaecker
Copy link
Collaborator Author

bnaecker commented Jun 6, 2022

Robert and I spoke offline about this. Here's the summary.

The high-level point is that we don't want to preclude future work, by over-constraining the API now. It's true that in some sense the two "axes" we're talking about here are related, not totally independent. But we'd like to allow an API in the future that lets operators control the NICs in two ways: whether they're constrained to IPv4 or IPv6, and how many addresses they have of each of those. Those can be separate knobs in the API, even if they're related concepts. We just want to make sure that what we build now doesn't make that impossible in the future.

An additional point that came up was that there are likely to be additional parameters or flags that we'd like to support on the level of the entire NIC. One example is IP spoofing, which could be a boolean flag indicating if that's supported / allowed.

@bnaecker
Copy link
Collaborator Author

Completed in #1186

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
None yet
Projects
None yet
Development

No branches or pull requests

2 participants