[DNM] attempt to use a semaphore to work around opte 758

oximeter/instruments/src/kstat/mod.rs

@@ -94,6 +94,7 @@ mod sampler;
 pub use sampler::CollectionDetails;
 pub use sampler::ExpirationBehavior;
 pub use sampler::KstatSampler;
+pub use sampler::KstatSemaphore;
 pub use sampler::TargetId;
 pub use sampler::TargetStatus;
 

oximeter/instruments/src/kstat/sampler.rs

@@ -313,6 +313,37 @@ impl<'a> From<Kstat<'a>> for KstatPath {
 pub(crate) const CREATION_TIME_PRUNE_INTERVAL: Duration =
     Duration::from_secs(60);
 
+/// A semaphore to prevent XDE creations/deletions and kstat reads from
+/// happening in parallel.
+///
+/// This is a semaphore with one permit, and is cheaply cloneable.
+///
+/// This is a temporary workaround for
+/// https://github.com/oxidecomputer/opte/issues/758. See
+/// https://github.com/oxidecomputer/omicron/issues/9211 for more details.
+#[derive(Clone, Debug)]
+pub struct KstatSemaphore {
+    semaphore: Arc<Mutex<()>>,
+}
+
+impl KstatSemaphore {
+    /// Create a new semaphore with one permit.
+    pub fn new() -> Self {
+        Self { semaphore: Arc::new(Mutex::new(())) }
+    }
+
+    /// Run some code guarded by the semaphore.
+    pub fn run<F, T>(&self, f: F) -> T
+    where
+        F: FnOnce() -> T,
+    {
+        let permit = self.semaphore.lock().unwrap();
+        let ret = f();
+        drop(permit);
+        ret
+    }
+}
+
 /// Type which owns the `kstat` chain and samples each target on an interval.
 ///
 /// This type runs in a separate tokio task. As targets are added, it schedules
@@ -324,6 +355,9 @@ pub(crate) const CREATION_TIME_PRUNE_INTERVAL: Duration =
 struct KstatSamplerWorker {
     log: Logger,
 
+    /// The semaphore used to prevent a deadlock within the illumos kernel.
+    semaphore: KstatSemaphore,
+
     /// The kstat chain.
     ctl: Option<Ctl>,
 
@@ -394,6 +428,7 @@ impl KstatSamplerWorker {
     /// Create a new sampler worker.
     fn new(
         log: Logger,
+        semaphore: KstatSemaphore,
         inbox: mpsc::Receiver<Request>,
         self_stat_queue: broadcast::Sender<Sample>,
         samples: Arc<Mutex<BTreeMap<TargetId, Vec<Sample>>>>,
@@ -402,6 +437,7 @@ impl KstatSamplerWorker {
         let ctl = Some(Ctl::new().map_err(Error::Kstat)?);
         let self_stats = hostname().map(self_stats::SelfStats::new);
         Ok(Self {
+            semaphore,
             ctl,
             log,
             targets: BTreeMap::new(),
@@ -840,19 +876,20 @@ impl KstatSamplerWorker {
 
         // Fetch each interested kstat, and include the data and creation times
         // for each of them.
-        let kstats = ctl
-            .iter()
-            .filter(|kstat| sampled_kstat.target.interested(kstat))
-            .map(|mut kstat| {
-                let data = ctl.read(&mut kstat).map_err(Error::Kstat)?;
-                let creation_time = Self::ensure_kstat_creation_time(
-                    &self.log,
-                    kstat,
-                    &mut self.creation_times,
-                )?;
-                Ok((creation_time, kstat, data))
-            })
-            .collect::<Result<Vec<_>, _>>();
+        let kstats = self.semaphore.run(|| {
+            ctl.iter()
+                .filter(|kstat| sampled_kstat.target.interested(kstat))
+                .map(|mut kstat| {
+                    let data = ctl.read(&mut kstat).map_err(Error::Kstat)?;
+                    let creation_time = Self::ensure_kstat_creation_time(
+                        &self.log,
+                        kstat,
+                        &mut self.creation_times,
+                    )?;
+                    Ok((creation_time, kstat, data))
+                })
+                .collect::<Result<Vec<_>, _>>()
+        });
         match kstats {
             Ok(k) if !k.is_empty() => {
                 sampled_kstat.time_of_last_collection = Some(now());
@@ -1230,13 +1267,14 @@ impl KstatSampler {
     pub const DEFAULT_SAMPLE_LIMIT: usize = 500;
 
     /// Create a new sampler.
-    pub fn new(log: &Logger) -> Result<Self, Error> {
-        Self::with_sample_limit(log, Self::DEFAULT_SAMPLE_LIMIT)
+    pub fn new(log: &Logger, semaphore: KstatSemaphore) -> Result<Self, Error> {
+        Self::with_sample_limit(log, semaphore, Self::DEFAULT_SAMPLE_LIMIT)
     }
 
     /// Create a new sampler with a sample limit.
     pub fn with_sample_limit(
         log: &Logger,
+        semaphore: KstatSemaphore,
         limit: usize,
     ) -> Result<Self, Error> {
         let samples = Arc::new(Mutex::new(BTreeMap::new()));
@@ -1245,6 +1283,7 @@ impl KstatSampler {
         let (outbox, inbox) = mpsc::channel(1);
         let worker = KstatSamplerWorker::new(
             log.new(o!("component" => "kstat-sampler-worker")),
+            semaphore,
             inbox,
             self_stat_tx,
             samples.clone(),

sled-agent/src/bootstrap/pre_server.rs

@@ -35,6 +35,7 @@ use illumos_utils::zone::Api;
 use illumos_utils::zone::Zones;
 use omicron_common::FileKv;
 use omicron_common::address::Ipv6Subnet;
+use oximeter_instruments::kstat::KstatSemaphore;
 use sled_agent_config_reconciler::ConfigReconcilerSpawnToken;
 use sled_hardware::DendriteAsic;
 use sled_hardware::SledMode;
@@ -48,6 +49,7 @@ use tokio::sync::oneshot;
 
 pub(super) struct BootstrapAgentStartup {
     pub(super) config: Config,
+    pub(super) semaphore: KstatSemaphore,
     pub(super) global_zone_bootstrap_ip: Ipv6Addr,
     pub(super) base_log: Logger,
     pub(super) startup_log: Logger,
@@ -136,6 +138,8 @@ impl BootstrapAgentStartup {
         let global_zone_bootstrap_ip =
             startup_networking.global_zone_bootstrap_ip;
 
+        let semaphore = KstatSemaphore::new();
+
         let service_manager = ServiceManager::new(
             &base_log,
             ddm_reconciler,
@@ -155,6 +159,7 @@ impl BootstrapAgentStartup {
 
         Ok(Self {
             config,
+            semaphore,
             global_zone_bootstrap_ip,
             base_log,
             startup_log: log,

sled-agent/src/bootstrap/server.rs

@@ -39,6 +39,7 @@ use omicron_ddm_admin_client::DdmError;
 use omicron_ddm_admin_client::types::EnableStatsRequest;
 use omicron_uuid_kinds::GenericUuid;
 use omicron_uuid_kinds::RackInitUuid;
+use oximeter_instruments::kstat::KstatSemaphore;
 use sled_agent_config_reconciler::ConfigReconcilerSpawnToken;
 use sled_agent_config_reconciler::InternalDisksReceiver;
 use sled_agent_types::rack_init::RackInitializeRequestParams;
@@ -175,6 +176,7 @@ impl Server {
         // fail to start.
         let BootstrapAgentStartup {
             config,
+            semaphore,
             global_zone_bootstrap_ip,
             base_log,
             startup_log,
@@ -246,6 +248,7 @@ impl Server {
             let start_sled_agent_request = ledger.into_inner();
             let sled_agent_server = start_sled_agent(
                 &config,
+                semaphore.clone(),
                 start_sled_agent_request,
                 long_running_task_handles.clone(),
                 config_reconciler_spawn_token,
@@ -275,6 +278,7 @@ impl Server {
         // agent state.
         let inner = Inner {
             config,
+            semaphore,
             state,
             sled_init_rx,
             sled_reset_rx,
@@ -356,6 +360,7 @@ impl From<SledAgentServerStartError> for StartError {
 #[allow(clippy::too_many_arguments)]
 async fn start_sled_agent(
     config: &SledConfig,
+    semaphore: KstatSemaphore,
     request: StartSledAgentRequest,
     long_running_task_handles: LongRunningTaskHandles,
     config_reconciler_spawn_token: ConfigReconcilerSpawnToken,
@@ -407,6 +412,7 @@ async fn start_sled_agent(
     // Server does not exist, initialize it.
     let server = SledAgentServer::start(
         config,
+        semaphore,
         base_log.clone(),
         request.clone(),
         long_running_task_handles.clone(),
@@ -493,6 +499,7 @@ async fn sled_config_paths(
 
 struct Inner {
     config: SledConfig,
+    semaphore: KstatSemaphore,
     state: SledAgentState,
     sled_init_rx: mpsc::Receiver<(
         StartSledAgentRequest,
@@ -560,6 +567,7 @@ impl Inner {
 
                 let response = match start_sled_agent(
                     &self.config,
+                    self.semaphore.clone(),
                     request,
                     self.long_running_task_handles.clone(),
                     config_reconciler_spawn_token,
@@ -686,7 +694,9 @@ impl Inner {
         sled_hardware::cleanup::delete_omicron_vnics(&log)
             .await
             .map_err(BootstrapError::Cleanup)?;
-        illumos_utils::opte::delete_all_xde_devices(&log)?;
+        // XDE deletions need to be protected by the semaphore.
+        self.semaphore
+            .run(|| illumos_utils::opte::delete_all_xde_devices(&log))?;
         Ok(())
     }
 

sled-agent/src/instance.rs

@@ -12,12 +12,13 @@ use crate::instance_manager::{
 };
 use crate::metrics::MetricsRequestQueue;
 use crate::nexus::NexusClient;
+use crate::port_manager::SledAgentPortManager;
 use crate::profile::*;
 use crate::zone_bundle::ZoneBundler;
 use chrono::Utc;
 use illumos_utils::dladm::Etherstub;
 use illumos_utils::link::VnicAllocator;
-use illumos_utils::opte::{DhcpCfg, PortCreateParams, PortManager};
+use illumos_utils::opte::{DhcpCfg, PortCreateParams};
 use illumos_utils::running_zone::{RunningZone, ZoneBuilderFactory};
 use illumos_utils::zone::PROPOLIS_ZONE_PREFIX;
 use illumos_utils::zpool::ZpoolOrRamdisk;
@@ -513,7 +514,7 @@ struct InstanceRunner {
 
     // Reference to the port manager for creating OPTE ports when starting the
     // instance
-    port_manager: PortManager,
+    port_manager: SledAgentPortManager,
 
     // Guest NIC and OPTE port information
     requested_nics: Vec<NetworkInterface>,
@@ -2274,6 +2275,7 @@ mod tests {
     use omicron_common::api::internal::shared::{DhcpConfig, SledIdentifiers};
     use omicron_common::disk::DiskIdentity;
     use omicron_uuid_kinds::InternalZpoolUuid;
+    use oximeter_instruments::kstat::KstatSemaphore;
     use propolis_client::types::{
         InstanceMigrateStatusResponse, InstanceStateMonitorResponse,
     };
@@ -2521,8 +2523,9 @@ mod tests {
             Etherstub("mystub".to_string()),
             illumos_utils::fakes::dladm::Dladm::new(),
         );
-        let port_manager = PortManager::new(
+        let port_manager = SledAgentPortManager::new(
             log.new(o!("component" => "PortManager")),
+            KstatSemaphore::new(),
             Ipv6Addr::new(0xfd00, 0x1de, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01),
         );
 

sled-agent/src/instance_manager.rs

@@ -8,13 +8,13 @@ use crate::instance::Instance;
 use crate::instance::VmmStateOwner;
 use crate::metrics::MetricsRequestQueue;
 use crate::nexus::NexusClient;
+use crate::port_manager::SledAgentPortManager;
 use crate::vmm_reservoir::VmmReservoirManagerHandle;
 use crate::zone_bundle::BundleError;
 use crate::zone_bundle::ZoneBundler;
 
 use illumos_utils::dladm::Etherstub;
 use illumos_utils::link::VnicAllocator;
-use illumos_utils::opte::PortManager;
 use illumos_utils::running_zone::ZoneBuilderFactory;
 use omicron_common::api::external::ByteCount;
 use omicron_common::api::internal::nexus::SledVmmState;
@@ -64,7 +64,7 @@ pub enum Error {
 pub(crate) struct InstanceManagerServices {
     pub nexus_client: NexusClient,
     pub vnic_allocator: VnicAllocator<Etherstub>,
-    pub port_manager: PortManager,
+    pub port_manager: SledAgentPortManager,
     pub zone_bundler: ZoneBundler,
     pub zone_builder_factory: ZoneBuilderFactory,
     pub metrics_queue: MetricsRequestQueue,
@@ -93,7 +93,7 @@ impl InstanceManager {
         log: Logger,
         nexus_client: NexusClient,
         vnic_allocator: VnicAllocator<Etherstub>,
-        port_manager: PortManager,
+        port_manager: SledAgentPortManager,
         currently_managed_zpools_rx: CurrentlyManagedZpoolsReceiver,
         available_datasets_rx: AvailableDatasetsReceiver,
         zone_bundler: ZoneBundler,
@@ -123,7 +123,7 @@ impl InstanceManager {
         log: Logger,
         nexus_client: NexusClient,
         vnic_allocator: VnicAllocator<Etherstub>,
-        port_manager: PortManager,
+        port_manager: SledAgentPortManager,
         currently_managed_zpools_rx: CurrentlyManagedZpoolsReceiver,
         available_datasets_rx: AvailableDatasetsReceiver,
         zone_bundler: ZoneBundler,
@@ -406,7 +406,7 @@ struct InstanceManagerRunner {
     jobs: BTreeMap<PropolisUuid, Instance>,
 
     vnic_allocator: VnicAllocator<Etherstub>,
-    port_manager: PortManager,
+    port_manager: SledAgentPortManager,
     currently_managed_zpools_rx: CurrentlyManagedZpoolsReceiver,
     available_datasets_rx: AvailableDatasetsReceiver,
     zone_bundler: ZoneBundler,

sled-agent/src/lib.rs

@@ -27,6 +27,7 @@ mod instance_manager;
 mod long_running_tasks;
 mod metrics;
 mod nexus;
+mod port_manager;
 mod probe_manager;
 mod profile;
 pub mod rack_setup;

sled-agent/src/metrics.rs

@@ -11,6 +11,7 @@ use omicron_common::api::internal::shared::SledIdentifiers;
 use oximeter_instruments::kstat::CollectionDetails;
 use oximeter_instruments::kstat::Error as KstatError;
 use oximeter_instruments::kstat::KstatSampler;
+use oximeter_instruments::kstat::KstatSemaphore;
 use oximeter_instruments::kstat::TargetId;
 use oximeter_instruments::kstat::link::SledDataLink;
 use oximeter_instruments::kstat::link::SledDataLinkTarget;
@@ -358,10 +359,12 @@ impl MetricsManager {
     /// Construct a new metrics manager.
     pub fn new(
         log: &Logger,
+        semaphore: KstatSemaphore,
         identifiers: SledIdentifiers,
         address: Ipv6Addr,
     ) -> Result<Self, Error> {
-        let sampler = KstatSampler::new(log).map_err(Error::Kstat)?;
+        let sampler =
+            KstatSampler::new(log, semaphore).map_err(Error::Kstat)?;
         let server = start_producer_server(&log, identifiers.sled_id, address)?;
         server
             .registry()