From d06fe55fa38ea403d631190d11cca0595e61b737 Mon Sep 17 00:00:00 2001 From: Maxence Younsi Date: Sun, 24 Dec 2023 12:51:25 +0100 Subject: [PATCH] generic cresult generated code is ugly until we get https://github.com/mozilla/cbindgen/pull/785 merged. when this is done, type aliases can be used and it will look okay --- crates/pmacct-gauze-bindings/build.rs | 1 + crates/pmacct-gauze-lib/src/c_api.rs | 85 +++++--------- crates/pmacct-gauze-lib/src/error.rs | 109 ------------------ .../src/extensions/bmp_message.rs | 14 +-- crates/pmacct-gauze-lib/src/lib.rs | 3 +- .../pmacct-gauze-lib/src/result/bgp_result.rs | 20 ++++ .../pmacct-gauze-lib/src/result/bmp_result.rs | 81 +++++++++++++ crates/pmacct-gauze-lib/src/result/cresult.rs | 37 ++++++ crates/pmacct-gauze-lib/src/result/mod.rs | 13 +++ 9 files changed, 188 insertions(+), 175 deletions(-) delete mode 100644 crates/pmacct-gauze-lib/src/error.rs create mode 100644 crates/pmacct-gauze-lib/src/result/bgp_result.rs create mode 100644 crates/pmacct-gauze-lib/src/result/bmp_result.rs create mode 100644 crates/pmacct-gauze-lib/src/result/cresult.rs create mode 100644 crates/pmacct-gauze-lib/src/result/mod.rs diff --git a/crates/pmacct-gauze-bindings/build.rs b/crates/pmacct-gauze-bindings/build.rs index 03380ce..5ec5479 100644 --- a/crates/pmacct-gauze-bindings/build.rs +++ b/crates/pmacct-gauze-bindings/build.rs @@ -73,6 +73,7 @@ fn main() -> Result<(), Box> { .allowlist_file(format!("{header_location}/pmacct/src/bgp/bgp_ecommunity.h")) .allowlist_file(format!("{header_location}/pmacct/src/network.h")) .allowlist_file(format!("{header_location}/pmacct/src/log.h")) + .blocklist_file("/usr/local/include/pmacct_gauze_lib/pmacct_gauze_lib.h".to_string()) // Finish the builder and generate the bindings. .generate() // Unwrap the Result and panic on failure. diff --git a/crates/pmacct-gauze-lib/src/c_api.rs b/crates/pmacct-gauze-lib/src/c_api.rs index d8f64bf..2d581bd 100644 --- a/crates/pmacct-gauze-lib/src/c_api.rs +++ b/crates/pmacct-gauze-lib/src/c_api.rs @@ -13,7 +13,7 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use netgauze_bgp_pkt::BgpMessage; use netgauze_bgp_pkt::nlri::MplsLabel; use netgauze_bgp_pkt::path_attribute::{As4Path, AsPath, MpReach, MpUnreach, PathAttributeValue}; -use crate::error::ParseError; +use netgauze_bgp_pkt::update::BgpUpdateMessage; use crate::extensions::bgp_attribute::ExtendBgpAttribute; use crate::extensions::bmp_message::ExtendBmpMessage; use crate::extensions::community::{ExtendLargeCommunity, ExtendExtendedCommunity}; @@ -24,9 +24,14 @@ use crate::extensions::rd::ExtendRd; use crate::log::{LogPriority, pmacct_log}; use crate::macros::free_cslice_t; use crate::option::COption; +use crate::result::bgp_result::{BgpParseError, BmpBgpResult, ParsedBgp}; +use crate::result::bmp_result::{BmpParseError, BmpResult}; +use crate::result::cresult::CResult; +use crate::result::ParseError; use crate::slice::CSlice; pub struct BmpMessageValueOpaque(BmpMessageValue); +pub struct BgpUpdateMessageOpaque(BgpUpdateMessage); #[no_mangle] pub extern "C" fn netgauze_print_packet(buffer: *const libc::c_char, len: u32) -> u32 { @@ -44,50 +49,32 @@ pub extern "C" fn netgauze_print_packet(buffer: *const libc::c_char, len: u32) - #[repr(C)] #[derive(Debug)] pub struct ParsedBmp { + read_bytes: u32, common_header: bmp_common_hdr, peer_header: COption, pub message: *mut BmpMessageValueOpaque, } -#[repr(C)] -#[derive(Debug)] -pub struct ParseOk { - read_bytes: u32, - pub parsed: T, -} - -#[repr(C)] -#[derive(Debug)] -pub enum ParseResultEnum { - ParseSuccess(ParseOk), - ParseFailure(ParseError), -} - - #[no_mangle] -pub extern "C" fn netgauze_parse_packet(buffer: *const libc::c_char, buf_len: u32) -> ParseResultEnum { +pub extern "C" fn netgauze_parse_packet(buffer: *const libc::c_char, buf_len: u32) -> BmpResult { let s = unsafe { slice::from_raw_parts(buffer as *const u8, buf_len as usize) }; let span = Span::new(s); let result = BmpMessage::from_wire(span, &HashMap::new()); if let Ok((end_span, msg)) = result { let read_bytes = span.offset(&end_span) as u32; - let result = ParseResultEnum::ParseSuccess(ParseOk { + return BmpResult::Ok(ParsedBmp { read_bytes, - parsed: ParsedBmp { - common_header: bmp_common_hdr { - version: msg.get_version().into(), - len: read_bytes, - type_: msg.get_type().into(), - }, - peer_header: msg.get_pmacct_peer_hdr()?.into(), - message: Box::into_raw(Box::new(match msg { - BmpMessage::V3(value) => BmpMessageValueOpaque(value) - })), + common_header: bmp_common_hdr { + version: msg.get_version().into(), + len: read_bytes, + type_: msg.get_type().into(), }, + peer_header: msg.get_pmacct_peer_hdr()?.into(), + message: Box::into_raw(Box::new(match msg { + BmpMessage::V3(value) => BmpMessageValueOpaque(value) + })), }); - - return result; } let err = result.err().unwrap(); @@ -95,17 +82,17 @@ pub extern "C" fn netgauze_parse_packet(buffer: *const libc::c_char, buf_len: u3 let netgauze_error = CString::new(err.to_string()).unwrap(); - ParseError::NetgauzeError(netgauze_error.into_raw()).into() + BmpParseError::Netgauze(netgauze_error.into_raw()).into() } #[no_mangle] -pub extern "C" fn bmp_init_get_tlvs(bmp_init: *const BmpMessageValueOpaque) -> CSlice { +pub extern "C" fn bmp_init_get_tlvs(bmp_init: *const BmpMessageValueOpaque) -> CResult, BmpParseError> { let bmp_init = unsafe { &bmp_init.as_ref().unwrap().0 }; let init = match bmp_init { BmpMessageValue::Initiation(init) => init, - _ => unreachable!() // TODO make it an error + _ => return BmpParseError::WrongBmpMessageType.into() }; let mut tlvs = Vec::::with_capacity(init.information().len()); @@ -125,7 +112,7 @@ pub extern "C" fn bmp_init_get_tlvs(bmp_init: *const BmpMessageValueOpaque) -> C // println!("bmp_init_get_tlvs: {:#?}", &c_slice); - c_slice + CResult::Ok(c_slice) } free_cslice_t!(bmp_log_tlv); @@ -168,8 +155,6 @@ pub extern "C" fn netgauze_bgp_parse_nlri_naive_copy(bmp_rm: *const BmpMessageVa free_cslice_t!(u8); -/// pmacct C code MUST reallocate and copy all values -/// that need freeing to ensure safe memory freeing from C #[repr(C)] pub struct ProcessPacket { update_type: u32, @@ -230,20 +215,10 @@ impl Default for BgpParsedAttributes { } } -#[repr(C)] -#[derive(Debug)] -pub struct BgpParseResult { - packets: CSlice, - update_count: usize, -} - - pub fn reconcile_as24path(as_path: *mut aspath, as4_path: *mut aspath) -> *mut aspath { - if !as_path.is_null() && !as4_path.is_null() { let reconciled = unsafe { aspath_reconcile_as4(as_path, as4_path) }; if !reconciled.is_null() { - unsafe { aspath_free(as_path); aspath_free(as4_path); @@ -260,22 +235,21 @@ pub fn reconcile_as24path(as_path: *mut aspath, as4_path: *mut aspath) -> *mut a return as4_path; } - -// TODO use ParseError #[no_mangle] -pub extern "C" fn netgauze_bgp_parse_nlri(peer: *mut bgp_peer, bmp_rm: *const BmpMessageValueOpaque) -> BgpParseResult { +pub extern "C" fn netgauze_bgp_parse_nlri(peer: *mut bgp_peer, bmp_rm: *const BmpMessageValueOpaque) -> BmpBgpResult { + let bmp_rm = unsafe { bmp_rm.as_ref().unwrap() }; let bmp_rm = match &bmp_rm.0 { BmpMessageValue::RouteMonitoring(rm) => { rm } - _ => unreachable!() + _ => return CResult::Err(ParseError::ParseErrorBmp(BmpParseError::WrongBmpMessageType)) }; let update = match bmp_rm.update_message() { BgpMessage::Update(update) => update, - _ => unreachable!() + _ => return CResult::Err(ParseError::ParseErrorBgp(BgpParseError::WrongBgpMessageType)) }; let mut packets = Vec::with_capacity(update.withdraw_routes().len() + update.nlri().len()); @@ -318,7 +292,6 @@ pub extern "C" fn netgauze_bgp_parse_nlri(peer: *mut bgp_peer, bmp_rm: *const Bm for _attr in update.path_attributes() { match _attr.value() { PathAttributeValue::AsPath(aspath) => { - let extended_length = _attr.extended_length(); let mut bytes = Vec::with_capacity(aspath.len(extended_length)); let bytes = { @@ -743,15 +716,11 @@ pub extern "C" fn netgauze_bgp_parse_nlri(peer: *mut bgp_peer, bmp_rm: *const Bm } } - for (idx, packet) in packets.iter().enumerate() { - // println!("Packet [{}/{}] {:#?}", idx + 1, packets.len(), packet); - } - unsafe { - BgpParseResult { + BmpBgpResult::Ok(ParsedBgp { update_count: packets.iter().filter(|x| x.update_type == BGP_NLRI_UPDATE).count(), packets: CSlice::from_vec(packets), - } + }) } } diff --git a/crates/pmacct-gauze-lib/src/error.rs b/crates/pmacct-gauze-lib/src/error.rs deleted file mode 100644 index 63c3e62..0000000 --- a/crates/pmacct-gauze-lib/src/error.rs +++ /dev/null @@ -1,109 +0,0 @@ -use std::convert::Infallible; -use std::error::Error; -use std::ffi::CString; -use std::fmt::{Display, Formatter}; -use std::ops::{ControlFlow, FromResidual, Try}; -use c_str_macro::c_str; -use crate::c_api::{ParsedBmp, ParseOk, ParseResultEnum}; -use libc::c_char; - -impl Display for ParseError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", &self) - } -} - -impl Error for ParseError {} - -impl FromResidual for ParseResultEnum { - fn from_residual(residual: ::Residual) -> Self { - match residual { - Err(err) => ParseResultEnum::ParseFailure(err), - _ => unreachable!("residual should always be the error") - } - } -} - -// TODO make error generic in ParseError -// TODO rename ParseError to CResult - -#[repr(C)] -#[derive(Clone, PartialEq, Eq, Debug)] -pub enum ParseError { - RouteDistinguisherError, - IpAddrError, - NetgauzeError(*mut c_char), - StringConversionError, -} - -impl ParseError { - fn as_ptr(&self) -> *const c_char { - match self { - ParseError::RouteDistinguisherError => c_str! { - "ParseError::RouteDistinguisherError" - }.as_ptr(), - ParseError::NetgauzeError(err) => { - return *err as *const c_char; - } - ParseError::StringConversionError => c_str! { - "ParseError::StringConversionError" - }.as_ptr(), - ParseError::IpAddrError => c_str! { - "ParseError::IpAddrError" - }.as_ptr(), - } - } -} -#[no_mangle] -pub extern "C" fn parse_error_str(error: &'static ParseError) -> *const c_char { - error.as_ptr() -} - -#[no_mangle] -pub extern "C" fn parse_result_free(value: ParseResultEnum) { - match value { - ParseResultEnum::ParseSuccess(parse_ok) => unsafe { - drop(Box::from_raw(parse_ok.parsed.message)); - } - ParseResultEnum::ParseFailure(parse_error) => { - match parse_error { - ParseError::NetgauzeError(err) => unsafe { - drop(CString::from_raw(err)); - }, - ParseError::RouteDistinguisherError - | ParseError::StringConversionError - | ParseError::IpAddrError => {} - } - } - }; -} -impl Try for ParseResultEnum { - /// [Self::ParseSuccess] - type Output = Self; - - /// Result - type Residual = Result; - - fn from_output(output: Self::Output) -> Self { - output - } - - fn branch(self) -> ControlFlow { - match self { - ParseResultEnum::ParseSuccess(_) => ControlFlow::Continue(self), - ParseResultEnum::ParseFailure(err) => ControlFlow::Break(Err(err)) - } - } -} - -impl From> for ParseResultEnum { - fn from(value: ParseOk) -> Self { - Self::ParseSuccess(value) - } -} - -impl From for ParseResultEnum { - fn from(value: ParseError) -> Self { - Self::ParseFailure(value) - } -} diff --git a/crates/pmacct-gauze-lib/src/extensions/bmp_message.rs b/crates/pmacct-gauze-lib/src/extensions/bmp_message.rs index 02a6cf0..225dd80 100644 --- a/crates/pmacct-gauze-lib/src/extensions/bmp_message.rs +++ b/crates/pmacct-gauze-lib/src/extensions/bmp_message.rs @@ -2,13 +2,13 @@ use netgauze_bgp_pkt::wire::serializer::IpAddrWritingError; use netgauze_bgp_pkt::wire::serializer::nlri::RouteDistinguisherWritingError; use netgauze_bmp_pkt::{BmpMessage, BmpMessageValue, PeerHeader}; use pmacct_gauze_bindings::bmp_peer_hdr; -use crate::error::ParseError; use crate::extensions::ipaddr::{ExtendIpAddr, IpAddrBytes}; use crate::extensions::rd::{ExtendRd, RouteDistinguisherBytes}; +use crate::result::bmp_result::BmpParseError; pub trait ExtendBmpMessage { fn get_peer_header(&self) -> Option<&PeerHeader>; - fn get_pmacct_peer_hdr(&self) -> Result, ParseError>; + fn get_pmacct_peer_hdr(&self) -> Result, BmpParseError>; } impl ExtendBmpMessage for BmpMessage { @@ -31,7 +31,7 @@ impl ExtendBmpMessage for BmpMessage { } } - fn get_pmacct_peer_hdr(&self) -> Result, ParseError> { + fn get_pmacct_peer_hdr(&self) -> Result, BmpParseError> { let peer_hdr = if let Some(peer_hdr) = self.get_peer_header() { peer_hdr } else { @@ -51,12 +51,12 @@ impl ExtendBmpMessage for BmpMessage { } } -impl From for ParseError { +impl From for BmpParseError { fn from(_: RouteDistinguisherWritingError) -> Self { - Self::RouteDistinguisherError + Self::RouteDistinguisher } } -impl From for ParseError { - fn from(_: IpAddrWritingError) -> Self { Self::IpAddrError } +impl From for BmpParseError { + fn from(_: IpAddrWritingError) -> Self { Self::IpAddr } } \ No newline at end of file diff --git a/crates/pmacct-gauze-lib/src/lib.rs b/crates/pmacct-gauze-lib/src/lib.rs index 59e5f5b..18c271b 100644 --- a/crates/pmacct-gauze-lib/src/lib.rs +++ b/crates/pmacct-gauze-lib/src/lib.rs @@ -12,9 +12,10 @@ pub mod c_api; #[cfg(feature = "capi")] pub mod option; #[cfg(feature = "capi")] -pub mod error; +pub mod result; #[cfg(feature = "capi")] pub mod slice; + #[macro_use] pub mod macros; diff --git a/crates/pmacct-gauze-lib/src/result/bgp_result.rs b/crates/pmacct-gauze-lib/src/result/bgp_result.rs new file mode 100644 index 0000000..b03f0c7 --- /dev/null +++ b/crates/pmacct-gauze-lib/src/result/bgp_result.rs @@ -0,0 +1,20 @@ +use crate::c_api::ProcessPacket; +use crate::result::cresult::CResult; +use crate::result::ParseError; +use crate::slice::CSlice; + +pub type BgpResult = CResult; +pub type BmpBgpResult = CResult; + +#[repr(C)] +#[derive(Debug)] +pub enum BgpParseError { + WrongBgpMessageType +} + +#[repr(C)] +#[derive(Debug)] +pub struct ParsedBgp { + pub packets: CSlice, + pub update_count: usize, +} diff --git a/crates/pmacct-gauze-lib/src/result/bmp_result.rs b/crates/pmacct-gauze-lib/src/result/bmp_result.rs new file mode 100644 index 0000000..114e7b9 --- /dev/null +++ b/crates/pmacct-gauze-lib/src/result/bmp_result.rs @@ -0,0 +1,81 @@ +use std::error::Error; +use std::ffi::CString; +use std::fmt::{Display, Formatter}; +use c_str_macro::c_str; +use libc::c_char; +use crate::c_api::ParsedBmp; +use crate::result::cresult::CResult; + +pub type BmpResult = CResult; + +#[repr(C)] +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum BmpParseError { + RouteDistinguisher, + IpAddr, + Netgauze(*mut c_char), + StringConversion, + WrongBmpMessageType, +} + + +impl Display for BmpParseError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", &self) + } +} + +impl Error for BmpParseError {} + +impl BmpParseError { + fn as_str_ptr(&self) -> *const c_char { + match self { + BmpParseError::RouteDistinguisher => c_str! { + "BmpParseError::RouteDistinguisher" + }.as_ptr(), + BmpParseError::Netgauze(err) => { + return *err as *const c_char; + } + BmpParseError::StringConversion => c_str! { + "BmpParseError::StringConversion" + }.as_ptr(), + BmpParseError::IpAddr => c_str! { + "BmpParseError::IpAddr" + }.as_ptr(), + BmpParseError::WrongBmpMessageType => c_str! { + "BmpParseError::WrongMessageType" + }.as_ptr(), + } + } +} + +impl From for CResult { + fn from(value: BmpParseError) -> Self { + Self::Err(value) + } +} + +#[no_mangle] +pub extern "C" fn bmp_error_str(error: BmpParseError) -> *const c_char { + error.as_str_ptr() +} + +#[no_mangle] +pub extern "C" fn bmp_result_free(value: BmpResult) { + match value { + CResult::Ok(parse_ok) => unsafe { + drop(Box::from_raw(parse_ok.message)); + } + CResult::Err(parse_error) => { + match parse_error { + BmpParseError::Netgauze(err) => unsafe { + drop(CString::from_raw(err)); + }, + BmpParseError::RouteDistinguisher + | BmpParseError::StringConversion + | BmpParseError::IpAddr + | BmpParseError::WrongBmpMessageType => {} + } + } + }; +} \ No newline at end of file diff --git a/crates/pmacct-gauze-lib/src/result/cresult.rs b/crates/pmacct-gauze-lib/src/result/cresult.rs new file mode 100644 index 0000000..9de4030 --- /dev/null +++ b/crates/pmacct-gauze-lib/src/result/cresult.rs @@ -0,0 +1,37 @@ +use std::convert::Infallible; +use std::ops::{ControlFlow, FromResidual, Try}; + +#[repr(C)] +#[derive(Debug)] +pub enum CResult { + Ok(S), + Err(E), +} + +impl FromResidual for CResult { + fn from_residual(residual: ::Residual) -> Self { + match residual { + Err(err) => CResult::Err(err), + _ => unreachable!("residual should always be the error") + } + } +} + +impl Try for CResult { + /// [Self::Ok] + type Output = Self; + + /// Result + type Residual = Result; + + fn from_output(output: Self::Output) -> Self { + output + } + + fn branch(self) -> ControlFlow { + match self { + CResult::Ok(_) => ControlFlow::Continue(self), + CResult::Err(err) => ControlFlow::Break(Err(err)) + } + } +} \ No newline at end of file diff --git a/crates/pmacct-gauze-lib/src/result/mod.rs b/crates/pmacct-gauze-lib/src/result/mod.rs new file mode 100644 index 0000000..0dea779 --- /dev/null +++ b/crates/pmacct-gauze-lib/src/result/mod.rs @@ -0,0 +1,13 @@ +use crate::result::bgp_result::BgpParseError; +use crate::result::bmp_result::BmpParseError; + +pub mod cresult; +pub mod bmp_result; +pub mod bgp_result; + +#[repr(C)] +#[derive(Debug)] +pub enum ParseError { + ParseErrorBgp(BgpParseError), + ParseErrorBmp(BmpParseError) +} \ No newline at end of file