From 7d027346b18c46f8e224c5d3f2b337afa51a6c66 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 27 Jun 2024 13:57:39 +0100 Subject: [PATCH 01/16] Use new toolchain definition, import illumos-rs. --- .cargo/config.toml | 3 + Cargo.lock | 154 ++++++++++++++++++++++++++++++++ Cargo.toml | 2 + xde/.cargo/config.toml | 2 +- xde/Cargo.toml | 8 +- xde/rust-toolchain.toml | 3 +- xde/x86_64-illumos.json | 39 ++++++++ xde/x86_64-unknown-unknown.json | 28 ------ 8 files changed, 205 insertions(+), 34 deletions(-) create mode 100644 xde/x86_64-illumos.json delete mode 100644 xde/x86_64-unknown-unknown.json diff --git a/.cargo/config.toml b/.cargo/config.toml index f37bce65..7a9ef735 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -5,3 +5,6 @@ kbench = "bench --package opte-bench --bench xde --" [env] CARGO_WORKSPACE_DIR = { value = "", relative = true } + +[net] +git-fetch-with-cli = true diff --git a/Cargo.lock b/Cargo.lock index 4793db03..5d5f7656 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -126,6 +126,38 @@ version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" +[[package]] +name = "bindgen" +version = "0.68.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "726e4313eb6ec35d2730258ad4e15b547ee75d6afaa1361a922e78e59b7d8078" +dependencies = [ + "bitflags 2.5.0", + "cexpr", + "clang-sys", + "lazy_static", + "lazycell", + "log", + "peeking_take_while", + "prettyplease", + "proc-macro2", + "quote", + "regex", + "rustc-hash", + "shlex", + "syn 2.0.68", + "which", +] + +[[package]] +name = "bindings" +version = "0.1.0" +source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=luqmana/rusty-modules#e3f8c3d6c38e28c45f7ebc35a6f8bc06f7bd7f31" +dependencies = [ + "anyhow", + "bindgen", +] + [[package]] name = "bitflags" version = "1.3.2" @@ -217,6 +249,15 @@ dependencies = [ "once_cell", ] +[[package]] +name = "cexpr" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6fac387a98bb7c37292057cffc56d62ecb629900026402633ae9160df93a8766" +dependencies = [ + "nom", +] + [[package]] name = "cfg-if" version = "1.0.0" @@ -262,6 +303,17 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b0fc239e0f6cb375d2402d48afb92f76f5404fd1df208a41930ec81eda078bea" +[[package]] +name = "clang-sys" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b023947811758c97c59bf9d1c188fd619ad4718dcaa767947df1cadb14f39f4" +dependencies = [ + "glob", + "libc", + "libloading", +] + [[package]] name = "clap" version = "4.5.7" @@ -796,6 +848,12 @@ version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40ecd4077b5ae9fd2e9e169b102c6c330d0605168eb0e8bf79952b256dbefffd" +[[package]] +name = "glob" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" + [[package]] name = "goblin" version = "0.8.2" @@ -860,12 +918,31 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d231dfb89cfffdbc30e7fc41579ed6066ad03abda9e567ccafae602b97ec5024" +[[package]] +name = "home" +version = "0.5.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3d1354bf6b7235cb4a0576c2619fd4ed18183f689b12b006a0ee7329eeff9a5" +dependencies = [ + "windows-sys 0.52.0", +] + [[package]] name = "ident_case" version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" +[[package]] +name = "illumos" +version = "0.1.0" +source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=luqmana/rusty-modules#e3f8c3d6c38e28c45f7ebc35a6f8bc06f7bd7f31" +dependencies = [ + "bindings", + "bitflags 2.5.0", + "macros", +] + [[package]] name = "illumos-sys-hdrs" version = "0.1.0" @@ -962,6 +1039,12 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" +[[package]] +name = "lazycell" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" + [[package]] name = "libc" version = "0.2.155" @@ -984,6 +1067,16 @@ dependencies = [ "once_cell", ] +[[package]] +name = "libloading" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e310b3a6b5907f99202fcdb4960ff45b93735d7c7d96b760fcff8db2dc0e103d" +dependencies = [ + "cfg-if", + "windows-targets 0.48.5", +] + [[package]] name = "libnet" version = "0.1.0" @@ -1056,6 +1149,20 @@ version = "0.4.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" +[[package]] +name = "macros" +version = "0.1.0" +source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=luqmana/rusty-modules#e3f8c3d6c38e28c45f7ebc35a6f8bc06f7bd7f31" +dependencies = [ + "bindings", + "proc-macro2", + "quote", + "regex-lite", + "serde", + "serde_tokenstream", + "syn 2.0.68", +] + [[package]] name = "managed" version = "0.8.0" @@ -1378,6 +1485,12 @@ dependencies = [ "rusticata-macros", ] +[[package]] +name = "peeking_take_while" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "19b17cddbe7ec3f8bc800887bab5e717348c95ea2ca0b1bf0837fb964dc67099" + [[package]] name = "pest" version = "2.7.10" @@ -1509,6 +1622,16 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbc83ee4a840062f368f9096d80077a9841ec117e17e7f700df81958f1451254" +[[package]] +name = "prettyplease" +version = "0.2.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f12335488a2f3b0a83b14edad48dca9879ce89b2edd10e80237e4e852dd645e" +dependencies = [ + "proc-macro2", + "syn 2.0.68", +] + [[package]] name = "proc-macro-crate" version = "1.3.1" @@ -1654,6 +1777,12 @@ dependencies = [ "regex-syntax", ] +[[package]] +name = "regex-lite" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53a49587ad06b26609c52e423de037e7f57f20d53535d66e08c695f347df952a" + [[package]] name = "regex-syntax" version = "0.8.4" @@ -1678,6 +1807,12 @@ version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f" +[[package]] +name = "rustc-hash" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" + [[package]] name = "rusticata-macros" version = "4.1.0" @@ -1872,6 +2007,12 @@ dependencies = [ "digest", ] +[[package]] +name = "shlex" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" + [[package]] name = "signal-hook-registry" version = "1.4.2" @@ -2435,6 +2576,18 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "which" +version = "4.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87ba24419a2078cd2b0f2ede2691b6c66d8e47836da3b6db8265ebad47afbfc7" +dependencies = [ + "either", + "home", + "once_cell", + "rustix", +] + [[package]] name = "winapi" version = "0.3.9" @@ -2629,6 +2782,7 @@ version = "0.1.0" dependencies = [ "bitflags 2.5.0", "crc32fast", + "illumos", "illumos-sys-hdrs", "opte", "oxide-vpc", diff --git a/Cargo.toml b/Cargo.toml index 67f739d4..17d0aa3d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ ctor = "0.2" darling = "0.20" dyn-clone = "1.0" heapless = "0.8" +illumos = { git = "ssh://git@github.com/oxidecomputer/illumos-rs.git", branch = "luqmana/rusty-modules" } ipnetwork = { version = "0.20", default-features = false } itertools = { version = "0.12", default-features = false } libc = "0.2" @@ -81,3 +82,4 @@ poptrie = { git = "https://github.com/oxidecomputer/poptrie", branch = "multipat [profile.release] debug = 2 +lto = true diff --git a/xde/.cargo/config.toml b/xde/.cargo/config.toml index c05d2054..f3abb832 100644 --- a/xde/.cargo/config.toml +++ b/xde/.cargo/config.toml @@ -1,5 +1,5 @@ [build] -target = "x86_64-unknown-unknown.json" +target = "x86_64-illumos.json" [unstable] build-std = ["core", "alloc"] \ No newline at end of file diff --git a/xde/Cargo.toml b/xde/Cargo.toml index 24ab5076..5b38ba7c 100644 --- a/xde/Cargo.toml +++ b/xde/Cargo.toml @@ -12,11 +12,13 @@ opte = { workspace = true, features = ["engine", "kernel"], default-features = f oxide-vpc = { workspace = true, features = ["engine", "kernel"], default-features = false } bitflags.workspace = true +crc32fast.workspace = true +illumos.workspace = true postcard.workspace = true serde.workspace = true -crc32fast.workspace = true [lib] -crate-type = ["staticlib"] +crate-type = ["cdylib"] name = "xde" - +bench = false +test = false diff --git a/xde/rust-toolchain.toml b/xde/rust-toolchain.toml index fe1a3bfa..7632024d 100644 --- a/xde/rust-toolchain.toml +++ b/xde/rust-toolchain.toml @@ -1,5 +1,4 @@ [toolchain] -channel = "nightly-2024-05-12" -target = "x86_64-unknown-illumos" +channel = "nightly-2024-06-27" components = [ "clippy", "rustfmt", "rust-src" ] profile = "minimal" diff --git a/xde/x86_64-illumos.json b/xde/x86_64-illumos.json new file mode 100644 index 00000000..cfa17ab4 --- /dev/null +++ b/xde/x86_64-illumos.json @@ -0,0 +1,39 @@ +{ + "arch": "x86_64", + "code-model": "kernel", + "cpu": "x86-64", + "data-layout": "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128", + "disable-redzone": true, + "dll-prefix": "", + "dll-suffix": "", + "dynamic-linking": true, + "eh-frame-header": false, + "emit-debug-gdb-scripts": false, + "features": "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2,+soft-float", + "frame-pointer": "always", + "is-like-solaris": true, + "linker": "ld", + "linker-flavor": "ld", + "linker-is-gnu": false, + "llvm-target": "x86_64-none-none", + "max-atomic-width": 64, + "only-cdylib": true, + "os": "none", + "relocation-model": "static", + "panic-strategy": "abort", + "post-link-args": { + "ld": [ + "-ztype=kmod", + "-Ndrv/dld", + "-Ndrv/ip", + "-Nmisc/dls", + "-Nmisc/mac" + ] + }, + "pre-link-args": { + "ld": [ + "-r" + ] + }, + "target-pointer-width": "64" +} diff --git a/xde/x86_64-unknown-unknown.json b/xde/x86_64-unknown-unknown.json deleted file mode 100644 index 4cafc73d..00000000 --- a/xde/x86_64-unknown-unknown.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "arch": "x86_64", - "code-model": "kernel", - "cpu": "x86-64", - "data-layout": "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128", - "disable-redzone": true, - "dynamic-linking": false, - "eh-frame-header": false, - "frame-pointer": "always", - "executables": true, - "features": "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2,+soft-float", - "has-rpath": true, - "is-builtin": false, - "is-like-solaris": true, - "limit-rdylib-exports": false, - "linker": "ld", - "llvm-target": "x86_64-none-none", - "max-atomic-width": 64, - "no-default-libraries": true, - "os": "none", - "panic-strategy": "abort", - "relax-elf-relocations": false, - "relocation-model": "static", - "relro-level": "full", - "staticlib-prefix": "", - "target-family": "unix", - "target-pointer-width": "64" -} From be94176be85da8b9277487608e4a2a9e1950d625 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 27 Jun 2024 18:45:37 +0100 Subject: [PATCH 02/16] The great culling of KMutex/RwLock. Still some minor issues to shake out, but I like it. --- Cargo.lock | 9 +- Cargo.toml | 2 +- lib/opte/Cargo.toml | 3 +- lib/opte/src/ddi/sync.rs | 425 +++++----------------------- lib/opte/src/dynamic.rs | 4 +- lib/opte/src/engine/port.rs | 5 +- lib/opte/src/engine/snat.rs | 3 +- lib/oxide-vpc/src/engine/overlay.rs | 18 +- xde/src/lib.rs | 9 - xde/src/route.rs | 2 - xde/src/xde.rs | 81 +++++- 11 files changed, 167 insertions(+), 394 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d5f7656..a49370ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -152,7 +152,7 @@ dependencies = [ [[package]] name = "bindings" version = "0.1.0" -source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=luqmana/rusty-modules#e3f8c3d6c38e28c45f7ebc35a6f8bc06f7bd7f31" +source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" dependencies = [ "anyhow", "bindgen", @@ -936,7 +936,7 @@ checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" [[package]] name = "illumos" version = "0.1.0" -source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=luqmana/rusty-modules#e3f8c3d6c38e28c45f7ebc35a6f8bc06f7bd7f31" +source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" dependencies = [ "bindings", "bitflags 2.5.0", @@ -1074,7 +1074,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e310b3a6b5907f99202fcdb4960ff45b93735d7c7d96b760fcff8db2dc0e103d" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.5", ] [[package]] @@ -1152,7 +1152,7 @@ checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" [[package]] name = "macros" version = "0.1.0" -source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=luqmana/rusty-modules#e3f8c3d6c38e28c45f7ebc35a6f8bc06f7bd7f31" +source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" dependencies = [ "bindings", "proc-macro2", @@ -1324,6 +1324,7 @@ dependencies = [ "derror-macro", "dyn-clone", "heapless", + "illumos", "illumos-sys-hdrs", "itertools 0.12.1", "kstat-macro", diff --git a/Cargo.toml b/Cargo.toml index 17d0aa3d..10e148ab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,7 +50,7 @@ ctor = "0.2" darling = "0.20" dyn-clone = "1.0" heapless = "0.8" -illumos = { git = "ssh://git@github.com/oxidecomputer/illumos-rs.git", branch = "luqmana/rusty-modules" } +illumos = { git = "ssh://git@github.com/oxidecomputer/illumos-rs.git", branch = "felixmcfelix/rusty-for-opte" } ipnetwork = { version = "0.20", default-features = false } itertools = { version = "0.12", default-features = false } libc = "0.2" diff --git a/lib/opte/Cargo.toml b/lib/opte/Cargo.toml index 0eb965b3..e1d87d6b 100644 --- a/lib/opte/Cargo.toml +++ b/lib/opte/Cargo.toml @@ -10,7 +10,7 @@ repository.workspace = true default = ["api", "std"] api = [] engine = ["api", "dep:crc32fast", "dep:derror-macro", "dep:heapless", "dep:itertools", "dep:zerocopy"] -kernel = ["illumos-sys-hdrs/kernel"] +kernel = ["illumos-sys-hdrs/kernel", "dep:illumos"] # This feature indicates that OPTE is being built with std. This is # mostly useful to consumers of the API, providing convenient methods # for working with the API types in a std context. @@ -31,6 +31,7 @@ cfg-if.workspace = true crc32fast = { workspace = true, optional = true } dyn-clone.workspace = true heapless = { workspace = true, optional = true } +illumos = { workspace = true, optional = true } itertools = { workspace = true, optional = true } postcard.workspace = true serde.workspace = true diff --git a/lib/opte/src/ddi/sync.rs b/lib/opte/src/ddi/sync.rs index 6050a738..f5d74208 100644 --- a/lib/opte/src/ddi/sync.rs +++ b/lib/opte/src/ddi/sync.rs @@ -2,401 +2,118 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2022 Oxide Computer Company +// Copyright 2024 Oxide Computer Company //! Safe abstractions for synchronization primitives. -//! -//! TODO: This should be in its own crate, wrapping the illumos-ddi-dki -//! crate. But for now just let it live here. -use core::ops::Deref; -use core::ops::DerefMut; - -cfg_if! { - if #[cfg(all(not(feature = "std"), not(test)))] { - use core::cell::UnsafeCell; - use core::ptr; - use illumos_sys_hdrs::{ - kmutex_t, krw_t, krwlock_t, mutex_enter, mutex_exit, - mutex_destroy, mutex_init, rw_enter, rw_exit, rw_init, - rw_destroy - }; - } else { - use std::sync::Mutex; - } -} - -use illumos_sys_hdrs::kmutex_type_t; -use illumos_sys_hdrs::krw_type_t; - -/// Exposes the illumos mutex(9F) API in a safe manner. We name it -/// `KMutex` (Kernel Mutex) on purpose. The API for a kernel mutex -/// isn't quite the same as a userland `Mutex`, and there's no reason -/// that we have to use that exact name. Using `KMutex` makes it -/// obvious that we are using a mutex, but not the one that comes from -/// std. -/// -/// Our `kmutex_t` implementation has no self referential pointers, so -/// there is no reason it needs to pin to a location. This allows us to -/// have an API that looks more Rust-like: returning a `KMutex` value -/// that is initialized and can be placed anywhere. This is in contrast -/// to the typical illumos API where you have a `kmutex_t` embedded in -/// your structure (or as a global) and pass a pointer to -/// `mutex_init(9F)` to initialize it in place. -/// -/// For now we assume only `Sized` types are protected by a mutex. For -/// some reason, rust-for-linux adds a `?Sized` bound for the type -/// definition as well as various impl blocks, minus the one that deals -/// with creating a new mutex. I'm not sure why they do this, -/// esepcially if the impl prevents you from creating a mutex holding a -/// DST. I'm not sure if a mutex should ever hold a DST, because a DST -/// is necessairly a pointer, and we would need to make sure that if a -/// shared reference was passed in that it's the only one outstanindg. -/// -/// It seems the std Mutex also does this, but once against I'm not -/// sure why. -#[cfg(all(not(feature = "std"), not(test)))] -pub struct KMutex { - // The mutex(9F) structure. - mutex: KMutexInner, - - // The data this mutex protects. - data: UnsafeCell, -} - -#[cfg(all(not(feature = "std"), not(test)))] -struct KMutexInner(UnsafeCell); - -#[cfg(all(not(feature = "std"), not(test)))] -impl Drop for KMutexInner { - fn drop(&mut self) { - unsafe { mutex_destroy(self.0.get()) } - } -} - -pub enum KMutexType { - Adaptive = kmutex_type_t::MUTEX_ADAPTIVE as isize, - Spin = kmutex_type_t::MUTEX_SPIN as isize, - Driver = kmutex_type_t::MUTEX_DRIVER as isize, - Default = kmutex_type_t::MUTEX_DEFAULT as isize, -} - -impl From for kmutex_type_t { - fn from(mtype: KMutexType) -> Self { - match mtype { - KMutexType::Adaptive => kmutex_type_t::MUTEX_ADAPTIVE, - KMutexType::Spin => kmutex_type_t::MUTEX_SPIN, - KMutexType::Driver => kmutex_type_t::MUTEX_DRIVER, - KMutexType::Default => kmutex_type_t::MUTEX_DEFAULT, - } - } -} - -// TODO understand: -// -// o Why does rust-for-linux use `T: ?Sized` for struct def. -#[cfg(all(not(feature = "std"), not(test)))] -impl KMutex { - pub fn into_inner(self) -> T - where - T: Sized, - { - self.data.into_inner() - } - - /// Create, initialize, and return a new kernel mutex (mutex(9F)) - /// of type `mtype`, and wrap it around `val`. The returned - /// `KMutex` is the new owner of `val`. All access from here on out - /// must be done by acquiring a `KMutexGuard` via the `lock()` - /// method. - pub fn new(val: T, mtype: KMutexType) -> Self { - let mut kmutex = kmutex_t { _opaque: 0 }; - // TODO This assumes the mutex is never used in interrupt - // context. Need to pass 4th arg to set priority. - // - // We never use the mutex name argument. - // - // Safety: ???. - unsafe { - mutex_init(&mut kmutex, ptr::null(), mtype.into(), ptr::null()); - } - - KMutex { - mutex: KMutexInner(UnsafeCell::new(kmutex)), - data: UnsafeCell::new(val), - } - } - - /// Try to acquire the mutex guard to gain access to the underlying - /// value. If the guard is currently held, then this call will - /// block. The mutex is released when the guard is dropped. - pub fn lock(&self) -> KMutexGuard { - // Safety: ???. - unsafe { mutex_enter(self.mutex.0.get()) }; - KMutexGuard { lock: self } - } -} - -unsafe impl Send for KMutex {} -unsafe impl Sync for KMutex {} - -#[cfg(all(not(feature = "std"), not(test)))] -pub struct KMutexGuard<'a, T: 'a> { - lock: &'a KMutex, -} #[cfg(all(not(feature = "std"), not(test)))] -impl Drop for KMutexGuard<'_, T> { - fn drop(&mut self) { - // Safety: ???. - unsafe { mutex_exit(self.lock.mutex.0.get()) }; - } -} - -#[cfg(all(not(feature = "std"), not(test)))] -impl Deref for KMutexGuard<'_, T> { - type Target = T; - - fn deref(&self) -> &T { - unsafe { &*self.lock.data.get() } - } -} +pub use illumos::sync::*; -#[cfg(all(not(feature = "std"), not(test)))] -impl DerefMut for KMutexGuard<'_, T> { - fn deref_mut(&mut self) -> &mut T { - unsafe { &mut *self.lock.data.get() } - } -} - -// In a std environment we just wrap `Mutex`. #[cfg(any(feature = "std", test))] -pub struct KMutex { - inner: Mutex, -} - -#[cfg(any(feature = "std", test))] -pub struct KMutexGuard<'a, T: 'a> { - guard: std::sync::MutexGuard<'a, T>, -} +pub use wrapper::*; #[cfg(any(feature = "std", test))] -impl Deref for KMutexGuard<'_, T> { - type Target = T; +mod wrapper { + use core::ops::Deref; + use core::ops::DerefMut; + use std::sync::Mutex; - fn deref(&self) -> &T { - self.guard.deref() - } -} - -#[cfg(any(feature = "std", test))] -impl DerefMut for KMutexGuard<'_, T> { - fn deref_mut(&mut self) -> &mut T { - self.guard.deref_mut() - } -} - -#[cfg(any(feature = "std", test))] -impl KMutex { - pub fn into_inner(self) -> T - where - T: Sized, - { - self.inner.into_inner().unwrap() + // In a std environment we just wrap `Mutex`. + pub struct KMutex { + inner: Mutex, } - pub fn new(val: T, _mtype: KMutexType) -> Self { - KMutex { inner: Mutex::new(val) } + pub struct KMutexGuard<'a, T: 'a> { + guard: std::sync::MutexGuard<'a, T>, } - pub fn lock(&self) -> KMutexGuard { - let guard = self.inner.lock().unwrap(); - KMutexGuard { guard } - } -} - -/// A wrapper around illumos rwlock(9F) -#[cfg(all(not(feature = "std"), not(test)))] -pub struct KRwLock { - rwl: UnsafeCell, - data: UnsafeCell, -} + impl Deref for KMutexGuard<'_, T> { + type Target = T; -#[cfg(all(not(feature = "std"), not(test)))] -impl Drop for KRwLock { - fn drop(&mut self) { - unsafe { rw_destroy(self.rwl.get()) } - } -} - -pub enum KRwLockType { - Driver = krw_type_t::RW_DRIVER as isize, - Default = krw_type_t::RW_DEFAULT as isize, -} - -impl From for krw_type_t { - fn from(typ: KRwLockType) -> Self { - match typ { - KRwLockType::Driver => krw_type_t::RW_DRIVER, - KRwLockType::Default => krw_type_t::RW_DEFAULT, + fn deref(&self) -> &T { + self.guard.deref() } } -} -#[cfg(all(not(feature = "std"), not(test)))] -pub enum KRwEnterType { - Writer = krw_t::RW_WRITER.0 as isize, - Reader = krw_t::RW_READER.0 as isize, - ReaderStarveWriter = krw_t::RW_READER_STARVEWRITER.0 as isize, -} - -#[cfg(all(not(feature = "std"), not(test)))] -impl From for krw_t { - fn from(typ: KRwEnterType) -> Self { - match typ { - KRwEnterType::Writer => krw_t::RW_WRITER, - KRwEnterType::Reader => krw_t::RW_READER, - KRwEnterType::ReaderStarveWriter => krw_t::RW_READER_STARVEWRITER, + impl DerefMut for KMutexGuard<'_, T> { + fn deref_mut(&mut self) -> &mut T { + self.guard.deref_mut() } } -} -#[cfg(all(not(feature = "std"), not(test)))] -impl KRwLock { - pub const fn new(val: T) -> Self { - let rwl = krwlock_t { _opaque: 0 }; - KRwLock { rwl: UnsafeCell::new(rwl), data: UnsafeCell::new(val) } - } + impl KMutex { + pub fn into_inner(self) -> T + where + T: Sized, + { + self.inner.into_inner().unwrap() + } - pub fn init(&mut self, typ: KRwLockType) { - unsafe { - rw_init(self.rwl.get_mut(), ptr::null(), typ.into(), ptr::null()); + pub fn new(val: T) -> Self { + KMutex { inner: Mutex::new(val) } } - } - pub fn read(&self) -> KRwLockReadGuard { - unsafe { rw_enter(self.rwl.get(), krw_t::RW_READER) }; - KRwLockReadGuard { lock: self } + pub fn lock(&self) -> KMutexGuard { + let guard = self.inner.lock().unwrap(); + KMutexGuard { guard } + } } - pub fn write(&self) -> KRwLockWriteGuard { - unsafe { rw_enter(self.rwl.get(), krw_t::RW_WRITER) }; - KRwLockWriteGuard { lock: self } + // In a std environment we just wrap `RwLock`. + pub struct KRwLock { + inner: std::sync::RwLock, } -} -unsafe impl Send for KRwLock {} -unsafe impl Sync for KRwLock {} - -#[cfg(all(not(feature = "std"), not(test)))] -pub struct KRwLockReadGuard<'a, T: 'a> { - lock: &'a KRwLock, -} - -#[cfg(all(not(feature = "std"), not(test)))] -impl Deref for KRwLockReadGuard<'_, T> { - type Target = T; - fn deref(&self) -> &T { - unsafe { &*self.lock.data.get() } - } -} - -#[cfg(all(not(feature = "std"), not(test)))] -impl Drop for KRwLockReadGuard<'_, T> { - fn drop(&mut self) { - unsafe { rw_exit(self.lock.rwl.get()) }; + pub struct KRwLockReadGuard<'a, T: 'a> { + guard: std::sync::RwLockReadGuard<'a, T>, } -} -#[cfg(all(not(feature = "std"), not(test)))] -pub struct KRwLockWriteGuard<'a, T: 'a> { - lock: &'a KRwLock, -} - -#[cfg(all(not(feature = "std"), not(test)))] -impl Deref for KRwLockWriteGuard<'_, T> { - type Target = T; - fn deref(&self) -> &T { - unsafe { &*self.lock.data.get() } + pub struct KRwLockWriteGuard<'a, T: 'a> { + guard: std::sync::RwLockWriteGuard<'a, T>, } -} -#[cfg(all(not(feature = "std"), not(test)))] -impl DerefMut for KRwLockWriteGuard<'_, T> { - fn deref_mut(&mut self) -> &mut T { - unsafe { &mut *self.lock.data.get() } - } -} + impl Deref for KRwLockReadGuard<'_, T> { + type Target = T; -#[cfg(all(not(feature = "std"), not(test)))] -impl Drop for KRwLockWriteGuard<'_, T> { - fn drop(&mut self) { - unsafe { rw_exit(self.lock.rwl.get()) }; - } -} - -// In a std environment we just wrap `RwLock`. -#[cfg(any(feature = "std", test))] -pub struct KRwLock { - inner: std::sync::RwLock, -} - -#[cfg(any(feature = "std", test))] -pub struct KRwLockReadGuard<'a, T: 'a> { - guard: std::sync::RwLockReadGuard<'a, T>, -} - -#[cfg(any(feature = "std", test))] -pub struct KRwLockWriteGuard<'a, T: 'a> { - guard: std::sync::RwLockWriteGuard<'a, T>, -} - -#[cfg(any(feature = "std", test))] -impl Deref for KRwLockReadGuard<'_, T> { - type Target = T; - - fn deref(&self) -> &T { - self.guard.deref() + fn deref(&self) -> &T { + self.guard.deref() + } } -} - -#[cfg(any(feature = "std", test))] -impl Deref for KRwLockWriteGuard<'_, T> { - type Target = T; - fn deref(&self) -> &T { - self.guard.deref() - } -} + impl Deref for KRwLockWriteGuard<'_, T> { + type Target = T; -#[cfg(any(feature = "std", test))] -impl DerefMut for KRwLockWriteGuard<'_, T> { - fn deref_mut(&mut self) -> &mut T { - self.guard.deref_mut() + fn deref(&self) -> &T { + self.guard.deref() + } } -} -#[cfg(any(feature = "std", test))] -impl KRwLock { - pub fn into_inner(self) -> T - where - T: Sized, - { - self.inner.into_inner().unwrap() + impl DerefMut for KRwLockWriteGuard<'_, T> { + fn deref_mut(&mut self) -> &mut T { + self.guard.deref_mut() + } } - pub fn new(val: T) -> Self { - KRwLock { inner: std::sync::RwLock::new(val) } - } + impl KRwLock { + pub fn into_inner(self) -> T + where + T: Sized, + { + self.inner.into_inner().unwrap() + } - pub fn init(&mut self, _typ: KRwLockType) {} + pub fn new(val: T) -> Self { + KRwLock { inner: std::sync::RwLock::new(val) } + } - pub fn read(&self) -> KRwLockReadGuard { - let guard = self.inner.read().unwrap(); - KRwLockReadGuard { guard } - } + pub fn read(&self) -> KRwLockReadGuard { + let guard = self.inner.read().unwrap(); + KRwLockReadGuard { guard } + } - pub fn write(&self) -> KRwLockWriteGuard { - let guard = self.inner.write().unwrap(); - KRwLockWriteGuard { guard } + pub fn write(&self) -> KRwLockWriteGuard { + let guard = self.inner.write().unwrap(); + KRwLockWriteGuard { guard } + } } } diff --git a/lib/opte/src/dynamic.rs b/lib/opte/src/dynamic.rs index ad1913e8..d20d1d8d 100644 --- a/lib/opte/src/dynamic.rs +++ b/lib/opte/src/dynamic.rs @@ -13,7 +13,6 @@ // TODO: Implement the generated outputs to reduce cost of, e.g., DHCP responses. use crate::ddi::sync::KRwLock; -use crate::ddi::sync::KRwLockType; use alloc::sync::Arc; use core::fmt::Debug; use core::ops::Deref; @@ -37,8 +36,7 @@ pub struct Snapshot { impl From for Dynamic { fn from(value: T) -> Self { - let mut inner = KRwLock::new(value.into()); - inner.init(KRwLockType::Driver); + let inner = KRwLock::new(value.into()); Self(InnerDynamic { inner, epoch: AtomicU64::default() }.into()) } diff --git a/lib/opte/src/engine/port.rs b/lib/opte/src/engine/port.rs index a5cdb4de..8759863b 100644 --- a/lib/opte/src/engine/port.rs +++ b/lib/opte/src/engine/port.rs @@ -49,7 +49,6 @@ use crate::ddi::kstat::KStatNamed; use crate::ddi::kstat::KStatProvider; use crate::ddi::kstat::KStatU64; use crate::ddi::sync::KMutex; -use crate::ddi::sync::KMutexType; use crate::ddi::time::Moment; use crate::engine::flow_table::ExpiryPolicy; use crate::engine::tcp::TcpMeta; @@ -298,7 +297,7 @@ impl PortBuilder { ectx: self.ectx, epoch: AtomicU64::new(1), net, - data: KMutex::new(data, KMutexType::Driver), + data: KMutex::new(data), }) } @@ -350,7 +349,7 @@ impl PortBuilder { name_cstr, mac, ectx, - layers: KMutex::new(Vec::new(), KMutexType::Driver), + layers: KMutex::new(Vec::new()), } } diff --git a/lib/opte/src/engine/snat.rs b/lib/opte/src/engine/snat.rs index 39b2ed85..e8303e25 100644 --- a/lib/opte/src/engine/snat.rs +++ b/lib/opte/src/engine/snat.rs @@ -29,7 +29,6 @@ use super::rule::ResourceEntry; use super::rule::ResourceError; use super::rule::StatefulAction; use crate::ddi::sync::KMutex; -use crate::ddi::sync::KMutexType; use crate::engine::icmp::QueryEcho; use alloc::collections::btree_map::BTreeMap; use alloc::string::ToString; @@ -133,7 +132,7 @@ impl NatPool { /// Create a new NAT pool, with no entries. pub fn new() -> Self { - NatPool { free_list: KMutex::new(BTreeMap::new(), KMutexType::Driver) } + NatPool { free_list: KMutex::new(BTreeMap::new()) } } // A helper function to verify correct operation during testing. diff --git a/lib/oxide-vpc/src/engine/overlay.rs b/lib/oxide-vpc/src/engine/overlay.rs index 730ac41e..7e9469df 100644 --- a/lib/oxide-vpc/src/engine/overlay.rs +++ b/lib/oxide-vpc/src/engine/overlay.rs @@ -30,9 +30,7 @@ use opte::api::MacAddr; use opte::api::OpteError; use opte::ddi::sync::KMutex; use opte::ddi::sync::KMutexGuard; -use opte::ddi::sync::KMutexType; use opte::ddi::sync::KRwLock; -use opte::ddi::sync::KRwLockType; use opte::engine::ether::EtherMeta; use opte::engine::ether::EtherMod; use opte::engine::ether::EtherType; @@ -511,7 +509,7 @@ impl VpcMappings { } pub fn new() -> Self { - VpcMappings { inner: KMutex::new(BTreeMap::new(), KMutexType::Driver) } + VpcMappings { inner: KMutex::new(BTreeMap::new()) } } } @@ -586,14 +584,12 @@ impl Virt2Boundary { } pub fn new() -> Self { - let mut pt4 = KRwLock::new(Poptrie::default()); - pt4.init(KRwLockType::Driver); - let mut pt6 = KRwLock::new(Poptrie::default()); - pt6.init(KRwLockType::Driver); + let pt4 = KRwLock::new(Poptrie::default()); + let pt6 = KRwLock::new(Poptrie::default()); Virt2Boundary { - ip4: KMutex::new(BTreeMap::new(), KMutexType::Driver), - ip6: KMutex::new(BTreeMap::new(), KMutexType::Driver), + ip4: KMutex::new(BTreeMap::new()), + ip6: KMutex::new(BTreeMap::new()), pt4, pt6, } @@ -747,8 +743,8 @@ impl Virt2Phys { pub fn new() -> Self { Virt2Phys { - ip4: KMutex::new(BTreeMap::new(), KMutexType::Driver), - ip6: KMutex::new(BTreeMap::new(), KMutexType::Driver), + ip4: KMutex::new(BTreeMap::new()), + ip6: KMutex::new(BTreeMap::new()), } } } diff --git a/xde/src/lib.rs b/xde/src/lib.rs index 726f1ef4..8a7b8ac6 100644 --- a/xde/src/lib.rs +++ b/xde/src/lib.rs @@ -81,15 +81,6 @@ unsafe impl GlobalAlloc for KmemAlloc { } } -#[panic_handler] -fn panic_hdlr(info: &PanicInfo) -> ! { - let msg = CString::new(format!("{}", info)).expect("cstring new"); - unsafe { - cmn_err(CE_WARN, msg.as_ptr()); - panic(msg.as_ptr()); - } -} - // The GlobalAlloc is using KM_SLEEP; we can never hit this. However, the // compiler forces us to define it, so we do. #[alloc_error_handler] diff --git a/xde/src/route.rs b/xde/src/route.rs index 089bc36c..7603bdf3 100644 --- a/xde/src/route.rs +++ b/xde/src/route.rs @@ -17,7 +17,6 @@ use core::ptr; use core::time::Duration; use illumos_sys_hdrs::*; use opte::ddi::sync::KRwLock; -use opte::ddi::sync::KRwLockType; use opte::ddi::time::Moment; use opte::engine::ether::EtherAddr; use opte::engine::ip6::Ipv6Addr; @@ -521,7 +520,6 @@ pub struct RouteCache(Arc>>); impl Default for RouteCache { fn default() -> Self { let mut lock = KRwLock::new(BTreeMap::new()); - lock.init(KRwLockType::Driver); Self(lock.into()) } } diff --git a/xde/src/xde.rs b/xde/src/xde.rs index ce859a2e..4529dc49 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -53,9 +53,7 @@ use opte::api::SetXdeUnderlayReq; use opte::api::XDE_IOC_OPTE_CMD; use opte::d_error::LabelBlock; use opte::ddi::sync::KMutex; -use opte::ddi::sync::KMutexType; use opte::ddi::sync::KRwLock; -use opte::ddi::sync::KRwLockType; use opte::ddi::time::Interval; use opte::ddi::time::Periodic; use opte::engine::ether::EtherAddr; @@ -107,6 +105,82 @@ use oxide_vpc::engine::router; use oxide_vpc::engine::VpcNetwork; use oxide_vpc::engine::VpcParser; +// use illumos::driver::cb::CharBlockOps; +// use illumos::driver::cb::OpenFlags; +// use illumos::driver::Driver; +// use illumos::module; +// use illumos::module::Module; + +// module!( +// kind = Driver { +// info = "$ENV{CARGO_PKG_NAME} $ENV{CARGO_PKG_VERSION}", +// }, +// type = Xde2, +// ); + +// struct Xde2 { +// devs: Arc>, +// } + +// impl Driver for Xde2 { +// fn get_dev_info( +// &self, +// dev: illumos::driver::Dev, +// ) -> Option { +// todo!() +// } + +// fn get_dev_instance( +// &self, +// dev: illumos::driver::Dev, +// ) -> Option { +// todo!() +// } + +// fn attach( +// &self, +// dev_info: illumos::driver::DevInfo, +// ) -> illumos::driver::AttachResult { +// // dev_info.get_dev_info() +// todo!() +// } + +// fn detach( +// &self, +// instance: illumos::driver::DevInstance, +// ) -> illumos::driver::DetachResult { +// todo!() +// } +// } + +// impl Module for Xde2 { +// fn init() -> Result { +// todo!() +// } + +// fn fini(self) { +// todo!() +// } +// } + +// impl CharBlockOps for Xde2 { +// fn open( +// &self, +// minor: illumos::driver::DevMinor, +// flags: OpenFlags, +// ) -> Result { +// todo!() +// } + +// fn close( +// &self, +// minor: illumos::driver::DevMinor, +// open_flags: OpenFlags, +// ) -> Result<(), illumos::Error> { +// todo!() +// } +// } + // Entry limits for the various flow tables. // // Safety: Despite the name of `new_unchecked`, there actually is a compile-time @@ -253,7 +327,7 @@ impl XdeState { fn new() -> Self { let ectx = Arc::new(ExecCtx { log: Box::new(opte::KernelLog {}) }); XdeState { - underlay: KMutex::new(None, KMutexType::Driver), + underlay: KMutex::new(None), ectx, vpc_map: Arc::new(overlay::VpcMappings::new()), v2b: Arc::new(overlay::Virt2Boundary::new()), @@ -299,7 +373,6 @@ pub struct XdeDev { #[cfg(not(test))] #[no_mangle] unsafe extern "C" fn _init() -> c_int { - xde_devs.init(KRwLockType::Driver); mac::mac_init_ops(addr_of_mut!(xde_devops), XDE_STR); match mod_install(&xde_linkage) { From 4f415bee4ea4eb95831b01be890710b35fdcb969 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 28 Jun 2024 09:21:42 +0100 Subject: [PATCH 03/16] Ugly but works (for now). --- xde/src/lib.rs | 35 ----------------------------------- xde/src/route.rs | 2 +- xde/src/xde.rs | 48 ++++++++++++++++++++++++++---------------------- 3 files changed, 27 insertions(+), 58 deletions(-) diff --git a/xde/src/lib.rs b/xde/src/lib.rs index 8a7b8ac6..fc558a31 100644 --- a/xde/src/lib.rs +++ b/xde/src/lib.rs @@ -49,38 +49,6 @@ pub mod secpolicy; pub mod sys; pub mod xde; -// On alignment, `kmem_alloc(9F)` has this of offer: -// -// > The allocated memory is at least double-word aligned, so it can -// > hold any C data structure. No greater alignment can be assumed. -// -// I really hate when documentation uses "word", because that seems to -// mean different things in different contexts. In this case I have to -// assume it means native integer size, or 32-bit in the case our our -// AMD64 kernel. This implis all allocations are at least 8-byte -// aligned, but could be more. However, the last sentence in the quote -// above says that you cannot assume alignment is ever greater than 8 -// bytes. Therefore, it seems best to assume it's 8 bytes. For the -// purposes of implementing GlobalAlloc, I believe this means that we -// should return NULL for any Layout which requests more than 8-byte -// alignment (or probably just panic since I never expect this). -// Furthermore, things that can use smaller alignment will just have -// to live with the larger alignment. -struct KmemAlloc; - -unsafe impl GlobalAlloc for KmemAlloc { - unsafe fn alloc(&self, layout: Layout) -> *mut u8 { - if layout.align() > 8 { - panic!("kernel alloc greater than 8-byte alignment"); - } - kmem_alloc(layout.size(), KM_SLEEP) as *mut u8 - } - - unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { - kmem_free(ptr as *mut c_void, layout.size() as size_t) - } -} - // The GlobalAlloc is using KM_SLEEP; we can never hit this. However, the // compiler forces us to define it, so we do. #[alloc_error_handler] @@ -88,9 +56,6 @@ fn alloc_error(_: Layout) -> ! { panic!("allocation error"); } -#[global_allocator] -static A: KmemAlloc = KmemAlloc; - // This is a hack to get around the fact that liballoc includes // calls to _Unwind_Resume, supposedly because it is not compiled // with `panic=abort`. This is all a little bit beyond me but I just diff --git a/xde/src/route.rs b/xde/src/route.rs index 7603bdf3..b6a93f32 100644 --- a/xde/src/route.rs +++ b/xde/src/route.rs @@ -519,7 +519,7 @@ pub struct RouteCache(Arc>>); impl Default for RouteCache { fn default() -> Self { - let mut lock = KRwLock::new(BTreeMap::new()); + let lock = KRwLock::new(BTreeMap::new()); Self(lock.into()) } } diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 4529dc49..ffb72cf6 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -200,7 +200,7 @@ static mut XDE_CTL_MINOR: minor_t = 0; /// A list of xde devices instantiated through xde_ioc_create. #[allow(clippy::vec_box)] -static mut xde_devs: KRwLock>> = KRwLock::new(Vec::new()); +static mut xde_devs: Option>>> = None; /// DDI dev info pointer to the attached xde device. static mut xde_dip: *mut dev_info = ptr::null_mut(); @@ -375,6 +375,10 @@ pub struct XdeDev { unsafe extern "C" fn _init() -> c_int { mac::mac_init_ops(addr_of_mut!(xde_devops), XDE_STR); + unsafe { + xde_devs = Some(KRwLock::new(vec![])); + } + match mod_install(&xde_linkage) { 0 => 0, err => { @@ -729,7 +733,7 @@ fn create_xde(req: &CreateXdeReq) -> Result { // // This does mean that the current Rx path is blocked on device // creation, but that's a price we need to pay for the moment. - let mut devs = unsafe { xde_devs.write() }; + let mut devs = unsafe { xde_devs.as_ref().unwrap().write() }; if devs.iter().any(|x| x.devname == req.xde_devname) { return Err(OpteError::PortExists(req.xde_devname.clone())); } @@ -891,7 +895,7 @@ fn create_xde(req: &CreateXdeReq) -> Result { #[no_mangle] fn delete_xde(req: &DeleteXdeReq) -> Result { let state = get_xde_state(); - let mut devs = unsafe { xde_devs.write() }; + let mut devs = unsafe { xde_devs.as_ref().unwrap().write() }; let index = match devs.iter().position(|x| x.devname == req.xde_devname) { Some(index) => index, None => return Err(OpteError::PortNotFound(req.xde_devname.clone())), @@ -980,7 +984,7 @@ fn clear_xde_underlay() -> Result { msg: "underlay not yet initialized".into(), }); } - if unsafe { xde_devs.read().len() } > 0 { + if unsafe { xde_devs.as_ref().unwrap().read().len() } > 0 { return Err(OpteError::System { errno: EBUSY, msg: "underlay in use by attached ports".into(), @@ -1322,7 +1326,7 @@ unsafe extern "C" fn xde_detach( _ => return DDI_FAILURE, } - if xde_devs.read().len() > 0 { + if xde_devs.as_ref().unwrap().read().len() > 0 { warn!("failed to detach: outstanding ports"); return DDI_FAILURE; } @@ -1516,7 +1520,7 @@ fn guest_loopback( ) -> *mut mblk_t { use Direction::*; let ether_dst = pkt.meta().inner.ether.dst; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let maybe_dest_dev = devs.iter().find(|x| x.vni == vni && x.port.mac_addr() == ether_dst); @@ -1957,7 +1961,7 @@ unsafe fn xde_rx_one( }; let meta = pkt.meta(); - let devs = xde_devs.read(); + let devs = xde_devs.as_ref().unwrap().read(); // Determine where to send packet based on Geneve VNI and // destination MAC address. @@ -2009,7 +2013,7 @@ unsafe fn xde_rx_one( #[no_mangle] fn add_router_entry_hdlr(env: &mut IoctlEnvelope) -> Result { let req: AddRouterEntryReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2024,7 +2028,7 @@ fn del_router_entry_hdlr( env: &mut IoctlEnvelope, ) -> Result { let req: DelRouterEntryReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2037,7 +2041,7 @@ fn del_router_entry_hdlr( #[no_mangle] fn add_fw_rule_hdlr(env: &mut IoctlEnvelope) -> Result { let req: AddFwRuleReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2051,7 +2055,7 @@ fn add_fw_rule_hdlr(env: &mut IoctlEnvelope) -> Result { #[no_mangle] fn rem_fw_rule_hdlr(env: &mut IoctlEnvelope) -> Result { let req: RemFwRuleReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2065,7 +2069,7 @@ fn rem_fw_rule_hdlr(env: &mut IoctlEnvelope) -> Result { #[no_mangle] fn set_fw_rules_hdlr(env: &mut IoctlEnvelope) -> Result { let req: SetFwRulesReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2131,7 +2135,7 @@ fn list_layers_hdlr( env: &mut IoctlEnvelope, ) -> Result { let req: api::ListLayersReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2144,7 +2148,7 @@ fn list_layers_hdlr( #[no_mangle] fn clear_uft_hdlr(env: &mut IoctlEnvelope) -> Result { let req: api::ClearUftReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2158,7 +2162,7 @@ fn clear_uft_hdlr(env: &mut IoctlEnvelope) -> Result { #[no_mangle] fn clear_lft_hdlr(env: &mut IoctlEnvelope) -> Result { let req: api::ClearLftReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2174,7 +2178,7 @@ fn dump_uft_hdlr( env: &mut IoctlEnvelope, ) -> Result { let req: api::DumpUftReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2189,7 +2193,7 @@ fn dump_layer_hdlr( env: &mut IoctlEnvelope, ) -> Result { let req: api::DumpLayerReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2204,7 +2208,7 @@ fn dump_tcp_flows_hdlr( env: &mut IoctlEnvelope, ) -> Result { let req: api::DumpTcpFlowsReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2217,7 +2221,7 @@ fn dump_tcp_flows_hdlr( #[no_mangle] fn set_external_ips_hdlr(env: &mut IoctlEnvelope) -> Result { let req: oxide_vpc::api::SetExternalIpsReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2231,7 +2235,7 @@ fn set_external_ips_hdlr(env: &mut IoctlEnvelope) -> Result { #[no_mangle] fn allow_cidr_hdlr(env: &mut IoctlEnvelope) -> Result { let req: oxide_vpc::api::AllowCidrReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2248,7 +2252,7 @@ fn remove_cidr_hdlr( env: &mut IoctlEnvelope, ) -> Result { let req: oxide_vpc::api::RemoveCidrReq = env.copy_in_req()?; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; let mut iter = devs.iter(); let dev = match iter.find(|x| x.devname == req.port_name) { Some(dev) => dev, @@ -2262,7 +2266,7 @@ fn remove_cidr_hdlr( #[no_mangle] fn list_ports_hdlr() -> Result { let mut resp = ListPortsResp { ports: vec![] }; - let devs = unsafe { xde_devs.read() }; + let devs = unsafe { xde_devs.as_ref().unwrap().read() }; for dev in devs.iter() { let ipv4_state = dev.vpc_cfg.ipv4_cfg().map(|cfg| cfg.external_ips.load()); From de47912e302c010559e1c0b9ebed68d304032e0f Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 28 Jun 2024 10:09:28 +0100 Subject: [PATCH 04/16] Update xtask. --- xtask/src/main.rs | 39 ++++++++------------------------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/xtask/src/main.rs b/xtask/src/main.rs index e5de6409..46fca60f 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -20,7 +20,7 @@ fn cargo_meta() -> &'static Metadata { .get_or_init(|| cargo_metadata::MetadataCommand::new().exec().unwrap()) } -const KMOD_TARGET: &str = "x86_64-unknown-unknown"; +const KMOD_TARGET: &str = "x86_64-illumos"; const LINK_TARGET: &str = "i686-unknown-illumos"; /// Development functions for OPTE. @@ -380,41 +380,18 @@ impl BuildTarget { let meta = cargo_meta(); build_cargo_bin(&[], !debug, Some("xde"), false)?; - let (folder, out_name) = if debug { - ("debug", "xde.dbg") - } else { - ("release", "xde") - }; - let target_dir = meta - .target_directory - .join(format!("{KMOD_TARGET}/{folder}")); - - println!("Linking xde kmod..."); - Command::new("ld") - .args([ - "-ztype=kmod", - "-Ndrv/dld", - "-Ndrv/ip", - "-Nmisc/dls", - "-Nmisc/mac", - "-z", - "allextract", - &format!("{target_dir}/xde.a"), - "-o", - &format!("{target_dir}/{out_name}"), - ]) - .output_nocapture() - .context("failed to link XDE kernel module")?; + let folder = if debug { "debug" } else { "release" }; println!("Building xde dev link helper ({profile})."); build_cargo_bin(&[], !debug, Some("xde/xde-link"), false)?; // verify no panicking in the devfsadm plugin - let nm_output = Command::new("nm") - .arg(meta.target_directory.join(format!( - "i686-unknown-illumos/{folder}/libxde_link.so" - ))) - .output()?; + let nm_output = + Command::new("nm") + .arg(meta.target_directory.join(format!( + "{LINK_TARGET}/{folder}/libxde_link.so" + ))) + .output()?; if nm_output.status.success() { if std::str::from_utf8(&nm_output.stdout)? From 1f60ca653ab965d10e6d4bcbbe110fa710ad9ce5 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 28 Jun 2024 10:24:08 +0100 Subject: [PATCH 05/16] Temporarily private repo. --- .github/buildomat/jobs/bench.sh | 3 +++ .github/buildomat/jobs/opte-api.sh | 3 +++ .github/buildomat/jobs/opte-ioctl.sh | 3 +++ .github/buildomat/jobs/opte.sh | 3 +++ .github/buildomat/jobs/opteadm.sh | 3 +++ .github/buildomat/jobs/oxide-vpc.sh | 3 +++ .github/buildomat/jobs/p5p.sh | 3 +++ .github/buildomat/jobs/test.sh | 3 +++ .github/buildomat/jobs/xde.sh | 3 +++ 9 files changed, 27 insertions(+) diff --git a/.github/buildomat/jobs/bench.sh b/.github/buildomat/jobs/bench.sh index 458c8bd0..b9d35c87 100644 --- a/.github/buildomat/jobs/bench.sh +++ b/.github/buildomat/jobs/bench.sh @@ -7,6 +7,9 @@ #: output_rules = [ #: "=/work/bench-results.tgz", #: ] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: #: [[publish]] #: series = "benchmark" diff --git a/.github/buildomat/jobs/opte-api.sh b/.github/buildomat/jobs/opte-api.sh index eb4d0a7b..c9f1625f 100755 --- a/.github/buildomat/jobs/opte-api.sh +++ b/.github/buildomat/jobs/opte-api.sh @@ -5,6 +5,9 @@ #: target = "helios-2.0" #: rust_toolchain = "nightly-2024-05-12" #: output_rules = [] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: set -o errexit diff --git a/.github/buildomat/jobs/opte-ioctl.sh b/.github/buildomat/jobs/opte-ioctl.sh index fdc61df0..238d627c 100755 --- a/.github/buildomat/jobs/opte-ioctl.sh +++ b/.github/buildomat/jobs/opte-ioctl.sh @@ -5,6 +5,9 @@ #: target = "helios-2.0" #: rust_toolchain = "nightly-2024-05-12" #: output_rules = [] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: set -o errexit diff --git a/.github/buildomat/jobs/opte.sh b/.github/buildomat/jobs/opte.sh index a04d14a5..be792e3b 100755 --- a/.github/buildomat/jobs/opte.sh +++ b/.github/buildomat/jobs/opte.sh @@ -5,6 +5,9 @@ #: target = "helios-2.0" #: rust_toolchain = "nightly-2024-05-12" #: output_rules = [] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: set -o errexit diff --git a/.github/buildomat/jobs/opteadm.sh b/.github/buildomat/jobs/opteadm.sh index 56133dde..bc6bb983 100755 --- a/.github/buildomat/jobs/opteadm.sh +++ b/.github/buildomat/jobs/opteadm.sh @@ -10,6 +10,9 @@ #: "=/work/release/opteadm", #: "=/work/release/opteadm.release.sha256", #: ] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: set -o errexit diff --git a/.github/buildomat/jobs/oxide-vpc.sh b/.github/buildomat/jobs/oxide-vpc.sh index 65e97ab9..802a0383 100755 --- a/.github/buildomat/jobs/oxide-vpc.sh +++ b/.github/buildomat/jobs/oxide-vpc.sh @@ -5,6 +5,9 @@ #: target = "helios-2.0" #: rust_toolchain = "nightly-2024-05-12" #: output_rules = [] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: set -o errexit diff --git a/.github/buildomat/jobs/p5p.sh b/.github/buildomat/jobs/p5p.sh index 1d51caff..9d3d78e0 100755 --- a/.github/buildomat/jobs/p5p.sh +++ b/.github/buildomat/jobs/p5p.sh @@ -8,6 +8,9 @@ #: "=/out/opte.p5p", #: "=/out/opte.p5p.sha256", #: ] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: #: [[publish]] #: series = "repo" diff --git a/.github/buildomat/jobs/test.sh b/.github/buildomat/jobs/test.sh index 00262f91..e276c547 100755 --- a/.github/buildomat/jobs/test.sh +++ b/.github/buildomat/jobs/test.sh @@ -7,6 +7,9 @@ #: output_rules = [ #: "/work/*.log", #: ] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: #: [dependencies.xde] #: job = "opte-xde" diff --git a/.github/buildomat/jobs/xde.sh b/.github/buildomat/jobs/xde.sh index af1aa68b..aea04481 100755 --- a/.github/buildomat/jobs/xde.sh +++ b/.github/buildomat/jobs/xde.sh @@ -16,6 +16,9 @@ #: "=/work/test/loopback", #: "=/work/xde.conf", #: ] +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] #: #: [[publish]] #: series = "module" From ff23b611de7aed3fccbae79d713f77c5036cc2c4 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 28 Jun 2024 10:32:39 +0100 Subject: [PATCH 06/16] Let's try that again. --- Cargo.lock | 6 +++--- Cargo.toml | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a49370ed..1c214afc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -152,7 +152,7 @@ dependencies = [ [[package]] name = "bindings" version = "0.1.0" -source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" dependencies = [ "anyhow", "bindgen", @@ -936,7 +936,7 @@ checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" [[package]] name = "illumos" version = "0.1.0" -source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" dependencies = [ "bindings", "bitflags 2.5.0", @@ -1152,7 +1152,7 @@ checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" [[package]] name = "macros" version = "0.1.0" -source = "git+ssh://git@github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" dependencies = [ "bindings", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 10e148ab..61154a63 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,7 +50,8 @@ ctor = "0.2" darling = "0.20" dyn-clone = "1.0" heapless = "0.8" -illumos = { git = "ssh://git@github.com/oxidecomputer/illumos-rs.git", branch = "felixmcfelix/rusty-for-opte" } +illumos = { git = "https://github.com/oxidecomputer/illumos-rs.git", branch = "felixmcfelix/rusty-for-opte" } +# illumos = { git = "ssh://git@github.com/oxidecomputer/illumos-rs.git", branch = "felixmcfelix/rusty-for-opte" } ipnetwork = { version = "0.20", default-features = false } itertools = { version = "0.12", default-features = false } libc = "0.2" From 9fa0380ac88b4394f5a60ec06be713e1c5c8fc30 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 12:06:44 +0100 Subject: [PATCH 07/16] Attempt to install clang to satisfy bindgen... --- .github/buildomat/jobs/opte.sh | 2 ++ .github/buildomat/jobs/oxide-vpc.sh | 2 ++ .github/buildomat/jobs/p5p.sh | 2 ++ .github/buildomat/jobs/xde.sh | 1 + 4 files changed, 7 insertions(+) diff --git a/.github/buildomat/jobs/opte.sh b/.github/buildomat/jobs/opte.sh index be792e3b..3fe3d9fc 100755 --- a/.github/buildomat/jobs/opte.sh +++ b/.github/buildomat/jobs/opte.sh @@ -18,6 +18,8 @@ function header { echo "# ==== $* ==== #" } +pfexec pkg install clang-15 + cargo --version rustc --version diff --git a/.github/buildomat/jobs/oxide-vpc.sh b/.github/buildomat/jobs/oxide-vpc.sh index 802a0383..0d65a653 100755 --- a/.github/buildomat/jobs/oxide-vpc.sh +++ b/.github/buildomat/jobs/oxide-vpc.sh @@ -18,6 +18,8 @@ function header { echo "# ==== $* ==== #" } +pfexec pkg install clang-15 + cargo --version rustc --version diff --git a/.github/buildomat/jobs/p5p.sh b/.github/buildomat/jobs/p5p.sh index 9d3d78e0..99cf0f7d 100755 --- a/.github/buildomat/jobs/p5p.sh +++ b/.github/buildomat/jobs/p5p.sh @@ -27,6 +27,8 @@ set -o errexit set -o pipefail set -o xtrace +pfexec pkg install clang-15 + # # TGT_BASE allows one to run this more easily in their local # environment: diff --git a/.github/buildomat/jobs/xde.sh b/.github/buildomat/jobs/xde.sh index aea04481..7c7d27c9 100755 --- a/.github/buildomat/jobs/xde.sh +++ b/.github/buildomat/jobs/xde.sh @@ -72,6 +72,7 @@ cargo --version rustc --version install_pkg jq +install_pkg clang-15 pushd xde From 00b27b6bdadd34f9ffa0f7063797275e4d5b7a55 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 12:22:39 +0100 Subject: [PATCH 08/16] Iterating on CI. --- .github/buildomat/jobs/p5p.sh | 5 ++++- .github/buildomat/jobs/xde.sh | 13 +++++-------- xde/build-debug.sh | 17 ----------------- xde/build.sh | 30 ------------------------------ 4 files changed, 9 insertions(+), 56 deletions(-) delete mode 100755 xde/build-debug.sh delete mode 100755 xde/build.sh diff --git a/.github/buildomat/jobs/p5p.sh b/.github/buildomat/jobs/p5p.sh index 99cf0f7d..a31eb3d1 100755 --- a/.github/buildomat/jobs/p5p.sh +++ b/.github/buildomat/jobs/p5p.sh @@ -37,7 +37,7 @@ pfexec pkg install clang-15 # TGT_BASE=${TGT_BASE:=/work} -REL_SRC=target/x86_64-unknown-unknown/release +REL_SRC=target/x86_64-illumos/release REL_TGT=$TGT_BASE/release mkdir -p $REL_TGT @@ -49,9 +49,12 @@ function header { cargo --version rustc --version +# TODO: shouldn't these be copied as a dependency/output of xde.sh? header "build xde and opteadm (release+debug)" ptime -m cargo xtask build +# TODO: doesn't this dupe xde.sh? + # # Inspect the kernel module for bad relocations in case the old # codegen issue ever shows its face again. diff --git a/.github/buildomat/jobs/xde.sh b/.github/buildomat/jobs/xde.sh index 7c7d27c9..46050e6d 100755 --- a/.github/buildomat/jobs/xde.sh +++ b/.github/buildomat/jobs/xde.sh @@ -42,11 +42,11 @@ set -o xtrace # TGT_BASE=${TGT_BASE:=/work} -DBG_SRC=target/x86_64-unknown-unknown/debug +DBG_SRC=target/x86_64-illumos/debug DBG_LINK_SRC=target/i686-unknown-illumos/debug DBG_TGT=$TGT_BASE/debug -REL_SRC=target/x86_64-unknown-unknown/release +REL_SRC=target/x86_64-illumos/release REL_LINK_SRC=target/i686-unknown-illumos/release REL_TGT=$TGT_BASE/release @@ -90,14 +90,11 @@ ptime -m cargo clippy -- \ --allow clippy::uninlined-format-args --allow clippy::bad_bit_mask popd -header "build xde (debug)" -ptime -m ./build-debug.sh - -header "build xde (release)" -ptime -m ./build.sh - popd +header "build xde (release/debug)" +ptime cargo xtask build + # # Inspect the kernel module for bad relocations in case the old # codegen issue ever shows its face again. diff --git a/xde/build-debug.sh b/xde/build-debug.sh deleted file mode 100755 index e6ed4f7f..00000000 --- a/xde/build-debug.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/bash - -DBG_DIR=../target/x86_64-unknown-unknown/debug/ - -cargo -v build - -ld -ztype=kmod \ - -N"drv/dld" \ - -N"drv/ip" \ - -N"misc/dls" \ - -N"misc/mac" \ - -z allextract $DBG_DIR/xde.a \ - -o $DBG_DIR/xde.dbg - -# Also build devfsadm plugin -pushd xde-link -cargo -v build \ No newline at end of file diff --git a/xde/build.sh b/xde/build.sh deleted file mode 100755 index b6a2b8a5..00000000 --- a/xde/build.sh +++ /dev/null @@ -1,30 +0,0 @@ -#!/bin/bash - -set -xe - -REL_DIR=../target/x86_64-unknown-unknown/release/ - -cargo -v build --release - -ld -ztype=kmod \ - -N"drv/dld" \ - -N"drv/ip" \ - -N"misc/dls" \ - -N"misc/mac" \ - -z allextract $REL_DIR/xde.a \ - -o $REL_DIR/xde - -# Also build devfsadm plugin -pushd xde-link -cargo -v build --release - - -# We don't want to panic in the devfsadm plugin but enforcing that -# is a bit tricky. For now, just manually verify w/ nm: -set +e -nm ../../target/i686-unknown-illumos/release/libxde_link.so | grep panicking -if [ $? -eq 0 ]; then - echo "ERROR: devfsadm plugin may panic!" - exit 1 -fi -popd From c02b84b6a150ce776d781fa35fa42986ddb10ecd Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 12:51:29 +0100 Subject: [PATCH 09/16] Hopefully fixup packaging? --- .github/buildomat/jobs/p5p.sh | 36 +++++++++++++++++------------------ pkg/build.sh | 4 ++-- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/.github/buildomat/jobs/p5p.sh b/.github/buildomat/jobs/p5p.sh index a31eb3d1..50b99650 100755 --- a/.github/buildomat/jobs/p5p.sh +++ b/.github/buildomat/jobs/p5p.sh @@ -22,13 +22,13 @@ #: name = "opte.p5p.sha256" #: from_output = "/out/opte.p5p.sha256" #: +#: [dependencies.xde] +#: job = "opte-xde" set -o errexit set -o pipefail set -o xtrace -pfexec pkg install clang-15 - # # TGT_BASE allows one to run this more easily in their local # environment: @@ -37,11 +37,6 @@ pfexec pkg install clang-15 # TGT_BASE=${TGT_BASE:=/work} -REL_SRC=target/x86_64-illumos/release -REL_TGT=$TGT_BASE/release - -mkdir -p $REL_TGT - function header { echo "# ==== $* ==== #" } @@ -49,20 +44,23 @@ function header { cargo --version rustc --version -# TODO: shouldn't these be copied as a dependency/output of xde.sh? -header "build xde and opteadm (release+debug)" -ptime -m cargo xtask build +header "move artifacts for packaging" +# Copy in just-built artifacts +DBG_SRC=target/x86_64-illumos/debug +DBG_LINK_SRC=target/i686-unknown-illumos/debug -# TODO: doesn't this dupe xde.sh? +REL_SRC=target/x86_64-illumos/release +REL_LINK_SRC=target/i686-unknown-illumos/release -# -# Inspect the kernel module for bad relocations in case the old -# codegen issue ever shows its face again. -# -if elfdump $REL_SRC/xde | grep GOTPCREL; then - echo "found GOTPCREL relocation in release build" - exit 1 -fi +mkdir -p $REL_SRC +cp /input/xde/work/release/xde $REL_SRC +mkdir -p $DBG_SRC +cp /input/xde/work/debug/xde $DBG_SRC + +mkdir -p $REL_LINK_SRC +cp /input/xde/work/release/xde_link.so $REL_LINK_SRC/libxde_link.so +mkdir -p $DBG_LINK_SRC +cp /input/xde/work/debug/xde_link.dbg.so $DBG_LINK_SRC/libxde_link.so header "package opte" cargo xtask package --skip-build diff --git a/pkg/build.sh b/pkg/build.sh index d780d963..0d14d358 100755 --- a/pkg/build.sh +++ b/pkg/build.sh @@ -14,13 +14,13 @@ mkdir -p proto/kernel/drv/amd64 mkdir -p proto/opt/oxide/opte/bin mkdir -p proto/usr/lib/devfsadm/linkmod cp ../target/release/opteadm proto/opt/oxide/opte/bin/ -cp ../target/x86_64-unknown-unknown/release/xde proto/kernel/drv/amd64 +cp ../target/x86_64-illumos/release/xde proto/kernel/drv/amd64 cp ../xde/xde.conf proto/kernel/drv/ cp ../target/i686-unknown-illumos/release/libxde_link.so proto/usr/lib/devfsadm/linkmod/SUNW_xde_link.so if [ -z ${RELEASE_ONLY+x} ]; then cp ../target/debug/opteadm proto/opt/oxide/opte/bin/opteadm.dbg - cp ../target/x86_64-unknown-unknown/debug/xde.dbg proto/kernel/drv/amd64/xde.dbg + cp ../target/x86_64-illumos/debug/xde.dbg proto/kernel/drv/amd64/xde.dbg INC_DEBUG="" else INC_DEBUG="#" From fedb88aa017952ccd4a4de29fb4c05ed4e98119f Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 12:55:43 +0100 Subject: [PATCH 10/16] ... --- .github/buildomat/jobs/p5p.sh | 2 +- .github/buildomat/jobs/xde.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/buildomat/jobs/p5p.sh b/.github/buildomat/jobs/p5p.sh index 50b99650..848562b8 100755 --- a/.github/buildomat/jobs/p5p.sh +++ b/.github/buildomat/jobs/p5p.sh @@ -55,7 +55,7 @@ REL_LINK_SRC=target/i686-unknown-illumos/release mkdir -p $REL_SRC cp /input/xde/work/release/xde $REL_SRC mkdir -p $DBG_SRC -cp /input/xde/work/debug/xde $DBG_SRC +cp /input/xde/work/debug/xde.dbg $DBG_SRC/xde mkdir -p $REL_LINK_SRC cp /input/xde/work/release/xde_link.so $REL_LINK_SRC/libxde_link.so diff --git a/.github/buildomat/jobs/xde.sh b/.github/buildomat/jobs/xde.sh index 46050e6d..f6a7646b 100755 --- a/.github/buildomat/jobs/xde.sh +++ b/.github/buildomat/jobs/xde.sh @@ -99,7 +99,7 @@ ptime cargo xtask build # Inspect the kernel module for bad relocations in case the old # codegen issue ever shows its face again. # -if elfdump $DBG_SRC/xde.dbg | grep GOTPCREL; then +if elfdump $DBG_SRC/xde | grep GOTPCREL; then echo "found GOTPCREL relocation in debug build" exit 1 fi @@ -109,7 +109,7 @@ if elfdump $REL_SRC/xde | grep GOTPCREL; then exit 1 fi -cp $DBG_SRC/xde.dbg $DBG_TGT/ +cp $DBG_SRC/xde $DBG_TGT/xde.dbg sha256sum $DBG_TGT/xde.dbg > $DBG_TGT/xde.dbg.sha256 cp $DBG_LINK_SRC/libxde_link.so $DBG_TGT/xde_link.dbg.so From bccfd773d7acc5296472f06cd8f4ae869cc0b9f9 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 13:57:40 +0100 Subject: [PATCH 11/16] Is this it? --- .github/buildomat/jobs/p5p.sh | 10 ++++++++++ pkg/build.sh | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/buildomat/jobs/p5p.sh b/.github/buildomat/jobs/p5p.sh index 848562b8..561517e9 100755 --- a/.github/buildomat/jobs/p5p.sh +++ b/.github/buildomat/jobs/p5p.sh @@ -24,6 +24,9 @@ #: #: [dependencies.xde] #: job = "opte-xde" +#; +#: [dependencies.opteadm] +#: job = "opteadm" set -o errexit set -o pipefail @@ -48,9 +51,11 @@ header "move artifacts for packaging" # Copy in just-built artifacts DBG_SRC=target/x86_64-illumos/debug DBG_LINK_SRC=target/i686-unknown-illumos/debug +DBG_ADM_SRC=target/debug REL_SRC=target/x86_64-illumos/release REL_LINK_SRC=target/i686-unknown-illumos/release +REL_ADM_SRC=target/release mkdir -p $REL_SRC cp /input/xde/work/release/xde $REL_SRC @@ -62,6 +67,11 @@ cp /input/xde/work/release/xde_link.so $REL_LINK_SRC/libxde_link.so mkdir -p $DBG_LINK_SRC cp /input/xde/work/debug/xde_link.dbg.so $DBG_LINK_SRC/libxde_link.so +mkdir -p $REL_ADM_SRC +cp /input/opteadm/work/release/opteadm $REL_ADM_SRC +mkdir -p $DBG_ADM_SRC +cp /input/opteadm/work/debug/opteadm $DBG_ADM_SRC + header "package opte" cargo xtask package --skip-build diff --git a/pkg/build.sh b/pkg/build.sh index 0d14d358..d970c336 100755 --- a/pkg/build.sh +++ b/pkg/build.sh @@ -20,7 +20,7 @@ cp ../target/i686-unknown-illumos/release/libxde_link.so proto/usr/lib/devfsadm/ if [ -z ${RELEASE_ONLY+x} ]; then cp ../target/debug/opteadm proto/opt/oxide/opte/bin/opteadm.dbg - cp ../target/x86_64-illumos/debug/xde.dbg proto/kernel/drv/amd64/xde.dbg + cp ../target/x86_64-illumos/debug/xde proto/kernel/drv/amd64/xde.dbg INC_DEBUG="" else INC_DEBUG="#" From 32d420fa937fc122a7566e9da6a5f79156334a02 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 14:46:11 +0100 Subject: [PATCH 12/16] Move style checks up to top-level fmt xtask. I'm in the CI weeds anyway, better make something that fails extraordinarily quickly. --- .github/buildomat/jobs/lint.sh | 21 +++++++++++++++++++++ .github/buildomat/jobs/opte-api.sh | 3 --- .github/buildomat/jobs/opte-ioctl.sh | 3 --- .github/buildomat/jobs/opte.sh | 3 --- .github/buildomat/jobs/opteadm.sh | 3 --- .github/buildomat/jobs/oxide-vpc.sh | 3 --- .github/buildomat/jobs/xde.sh | 3 --- xtask/src/main.rs | 21 +++++++++++++++------ 8 files changed, 36 insertions(+), 24 deletions(-) create mode 100644 .github/buildomat/jobs/lint.sh diff --git a/.github/buildomat/jobs/lint.sh b/.github/buildomat/jobs/lint.sh new file mode 100644 index 00000000..7464ccaa --- /dev/null +++ b/.github/buildomat/jobs/lint.sh @@ -0,0 +1,21 @@ +#!/bin/bash +#: +#: name = "lint" +#: variety = "basic" +#: target = "helios-2.0" +#: rust_toolchain = "nightly-2024-05-12" +#: access_repos = [ +#: "oxidecomputer/illumos-rs", +#: ] +#: + +set -o errexit +set -o pipefail +set -o xtrace + +function header { + echo "# ==== $* ==== #" +} + +header "check style" +ptime -m cargo xtask fmt --check diff --git a/.github/buildomat/jobs/opte-api.sh b/.github/buildomat/jobs/opte-api.sh index c9f1625f..25885fd4 100755 --- a/.github/buildomat/jobs/opte-api.sh +++ b/.github/buildomat/jobs/opte-api.sh @@ -26,9 +26,6 @@ cd crates/opte-api header "check API_VERSION" ./check-api-version.sh -header "check style" -ptime -m cargo +nightly-2024-05-12 fmt -- --check - header "analyze std" ptime -m cargo clippy --all-targets diff --git a/.github/buildomat/jobs/opte-ioctl.sh b/.github/buildomat/jobs/opte-ioctl.sh index 238d627c..c90088c3 100755 --- a/.github/buildomat/jobs/opte-ioctl.sh +++ b/.github/buildomat/jobs/opte-ioctl.sh @@ -23,8 +23,5 @@ rustc --version cd lib/opte-ioctl -header "check style" -ptime -m cargo +nightly-2024-05-12 fmt -- --check - header "analyze" ptime -m cargo clippy --all-targets diff --git a/.github/buildomat/jobs/opte.sh b/.github/buildomat/jobs/opte.sh index 3fe3d9fc..803fef4d 100755 --- a/.github/buildomat/jobs/opte.sh +++ b/.github/buildomat/jobs/opte.sh @@ -25,9 +25,6 @@ rustc --version cd lib/opte -header "check style" -ptime -m cargo +nightly-2024-05-12 fmt -- --check - header "check docs" # # I believe this means any doc warnings in deps will cause this to diff --git a/.github/buildomat/jobs/opteadm.sh b/.github/buildomat/jobs/opteadm.sh index bc6bb983..a40a14a9 100755 --- a/.github/buildomat/jobs/opteadm.sh +++ b/.github/buildomat/jobs/opteadm.sh @@ -28,9 +28,6 @@ rustc --version pushd bin/opteadm -header "check style" -ptime -m cargo +nightly-2024-05-12 fmt -- --check - header "analyze" ptime -m cargo clippy --all-targets diff --git a/.github/buildomat/jobs/oxide-vpc.sh b/.github/buildomat/jobs/oxide-vpc.sh index 0d65a653..1a85ad8e 100755 --- a/.github/buildomat/jobs/oxide-vpc.sh +++ b/.github/buildomat/jobs/oxide-vpc.sh @@ -25,9 +25,6 @@ rustc --version cd lib/oxide-vpc -header "check style" -ptime -m cargo +nightly-2024-05-12 fmt -- --check - header "check docs" # # I believe this means any doc warnings in deps will cause this to diff --git a/.github/buildomat/jobs/xde.sh b/.github/buildomat/jobs/xde.sh index f6a7646b..028a6c6b 100755 --- a/.github/buildomat/jobs/xde.sh +++ b/.github/buildomat/jobs/xde.sh @@ -78,9 +78,6 @@ pushd xde cp xde.conf /work/xde.conf -header "check style" -ptime -m cargo +nightly-2024-05-12 fmt -p xde -p xde-link -- --check - header "analyze" ptime -m cargo clippy -- \ --allow clippy::uninlined-format-args --allow clippy::bad_bit_mask diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 46fca60f..d966593c 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -61,7 +61,11 @@ enum Xtask { }, /// Format the repository with `rustfmt`. - Fmt, + Fmt { + /// Run rustfmt in check mode. + #[arg(long)] + check: bool, + }, } #[derive(Debug, Args)] @@ -140,17 +144,22 @@ fn main() -> anyhow::Result<()> { Ok(()) } - Xtask::Fmt => { + Xtask::Fmt { check } => { let meta = cargo_meta(); // This is explicitly `cargo` rather than CARGO as we might // be swapping toolchains to do this from the current cargo. - Command::new("cargo") - .arg(format!("+{}", get_current_nightly_toolchain()?)) + let mut c = Command::new("cargo"); + c.arg(format!("+{}", get_current_nightly_toolchain()?)) .args(["fmt", "--all"]) .env_remove("RUSTUP_TOOLCHAIN") - .current_dir(&meta.workspace_root) - .output_nocapture()?; + .current_dir(&meta.workspace_root); + + if check { + c.arg("--check"); + } + + c.output_nocapture()?; Ok(()) } From b7bec37ce2ca2394cc786c147ef34d97ef6abf7f Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 15:10:19 +0100 Subject: [PATCH 13/16] Toolchain versioning... --- .github/buildomat/jobs/lint.sh | 2 +- .github/buildomat/jobs/opte-api.sh | 2 +- .github/buildomat/jobs/opte-ioctl.sh | 2 +- .github/buildomat/jobs/opte.sh | 6 +++--- .github/buildomat/jobs/opteadm.sh | 2 +- .github/buildomat/jobs/oxide-vpc.sh | 6 +++--- .github/buildomat/jobs/p5p.sh | 2 +- .github/buildomat/jobs/xde.sh | 4 ++-- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/buildomat/jobs/lint.sh b/.github/buildomat/jobs/lint.sh index 7464ccaa..3b50c124 100644 --- a/.github/buildomat/jobs/lint.sh +++ b/.github/buildomat/jobs/lint.sh @@ -3,7 +3,7 @@ #: name = "lint" #: variety = "basic" #: target = "helios-2.0" -#: rust_toolchain = "nightly-2024-05-12" +#: rust_toolchain = "nightly-2024-06-27" #: access_repos = [ #: "oxidecomputer/illumos-rs", #: ] diff --git a/.github/buildomat/jobs/opte-api.sh b/.github/buildomat/jobs/opte-api.sh index 25885fd4..505ad340 100755 --- a/.github/buildomat/jobs/opte-api.sh +++ b/.github/buildomat/jobs/opte-api.sh @@ -3,7 +3,7 @@ #: name = "opte-api" #: variety = "basic" #: target = "helios-2.0" -#: rust_toolchain = "nightly-2024-05-12" +#: rust_toolchain = "nightly-2024-06-27" #: output_rules = [] #: access_repos = [ #: "oxidecomputer/illumos-rs", diff --git a/.github/buildomat/jobs/opte-ioctl.sh b/.github/buildomat/jobs/opte-ioctl.sh index c90088c3..98190941 100755 --- a/.github/buildomat/jobs/opte-ioctl.sh +++ b/.github/buildomat/jobs/opte-ioctl.sh @@ -3,7 +3,7 @@ #: name = "opte-ioctl" #: variety = "basic" #: target = "helios-2.0" -#: rust_toolchain = "nightly-2024-05-12" +#: rust_toolchain = "nightly-2024-06-27" #: output_rules = [] #: access_repos = [ #: "oxidecomputer/illumos-rs", diff --git a/.github/buildomat/jobs/opte.sh b/.github/buildomat/jobs/opte.sh index 803fef4d..f855b293 100755 --- a/.github/buildomat/jobs/opte.sh +++ b/.github/buildomat/jobs/opte.sh @@ -3,7 +3,7 @@ #: name = "opte" #: variety = "basic" #: target = "helios-2.0" -#: rust_toolchain = "nightly-2024-05-12" +#: rust_toolchain = "nightly-2024-06-27" #: output_rules = [] #: access_repos = [ #: "oxidecomputer/illumos-rs", @@ -32,13 +32,13 @@ header "check docs" # # Use nightly which is needed for the `kernel` feature. RUSTDOCFLAGS="-D warnings" ptime -m \ - cargo +nightly-2024-05-12 doc --no-default-features --features=api,std,engine,kernel + cargo +nightly-2024-06-27 doc --no-default-features --features=api,std,engine,kernel header "analyze std + api" ptime -m cargo clippy --all-targets header "analyze no_std + engine + kernel" -ptime -m cargo +nightly-2024-05-12 clippy --no-default-features --features engine,kernel +ptime -m cargo +nightly-2024-06-27 clippy --no-default-features --features engine,kernel header "test" ptime -m cargo test diff --git a/.github/buildomat/jobs/opteadm.sh b/.github/buildomat/jobs/opteadm.sh index a40a14a9..1c28e115 100755 --- a/.github/buildomat/jobs/opteadm.sh +++ b/.github/buildomat/jobs/opteadm.sh @@ -3,7 +3,7 @@ #: name = "opteadm" #: variety = "basic" #: target = "helios-2.0" -#: rust_toolchain = "nightly-2024-05-12" +#: rust_toolchain = "nightly-2024-06-27" #: output_rules = [ #: "=/work/debug/opteadm", #: "=/work/debug/opteadm.debug.sha256", diff --git a/.github/buildomat/jobs/oxide-vpc.sh b/.github/buildomat/jobs/oxide-vpc.sh index 1a85ad8e..87da3073 100755 --- a/.github/buildomat/jobs/oxide-vpc.sh +++ b/.github/buildomat/jobs/oxide-vpc.sh @@ -3,7 +3,7 @@ #: name = "oxide-vpc" #: variety = "basic" #: target = "helios-2.0" -#: rust_toolchain = "nightly-2024-05-12" +#: rust_toolchain = "nightly-2024-06-27" #: output_rules = [] #: access_repos = [ #: "oxidecomputer/illumos-rs", @@ -32,13 +32,13 @@ header "check docs" # # Use nightly which is needed for the `kernel` feature. RUSTDOCFLAGS="-D warnings" ptime -m \ - cargo +nightly-2024-05-12 doc --no-default-features --features=api,std,engine,kernel + cargo +nightly-2024-06-27 doc --no-default-features --features=api,std,engine,kernel header "analyze std + api + usdt" ptime -m cargo clippy --features usdt --all-targets header "analyze no_std + engine + kernel" -ptime -m cargo +nightly-2024-05-12 clippy --no-default-features --features engine,kernel +ptime -m cargo +nightly-2024-06-27 clippy --no-default-features --features engine,kernel header "test" ptime -m cargo test diff --git a/.github/buildomat/jobs/p5p.sh b/.github/buildomat/jobs/p5p.sh index 561517e9..c44da368 100755 --- a/.github/buildomat/jobs/p5p.sh +++ b/.github/buildomat/jobs/p5p.sh @@ -3,7 +3,7 @@ #: name = "opte-p5p" #: variety = "basic" #: target = "helios-2.0" -#: rust_toolchain = "nightly-2024-05-12" +#: rust_toolchain = "nightly-2024-06-27" #: output_rules = [ #: "=/out/opte.p5p", #: "=/out/opte.p5p.sha256", diff --git a/.github/buildomat/jobs/xde.sh b/.github/buildomat/jobs/xde.sh index 028a6c6b..c95900d8 100755 --- a/.github/buildomat/jobs/xde.sh +++ b/.github/buildomat/jobs/xde.sh @@ -3,7 +3,7 @@ #: name = "opte-xde" #: variety = "basic" #: target = "helios-2.0" -#: rust_toolchain = "nightly-2024-05-12" +#: rust_toolchain = "nightly-2024-06-27" #: output_rules = [ #: "=/work/debug/xde.dbg", #: "=/work/debug/xde.dbg.sha256", @@ -121,7 +121,7 @@ sha256sum $REL_TGT/xde_link.so > $REL_TGT/xde_link.so.sha256 header "build xde integration tests" pushd xde-tests -cargo +nightly-2024-05-12 fmt -- --check +cargo +nightly-2024-06-27 fmt -- --check cargo clippy --all-targets cargo build --test loopback loopback_test=$( From ef0bc86a16787c5a17f32d1aaee371ec201974e5 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 15:20:51 +0100 Subject: [PATCH 14/16] Migrate to illumos-rs `note!`/`warn!` --- Cargo.lock | 18 ++++++------- crates/illumos-sys-hdrs/src/kernel.rs | 2 -- lib/opte/src/engine/mod.rs | 33 +++++------------------ lib/opte/src/lib.rs | 21 ++++++++------- xde/src/lib.rs | 39 --------------------------- xde/src/xde.rs | 33 ++++++++++++----------- 6 files changed, 44 insertions(+), 102 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b3c355d2..9133baa7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -132,7 +132,7 @@ version = "0.68.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "726e4313eb6ec35d2730258ad4e15b547ee75d6afaa1361a922e78e59b7d8078" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.6.0", "cexpr", "clang-sys", "lazy_static", @@ -145,14 +145,14 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn 2.0.68", + "syn 2.0.71", "which", ] [[package]] name = "bindings" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#bd985eeca879edccc94171af823a1dac2e6c6a74" dependencies = [ "anyhow", "bindgen", @@ -935,10 +935,10 @@ checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" [[package]] name = "illumos" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#bd985eeca879edccc94171af823a1dac2e6c6a74" dependencies = [ "bindings", - "bitflags 2.5.0", + "bitflags 2.6.0", "macros", ] @@ -1073,7 +1073,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e310b3a6b5907f99202fcdb4960ff45b93735d7c7d96b760fcff8db2dc0e103d" dependencies = [ "cfg-if", - "windows-targets 0.52.5", + "windows-targets 0.48.5", ] [[package]] @@ -1151,7 +1151,7 @@ checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" [[package]] name = "macros" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#735a4daf33a7d5a6368f7df76d88ed4f231ab24a" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#bd985eeca879edccc94171af823a1dac2e6c6a74" dependencies = [ "bindings", "proc-macro2", @@ -1159,7 +1159,7 @@ dependencies = [ "regex-lite", "serde", "serde_tokenstream", - "syn 2.0.68", + "syn 2.0.71", ] [[package]] @@ -1629,7 +1629,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5f12335488a2f3b0a83b14edad48dca9879ce89b2edd10e80237e4e852dd645e" dependencies = [ "proc-macro2", - "syn 2.0.68", + "syn 2.0.71", ] [[package]] diff --git a/crates/illumos-sys-hdrs/src/kernel.rs b/crates/illumos-sys-hdrs/src/kernel.rs index 12d30478..3cfcbf7a 100644 --- a/crates/illumos-sys-hdrs/src/kernel.rs +++ b/crates/illumos-sys-hdrs/src/kernel.rs @@ -421,8 +421,6 @@ extern "C" { pub fn bcopy(src: *const c_void, dst: *mut c_void, count: size_t); - pub fn cmn_err(code: c_int, msg: *const c_char, ...); - pub fn ddi_copyin( buf: *const c_void, driverbuf: *mut c_void, diff --git a/lib/opte/src/engine/mod.rs b/lib/opte/src/engine/mod.rs index 93d7132a..982c8c30 100644 --- a/lib/opte/src/engine/mod.rs +++ b/lib/opte/src/engine/mod.rs @@ -130,23 +130,14 @@ cfg_if! { #[macro_export] macro_rules! dbg_macro { ($s:tt) => { - unsafe { - if ::opte::engine::opte_debug != 0 { - let out_str = format!(concat!($s, "\0")); - // Unwrap safety: we just concat'd a NUL. - let cstr = ::core::ffi::CStr::from_bytes_with_nul(out_str.as_bytes()).unwrap(); - ::illumos_sys_hdrs::cmn_err(::illumos_sys_hdrs::CE_NOTE, cstr.as_ptr()); - } + if unsafe {::opte::engine::opte_debug != 0} { + ::illumos::note!($s); } }; ($s:tt, $($arg:tt)*) => { - unsafe { - if ::opte::engine::opte_debug != 0 { - let out_str = format!(concat!($s, "\0"), $($arg)*); - // Unwrap safety: we just concat'd a NUL. - let cstr = ::core::ffi::CStr::from_bytes_with_nul(out_str.as_bytes()).unwrap(); - ::illumos_sys_hdrs::cmn_err(::illumos_sys_hdrs::CE_NOTE, cstr.as_ptr()); - } + if unsafe{::opte::engine::opte_debug != 0} { + let out_str = format!(concat!($s, "\0"), $($arg)*); + ::illumos::note!($s, $($arg)*); } }; } @@ -154,20 +145,10 @@ cfg_if! { #[macro_export] macro_rules! err_macro { ($s:tt) => { - unsafe { - let out_str = format!(concat!($s, "\0")); - // Unwrap safety: we just concat'd a NUL. - let cstr = ::core::ffi::CStr::from_bytes_with_nul(out_str.as_bytes()).unwrap(); - ::illumos_sys_hdrs::cmn_err(::illumos_sys_hdrs::CE_WARN, cstr.as_ptr()); - } + ::illumos::warn_!($s); }; ($s:tt, $($arg:tt)*) => { - unsafe { - let out_str = format!(concat!($s, "\0"), $($arg)*); - // Unwrap safety: we just concat'd a NUL. - let cstr = ::core::ffi::CStr::from_bytes_with_nul(out_str.as_bytes()).unwrap(); - ::illumos_sys_hdrs::cmn_err(::illumos_sys_hdrs::CE_WARN, cstr.as_ptr()); - } + ::illumos::warn_!($s, $($arg)*); }; } } diff --git a/lib/opte/src/lib.rs b/lib/opte/src/lib.rs index 970e02ee..75a5dd90 100644 --- a/lib/opte/src/lib.rs +++ b/lib/opte/src/lib.rs @@ -236,16 +236,17 @@ pub struct KernelLog {} #[cfg(all(feature = "kernel", not(feature = "std"), not(test)))] impl LogProvider for KernelLog { fn log(&self, level: LogLevel, msg: &str) { - use illumos_sys_hdrs as ddi; - - let cmn_level = match level { - LogLevel::Note => ddi::CE_NOTE, - LogLevel::Warn => ddi::CE_WARN, - LogLevel::Error => ddi::CE_WARN, - }; - - let msg_arg = alloc::ffi::CString::new(msg).unwrap(); - unsafe { ddi::cmn_err(cmn_level, msg_arg.as_ptr()) } + match level { + LogLevel::Note => { + illumos::note!("{}", msg); + } + LogLevel::Warn => { + illumos::warn_!("{}", msg); + } + LogLevel::Error => { + illumos::warn_!("{}", msg); + } + } } } diff --git a/xde/src/lib.rs b/xde/src/lib.rs index fc558a31..04d96b31 100644 --- a/xde/src/lib.rs +++ b/xde/src/lib.rs @@ -27,18 +27,7 @@ mod ioctl; #[macro_use] extern crate alloc; -use alloc::ffi::CString; -use core::alloc::GlobalAlloc; use core::alloc::Layout; -use core::panic::PanicInfo; -use illumos_sys_hdrs::c_void; -use illumos_sys_hdrs::cmn_err; -use illumos_sys_hdrs::kmem_alloc; -use illumos_sys_hdrs::kmem_free; -use illumos_sys_hdrs::panic; -use illumos_sys_hdrs::size_t; -use illumos_sys_hdrs::CE_WARN; -use illumos_sys_hdrs::KM_SLEEP; pub mod dls; pub mod ip; @@ -67,31 +56,3 @@ fn alloc_error(_: Layout) -> ! { fn _Unwind_Resume() -> ! { panic!("_Unwind_Resume called"); } - -// NOTE: We allow unused_unsafe so these macros can be used freely in -// unsafe and non-unsafe functions. -#[macro_export] -macro_rules! warn { - ($format:expr) => { - let msg = CString::new(format!($format)).unwrap(); - #[allow(unused_unsafe)] - unsafe { cmn_err(CE_WARN, msg.as_ptr()) }; - }; - ($format:expr, $($args:expr),*) => { - let msg = CString::new(format!($format, $($args),*)).unwrap(); - #[allow(unused_unsafe)] - unsafe { cmn_err(CE_WARN, msg.as_ptr()) }; - }; -} - -#[macro_export] -macro_rules! note { - ($format:expr) => { - let msg = CString::new(format!($format)); - cmn_err(CE_NOTE, msg.as_ptr()); - }; - ($format:expr, $($args:expr),*) => { - let msg = CString::new(format!($format, $($args),*)); - cmn_err(CE_NOTE, msg.as_ptr()); - }; -} diff --git a/xde/src/xde.rs b/xde/src/xde.rs index dda6288a..5e3cf731 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -28,7 +28,6 @@ use crate::route::RouteCache; use crate::route::RouteKey; use crate::secpolicy; use crate::sys; -use crate::warn; use alloc::boxed::Box; use alloc::ffi::CString; use alloc::string::String; @@ -41,6 +40,7 @@ use core::ptr; use core::ptr::addr_of; use core::ptr::addr_of_mut; use core::time::Duration; +use illumos::warn_; use illumos_sys_hdrs::*; use opte::api::ClearXdeUnderlayReq; use opte::api::CmdOk; @@ -382,7 +382,7 @@ unsafe extern "C" fn _init() -> c_int { match mod_install(&xde_linkage) { 0 => 0, err => { - warn!("mod_install failed: {}", err); + warn_!("mod_install failed: {}", err); mac::mac_fini_ops(addr_of_mut!(xde_devops)); err } @@ -403,7 +403,7 @@ unsafe extern "C" fn _fini() -> c_int { 0 } err => { - warn!("mod remove failed: {}", err); + warn_!("mod remove failed: {}", err); err } } @@ -440,7 +440,7 @@ unsafe extern "C" fn xde_open( match secpolicy::secpolicy_dl_config(credp) { 0 => {} err => { - warn!("secpolicy_dl_config failed: {err}"); + warn_!("secpolicy_dl_config failed: {err}"); return err; } } @@ -919,7 +919,7 @@ fn delete_xde(req: &DeleteXdeReq) -> Result { match unsafe { dls::dls_devnet_create(xde.mh, xde.linkid, 0) } { 0 => {} err => { - warn!("failed to recreate DLS devnet entry: {}", err); + warn_!("failed to recreate DLS devnet entry: {}", err); } }; return Err(OpteError::System { @@ -1026,7 +1026,7 @@ fn clear_xde_underlay() -> Result { // 2. Remove MAC client handle if Arc::into_inner(u.mch).is_none() { - warn!( + warn_!( "underlay {} has outstanding mac client handle refs", u.name ); @@ -1127,7 +1127,7 @@ unsafe extern "C" fn xde_attach( ) { 0 => {} err => { - warn!("failed to create xde control device: {err}"); + warn_!("failed to create xde control device: {err}"); return DDI_FAILURE; } } @@ -1217,7 +1217,7 @@ unsafe fn driver_prop_exists(dip: *mut dev_info, pname: &str) -> bool { let name = match CString::new(pname) { Ok(s) => s, Err(e) => { - warn!("bad driver prop string name: {}: {:?}", pname, e); + warn_!("bad driver prop string name: {}: {:?}", pname, e); return false; } }; @@ -1240,7 +1240,7 @@ unsafe fn get_driver_prop_bool( let name = match CString::new(pname) { Ok(s) => s, Err(e) => { - warn!("bad driver prop string name: {}: {:?}", pname, e); + warn_!("bad driver prop string name: {}: {:?}", pname, e); return None; } }; @@ -1262,7 +1262,7 @@ unsafe fn get_driver_prop_bool( // between a true value of 1 and the case where the user entered // gibberish. In this case we treat gibberish as true. if ret == 99 { - warn!("driver prop {} not found", pname); + warn_!("driver prop {} not found", pname); return None; } @@ -1277,7 +1277,7 @@ unsafe fn get_driver_prop_string( let name = match CString::new(pname) { Ok(s) => s, Err(e) => { - warn!("bad driver prop string name: {}: {:?}", pname, e); + warn_!("bad driver prop string name: {}: {:?}", pname, e); return None; } }; @@ -1291,16 +1291,17 @@ unsafe fn get_driver_prop_string( &mut value, ); if ret != DDI_PROP_SUCCESS { - warn!("failed to get driver property {}", pname); + warn_!("failed to get driver property {}", pname); return None; } let s = CStr::from_ptr(value); let s = match s.to_str() { Ok(s) => s, Err(e) => { - warn!( + warn_!( "failed to create string from property value for {}: {:?}", - pname, e + pname, + e ); return None; } @@ -1321,7 +1322,7 @@ unsafe extern "C" fn xde_detach( } if xde_devs.as_ref().unwrap().read().len() > 0 { - warn!("failed to detach: outstanding ports"); + warn_!("failed to detach: outstanding ports"); return DDI_FAILURE; } @@ -1335,7 +1336,7 @@ unsafe extern "C" fn xde_detach( let underlay = state_ref.underlay.lock(); if underlay.is_some() { - warn!("failed to detach: underlay is set"); + warn_!("failed to detach: underlay is set"); return DDI_FAILURE; } } From bf8a4f2bf54a09415c825968b7ebf50ae80a6da0 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 23:40:56 +0100 Subject: [PATCH 15/16] Move to illumos-rs Instant API for time --- lib/opte/src/ddi/time.rs | 286 +---------------------- lib/opte/src/engine/flow_table.rs | 33 +-- lib/opte/src/engine/layer.rs | 6 +- lib/opte/src/engine/port.rs | 14 +- lib/oxide-vpc/tests/integration_tests.rs | 8 +- xde/src/route.rs | 20 +- xde/src/xde.rs | 9 +- 7 files changed, 52 insertions(+), 324 deletions(-) diff --git a/lib/opte/src/ddi/time.rs b/lib/opte/src/ddi/time.rs index f552ba73..35409edf 100644 --- a/lib/opte/src/ddi/time.rs +++ b/lib/opte/src/ddi/time.rs @@ -2,295 +2,19 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2022 Oxide Computer Company +// Copyright 2024 Oxide Computer Company //! Moments, periodics, etc. -use core::ops::Add; -use core::time::Duration; - -cfg_if! { - if #[cfg(all(not(feature = "std"), not(test)))] { - use alloc::boxed::Box; - use alloc::ffi::CString; - use illumos_sys_hdrs as ddi; - } else { - use std::time::Instant; - } -} /// The number of milliseconds in a second. pub const MILLIS: u64 = 1_000; /// The number of nanoseconds in a second. pub const NANOS: u64 = 1_000_000_000; /// The conversion from nanoseconds to milliseconds. -pub const NANOS_TO_MILLIS: u64 = 1_000_000; - -/// A moment in time. -#[derive(Clone, Copy, Debug)] -pub struct Moment { - #[cfg(all(not(feature = "std"), not(test)))] - inner: ddi::hrtime_t, - - #[cfg(any(feature = "std", test))] - inner: Instant, -} - -impl Add for Moment { - type Output = Self; - - fn add(self, rhs: Duration) -> Self::Output { - cfg_if! { - if #[cfg(all(not(feature = "std"), not(test)))] { - let new = self.inner + (rhs.as_secs() * NANOS) as i64 + - rhs.subsec_nanos() as i64; - Moment { inner: new } - } else { - let new = self.inner + rhs; - Moment { inner: new } - } - } - } -} - -impl Moment { - /// Compute the delta between `now - self` and return as - /// milliseconds. - /// - /// Saturates to zero if `earlier` is later than `self`. - pub fn delta_as_millis(&self, earlier: Moment) -> u64 { - cfg_if! { - if #[cfg(all(not(feature = "std"), not(test)))] { - (self.inner as u64).saturating_sub(earlier.inner as u64) / NANOS_TO_MILLIS - } else { - let delta = self.inner.saturating_duration_since(earlier.inner); - delta.as_secs() * MILLIS + delta.subsec_millis() as u64 - } - } - } - - pub fn now() -> Self { - cfg_if! { - if #[cfg(all(not(feature = "std"), not(test)))] { - Self { inner: unsafe { ddi::gethrtime() } } - } else { - Self { inner: Instant::now() } - } - } - } - - /// Return the underlying timestamp for debugging purposes - /// if supported on the current platform. - #[allow(dead_code)] - pub(crate) fn raw_millis(&self) -> Option { - cfg_if! { - if #[cfg(all(not(feature = "std"), not(test)))] { - Some(self.inner as u64 / NANOS_TO_MILLIS) - } else { - None - } - } - } -} - -impl Default for Moment { - fn default() -> Self { - Self::now() - } -} - -/// Execute a callback periodically. -/// -/// NOTE: We could provide a user context impl using the timer_create -/// POSIX API, but as there is no reason to actually have a periodic -/// in testing we do not bother. The reason we do not need a periodic -/// in testing is because it would only be testing the periodic impl, -/// and in our case we only care about the kernel context periodic, -/// which means we must be running in kernel context to actually test -/// it. -/// -/// NOTE: A periodic **cannot** implement `Clone` as it represents a -/// unique resource on the system. -#[cfg(all(not(feature = "std"), not(test)))] -#[derive(Debug)] -pub struct Periodic { - // The following three fields are not needed for the - // implementation, but they may prove useful in debugging. - #[allow(dead_code)] - name: CString, - #[allow(dead_code)] - interval: i64, - #[allow(dead_code)] - system_cb: unsafe extern "C" fn(*mut ddi::c_void), - - // The opaque handle returned by ddi_periodic_add(9F); needed to - // later delete the periodic during Drop. - periodic: *const ddi::ddi_periodic, - - // Technically we are lying here. While the periodic is alive both - // the system and this value hold a copy of the raw pointer to the - // context value: so we actually have an aliased (*const) pointer. - // However, the system treats this as an opaque pointer and does - // nothing with it except give the value to the callback. The API - // provided to the user is such that they are considered the sole - // owner of the boxed T, and thus the callback should receive an - // `&mut T`. We provide this safely by holding the following - // property: - // - // While the periodic is alive only the `_periodic_cb` is allowed - // to access the `raw_ctx` pointer. - // - // When the Periodic is dropped we first delete the system's - // periodic via `ddi_periodic_delete(9F)`, returning the sole - // ownership of `raw_ctx` back to `Peridioc`. This allows us to - // safely put the context back in its `Box` and drop it. - raw_ctx: *mut PeriodicCtx, -} - -#[cfg(all(not(feature = "std"), not(test)))] -struct PeriodicCtx { - arg: Box, - cb: fn(&mut T), -} - -#[cfg(all(not(feature = "std"), not(test)))] -impl PeriodicCtx { - pub fn call(&mut self) { - (self.cb)(&mut self.arg); - } -} +pub const NANOS_TO_MILLIS: u64 = NANOS / MILLIS; -/// Periodic callback -/// -/// # Safety -/// We know the arg is non-null because the periodic ctor -/// always builds a PeriodicCtx to pas to this callback. #[cfg(all(not(feature = "std"), not(test)))] -pub unsafe extern "C" fn _periodic_cb(arg: *mut ddi::c_void) { - assert!(!arg.is_null()); - let ctx = &mut *(arg as *mut PeriodicCtx); - ctx.call(); -} - -#[cfg(all(not(feature = "std"), not(test)))] -impl Periodic { - /// Create a new periodic. - /// - /// The `Box` is owned by the periodic itself and the callback - /// is passed an `&mut T`. - pub fn new( - name: CString, - cb: fn(&mut T), - arg: Box, - interval: Interval, - ) -> Self { - let pctx = Box::new(PeriodicCtx:: { arg, cb }); - let raw_ctx = Box::into_raw(pctx); - let interval = interval.as_nanos() as i64; - - let periodic = unsafe { - ddi::ddi_periodic_add( - _periodic_cb::, - raw_ctx as *mut ddi::c_void, - interval, - 0, - ) - }; - - Self { name, interval, system_cb: _periodic_cb::, periodic, raw_ctx } - } -} - -#[cfg(all(not(feature = "std"), not(test)))] -impl Drop for Periodic { - fn drop(&mut self) { - // Safety: We know that `self.periodic` was created via a - // corresponding call to `ddi_periodic_add(9F)`. - unsafe { ddi::ddi_periodic_delete(self.periodic) }; - - // Safety: Now that the system's periodic is deleted we know - // `self.raw_ctx` is the last pointer and we can once again - // own it to allow the memory to be deallocated. - unsafe { - let _ = Box::from_raw(self.raw_ctx); - } - } -} - -// Currently the `ddi_periodic_add(9F)` contract dictates the -// following about the internval value. -// -// > The caller must specify interval as an even, non-zero multiple of -// > 10ms. No other values are supported at this time. The interval -// > specified is a lower bound on the interval between executions of -// > the callback. -// -// The system will implicitly round up the value if it doesn't meet -// this contract. However, we use PerioidicInterval to enforce this -// contract at compile-time so that it's clear to the developer when -// they are using an internval that cannot be met by the system. -const SYSTEM_PERIODIC_RESOLUTION_IN_NANOS: u64 = 10_000_000; - -/// An interval designed specifically for a `Periodic`. -/// -/// Ensures that an interval value is always a multiple of 10ms as -/// dictated by the `ddi_periodic_add(9F)` API this abstraction is -/// built upon. -pub struct Interval(u64); - -impl Interval { - pub const fn as_nanos(&self) -> u64 { - self.0 - } - - pub const fn from_duration(dur: Duration) -> Self { - let secs = dur.as_secs(); - let nanos = dur.subsec_nanos() as u64; - - assert!( - nanos % SYSTEM_PERIODIC_RESOLUTION_IN_NANOS == 0, - "interval is not multiple of 10ms" - ); - - Self((secs * NANOS) + nanos) - } -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - #[should_panic] - fn bad_interval() { - let ms1 = NANOS_TO_MILLIS as u32; - let ms9 = 9 * NANOS_TO_MILLIS as u32; - let ms99 = 99 * NANOS_TO_MILLIS as u32; - let ms101 = 101 * NANOS_TO_MILLIS as u32; - - let _x = Interval::from_duration(Duration::new(1, ms1)); - let _x = Interval::from_duration(Duration::new(1, ms9)); - let _x = Interval::from_duration(Duration::new(1, ms99)); - let _x = Interval::from_duration(Duration::new(1, ms101)); - } - - #[test] - fn good_interval() { - let ms10 = 10 * NANOS_TO_MILLIS as u32; - let ms100 = 100 * NANOS_TO_MILLIS as u32; - let ms200 = 200 * NANOS_TO_MILLIS as u32; - let ms500 = 500 * NANOS_TO_MILLIS as u32; +pub use illumos::time::*; - // We write the nanoseconds out by hand in case there are bugs - // in the conversion constants. - let mut x = Interval::from_duration(Duration::new(0, ms10)); - assert_eq!(x.as_nanos(), 10_000_000); - x = Interval::from_duration(Duration::new(1, ms10)); - assert_eq!(x.as_nanos(), 1_010_000_000); - x = Interval::from_duration(Duration::new(1, ms100)); - assert_eq!(x.as_nanos(), 1_100_000_000); - x = Interval::from_duration(Duration::new(1, ms200)); - assert_eq!(x.as_nanos(), 1_200_000_000); - x = Interval::from_duration(Duration::new(1, ms500)); - assert_eq!(x.as_nanos(), 1_500_000_000); - } -} +#[cfg(any(feature = "std", test))] +pub use std::time::Instant; diff --git a/lib/opte/src/engine/flow_table.rs b/lib/opte/src/engine/flow_table.rs index 4bd53d27..956cdb4b 100644 --- a/lib/opte/src/engine/flow_table.rs +++ b/lib/opte/src/engine/flow_table.rs @@ -10,7 +10,7 @@ //! tables: UFT, LFT, and the TCP Flow Table. use super::packet::InnerFlowId; -use crate::ddi::time::Moment; +use crate::ddi::time::Instant; use crate::ddi::time::MILLIS; use alloc::boxed::Box; use alloc::collections::BTreeMap; @@ -19,6 +19,7 @@ use alloc::string::String; use alloc::vec::Vec; use core::fmt; use core::num::NonZeroU32; +use core::time::Duration; #[cfg(all(not(feature = "std"), not(test)))] use illumos_sys_hdrs::uintptr_t; use opte_api::OpteError; @@ -49,8 +50,8 @@ impl Ttl { } /// Is `last_hit` expired? - pub fn is_expired(&self, last_hit: Moment, now: Moment) -> bool { - now.delta_as_millis(last_hit) >= self.0 + pub fn is_expired(&self, last_hit: Instant, now: Instant) -> bool { + now.duration_since(last_hit) >= Duration::from_millis(self.0) } /// Create a new TTL based on seconds. @@ -63,11 +64,11 @@ impl Ttl { pub trait ExpiryPolicy: fmt::Debug { /// Returns whether the given flow should be removed, given current flow /// state, the time a packet was last received, and the current time. - fn is_expired(&self, entry: &FlowEntry, now: Moment) -> bool; + fn is_expired(&self, entry: &FlowEntry, now: Instant) -> bool; } impl ExpiryPolicy for Ttl { - fn is_expired(&self, entry: &FlowEntry, now: Moment) -> bool { + fn is_expired(&self, entry: &FlowEntry, now: Instant) -> bool { entry.is_expired(now, *self) } } @@ -131,7 +132,7 @@ where self.map.remove(flowid); } - pub fn expire_flows(&mut self, now: Moment, f: F) -> Vec + pub fn expire_flows(&mut self, now: Instant, f: F) -> Vec where F: Fn(&S) -> InnerFlowId, { @@ -221,8 +222,8 @@ fn flow_expired_probe( port: &CString, name: &CString, flowid: &InnerFlowId, - last_hit: Option, - now: Option, + last_hit: Option, + now: Option, ) { cfg_if! { if #[cfg(all(not(feature = "std"), not(test)))] { @@ -231,8 +232,8 @@ fn flow_expired_probe( port.as_ptr() as uintptr_t, name.as_ptr() as uintptr_t, flowid, - last_hit.and_then(|m| m.raw_millis()).unwrap_or_default() as usize, - now.and_then(|m| m.raw_millis()).unwrap_or_default() as usize, + last_hit.map(|m| m.raw_millis()).unwrap_or_default() as usize, + now.map(|m| m.raw_millis()).unwrap_or_default() as usize, ); } } else if #[cfg(feature = "usdt")] { @@ -265,7 +266,7 @@ pub struct FlowEntry { hits: u64, /// This tracks the last time the flow was matched. - last_hit: Moment, + last_hit: Instant, /// Records whether this flow predates a rule change, and /// must rerun rule processing before `state` can be used. @@ -291,7 +292,7 @@ impl FlowEntry { pub fn hit(&mut self) { self.hits += 1; - self.last_hit = Moment::now(); + self.last_hit = Instant::now(); } pub fn is_dirty(&self) -> bool { @@ -302,16 +303,16 @@ impl FlowEntry { self.dirty = false } - pub fn last_hit(&self) -> &Moment { + pub fn last_hit(&self) -> &Instant { &self.last_hit } - fn is_expired(&self, now: Moment, ttl: Ttl) -> bool { + fn is_expired(&self, now: Instant, ttl: Ttl) -> bool { ttl.is_expired(self.last_hit, now) } fn new(state: S) -> Self { - FlowEntry { state, hits: 0, last_hit: Moment::now(), dirty: false } + FlowEntry { state, hits: 0, last_hit: Instant::now(), dirty: false } } } @@ -370,7 +371,7 @@ mod test { FlowTable::new("port", "flow-expired-test", FT_SIZE.unwrap(), None); assert_eq!(ft.num_flows(), 0); ft.add(flowid, ()).unwrap(); - let now = Moment::now(); + let now = Instant::now(); assert_eq!(ft.num_flows(), 1); ft.expire_flows(now, |_| FLOW_ID_DEFAULT); assert_eq!(ft.num_flows(), 1); diff --git a/lib/opte/src/engine/layer.rs b/lib/opte/src/engine/layer.rs index bfdb053c..054c01ea 100644 --- a/lib/opte/src/engine/layer.rs +++ b/lib/opte/src/engine/layer.rs @@ -39,7 +39,7 @@ use crate::ddi::kstat; use crate::ddi::kstat::KStatNamed; use crate::ddi::kstat::KStatProvider; use crate::ddi::kstat::KStatU64; -use crate::ddi::time::Moment; +use crate::ddi::time::Instant; use crate::ExecCtx; use crate::LogLevel; use alloc::ffi::CString; @@ -223,7 +223,7 @@ impl LayerFlowTable { LftDump { ft_in: self.ft_in.dump(), ft_out: self.ft_out.dump() } } - fn expire_flows(&mut self, now: Moment) { + fn expire_flows(&mut self, now: Instant) { // XXX The two sides can have different traffic patterns and // thus one side could be considered expired while the other // is active. You could have one side seeing packets while the @@ -639,7 +639,7 @@ impl Layer { /// Expire all flows whose TTL has been reached based on the /// passed in moment. - pub(crate) fn expire_flows(&mut self, now: Moment) { + pub(crate) fn expire_flows(&mut self, now: Instant) { self.ft.expire_flows(now); self.stats.vals.flows.set(self.ft.num_flows() as u64); } diff --git a/lib/opte/src/engine/port.rs b/lib/opte/src/engine/port.rs index 8759863b..4fe41417 100644 --- a/lib/opte/src/engine/port.rs +++ b/lib/opte/src/engine/port.rs @@ -49,7 +49,7 @@ use crate::ddi::kstat::KStatNamed; use crate::ddi::kstat::KStatProvider; use crate::ddi::kstat::KStatU64; use crate::ddi::sync::KMutex; -use crate::ddi::time::Moment; +use crate::ddi::time::Instant; use crate::engine::flow_table::ExpiryPolicy; use crate::engine::tcp::TcpMeta; use crate::ExecCtx; @@ -1007,14 +1007,14 @@ impl Port { /// /// * [`PortState::Running`] #[cfg(any(feature = "std", test))] - pub fn expire_flows_at(&self, now: Moment) -> Result<()> { + pub fn expire_flows_at(&self, now: Instant) -> Result<()> { self.expire_flows_inner(Some(now)) } #[inline(always)] - fn expire_flows_inner(&self, now: Option) -> Result<()> { + fn expire_flows_inner(&self, now: Option) -> Result<()> { let mut data = self.data.lock(); - let now = now.unwrap_or_else(Moment::now); + let now = now.unwrap_or_else(Instant::now); check_state!(data.state, [PortState::Running])?; for l in &mut data.layers { @@ -1869,7 +1869,7 @@ impl Port { dir: Direction, ufid: &InnerFlowId, epoch: u64, - last_hit: &Moment, + last_hit: &Instant, ) { cfg_if::cfg_if! { if #[cfg(all(not(feature = "std"), not(test)))] { @@ -1879,7 +1879,7 @@ impl Port { self.name_cstr.as_ptr() as uintptr_t, ufid, epoch as uintptr_t, - last_hit.raw_millis().unwrap_or_default() as usize + last_hit.raw_millis() as usize ); } } else if #[cfg(feature = "usdt")] { @@ -2656,7 +2656,7 @@ impl ExpiryPolicy for TcpExpiry { fn is_expired( &self, entry: &FlowEntry, - now: Moment, + now: Instant, ) -> bool { let ttl = match entry.state().tcp_state.tcp_state() { TcpState::TimeWait => self.time_wait_ttl, diff --git a/lib/oxide-vpc/tests/integration_tests.rs b/lib/oxide-vpc/tests/integration_tests.rs index e22f3d94..ded40b54 100644 --- a/lib/oxide-vpc/tests/integration_tests.rs +++ b/lib/oxide-vpc/tests/integration_tests.rs @@ -19,7 +19,7 @@ use common::icmp::*; use common::*; use opte::api::MacAddr; use opte::api::OpteError; -use opte::ddi::time::Moment; +use opte::ddi::time::Instant; use opte::engine::arp::ArpEthIpv4; use opte::engine::arp::ArpEthIpv4Raw; use opte::engine::dhcpv6; @@ -1936,7 +1936,7 @@ fn flow_expiration() { g1.vpc_map.add(g2_cfg.ipv4().private_ip.into(), g2_cfg.phys_addr()); g1.port.start(); set!(g1, "port_state=running"); - let now = Moment::now(); + let now = Instant::now(); // ================================================================ // Run the packet through g1's port in the outbound direction and @@ -3447,7 +3447,7 @@ fn tcp_outbound() { let mut g1 = oxide_net_setup("g1_port", &g1_cfg, None, None); g1.port.start(); set!(g1, "port_state=running"); - // let now = Moment::now(); + // let now = Instant::now(); // Add default route. router::add_entry( @@ -3471,7 +3471,7 @@ fn tcp_outbound() { // - TCP state machine info should be cleaned up after an active close. // TimeWait state has a ~2min lifetime before we flush it -- it should still // be present at UFT expiry: - let now = Moment::now(); + let now = Instant::now(); g1.port .expire_flows_at(now + Duration::new(FLOW_DEF_EXPIRE_SECS + 1, 0)) .unwrap(); diff --git a/xde/src/route.rs b/xde/src/route.rs index b6a93f32..dd99543a 100644 --- a/xde/src/route.rs +++ b/xde/src/route.rs @@ -17,7 +17,7 @@ use core::ptr; use core::time::Duration; use illumos_sys_hdrs::*; use opte::ddi::sync::KRwLock; -use opte::ddi::time::Moment; +use opte::ddi::time::Instant; use opte::engine::ether::EtherAddr; use opte::engine::ip6::Ipv6Addr; @@ -531,7 +531,7 @@ impl RouteCache { /// query, or computes the current route using `next_hop` on miss or /// discovery of a stale entry. pub fn next_hop<'b>(&self, key: RouteKey, xde: &'b XdeDev) -> Route<'b> { - let t = Moment::now(); + let t = Instant::now(); let (maybe_route, map_ptr_int) = { let route_cache = self.0.read(); @@ -605,7 +605,7 @@ impl RouteCache { pub fn remove_routes(&self) { let mut route_cache = self.0.write(); - let t = Moment::now(); + let t = Instant::now(); let ptr: *const BTreeMap<_, _> = &*route_cache; let map_ptr_int = ptr as uintptr_t; @@ -633,18 +633,16 @@ pub struct CachedRoute { pub src: EtherAddr, pub dst: EtherAddr, pub underlay_idx: u8, - pub timestamp: Moment, + pub timestamp: Instant, } impl CachedRoute { - fn is_valid(&self, t: Moment) -> bool { - u128::from(t.delta_as_millis(self.timestamp)) - <= EXPIRE_ROUTE_LIFETIME.as_millis() + fn is_valid(&self, t: Instant) -> bool { + t.duration_since(self.timestamp) <= EXPIRE_ROUTE_LIFETIME } - fn is_retained(&self, t: Moment) -> bool { - u128::from(t.delta_as_millis(self.timestamp)) - <= REMOVE_ROUTE_LIFETIME.as_millis() + fn is_retained(&self, t: Instant) -> bool { + t.duration_since(self.timestamp) <= REMOVE_ROUTE_LIFETIME } fn into_route(self, xde: &XdeDev) -> Route<'_> { @@ -671,7 +669,7 @@ pub struct Route<'a> { } impl<'a> Route<'a> { - fn cached(&self, xde: &XdeDev, timestamp: Moment) -> CachedRoute { + fn cached(&self, xde: &XdeDev, timestamp: Instant) -> CachedRoute { // As unfortunate as `into_route`. let port_0: &xde_underlay_port = &xde.u1; let underlay_idx = diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 5e3cf731..b2ebb57f 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -696,7 +696,12 @@ unsafe extern "C" fn xde_ioc_opte_cmd(karg: *mut c_void, mode: c_int) -> c_int { } } -const ONE_SECOND: Interval = Interval::from_duration(Duration::new(1, 0)); +const ONE_SECOND: Interval = + if let Some(v) = Interval::from_duration(Duration::new(1, 0)) { + v + } else { + panic!("chosen duration must be 10ms-aligned") + }; #[no_mangle] fn expire_periodic(port: &mut Arc>) { @@ -1127,7 +1132,7 @@ unsafe extern "C" fn xde_attach( ) { 0 => {} err => { - warn_!("failed to create xde control device: {err}"); + warn_!("failed to create xde control device: {}", err); return DDI_FAILURE; } } From 6b6b3bb1a0c45bf798a908ef4714b0bc69ba2505 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 19 Jul 2024 23:57:00 +0100 Subject: [PATCH 16/16] Newer illumos-rs. --- Cargo.lock | 8 ++++---- Cargo.toml | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9133baa7..c085a130 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -152,7 +152,7 @@ dependencies = [ [[package]] name = "bindings" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#bd985eeca879edccc94171af823a1dac2e6c6a74" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#7e45ccadc25809f4f009d9466d77442617041c6d" dependencies = [ "anyhow", "bindgen", @@ -935,7 +935,7 @@ checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" [[package]] name = "illumos" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#bd985eeca879edccc94171af823a1dac2e6c6a74" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#7e45ccadc25809f4f009d9466d77442617041c6d" dependencies = [ "bindings", "bitflags 2.6.0", @@ -1073,7 +1073,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e310b3a6b5907f99202fcdb4960ff45b93735d7c7d96b760fcff8db2dc0e103d" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] @@ -1151,7 +1151,7 @@ checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" [[package]] name = "macros" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#bd985eeca879edccc94171af823a1dac2e6c6a74" +source = "git+https://github.com/oxidecomputer/illumos-rs.git?branch=felixmcfelix/rusty-for-opte#7e45ccadc25809f4f009d9466d77442617041c6d" dependencies = [ "bindings", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 61154a63..0b695b30 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,7 @@ dyn-clone = "1.0" heapless = "0.8" illumos = { git = "https://github.com/oxidecomputer/illumos-rs.git", branch = "felixmcfelix/rusty-for-opte" } # illumos = { git = "ssh://git@github.com/oxidecomputer/illumos-rs.git", branch = "felixmcfelix/rusty-for-opte" } +# illumos = { path = "../illumos-rs/illumos" } ipnetwork = { version = "0.20", default-features = false } itertools = { version = "0.12", default-features = false } libc = "0.2"