Skip to content

Commit

Permalink
change(socket/icmp): split ICMPv4/v6 processing
Browse files Browse the repository at this point in the history 
Splitting the accept and process functions into separate functions for
IPv4 and IPv6 allows us to avoid the `IpRepr` enum and instead use the
`Ipv4Repr` and `Ipv6Repr` structs directly. This reduces the binary size
by 1.5 KiB.
  • Loading branch information
@thvdveld
thvdveld committed Jan 5, 2024
1 parent 5ee728a commit 951b59b
Show file tree
Hide file tree
Showing 18 changed files with 333 additions and 327 deletions.
14 changes: 4 additions & 10 deletions examples/ping.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,8 @@ fn main() {
remote_addr
);
icmp_repr.emit(
&iface
.get_source_address_ipv6(&address)
.unwrap()
.into_address(),
&remote_addr,
&iface.get_source_address_ipv6(&address).unwrap(),
&address,
&mut icmp_packet,
&device_caps.checksum,
);
Expand Down Expand Up @@ -223,11 +220,8 @@ fn main() {
IpAddress::Ipv6(address) => {
let icmp_packet = Icmpv6Packet::new_checked(&payload).unwrap();
let icmp_repr = Icmpv6Repr::parse(
&remote_addr,
&iface
.get_source_address_ipv6(&address)
.unwrap()
.into_address(),
&address,
&iface.get_source_address_ipv6(&address).unwrap(),
&icmp_packet,
&device_caps.checksum,
)
Expand Down
23 changes: 8 additions & 15 deletions src/iface/interface/ipv4.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,13 @@ impl InterfaceInner {
if udp_packet.src_port() == dhcp_socket.server_port
&& udp_packet.dst_port() == dhcp_socket.client_port
{
let (src_addr, dst_addr) = (ip_repr.src_addr(), ip_repr.dst_addr());
let udp_repr = check!(UdpRepr::parse(
&udp_packet,
&src_addr,
&dst_addr,
&ipv4_repr.src_addr.into(),
&ipv4_repr.dst_addr.into(),
&self.caps.checksum
));
let udp_payload = udp_packet.payload();

dhcp_socket.process(self, &ipv4_repr, &udp_repr, udp_payload);
dhcp_socket.process(self, &ipv4_repr, &udp_repr, udp_packet.payload());
return None;
}
}
Expand All @@ -200,7 +197,7 @@ impl InterfaceInner {
}

match ipv4_repr.next_header {
IpProtocol::Icmp => self.process_icmpv4(sockets, ip_repr, ip_payload),
IpProtocol::Icmp => self.process_icmpv4(sockets, ipv4_repr, ip_payload),

#[cfg(feature = "proto-igmp")]
IpProtocol::Igmp => self.process_igmp(ipv4_repr, ip_payload),
Expand Down Expand Up @@ -298,7 +295,7 @@ impl InterfaceInner {
pub(super) fn process_icmpv4<'frame>(
&mut self,
_sockets: &mut SocketSet,
ip_repr: IpRepr,
ip_repr: Ipv4Repr,
ip_payload: &'frame [u8],
) -> Option<Packet<'frame>> {
let icmp_packet = check!(Icmpv4Packet::new_checked(ip_payload));
Expand All @@ -312,8 +309,8 @@ impl InterfaceInner {
.items_mut()
.filter_map(|i| icmp::Socket::downcast_mut(&mut i.socket))
{
if icmp_socket.accepts(self, &ip_repr, &icmp_repr.into()) {
icmp_socket.process(self, &ip_repr, &icmp_repr.into());
if icmp_socket.accepts_v4(self, &ip_repr, &icmp_repr) {
icmp_socket.process_v4(self, &ip_repr, &icmp_repr);
handled_by_icmp_socket = true;
}
}
Expand All @@ -331,11 +328,7 @@ impl InterfaceInner {
seq_no,
data,
};
match ip_repr {
IpRepr::Ipv4(ipv4_repr) => self.icmpv4_reply(ipv4_repr, icmp_reply_repr),
#[allow(unreachable_patterns)]
_ => unreachable!(),
}
self.icmpv4_reply(ip_repr, icmp_reply_repr)
}

// Ignore any echo replies.
Expand Down
50 changes: 21 additions & 29 deletions src/iface/interface/ipv6.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl InterfaceInner {
ip_payload: &'frame [u8],
) -> Option<Packet<'frame>> {
match nxt_hdr {
IpProtocol::Icmpv6 => self.process_icmpv6(sockets, ipv6_repr.into(), ip_payload),
IpProtocol::Icmpv6 => self.process_icmpv6(sockets, ipv6_repr, ip_payload),

#[cfg(any(feature = "socket-udp", feature = "socket-dns"))]
IpProtocol::Udp => self.process_udp(
Expand Down Expand Up @@ -296,13 +296,13 @@ impl InterfaceInner {
pub(super) fn process_icmpv6<'frame>(
&mut self,
_sockets: &mut SocketSet,
ip_repr: IpRepr,
ip_repr: Ipv6Repr,
ip_payload: &'frame [u8],
) -> Option<Packet<'frame>> {
let icmp_packet = check!(Icmpv6Packet::new_checked(ip_payload));
let icmp_repr = check!(Icmpv6Repr::parse(
&ip_repr.src_addr(),
&ip_repr.dst_addr(),
&ip_repr.src_addr,
&ip_repr.dst_addr,
&icmp_packet,
&self.caps.checksum,
));
Expand All @@ -317,8 +317,8 @@ impl InterfaceInner {
.items_mut()
.filter_map(|i| IcmpSocket::downcast_mut(&mut i.socket))
{
if icmp_socket.accepts(self, &ip_repr, &icmp_repr.into()) {
icmp_socket.process(self, &ip_repr, &icmp_repr.into());
if icmp_socket.accepts_v6(self, &ip_repr, &icmp_repr) {
icmp_socket.process_v6(self, &ip_repr, &icmp_repr);
handled_by_icmp_socket = true;
}
}
Expand All @@ -330,35 +330,27 @@ impl InterfaceInner {
ident,
seq_no,
data,
} => match ip_repr {
IpRepr::Ipv6(ipv6_repr) => {
let icmp_reply_repr = Icmpv6Repr::EchoReply {
ident,
seq_no,
data,
};
self.icmpv6_reply(ipv6_repr, icmp_reply_repr)
}
#[allow(unreachable_patterns)]
_ => unreachable!(),
},
} => {
let icmp_reply_repr = Icmpv6Repr::EchoReply {
ident,
seq_no,
data,
};
self.icmpv6_reply(ip_repr, icmp_reply_repr)
}

// Ignore any echo replies.
Icmpv6Repr::EchoReply { .. } => None,

// Forward any NDISC packets to the ndisc packet handler
#[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))]
Icmpv6Repr::Ndisc(repr) if ip_repr.hop_limit() == 0xff => match ip_repr {
IpRepr::Ipv6(ipv6_repr) => match self.caps.medium {
#[cfg(feature = "medium-ethernet")]
Medium::Ethernet => self.process_ndisc(ipv6_repr, repr),
#[cfg(feature = "medium-ieee802154")]
Medium::Ieee802154 => self.process_ndisc(ipv6_repr, repr),
#[cfg(feature = "medium-ip")]
Medium::Ip => None,
},
#[allow(unreachable_patterns)]
_ => unreachable!(),
Icmpv6Repr::Ndisc(repr) if ip_repr.hop_limit == 0xff => match self.caps.medium {
#[cfg(feature = "medium-ethernet")]
Medium::Ethernet => self.process_ndisc(ip_repr, repr),
#[cfg(feature = "medium-ieee802154")]
Medium::Ieee802154 => self.process_ndisc(ip_repr, repr),
#[cfg(feature = "medium-ip")]
Medium::Ip => None,
},

// Don't report an error if a packet with unknown type
Expand Down
4 changes: 2 additions & 2 deletions src/iface/interface/sixlowpan.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,8 +508,8 @@ impl InterfaceInner {
match &mut packet.payload {
IpPayload::Icmpv6(icmp_repr) => {
icmp_repr.emit(
&packet.header.src_addr.into(),
&packet.header.dst_addr.into(),
&packet.header.src_addr,
&packet.header.dst_addr,
&mut Icmpv6Packet::new_unchecked(&mut buffer[..icmp_repr.buffer_len()]),
checksum_caps,
);
Expand Down
5 changes: 3 additions & 2 deletions src/iface/interface/tests/ipv4.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,6 @@ fn test_icmpv4_socket(#[case] medium: Medium) {
payload_len: 24,
hop_limit: 64,
};
let ip_repr = IpRepr::Ipv4(ipv4_repr);

// Open a socket and ensure the packet is handled due to the listening
// socket.
Expand All @@ -583,7 +582,9 @@ fn test_icmpv4_socket(#[case] medium: Medium) {
..ipv4_repr
};
assert_eq!(
iface.inner.process_icmpv4(&mut sockets, ip_repr, icmp_data),
iface
.inner
.process_icmpv4(&mut sockets, ipv4_repr, icmp_data),
Some(Packet::new_ipv4(ipv4_reply, IpPayload::Icmpv4(echo_reply)))
);

Expand Down
8 changes: 4 additions & 4 deletions src/iface/interface/tests/ipv6.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ fn parse_ipv6(data: &[u8]) -> crate::wire::Result<Packet<'_>> {
IpProtocol::IpSecAh => todo!(),
IpProtocol::Icmpv6 => {
let icmp = Icmpv6Repr::parse(
&ipv6.src_addr.into(),
&ipv6.dst_addr.into(),
&ipv6.src_addr,
&ipv6.dst_addr,
&Icmpv6Packet::new_checked(ipv6_header.payload())?,
&Default::default(),
)?;
Expand Down Expand Up @@ -707,8 +707,8 @@ fn test_handle_valid_ndisc_request(#[case] medium: Medium) {
frame.set_ethertype(EthernetProtocol::Ipv6);
ip_repr.emit(frame.payload_mut(), &ChecksumCapabilities::default());
solicit.emit(
&remote_ip_addr.into(),
&local_ip_addr.solicited_node().into(),
&remote_ip_addr,
&local_ip_addr.solicited_node(),
&mut Icmpv6Packet::new_unchecked(&mut frame.payload_mut()[ip_repr.header_len()..]),
&ChecksumCapabilities::default(),
);
Expand Down
67 changes: 35 additions & 32 deletions src/iface/neighbor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,10 @@ impl Cache {
#[cfg(test)]
mod test {
use super::*;
use crate::wire::ip::test::{MOCK_IP_ADDR_1, MOCK_IP_ADDR_2, MOCK_IP_ADDR_3, MOCK_IP_ADDR_4};
#[cfg(all(feature = "proto-ipv4", not(feature = "proto-ipv6")))]
use crate::wire::ipv4::test::{MOCK_IP_ADDR_1, MOCK_IP_ADDR_2, MOCK_IP_ADDR_3, MOCK_IP_ADDR_4};
#[cfg(feature = "proto-ipv6")]
use crate::wire::ipv6::test::{MOCK_IP_ADDR_1, MOCK_IP_ADDR_2, MOCK_IP_ADDR_3, MOCK_IP_ADDR_4};

use crate::wire::EthernetAddress;

Expand All @@ -177,45 +180,45 @@ mod test {
let mut cache = Cache::new();

assert!(!cache
.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0))
.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0))
.found());
assert!(!cache
.lookup(&MOCK_IP_ADDR_2, Instant::from_millis(0))
.lookup(&MOCK_IP_ADDR_2.into(), Instant::from_millis(0))
.found());

cache.fill(MOCK_IP_ADDR_1, HADDR_A, Instant::from_millis(0));
cache.fill(MOCK_IP_ADDR_1.into(), HADDR_A, Instant::from_millis(0));
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0)),
cache.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0)),
Answer::Found(HADDR_A)
);
assert!(!cache
.lookup(&MOCK_IP_ADDR_2, Instant::from_millis(0))
.lookup(&MOCK_IP_ADDR_2.into(), Instant::from_millis(0))
.found());
assert!(!cache
.lookup(
&MOCK_IP_ADDR_1,
&MOCK_IP_ADDR_1.into(),
Instant::from_millis(0) + Cache::ENTRY_LIFETIME * 2
)
.found(),);

cache.fill(MOCK_IP_ADDR_1, HADDR_A, Instant::from_millis(0));
cache.fill(MOCK_IP_ADDR_1.into(), HADDR_A, Instant::from_millis(0));
assert!(!cache
.lookup(&MOCK_IP_ADDR_2, Instant::from_millis(0))
.lookup(&MOCK_IP_ADDR_2.into(), Instant::from_millis(0))
.found());
}

#[test]
fn test_expire() {
let mut cache = Cache::new();

cache.fill(MOCK_IP_ADDR_1, HADDR_A, Instant::from_millis(0));
cache.fill(MOCK_IP_ADDR_1.into(), HADDR_A, Instant::from_millis(0));
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0)),
cache.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0)),
Answer::Found(HADDR_A)
);
assert!(!cache
.lookup(
&MOCK_IP_ADDR_1,
&MOCK_IP_ADDR_1.into(),
Instant::from_millis(0) + Cache::ENTRY_LIFETIME * 2
)
.found(),);
Expand All @@ -225,14 +228,14 @@ mod test {
fn test_replace() {
let mut cache = Cache::new();

cache.fill(MOCK_IP_ADDR_1, HADDR_A, Instant::from_millis(0));
cache.fill(MOCK_IP_ADDR_1.into(), HADDR_A, Instant::from_millis(0));
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0)),
cache.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0)),
Answer::Found(HADDR_A)
);
cache.fill(MOCK_IP_ADDR_1, HADDR_B, Instant::from_millis(0));
cache.fill(MOCK_IP_ADDR_1.into(), HADDR_B, Instant::from_millis(0));
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0)),
cache.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0)),
Answer::Found(HADDR_B)
);
}
Expand All @@ -241,23 +244,23 @@ mod test {
fn test_evict() {
let mut cache = Cache::new();

cache.fill(MOCK_IP_ADDR_1, HADDR_A, Instant::from_millis(100));
cache.fill(MOCK_IP_ADDR_2, HADDR_B, Instant::from_millis(50));
cache.fill(MOCK_IP_ADDR_3, HADDR_C, Instant::from_millis(200));
cache.fill(MOCK_IP_ADDR_1.into(), HADDR_A, Instant::from_millis(100));
cache.fill(MOCK_IP_ADDR_2.into(), HADDR_B, Instant::from_millis(50));
cache.fill(MOCK_IP_ADDR_3.into(), HADDR_C, Instant::from_millis(200));
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_2, Instant::from_millis(1000)),
cache.lookup(&MOCK_IP_ADDR_2.into(), Instant::from_millis(1000)),
Answer::Found(HADDR_B)
);
assert!(!cache
.lookup(&MOCK_IP_ADDR_4, Instant::from_millis(1000))
.lookup(&MOCK_IP_ADDR_4.into(), Instant::from_millis(1000))
.found());

cache.fill(MOCK_IP_ADDR_4, HADDR_D, Instant::from_millis(300));
cache.fill(MOCK_IP_ADDR_4.into(), HADDR_D, Instant::from_millis(300));
assert!(!cache
.lookup(&MOCK_IP_ADDR_2, Instant::from_millis(1000))
.lookup(&MOCK_IP_ADDR_2.into(), Instant::from_millis(1000))
.found());
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_4, Instant::from_millis(1000)),
cache.lookup(&MOCK_IP_ADDR_4.into(), Instant::from_millis(1000)),
Answer::Found(HADDR_D)
);
}
Expand All @@ -267,17 +270,17 @@ mod test {
let mut cache = Cache::new();

assert_eq!(
cache.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0)),
cache.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0)),
Answer::NotFound
);

cache.limit_rate(Instant::from_millis(0));
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(100)),
cache.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(100)),
Answer::RateLimited
);
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(2000)),
cache.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(2000)),
Answer::NotFound
);
}
Expand All @@ -286,21 +289,21 @@ mod test {
fn test_flush() {
let mut cache = Cache::new();

cache.fill(MOCK_IP_ADDR_1, HADDR_A, Instant::from_millis(0));
cache.fill(MOCK_IP_ADDR_1.into(), HADDR_A, Instant::from_millis(0));
assert_eq!(
cache.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0)),
cache.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0)),
Answer::Found(HADDR_A)
);
assert!(!cache
.lookup(&MOCK_IP_ADDR_2, Instant::from_millis(0))
.lookup(&MOCK_IP_ADDR_2.into(), Instant::from_millis(0))
.found());

cache.flush();
assert!(!cache
.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0))
.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0))
.found());
assert!(!cache
.lookup(&MOCK_IP_ADDR_1, Instant::from_millis(0))
.lookup(&MOCK_IP_ADDR_1.into(), Instant::from_millis(0))
.found());
}
}
Loading

0 comments on commit 951b59b

Please sign in to comment.