diff --git a/bin/propolis-server/src/lib/initializer.rs b/bin/propolis-server/src/lib/initializer.rs index 8e5d4961b..c53ddb34d 100644 --- a/bin/propolis-server/src/lib/initializer.rs +++ b/bin/propolis-server/src/lib/initializer.rs @@ -771,32 +771,27 @@ impl MachineInitializer<'_> { info!(self.log, "Creating vNIC {}", device_name); let bdf: pci::Bdf = nic.device_spec.pci_path.into(); - // Set viona device parameters if possible. + // Set viona device parameters. The parameters here (copy_data and + // header_pad) require `viona::ApiVersion::V3`, below Propolis' + // minimum of V6, so we can always set them. // // The values chosen here are tuned to maximize performance when // Propolis is used with OPTE in a full Oxide rack deployment, // although they should not negatively impact use outside those // conditions. These parameters and their effects (save for // performance delta) are not guest-visible. - let params = if virtio::viona::api_version() - .expect("can query viona version") - >= virtio::viona::ApiVersion::V3 - { - Some(virtio::viona::DeviceParams { - // Loan guest packet data, rather than allocating fresh - // buffers and copying it. - copy_data: false, - // Leave room for underlay encapsulation: - // - ethernet: 14 - // - IPv6: 40 - // - UDP: 8 - // - Geneve: 8–16 (due to options) - // - (and then round up to nearest 8) - header_pad: 80, - }) - } else { - None - }; + let params = Some(virtio::viona::DeviceParams { + // Loan guest packet data, rather than allocating fresh + // buffers and copying it. + copy_data: false, + // Leave room for underlay encapsulation: + // - ethernet: 14 + // - IPv6: 40 + // - UDP: 8 + // - Geneve: 8–16 (due to options) + // - (and then round up to nearest 8) + header_pad: 80, + }); let viona = virtio::PciVirtioViona::new( &nic.backend_spec.vnic_name, diff --git a/bin/propolis-standalone/src/main.rs b/bin/propolis-standalone/src/main.rs index cc5a1d31c..42a97319a 100644 --- a/bin/propolis-standalone/src/main.rs +++ b/bin/propolis-standalone/src/main.rs @@ -1251,15 +1251,9 @@ fn setup_instance( config::VionaDeviceParams::from_opts(&dev.options) .expect("viona params are valid"); - if viona_params.is_some() - && hw::virtio::viona::api_version() - .expect("can query viona version") - < hw::virtio::viona::ApiVersion::V3 - { - // lazy cop-out for now - panic!("can't set viona params on too-old version"); - } - + // The viona_params here (currently just copy_data and + // header_pad) require `viona::ApiVersion::V3`, below + // Propolis' minimum of V6, so we can always set them. let viona = hw::virtio::PciVirtioViona::new( vnic_name, &hdl, @@ -1452,18 +1446,12 @@ fn api_version_checks(log: &slog::Logger) -> std::io::Result<()> { } Err(VersionCheckError { component, + err: source @ Error::TooLow { .. }, path: _, - err: Error::Mismatch(act, exp), }) => { // Make noise about version mismatch, but soldier on and let the // user decide if they want to quit - slog::error!( - log, - "{} API version mismatch {} != {}", - component, - act, - exp - ); + slog::error!(log, "{component}: {source}"); Ok(()) } Ok(_) => Ok(()), diff --git a/lib/propolis/src/api_version.rs b/lib/propolis/src/api_version.rs index a2370fe98..00ba0ecba 100644 --- a/lib/propolis/src/api_version.rs +++ b/lib/propolis/src/api_version.rs @@ -7,8 +7,14 @@ pub enum Error { #[error("IO Error")] Io(#[from] std::io::Error), - #[error("API version {0} did not match expectation {1}")] - Mismatch(u32, u32), + // Newer APIs are backwards compatible with older (for now?), so don't + // bother trying to express "we need a version before .." or any of that. + // + // Also assume that the component being versioned is either part of the OS + // or that the OS version is a decent proxy for it. Not necessarily true in + // general, but true for viona and bhyve. + #[error("API version {have} is not at or above {want}. OS is too old?")] + TooLow { have: u32, want: u32 }, } #[derive(Debug, thiserror::Error)] diff --git a/lib/propolis/src/hw/virtio/viona.rs b/lib/propolis/src/hw/virtio/viona.rs index 1ae5d8000..c9ec21347 100644 --- a/lib/propolis/src/hw/virtio/viona.rs +++ b/lib/propolis/src/hw/virtio/viona.rs @@ -226,8 +226,6 @@ impl Inner { } /// Configuration parmaeters for the underlying viona device -/// -/// These parameters assume an [viona_api::ApiVersion::V3] device #[derive(Copy, Clone)] pub struct DeviceParams { /// When transmitting packets, should viona (allocate and) copy the entire @@ -237,12 +235,18 @@ pub struct DeviceParams { /// There is a performance cost to copying the full packet, but it avoids /// certain issues pertaining to looped-back viona packets being delivered /// to native zones on the machine. + /// + /// This parameter requires [viona_api::ApiVersion::V3] or greater. This is + /// before Propolis' minimum viona API version and can always be set. pub copy_data: bool, /// Byte count for padding added to the head of transmitted packets. This /// padding can be used by subsequent operations in the transmission chain, /// such as encapsulation, which would otherwise need to re-allocate for the /// larger header. + /// + /// This parameter requires [viona_api::ApiVersion::V3] or greater. This is + /// before Propolis' minimum viona API version and can always be set. pub header_pad: u16, } impl DeviceParams { @@ -1393,11 +1397,12 @@ use bits::*; pub(crate) fn check_api_version() -> Result<(), crate::api_version::Error> { let vers = viona_api::api_version()?; - // viona only requires the V2 bits for now - let compare = viona_api::ApiVersion::V2; + // when setting up a vNIC, Propolis will unconditionally do the SET_PAIRS + // ioctl, which requires V6. + let want = viona_api::ApiVersion::V6 as u32; - if vers < compare { - Err(crate::api_version::Error::Mismatch(vers, compare as u32)) + if vers < want { + Err(crate::api_version::Error::TooLow { have: vers, want }) } else { Ok(()) } diff --git a/lib/propolis/src/vmm/mod.rs b/lib/propolis/src/vmm/mod.rs index 033bc8991..80a25cfcb 100644 --- a/lib/propolis/src/vmm/mod.rs +++ b/lib/propolis/src/vmm/mod.rs @@ -19,10 +19,10 @@ pub(crate) fn check_api_version() -> Result<(), crate::api_version::Error> { let vers = ctl.api_version()?; // propolis only requires the bits provided by V8, currently - let compare = bhyve_api::ApiVersion::V8; + let want = bhyve_api::ApiVersion::V8 as u32; - if vers < compare { - return Err(crate::api_version::Error::Mismatch(vers, compare as u32)); + if vers < want { + return Err(crate::api_version::Error::TooLow { have: vers, want }); } Ok(())