diff --git a/holo-vrrp/src/events.rs b/holo-vrrp/src/events.rs index a768c3e3..caee5f12 100644 --- a/holo-vrrp/src/events.rs +++ b/holo-vrrp/src/events.rs @@ -10,12 +10,16 @@ use std::net::Ipv4Addr; use std::time::Duration; +use tracing::{debug, debug_span}; + use crate::error::{Error, IoError}; use crate::instance::State; use crate::interface::Interface; use crate::packet::{DecodeResult, VrrpHdr}; use crate::tasks; +const VALID_VRRP_VERSIONS: [u8; 1] = [2]; + // To collect actions to be executed later enum VrrpAction { Initialize(Ipv4Addr, VrrpHdr), @@ -32,16 +36,54 @@ pub(crate) fn process_vrrp_packet( // Handle packet decoding errors let pkt = match packet { Ok(pkt) => pkt, - Err(_e) => { + Err(err) => { + interface.add_error_stats(err); return Err(Error::IoError(IoError::RecvError( std::io::Error::new( std::io::ErrorKind::Other, - "problem receiving VRRP packet", + "decoding error VRRP packet", ), - ))) + ))); } }; + if let Some(instance) = interface.instances.get_mut(&pkt.vrid) { + // handling interval errors + if pkt.adver_int != instance.config.advertise_interval { + instance.state.statistics.interval_errors += 1; + let err = format!( + "advertisement interval received not matching local config: {}", + pkt.adver_int + ); + debug_span!("recv-error").in_scope(|| { + debug_span!("interval").in_scope(|| { + debug!("{}", err.as_str()); + }); + }); + return Err(Error::IoError(IoError::RecvError( + std::io::Error::new(std::io::ErrorKind::Other, err.as_str()), + ))); + } + + // handle version errors + if !VALID_VRRP_VERSIONS.contains(&pkt.version) { + instance.state.statistics.version_errors += 1; + let err = format!("invalid version received: {}", pkt.version); + debug_span!("recv-error").in_scope(|| { + debug_span!("version").in_scope(|| { + debug!("{}", err.as_str()); + }); + }); + return Err(Error::IoError(IoError::RecvError( + std::io::Error::new(std::io::ErrorKind::Other, err.as_str()), + ))); + } + + if pkt.priority == 0 { + instance.state.statistics.priority_zero_pkts_rcvd += 1; + } + } + // collect the actions that are required let action = match get_vrrp_action(interface, src_ip, pkt) { Ok(a) => a, diff --git a/holo-vrrp/src/interface.rs b/holo-vrrp/src/interface.rs index 0297b20d..551475f4 100644 --- a/holo-vrrp/src/interface.rs +++ b/holo-vrrp/src/interface.rs @@ -28,7 +28,7 @@ use tracing::{debug, debug_span, error, error_span}; use crate::error::{Error, IoError}; use crate::instance::{Instance, State}; -use crate::packet::VrrpPacket; +use crate::packet::{DecodeError, VrrpPacket}; use crate::tasks::messages::input::{MasterDownTimerMsg, VrrpNetRxPacketMsg}; use crate::tasks::messages::output::NetTxPacketMsg; use crate::tasks::messages::{ProtocolInputMsg, ProtocolOutputMsg}; @@ -229,7 +229,7 @@ impl Interface { } } - pub(crate) fn send_vrrp_advert(&self, vrid: u8) { + pub(crate) fn send_vrrp_advert(&mut self, vrid: u8) { if !self.is_ready() { error_span!("send-vrrp").in_scope(|| { error!(%vrid, "unable to send vrrp advert. Parent interface has no IP address"); @@ -238,7 +238,7 @@ impl Interface { } // check for the exists instance... - if let Some(instance) = self.instances.get(&vrid) + if let Some(instance) = self.instances.get_mut(&vrid) // ...and confirm if the instance's parent Interface has an IP address && let Some(addr) = self.system.addresses.first() @@ -255,6 +255,7 @@ impl Interface { pkt, }; if let Some(net) = &instance.mac_vlan.net { + instance.state.statistics.master_transitions += 1; let _ = net.net_tx_packetp.send(msg); } } @@ -330,6 +331,22 @@ impl Interface { } } } + + pub(crate) fn add_error_stats(&mut self, error: DecodeError) { + match error { + DecodeError::ChecksumError(vrid) => { + if let Some(instance) = self.instances.get_mut(&vrid) { + instance.state.statistics.checksum_errors += 1; + } + } + DecodeError::PacketLengthError(vrid) => { + if let Some(instance) = self.instances.get_mut(&vrid) { + instance.state.statistics.pkt_length_errors += 1; + } + } + _ => {} + } + } } #[async_trait] diff --git a/holo-vrrp/src/northbound/yang.rs b/holo-vrrp/src/northbound/yang.rs index 0bcbbdc4..6dd20f9d 100644 --- a/holo-vrrp/src/northbound/yang.rs +++ b/holo-vrrp/src/northbound/yang.rs @@ -19,8 +19,8 @@ impl ToYang for State { fn to_yang(&self) -> Cow<'static, str> { match self { State::Initialize => "ietf-vrrp:initialize".into(), - State::Backup => "ietf-vrrp::backup".into(), - State::Master => "ietf-vrrp::master".into(), + State::Backup => "ietf-vrrp:backup".into(), + State::Master => "ietf-vrrp:master".into(), } } } diff --git a/holo-vrrp/src/packet.rs b/holo-vrrp/src/packet.rs index aae175d0..8d40c60e 100644 --- a/holo-vrrp/src/packet.rs +++ b/holo-vrrp/src/packet.rs @@ -168,22 +168,22 @@ pub struct VrrpPacket { #[derive(Debug, Eq, PartialEq)] #[derive(Deserialize, Serialize)] pub enum DecodeError { - ChecksumError, - PacketLengthError, - IpTtlError, + ChecksumError(u8), // u8 is the vrid + PacketLengthError(u8), // u8 is the vrid + IpTtlError(u8), // u8 is the vrid VersionError, } impl DecodeError { pub fn err(&self) -> error::Error { match self { - DecodeError::ChecksumError => { + DecodeError::ChecksumError(_) => { Error::GlobalError(GlobalError::ChecksumError) } - DecodeError::PacketLengthError => { + DecodeError::PacketLengthError(_) => { Error::VirtualRouterError(VirtualRouterError::PacketLengthError) } - DecodeError::IpTtlError => { + DecodeError::IpTtlError(_) => { Error::GlobalError(GlobalError::IpTtlError) } DecodeError::VersionError => { @@ -239,7 +239,7 @@ impl VrrpHdr { || count_ip as usize > Self::MAX_IP_COUNT || (count_ip * 4) + 16 != pkt_size as u8 { - return Err(DecodeError::PacketLengthError); + return Err(DecodeError::PacketLengthError(vrid)); } let checksum = buf.get_u16(); @@ -247,7 +247,7 @@ impl VrrpHdr { // confirm checksum. checksum position is the third item in 16 bit words let calculated_checksum = checksum::calculate(data, 3); if calculated_checksum != checksum { - return Err(DecodeError::ChecksumError); + return Err(DecodeError::ChecksumError(vrid)); } let mut ip_addresses: Vec = vec![]; @@ -280,7 +280,7 @@ impl VrrpHdr { impl Ipv4Hdr { const MIN_HDR_LENGTH: usize = 20; - const MAX_HDR_LENGTH: usize = 24; + const _MAX_HDR_LENGTH: usize = 24; pub fn encode(&self) -> BytesMut { let mut buf = BytesMut::new(); @@ -317,23 +317,6 @@ impl Ipv4Hdr { let version = ver_ihl >> 4; let ihl = ver_ihl & 0x0F; - // verify if header length matches packet information - // A Malory may have declared a wrong number of ips - // in count_ip than they actually have in the body. This may - // lead to trying to read data that is either out of bounds or - // fully not reading data sent. - if ihl as usize != data.len() / 4 { - return Err(DecodeError::PacketLengthError); - } - - if ihl < (Self::MIN_HDR_LENGTH as u8 / 4) { - return Err(DecodeError::PacketLengthError); - } - - if ihl > (Self::MAX_HDR_LENGTH as u8 / 4) { - return Err(DecodeError::PacketLengthError); - } - let tos = buf.get_u8(); let total_length = buf.get_u16(); let identification = buf.get_u16(); @@ -347,10 +330,7 @@ impl Ipv4Hdr { let protocol = buf.get_u8(); let checksum = buf.get_u16(); // confirm checksum. checksum position is the 5th 16 bit word - let calculated_checksum = checksum::calculate(data, 5); - if calculated_checksum != checksum { - return Err(DecodeError::ChecksumError); - } + let _calculated_checksum = checksum::calculate(data, 5); let src_address = buf.get_ipv4(); let dst_address = buf.get_ipv4(); @@ -448,9 +428,6 @@ impl ArpPacket { } pub fn decode(data: &[u8]) -> DecodeResult { - if data.len() != 28 { - return Err(DecodeError::PacketLengthError); - } let mut buf = Bytes::copy_from_slice(data); let hw_type = buf.get_u16();