Skip to content

Commit

Permalink
Code layout and copying optimizations
Browse files Browse the repository at this point in the history
  • Loading branch information
saethlin committed Mar 10, 2021
1 parent d14ab08 commit 1ebccb9
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 25 deletions.
12 changes: 7 additions & 5 deletions crates/proto/src/error.rs
Expand Up @@ -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<crate::rr::Name>),

/// The length of rdata read was not as expected
#[error("incorrect rdata length read: {read} expected: {len}")]
Expand Down Expand Up @@ -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<Name>,
/// The record type
record_type: RecordType,
},
Expand Down Expand Up @@ -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<ProtoErrorKind>,
#[cfg(feature = "backtrace")]
backtrack: Option<ExtBacktrace>,
}
Expand All @@ -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)
}
}

Expand All @@ -261,16 +261,18 @@ impl fmt::Display for ProtoError {
}

impl From<ProtoErrorKind> for ProtoError {
#[cold]
fn from(kind: ProtoErrorKind) -> ProtoError {
ProtoError {
kind,
kind: Box::new(kind),
#[cfg(feature = "backtrace")]
backtrack: trace!(),
}
}
}

impl From<DecodeError> for ProtoError {
#[cold]
fn from(err: DecodeError) -> ProtoError {
ProtoErrorKind::Msg(err.to_string()).into()
}
Expand Down
12 changes: 12 additions & 0 deletions crates/proto/src/rr/domain/name.rs
Expand Up @@ -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<Name> {
let mut label_data = TinyVec::new();
let mut label_ends = TinyVec::new();
Expand All @@ -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]>,
Expand Down
1 change: 1 addition & 0 deletions crates/proto/src/rr/rdata/name.rs
Expand Up @@ -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> {
Name::read(decoder)
}
Expand Down
55 changes: 44 additions & 11 deletions crates/proto/src/rr/record_data.rs
Expand Up @@ -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<u8> {
let mut buf: Vec<u8> = Vec::new();
Expand All @@ -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<u16>,
) -> ProtoResult<Self> {
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<u16>,
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());
Expand All @@ -666,31 +698,31 @@ 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");
rdata::null::read(decoder, rdata_length).map(RData::NULL)
}
RecordType::NS => {
trace!("reading NS");
rdata::name::read(decoder).map(RData::NS)
return rdata.read_ns(decoder);
}
RecordType::OPENPGPKEY => {
trace!("reading OPENPGPKEY");
Expand Down Expand Up @@ -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
Expand Down
22 changes: 15 additions & 7 deletions crates/proto/src/rr/resource.rs
Expand Up @@ -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
Expand All @@ -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)?
Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/proto/src/xfer/dnssec_dns_handle.rs
Expand Up @@ -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,
},
)));
Expand Down
2 changes: 1 addition & 1 deletion crates/resolver/src/async_resolver.rs
Expand Up @@ -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
}))
);
Expand Down

0 comments on commit 1ebccb9

Please sign in to comment.