From 173f5d33beafb766ae4c045bd03d9909484bd5cc Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 24 Apr 2024 11:32:35 +0100 Subject: [PATCH 01/30] WIP: Deliver packets via DLS client to escape S/W --- xde/src/dls.rs | 423 ++++++++++++++++++++++++++++++++++++++++++++- xde/src/mac.rs | 32 ++++ xde/src/mac_sys.rs | 21 ++- xde/src/xde.rs | 114 ++++++++++-- 4 files changed, 574 insertions(+), 16 deletions(-) diff --git a/xde/src/dls.rs b/xde/src/dls.rs index 7f9c3a5a..c41c11f9 100644 --- a/xde/src/dls.rs +++ b/xde/src/dls.rs @@ -2,15 +2,39 @@ // 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 // stuff we need from dls +use crate::ip::kcondvar_t; +use crate::ip::krwlock_t; +use crate::ip::list_node_t; +use crate::ip::major_t; +use crate::ip::minor_t; +use crate::ip::queue_t; +use crate::ip::t_uscalar_t; use crate::mac; +use crate::mac::mac_client_handle; +use crate::mac::mac_handle; +use crate::mac::mac_promisc_handle; +use crate::mac::MacPerimeterHandle; +use crate::mac::MacTxFlags; +use crate::mac_sys::mac_tx_cookie_t; +use crate::mac_sys::MAC_DROP_ON_NO_DESC; +use core::ffi::c_void; +use core::mem::MaybeUninit; +use core::ptr; use illumos_sys_hdrs::boolean_t; +use illumos_sys_hdrs::c_char; use illumos_sys_hdrs::c_int; +use illumos_sys_hdrs::c_uint; use illumos_sys_hdrs::datalink_id_t; +use illumos_sys_hdrs::kmutex_t; +use illumos_sys_hdrs::mblk_t; +use illumos_sys_hdrs::uintptr_t; use illumos_sys_hdrs::zoneid_t; +use opte::engine::packet::Packet; +use opte::engine::packet::PacketState; extern "C" { pub fn dls_devnet_create( @@ -24,4 +48,401 @@ extern "C" { linkid: *mut datalink_id_t, wait: boolean_t, ) -> c_int; + + pub fn dls_mgmt_get_linkid( + name: *const c_char, + linkid: *mut datalink_id_t, + ) -> c_int; +} + +// Private DLS functions needed to get us a Tx path. +extern "C" { + pub type dls_devnet_s; + pub type dls_link; + + // ALL OF THESE REQUIRE THE MAC PERIMETER. + pub fn dls_devnet_hold_link( + link: datalink_id_t, + ddhp: *mut dls_dl_handle, + dlpp: *mut *mut dls_link, + ) -> c_int; + pub fn dls_devnet_rele_link(dlh: dls_dl_handle, dlp: *mut dls_link); + pub fn dls_open( + dlp: *mut dls_link, + ddh: dls_dl_handle, + dsp: *mut dld_str_s, + ) -> c_int; + pub fn dls_close(dsp: *mut dld_str_s); + + // THIS DOES NOT. + pub fn str_mdata_fastpath_put( + dsp: *const dld_str_s, + mp: *mut mblk_t, + f_hint: uintptr_t, + flag: u16, + ) -> mac_tx_cookie_t; +} + +// struct dld_str_s { /* Protected by */ +// /* +// * Major number of the device +// */ +// major_t ds_major; /* WO */ +// /* +// * Ephemeral minor number for the object. +// */ +// minor_t ds_minor; /* WO */ +// /* +// * PPA number this stream is attached to. +// */ +// t_uscalar_t ds_ppa; /* SL */ +// /* +// * Read/write queues for the stream which the object represents. +// */ +// queue_t *ds_rq; /* WO */ +// queue_t *ds_wq; /* WO */ +// /* +// * Stream is open to DLD_CONTROL (control node) or +// * DLD_DLPI (DLS provider) node. +// */ +// uint_t ds_type; /* WO */ +// /* +// * The following fields are only used for DLD_DLPI type objects. +// */ +// /* +// * Current DLPI state. +// */ +// t_uscalar_t ds_dlstate; /* SL */ +// /* +// * DLPI style +// */ +// t_uscalar_t ds_style; /* WO */ +// /* +// * Currently bound DLSAP. +// */ +// uint16_t ds_sap; /* SL */ +// /* +// * Handle of the MAC that is used by the data-link interface. +// */ +// mac_handle_t ds_mh; /* SL */ +// mac_client_handle_t ds_mch; /* SL */ +// /* +// * Promiscuity level information. +// */ +// uint32_t ds_promisc; /* SL */ +// mac_promisc_handle_t ds_mph; +// mac_promisc_handle_t ds_vlan_mph; + +// /* +// * Immutable information of the MAC which the channel is using. +// */ +// const mac_info_t *ds_mip; /* SL */ +// /* +// * Current packet priority. +// */ +// uint_t ds_pri; /* SL */ +// /* +// * Handle of our MAC notification callback. +// */ +// mac_notify_handle_t ds_mnh; /* SL */ +// /* +// * Set of enabled DL_NOTE... notifications. (See dlpi.h). +// */ +// uint32_t ds_notifications; /* SL */ +// /* +// * Mode: unitdata, fast-path or raw. +// */ +// dld_str_mode_t ds_mode; /* SL */ +// /* +// * Native mode state. +// */ +// boolean_t ds_native; /* SL */ +// /* +// * IP polling is operational if this flag is set. +// */ +// boolean_t ds_polling; /* SL */ +// boolean_t ds_direct; /* SL */ +// /* +// * LSO is enabled if ds_lso is set. +// */ +// boolean_t ds_lso; /* SL */ +// uint64_t ds_lso_max; /* SL */ +// /* +// * State of DLPI user: may be active (regular network layer), +// * passive (snoop-like monitoring), or unknown (not yet +// * determined). +// */ +// dld_passivestate_t ds_passivestate; /* SL */ +// /* +// * Dummy mblk used for flow-control. +// */ +// mblk_t *ds_tx_flow_mp; /* ds_lock */ +// /* +// * List of queued DLPI requests. These will be processed +// * by a taskq thread. This block is protected by ds_lock +// */ +// kmutex_t ds_lock; +// krwlock_t ds_rw_lock; +// kcondvar_t ds_datathr_cv; /* ds_lock */ +// uint_t ds_datathr_cnt; /* ds_lock */ +// mblk_t *ds_pending_head; /* ds_lock */ +// mblk_t *ds_pending_tail; /* ds_lock */ +// kcondvar_t ds_dlpi_pending_cv; /* ds_lock */ +// uint32_t +// ds_dlpi_pending : 1, /* ds_lock */ +// ds_local : 1, +// ds_pad : 30; /* ds_lock */ +// dls_link_t *ds_dlp; /* SL */ +// dls_multicst_addr_t *ds_dmap; /* ds_rw_lock */ +// dls_rx_t ds_rx; /* ds_lock */ +// void *ds_rx_arg; /* ds_lock */ +// uint_t ds_nactive; /* SL */ +// dld_str_t *ds_next; /* SL */ +// dls_head_t *ds_head; +// dls_dl_handle_t ds_ddh; +// list_node_t ds_tqlist; + +// /* +// * driver private data set by the driver when calling dld_str_open(). +// */ +// void *ds_private; + +// boolean_t ds_lowlink; /* SL */ +// boolean_t ds_nonip; /* SL */ +// }; + +#[repr(C)] +pub enum dld_str_mode_t { + DLD_UNITDATA, + DLD_FASTPATH, + DLD_RAW, +} + +#[repr(C)] +pub enum dld_passivestate_t { + DLD_UNINITIALIZED, + DLD_PASSIVE, + DLD_ACTIVE, +} + +#[repr(C)] +pub struct dld_str_s { + ds_major: major_t, + ds_minor: minor_t, + + ds_ppa: t_uscalar_t, + + ds_rq: *mut queue_t, + ds_wq: *mut queue_t, + + ds_type: c_uint, + + ds_dlstate: t_uscalar_t, + ds_style: t_uscalar_t, + ds_sap: u16, + + ds_mh: *mut mac_handle, + ds_mch: *mut mac_client_handle, + + ds_promisc: u32, + ds_mph: *mut mac_promisc_handle, + ds_vlan_mph: *mut mac_promisc_handle, + + ds_mip: *const c_void, // mac_info_t + + ds_pri: c_uint, + + ds_mnh: *mut c_void, // mac_notify_handle_t + + ds_notifications: u32, + + ds_mode: dld_str_mode_t, + + ds_native: boolean_t, + + ds_polling: boolean_t, + ds_direct: boolean_t, + + ds_lso: boolean_t, + ds_lso_max: u64, + + ds_passivestate: dld_passivestate_t, + + ds_tx_flow_mp: *mut mblk_t, + + /* + * List of queued DLPI requests. These will be processed + * by a taskq thread. This block is protected by ds_lock + */ + ds_lock: kmutex_t, + ds_rw_lock: krwlock_t, + ds_datathr_cv: kcondvar_t, + ds_datathr_cnt: c_uint, + ds_pending_head: *mut mblk_t, + ds_pending_tail: *mut mblk_t, + ds_dlpi_pending_cv: kcondvar_t, + // bitset: ds_dlpi_pending + ds_local + ds_pad + dl_dlpi_pend_local: u32, + + ds_dlp: *mut dls_link, + ds_dmap: *mut c_void, // dls_multicst_addr_t + ds_rx: *mut c_void, // dls_rx_t + ds_rx_arg: *mut c_void, + ds_nactive: c_uint, + ds_next: *mut dld_str_s, + ds_head: *mut c_void, // dls_head_t + ds_ddh: dls_dl_handle, + ds_tqlist: list_node_t, + + ds_private: *mut c_void, + + ds_lowlink: boolean_t, + ds_nonip: boolean_t, +} + +impl core::fmt::Debug for dld_str_s { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "") + } +} + +pub type dls_dl_handle = *mut dls_devnet_s; + +#[derive(Debug)] +pub struct DlsLink { + inner: Option, +} + +#[derive(Debug)] +pub struct DlsLinkInner { + dlp: *mut dls_link, + dlh: dls_dl_handle, + link: datalink_id_t, +} + +impl DlsLink { + pub fn hold(mph: &MacPerimeterHandle) -> Result { + let mut dlp = ptr::null_mut(); + let mut dlh = ptr::null_mut(); + let link = mph.linkid(); + + let res = unsafe { dls_devnet_hold_link(link, &mut dlh, &mut dlp) }; + if res == 0 { + Ok(Self { inner: Some(DlsLinkInner { dlp, dlh, link }) }) + } else { + Err(res) + } + } + + // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. + pub fn release(mut self, mph: &MacPerimeterHandle) { + if let Some(inner) = self.inner.take() { + if mph.linkid() != inner.link { + panic!("Tried to free link hold with the wrong MAC perimeter: saw {}, wanted {}", + mph.linkid(), inner.link); + } + unsafe { + dls_devnet_rele_link(inner.dlh, inner.dlp); + } + } + } + + pub fn open_stream( + &self, + mph: &MacPerimeterHandle, + ) -> Result { + let Some(inner) = self.inner.as_ref() else { + return Err(-1); + }; + + if mph.linkid() != inner.link { + panic!("Tried to open stream with the wrong MAC perimeter: saw {}, wanted {}", + mph.linkid(), inner.link); + } + + let mut stream = MaybeUninit::zeroed(); + let res = + unsafe { dls_open(inner.dlp, inner.dlh, stream.as_mut_ptr()) }; + if res == 0 { + let dld_str = unsafe { stream.assume_init() }; + Ok(DldStream { + inner: Some(DldStreamInner { dld_str, link: mph.linkid() }), + }) + } else { + Err(res) + } + } +} + +impl Drop for DlsLink { + fn drop(&mut self) { + if let Some(inner) = self.inner.take() { + opte::engine::err!( + "dropped hold on link {} without releasing!!", + inner.link + ); + } + } +} + +#[derive(Debug)] +pub struct DldStream { + inner: Option, +} + +#[derive(Debug)] +pub struct DldStreamInner { + dld_str: dld_str_s, + link: datalink_id_t, +} + +impl DldStream { + pub fn tx_drop_on_no_desc( + &self, + pkt: Packet, + hint: uintptr_t, + flags: MacTxFlags, + ) { + let Some(inner) = self.inner.as_ref() else { + // XXX: probably handle or signal an error here. + return; + }; + // We must unwrap the raw `mblk_t` out of the `pkt` here, + // otherwise the mblk_t would be dropped at the end of this + // function along with `pkt`. + let mut raw_flags = flags.bits(); + raw_flags |= MAC_DROP_ON_NO_DESC; + unsafe { + str_mdata_fastpath_put( + &inner.dld_str, + pkt.unwrap_mblk(), + hint, + raw_flags, + ) + }; + } + + // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. + pub fn release(mut self, mph: &MacPerimeterHandle) { + if let Some(mut inner) = self.inner.take() { + if mph.linkid() != inner.link { + opte::engine::err!("Tried to free link hold with the wrong MAC perimeter: saw {}, wanted {}", + mph.linkid(), inner.link); + } + unsafe { + dls_close(&mut inner.dld_str); + } + } + } +} + +impl Drop for DldStream { + fn drop(&mut self) { + if let Some(inner) = self.inner.take() { + opte::engine::err!( + "dropped stream on link {} without releasing!!", + inner.link + ); + } + } } diff --git a/xde/src/mac.rs b/xde/src/mac.rs index e68aac05..7741bd51 100644 --- a/xde/src/mac.rs +++ b/xde/src/mac.rs @@ -329,3 +329,35 @@ impl Drop for MacUnicastHandle { unsafe { mac_unicast_remove(self.mch.mch, self.muh) }; } } + +// XXX: cleanup + +/// Safe wrapper around a `mac_perim_handle_t`. +pub struct MacPerimeterHandle { + mph: mac_perim_handle, + link: datalink_id_t, +} + +impl MacPerimeterHandle { + pub fn from_linkid(link: datalink_id_t) -> Result { + let mut mph = 0; + let res = unsafe { mac_perim_enter_by_linkid(link, &mut mph) }; + if res == 0 { + Ok(Self { mph, link }) + } else { + Err(res) + } + } + + pub fn linkid(&self) -> datalink_id_t { + self.link + } +} + +impl Drop for MacPerimeterHandle { + fn drop(&mut self) { + unsafe { + mac_perim_exit(self.mph); + } + } +} diff --git a/xde/src/mac_sys.rs b/xde/src/mac_sys.rs index 352193e7..e4fb9ee8 100644 --- a/xde/src/mac_sys.rs +++ b/xde/src/mac_sys.rs @@ -2,7 +2,7 @@ // 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 // stuff we need from mac @@ -11,6 +11,7 @@ use illumos_sys_hdrs::c_char; use illumos_sys_hdrs::c_int; use illumos_sys_hdrs::c_uint; use illumos_sys_hdrs::c_void; +use illumos_sys_hdrs::datalink_id_t; use illumos_sys_hdrs::ddi_info_cmd_t; use illumos_sys_hdrs::dev_info; use illumos_sys_hdrs::dev_ops; @@ -160,6 +161,24 @@ extern "C" { pub fn mac_private_minor() -> minor_t; } +// Private MAC functions needed to get us a Tx path. + +// Not quite a `void*` -- this includes extra flags etc. +pub type mac_perim_handle = uintptr_t; +extern "C" { + pub fn mac_perim_enter_by_mh(mh: mac_handle, mph: *mut mac_perim_handle); + pub fn mac_perim_enter_by_macname( + name: *const c_char, + mph: *mut mac_perim_handle, + ) -> c_int; + pub fn mac_perim_enter_by_linkid( + link: datalink_id_t, + mph: *mut mac_perim_handle, + ) -> c_int; + pub fn mac_perim_exit(mph: mac_perim_handle); + pub fn mac_perim_held(mh: mac_handle) -> boolean_t; +} + #[repr(C)] #[derive(Debug)] pub enum mac_diag { diff --git a/xde/src/xde.rs b/xde/src/xde.rs index e588b4cd..9de2e659 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -13,6 +13,8 @@ //#![allow(clippy::arc_with_non_send_sync)] use crate::dls; +use crate::dls::DldStream; +use crate::dls::DlsLink; use crate::ioctl::IoctlEnvelope; use crate::ip; use crate::mac; @@ -21,6 +23,7 @@ use crate::mac::mac_private_minor; use crate::mac::MacClientHandle; use crate::mac::MacHandle; use crate::mac::MacOpenFlags; +use crate::mac::MacPerimeterHandle; use crate::mac::MacPromiscHandle; use crate::mac::MacTxFlags; use crate::mac::MacUnicastHandle; @@ -231,11 +234,14 @@ struct xde_underlay_port { /// MAC client handle for tx/rx on the underlay link. mch: Arc, - /// MAC client handle for tx/rx on the underlay link. - muh: MacUnicastHandle, - + // /// MAC client handle for tx/rx on the underlay link. + // muh: MacUnicastHandle, /// MAC promiscuous handle for receiving packets on the underlay link. mph: MacPromiscHandle, + + link: DlsLink, + + stream: DldStream, } struct XdeState { @@ -951,6 +957,44 @@ fn create_underlay_port( link_name: String, mc_name: &str, ) -> Result { + let mut link_id = 0; + let link_cstr = CString::new(link_name.as_str()).unwrap(); + unsafe { + match dls::dls_mgmt_get_linkid(link_cstr.as_ptr(), &mut link_id) { + 0 => {} + err => { + return Err(OpteError::System { + errno: EFAULT, + msg: format!("failed to get linkid for {link_name}: {err}"), + }) + } + } + } + + let mph = MacPerimeterHandle::from_linkid(link_id).map_err(|e| { + OpteError::System { + errno: EFAULT, + msg: format!("failed to grab MAC perimeter for {link_name}: {e}"), + } + })?; + + // XXX: error handling will be REALLY fucking bad here with these panic drops. + let link = DlsLink::hold(&mph).map_err(|e| OpteError::System { + errno: EFAULT, + msg: format!( + "failed to grab hold on link perimeter for {link_name}: {e}" + ), + })?; + + let stream = link.open_stream(&mph).map_err(|e| OpteError::System { + errno: EFAULT, + msg: format!("failed to grab open stream for {link_name}: {e}"), + })?; + + drop(mph); + + // Promisc setup. + // Grab mac handle for underlying link let mh = MacHandle::open_by_link_name(&link_name).map(Arc::new).map_err( |e| OpteError::System { @@ -990,11 +1034,17 @@ fn create_underlay_port( // requires that if there is a single mac client on a link, that client must // have an L2 address. This was not caught until recently, because this is // only enforced as a debug assert in the kernel. - let mac = EtherAddr::from([0xa8, 0x40, 0x25, 0xff, 0x00, 0x00]); - let muh = mch.add_unicast(mac).map_err(|e| OpteError::System { - errno: EFAULT, - msg: format!("mac_unicast_add failed for {link_name}: {e}"), - })?; + + // ks: + // ON ABOVE: we are never the only MAC client on a link. If we somehow are, we + // should find a way to enfore the above constraint. + // (even so, the presence of a dls-visible link implies a client?) + + // let mac = EtherAddr::from([0xa8, 0x40, 0x25, 0xff, 0x00, 0x00]); + // let muh = mch.add_unicast(mac).map_err(|e| OpteError::System { + // errno: EFAULT, + // msg: format!("mac_unicast_add failed for {link_name}: {e}"), + // })?; Ok(xde_underlay_port { name: link_name, @@ -1002,7 +1052,9 @@ fn create_underlay_port( mh, mch, mph, - muh, + // muh, + link, + stream, }) } @@ -1159,7 +1211,7 @@ unsafe extern "C" fn xde_detach( // 1. Remove promisc and unicast callbacks drop(u.mph); - drop(u.muh); + // drop(u.muh); // 2. Remove MAC client handle if Arc::strong_count(&u.mch) > 1 { @@ -1177,6 +1229,34 @@ unsafe extern "C" fn xde_detach( return DDI_FAILURE; } drop(u.mh); + + // XXX: test code + let mut link_id = 0; + let link_cstr = CString::new(u.name.as_str()).unwrap(); + unsafe { + match dls::dls_mgmt_get_linkid(link_cstr.as_ptr(), &mut link_id) + { + 0 => {} + err => { + warn!("failed to get linkid for {}: {}", u.name, err); + return DDI_FAILURE; + } + } + } + + let mph = match MacPerimeterHandle::from_linkid(link_id) { + Ok(mph) => mph, + Err(err) => { + warn!( + "failed to grab MAC perimeter for {}: {}", + u.name, err + ); + return DDI_FAILURE; + } + }; + + u.stream.release(&mph); + u.link.release(&mph); } } @@ -1471,7 +1551,8 @@ unsafe extern "C" fn xde_mc_tx( // Choose u1 as a starting point. This may be changed in the next_hop // function when we are actually able to determine what interface should be // used. - let mch = &src_dev.u1.mch; + // let mch = &src_dev.u1.mch; + let stream = &src_dev.u1.stream; let hint = 0; // Send straight to underlay in passthrough mode. @@ -1482,7 +1563,7 @@ unsafe extern "C" fn xde_mc_tx( // refresh my memory on all of this. // // TODO Is there way to set mac_tx to must use result? - mch.tx_drop_on_no_desc(pkt, hint, MacTxFlags::empty()); + stream.tx_drop_on_no_desc(pkt, hint, MacTxFlags::empty()); return ptr::null_mut(); } @@ -1550,7 +1631,12 @@ unsafe extern "C" fn xde_mc_tx( // Unwrap: We know the packet is good because we just // unwrapped it above. let new_pkt = Packet::::wrap_mblk(mblk).unwrap(); - underlay_dev.mch.tx_drop_on_no_desc( + // underlay_dev.mch.tx_drop_on_no_desc( + // new_pkt, + // hint, + // MacTxFlags::empty(), + // ); + underlay_dev.stream.tx_drop_on_no_desc( new_pkt, hint, MacTxFlags::empty(), @@ -1566,7 +1652,7 @@ unsafe extern "C" fn xde_mc_tx( } Ok(ProcessResult::Bypass) => { - mch.tx_drop_on_no_desc(pkt, hint, MacTxFlags::empty()); + stream.tx_drop_on_no_desc(pkt, hint, MacTxFlags::empty()); } Err(_) => {} From 94c52646d7a41185ccda77824cb1d0cf188b83e1 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 24 Apr 2024 13:15:48 +0100 Subject: [PATCH 02/30] WIP: Put the promisc callback on the right mac client. --- xde/src/dls.rs | 42 +++++++++++++++++++- xde/src/mac.rs | 12 +++--- xde/src/xde.rs | 102 +++++++++++++++++++++++++++++-------------------- 3 files changed, 106 insertions(+), 50 deletions(-) diff --git a/xde/src/dls.rs b/xde/src/dls.rs index c41c11f9..3c2641f7 100644 --- a/xde/src/dls.rs +++ b/xde/src/dls.rs @@ -15,12 +15,17 @@ use crate::ip::queue_t; use crate::ip::t_uscalar_t; use crate::mac; use crate::mac::mac_client_handle; +use crate::mac::mac_client_promisc_type_t; use crate::mac::mac_handle; +use crate::mac::mac_promisc_add; use crate::mac::mac_promisc_handle; +use crate::mac::mac_rx_fn; use crate::mac::MacPerimeterHandle; +use crate::mac::MacPromiscHandle; use crate::mac::MacTxFlags; use crate::mac_sys::mac_tx_cookie_t; use crate::mac_sys::MAC_DROP_ON_NO_DESC; +use alloc::sync::Arc; use core::ffi::c_void; use core::mem::MaybeUninit; use core::ptr; @@ -350,7 +355,7 @@ impl DlsLink { pub fn open_stream( &self, mph: &MacPerimeterHandle, - ) -> Result { + ) -> Result, c_int> { let Some(inner) = self.inner.as_ref() else { return Err(-1); }; @@ -367,7 +372,8 @@ impl DlsLink { let dld_str = unsafe { stream.assume_init() }; Ok(DldStream { inner: Some(DldStreamInner { dld_str, link: mph.linkid() }), - }) + } + .into()) } else { Err(res) } @@ -397,6 +403,38 @@ pub struct DldStreamInner { } impl DldStream { + /// Register promiscuous callback to receive packets on the underlying MAC. + pub fn add_promisc( + self: &Arc, + ptype: mac_client_promisc_type_t, + promisc_fn: mac_rx_fn, + flags: u16, + ) -> Result, c_int> { + let Some(inner) = self.inner.as_ref() else { + return Err(-1); + }; + + let mut mph = ptr::null_mut(); + + // `MacPromiscHandle` keeps a reference to this `MacClientHandle` + // until it is removed and so we can safely access it from the + // callback via the `arg` pointer. + let _parent = self.clone(); + let arg = Arc::as_ptr(&_parent) as *mut c_void; + let mch = inner.dld_str.ds_mch; + let ret = unsafe { + // NOTE: arg is reinterpreted as `mac_resource_handle` -> `mac_ring` + // in `mac_rx_common`. Is what we've been doing here and before even safe? + mac_promisc_add(mch, ptype, promisc_fn, arg, &mut mph, flags) + }; + + if ret == 0 { + Ok(MacPromiscHandle { mph, _parent }) + } else { + Err(ret) + } + } + pub fn tx_drop_on_no_desc( &self, pkt: Packet, diff --git a/xde/src/mac.rs b/xde/src/mac.rs index 7741bd51..44ab132a 100644 --- a/xde/src/mac.rs +++ b/xde/src/mac.rs @@ -202,7 +202,7 @@ impl MacClientHandle { ptype: mac_client_promisc_type_t, promisc_fn: mac_rx_fn, flags: u16, - ) -> Result { + ) -> Result, c_int> { let mut mph = ptr::null_mut(); // `MacPromiscHandle` keeps a reference to this `MacClientHandle` @@ -215,7 +215,7 @@ impl MacClientHandle { }; if ret == 0 { - Ok(MacPromiscHandle { mph, _mch: mch }) + Ok(MacPromiscHandle { mph, _parent: mch }) } else { Err(ret) } @@ -294,15 +294,15 @@ impl Drop for MacClientHandle { /// Safe wrapper around a `mac_promisc_handle_t`. #[derive(Debug)] -pub struct MacPromiscHandle { +pub struct MacPromiscHandle

{ /// The underlying `mac_promisc_handle_t`. - mph: *mut mac_promisc_handle, + pub(crate) mph: *mut mac_promisc_handle, /// The `MacClientHandle` used to create this promiscuous callback. - _mch: Arc, + pub(crate) _parent: Arc

, } -impl Drop for MacPromiscHandle { +impl

Drop for MacPromiscHandle

{ fn drop(&mut self) { // Safety: We know that a `MacPromiscHandle` can only exist if a // mac promisc handle was successfully obtained, and thus `mph` diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 9de2e659..f4f448df 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -229,19 +229,19 @@ struct xde_underlay_port { mac: [u8; 6], /// MAC handle to the underlay link. - mh: Arc, + // mh: Arc, /// MAC client handle for tx/rx on the underlay link. - mch: Arc, + // mch: Arc, // /// MAC client handle for tx/rx on the underlay link. // muh: MacUnicastHandle, /// MAC promiscuous handle for receiving packets on the underlay link. - mph: MacPromiscHandle, + mph: MacPromiscHandle, link: DlsLink, - stream: DldStream, + stream: Arc, } struct XdeState { @@ -995,29 +995,7 @@ fn create_underlay_port( // Promisc setup. - // Grab mac handle for underlying link - let mh = MacHandle::open_by_link_name(&link_name).map(Arc::new).map_err( - |e| OpteError::System { - errno: EFAULT, - msg: format!("failed to open link {link_name} for underlay: {e}"), - }, - )?; - - // Get a mac client handle as well. - // - let oflags = MacOpenFlags::NONE; - let mch = MacClientHandle::open(&mh, Some(mc_name), oflags, 0) - .map(Arc::new) - .map_err(|e| OpteError::System { - errno: EFAULT, - msg: format!("mac_client_open failed for {link_name}: {e}"), - })?; - - // Setup promiscuous callback to receive all packets on this link. - // - // We specify `MAC_PROMISC_FLAGS_NO_TX_LOOP` here to skip receiving copies - // of outgoing packets we sent ourselves. - let mph = mch + let mph = stream .add_promisc( mac::mac_client_promisc_type_t::MAC_CLIENT_PROMISC_ALL, xde_rx, @@ -1028,6 +1006,39 @@ fn create_underlay_port( msg: format!("mac_promisc_add failed for {link_name}: {e}"), })?; + // Grab mac handle for underlying link + let mh = MacHandle::open_by_link_name(&link_name).map(Arc::new).map_err( + |e| OpteError::System { + errno: EFAULT, + msg: format!("failed to open link {link_name} for underlay: {e}"), + }, + )?; + + // // Get a mac client handle as well. + // // + // let oflags = MacOpenFlags::NONE; + // let mch = MacClientHandle::open(&mh, Some(mc_name), oflags, 0) + // .map(Arc::new) + // .map_err(|e| OpteError::System { + // errno: EFAULT, + // msg: format!("mac_client_open failed for {link_name}: {e}"), + // })?; + + // // Setup promiscuous callback to receive all packets on this link. + // // + // // We specify `MAC_PROMISC_FLAGS_NO_TX_LOOP` here to skip receiving copies + // // of outgoing packets we sent ourselves. + // let mph = mch + // .add_promisc( + // mac::mac_client_promisc_type_t::MAC_CLIENT_PROMISC_ALL, + // xde_rx, + // mac::MAC_PROMISC_FLAGS_NO_TX_LOOP, + // ) + // .map_err(|e| OpteError::System { + // errno: EFAULT, + // msg: format!("mac_promisc_add failed for {link_name}: {e}"), + // })?; + // Set up a unicast callback. The MAC address here is a sentinel value with // nothing real behind it. This is why we picked the zero value in the Oxide // OUI space for virtual MACs. The reason this is being done is that illumos @@ -1049,8 +1060,8 @@ fn create_underlay_port( Ok(xde_underlay_port { name: link_name, mac: mh.get_mac_addr(), - mh, - mch, + // mh, + // mch, mph, // muh, link, @@ -1200,7 +1211,7 @@ unsafe extern "C" fn xde_detach( for u in [u1, u2] { // Clear all Rx paths - u.mch.clear_rx(); + // u.mch.clear_rx(); // We have a chain of refs here: // 1. `MacPromiscHandle` holds a ref to `MacClientHandle`, and @@ -1212,23 +1223,30 @@ unsafe extern "C" fn xde_detach( // 1. Remove promisc and unicast callbacks drop(u.mph); // drop(u.muh); - - // 2. Remove MAC client handle - if Arc::strong_count(&u.mch) > 1 { + let Ok(stream) = Arc::try_unwrap(u.stream) else { warn!( "underlay {} has outstanding mac client handle refs", u.name ); return DDI_FAILURE; - } - drop(u.mch); + }; - // Finally, we can cleanup the MAC handle for this underlay - if Arc::strong_count(&u.mh) > 1 { - warn!("underlay {} has outstanding mac handle refs", u.name); - return DDI_FAILURE; - } - drop(u.mh); + // // 2. Remove MAC client handle + // if Arc::strong_count(&u.stream) > 1 { + // warn!( + // "underlay {} has outstanding mac client handle refs", + // u.name + // ); + // return DDI_FAILURE; + // } + // drop(u.mch); + + // // Finally, we can cleanup the MAC handle for this underlay + // if Arc::strong_count(&u.mh) > 1 { + // warn!("underlay {} has outstanding mac handle refs", u.name); + // return DDI_FAILURE; + // } + // drop(u.mh); // XXX: test code let mut link_id = 0; @@ -1255,7 +1273,7 @@ unsafe extern "C" fn xde_detach( } }; - u.stream.release(&mph); + stream.release(&mph); u.link.release(&mph); } } From c34e257bda698f8f545846568218540c8246761d Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 14 Jun 2024 18:26:47 +0100 Subject: [PATCH 03/30] Fixup for real! --- xde/src/dls.rs | 5 +++-- xde/src/mac.rs | 11 ++++++---- xde/src/xde.rs | 58 ++++++++++++++++---------------------------------- 3 files changed, 28 insertions(+), 46 deletions(-) diff --git a/xde/src/dls.rs b/xde/src/dls.rs index 3c2641f7..ec46d02c 100644 --- a/xde/src/dls.rs +++ b/xde/src/dls.rs @@ -420,7 +420,8 @@ impl DldStream { // until it is removed and so we can safely access it from the // callback via the `arg` pointer. let _parent = self.clone(); - let arg = Arc::as_ptr(&_parent) as *mut c_void; + let parent = Arc::into_raw(_parent) as *mut _; + let arg = parent as *mut c_void; let mch = inner.dld_str.ds_mch; let ret = unsafe { // NOTE: arg is reinterpreted as `mac_resource_handle` -> `mac_ring` @@ -429,7 +430,7 @@ impl DldStream { }; if ret == 0 { - Ok(MacPromiscHandle { mph, _parent }) + Ok(MacPromiscHandle { mph, parent }) } else { Err(ret) } diff --git a/xde/src/mac.rs b/xde/src/mac.rs index 4b4cfb25..b562d2c6 100644 --- a/xde/src/mac.rs +++ b/xde/src/mac.rs @@ -2,12 +2,14 @@ // 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 the mac client API. //! //! NOTE: This module is re-exporting all of the sys definitions at //! the moment out of laziness. +use crate::dls::DldStreamInner; + pub use super::mac_sys::*; use alloc::ffi::CString; use alloc::string::String; @@ -221,7 +223,7 @@ impl MacClientHandle { }; if ret == 0 { - Ok(MacPromiscHandle { mph, mch }) + Ok(MacPromiscHandle { mph, parent: mch }) } else { Err(ret) } @@ -305,7 +307,8 @@ pub struct MacPromiscHandle

{ pub(crate) mph: *mut mac_promisc_handle, /// The `MacClientHandle` used to create this promiscuous callback. - mch: *const MacClientHandle, + /// MUST BE A RAW ARC. + pub(crate) parent: *const P, } impl

Drop for MacPromiscHandle

{ @@ -315,7 +318,7 @@ impl

Drop for MacPromiscHandle

{ // is valid. unsafe { mac_promisc_remove(self.mph); - Arc::from_raw(self.mch); // dropped immediately + Arc::from_raw(self.parent); // dropped immediately }; } } diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 3130af1c..eecccd99 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -937,9 +937,6 @@ fn clear_xde_underlay() -> Result { }; for u in [u1, u2] { - // Clear all Rx paths - u.mch.clear_rx(); - // We have a chain of refs here: // 1. `MacPromiscHandle` holds a ref to `MacClientHandle`, and // 2. `MacUnicastHandle` holds a ref to `MacClientHandle`, and @@ -965,8 +962,14 @@ fn clear_xde_underlay() -> Result { { 0 => {} err => { - warn!("failed to get linkid for {}: {}", u.name, err); - return DDI_FAILURE; + return Err(OpteError::System { + errno: EBUSY, + msg: format!( + "failed to get linkid for {}: {err}", + u.name + ) + .into(), + }); } } } @@ -974,15 +977,18 @@ fn clear_xde_underlay() -> Result { let mph = match MacPerimeterHandle::from_linkid(link_id) { Ok(mph) => mph, Err(err) => { - warn!( - "failed to grab MAC perimeter for {}: {}", - u.name, err - ); - return DDI_FAILURE; + return Err(OpteError::System { + errno: EBUSY, + msg: format!( + "failed to grab MAC perimeter for {}: {err}", + u.name + ) + .into(), + }); } }; - stream.release(&mph); + Arc::try_unwrap(u.stream).unwrap().release(&mph); u.link.release(&mph); } } @@ -1106,7 +1112,7 @@ fn create_underlay_port( } })?; - // XXX: error handling will be REALLY fucking bad here with these panic drops. + // XXX: error handling will be REALLY bad here with these panic drops. let link = DlsLink::hold(&mph).map_err(|e| OpteError::System { errno: EFAULT, msg: format!( @@ -1142,31 +1148,6 @@ fn create_underlay_port( }, )?; - // // Get a mac client handle as well. - // // - // let oflags = MacOpenFlags::NONE; - // let mch = MacClientHandle::open(&mh, Some(mc_name), oflags, 0) - // .map(Arc::new) - // .map_err(|e| OpteError::System { - // errno: EFAULT, - // msg: format!("mac_client_open failed for {link_name}: {e}"), - // })?; - - // // Setup promiscuous callback to receive all packets on this link. - // // - // // We specify `MAC_PROMISC_FLAGS_NO_TX_LOOP` here to skip receiving copies - // // of outgoing packets we sent ourselves. - // let mph = mch - // .add_promisc( - // mac::mac_client_promisc_type_t::MAC_CLIENT_PROMISC_ALL, - // xde_rx, - // mac::MAC_PROMISC_FLAGS_NO_TX_LOOP, - // ) - // .map_err(|e| OpteError::System { - // errno: EFAULT, - // msg: format!("mac_promisc_add failed for {link_name}: {e}"), - // })?; - // Set up a unicast callback. The MAC address here is a sentinel value with // nothing real behind it. This is why we picked the zero value in the Oxide // OUI space for virtual MACs. The reason this is being done is that illumos @@ -1188,10 +1169,7 @@ fn create_underlay_port( Ok(xde_underlay_port { name: link_name, mac: mh.get_mac_addr(), - // mh, - // mch, mph, - // muh, link, stream, }) From b542fac9126c4fb752db2f3a2184e15396b34038 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 17 Jun 2024 13:25:25 +0100 Subject: [PATCH 04/30] Found the bug. --- xde/src/dls.rs | 5 ++++- xde/src/mac.rs | 2 -- xde/src/xde.rs | 19 ++++--------------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/xde/src/dls.rs b/xde/src/dls.rs index ec46d02c..dd7f9762 100644 --- a/xde/src/dls.rs +++ b/xde/src/dls.rs @@ -353,7 +353,7 @@ impl DlsLink { } pub fn open_stream( - &self, + mut self, mph: &MacPerimeterHandle, ) -> Result, c_int> { let Some(inner) = self.inner.as_ref() else { @@ -369,12 +369,15 @@ impl DlsLink { let res = unsafe { dls_open(inner.dlp, inner.dlh, stream.as_mut_ptr()) }; if res == 0 { + // DLP is held/consumed by dls_open. + _ = self.inner.take(); let dld_str = unsafe { stream.assume_init() }; Ok(DldStream { inner: Some(DldStreamInner { dld_str, link: mph.linkid() }), } .into()) } else { + self.release(mph); Err(res) } } diff --git a/xde/src/mac.rs b/xde/src/mac.rs index b562d2c6..709c7fcb 100644 --- a/xde/src/mac.rs +++ b/xde/src/mac.rs @@ -8,8 +8,6 @@ //! //! NOTE: This module is re-exporting all of the sys definitions at //! the moment out of laziness. -use crate::dls::DldStreamInner; - pub use super::mac_sys::*; use alloc::ffi::CString; use alloc::string::String; diff --git a/xde/src/xde.rs b/xde/src/xde.rs index eecccd99..b5fc503a 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -21,11 +21,9 @@ use crate::mac::mac_getinfo; use crate::mac::mac_private_minor; use crate::mac::MacClientHandle; use crate::mac::MacHandle; -use crate::mac::MacOpenFlags; use crate::mac::MacPerimeterHandle; use crate::mac::MacPromiscHandle; use crate::mac::MacTxFlags; -use crate::mac::MacUnicastHandle; use crate::route::Route; use crate::route::RouteCache; use crate::route::RouteKey; @@ -215,19 +213,10 @@ pub struct xde_underlay_port { /// The MAC address associated with this underlay port. pub mac: [u8; 6], - /// MAC handle to the underlay link. - // mh: Arc, - - /// MAC client handle for tx/rx on the underlay link. - // mch: Arc, - - // /// MAC client handle for tx/rx on the underlay link. - // muh: MacUnicastHandle, /// MAC promiscuous handle for receiving packets on the underlay link. mph: MacPromiscHandle, - link: DlsLink, - + /// XXX stream: Arc, } @@ -988,8 +977,8 @@ fn clear_xde_underlay() -> Result { } }; + // XXX: is this the right tool? Think about ownership etc. Arc::try_unwrap(u.stream).unwrap().release(&mph); - u.link.release(&mph); } } @@ -1089,7 +1078,8 @@ unsafe extern "C" fn xde_attach( /// Setup underlay port atop the given link. fn create_underlay_port( link_name: String, - mc_name: &str, + // This parameter is likely to be used as part of the flows work. + _mc_name: &str, ) -> Result { let mut link_id = 0; let link_cstr = CString::new(link_name.as_str()).unwrap(); @@ -1170,7 +1160,6 @@ fn create_underlay_port( name: link_name, mac: mh.get_mac_addr(), mph, - link, stream, }) } From dff4a589924f661455b895368417524c643d3ade Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 17 Jun 2024 15:38:45 +0100 Subject: [PATCH 05/30] Reorganisation, cleanup. --- xde/src/dls.rs | 490 ----------------------------- xde/src/dls/mod.rs | 203 ++++++++++++ xde/src/dls/sys.rs | 180 +++++++++++ xde/src/lib.rs | 1 - xde/src/{mac.rs => mac/mod.rs} | 4 +- xde/src/{mac_sys.rs => mac/sys.rs} | 0 6 files changed, 386 insertions(+), 492 deletions(-) delete mode 100644 xde/src/dls.rs create mode 100644 xde/src/dls/mod.rs create mode 100644 xde/src/dls/sys.rs rename xde/src/{mac.rs => mac/mod.rs} (99%) rename xde/src/{mac_sys.rs => mac/sys.rs} (100%) diff --git a/xde/src/dls.rs b/xde/src/dls.rs deleted file mode 100644 index dd7f9762..00000000 --- a/xde/src/dls.rs +++ /dev/null @@ -1,490 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// 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 2024 Oxide Computer Company - -// stuff we need from dls - -use crate::ip::kcondvar_t; -use crate::ip::krwlock_t; -use crate::ip::list_node_t; -use crate::ip::major_t; -use crate::ip::minor_t; -use crate::ip::queue_t; -use crate::ip::t_uscalar_t; -use crate::mac; -use crate::mac::mac_client_handle; -use crate::mac::mac_client_promisc_type_t; -use crate::mac::mac_handle; -use crate::mac::mac_promisc_add; -use crate::mac::mac_promisc_handle; -use crate::mac::mac_rx_fn; -use crate::mac::MacPerimeterHandle; -use crate::mac::MacPromiscHandle; -use crate::mac::MacTxFlags; -use crate::mac_sys::mac_tx_cookie_t; -use crate::mac_sys::MAC_DROP_ON_NO_DESC; -use alloc::sync::Arc; -use core::ffi::c_void; -use core::mem::MaybeUninit; -use core::ptr; -use illumos_sys_hdrs::boolean_t; -use illumos_sys_hdrs::c_char; -use illumos_sys_hdrs::c_int; -use illumos_sys_hdrs::c_uint; -use illumos_sys_hdrs::datalink_id_t; -use illumos_sys_hdrs::kmutex_t; -use illumos_sys_hdrs::mblk_t; -use illumos_sys_hdrs::uintptr_t; -use illumos_sys_hdrs::zoneid_t; -use opte::engine::packet::Packet; -use opte::engine::packet::PacketState; - -extern "C" { - pub fn dls_devnet_create( - mh: *mut mac::mac_handle, - linkid: datalink_id_t, - zoneid: zoneid_t, - ) -> c_int; - - pub fn dls_devnet_destroy( - mh: *mut mac::mac_handle, - linkid: *mut datalink_id_t, - wait: boolean_t, - ) -> c_int; - - pub fn dls_mgmt_get_linkid( - name: *const c_char, - linkid: *mut datalink_id_t, - ) -> c_int; -} - -// Private DLS functions needed to get us a Tx path. -extern "C" { - pub type dls_devnet_s; - pub type dls_link; - - // ALL OF THESE REQUIRE THE MAC PERIMETER. - pub fn dls_devnet_hold_link( - link: datalink_id_t, - ddhp: *mut dls_dl_handle, - dlpp: *mut *mut dls_link, - ) -> c_int; - pub fn dls_devnet_rele_link(dlh: dls_dl_handle, dlp: *mut dls_link); - pub fn dls_open( - dlp: *mut dls_link, - ddh: dls_dl_handle, - dsp: *mut dld_str_s, - ) -> c_int; - pub fn dls_close(dsp: *mut dld_str_s); - - // THIS DOES NOT. - pub fn str_mdata_fastpath_put( - dsp: *const dld_str_s, - mp: *mut mblk_t, - f_hint: uintptr_t, - flag: u16, - ) -> mac_tx_cookie_t; -} - -// struct dld_str_s { /* Protected by */ -// /* -// * Major number of the device -// */ -// major_t ds_major; /* WO */ -// /* -// * Ephemeral minor number for the object. -// */ -// minor_t ds_minor; /* WO */ -// /* -// * PPA number this stream is attached to. -// */ -// t_uscalar_t ds_ppa; /* SL */ -// /* -// * Read/write queues for the stream which the object represents. -// */ -// queue_t *ds_rq; /* WO */ -// queue_t *ds_wq; /* WO */ -// /* -// * Stream is open to DLD_CONTROL (control node) or -// * DLD_DLPI (DLS provider) node. -// */ -// uint_t ds_type; /* WO */ -// /* -// * The following fields are only used for DLD_DLPI type objects. -// */ -// /* -// * Current DLPI state. -// */ -// t_uscalar_t ds_dlstate; /* SL */ -// /* -// * DLPI style -// */ -// t_uscalar_t ds_style; /* WO */ -// /* -// * Currently bound DLSAP. -// */ -// uint16_t ds_sap; /* SL */ -// /* -// * Handle of the MAC that is used by the data-link interface. -// */ -// mac_handle_t ds_mh; /* SL */ -// mac_client_handle_t ds_mch; /* SL */ -// /* -// * Promiscuity level information. -// */ -// uint32_t ds_promisc; /* SL */ -// mac_promisc_handle_t ds_mph; -// mac_promisc_handle_t ds_vlan_mph; - -// /* -// * Immutable information of the MAC which the channel is using. -// */ -// const mac_info_t *ds_mip; /* SL */ -// /* -// * Current packet priority. -// */ -// uint_t ds_pri; /* SL */ -// /* -// * Handle of our MAC notification callback. -// */ -// mac_notify_handle_t ds_mnh; /* SL */ -// /* -// * Set of enabled DL_NOTE... notifications. (See dlpi.h). -// */ -// uint32_t ds_notifications; /* SL */ -// /* -// * Mode: unitdata, fast-path or raw. -// */ -// dld_str_mode_t ds_mode; /* SL */ -// /* -// * Native mode state. -// */ -// boolean_t ds_native; /* SL */ -// /* -// * IP polling is operational if this flag is set. -// */ -// boolean_t ds_polling; /* SL */ -// boolean_t ds_direct; /* SL */ -// /* -// * LSO is enabled if ds_lso is set. -// */ -// boolean_t ds_lso; /* SL */ -// uint64_t ds_lso_max; /* SL */ -// /* -// * State of DLPI user: may be active (regular network layer), -// * passive (snoop-like monitoring), or unknown (not yet -// * determined). -// */ -// dld_passivestate_t ds_passivestate; /* SL */ -// /* -// * Dummy mblk used for flow-control. -// */ -// mblk_t *ds_tx_flow_mp; /* ds_lock */ -// /* -// * List of queued DLPI requests. These will be processed -// * by a taskq thread. This block is protected by ds_lock -// */ -// kmutex_t ds_lock; -// krwlock_t ds_rw_lock; -// kcondvar_t ds_datathr_cv; /* ds_lock */ -// uint_t ds_datathr_cnt; /* ds_lock */ -// mblk_t *ds_pending_head; /* ds_lock */ -// mblk_t *ds_pending_tail; /* ds_lock */ -// kcondvar_t ds_dlpi_pending_cv; /* ds_lock */ -// uint32_t -// ds_dlpi_pending : 1, /* ds_lock */ -// ds_local : 1, -// ds_pad : 30; /* ds_lock */ -// dls_link_t *ds_dlp; /* SL */ -// dls_multicst_addr_t *ds_dmap; /* ds_rw_lock */ -// dls_rx_t ds_rx; /* ds_lock */ -// void *ds_rx_arg; /* ds_lock */ -// uint_t ds_nactive; /* SL */ -// dld_str_t *ds_next; /* SL */ -// dls_head_t *ds_head; -// dls_dl_handle_t ds_ddh; -// list_node_t ds_tqlist; - -// /* -// * driver private data set by the driver when calling dld_str_open(). -// */ -// void *ds_private; - -// boolean_t ds_lowlink; /* SL */ -// boolean_t ds_nonip; /* SL */ -// }; - -#[repr(C)] -pub enum dld_str_mode_t { - DLD_UNITDATA, - DLD_FASTPATH, - DLD_RAW, -} - -#[repr(C)] -pub enum dld_passivestate_t { - DLD_UNINITIALIZED, - DLD_PASSIVE, - DLD_ACTIVE, -} - -#[repr(C)] -pub struct dld_str_s { - ds_major: major_t, - ds_minor: minor_t, - - ds_ppa: t_uscalar_t, - - ds_rq: *mut queue_t, - ds_wq: *mut queue_t, - - ds_type: c_uint, - - ds_dlstate: t_uscalar_t, - ds_style: t_uscalar_t, - ds_sap: u16, - - ds_mh: *mut mac_handle, - ds_mch: *mut mac_client_handle, - - ds_promisc: u32, - ds_mph: *mut mac_promisc_handle, - ds_vlan_mph: *mut mac_promisc_handle, - - ds_mip: *const c_void, // mac_info_t - - ds_pri: c_uint, - - ds_mnh: *mut c_void, // mac_notify_handle_t - - ds_notifications: u32, - - ds_mode: dld_str_mode_t, - - ds_native: boolean_t, - - ds_polling: boolean_t, - ds_direct: boolean_t, - - ds_lso: boolean_t, - ds_lso_max: u64, - - ds_passivestate: dld_passivestate_t, - - ds_tx_flow_mp: *mut mblk_t, - - /* - * List of queued DLPI requests. These will be processed - * by a taskq thread. This block is protected by ds_lock - */ - ds_lock: kmutex_t, - ds_rw_lock: krwlock_t, - ds_datathr_cv: kcondvar_t, - ds_datathr_cnt: c_uint, - ds_pending_head: *mut mblk_t, - ds_pending_tail: *mut mblk_t, - ds_dlpi_pending_cv: kcondvar_t, - // bitset: ds_dlpi_pending + ds_local + ds_pad - dl_dlpi_pend_local: u32, - - ds_dlp: *mut dls_link, - ds_dmap: *mut c_void, // dls_multicst_addr_t - ds_rx: *mut c_void, // dls_rx_t - ds_rx_arg: *mut c_void, - ds_nactive: c_uint, - ds_next: *mut dld_str_s, - ds_head: *mut c_void, // dls_head_t - ds_ddh: dls_dl_handle, - ds_tqlist: list_node_t, - - ds_private: *mut c_void, - - ds_lowlink: boolean_t, - ds_nonip: boolean_t, -} - -impl core::fmt::Debug for dld_str_s { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - write!(f, "") - } -} - -pub type dls_dl_handle = *mut dls_devnet_s; - -#[derive(Debug)] -pub struct DlsLink { - inner: Option, -} - -#[derive(Debug)] -pub struct DlsLinkInner { - dlp: *mut dls_link, - dlh: dls_dl_handle, - link: datalink_id_t, -} - -impl DlsLink { - pub fn hold(mph: &MacPerimeterHandle) -> Result { - let mut dlp = ptr::null_mut(); - let mut dlh = ptr::null_mut(); - let link = mph.linkid(); - - let res = unsafe { dls_devnet_hold_link(link, &mut dlh, &mut dlp) }; - if res == 0 { - Ok(Self { inner: Some(DlsLinkInner { dlp, dlh, link }) }) - } else { - Err(res) - } - } - - // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. - pub fn release(mut self, mph: &MacPerimeterHandle) { - if let Some(inner) = self.inner.take() { - if mph.linkid() != inner.link { - panic!("Tried to free link hold with the wrong MAC perimeter: saw {}, wanted {}", - mph.linkid(), inner.link); - } - unsafe { - dls_devnet_rele_link(inner.dlh, inner.dlp); - } - } - } - - pub fn open_stream( - mut self, - mph: &MacPerimeterHandle, - ) -> Result, c_int> { - let Some(inner) = self.inner.as_ref() else { - return Err(-1); - }; - - if mph.linkid() != inner.link { - panic!("Tried to open stream with the wrong MAC perimeter: saw {}, wanted {}", - mph.linkid(), inner.link); - } - - let mut stream = MaybeUninit::zeroed(); - let res = - unsafe { dls_open(inner.dlp, inner.dlh, stream.as_mut_ptr()) }; - if res == 0 { - // DLP is held/consumed by dls_open. - _ = self.inner.take(); - let dld_str = unsafe { stream.assume_init() }; - Ok(DldStream { - inner: Some(DldStreamInner { dld_str, link: mph.linkid() }), - } - .into()) - } else { - self.release(mph); - Err(res) - } - } -} - -impl Drop for DlsLink { - fn drop(&mut self) { - if let Some(inner) = self.inner.take() { - opte::engine::err!( - "dropped hold on link {} without releasing!!", - inner.link - ); - } - } -} - -#[derive(Debug)] -pub struct DldStream { - inner: Option, -} - -#[derive(Debug)] -pub struct DldStreamInner { - dld_str: dld_str_s, - link: datalink_id_t, -} - -impl DldStream { - /// Register promiscuous callback to receive packets on the underlying MAC. - pub fn add_promisc( - self: &Arc, - ptype: mac_client_promisc_type_t, - promisc_fn: mac_rx_fn, - flags: u16, - ) -> Result, c_int> { - let Some(inner) = self.inner.as_ref() else { - return Err(-1); - }; - - let mut mph = ptr::null_mut(); - - // `MacPromiscHandle` keeps a reference to this `MacClientHandle` - // until it is removed and so we can safely access it from the - // callback via the `arg` pointer. - let _parent = self.clone(); - let parent = Arc::into_raw(_parent) as *mut _; - let arg = parent as *mut c_void; - let mch = inner.dld_str.ds_mch; - let ret = unsafe { - // NOTE: arg is reinterpreted as `mac_resource_handle` -> `mac_ring` - // in `mac_rx_common`. Is what we've been doing here and before even safe? - mac_promisc_add(mch, ptype, promisc_fn, arg, &mut mph, flags) - }; - - if ret == 0 { - Ok(MacPromiscHandle { mph, parent }) - } else { - Err(ret) - } - } - - pub fn tx_drop_on_no_desc( - &self, - pkt: Packet, - hint: uintptr_t, - flags: MacTxFlags, - ) { - let Some(inner) = self.inner.as_ref() else { - // XXX: probably handle or signal an error here. - return; - }; - // We must unwrap the raw `mblk_t` out of the `pkt` here, - // otherwise the mblk_t would be dropped at the end of this - // function along with `pkt`. - let mut raw_flags = flags.bits(); - raw_flags |= MAC_DROP_ON_NO_DESC; - unsafe { - str_mdata_fastpath_put( - &inner.dld_str, - pkt.unwrap_mblk(), - hint, - raw_flags, - ) - }; - } - - // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. - pub fn release(mut self, mph: &MacPerimeterHandle) { - if let Some(mut inner) = self.inner.take() { - if mph.linkid() != inner.link { - opte::engine::err!("Tried to free link hold with the wrong MAC perimeter: saw {}, wanted {}", - mph.linkid(), inner.link); - } - unsafe { - dls_close(&mut inner.dld_str); - } - } - } -} - -impl Drop for DldStream { - fn drop(&mut self) { - if let Some(inner) = self.inner.take() { - opte::engine::err!( - "dropped stream on link {} without releasing!!", - inner.link - ); - } - } -} diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs new file mode 100644 index 00000000..454c8a20 --- /dev/null +++ b/xde/src/dls/mod.rs @@ -0,0 +1,203 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// 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 2024 Oxide Computer Company + +// stuff we need from dls + +pub mod sys; + +use crate::mac::mac_client_promisc_type_t; +use crate::mac::mac_promisc_add; +use crate::mac::mac_rx_fn; +use crate::mac::MacPerimeterHandle; +use crate::mac::MacPromiscHandle; +use crate::mac::MacTxFlags; +use crate::mac::MAC_DROP_ON_NO_DESC; +use alloc::sync::Arc; +use core::ffi::c_void; +use core::mem::MaybeUninit; +use core::ptr; +use illumos_sys_hdrs::c_int; +use illumos_sys_hdrs::datalink_id_t; +use illumos_sys_hdrs::uintptr_t; +use opte::engine::packet::Packet; +use opte::engine::packet::PacketState; +pub use sys::*; + +#[derive(Debug)] +pub struct DlsLink { + inner: Option, +} + +#[derive(Debug)] +pub struct DlsLinkInner { + dlp: *mut dls_link, + dlh: dls_dl_handle, + link: datalink_id_t, +} + +impl DlsLink { + pub fn hold(mph: &MacPerimeterHandle) -> Result { + let mut dlp = ptr::null_mut(); + let mut dlh = ptr::null_mut(); + let link = mph.linkid(); + + let res = unsafe { dls_devnet_hold_link(link, &mut dlh, &mut dlp) }; + if res == 0 { + Ok(Self { inner: Some(DlsLinkInner { dlp, dlh, link }) }) + } else { + Err(res) + } + } + + // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. + pub fn release(mut self, mph: &MacPerimeterHandle) { + if let Some(inner) = self.inner.take() { + if mph.linkid() != inner.link { + panic!("Tried to free link hold with the wrong MAC perimeter: saw {}, wanted {}", + mph.linkid(), inner.link); + } + unsafe { + dls_devnet_rele_link(inner.dlh, inner.dlp); + } + } + } + + pub fn open_stream( + mut self, + mph: &MacPerimeterHandle, + ) -> Result, c_int> { + let Some(inner) = self.inner.as_ref() else { + return Err(-1); + }; + + if mph.linkid() != inner.link { + panic!("Tried to open stream with the wrong MAC perimeter: saw {}, wanted {}", + mph.linkid(), inner.link); + } + + let mut stream = MaybeUninit::zeroed(); + let res = + unsafe { dls_open(inner.dlp, inner.dlh, stream.as_mut_ptr()) }; + if res == 0 { + // DLP is held/consumed by dls_open. + _ = self.inner.take(); + let dld_str = unsafe { stream.assume_init() }; + Ok(DldStream { + inner: Some(DldStreamInner { dld_str, link: mph.linkid() }), + } + .into()) + } else { + self.release(mph); + Err(res) + } + } +} + +impl Drop for DlsLink { + fn drop(&mut self) { + if let Some(inner) = self.inner.take() { + opte::engine::err!( + "dropped hold on link {} without releasing!!", + inner.link + ); + } + } +} + +#[derive(Debug)] +pub struct DldStream { + inner: Option, +} + +#[derive(Debug)] +pub struct DldStreamInner { + dld_str: dld_str_s, + link: datalink_id_t, +} + +impl DldStream { + /// Register promiscuous callback to receive packets on the underlying MAC. + pub fn add_promisc( + self: &Arc, + ptype: mac_client_promisc_type_t, + promisc_fn: mac_rx_fn, + flags: u16, + ) -> Result, c_int> { + let Some(inner) = self.inner.as_ref() else { + return Err(-1); + }; + + let mut mph = ptr::null_mut(); + + // `MacPromiscHandle` keeps a reference to this `MacClientHandle` + // until it is removed and so we can safely access it from the + // callback via the `arg` pointer. + let _parent = self.clone(); + let parent = Arc::into_raw(_parent) as *mut _; + let arg = parent as *mut c_void; + let mch = inner.dld_str.ds_mch; + let ret = unsafe { + // NOTE: arg is reinterpreted as `mac_resource_handle` -> `mac_ring` + // in `mac_rx_common`. Is what we've been doing here and before even safe? + mac_promisc_add(mch, ptype, promisc_fn, arg, &mut mph, flags) + }; + + if ret == 0 { + Ok(MacPromiscHandle { mph, parent }) + } else { + Err(ret) + } + } + + pub fn tx_drop_on_no_desc( + &self, + pkt: Packet, + hint: uintptr_t, + flags: MacTxFlags, + ) { + let Some(inner) = self.inner.as_ref() else { + // XXX: probably handle or signal an error here. + return; + }; + // We must unwrap the raw `mblk_t` out of the `pkt` here, + // otherwise the mblk_t would be dropped at the end of this + // function along with `pkt`. + let mut raw_flags = flags.bits(); + raw_flags |= MAC_DROP_ON_NO_DESC; + unsafe { + str_mdata_fastpath_put( + &inner.dld_str, + pkt.unwrap_mblk(), + hint, + raw_flags, + ) + }; + } + + // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. + pub fn release(mut self, mph: &MacPerimeterHandle) { + if let Some(mut inner) = self.inner.take() { + if mph.linkid() != inner.link { + opte::engine::err!("Tried to free link hold with the wrong MAC perimeter: saw {}, wanted {}", + mph.linkid(), inner.link); + } + unsafe { + dls_close(&mut inner.dld_str); + } + } + } +} + +impl Drop for DldStream { + fn drop(&mut self) { + if let Some(inner) = self.inner.take() { + opte::engine::err!( + "dropped stream on link {} without releasing!!", + inner.link + ); + } + } +} diff --git a/xde/src/dls/sys.rs b/xde/src/dls/sys.rs new file mode 100644 index 00000000..17b38606 --- /dev/null +++ b/xde/src/dls/sys.rs @@ -0,0 +1,180 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// 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 2024 Oxide Computer Company + +// stuff we need from dls + +use crate::ip::kcondvar_t; +use crate::ip::krwlock_t; +use crate::ip::list_node_t; +use crate::ip::major_t; +use crate::ip::minor_t; +use crate::ip::queue_t; +use crate::ip::t_uscalar_t; +use crate::mac; +use crate::mac::mac_client_handle; +use crate::mac::mac_handle; +use crate::mac::mac_promisc_handle; +use crate::mac::mac_tx_cookie_t; +use core::ffi::c_void; +use illumos_sys_hdrs::boolean_t; +use illumos_sys_hdrs::c_char; +use illumos_sys_hdrs::c_int; +use illumos_sys_hdrs::c_uint; +use illumos_sys_hdrs::datalink_id_t; +use illumos_sys_hdrs::kmutex_t; +use illumos_sys_hdrs::mblk_t; +use illumos_sys_hdrs::uintptr_t; +use illumos_sys_hdrs::zoneid_t; + +extern "C" { + pub fn dls_devnet_create( + mh: *mut mac::mac_handle, + linkid: datalink_id_t, + zoneid: zoneid_t, + ) -> c_int; + + pub fn dls_devnet_destroy( + mh: *mut mac::mac_handle, + linkid: *mut datalink_id_t, + wait: boolean_t, + ) -> c_int; + + pub fn dls_mgmt_get_linkid( + name: *const c_char, + linkid: *mut datalink_id_t, + ) -> c_int; +} + +// Private DLS functions needed to have a Tx path on top of +// an existing link while circumventing `ip`. +extern "C" { + pub type dls_devnet_s; + pub type dls_link; + + /// Transmit a packet chain on a given link. + /// This is effectively one layer above mac_tx. + pub fn str_mdata_fastpath_put( + dsp: *const dld_str_s, + mp: *mut mblk_t, + f_hint: uintptr_t, + flag: u16, + ) -> mac_tx_cookie_t; + + // NOTE: ALL BELOW FUNCTIONS REQUIRE THE MAC PERIMETER TO BE HELD. + pub fn dls_devnet_hold_link( + link: datalink_id_t, + ddhp: *mut dls_dl_handle, + dlpp: *mut *mut dls_link, + ) -> c_int; + + pub fn dls_devnet_rele_link(dlh: dls_dl_handle, dlp: *mut dls_link); + + pub fn dls_open( + dlp: *mut dls_link, + ddh: dls_dl_handle, + dsp: *mut dld_str_s, + ) -> c_int; + + pub fn dls_close(dsp: *mut dld_str_s); +} + +#[repr(C)] +pub enum dld_str_mode_t { + DLD_UNITDATA, + DLD_FASTPATH, + DLD_RAW, +} + +#[repr(C)] +pub enum dld_passivestate_t { + DLD_UNINITIALIZED, + DLD_PASSIVE, + DLD_ACTIVE, +} + +// +#[repr(C)] +pub struct dld_str_s { + ds_major: major_t, + ds_minor: minor_t, + + ds_ppa: t_uscalar_t, + + ds_rq: *mut queue_t, + ds_wq: *mut queue_t, + + ds_type: c_uint, + + ds_dlstate: t_uscalar_t, + ds_style: t_uscalar_t, + ds_sap: u16, + + ds_mh: *mut mac_handle, + pub ds_mch: *mut mac_client_handle, + + ds_promisc: u32, + ds_mph: *mut mac_promisc_handle, + ds_vlan_mph: *mut mac_promisc_handle, + + ds_mip: *const c_void, // mac_info_t + + ds_pri: c_uint, + + ds_mnh: *mut c_void, // mac_notify_handle_t + + ds_notifications: u32, + + ds_mode: dld_str_mode_t, + + ds_native: boolean_t, + + ds_polling: boolean_t, + ds_direct: boolean_t, + + ds_lso: boolean_t, + ds_lso_max: u64, + + ds_passivestate: dld_passivestate_t, + + ds_tx_flow_mp: *mut mblk_t, + + /* + * List of queued DLPI requests. These will be processed + * by a taskq thread. This block is protected by ds_lock + */ + ds_lock: kmutex_t, + ds_rw_lock: krwlock_t, + ds_datathr_cv: kcondvar_t, + ds_datathr_cnt: c_uint, + ds_pending_head: *mut mblk_t, + ds_pending_tail: *mut mblk_t, + ds_dlpi_pending_cv: kcondvar_t, + // bitset: ds_dlpi_pending + ds_local + ds_pad + dl_dlpi_pend_local: u32, + + ds_dlp: *mut dls_link, + ds_dmap: *mut c_void, // dls_multicst_addr_t + ds_rx: *mut c_void, // dls_rx_t + ds_rx_arg: *mut c_void, + ds_nactive: c_uint, + ds_next: *mut dld_str_s, + ds_head: *mut c_void, // dls_head_t + ds_ddh: dls_dl_handle, + ds_tqlist: list_node_t, + + ds_private: *mut c_void, + + ds_lowlink: boolean_t, + ds_nonip: boolean_t, +} + +impl core::fmt::Debug for dld_str_s { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "") + } +} + +pub type dls_dl_handle = *mut dls_devnet_s; diff --git a/xde/src/lib.rs b/xde/src/lib.rs index 726f1ef4..92c78301 100644 --- a/xde/src/lib.rs +++ b/xde/src/lib.rs @@ -43,7 +43,6 @@ use illumos_sys_hdrs::KM_SLEEP; pub mod dls; pub mod ip; pub mod mac; -mod mac_sys; pub mod route; pub mod secpolicy; pub mod sys; diff --git a/xde/src/mac.rs b/xde/src/mac/mod.rs similarity index 99% rename from xde/src/mac.rs rename to xde/src/mac/mod.rs index 709c7fcb..655968a6 100644 --- a/xde/src/mac.rs +++ b/xde/src/mac/mod.rs @@ -8,7 +8,8 @@ //! //! NOTE: This module is re-exporting all of the sys definitions at //! the moment out of laziness. -pub use super::mac_sys::*; +pub mod sys; + use alloc::ffi::CString; use alloc::string::String; use alloc::string::ToString; @@ -22,6 +23,7 @@ use opte::engine::ether::EtherAddr; use opte::engine::packet::Initialized; use opte::engine::packet::Packet; use opte::engine::packet::PacketState; +pub use sys::*; /// Errors while opening a MAC handle. #[derive(Debug)] diff --git a/xde/src/mac_sys.rs b/xde/src/mac/sys.rs similarity index 100% rename from xde/src/mac_sys.rs rename to xde/src/mac/sys.rs From 360955cb0bc4e6c75d577055b5cc4a9d253ec146 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 17 Jun 2024 15:50:57 +0100 Subject: [PATCH 06/30] Clippy. --- xde/src/dls/mod.rs | 2 +- xde/src/xde.rs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index 454c8a20..4e25b663 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -4,7 +4,7 @@ // Copyright 2024 Oxide Computer Company -// stuff we need from dls +//! Safe abstractions around DLS public and private functions. pub mod sys; diff --git a/xde/src/xde.rs b/xde/src/xde.rs index b5fc503a..2c961168 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -956,8 +956,7 @@ fn clear_xde_underlay() -> Result { msg: format!( "failed to get linkid for {}: {err}", u.name - ) - .into(), + ), }); } } @@ -971,8 +970,7 @@ fn clear_xde_underlay() -> Result { msg: format!( "failed to grab MAC perimeter for {}: {err}", u.name - ) - .into(), + ), }); } }; From 1da884ecbf1e8aecdbac703313ae3931c4a304be Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 17 Jun 2024 17:31:35 +0100 Subject: [PATCH 07/30] Refactoring, newtype for DLS Link IDs --- xde/src/dls/mod.rs | 148 +++++++++++++++++++++++++++------------------ xde/src/mac/mod.rs | 89 ++++++++++++++------------- xde/src/xde.rs | 78 +++++++++++------------- 3 files changed, 171 insertions(+), 144 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index 4e25b663..d6248dfe 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -8,56 +8,103 @@ pub mod sys; -use crate::mac::mac_client_promisc_type_t; -use crate::mac::mac_promisc_add; -use crate::mac::mac_rx_fn; +use crate::mac::mac_client_handle; +use crate::mac::MacClient; use crate::mac::MacPerimeterHandle; -use crate::mac::MacPromiscHandle; use crate::mac::MacTxFlags; use crate::mac::MAC_DROP_ON_NO_DESC; use alloc::sync::Arc; -use core::ffi::c_void; +use core::ffi::CStr; +use core::fmt::Display; use core::mem::MaybeUninit; use core::ptr; use illumos_sys_hdrs::c_int; use illumos_sys_hdrs::datalink_id_t; use illumos_sys_hdrs::uintptr_t; +use illumos_sys_hdrs::ENOENT; use opte::engine::packet::Packet; use opte::engine::packet::PacketState; pub use sys::*; +/// An integer ID used by DLS to refer to a given link. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct LinkId(u32); + +impl LinkId { + /// Request the link ID for a device using its name. + pub fn from_name(name: impl AsRef) -> Result { + let mut link_id = 0; + + unsafe { + match dls_mgmt_get_linkid(name.as_ref().as_ptr(), &mut link_id) { + 0 => Ok(LinkId(link_id)), + ENOENT => Err(LinkError::NotFound), + err => Err(LinkError::Other(err)), + } + } + } +} + +impl From for datalink_id_t { + fn from(val: LinkId) -> Self { + val.0 + } +} + +/// Errors encountered while querying DLS for a `LinkId`. +pub enum LinkError { + NotFound, + Other(i32), +} + +impl Display for LinkError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + LinkError::NotFound => write!(f, "link not found"), + LinkError::Other(e) => write!(f, "unknown error ({e})"), + } + } +} + +/// A hold on an existing link managed by DLS. #[derive(Debug)] pub struct DlsLink { inner: Option, + link: LinkId, } #[derive(Debug)] -pub struct DlsLinkInner { +struct DlsLinkInner { dlp: *mut dls_link, dlh: dls_dl_handle, - link: datalink_id_t, } impl DlsLink { + /// Place a hold on an existing link, pub fn hold(mph: &MacPerimeterHandle) -> Result { let mut dlp = ptr::null_mut(); let mut dlh = ptr::null_mut(); - let link = mph.linkid(); + let link = mph.link_id(); - let res = unsafe { dls_devnet_hold_link(link, &mut dlh, &mut dlp) }; + let res = + unsafe { dls_devnet_hold_link(link.into(), &mut dlh, &mut dlp) }; if res == 0 { - Ok(Self { inner: Some(DlsLinkInner { dlp, dlh, link }) }) + Ok(Self { inner: Some(DlsLinkInner { dlp, dlh }), link }) } else { Err(res) } } + pub fn link_id(&self) -> LinkId { + self.link + } + // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. pub fn release(mut self, mph: &MacPerimeterHandle) { if let Some(inner) = self.inner.take() { - if mph.linkid() != inner.link { - panic!("Tried to free link hold with the wrong MAC perimeter: saw {}, wanted {}", - mph.linkid(), inner.link); + if mph.link_id() != self.link { + panic!("Tried to free link hold with the wrong MAC perimeter: saw {:?}, wanted {:?}", + mph.link_id(),self.link); } unsafe { dls_devnet_rele_link(inner.dlh, inner.dlp); @@ -73,9 +120,9 @@ impl DlsLink { return Err(-1); }; - if mph.linkid() != inner.link { - panic!("Tried to open stream with the wrong MAC perimeter: saw {}, wanted {}", - mph.linkid(), inner.link); + if mph.link_id() != self.link { + panic!("Tried to open stream with the wrong MAC perimeter: saw {:?}, wanted {:?}", + mph.link_id(),self.link); } let mut stream = MaybeUninit::zeroed(); @@ -86,7 +133,8 @@ impl DlsLink { _ = self.inner.take(); let dld_str = unsafe { stream.assume_init() }; Ok(DldStream { - inner: Some(DldStreamInner { dld_str, link: mph.linkid() }), + inner: Some(DldStreamInner { dld_str }), + link: mph.link_id(), } .into()) } else { @@ -98,58 +146,30 @@ impl DlsLink { impl Drop for DlsLink { fn drop(&mut self) { - if let Some(inner) = self.inner.take() { + if self.inner.take().is_some() { opte::engine::err!( - "dropped hold on link {} without releasing!!", - inner.link + "dropped hold on link {:?} without releasing!!", + self.link ); } } } +/// A DLS message stream derived from a #[derive(Debug)] pub struct DldStream { inner: Option, + link: LinkId, } #[derive(Debug)] -pub struct DldStreamInner { +struct DldStreamInner { dld_str: dld_str_s, - link: datalink_id_t, } impl DldStream { - /// Register promiscuous callback to receive packets on the underlying MAC. - pub fn add_promisc( - self: &Arc, - ptype: mac_client_promisc_type_t, - promisc_fn: mac_rx_fn, - flags: u16, - ) -> Result, c_int> { - let Some(inner) = self.inner.as_ref() else { - return Err(-1); - }; - - let mut mph = ptr::null_mut(); - - // `MacPromiscHandle` keeps a reference to this `MacClientHandle` - // until it is removed and so we can safely access it from the - // callback via the `arg` pointer. - let _parent = self.clone(); - let parent = Arc::into_raw(_parent) as *mut _; - let arg = parent as *mut c_void; - let mch = inner.dld_str.ds_mch; - let ret = unsafe { - // NOTE: arg is reinterpreted as `mac_resource_handle` -> `mac_ring` - // in `mac_rx_common`. Is what we've been doing here and before even safe? - mac_promisc_add(mch, ptype, promisc_fn, arg, &mut mph, flags) - }; - - if ret == 0 { - Ok(MacPromiscHandle { mph, parent }) - } else { - Err(ret) - } + pub fn link_id(&self) -> LinkId { + self.link } pub fn tx_drop_on_no_desc( @@ -180,9 +200,9 @@ impl DldStream { // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. pub fn release(mut self, mph: &MacPerimeterHandle) { if let Some(mut inner) = self.inner.take() { - if mph.linkid() != inner.link { - opte::engine::err!("Tried to free link hold with the wrong MAC perimeter: saw {}, wanted {}", - mph.linkid(), inner.link); + if mph.link_id() != self.link { + opte::engine::err!("Tried to free link hold with the wrong MAC perimeter: saw {:?}, wanted {:?}", + mph.link_id(), self.link); } unsafe { dls_close(&mut inner.dld_str); @@ -191,12 +211,22 @@ impl DldStream { } } +impl MacClient for DldStream { + fn mac_client_handle(&self) -> Result<*mut mac_client_handle, c_int> { + let Some(inner) = self.inner.as_ref() else { + return Err(-1); + }; + + Ok(inner.dld_str.ds_mch) + } +} + impl Drop for DldStream { fn drop(&mut self) { - if let Some(inner) = self.inner.take() { + if self.inner.take().is_some() { opte::engine::err!( - "dropped stream on link {} without releasing!!", - inner.link + "dropped stream on link {:?} without releasing!!", + self.link, ); } } diff --git a/xde/src/mac/mod.rs b/xde/src/mac/mod.rs index 655968a6..ad3a79c2 100644 --- a/xde/src/mac/mod.rs +++ b/xde/src/mac/mod.rs @@ -10,6 +10,7 @@ //! the moment out of laziness. pub mod sys; +use crate::dls::LinkId; use alloc::ffi::CString; use alloc::string::String; use alloc::string::ToString; @@ -198,37 +199,6 @@ impl MacClientHandle { } } - /// Register promiscuous callback to receive packets on the underlying MAC. - pub fn add_promisc( - self: &Arc, - ptype: mac_client_promisc_type_t, - promisc_fn: mac_rx_fn, - flags: u16, - ) -> Result, c_int> { - let mut mph = ptr::null_mut(); - - // `MacPromiscHandle` keeps a reference to this `MacClientHandle` - // until it is removed and so we can safely access it from the - // callback via the `arg` pointer. - let mch = Arc::into_raw(self.clone()); - let ret = unsafe { - mac_promisc_add( - self.mch, - ptype, - promisc_fn, - mch as *mut c_void, - &mut mph, - flags, - ) - }; - - if ret == 0 { - Ok(MacPromiscHandle { mph, parent: mch }) - } else { - Err(ret) - } - } - /// Send the [`Packet`] on this client. /// /// If the packet cannot be sent, return it. If you want to drop @@ -300,15 +270,52 @@ impl Drop for MacClientHandle { } } +pub trait MacClient { + fn mac_client_handle(&self) -> Result<*mut mac_client_handle, c_int>; +} + +impl MacClient for MacClientHandle { + fn mac_client_handle(&self) -> Result<*mut mac_client_handle, c_int> { + Ok(self.mch) + } +} + /// Safe wrapper around a `mac_promisc_handle_t`. #[derive(Debug)] pub struct MacPromiscHandle

{ /// The underlying `mac_promisc_handle_t`. - pub(crate) mph: *mut mac_promisc_handle, + mph: *mut mac_promisc_handle, + + /// The parent used to create this promiscuous callback. + parent: *const P, +} + +impl MacPromiscHandle

{ + /// Register a promiscuous callback to receive packets on the underlying MAC. + pub fn new( + parent: Arc

, + ptype: mac_client_promisc_type_t, + promisc_fn: mac_rx_fn, + flags: u16, + ) -> Result, c_int> { + let mut mph = ptr::null_mut(); + let mch = parent.mac_client_handle()?; + let parent = Arc::into_raw(parent); + let arg = parent as *mut c_void; + + // SAFETY: `MacPromiscHandle` keeps a reference to this `P` + // until it is removed and so we can safely access it from the + // callback via the `arg` pointer. + let ret = unsafe { + mac_promisc_add(mch, ptype, promisc_fn, arg, &mut mph, flags) + }; - /// The `MacClientHandle` used to create this promiscuous callback. - /// MUST BE A RAW ARC. - pub(crate) parent: *const P, + if ret == 0 { + Ok(Self { mph, parent }) + } else { + Err(ret) + } + } } impl

Drop for MacPromiscHandle

{ @@ -342,18 +349,17 @@ impl Drop for MacUnicastHandle { } } -// XXX: cleanup - /// Safe wrapper around a `mac_perim_handle_t`. pub struct MacPerimeterHandle { mph: mac_perim_handle, - link: datalink_id_t, + link: LinkId, } impl MacPerimeterHandle { - pub fn from_linkid(link: datalink_id_t) -> Result { + /// Attempt to acquire the MAC perimeter for a given link. + pub fn from_linkid(link: LinkId) -> Result { let mut mph = 0; - let res = unsafe { mac_perim_enter_by_linkid(link, &mut mph) }; + let res = unsafe { mac_perim_enter_by_linkid(link.into(), &mut mph) }; if res == 0 { Ok(Self { mph, link }) } else { @@ -361,7 +367,8 @@ impl MacPerimeterHandle { } } - pub fn linkid(&self) -> datalink_id_t { + /// Returns the ID of the link whose MAC perimeter is held. + pub fn link_id(&self) -> LinkId { self.link } } diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 2c961168..4920dd69 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -15,6 +15,7 @@ use crate::dls; use crate::dls::DldStream; use crate::dls::DlsLink; +use crate::dls::LinkId; use crate::ioctl::IoctlEnvelope; use crate::mac; use crate::mac::mac_getinfo; @@ -944,25 +945,11 @@ fn clear_xde_underlay() -> Result { // walkers finishing. Accordingly, no one else will have or try to // clone the MAC client handle. - let mut link_id = 0; - let link_cstr = CString::new(u.name.as_str()).unwrap(); - unsafe { - match dls::dls_mgmt_get_linkid(link_cstr.as_ptr(), &mut link_id) - { - 0 => {} - err => { - return Err(OpteError::System { - errno: EBUSY, - msg: format!( - "failed to get linkid for {}: {err}", - u.name - ), - }); - } - } - } - - let mph = match MacPerimeterHandle::from_linkid(link_id) { + // 2. Close the open stream handle. + // This requires that we take the MAC perimeter on this device, which + // will be dropped at the end of this scope. + let mph = match MacPerimeterHandle::from_linkid(u.stream.link_id()) + { Ok(mph) => mph, Err(err) => { return Err(OpteError::System { @@ -975,8 +962,18 @@ fn clear_xde_underlay() -> Result { } }; - // XXX: is this the right tool? Think about ownership etc. - Arc::try_unwrap(u.stream).unwrap().release(&mph); + match Arc::into_inner(u.stream) { + Some(stream) => stream.release(&mph), + None => { + return Err(OpteError::System { + errno: EBUSY, + msg: format!( + "DLS stream for {} has outstanding refs", + u.name + ), + }) + } + } } } @@ -1079,19 +1076,13 @@ fn create_underlay_port( // This parameter is likely to be used as part of the flows work. _mc_name: &str, ) -> Result { - let mut link_id = 0; let link_cstr = CString::new(link_name.as_str()).unwrap(); - unsafe { - match dls::dls_mgmt_get_linkid(link_cstr.as_ptr(), &mut link_id) { - 0 => {} - err => { - return Err(OpteError::System { - errno: EFAULT, - msg: format!("failed to get linkid for {link_name}: {err}"), - }) - } - } - } + + let link_id = + LinkId::from_name(link_cstr).map_err(|err| OpteError::System { + errno: EFAULT, + msg: format!("failed to get linkid for {link_name}: {err}"), + })?; let mph = MacPerimeterHandle::from_linkid(link_id).map_err(|e| { OpteError::System { @@ -1116,17 +1107,16 @@ fn create_underlay_port( drop(mph); // Promisc setup. - - let mph = stream - .add_promisc( - mac::mac_client_promisc_type_t::MAC_CLIENT_PROMISC_ALL, - xde_rx, - mac::MAC_PROMISC_FLAGS_NO_TX_LOOP, - ) - .map_err(|e| OpteError::System { - errno: EFAULT, - msg: format!("mac_promisc_add failed for {link_name}: {e}"), - })?; + let mph = MacPromiscHandle::new( + stream.clone(), + mac::mac_client_promisc_type_t::MAC_CLIENT_PROMISC_ALL, + xde_rx, + mac::MAC_PROMISC_FLAGS_NO_TX_LOOP, + ) + .map_err(|e| OpteError::System { + errno: EFAULT, + msg: format!("mac_promisc_add failed for {link_name}: {e}"), + })?; // Grab mac handle for underlying link let mh = MacHandle::open_by_link_name(&link_name).map(Arc::new).map_err( From 54182c9876434824bc692d680b52bb046cb415b6 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 17 Jun 2024 17:54:56 +0100 Subject: [PATCH 08/30] Comment tweaks etc. --- xde/src/xde.rs | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 4920dd69..15a3a8ff 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -217,7 +217,8 @@ pub struct xde_underlay_port { /// MAC promiscuous handle for receiving packets on the underlay link. mph: MacPromiscHandle, - /// XXX + /// DLS-level handle on a device for promiscuous registration and + /// packet Tx. stream: Arc, } @@ -962,6 +963,7 @@ fn clear_xde_underlay() -> Result { } }; + // Finally, we can clean up the TX stream on the underlay device. match Arc::into_inner(u.stream) { Some(stream) => stream.release(&mph), None => { @@ -1091,7 +1093,6 @@ fn create_underlay_port( } })?; - // XXX: error handling will be REALLY bad here with these panic drops. let link = DlsLink::hold(&mph).map_err(|e| OpteError::System { errno: EFAULT, msg: format!( @@ -1106,7 +1107,10 @@ fn create_underlay_port( drop(mph); - // Promisc setup. + // Setup promiscuous callback to receive all packets on this link. + // + // We specify `MAC_PROMISC_FLAGS_NO_TX_LOOP` here to skip receiving copies + // of outgoing packets we sent ourselves. let mph = MacPromiscHandle::new( stream.clone(), mac::mac_client_promisc_type_t::MAC_CLIENT_PROMISC_ALL, @@ -1118,7 +1122,7 @@ fn create_underlay_port( msg: format!("mac_promisc_add failed for {link_name}: {e}"), })?; - // Grab mac handle for underlying link + // Grab mac handle for underlying link, to retrieve its MAC address. let mh = MacHandle::open_by_link_name(&link_name).map(Arc::new).map_err( |e| OpteError::System { errno: EFAULT, @@ -1126,24 +1130,6 @@ fn create_underlay_port( }, )?; - // Set up a unicast callback. The MAC address here is a sentinel value with - // nothing real behind it. This is why we picked the zero value in the Oxide - // OUI space for virtual MACs. The reason this is being done is that illumos - // requires that if there is a single mac client on a link, that client must - // have an L2 address. This was not caught until recently, because this is - // only enforced as a debug assert in the kernel. - - // ks: - // ON ABOVE: we are never the only MAC client on a link. If we somehow are, we - // should find a way to enfore the above constraint. - // (even so, the presence of a dls-visible link implies a client?) - - // let mac = EtherAddr::from([0xa8, 0x40, 0x25, 0xff, 0x00, 0x00]); - // let muh = mch.add_unicast(mac).map_err(|e| OpteError::System { - // errno: EFAULT, - // msg: format!("mac_unicast_add failed for {link_name}: {e}"), - // })?; - Ok(xde_underlay_port { name: link_name, mac: mh.get_mac_addr(), @@ -1606,7 +1592,6 @@ unsafe fn xde_mc_tx_one( // Choose u1 as a starting point. This may be changed in the next_hop // function when we are actually able to determine what interface should be // used. - // let mch = &src_dev.u1.mch; let stream = &src_dev.u1.stream; let hint = 0; @@ -1692,11 +1677,6 @@ unsafe fn xde_mc_tx_one( // Unwrap: We know the packet is good because we just // unwrapped it above. let new_pkt = Packet::::wrap_mblk(mblk).unwrap(); - // underlay_dev.mch.tx_drop_on_no_desc( - // new_pkt, - // hint, - // MacTxFlags::empty(), - // ); underlay_dev.stream.tx_drop_on_no_desc( new_pkt, hint, From bb25fdefe546ee92f7a40932399ca4e4b4501fcd Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 17 Jun 2024 18:04:51 +0100 Subject: [PATCH 09/30] More commentary. --- xde/src/dls/mod.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index d6248dfe..15f3b414 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -80,7 +80,7 @@ struct DlsLinkInner { } impl DlsLink { - /// Place a hold on an existing link, + /// Place a hold on an existing link. pub fn hold(mph: &MacPerimeterHandle) -> Result { let mut dlp = ptr::null_mut(); let mut dlh = ptr::null_mut(); @@ -95,11 +95,15 @@ impl DlsLink { } } + /// Returns the ID of the link which is held. pub fn link_id(&self) -> LinkId { self.link } - // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. + /// Release a hold on a given link. + /// + /// This operation requires that you acquire the MAC perimeter + /// for the target device. pub fn release(mut self, mph: &MacPerimeterHandle) { if let Some(inner) = self.inner.take() { if mph.link_id() != self.link { @@ -112,6 +116,7 @@ impl DlsLink { } } + /// Convert a hold into a `DldStream` for packet Rx/Tx. pub fn open_stream( mut self, mph: &MacPerimeterHandle, @@ -155,7 +160,8 @@ impl Drop for DlsLink { } } -/// A DLS message stream derived from a +/// A DLS message stream on a target link, allowing packet +/// Rx and Tx. #[derive(Debug)] pub struct DldStream { inner: Option, @@ -168,10 +174,18 @@ struct DldStreamInner { } impl DldStream { + /// Returns the ID of the link this stream belongs to. pub fn link_id(&self) -> LinkId { self.link } + /// Send the [`Packet`] on this client, dropping if there is no + /// descriptor available + /// + /// This function always consumes the [`Packet`]. + /// + /// XXX The underlying mac_tx() function accepts a packet chain, + /// but for now we pass only a single packet at a time. pub fn tx_drop_on_no_desc( &self, pkt: Packet, @@ -197,7 +211,10 @@ impl DldStream { }; } - // XXX: cleanup REQUIRES that we hold the MAC perimeter handle. + /// Close this stream. + /// + /// This operation requires that you acquire the MAC perimeter + /// for the parent device. pub fn release(mut self, mph: &MacPerimeterHandle) { if let Some(mut inner) = self.inner.take() { if mph.link_id() != self.link { From 3dc09de0591a1b2c0e78ce352a6a4ed632f6e452 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 19 Jun 2024 12:39:53 +0100 Subject: [PATCH 10/30] Should now have the correct attach/detach refs I'm going to try coming/going via dld_open and related, though we'll see if that works in quite the same way. --- xde-tests/tests/loopback.rs | 2 +- xde/src/dls/mod.rs | 15 +++++++++++++-- xde/src/dls/sys.rs | 16 +++++++++++++++- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/xde-tests/tests/loopback.rs b/xde-tests/tests/loopback.rs index fa01230a..a0284807 100644 --- a/xde-tests/tests/loopback.rs +++ b/xde-tests/tests/loopback.rs @@ -11,7 +11,7 @@ fn test_xde_loopback() -> Result<()> { let topol = xde_tests::two_node_topology()?; // Now we should be able to ping b from a on the overlay. - &topol.nodes[0] + _ = &topol.nodes[0] .zone .zone .zexec(&format!("ping {}", &topol.nodes[1].port.ip()))?; diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index 15f3b414..c8f536e9 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -86,11 +86,16 @@ impl DlsLink { let mut dlh = ptr::null_mut(); let link = mph.link_id(); - let res = - unsafe { dls_devnet_hold_link(link.into(), &mut dlh, &mut dlp) }; + let res = unsafe { dls_devnet_hold(link.into(), &mut dlh) }; + if res != 0 { + return Err(res); + } + + let res = unsafe { dls_link_hold(dls_devnet_mac(dlh), &mut dlp) }; if res == 0 { Ok(Self { inner: Some(DlsLinkInner { dlp, dlh }), link }) } else { + unsafe { dls_devnet_rele(dlh) }; Err(res) } } @@ -222,7 +227,13 @@ impl DldStream { mph.link_id(), self.link); } unsafe { + // NOTE: this is reimplementing dld_str_detach + // but we're avoiding capab negotiation/disable and + // mac notify callbacks. Should we just come in through + // dld_open/dld_close/dld_wput? That would make it a bit + // weirder to set the promisc handle. dls_close(&mut inner.dld_str); + dls_devnet_rele(inner.dld_str.ds_ddh) } } } diff --git a/xde/src/dls/sys.rs b/xde/src/dls/sys.rs index 17b38606..8442403c 100644 --- a/xde/src/dls/sys.rs +++ b/xde/src/dls/sys.rs @@ -72,6 +72,20 @@ extern "C" { pub fn dls_devnet_rele_link(dlh: dls_dl_handle, dlp: *mut dls_link); + pub fn dls_devnet_hold( + link: datalink_id_t, + ddhp: *mut dls_dl_handle, + ) -> c_int; + + pub fn dls_devnet_rele(dlh: dls_dl_handle); + + pub fn dls_link_hold( + name: *const c_char, + dlpp: *mut *mut dls_link, + ) -> c_int; + + pub fn dls_devnet_mac(dlh: dls_dl_handle) -> *const c_char; + pub fn dls_open( dlp: *mut dls_link, ddh: dls_dl_handle, @@ -162,7 +176,7 @@ pub struct dld_str_s { ds_nactive: c_uint, ds_next: *mut dld_str_s, ds_head: *mut c_void, // dls_head_t - ds_ddh: dls_dl_handle, + pub ds_ddh: dls_dl_handle, ds_tqlist: list_node_t, ds_private: *mut c_void, From c84ec5ac33c4f6726586b1caf46d58260e554e60 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 19 Jun 2024 16:50:03 +0100 Subject: [PATCH 11/30] Simplify creation/cleanup. --- xde/src/dls/mod.rs | 85 +++++++++++++++++++++------------------------- xde/src/xde.rs | 50 ++------------------------- 2 files changed, 41 insertions(+), 94 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index c8f536e9..75e67146 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -14,6 +14,7 @@ use crate::mac::MacPerimeterHandle; use crate::mac::MacTxFlags; use crate::mac::MAC_DROP_ON_NO_DESC; use alloc::sync::Arc; +use illumos_sys_hdrs::ENOENT; use core::ffi::CStr; use core::fmt::Display; use core::mem::MaybeUninit; @@ -21,7 +22,6 @@ use core::ptr; use illumos_sys_hdrs::c_int; use illumos_sys_hdrs::datalink_id_t; use illumos_sys_hdrs::uintptr_t; -use illumos_sys_hdrs::ENOENT; use opte::engine::packet::Packet; use opte::engine::packet::PacketState; pub use sys::*; @@ -36,11 +36,12 @@ impl LinkId { let mut link_id = 0; unsafe { - match dls_mgmt_get_linkid(name.as_ref().as_ptr(), &mut link_id) { - 0 => Ok(LinkId(link_id)), - ENOENT => Err(LinkError::NotFound), - err => Err(LinkError::Other(err)), - } + match dls_mgmt_get_linkid(name.as_ref().as_ptr(), &mut link_id) + { + 0 => Ok(LinkId(link_id)), + ENOENT => Err(LinkError::NotFound), + err => Err(LinkError::Other(err)), + } } } } @@ -54,7 +55,7 @@ impl From for datalink_id_t { /// Errors encountered while querying DLS for a `LinkId`. pub enum LinkError { NotFound, - Other(i32), + Other(i32) } impl Display for LinkError { @@ -68,7 +69,7 @@ impl Display for LinkError { /// A hold on an existing link managed by DLS. #[derive(Debug)] -pub struct DlsLink { +struct DlsLink { inner: Option, link: LinkId, } @@ -81,7 +82,7 @@ struct DlsLinkInner { impl DlsLink { /// Place a hold on an existing link. - pub fn hold(mph: &MacPerimeterHandle) -> Result { + fn hold(mph: &MacPerimeterHandle) -> Result { let mut dlp = ptr::null_mut(); let mut dlh = ptr::null_mut(); let link = mph.link_id(); @@ -100,18 +101,13 @@ impl DlsLink { } } - /// Returns the ID of the link which is held. - pub fn link_id(&self) -> LinkId { - self.link - } - /// Release a hold on a given link. /// /// This operation requires that you acquire the MAC perimeter /// for the target device. - pub fn release(mut self, mph: &MacPerimeterHandle) { + fn release(mut self, mph: &MacPerimeterHandle) { if let Some(inner) = self.inner.take() { - if mph.link_id() != self.link { + if mph.link_id() !=self.link { panic!("Tried to free link hold with the wrong MAC perimeter: saw {:?}, wanted {:?}", mph.link_id(),self.link); } @@ -122,7 +118,7 @@ impl DlsLink { } /// Convert a hold into a `DldStream` for packet Rx/Tx. - pub fn open_stream( + fn open_stream( mut self, mph: &MacPerimeterHandle, ) -> Result, c_int> { @@ -130,7 +126,7 @@ impl DlsLink { return Err(-1); }; - if mph.link_id() != self.link { + if mph.link_id() !=self.link { panic!("Tried to open stream with the wrong MAC perimeter: saw {:?}, wanted {:?}", mph.link_id(),self.link); } @@ -159,7 +155,7 @@ impl Drop for DlsLink { if self.inner.take().is_some() { opte::engine::err!( "dropped hold on link {:?} without releasing!!", - self.link + self.link ); } } @@ -179,6 +175,12 @@ struct DldStreamInner { } impl DldStream { + pub fn open(link_id: LinkId) -> Result, c_int> { + let perim = MacPerimeterHandle::from_linkid(link_id)?; + let link_handle = DlsLink::hold(&perim)?; + link_handle.open_stream(&perim) + } + /// Returns the ID of the link this stream belongs to. pub fn link_id(&self) -> LinkId { self.link @@ -215,28 +217,6 @@ impl DldStream { ) }; } - - /// Close this stream. - /// - /// This operation requires that you acquire the MAC perimeter - /// for the parent device. - pub fn release(mut self, mph: &MacPerimeterHandle) { - if let Some(mut inner) = self.inner.take() { - if mph.link_id() != self.link { - opte::engine::err!("Tried to free link hold with the wrong MAC perimeter: saw {:?}, wanted {:?}", - mph.link_id(), self.link); - } - unsafe { - // NOTE: this is reimplementing dld_str_detach - // but we're avoiding capab negotiation/disable and - // mac notify callbacks. Should we just come in through - // dld_open/dld_close/dld_wput? That would make it a bit - // weirder to set the promisc handle. - dls_close(&mut inner.dld_str); - dls_devnet_rele(inner.dld_str.ds_ddh) - } - } - } } impl MacClient for DldStream { @@ -251,11 +231,24 @@ impl MacClient for DldStream { impl Drop for DldStream { fn drop(&mut self) { - if self.inner.take().is_some() { - opte::engine::err!( - "dropped stream on link {:?} without releasing!!", - self.link, - ); + if let Some(mut inner) = self.inner.take() { + match MacPerimeterHandle::from_linkid(self.link) { + Ok(_perim) => unsafe { + // NOTE: this is reimplementing dld_str_detach + // but we're avoiding capab negotiation/disable and + // mac notify callbacks. Should we just come in through + // dld_open/dld_close/dld_wput? That would make it a bit + // weirder to set the promisc handle, and I don't know how + // this would interact with + dls_close(&mut inner.dld_str); + dls_devnet_rele(inner.dld_str.ds_ddh) + }, + Err(e) => opte::engine::err!( + "couldn't acquire MAC perimeter (err {}): \ + dropped stream on link {:?} without releasing", + e, self.link, + ), + } } } } diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 15a3a8ff..15120061 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -14,7 +14,6 @@ use crate::dls; use crate::dls::DldStream; -use crate::dls::DlsLink; use crate::dls::LinkId; use crate::ioctl::IoctlEnvelope; use crate::mac; @@ -22,7 +21,6 @@ use crate::mac::mac_getinfo; use crate::mac::mac_private_minor; use crate::mac::MacClientHandle; use crate::mac::MacHandle; -use crate::mac::MacPerimeterHandle; use crate::mac::MacPromiscHandle; use crate::mac::MacTxFlags; use crate::route::Route; @@ -947,35 +945,7 @@ fn clear_xde_underlay() -> Result { // clone the MAC client handle. // 2. Close the open stream handle. - // This requires that we take the MAC perimeter on this device, which - // will be dropped at the end of this scope. - let mph = match MacPerimeterHandle::from_linkid(u.stream.link_id()) - { - Ok(mph) => mph, - Err(err) => { - return Err(OpteError::System { - errno: EBUSY, - msg: format!( - "failed to grab MAC perimeter for {}: {err}", - u.name - ), - }); - } - }; - - // Finally, we can clean up the TX stream on the underlay device. - match Arc::into_inner(u.stream) { - Some(stream) => stream.release(&mph), - None => { - return Err(OpteError::System { - errno: EBUSY, - msg: format!( - "DLS stream for {} has outstanding refs", - u.name - ), - }) - } - } + _ = Arc::into_inner(u.stream); } } @@ -1086,27 +1056,11 @@ fn create_underlay_port( msg: format!("failed to get linkid for {link_name}: {err}"), })?; - let mph = MacPerimeterHandle::from_linkid(link_id).map_err(|e| { - OpteError::System { - errno: EFAULT, - msg: format!("failed to grab MAC perimeter for {link_name}: {e}"), - } - })?; - - let link = DlsLink::hold(&mph).map_err(|e| OpteError::System { - errno: EFAULT, - msg: format!( - "failed to grab hold on link perimeter for {link_name}: {e}" - ), - })?; - - let stream = link.open_stream(&mph).map_err(|e| OpteError::System { + let stream = DldStream::open(link_id).map_err(|e| OpteError::System { errno: EFAULT, msg: format!("failed to grab open stream for {link_name}: {e}"), })?; - drop(mph); - // Setup promiscuous callback to receive all packets on this link. // // We specify `MAC_PROMISC_FLAGS_NO_TX_LOOP` here to skip receiving copies From 7a4b068f0e6c269cb3c3aa756169f0d7806c1528 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 19 Jun 2024 16:57:38 +0100 Subject: [PATCH 12/30] Hmm. --- xde/src/dls/mod.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index 75e67146..4488c314 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -14,7 +14,6 @@ use crate::mac::MacPerimeterHandle; use crate::mac::MacTxFlags; use crate::mac::MAC_DROP_ON_NO_DESC; use alloc::sync::Arc; -use illumos_sys_hdrs::ENOENT; use core::ffi::CStr; use core::fmt::Display; use core::mem::MaybeUninit; @@ -22,6 +21,7 @@ use core::ptr; use illumos_sys_hdrs::c_int; use illumos_sys_hdrs::datalink_id_t; use illumos_sys_hdrs::uintptr_t; +use illumos_sys_hdrs::ENOENT; use opte::engine::packet::Packet; use opte::engine::packet::PacketState; pub use sys::*; @@ -36,12 +36,11 @@ impl LinkId { let mut link_id = 0; unsafe { - match dls_mgmt_get_linkid(name.as_ref().as_ptr(), &mut link_id) - { - 0 => Ok(LinkId(link_id)), - ENOENT => Err(LinkError::NotFound), - err => Err(LinkError::Other(err)), - } + match dls_mgmt_get_linkid(name.as_ref().as_ptr(), &mut link_id) { + 0 => Ok(LinkId(link_id)), + ENOENT => Err(LinkError::NotFound), + err => Err(LinkError::Other(err)), + } } } } @@ -55,7 +54,7 @@ impl From for datalink_id_t { /// Errors encountered while querying DLS for a `LinkId`. pub enum LinkError { NotFound, - Other(i32) + Other(i32), } impl Display for LinkError { @@ -107,7 +106,7 @@ impl DlsLink { /// for the target device. fn release(mut self, mph: &MacPerimeterHandle) { if let Some(inner) = self.inner.take() { - if mph.link_id() !=self.link { + if mph.link_id() != self.link { panic!("Tried to free link hold with the wrong MAC perimeter: saw {:?}, wanted {:?}", mph.link_id(),self.link); } @@ -126,7 +125,7 @@ impl DlsLink { return Err(-1); }; - if mph.link_id() !=self.link { + if mph.link_id() != self.link { panic!("Tried to open stream with the wrong MAC perimeter: saw {:?}, wanted {:?}", mph.link_id(),self.link); } @@ -155,7 +154,7 @@ impl Drop for DlsLink { if self.inner.take().is_some() { opte::engine::err!( "dropped hold on link {:?} without releasing!!", - self.link + self.link ); } } @@ -239,14 +238,15 @@ impl Drop for DldStream { // mac notify callbacks. Should we just come in through // dld_open/dld_close/dld_wput? That would make it a bit // weirder to set the promisc handle, and I don't know how - // this would interact with + // this would interact with dls_close(&mut inner.dld_str); dls_devnet_rele(inner.dld_str.ds_ddh) }, Err(e) => opte::engine::err!( "couldn't acquire MAC perimeter (err {}): \ dropped stream on link {:?} without releasing", - e, self.link, + e, + self.link, ), } } From 0434f1587efe791a800e9dfcebc6050fc49bba9c Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 19 Jun 2024 17:13:41 +0100 Subject: [PATCH 13/30] Dls, not Dld. --- xde/src/dls/mod.rs | 24 ++++++++++++------------ xde/src/xde.rs | 8 ++++---- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index 4488c314..f970b827 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -108,7 +108,7 @@ impl DlsLink { if let Some(inner) = self.inner.take() { if mph.link_id() != self.link { panic!("Tried to free link hold with the wrong MAC perimeter: saw {:?}, wanted {:?}", - mph.link_id(),self.link); + mph.link_id(), self.link); } unsafe { dls_devnet_rele_link(inner.dlh, inner.dlp); @@ -116,18 +116,18 @@ impl DlsLink { } } - /// Convert a hold into a `DldStream` for packet Rx/Tx. + /// Convert a hold into a `DlsStream` for packet Rx/Tx. fn open_stream( mut self, mph: &MacPerimeterHandle, - ) -> Result, c_int> { + ) -> Result, c_int> { let Some(inner) = self.inner.as_ref() else { return Err(-1); }; if mph.link_id() != self.link { panic!("Tried to open stream with the wrong MAC perimeter: saw {:?}, wanted {:?}", - mph.link_id(),self.link); + mph.link_id(), self.link); } let mut stream = MaybeUninit::zeroed(); @@ -137,8 +137,8 @@ impl DlsLink { // DLP is held/consumed by dls_open. _ = self.inner.take(); let dld_str = unsafe { stream.assume_init() }; - Ok(DldStream { - inner: Some(DldStreamInner { dld_str }), + Ok(DlsStream { + inner: Some(DlsStreamInner { dld_str }), link: mph.link_id(), } .into()) @@ -163,17 +163,17 @@ impl Drop for DlsLink { /// A DLS message stream on a target link, allowing packet /// Rx and Tx. #[derive(Debug)] -pub struct DldStream { - inner: Option, +pub struct DlsStream { + inner: Option, link: LinkId, } #[derive(Debug)] -struct DldStreamInner { +struct DlsStreamInner { dld_str: dld_str_s, } -impl DldStream { +impl DlsStream { pub fn open(link_id: LinkId) -> Result, c_int> { let perim = MacPerimeterHandle::from_linkid(link_id)?; let link_handle = DlsLink::hold(&perim)?; @@ -218,7 +218,7 @@ impl DldStream { } } -impl MacClient for DldStream { +impl MacClient for DlsStream { fn mac_client_handle(&self) -> Result<*mut mac_client_handle, c_int> { let Some(inner) = self.inner.as_ref() else { return Err(-1); @@ -228,7 +228,7 @@ impl MacClient for DldStream { } } -impl Drop for DldStream { +impl Drop for DlsStream { fn drop(&mut self) { if let Some(mut inner) = self.inner.take() { match MacPerimeterHandle::from_linkid(self.link) { diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 15120061..2a05ec4e 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -13,7 +13,7 @@ //#![allow(clippy::arc_with_non_send_sync)] use crate::dls; -use crate::dls::DldStream; +use crate::dls::DlsStream; use crate::dls::LinkId; use crate::ioctl::IoctlEnvelope; use crate::mac; @@ -213,11 +213,11 @@ pub struct xde_underlay_port { pub mac: [u8; 6], /// MAC promiscuous handle for receiving packets on the underlay link. - mph: MacPromiscHandle, + mph: MacPromiscHandle, /// DLS-level handle on a device for promiscuous registration and /// packet Tx. - stream: Arc, + stream: Arc, } struct XdeState { @@ -1056,7 +1056,7 @@ fn create_underlay_port( msg: format!("failed to get linkid for {link_name}: {err}"), })?; - let stream = DldStream::open(link_id).map_err(|e| OpteError::System { + let stream = DlsStream::open(link_id).map_err(|e| OpteError::System { errno: EFAULT, msg: format!("failed to grab open stream for {link_name}: {e}"), })?; From d7d416a68981a2141290a56838004737607bc509 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 19 Jun 2024 17:32:28 +0100 Subject: [PATCH 14/30] Cleanup. --- xde/src/dls/mod.rs | 6 ++++-- xde/src/dls/sys.rs | 2 ++ xde/src/mac/mod.rs | 4 ++++ xde/src/xde.rs | 10 +++++++++- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index f970b827..29cad83e 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -111,7 +111,8 @@ impl DlsLink { mph.link_id(), self.link); } unsafe { - dls_devnet_rele_link(inner.dlh, inner.dlp); + dls_link_rele(inner.dlp); + dls_devnet_rele(inner.dlh); } } } @@ -238,7 +239,8 @@ impl Drop for DlsStream { // mac notify callbacks. Should we just come in through // dld_open/dld_close/dld_wput? That would make it a bit // weirder to set the promisc handle, and I don't know how - // this would interact with + // this would interact with the existing (logical) + // STREAMS up to ip. dls_close(&mut inner.dld_str); dls_devnet_rele(inner.dld_str.ds_ddh) }, diff --git a/xde/src/dls/sys.rs b/xde/src/dls/sys.rs index 8442403c..a08a469c 100644 --- a/xde/src/dls/sys.rs +++ b/xde/src/dls/sys.rs @@ -84,6 +84,8 @@ extern "C" { dlpp: *mut *mut dls_link, ) -> c_int; + pub fn dls_link_rele(dlp: *mut dls_link); + pub fn dls_devnet_mac(dlh: dls_dl_handle) -> *const c_char; pub fn dls_open( diff --git a/xde/src/mac/mod.rs b/xde/src/mac/mod.rs index ad3a79c2..70cf0aa9 100644 --- a/xde/src/mac/mod.rs +++ b/xde/src/mac/mod.rs @@ -270,6 +270,10 @@ impl Drop for MacClientHandle { } } +/// Structs which are (or contain) a usable MAC client. +/// +/// Currently, this is only used to enable promiscuous handler +/// registration. pub trait MacClient { fn mac_client_handle(&self) -> Result<*mut mac_client_handle, c_int>; } diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 2a05ec4e..605e109e 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -945,7 +945,15 @@ fn clear_xde_underlay() -> Result { // clone the MAC client handle. // 2. Close the open stream handle. - _ = Arc::into_inner(u.stream); + if Arc::into_inner(u.stream).is_none() { + return Err(OpteError::System { + errno: EBUSY, + msg: format!( + "underlay {} has outstanding dls_stream refs", + u.name + ), + }); + } } } From 303057bf8f4d7899944276797b07d35ce5658549 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 19 Jun 2024 17:45:59 +0100 Subject: [PATCH 15/30] Finalist tweaks --- xde/src/dls/sys.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/xde/src/dls/sys.rs b/xde/src/dls/sys.rs index a08a469c..cf307f04 100644 --- a/xde/src/dls/sys.rs +++ b/xde/src/dls/sys.rs @@ -64,14 +64,6 @@ extern "C" { ) -> mac_tx_cookie_t; // NOTE: ALL BELOW FUNCTIONS REQUIRE THE MAC PERIMETER TO BE HELD. - pub fn dls_devnet_hold_link( - link: datalink_id_t, - ddhp: *mut dls_dl_handle, - dlpp: *mut *mut dls_link, - ) -> c_int; - - pub fn dls_devnet_rele_link(dlh: dls_dl_handle, dlp: *mut dls_link); - pub fn dls_devnet_hold( link: datalink_id_t, ddhp: *mut dls_dl_handle, @@ -111,7 +103,8 @@ pub enum dld_passivestate_t { DLD_ACTIVE, } -// +// Direct translation of the illumos type type, with some +// pointer fields left as opaque void*s for simplicity. #[repr(C)] pub struct dld_str_s { ds_major: major_t, From 94819d51fde9c8a6532759bc5e0a4d7191c0a792 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 20 Jun 2024 12:10:19 +0100 Subject: [PATCH 16/30] Accidentally the wrong type. Good thing we don't perform any hairpin responses on inbound traffic, eh? --- xde/src/xde.rs | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 605e109e..6c7f4842 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -19,7 +19,6 @@ use crate::ioctl::IoctlEnvelope; use crate::mac; use crate::mac::mac_getinfo; use crate::mac::mac_private_minor; -use crate::mac::MacClientHandle; use crate::mac::MacHandle; use crate::mac::MacPromiscHandle; use crate::mac::MacTxFlags; @@ -926,23 +925,19 @@ fn clear_xde_underlay() -> Result { }; for u in [u1, u2] { - // We have a chain of refs here: - // 1. `MacPromiscHandle` holds a ref to `MacClientHandle`, and - // 2. `MacUnicastHandle` holds a ref to `MacClientHandle`, and - // 3. `MacClientHandle` holds a ref to `MacHandle`. - // We explicitly drop them in order here to ensure there are no - // outstanding refs. + // We have a chain of refs here: `MacPromiscHandle` holds a ref to + // `DldStream`. We explicitly drop them in order here to ensure + // there are no outstanding refs. // 1. Remove promisc and unicast callbacks drop(u.mph); // Although `xde_rx` can be called into without any running ports - // via the promisc and unicast handles, illumos guarantees that - // neither callback will be running here. `mac_promisc_remove` will - // either remove the callback immediately (if there are no walkers) - // or will mark the callback as condemned and await all active - // walkers finishing. Accordingly, no one else will have or try to - // clone the MAC client handle. + // via the promisc handle, illumos guarantees that this callback won't + // be running here. `mac_promisc_remove` will either remove the callback + // immediately (if there are no walkers) or will mark the callback as + // condemned and await all active walkers finishing. Accordingly, no one + // else will have or try to clone the Stream handle. // 2. Close the open stream handle. if Arc::into_inner(u.stream).is_none() { @@ -1802,9 +1797,9 @@ unsafe extern "C" fn xde_rx( // corresponding to the underlay port we're receiving on. Being // here in the callback means the `MacPromiscHandle` hasn't been // dropped yet and thus our `MacClientHandle` is also still valid. - let mch_ptr = arg as *const MacClientHandle; + let mch_ptr = arg as *const DlsStream; Arc::increment_strong_count(mch_ptr); - let mch: Arc = Arc::from_raw(mch_ptr); + let stream: Arc = Arc::from_raw(mch_ptr); let Ok(mut chain) = PacketChain::new(mp_chain) else { bad_packet_probe( @@ -1821,13 +1816,13 @@ unsafe extern "C" fn xde_rx( // of chains (port0, port1, ...), or hold tx until another // packet breaks the run targeting the same dest. while let Some(pkt) = chain.pop_front() { - xde_rx_one(&mch, mrh, pkt); + xde_rx_one(&stream, mrh, pkt); } } #[inline] unsafe fn xde_rx_one( - mch: &MacClientHandle, + stream: &DlsStream, mrh: *mut mac::mac_resource_handle, pkt: Packet, ) { @@ -1896,7 +1891,7 @@ unsafe fn xde_rx_one( mac::mac_rx(dev.mh, mrh, pkt.unwrap_mblk()); } Ok(ProcessResult::Hairpin(hppkt)) => { - mch.tx_drop_on_no_desc(hppkt, 0, MacTxFlags::empty()); + stream.tx_drop_on_no_desc(hppkt, 0, MacTxFlags::empty()); } _ => {} } From bf35bf2f218c3574fb0830e29b1e30ab78fba34a Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 10 Jul 2024 11:42:06 +0100 Subject: [PATCH 17/30] Review feedback: nits. --- xde/src/dls/mod.rs | 9 ++++----- xde/src/dls/sys.rs | 2 +- xde/src/xde.rs | 11 ++++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index 29cad83e..0f964b5c 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -13,7 +13,6 @@ use crate::mac::MacClient; use crate::mac::MacPerimeterHandle; use crate::mac::MacTxFlags; use crate::mac::MAC_DROP_ON_NO_DESC; -use alloc::sync::Arc; use core::ffi::CStr; use core::fmt::Display; use core::mem::MaybeUninit; @@ -28,7 +27,7 @@ pub use sys::*; /// An integer ID used by DLS to refer to a given link. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct LinkId(u32); +pub struct LinkId(datalink_id_t); impl LinkId { /// Request the link ID for a device using its name. @@ -121,9 +120,9 @@ impl DlsLink { fn open_stream( mut self, mph: &MacPerimeterHandle, - ) -> Result, c_int> { + ) -> Result { let Some(inner) = self.inner.as_ref() else { - return Err(-1); + panic!("attempted to open a DlsStream on freed link") }; if mph.link_id() != self.link { @@ -175,7 +174,7 @@ struct DlsStreamInner { } impl DlsStream { - pub fn open(link_id: LinkId) -> Result, c_int> { + pub fn open(link_id: LinkId) -> Result { let perim = MacPerimeterHandle::from_linkid(link_id)?; let link_handle = DlsLink::hold(&perim)?; link_handle.open_stream(&perim) diff --git a/xde/src/dls/sys.rs b/xde/src/dls/sys.rs index cf307f04..f0ce11cc 100644 --- a/xde/src/dls/sys.rs +++ b/xde/src/dls/sys.rs @@ -103,7 +103,7 @@ pub enum dld_passivestate_t { DLD_ACTIVE, } -// Direct translation of the illumos type type, with some +// Direct translation of the illumos type, with some // pointer fields left as opaque void*s for simplicity. #[repr(C)] pub struct dld_str_s { diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 6c7f4842..52f5b258 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -929,7 +929,7 @@ fn clear_xde_underlay() -> Result { // `DldStream`. We explicitly drop them in order here to ensure // there are no outstanding refs. - // 1. Remove promisc and unicast callbacks + // 1. Remove promisc callback. drop(u.mph); // Although `xde_rx` can be called into without any running ports @@ -1059,10 +1059,11 @@ fn create_underlay_port( msg: format!("failed to get linkid for {link_name}: {err}"), })?; - let stream = DlsStream::open(link_id).map_err(|e| OpteError::System { - errno: EFAULT, - msg: format!("failed to grab open stream for {link_name}: {e}"), - })?; + let stream = + Arc::new(DlsStream::open(link_id).map_err(|e| OpteError::System { + errno: EFAULT, + msg: format!("failed to grab open stream for {link_name}: {e}"), + })?); // Setup promiscuous callback to receive all packets on this link. // From 95a213d81014be11dd0148c605ce0cca66945951 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 1 Aug 2024 11:23:36 +0100 Subject: [PATCH 18/30] Alloc/free dld_str_t from the kmem cache. Relies on a small stlouis change to make the dld_str_cache pointer non-static. Tests are expected to fail. --- xde/src/dls/mod.rs | 46 +++++++++++++++++++++++++++++++++++----------- xde/src/dls/sys.rs | 9 ++++++--- xde/src/lib.rs | 1 + xde/src/xde.rs | 1 - 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/xde/src/dls/mod.rs b/xde/src/dls/mod.rs index 0f964b5c..a2133688 100644 --- a/xde/src/dls/mod.rs +++ b/xde/src/dls/mod.rs @@ -8,6 +8,8 @@ pub mod sys; +use crate::kmem::sys::kmem_cache_alloc; +use crate::kmem::sys::kmem_cache_free; use crate::mac::mac_client_handle; use crate::mac::MacClient; use crate::mac::MacPerimeterHandle; @@ -15,12 +17,13 @@ use crate::mac::MacTxFlags; use crate::mac::MAC_DROP_ON_NO_DESC; use core::ffi::CStr; use core::fmt::Display; -use core::mem::MaybeUninit; use core::ptr; +use core::ptr::NonNull; use illumos_sys_hdrs::c_int; use illumos_sys_hdrs::datalink_id_t; use illumos_sys_hdrs::uintptr_t; use illumos_sys_hdrs::ENOENT; +use illumos_sys_hdrs::KM_SLEEP; use opte::engine::packet::Packet; use opte::engine::packet::PacketState; pub use sys::*; @@ -130,13 +133,18 @@ impl DlsLink { mph.link_id(), self.link); } - let mut stream = MaybeUninit::zeroed(); - let res = - unsafe { dls_open(inner.dlp, inner.dlh, stream.as_mut_ptr()) }; + let dld_str = NonNull::new(unsafe { + kmem_cache_alloc(dld_str_cachep, KM_SLEEP) as *mut dld_str_s + }); + let Some(dld_str) = dld_str else { + self.release(mph); + return Err(-1); + }; + + let res = unsafe { dls_open(inner.dlp, inner.dlh, dld_str.as_ptr()) }; if res == 0 { // DLP is held/consumed by dls_open. _ = self.inner.take(); - let dld_str = unsafe { stream.assume_init() }; Ok(DlsStream { inner: Some(DlsStreamInner { dld_str }), link: mph.link_id(), @@ -170,7 +178,7 @@ pub struct DlsStream { #[derive(Debug)] struct DlsStreamInner { - dld_str: dld_str_s, + dld_str: NonNull, } impl DlsStream { @@ -209,7 +217,7 @@ impl DlsStream { raw_flags |= MAC_DROP_ON_NO_DESC; unsafe { str_mdata_fastpath_put( - &inner.dld_str, + inner.dld_str.as_ptr(), pkt.unwrap_mblk(), hint, raw_flags, @@ -224,13 +232,13 @@ impl MacClient for DlsStream { return Err(-1); }; - Ok(inner.dld_str.ds_mch) + Ok(unsafe { (*inner.dld_str.as_ptr()).ds_mch }) } } impl Drop for DlsStream { fn drop(&mut self) { - if let Some(mut inner) = self.inner.take() { + if let Some(inner) = self.inner.take() { match MacPerimeterHandle::from_linkid(self.link) { Ok(_perim) => unsafe { // NOTE: this is reimplementing dld_str_detach @@ -240,8 +248,24 @@ impl Drop for DlsStream { // weirder to set the promisc handle, and I don't know how // this would interact with the existing (logical) // STREAMS up to ip. - dls_close(&mut inner.dld_str); - dls_devnet_rele(inner.dld_str.ds_ddh) + dls_close(inner.dld_str.as_ptr()); + dls_devnet_rele((*inner.dld_str.as_ptr()).ds_ddh); + + (*inner.dld_str.as_ptr()).ds_mh = ptr::null_mut(); + (*inner.dld_str.as_ptr()).ds_mch = ptr::null_mut(); + // ddh NULL not checked by destroy fn or cleared by ddh + // but it probably should be. + (*inner.dld_str.as_ptr()).ds_ddh = ptr::null_mut(); + (*inner.dld_str.as_ptr()).ds_mip = ptr::null(); + + // already the case: + // dsp->ds_dlstate = DL_UNATTACHED; + // dsp->ds_sap = 0; + + kmem_cache_free( + dld_str_cachep, + inner.dld_str.as_ptr() as *mut _, + ); }, Err(e) => opte::engine::err!( "couldn't acquire MAC perimeter (err {}): \ diff --git a/xde/src/dls/sys.rs b/xde/src/dls/sys.rs index f0ce11cc..8ddf9f42 100644 --- a/xde/src/dls/sys.rs +++ b/xde/src/dls/sys.rs @@ -7,6 +7,7 @@ // stuff we need from dls use crate::ip::kcondvar_t; +use crate::ip::kmem_cache_t; use crate::ip::krwlock_t; use crate::ip::list_node_t; use crate::ip::major_t; @@ -57,7 +58,7 @@ extern "C" { /// Transmit a packet chain on a given link. /// This is effectively one layer above mac_tx. pub fn str_mdata_fastpath_put( - dsp: *const dld_str_s, + dsp: *mut dld_str_s, mp: *mut mblk_t, f_hint: uintptr_t, flag: u16, @@ -87,6 +88,8 @@ extern "C" { ) -> c_int; pub fn dls_close(dsp: *mut dld_str_s); + + pub static dld_str_cachep: *mut kmem_cache_t; } #[repr(C)] @@ -121,14 +124,14 @@ pub struct dld_str_s { ds_style: t_uscalar_t, ds_sap: u16, - ds_mh: *mut mac_handle, + pub ds_mh: *mut mac_handle, pub ds_mch: *mut mac_client_handle, ds_promisc: u32, ds_mph: *mut mac_promisc_handle, ds_vlan_mph: *mut mac_promisc_handle, - ds_mip: *const c_void, // mac_info_t + pub ds_mip: *const c_void, // mac_info_t ds_pri: c_uint, diff --git a/xde/src/lib.rs b/xde/src/lib.rs index 92c78301..8bdd06e4 100644 --- a/xde/src/lib.rs +++ b/xde/src/lib.rs @@ -42,6 +42,7 @@ use illumos_sys_hdrs::KM_SLEEP; pub mod dls; pub mod ip; +pub mod kmem; pub mod mac; pub mod route; pub mod secpolicy; diff --git a/xde/src/xde.rs b/xde/src/xde.rs index bae3d677..d2a67fa1 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -57,7 +57,6 @@ 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; use opte::engine::geneve::Vni; use opte::engine::headers::EncapMeta; use opte::engine::headers::IpAddr; From bb3cb41d4455a401e140b3c670643d51ac3c1b0a Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 1 Aug 2024 15:16:24 +0100 Subject: [PATCH 19/30] Initial CTF reading from within opteadm. --- Cargo.lock | 85 +++- Cargo.toml | 7 + bin/opteadm/Cargo.toml | 1 + bin/opteadm/src/bin/opteadm.rs | 64 ++- crates/ctf/Cargo.toml | 13 + crates/ctf/src/lib.rs | 745 +++++++++++++++++++++++++++++++++ 6 files changed, 906 insertions(+), 9 deletions(-) create mode 100644 crates/ctf/Cargo.toml create mode 100644 crates/ctf/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 5b83fdd8..6350d5c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -99,6 +99,12 @@ version = "1.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "69f7f8c3906b62b754cd5326047894316021dcfe5a194c8ea52bdd94934a3457" +[[package]] +name = "arrayref" +version = "0.3.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d151e35f61089500b617991b791fc8bfd237ae50cd5950803758a179b41e67a" + [[package]] name = "autocfg" version = "1.3.0" @@ -448,6 +454,17 @@ dependencies = [ "memchr", ] +[[package]] +name = "ctf" +version = "0.1.0" +dependencies = [ + "anyhow", + "arrayref", + "flate2", + "num_enum 0.7.3", + "object", +] + [[package]] name = "ctor" version = "0.2.8" @@ -582,7 +599,7 @@ source = "git+https://github.com/oxidecomputer/dlpi-sys#1d587ea98cf2d36f1b1624b0 dependencies = [ "libc", "libdlpi-sys", - "num_enum", + "num_enum 0.5.11", "pretty-hex 0.2.1", "thiserror", "tokio", @@ -647,6 +664,16 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "flate2" +version = "1.0.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f54427cfd1c7829e2a139fcefea601bf088ebca651d2bf53ebc600eac295dae" +dependencies = [ + "crc32fast", + "miniz_oxide", +] + [[package]] name = "fnv" version = "1.0.7" @@ -994,7 +1021,7 @@ dependencies = [ "colored", "dlpi", "libc", - "num_enum", + "num_enum 0.5.11", "nvpair", "nvpair-sys", "oxnet", @@ -1014,7 +1041,7 @@ dependencies = [ "colored", "dlpi", "libc", - "num_enum", + "num_enum 0.5.11", "nvpair", "nvpair-sys", "oxnet", @@ -1157,7 +1184,16 @@ version = "0.5.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1f646caf906c20226733ed5b1374287eb97e3c2a5c227ce668c1f2ce20ae57c9" dependencies = [ - "num_enum_derive", + "num_enum_derive 0.5.11", +] + +[[package]] +name = "num_enum" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e613fc340b2220f734a8595782c551f1250e969d87d3be1ae0579e8d4065179" +dependencies = [ + "num_enum_derive 0.7.3", ] [[package]] @@ -1172,6 +1208,18 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "num_enum_derive" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af1844ef2428cc3e1cb900be36181049ef3d3193c63e43026cfe202983b27a56" +dependencies = [ + "proc-macro-crate", + "proc-macro2", + "quote", + "syn 2.0.69", +] + [[package]] name = "nvpair" version = "0.5.0" @@ -1193,7 +1241,9 @@ version = "0.36.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "081b846d1d56ddfc18fdf1a922e4f6e07a11768ea1b92dec44e42b72712ccfce" dependencies = [ + "flate2", "memchr", + "ruzstd", ] [[package]] @@ -1302,6 +1352,7 @@ dependencies = [ "anyhow", "cfg-if", "clap", + "ctf", "libc", "libnet 0.1.0 (git+https://github.com/oxidecomputer/netadm-sys)", "opte", @@ -1724,6 +1775,16 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "ruzstd" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5022b253619b1ba797f243056276bed8ed1a73b0f5a7ce7225d524067644bf8f" +dependencies = [ + "byteorder", + "twox-hash", +] + [[package]] name = "ryu" version = "1.0.18" @@ -2004,6 +2065,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" +[[package]] +name = "static_assertions" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" + [[package]] name = "strsim" version = "0.11.1" @@ -2255,6 +2322,16 @@ dependencies = [ "once_cell", ] +[[package]] +name = "twox-hash" +version = "1.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97fee6b57c6a41524a810daee9286c02d7752c4253064d0b05472833a438f675" +dependencies = [ + "cfg-if", + "static_assertions", +] + [[package]] name = "typenum" version = "1.17.0" diff --git a/Cargo.toml b/Cargo.toml index 67f739d4..d5411030 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ repository = "https://github.com/oxidecomputer/opte" [workspace.dependencies] # Internal crates +ctf = { path = "crates/ctf" } derror-macro = { path = "crates/derror-macro" } illumos-sys-hdrs = { path = "crates/illumos-sys-hdrs" } kstat-macro = { path = "crates/kstat-macro" } @@ -79,5 +80,11 @@ zone = { git = "https://github.com/oxidecomputer/zone" } ztest = { git = "https://github.com/oxidecomputer/falcon", branch = "main" } poptrie = { git = "https://github.com/oxidecomputer/poptrie", branch = "multipath" } +# For CTF (temp?) +arrayref = "0.3" +flate2 = "1" +num_enum = "0.7" +object = "0.36" + [profile.release] debug = 2 diff --git a/bin/opteadm/Cargo.toml b/bin/opteadm/Cargo.toml index 199742fe..378918c3 100644 --- a/bin/opteadm/Cargo.toml +++ b/bin/opteadm/Cargo.toml @@ -7,6 +7,7 @@ license.workspace = true repository.workspace = true [dependencies] +ctf.workspace = true # XXX For the time being opteadm needs to set the engine feature to # get all the types. Once there types are move to their appropriate # place this feature flag will be replaced/removed. diff --git a/bin/opteadm/src/bin/opteadm.rs b/bin/opteadm/src/bin/opteadm.rs index ff5f2fd2..8bd87c87 100644 --- a/bin/opteadm/src/bin/opteadm.rs +++ b/bin/opteadm/src/bin/opteadm.rs @@ -7,6 +7,8 @@ use anyhow::Context; use clap::Args; use clap::Parser; +use ctf::Ctf; +use ctf::TypeRepr; use opte::api::Direction; use opte::api::DomainName; use opte::api::IpAddr; @@ -198,16 +200,26 @@ enum Command { }, /// Delete an xde device - DeleteXde { name: String }, + DeleteXde { + name: String, + }, /// Set up xde underlay devices - SetXdeUnderlay { u1: String, u2: String }, + SetXdeUnderlay { + u1: String, + u2: String, + }, /// Clear xde underlay devices ClearXdeUnderlay, /// Set a virtual-to-physical mapping - SetV2P { vpc_ip: IpAddr, vpc_mac: MacAddr, underlay_ip: Ipv6Addr, vni: Vni }, + SetV2P { + vpc_ip: IpAddr, + vpc_mac: MacAddr, + underlay_ip: Ipv6Addr, + vni: Vni, + }, /// Clear a virtual-to-physical mapping ClearV2P { @@ -218,10 +230,16 @@ enum Command { }, /// Set a virtual-to-boundary mapping - SetV2B { prefix: IpCidr, tunnel_endpoint: Vec }, + SetV2B { + prefix: IpCidr, + tunnel_endpoint: Vec, + }, /// Clear a virtual-to-boundary mapping - ClearV2B { prefix: IpCidr, tunnel_endpoint: Vec }, + ClearV2B { + prefix: IpCidr, + tunnel_endpoint: Vec, + }, /// Add a new router entry, either IPv4 or IPv6. AddRouterEntry { @@ -274,6 +292,9 @@ enum Command { #[arg(long = "dir")] direction: Option, }, + + // Test out some stuff. + Test, } #[derive(Debug, Parser)] @@ -565,6 +586,37 @@ fn print_port(t: &mut impl Write, pi: PortInfo) -> std::io::Result<()> { fn main() -> anyhow::Result<()> { let cmd = Command::parse(); + + if let Command::Test = cmd { + let a = Ctf::from_file("/kernel/drv/amd64/dld")?; + // println!("quack"); + // println!("{:#?}", a.sections); + // println!("{:?}", a); + + for (i, ty) in a.sections.types.iter().enumerate() { + let s = a.string_at(ty.name_offset); + println!("{s}, {i:?}") + } + + println!("{:?}", a.header); + + let dld = a.find_type_by_name("dld_str_s").unwrap(); + println!("{:?}", dld); + + if let TypeRepr::Struct(ref fields) = dld.repr { + println!("dld_str_s has:"); + for field in fields { + println!("{}, {}", field.name_offset, field.type_offset); + let field_name = a.string_at(field.name_offset); + let ty_name = a.type_name(field.type_offset); + // let ty_name = ""; + println!("- {field_name}: {ty_name:?} @ ({})", field.offset) + } + } + + return Ok(()); + }; + let hdl = opteadm::OpteAdm::open(OpteAdm::XDE_CTL)?; match cmd { @@ -846,6 +898,8 @@ fn main() -> anyhow::Result<()> { })?; } } + + _ => {} } Ok(()) diff --git a/crates/ctf/Cargo.toml b/crates/ctf/Cargo.toml new file mode 100644 index 00000000..b34c4a72 --- /dev/null +++ b/crates/ctf/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "ctf" +version = "0.1.0" +edition.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +anyhow.workspace = true +arrayref.workspace = true +flate2.workspace = true +num_enum.workspace = true +object.workspace = true \ No newline at end of file diff --git a/crates/ctf/src/lib.rs b/crates/ctf/src/lib.rs new file mode 100644 index 00000000..2c51c41a --- /dev/null +++ b/crates/ctf/src/lib.rs @@ -0,0 +1,745 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// 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 2024 Oxide Computer Company + +// NOTE: this is identical to +// https://github.com/oxidecomputer/ctf-bindgen/blob/main/src/ctf.rs +// apart from conversion to anyhow, some bugfixes, and some helper methods. +// We should open that out if this proves effective. + +use anyhow::anyhow; +use arrayref::array_ref; +use flate2::read::ZlibDecoder; +use num_enum::TryFromPrimitive; +use object::elf::FileHeader64; +use object::endian::LittleEndian; +use object::read::elf::ElfFile; +use object::Object; +use object::ObjectSection; +use object::ObjectSymbol; +use std::convert::TryFrom; +use std::ffi::CStr; +use std::fs; +use std::io::Read; +use std::mem::size_of; +use std::path::Path; + +/// The primary CTF type that holds all of the information extracted from an ELF +/// object file. This data structure contains minimally parsed CTF elements that +/// reference internal `ctf_data`. Functions in the Ctf implementation are +/// provided for resolving things like string and type references. +#[derive(Debug)] +pub struct Ctf { + /// Library file this CTF came from + pub libname: String, + + /// Version and flag information associated with this CTF. + pub preamble: Preamble, + + /// A collection of indices into the CTF data that tells us where to find + /// things. + pub header: Header, + + /// Parsed out sections containing CTF data structures. + pub sections: Sections, + + /// Names of functions from the ELF symtab data + pub function_names: Vec, + + ctf_data: Vec, + uncompressed: Option>, +} + +impl Ctf { + /// Create a Ctf instance from an ELF object file. + pub fn from_file>(path: P) -> anyhow::Result { + // parse the libname libfoo.so as foo + let libname = + path.as_ref().file_stem().unwrap().to_str().unwrap().to_string(); + + let libname = match libname.strip_prefix("lib") { + Some(without_prefix) => without_prefix.into(), + None => libname, + }; + + // + // parse the object file + // + let bin_data = fs::read(path)?; + let obj_file = ElfFile::<'_, FileHeader64, &[u8]>::parse( + bin_data.as_slice(), + )?; + + // XXX + let mut function_names = Vec::new(); + for s in obj_file.symbols() { + let name = match s.name() { + Ok(n) => n, + Err(_) => continue, + }; + if s.kind() == object::SymbolKind::Text { + function_names.push(name.to_owned()); + } + } + + // + // get the CTF section, bail if it's not there + // + let section = match obj_file.section_by_name(".SUNW_ctf") { + Some(section) => section, + None => Err(anyhow!("ctf section not found"))?, + }; + + // + // parse the raw CTF data + // + Self::parse_ctf_data(section.data()?, function_names, libname) + } + + /// First parse out the premable and headers, then decompress (if needed) + /// and parse section data. + fn parse_ctf_data( + data: &[u8], + function_names: Vec, + libname: String, + ) -> anyhow::Result { + // + // parse the preamble and header + // + let (preamble, data) = Self::parse_preamble(data)?; + let (header, data) = Self::parse_header(data)?; + + // + // decompress the remaining data if needed and parse the CTF section + // data + // + let (sections, uncompressed) = if preamble.compressed() { + let mut d = ZlibDecoder::new(data); + let mut uncompressed = Vec::new(); + d.read_to_end(&mut uncompressed)?; + (Sections::parse(&uncompressed, &header)?, Some(uncompressed)) + } else { + (Sections::parse(data, &header)?, None) + }; + + Ok(Ctf { + preamble, + header, + sections, + uncompressed, + function_names, + libname, + ctf_data: data.to_owned(), + }) + } + + /// Ensure the correct magic is in the preamble. Check the CTF is the + /// expected version. Extract flags + fn parse_preamble(data: &[u8]) -> anyhow::Result<(Preamble, &[u8])> { + assert!(data.len() >= 4); + + // + // check magic + // + let magic = u16::from_le_bytes([data[0], data[1]]); + if magic != MAGIC { + anyhow::bail!("ctf magic {} is not magical enough", magic); + } + + // + // check version + // + let version = data[2]; + if version != VERSION { + anyhow::bail!( + "ctf version {} not supported, only version 2", + version + ); + } + + // + // extract flags + // + let flags = data[3]; + + Ok((Preamble { version, flags }, &data[4..])) + } + + /// Parse the contents of the CTF header. + fn parse_header(d: &[u8]) -> anyhow::Result<(Header, &[u8])> { + let size = size_of::

(); + assert!(d.len() >= size); + + Ok(( + Header { + parent_label_offset: u32::from_le_bytes(*array_ref!(d, 0, 4)), + parent_name_offset: u32::from_le_bytes(*array_ref!(d, 4, 4)), + label_offset: u32::from_le_bytes(*array_ref!(d, 8, 4)), + object_offset: u32::from_le_bytes(*array_ref!(d, 12, 4)), + function_offset: u32::from_le_bytes(*array_ref!(d, 16, 4)), + type_offset: u32::from_le_bytes(*array_ref!(d, 20, 4)), + string_offset: u32::from_le_bytes(*array_ref!(d, 24, 4)), + string_section_length: u32::from_le_bytes(*array_ref!( + d, 28, 4 + )), + }, + &d[size..], + )) + } + + pub fn is_child(&self) -> bool { + self.header.parent_name_offset != 0 + } + + pub fn string_at(&self, offset: u32) -> &str { + let offset = offset as usize; + + let s_begin = self.header.string_offset as usize; + let s_end = s_begin + self.header.string_section_length as usize; + let d = match &self.uncompressed { + Some(u) => &u[s_begin..s_end], + None => &self.ctf_data[s_begin..s_end], + }; + + if offset > d.len() { + return "?"; + } + + let mut end = offset; + loop { + if end == d.len() { + break; + } + if d[end] == 0 { + end += 1; + break; + } + end += 1; + } + + let cs = CStr::from_bytes_with_nul(&d[offset..end]).unwrap(); + cs.to_str().unwrap() + } + + pub fn resolve_type(&self, type_id: u16) -> Option<&Type> { + let true_idx = if self.is_child() { + // TODO: actually resolve type for parent. + type_id.checked_sub(0x8001) + } else { + type_id.checked_sub(1) + }? as usize; + + println!("idx {type_id} -> {true_idx}"); + + self.sections.types.get(true_idx) + } + + pub fn type_name(&self, type_id: u16) -> Option<&str> { + let t = self.resolve_type(type_id)?; + Some(self.string_at(t.name_offset)) + } + + pub fn find_type_by_name(&self, name: impl AsRef) -> Option<&Type> { + self.sections + .types + .iter() + .find(|ty| self.string_at(ty.name_offset) == name.as_ref()) + } +} + +#[derive(Debug, Default)] +pub struct Preamble { + pub version: u8, + pub flags: u8, +} + +#[derive(Debug, Default)] +pub struct Sections { + pub labels: Vec