Self assembling zones should set their own MTU#3520
Merged
Conversation
Bump crucible rev to pick up: - Self assembling zones must set their own MTU - fix build break caused by merging oxidecomputer/crucible#801 - Issue extent flushes in parallel - Add Logger Option to Volume construct method - Update Rust crate libc to 0.2.147 - Update Rust crate percent-encoding to 2.3 - Retry jobs until they succeed - Reorder select arms so pings can't be starved out - Treat a skipped IO like an error IO for ACK results. - Retry pantry requests - Remove panics and asserts in dummy tests - Update Rust crate csv to 1.2.2 - Update Rust crate reedline to 0.21.0 - Set open file resource limit to the max - Update Rust crate ringbuffer to 0.14 - DTrace meet cmon - Widen assert values to u128 to deal with u64::MAX - Change size_to_validate from usize to u64 - Turn on live-repair test in CI - Increase flush_timeout for some tests, collect cores - Update to latest dropshot Bump propolis rev to pick up: - Self assembling zones must set their own MTU - Bump crucible rev to latest - Make the propolis zone self-assembling - Flesh out more PIIX3-PM to suppress log gripes - Bump crucible rev to latest - Restructure PM-timer under PIIX3 device - Fix inventory handling for nested child entities - Centralize vmm-data interface into bhyve_api - Clean up PCI device classes - Update openssl dep to 0.10.55 - Allow propolis-standalone to use VMM reservoir - only allow one request to reboot to be enqueued at a time Nexus is currently setting the MTU inside self-assembling zones, but this goes against the idea that self-assembling zones perform their own configuration. Remove the code in `RunningZone::boot` that performs commands on self-assembling zones, and set the MTU of $DATALINK in the Clickhouse and CockroachDB method scripts. Fixes oxidecomputer#3512
smklein
approved these changes
Jul 7, 2023
smklein
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for doing this. I know it's a pain to update all these repos, but I really appreciate this, and I think we'll appreciate going in this direction more-and-more when we start implementing reconciliation after sled agent reboot.
Comment on lines
+375
to
+382
| // If the zone is self-assembling, then SMF service(s) inside the zone | ||
| // will be creating the listen address for the zone's service(s), | ||
| // setting the appropriate ifprop MTU, and so on. The idea behind | ||
| // self-assembling zones is that once they boot there should be *no* | ||
| // zlogin required. | ||
|
|
||
| // Use the zone ID in order to check if /var/svc/profile/site.xml | ||
| // exists. |
Collaborator
There was a problem hiding this comment.
Thanks for making this explicit!
luqmana
reviewed
Jul 7, 2023
Comment on lines
+24
to
+25
| ipadm set-ifprop -t -p mtu=9000 -m ipv4 "$DATALINK" | ||
| ipadm set-ifprop -t -p mtu=9000 -m ipv6 "$DATALINK" |
Contributor
There was a problem hiding this comment.
Was there a bug or something for why we needed to explicitly set the ip-level MTU instead of just relying on the link's MTU?
Contributor
Author
There was a problem hiding this comment.
It was more an assurance thing
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
3 participants
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.
Bump crucible rev to pick up:
Bump propolis rev to pick up:
Nexus is currently setting the MTU inside self-assembling zones, but this goes against the idea that self-assembling zones perform their own configuration. Remove the code in
RunningZone::bootthat performs commands on self-assembling zones, and set the MTU of $DATALINK in the Clickhouse and CockroachDB method scripts.Fixes #3512