opteadm/OpteHdl cleanup
#735
Conversation
There was a problem hiding this comment.
Fell into a bit of a rabbithole around build-features while cleaning this up. I've opened #755 as a result of staring at one lockfile too many.
| pub struct InnerFlowId { | ||
| // Using a `u8` here for `proto` hides the enum repr from SDTs. | ||
| pub proto: u8, | ||
| // We could also theoretically get to a 38B packing if we reduce | ||
| // AddrPair's repr from `u16` to `u8`. However, on the dtrace/illumos | ||
| // side `union addrs` is 4B aligned -- in6_addr_t has a 4B alignment. | ||
| // So, this layout has to match that constraint -- placing addrs at | ||
| // offset 0x2 with `u16` discriminant sets up 4B alignment for the | ||
| // enum variant data (and this struct itself is 4B aligned). | ||
| pub addrs: AddrPair, | ||
| pub src_port: u16, | ||
| pub dst_port: u16, | ||
| } | ||
|
|
||
| impl InnerFlowId { | ||
| #[cfg(feature = "engine")] | ||
| pub fn crc32(&self) -> u32 { | ||
| let mut hasher = Hasher::new(); | ||
| self.hash(&mut hasher); | ||
| hasher.finalize() | ||
| } | ||
| } |
There was a problem hiding this comment.
The main reason this type lives here and not in opte_api is so that we can have this crc32 method implemented directly on the flow ID, while keeping crc32fast limited to engine. Adding a conditional import to opte_api itself felt... bad.
In the fullness of time (#754), we probably want our base FlowId trait, at which point opte::engine can add its own FlowIdExt: FlowId trait with methods like this.
| build_cargo_bin( | ||
| &["--bin", "opteadm"], | ||
| p_name, | ||
| Some("bin/opteadm"), | ||
| true, | ||
| ) |
There was a problem hiding this comment.
Building from the workspace root appeared to be pulling in more dependencies via feature unification.
The cost is minimal (~1s extra) for a further binary reduction to ~21.3MiB. Full-fat LTO still costs us an extra 18s (baseline on my machine w/o LTO is 30s).
|
Moved back to draft -- looks like there's a CI issue between older scripts pointing at |
Should close #755, practically speaking.
This PR moves all base ioctl operations into
opte_ioctl::OpteHdl, allowing us to remove the vast majority of code in theOpteAdmhandle.In addition, I've done some extra reorganisation and dependency grooming s.t.
opteadmno longer compiles anyenginefeatures/code from OPTE, which gets the release binary size from 46.2 -> 31.7 MiB. Thin LTO takes that down to 21.3 MiB. Opteadm compile time drops from ~40s onmasterto ~30s here 🎉.One of the blockers here was that several ioctls relied on serialising/deserialising types which were only exposed as part of the
enginefeature flag. Accordingly, a few type definitions have been moved intoopte_api(RuleId,TcpState) andopte::api(InnerFlowId). I've tried to be cognisant of the longer term goal of having primitives likeInnerFlowIdbe chosen by theNetworkImplby parametrising theDumpioctls.Closes #529.
Closes #755.