From adcdb9fdb88aed6a811e0c82d06e58f1c420a01b Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Wed, 31 Mar 2021 15:38:51 -0700 Subject: [PATCH 1/5] Implement ser/der for QueuePairEndpoint --- Cargo.toml | 11 +++++++ src/lib.rs | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 3d3720b..37ec879 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,5 +26,16 @@ exclude = ["vendor/rdma-core/build/"] travis-ci = { repository = "jonhoo/rust-ibverbs" } maintenance = { status = "looking-for-maintainer" } +[dependencies.serde] +version = "1.0" +optional = true +features = ["derive"] + [build-dependencies] bindgen = "0.36" + +[features] +default = ["serde"] + +[dev-dependencies] +bincode = "1.3" diff --git a/src/lib.rs b/src/lib.rs index 13fb2a2..8717599 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -766,16 +766,106 @@ pub struct PreparedQueuePair<'res> { rnr_retry: u8, } +#[cfg(feature = "serde")] +extern crate serde; +#[cfg(feature = "serde")] +use serde::{Deserialize, Serialize}; + +// We go through this rigeramol because serde does now know how to derive unions. + +/// A (serde) serializable representation of a `QueuePairEndpoint`. +#[derive(Copy, Clone, Debug)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +pub struct PortableQPEndpoint { + num: u32, + lid: u16, + gid_subnet_prefix: u64, + gid_interface_id: u64, +} + +impl From for PortableQPEndpoint { + fn from(qpe: QueuePairEndpoint) -> PortableQPEndpoint { + PortableQPEndpoint { + num: qpe.num, + lid: qpe.lid, + // TODO: they're actually be64 and fixit + gid_subnet_prefix: unsafe { qpe.gid.global.subnet_prefix }, + gid_interface_id: unsafe { qpe.gid.global.interface_id }, + } + } +} + +impl From for QueuePairEndpoint { + fn from(pqpe: PortableQPEndpoint) -> QueuePairEndpoint { + let mut gid = ffi::ibv_gid::default(); + gid.global.subnet_prefix = pqpe.gid_subnet_prefix; + gid.global.interface_id = pqpe.gid_interface_id; + QueuePairEndpoint { + num: pqpe.num, + lid: pqpe.lid, + gid: gid, + } + } +} + +#[cfg(all(test, feature = "serde"))] +mod test_serde { + use super::*; + extern crate bincode; + #[test] + fn encode_decode() { + let qpe_default = QueuePairEndpoint { + num: 72, + lid: 9, + gid: Default::default(), + }; + + let mut qpe = qpe_default; + qpe.gid.global.subnet_prefix = 87; + qpe.gid.global.interface_id = 192; + let encoded = bincode::serialize(&qpe).unwrap(); + + let decoded: QueuePairEndpoint = bincode::deserialize(&encoded).unwrap(); + assert_eq!(qpe, decoded); + assert_ne!(qpe, qpe_default); + } +} + /// An identifier for the network endpoint of a `QueuePair`. /// /// Internally, this contains the `QueuePair`'s `qp_num`, as well as the context's `lid` and `gid`. #[derive(Copy, Clone)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "serde", serde(from = "PortableQPEndpoint"))] +#[cfg_attr(feature = "serde", serde(into = "PortableQPEndpoint"))] pub struct QueuePairEndpoint { num: u32, lid: u16, gid: ffi::ibv_gid, } +impl PartialEq for QueuePairEndpoint { + fn eq(&self, rhs: &QueuePairEndpoint) -> bool { + self.num == rhs.num + && self.lid == rhs.lid + && unsafe { + self.gid.global.subnet_prefix == rhs.gid.global.subnet_prefix + && self.gid.global.interface_id == rhs.gid.global.interface_id + } + } +} + +impl std::fmt::Debug for QueuePairEndpoint { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f,"QueuePairEndpoint {{ num: {:?}, lid: {:?}, gid_subnet_prefix: {:?}, gid_interface_id: {:?} }}", + self.num, + self.lid, + unsafe{self.gid.global.subnet_prefix}, + unsafe{self.gid.global.interface_id}, + ) + } +} + impl<'res> PreparedQueuePair<'res> { /// Get the network endpoint for this `QueuePair`. /// From 04083485d316fb781eadd13ee167d983f0ba5611 Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Wed, 31 Mar 2021 22:24:38 -0700 Subject: [PATCH 2/5] Make recommended changes --- src/lib.rs | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8717599..d7339c0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -776,11 +776,11 @@ use serde::{Deserialize, Serialize}; /// A (serde) serializable representation of a `QueuePairEndpoint`. #[derive(Copy, Clone, Debug)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[doc(hidden)] pub struct PortableQPEndpoint { num: u32, lid: u16, - gid_subnet_prefix: u64, - gid_interface_id: u64, + gid_raw: [u8; 16], } impl From for PortableQPEndpoint { @@ -788,9 +788,7 @@ impl From for PortableQPEndpoint { PortableQPEndpoint { num: qpe.num, lid: qpe.lid, - // TODO: they're actually be64 and fixit - gid_subnet_prefix: unsafe { qpe.gid.global.subnet_prefix }, - gid_interface_id: unsafe { qpe.gid.global.interface_id }, + gid_raw: unsafe { qpe.gid.raw }, } } } @@ -798,12 +796,11 @@ impl From for PortableQPEndpoint { impl From for QueuePairEndpoint { fn from(pqpe: PortableQPEndpoint) -> QueuePairEndpoint { let mut gid = ffi::ibv_gid::default(); - gid.global.subnet_prefix = pqpe.gid_subnet_prefix; - gid.global.interface_id = pqpe.gid_interface_id; + gid.raw = pqpe.gid_raw; QueuePairEndpoint { num: pqpe.num, lid: pqpe.lid, - gid: gid, + gid, } } } @@ -846,22 +843,16 @@ pub struct QueuePairEndpoint { impl PartialEq for QueuePairEndpoint { fn eq(&self, rhs: &QueuePairEndpoint) -> bool { - self.num == rhs.num - && self.lid == rhs.lid - && unsafe { - self.gid.global.subnet_prefix == rhs.gid.global.subnet_prefix - && self.gid.global.interface_id == rhs.gid.global.interface_id - } + self.num == rhs.num && self.lid == rhs.lid && unsafe { self.gid.raw == rhs.gid.raw } } } impl std::fmt::Debug for QueuePairEndpoint { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f,"QueuePairEndpoint {{ num: {:?}, lid: {:?}, gid_subnet_prefix: {:?}, gid_interface_id: {:?} }}", - self.num, - self.lid, - unsafe{self.gid.global.subnet_prefix}, - unsafe{self.gid.global.interface_id}, + write!( + f, + "QueuePairEndpoint {{ num: {:?}, lid: {:?}, gid: union not shown }}", + self.num, self.lid, ) } } From 3cc8d7915c94bc229543a9752370981003a4a586 Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Tue, 6 Apr 2021 16:32:55 -0700 Subject: [PATCH 3/5] Implement `Gid` as a native type. Encode that `Gid` is really just a `[u8;16]`. We also just apply `serde` directly to `QueuePairEndpoint` without exposing any implementation types. We can do this because `serde` can be applied to `Gid`. --- Cargo.toml | 1 + src/lib.rs | 158 ++++++++++++++++++++++++++++------------------------- 2 files changed, 86 insertions(+), 73 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 37ec879..cc13954 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,7 @@ [package] name = "ibverbs" version = "0.4.4" +edition = "2018" description = "Bindings for RDMA ibverbs through rdma-core" readme = "README.md" diff --git a/src/lib.rs b/src/lib.rs index d7339c0..8740d16 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,6 +64,7 @@ // avoid warnings about RDMAmojo, iWARP, InfiniBand, etc. not being in backticks #![cfg_attr(feature = "cargo-clippy", allow(doc_markdown))] +use std::convert::TryInto; use std::ffi::CStr; use std::io; use std::marker::PhantomData; @@ -85,6 +86,9 @@ pub use ffi::ibv_wc; pub use ffi::ibv_wc_opcode; pub use ffi::ibv_wc_status; +#[cfg(feature = "serde")] +use serde::{Deserialize, Serialize}; + /// Access flags for use with `QueuePair` and `MemoryRegion`. pub use ffi::ibv_access_flags; @@ -260,7 +264,7 @@ impl<'devlist> Device<'devlist> { pub struct Context { ctx: *mut ffi::ibv_context, port_attr: ffi::ibv_port_attr, - gid: ffi::ibv_gid, + gid: Gid, } unsafe impl Sync for Context {} @@ -308,8 +312,9 @@ impl Context { } } - let mut gid = ffi::ibv_gid::default(); - let ok = unsafe { ffi::ibv_query_gid(ctx, PORT_NUM, 0, &mut gid as *mut _) }; + // let mut gid = ffi::ibv_gid::default(); + let mut gid = Gid::default(); + let ok = unsafe { ffi::ibv_query_gid(ctx, PORT_NUM, 0, gid.ffi()) }; if ok != 0 { return Err(io::Error::last_os_error()); } @@ -766,95 +771,78 @@ pub struct PreparedQueuePair<'res> { rnr_retry: u8, } -#[cfg(feature = "serde")] -extern crate serde; -#[cfg(feature = "serde")] -use serde::{Deserialize, Serialize}; - -// We go through this rigeramol because serde does now know how to derive unions. - -/// A (serde) serializable representation of a `QueuePairEndpoint`. -#[derive(Copy, Clone, Debug)] +/// A Global identifier for ibv. +/// +/// This struct acts as a rust wrapper for `ffi::ibv_gid`. We use it instead of +/// `ffi::ibv_giv` because `ffi::ibv_gid` is actually an untagged union. +/// +/// ```c +/// union ibv_gid { +/// uint8_t raw[16]; +/// struct { +/// __be64 subnet_prefix; +/// __be64 interface_id; +/// } global; +/// }; +/// ``` +/// +/// It appears that `global` exists for convenience, but can be safely ignored. +/// For continuity, the methods `subnet_prefix` and `interface_id` are provided. +/// These methods read the array as big endian, regardless of native cpu +/// endianness. #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -#[doc(hidden)] -pub struct PortableQPEndpoint { - num: u32, - lid: u16, - gid_raw: [u8; 16], +#[derive(Default, Copy, Clone, Debug, Eq, PartialEq)] +struct Gid { + raw: [u8; 16], } -impl From for PortableQPEndpoint { - fn from(qpe: QueuePairEndpoint) -> PortableQPEndpoint { - PortableQPEndpoint { - num: qpe.num, - lid: qpe.lid, - gid_raw: unsafe { qpe.gid.raw }, - } +impl Gid { + /// Expose a pointer to the underlying `ffi::ibv_gid` object for use in ffi + /// calls. + fn ffi(&mut self) -> *mut ffi::ibv_gid { + self.raw.as_mut_ptr() as _ + } + + /// Expose the subnet_prefix component of the `Gid` as a u64. This is + /// equivalent to accessing the `global.subnet_prefix` component of the + /// `ffi::ibv_gid` union. + #[allow(dead_code)] + fn subnet_prefix(&self) -> u64 { + u64::from_be_bytes(self.raw[..8].try_into().unwrap()) + } + + /// Expose the interface_id component of the `Gid` as a u64. This is + /// equivalent to accessing the `global.interface_id` component of the + /// `ffi::ibv_gid` union. + #[allow(dead_code)] + fn interface_id(&self) -> u64 { + u64::from_be_bytes(self.raw[8..].try_into().unwrap()) } } -impl From for QueuePairEndpoint { - fn from(pqpe: PortableQPEndpoint) -> QueuePairEndpoint { - let mut gid = ffi::ibv_gid::default(); - gid.raw = pqpe.gid_raw; - QueuePairEndpoint { - num: pqpe.num, - lid: pqpe.lid, - gid, +impl From for Gid { + fn from(gid: ffi::ibv_gid) -> Self { + Self { + raw: unsafe { gid.raw }, } } } -#[cfg(all(test, feature = "serde"))] -mod test_serde { - use super::*; - extern crate bincode; - #[test] - fn encode_decode() { - let qpe_default = QueuePairEndpoint { - num: 72, - lid: 9, - gid: Default::default(), - }; - - let mut qpe = qpe_default; - qpe.gid.global.subnet_prefix = 87; - qpe.gid.global.interface_id = 192; - let encoded = bincode::serialize(&qpe).unwrap(); - - let decoded: QueuePairEndpoint = bincode::deserialize(&encoded).unwrap(); - assert_eq!(qpe, decoded); - assert_ne!(qpe, qpe_default); +impl From for ffi::ibv_gid { + fn from(mut gid: Gid) -> Self { + unsafe { *gid.ffi() } } } /// An identifier for the network endpoint of a `QueuePair`. /// /// Internally, this contains the `QueuePair`'s `qp_num`, as well as the context's `lid` and `gid`. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "serde", serde(from = "PortableQPEndpoint"))] -#[cfg_attr(feature = "serde", serde(into = "PortableQPEndpoint"))] pub struct QueuePairEndpoint { num: u32, lid: u16, - gid: ffi::ibv_gid, -} - -impl PartialEq for QueuePairEndpoint { - fn eq(&self, rhs: &QueuePairEndpoint) -> bool { - self.num == rhs.num && self.lid == rhs.lid && unsafe { self.gid.raw == rhs.gid.raw } - } -} - -impl std::fmt::Debug for QueuePairEndpoint { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "QueuePairEndpoint {{ num: {:?}, lid: {:?}, gid: union not shown }}", - self.num, self.lid, - ) - } + gid: Gid, } impl<'res> PreparedQueuePair<'res> { @@ -931,7 +919,7 @@ impl<'res> PreparedQueuePair<'res> { attr.ah_attr.sl = 0; attr.ah_attr.src_path_bits = 0; attr.ah_attr.port_num = PORT_NUM; - attr.ah_attr.grh.dgid = remote.gid; + attr.ah_attr.grh.dgid = remote.gid.into(); attr.ah_attr.grh.hop_limit = 0xff; let mask = ffi::ibv_qp_attr_mask::IBV_QP_STATE | ffi::ibv_qp_attr_mask::IBV_QP_AV @@ -1319,3 +1307,27 @@ impl<'a> Drop for QueuePair<'a> { } } } + +#[cfg(all(test, feature = "serde"))] +mod test_serde { + use super::*; + extern crate bincode; + #[test] + fn encode_decode() { + let qpe_default = QueuePairEndpoint { + num: 72, + lid: 9, + gid: Default::default(), + }; + + let mut qpe = qpe_default; + qpe.gid.raw = unsafe { std::mem::transmute([87_u64.to_be(), 192_u64.to_be()]) }; + let encoded = bincode::serialize(&qpe).unwrap(); + + let decoded: QueuePairEndpoint = bincode::deserialize(&encoded).unwrap(); + assert_eq!(decoded.gid.subnet_prefix(), 87); + assert_eq!(decoded.gid.interface_id(), 192); + assert_eq!(qpe, decoded); + assert_ne!(qpe, qpe_default); + } +} From 6d1a9dc598551d8b4ffa75c026d4088a975b928f Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Wed, 7 Apr 2021 18:20:12 -0700 Subject: [PATCH 4/5] Add traits to Gid --- src/lib.rs | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8740d16..e27e8bc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -314,7 +314,7 @@ impl Context { // let mut gid = ffi::ibv_gid::default(); let mut gid = Gid::default(); - let ok = unsafe { ffi::ibv_query_gid(ctx, PORT_NUM, 0, gid.ffi()) }; + let ok = unsafe { ffi::ibv_query_gid(ctx, PORT_NUM, 0, gid.as_mut() as _) }; if ok != 0 { return Err(io::Error::last_os_error()); } @@ -791,18 +791,13 @@ pub struct PreparedQueuePair<'res> { /// These methods read the array as big endian, regardless of native cpu /// endianness. #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -#[derive(Default, Copy, Clone, Debug, Eq, PartialEq)] +#[derive(Default, Copy, Clone, Debug, Eq, PartialEq, Hash)] +#[repr(transparent)] struct Gid { raw: [u8; 16], } impl Gid { - /// Expose a pointer to the underlying `ffi::ibv_gid` object for use in ffi - /// calls. - fn ffi(&mut self) -> *mut ffi::ibv_gid { - self.raw.as_mut_ptr() as _ - } - /// Expose the subnet_prefix component of the `Gid` as a u64. This is /// equivalent to accessing the `global.subnet_prefix` component of the /// `ffi::ibv_gid` union. @@ -830,7 +825,25 @@ impl From for Gid { impl From for ffi::ibv_gid { fn from(mut gid: Gid) -> Self { - unsafe { *gid.ffi() } + *gid.as_mut() + } +} + +impl AsRef for Gid { + fn as_ref(&self) -> &ffi::ibv_gid { + unsafe { self.raw.as_ptr().cast::().as_ref().unwrap() } + } +} + +impl AsMut for Gid { + fn as_mut(&mut self) -> &mut ffi::ibv_gid { + unsafe { + self.raw + .as_mut_ptr() + .cast::() + .as_mut() + .unwrap() + } } } From d2b15030100ce5acb7ed8fdc87832d1c13198ce8 Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Wed, 7 Apr 2021 23:53:45 -0700 Subject: [PATCH 5/5] Optimize `AsMut` & `AsRef` conversion. Make some recommended changes for conversion traits. --- src/lib.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e27e8bc..25647c8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -314,7 +314,7 @@ impl Context { // let mut gid = ffi::ibv_gid::default(); let mut gid = Gid::default(); - let ok = unsafe { ffi::ibv_query_gid(ctx, PORT_NUM, 0, gid.as_mut() as _) }; + let ok = unsafe { ffi::ibv_query_gid(ctx, PORT_NUM, 0, gid.as_mut()) }; if ok != 0 { return Err(io::Error::last_os_error()); } @@ -831,19 +831,13 @@ impl From for ffi::ibv_gid { impl AsRef for Gid { fn as_ref(&self) -> &ffi::ibv_gid { - unsafe { self.raw.as_ptr().cast::().as_ref().unwrap() } + unsafe { &*self.raw.as_ptr().cast::() } } } impl AsMut for Gid { fn as_mut(&mut self) -> &mut ffi::ibv_gid { - unsafe { - self.raw - .as_mut_ptr() - .cast::() - .as_mut() - .unwrap() - } + unsafe { &mut *self.raw.as_mut_ptr().cast::() } } }