From 1ebccb9807bda918ffef7d73a4854d48e68e1359 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 9 Mar 2021 23:09:25 -0500 Subject: [PATCH] Code layout and copying optimizations --- crates/proto/src/error.rs | 12 +++-- crates/proto/src/rr/domain/name.rs | 12 +++++ crates/proto/src/rr/rdata/name.rs | 1 + crates/proto/src/rr/record_data.rs | 55 +++++++++++++++++----- crates/proto/src/rr/resource.rs | 22 ++++++--- crates/proto/src/xfer/dnssec_dns_handle.rs | 2 +- crates/resolver/src/async_resolver.rs | 2 +- 7 files changed, 81 insertions(+), 25 deletions(-) diff --git a/crates/proto/src/error.rs b/crates/proto/src/error.rs index 758a52722b..a30dcd0608 100644 --- a/crates/proto/src/error.rs +++ b/crates/proto/src/error.rs @@ -101,7 +101,7 @@ pub enum ProtoErrorKind { /// EDNS resource record label is not the root label, although required #[error("edns resource record label must be the root label (.): {0}")] - EdnsNameNotRoot(crate::rr::Name), + EdnsNameNotRoot(Box), /// The length of rdata read was not as expected #[error("incorrect rdata length read: {read} expected: {len}")] @@ -152,7 +152,7 @@ pub enum ProtoErrorKind { #[error("rrsigs are not present for record set name: {name} record_type: {record_type}")] RrsigsNotPresent { /// The record set name - name: Name, + name: Box, /// The record type record_type: RecordType, }, @@ -226,7 +226,7 @@ pub enum ProtoErrorKind { /// The error type for errors that get returned in the crate #[derive(Error, Clone, Debug)] pub struct ProtoError { - kind: ProtoErrorKind, + kind: Box, #[cfg(feature = "backtrace")] backtrack: Option, } @@ -239,7 +239,7 @@ impl ProtoError { /// If this is a ProtoErrorKind::Busy pub fn is_busy(&self) -> bool { - matches!(self.kind, ProtoErrorKind::Busy) + matches!(*self.kind, ProtoErrorKind::Busy) } } @@ -261,9 +261,10 @@ impl fmt::Display for ProtoError { } impl From for ProtoError { + #[cold] fn from(kind: ProtoErrorKind) -> ProtoError { ProtoError { - kind, + kind: Box::new(kind), #[cfg(feature = "backtrace")] backtrack: trace!(), } @@ -271,6 +272,7 @@ impl From for ProtoError { } impl From for ProtoError { + #[cold] fn from(err: DecodeError) -> ProtoError { ProtoErrorKind::Msg(err.to_string()).into() } diff --git a/crates/proto/src/rr/domain/name.rs b/crates/proto/src/rr/domain/name.rs index 90a23c0001..c152a2fa39 100644 --- a/crates/proto/src/rr/domain/name.rs +++ b/crates/proto/src/rr/domain/name.rs @@ -1107,6 +1107,7 @@ impl<'r> BinDecodable<'r> for Name { /// this has a max of 255 octets, with each label being less than 63. /// all names will be stored lowercase internally. /// This will consume the portions of the `Vec` which it is reading... + #[inline(always)] fn read(decoder: &mut BinDecoder<'r>) -> ProtoResult { let mut label_data = TinyVec::new(); let mut label_ends = TinyVec::new(); @@ -1119,6 +1120,17 @@ impl<'r> BinDecodable<'r> for Name { } } +impl Name { + #[inline(always)] + pub(crate) fn read_into(&mut self, decoder: &mut BinDecoder<'_>) -> Result<(), DecodeError> { + self.label_data.clear(); + self.label_ends.clear(); + read_inner(decoder, &mut self.label_data, &mut self.label_ends, None)?; + self.is_fqdn = true; + Ok(()) + } +} + fn read_inner( decoder: &mut BinDecoder<'_>, label_data: &mut TinyVec<[u8; 32]>, diff --git a/crates/proto/src/rr/rdata/name.rs b/crates/proto/src/rr/rdata/name.rs index 17569f5964..ae048d506a 100644 --- a/crates/proto/src/rr/rdata/name.rs +++ b/crates/proto/src/rr/rdata/name.rs @@ -44,6 +44,7 @@ use crate::rr::domain::Name; use crate::serialize::binary::*; /// Read the RData from the given Decoder +#[inline(always)] pub fn read(decoder: &mut BinDecoder<'_>) -> ProtoResult { Name::read(decoder) } diff --git a/crates/proto/src/rr/record_data.rs b/crates/proto/src/rr/record_data.rs index 800cf6351c..b6d3c162a4 100644 --- a/crates/proto/src/rr/record_data.rs +++ b/crates/proto/src/rr/record_data.rs @@ -624,6 +624,16 @@ pub enum RData { ZERO, } +macro_rules! reader { + ($fn:ident, $mod:ident, $variant:ident) => { + #[inline(never)] + fn $fn(&mut self, decoder: &mut BinDecoder<'_>) -> ProtoResult<()> { + *self = RData::$variant(rdata::$mod::read(decoder)?); + Ok(()) + } + }; +} + impl RData { fn to_bytes(&self) -> Vec { let mut buf: Vec = Vec::new(); @@ -636,26 +646,48 @@ impl RData { buf } - /// Read the RData from the given Decoder + reader!(read_a, a, A); + reader!(read_aaaa, aaaa, AAAA); + reader!(read_aname, name, ANAME); + reader!(read_cname, name, CNAME); + reader!(read_hinfo, hinfo, HINFO); + reader!(read_mx, mx, MX); + reader!(read_naptr, naptr, NAPTR); + reader!(read_ns, name, NS); + + /// Read RData from the given Decoder pub fn read( decoder: &mut BinDecoder<'_>, record_type: RecordType, rdata_length: Restrict, ) -> ProtoResult { + let mut this = RData::ZERO; + RData::read_into(decoder, record_type, rdata_length, &mut this)?; + Ok(this) + } + + /// Read RData from the given Decoder + #[inline(never)] + pub fn read_into( + decoder: &mut BinDecoder<'_>, + record_type: RecordType, + rdata_length: Restrict, + rdata: &mut Self, + ) -> ProtoResult<()> { let start_idx = decoder.index(); let result = match record_type { RecordType::A => { trace!("reading A"); - rdata::a::read(decoder).map(RData::A) + return rdata.read_a(decoder); } RecordType::AAAA => { trace!("reading AAAA"); - rdata::aaaa::read(decoder).map(RData::AAAA) + return rdata.read_aaaa(decoder); } RecordType::ANAME => { trace!("reading ANAME"); - rdata::name::read(decoder).map(RData::ANAME) + return rdata.read_aname(decoder); } rt @ RecordType::ANY | rt @ RecordType::AXFR | rt @ RecordType::IXFR => { return Err(ProtoErrorKind::UnknownRecordTypeValue(rt.into()).into()); @@ -666,23 +698,23 @@ impl RData { } RecordType::CNAME => { trace!("reading CNAME"); - rdata::name::read(decoder).map(RData::CNAME) + return rdata.read_cname(decoder); } RecordType::HINFO => { trace!("reading HINFO"); - rdata::hinfo::read(decoder).map(RData::HINFO) + return rdata.read_hinfo(decoder); } RecordType::ZERO => { trace!("reading EMPTY"); - return Ok(RData::ZERO); + Ok(RData::ZERO) } RecordType::MX => { trace!("reading MX"); - rdata::mx::read(decoder).map(RData::MX) + return rdata.read_mx(decoder); } RecordType::NAPTR => { trace!("reading NAPTR"); - rdata::naptr::read(decoder).map(RData::NAPTR) + return rdata.read_naptr(decoder); } RecordType::NULL => { trace!("reading NULL"); @@ -690,7 +722,7 @@ impl RData { } RecordType::NS => { trace!("reading NS"); - rdata::name::read(decoder).map(RData::NS) + return rdata.read_ns(decoder); } RecordType::OPENPGPKEY => { trace!("reading OPENPGPKEY"); @@ -746,7 +778,8 @@ impl RData { }) })?; - result + *rdata = result?; + Ok(()) } /// [RFC 4034](https://tools.ietf.org/html/rfc4034#section-6), DNSSEC Resource Records, March 2005 diff --git a/crates/proto/src/rr/resource.rs b/crates/proto/src/rr/resource.rs index 3b84a4a456..a89ef4c077 100644 --- a/crates/proto/src/rr/resource.rs +++ b/crates/proto/src/rr/resource.rs @@ -113,19 +113,23 @@ impl Record { pub fn read_into<'r>(decoder: &mut BinDecoder<'r>, record: &mut Record) -> ProtoResult<()> { // NAME an owner name, i.e., the name of the node to which this // resource record pertains. - record.name_labels = Name::read(decoder)?; + record.name_labels.read_into(decoder)?; // TYPE two octets containing one of the RR TYPE codes. record.rr_type = RecordType::read(decoder)?; #[cfg(feature = "mdns")] - let mut mdns_cache_flush = false; + { + record.mdns_cache_flush = false; + } // CLASS two octets containing one of the RR CLASS codes. record.dns_class = if record.rr_type == RecordType::OPT { // verify that the OPT record is Root if !record.name_labels.is_root() { - return Err(ProtoErrorKind::EdnsNameNotRoot(record.name_labels.clone()).into()); + return Err( + ProtoErrorKind::EdnsNameNotRoot(Box::new(record.name_labels.clone())).into(), + ); } // DNS Class is overloaded for OPT records in EDNS - RFC 6891 @@ -143,7 +147,7 @@ impl Record { let dns_class_value = decoder.read_u16()?.unverified(/*DNSClass::from_u16 will verify the value*/); if dns_class_value & MDNS_ENABLE_CACHE_FLUSH > 0 { - mdns_cache_flush = true; + record.mdns_cache_flush = true; DNSClass::from_u16(dns_class_value & !MDNS_ENABLE_CACHE_FLUSH)? } else { DNSClass::from_u16(dns_class_value)? @@ -185,7 +189,9 @@ impl Record { // according to the TYPE and CLASS of the resource record. // Adding restrict to the rdata length because it's used for many calculations later // and must be validated before hand - RData::read(decoder, record.rr_type, Restrict::new(rd_length))? + let mut data = RData::ZERO; // Something which is cheap to construct + RData::read_into(decoder, record.rr_type, Restrict::new(rd_length), &mut data)?; + data }; Ok(()) @@ -466,7 +472,7 @@ impl<'r> BinDecodable<'r> for Record { let class: DNSClass = if record_type == RecordType::OPT { // verify that the OPT record is Root if !name_labels.is_root() { - return Err(ProtoErrorKind::EdnsNameNotRoot(name_labels).into()); + return Err(ProtoErrorKind::EdnsNameNotRoot(Box::new(name_labels)).into()); } // DNS Class is overloaded for OPT records in EDNS - RFC 6891 @@ -526,7 +532,9 @@ impl<'r> BinDecodable<'r> for Record { // according to the TYPE and CLASS of the resource record. // Adding restrict to the rdata length because it's used for many calculations later // and must be validated before hand - RData::read(decoder, record_type, Restrict::new(rd_length))? + let mut data = RData::ZERO; // Something which is cheap to construct + RData::read_into(decoder, record_type, Restrict::new(rd_length), &mut data)?; + data }; Ok(Record { diff --git a/crates/proto/src/xfer/dnssec_dns_handle.rs b/crates/proto/src/xfer/dnssec_dns_handle.rs index 4c29331ebc..ed5130ac9a 100644 --- a/crates/proto/src/xfer/dnssec_dns_handle.rs +++ b/crates/proto/src/xfer/dnssec_dns_handle.rs @@ -763,7 +763,7 @@ where if verifications.is_empty() { return Err(E::from(ProtoError::from( ProtoErrorKind::RrsigsNotPresent { - name: rrset.name.clone(), + name: Box::new(rrset.name.clone()), record_type: rrset.record_type, }, ))); diff --git a/crates/resolver/src/async_resolver.rs b/crates/resolver/src/async_resolver.rs index 6e0c28be29..2521d90ba7 100644 --- a/crates/resolver/src/async_resolver.rs +++ b/crates/resolver/src/async_resolver.rs @@ -621,7 +621,7 @@ pub mod testing { let expected_str = format!( "{}", ResolveError::from(ProtoError::from(ProtoErrorKind::RrsigsNotPresent { - name, + name: Box::new(name), record_type: RecordType::A })) );