Skip to content

Commit

Permalink
Correctly type all InnerFlowId SDT args
Browse files Browse the repository at this point in the history
`uintptr_t` maybe be easy to stick in a probe, but it is also a footgun.
This fixdes the display for e.g. port-process-return, but does not
affect the actual panics mentioned in #476.
  • Loading branch information
FelixMcFelix committed Mar 14, 2024
1 parent 4de9c4a commit cb20ed2
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 31 deletions.
2 changes: 2 additions & 0 deletions crates/opte-api/src/ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ impl FromStr for IpAddr {
PartialOrd,
Serialize,
)]
#[repr(C)]
pub struct Ipv4Addr {
inner: [u8; 4],
}
Expand Down Expand Up @@ -554,6 +555,7 @@ impl Deref for Ipv4Addr {
Serialize,
Deserialize,
)]
#[repr(C)]
pub struct Ipv6Addr {
inner: [u8; 16],
}
Expand Down
6 changes: 3 additions & 3 deletions lib/opte/src/engine/flow_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ fn flow_expired_probe(
__dtrace_probe_flow__expired(
port.as_ptr() as uintptr_t,
name.as_ptr() as uintptr_t,
flowid as *const _ 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,
);
Expand Down Expand Up @@ -325,7 +325,7 @@ extern "C" {
pub fn __dtrace_probe_flow__expired(
port: uintptr_t,
layer: uintptr_t,
flowid: uintptr_t,
flowid: *const InnerFlowId,
last_hit: uintptr_t,
now: uintptr_t,
);
Expand All @@ -334,7 +334,7 @@ extern "C" {
dir: uintptr_t,
port: uintptr_t,
layer: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
epoch: uintptr_t,
);
}
Expand Down
16 changes: 8 additions & 8 deletions lib/opte/src/engine/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ impl Layer {
self.port_c.as_ptr() as uintptr_t,
self.name_c.as_ptr() as uintptr_t,
dir_c.as_ptr() as uintptr_t,
flow as *const _ as uintptr_t,
flow,
msg_c.as_ptr() as uintptr_t,
);
}
Expand Down Expand Up @@ -611,7 +611,7 @@ impl Layer {
self.port_c.as_ptr() as uintptr_t,
self.name_c.as_ptr() as uintptr_t,
dir_c.as_ptr() as uintptr_t,
flow as *const _ as uintptr_t,
flow,
msg_c.as_ptr() as uintptr_t,
);
}
Expand Down Expand Up @@ -648,7 +648,7 @@ impl Layer {
dir as uintptr_t,
self.port_c.as_ptr() as uintptr_t,
self.name_c.as_ptr() as uintptr_t,
ifid as *const _ as uintptr_t,
ifid,
);
}
} else if #[cfg(feature = "usdt")] {
Expand Down Expand Up @@ -1458,7 +1458,7 @@ impl Layer {
self.port_c.as_ptr() as uintptr_t,
self.name_c.as_ptr() as uintptr_t,
dir as uintptr_t,
flow_id as *const _ as uintptr_t,
flow_id,
);
}
} else if #[cfg(feature = "usdt")] {
Expand Down Expand Up @@ -1770,23 +1770,23 @@ extern "C" {
port: uintptr_t,
layer: uintptr_t,
dir: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
msg: uintptr_t,
);

pub fn __dtrace_probe_gen__ht__fail(
port: uintptr_t,
layer: uintptr_t,
dir: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
msg: uintptr_t,
);

pub fn __dtrace_probe_layer__process__entry(
dir: uintptr_t,
port: uintptr_t,
name: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
);
pub fn __dtrace_probe_layer__process__return(
dir: uintptr_t,
Expand All @@ -1804,7 +1804,7 @@ extern "C" {
port: uintptr_t,
layer: uintptr_t,
dir: uintptr_t,
flow: uintptr_t,
flow: *const InnerFlowId,
);
}

Expand Down
28 changes: 14 additions & 14 deletions lib/opte/src/engine/port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ impl<N: NetworkImpl> Port<N> {
__dtrace_probe_tcp__err(
dir as uintptr_t,
self.name_cstr.as_ptr() as uintptr_t,
flow as *const _ as uintptr_t,
flow,
mblk_addr,
msg_arg.as_ptr() as uintptr_t,
);
Expand Down Expand Up @@ -1411,7 +1411,7 @@ impl<N: NetworkImpl> Port<N> {
__dtrace_probe_port__process__entry(
dir as uintptr_t,
self.name_cstr.as_ptr() as uintptr_t,
flow as *const _ as uintptr_t,
flow,
epoch as uintptr_t,
pkt.mblk_addr(),
);
Expand Down Expand Up @@ -1456,8 +1456,8 @@ impl<N: NetworkImpl> Port<N> {
__dtrace_probe_port__process__return(
dir as uintptr_t,
self.name_cstr.as_ptr() as uintptr_t,
flow_before as *const _ as uintptr_t,
&flow_after as *const _ as uintptr_t,
flow_before,
flow_after,
epoch as uintptr_t,
pkt.mblk_addr(),
hp_pkt_ptr,
Expand Down Expand Up @@ -1879,7 +1879,7 @@ impl<N: NetworkImpl> Port<N> {
__dtrace_probe_uft__hit(
dir as uintptr_t,
self.name_cstr.as_ptr() as uintptr_t,
ufid as *const _ as uintptr_t,
ufid,
epoch as uintptr_t,
last_hit.raw_millis().unwrap_or_default() as usize
);
Expand Down Expand Up @@ -2342,7 +2342,7 @@ impl<N: NetworkImpl> Port<N> {
__dtrace_probe_uft__invalidate(
dir as uintptr_t,
self.name_cstr.as_ptr() as uintptr_t,
ufid as *const _ as uintptr_t,
ufid,
epoch as uintptr_t,
);
}
Expand Down Expand Up @@ -2379,7 +2379,7 @@ impl<N: NetworkImpl> Port<N> {
__dtrace_probe_uft__tcp__closed(
dir as uintptr_t,
self.name_cstr.as_ptr() as uintptr_t,
ufid as *const _ as uintptr_t,
ufid,
);
}
} else if #[cfg(feature = "usdt")] {
Expand Down Expand Up @@ -2661,15 +2661,15 @@ extern "C" {
pub fn __dtrace_probe_port__process__entry(
dir: uintptr_t,
port: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
epoch: uintptr_t,
pkt: uintptr_t,
);
pub fn __dtrace_probe_port__process__return(
dir: uintptr_t,
port: uintptr_t,
flow_before: uintptr_t,
flow_after: uintptr_t,
flow_before: *const InnerFlowId,
flow_after: *const InnerFlowId,
epoch: uintptr_t,
pkt: uintptr_t,
hp_pkt: uintptr_t,
Expand All @@ -2678,27 +2678,27 @@ extern "C" {
pub fn __dtrace_probe_tcp__err(
dir: uintptr_t,
port: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
pkt: uintptr_t,
msg: uintptr_t,
);
pub fn __dtrace_probe_uft__hit(
dir: uintptr_t,
port: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
epoch: uintptr_t,
last_hit: uintptr_t,
);
pub fn __dtrace_probe_uft__invalidate(
dir: uintptr_t,
port: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
epoch: uintptr_t,
);
pub fn __dtrace_probe_uft__tcp__closed(
dir: uintptr_t,
port: uintptr_t,
ifid: uintptr_t,
ifid: *const InnerFlowId,
);
}

Expand Down
8 changes: 4 additions & 4 deletions lib/opte/src/engine/tcp_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ impl TcpFlowState {
unsafe {
__dtrace_probe_tcp__flow__drop(
port.as_ptr() as uintptr_t,
flow_id as *const _ as uintptr_t,
flow_id,
&state as *const tcp_flow_state_sdt_arg as uintptr_t,
dir as uintptr_t,
flags as uintptr_t,
Expand Down Expand Up @@ -623,7 +623,7 @@ impl TcpFlowState {
unsafe {
__dtrace_probe_tcp__flow__state(
port.as_ptr() as uintptr_t,
flow_id as *const _ as uintptr_t,
flow_id,
self.tcp_state as uintptr_t,
new_state as uintptr_t,
);
Expand Down Expand Up @@ -674,14 +674,14 @@ impl From<&TcpFlowState> for tcp_flow_state_sdt_arg {
extern "C" {
pub fn __dtrace_probe_tcp__flow__state(
port: uintptr_t,
flow_id: uintptr_t,
flow_id: *const InnerFlowId,
prev_state: uintptr_t,
curr_state: uintptr_t,
);

pub fn __dtrace_probe_tcp__flow__drop(
port: uintptr_t,
flow_id: uintptr_t,
flow_id: *const InnerFlowId,
flow_state: uintptr_t,
dir: uintptr_t,
flags: uintptr_t,
Expand Down
5 changes: 3 additions & 2 deletions xde/src/xde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use alloc::string::String;
use alloc::string::ToString;
use alloc::sync::Arc;
use alloc::vec::Vec;
use opte::engine::packet::InnerFlowId;
use core::convert::TryInto;
use core::ffi::CStr;
use core::num::NonZeroU32;
Expand Down Expand Up @@ -127,7 +128,7 @@ extern "C" {
);
pub fn __dtrace_probe_guest__loopback(
mp: uintptr_t,
flow: uintptr_t,
flow: *const InnerFlowId,
src_port: uintptr_t,
dst_port: uintptr_t,
);
Expand Down Expand Up @@ -1333,7 +1334,7 @@ fn guest_loopback_probe(pkt: &Packet<Parsed>, src: &XdeDev, dst: &XdeDev) {
unsafe {
__dtrace_probe_guest__loopback(
pkt.mblk_addr(),
pkt.flow() as *const _ as uintptr_t,
pkt.flow(),
src.port.name_cstr().as_ptr() as uintptr_t,
dst.port.name_cstr().as_ptr() as uintptr_t,
)
Expand Down

0 comments on commit cb20ed2

Please sign in to comment.