Skip to content

Self assembling zones should set their own MTU #3520

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.

Sign up for GitHub

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

Merged
jmpesp merged 1 commit into from
Jul 7, 2023
Merged

Self assembling zones should set their own MTU #3520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ cookie = "0.16"
criterion = { version = "0.5.1", features = [ "async_tokio" ] }
crossbeam = "0.8"
crossterm = { version = "0.26.1", features = ["event-stream"] }
crucible-agent-client = { git = "https://github.com/oxidecomputer/crucible", rev = "df7e274edf0a1df287770e9567b27676f3ae4cc5" }
crucible-client-types = { git = "https://github.com/oxidecomputer/crucible", rev = "df7e274edf0a1df287770e9567b27676f3ae4cc5" }
crucible-pantry-client = { git = "https://github.com/oxidecomputer/crucible", rev = "df7e274edf0a1df287770e9567b27676f3ae4cc5" }
crucible-smf = { git = "https://github.com/oxidecomputer/crucible", rev = "df7e274edf0a1df287770e9567b27676f3ae4cc5" }
crucible-agent-client = { git = "https://github.com/oxidecomputer/crucible", rev = "c574ff1232aa66eb60069795fe45f0c47f5da51d" }
crucible-client-types = { git = "https://github.com/oxidecomputer/crucible", rev = "c574ff1232aa66eb60069795fe45f0c47f5da51d" }
crucible-pantry-client = { git = "https://github.com/oxidecomputer/crucible", rev = "c574ff1232aa66eb60069795fe45f0c47f5da51d" }
crucible-smf = { git = "https://github.com/oxidecomputer/crucible", rev = "c574ff1232aa66eb60069795fe45f0c47f5da51d" }
curve25519-dalek = "3"
datatest-stable = "0.1.3"
display-error-chain = "0.1.1"
Expand Down Expand Up @@ -264,9 +264,9 @@ pretty-hex = "0.3.0"
proc-macro2 = "1.0"
progenitor = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
progenitor-client = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "04a275736e9f3316de6bf2a4077d03acfa4e2cdf" }
propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "04a275736e9f3316de6bf2a4077d03acfa4e2cdf", features = [ "generated-migration" ] }
propolis-server = { git = "https://github.com/oxidecomputer/propolis", rev = "04a275736e9f3316de6bf2a4077d03acfa4e2cdf", default-features = false, features = ["mock-only"] }
bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "9e12522341048d7b8c38394f0ddeb18083685c2f" }
propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "9e12522341048d7b8c38394f0ddeb18083685c2f", features = [ "generated-migration" ] }
propolis-server = { git = "https://github.com/oxidecomputer/propolis", rev = "9e12522341048d7b8c38394f0ddeb18083685c2f", default-features = false, features = ["mock-only"] }
#propolis-server = { path = "../propolis/bin/propolis-server" }
proptest = "1.2.0"
quote = "1.0"
Expand Down
106 changes: 44 additions & 62 deletions illumos-utils/src/running_zone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,78 +372,60 @@ impl RunningZone {
}
})?;

// Pull the zone ID.
// 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.
Comment on lines +375 to +382

@smklein smklein Jul 7, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this explicit!

let id = Zones::id(&zone.name)
.await?
.ok_or_else(|| BootError::NoZoneId { zone: zone.name.clone() })?;
let site_profile_xml_exists =
std::path::Path::new(&zone.site_profile_xml_path()).exists();
let running_zone = RunningZone { id: Some(id), inner: zone };

// Make sure the control vnic has an IP MTU of 9000 inside the zone
const CONTROL_VNIC_MTU: usize = 9000;
let vnic = running_zone.inner.control_vnic.name().to_string();

// 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). This
// will create IP interfaces, and means that `create-if` here will fail
// due to the interface already existing. Checking the output of
// `show-if` is also problematic due to TOCTOU. Use the check for the
// existence of site.xml, which means the zone is performing this
// self-assembly, and skip create-if if so.
let running_zone = RunningZone { id: Some(id), inner: zone };

if !site_profile_xml_exists {
let args = vec![
IPADM.to_string(),
"create-if".to_string(),
"-t".to_string(),
vnic.clone(),
// If the zone is not self-assembling, make sure the control vnic
// has an IP MTU of 9000 inside the zone.
const CONTROL_VNIC_MTU: usize = 9000;
let vnic = running_zone.inner.control_vnic.name().to_string();

let commands = vec![
vec![
IPADM.to_string(),
"create-if".to_string(),
"-t".to_string(),
vnic.clone(),
],
vec![
IPADM.to_string(),
"set-ifprop".to_string(),
"-t".to_string(),
"-p".to_string(),
format!("mtu={}", CONTROL_VNIC_MTU),
"-m".to_string(),
"ipv4".to_string(),
vnic.clone(),
],
vec![
IPADM.to_string(),
"set-ifprop".to_string(),
"-t".to_string(),
"-p".to_string(),
format!("mtu={}", CONTROL_VNIC_MTU),
"-m".to_string(),
"ipv6".to_string(),
vnic,
],
];

running_zone.run_cmd(args)?;
} else {
// If the zone is self-assembling, then it's possible that the IP
// interface does not exist yet because it has not been brought up
// by the software in the zone. Run `create-if` here, but eat the
// error if there is one: this is safe unless the software that's
// part of self-assembly inside the zone is also trying to run
// `create-if` (instead of `create-addr`), and required for the
// `set-ifprop` commands below to pass.
let args = vec![
IPADM.to_string(),
"create-if".to_string(),
"-t".to_string(),
vnic.clone(),
];

let _result = running_zone.run_cmd(args);
}

let commands = vec![
vec![
IPADM.to_string(),
"set-ifprop".to_string(),
"-t".to_string(),
"-p".to_string(),
format!("mtu={}", CONTROL_VNIC_MTU),
"-m".to_string(),
"ipv4".to_string(),
vnic.clone(),
],
vec![
IPADM.to_string(),
"set-ifprop".to_string(),
"-t".to_string(),
"-p".to_string(),
format!("mtu={}", CONTROL_VNIC_MTU),
"-m".to_string(),
"ipv6".to_string(),
vnic,
],
];

for args in &commands {
running_zone.run_cmd(args)?;
for args in &commands {
running_zone.run_cmd(args)?;
}
}

Ok(running_zone)
Expand Down
12 changes: 6 additions & 6 deletions package-manifest.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,21 +241,21 @@ only_for_targets.image = "standard"
# 3. Use source.type = "manual" instead of "prebuilt"
source.type = "prebuilt"
source.repo = "crucible"
source.commit = "df7e274edf0a1df287770e9567b27676f3ae4cc5"
source.commit = "c574ff1232aa66eb60069795fe45f0c47f5da51d"
# The SHA256 digest is automatically posted to:
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/image/<commit>/crucible.sha256.txt
source.sha256 = "c1a7996dfdded9476d5a595f16fcd07e902bb02f2ec0858bb3625f9b2195a919"
source.sha256 = "72363fbbfde15689bdbafee53e415ca994903644a9fdbb33ca0bc72d65927f0f"
output.type = "zone"

[package.crucible-pantry]
service_name = "crucible_pantry"
only_for_targets.image = "standard"
source.type = "prebuilt"
source.repo = "crucible"
source.commit = "df7e274edf0a1df287770e9567b27676f3ae4cc5"
source.commit = "c574ff1232aa66eb60069795fe45f0c47f5da51d"
# The SHA256 digest is automatically posted to:
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/image/<commit>/crucible-pantry.sha256.txt
source.sha256 = "23611f3770a797b829c340e6dba25a4898678ece79db8534e06f76e438214e4c"
source.sha256 = "4645454e3af72ee68bc3916042f78deaac44a8c07bb6c1a1e5766b83f5326e07"
output.type = "zone"

# Refer to
Expand All @@ -266,10 +266,10 @@ service_name = "propolis-server"
only_for_targets.image = "standard"
source.type = "prebuilt"
source.repo = "propolis"
source.commit = "21ac8a9f5005f96caa384e3de0bd38283fc0188f"
source.commit = "9e12522341048d7b8c38394f0ddeb18083685c2f"
# The SHA256 digest is automatically posted to:
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/propolis/image/<commit>/propolis-server.sha256.txt
source.sha256 = "2a7b4bfa6d2b13714f68ec0095419218257ab584e763faac1d34baf38c8b09de"
source.sha256 = "084df2cf5866a17b5467dccdaddef5088cc021a1ee0eb3bea99472bcf67ea7fa"
output.type = "zone"

[package.maghemite]
Expand Down
7 changes: 7 additions & 0 deletions smf/clickhouse/method_script.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ if [[ $DATALINK == unknown ]] || [[ $GATEWAY == unknown ]]; then
exit "$SMF_EXIT_ERR_CONFIG"
fi

# TODO remove when https://github.com/oxidecomputer/stlouis/issues/435 is addressed
ipadm delete-if "$DATALINK" || true
ipadm create-if -t "$DATALINK"

ipadm set-ifprop -t -p mtu=9000 -m ipv4 "$DATALINK"
ipadm set-ifprop -t -p mtu=9000 -m ipv6 "$DATALINK"

ipadm show-addr "$DATALINK/ll" || ipadm create-addr -t -T addrconf "$DATALINK/ll"
ipadm show-addr "$DATALINK/omicron6" || ipadm create-addr -t -T static -a "$LISTEN_ADDR" "$DATALINK/omicron6"
route get -inet6 default -inet6 "$GATEWAY" || route add -inet6 default -inet6 "$GATEWAY"
Expand Down
7 changes: 7 additions & 0 deletions smf/cockroachdb/method_script.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ if [[ $DATALINK == unknown ]] || [[ $GATEWAY == unknown ]]; then
exit "$SMF_EXIT_ERR_CONFIG"
fi

# TODO remove when https://github.com/oxidecomputer/stlouis/issues/435 is addressed
ipadm delete-if "$DATALINK" || true
ipadm create-if -t "$DATALINK"

ipadm set-ifprop -t -p mtu=9000 -m ipv4 "$DATALINK"
ipadm set-ifprop -t -p mtu=9000 -m ipv6 "$DATALINK"
Comment on lines +24 to +25

@luqmana luqmana Jul 7, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@jmpesp jmpesp Jul 7, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was more an assurance thing


ipadm show-addr "$DATALINK/ll" || ipadm create-addr -t -T addrconf "$DATALINK/ll"
ipadm show-addr "$DATALINK/omicron6" || ipadm create-addr -t -T static -a "$LISTEN_ADDR" "$DATALINK/omicron6"
route get -inet6 default -inet6 "$GATEWAY" || route add -inet6 default -inet6 "$GATEWAY"
Expand Down