Be more specific about version errors, raise min required viona API

The recent multiqueue work includes an unconditional ioctl that was
introduced in V6 of the viona API. Propolis' expectation should have
been raised then, but it was missed in review; instead, trying to create
a vNIC on a too-old OS results in propolis panicking about
`Inappropriate ioctl for device`.

So, raise the minimum viona API as required. propolis-server will exit
at start if the OS is too old.

The message wasn't super clear though, complaining that:

> viona API version mismatch 4 != 6

so this comes with some adjustments to the version error reporting that
now produce this error on a too-old host:

>    0: checking version of viona
>    1: API version 4 is not at or above 6. OS is too old?

propolis-standalone allows you to try running something on a too-old
host on the expectation you're actively hacking on either the host or
Propolis, so it just offers

> ERRO viona: API version 4 is not at or above 6. OS is too old?

before continuing on (and in this case, panicking about the an
inappropriate ioctl).

Author: iximeow

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,

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(()),

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)]

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(())
     }

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(())