From fd758773d831e0f439551cb4ea44c76cb5a244a0 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Fri, 2 Aug 2024 10:43:47 +0200 Subject: [PATCH 01/25] Reset expiry of entries in the neighbor cache on packet reception --- src/iface/interface/ipv4.rs | 4 ++++ src/iface/interface/ipv6.rs | 4 ++++ src/iface/neighbor.rs | 10 ++++++++++ 3 files changed, 18 insertions(+) diff --git a/src/iface/interface/ipv4.rs b/src/iface/interface/ipv4.rs index 3a5a864ee..84b265c5e 100644 --- a/src/iface/interface/ipv4.rs +++ b/src/iface/interface/ipv4.rs @@ -196,6 +196,10 @@ impl InterfaceInner { } } + #[cfg(feature = "medium-ethernet")] + self.neighbor_cache + .reset_expiry_if_existing(IpAddress::Ipv4(ipv4_repr.src_addr), self.now); + match ipv4_repr.next_header { IpProtocol::Icmp => self.process_icmpv4(sockets, ipv4_repr, ip_payload), diff --git a/src/iface/interface/ipv6.rs b/src/iface/interface/ipv6.rs index a72492bb2..28c446304 100644 --- a/src/iface/interface/ipv6.rs +++ b/src/iface/interface/ipv6.rs @@ -228,6 +228,10 @@ impl InterfaceInner { #[cfg(not(feature = "socket-raw"))] let handled_by_raw_socket = false; + #[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))] + self.neighbor_cache + .reset_expiry_if_existing(IpAddress::Ipv6(ipv6_repr.src_addr), self.now); + self.process_nxt_hdr( sockets, meta, diff --git a/src/iface/neighbor.rs b/src/iface/neighbor.rs index 8fa1d7d5d..543d78c98 100644 --- a/src/iface/neighbor.rs +++ b/src/iface/neighbor.rs @@ -63,6 +63,16 @@ impl Cache { } } + pub fn reset_expiry_if_existing(&mut self, protocol_addr: IpAddress, timestamp: Instant) { + if let Some(Neighbor { + expires_at, + hardware_addr: _, + }) = self.storage.get_mut(&protocol_addr) + { + *expires_at = timestamp + Self::ENTRY_LIFETIME; + } + } + pub fn fill( &mut self, protocol_addr: IpAddress, From a11428dfc9ce29be7ae7eb2d7330049062ff23dc Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Fri, 2 Aug 2024 15:30:59 +0200 Subject: [PATCH 02/25] Updated based on dirbaio's comments - Check for unicast destination - Source IP matches cache - Source hardware address matches cache --- src/iface/interface/ethernet.rs | 12 +++++++++--- src/iface/interface/ipv4.rs | 10 ++++++++-- src/iface/interface/ipv6.rs | 10 ++++++++-- src/iface/interface/mod.rs | 4 ++-- src/iface/interface/sixlowpan.rs | 10 +++++++++- src/iface/interface/tests/ipv4.rs | 5 +++++ src/iface/interface/tests/ipv6.rs | 15 +++++++++++++++ src/iface/neighbor.rs | 13 ++++++++++--- src/wire/mod.rs | 24 ++++++++++++++++++++++++ 9 files changed, 90 insertions(+), 13 deletions(-) diff --git a/src/iface/interface/ethernet.rs b/src/iface/interface/ethernet.rs index 4d29faa11..da2d01229 100644 --- a/src/iface/interface/ethernet.rs +++ b/src/iface/interface/ethernet.rs @@ -25,13 +25,19 @@ impl InterfaceInner { EthernetProtocol::Ipv4 => { let ipv4_packet = check!(Ipv4Packet::new_checked(eth_frame.payload())); - self.process_ipv4(sockets, meta, &ipv4_packet, fragments) - .map(EthernetPacket::Ip) + self.process_ipv4( + sockets, + meta, + eth_frame.src_addr().into(), + &ipv4_packet, + fragments, + ) + .map(EthernetPacket::Ip) } #[cfg(feature = "proto-ipv6")] EthernetProtocol::Ipv6 => { let ipv6_packet = check!(Ipv6Packet::new_checked(eth_frame.payload())); - self.process_ipv6(sockets, meta, &ipv6_packet) + self.process_ipv6(sockets, meta, eth_frame.src_addr().into(), &ipv6_packet) .map(EthernetPacket::Ip) } // Drop all other traffic. diff --git a/src/iface/interface/ipv4.rs b/src/iface/interface/ipv4.rs index 84b265c5e..e75159ea2 100644 --- a/src/iface/interface/ipv4.rs +++ b/src/iface/interface/ipv4.rs @@ -91,6 +91,7 @@ impl InterfaceInner { &mut self, sockets: &mut SocketSet, meta: PacketMeta, + source_hardware_addr: HardwareAddress, ipv4_packet: &Ipv4Packet<&'a [u8]>, frag: &'a mut FragmentsBuffer, ) -> Option> { @@ -197,8 +198,13 @@ impl InterfaceInner { } #[cfg(feature = "medium-ethernet")] - self.neighbor_cache - .reset_expiry_if_existing(IpAddress::Ipv4(ipv4_repr.src_addr), self.now); + if self.is_unicast_v4(ipv4_repr.dst_addr) { + self.neighbor_cache.reset_expiry_if_existing( + IpAddress::Ipv4(ipv4_repr.src_addr), + source_hardware_addr, + self.now, + ); + } match ipv4_repr.next_header { IpProtocol::Icmp => self.process_icmpv4(sockets, ipv4_repr, ip_payload), diff --git a/src/iface/interface/ipv6.rs b/src/iface/interface/ipv6.rs index 28c446304..a8d29e48c 100644 --- a/src/iface/interface/ipv6.rs +++ b/src/iface/interface/ipv6.rs @@ -187,6 +187,7 @@ impl InterfaceInner { &mut self, sockets: &mut SocketSet, meta: PacketMeta, + source_hardware_addr: HardwareAddress, ipv6_packet: &Ipv6Packet<&'frame [u8]>, ) -> Option> { let ipv6_repr = check!(Ipv6Repr::parse(ipv6_packet)); @@ -229,8 +230,13 @@ impl InterfaceInner { let handled_by_raw_socket = false; #[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))] - self.neighbor_cache - .reset_expiry_if_existing(IpAddress::Ipv6(ipv6_repr.src_addr), self.now); + if ipv6_repr.dst_addr.is_unicast() { + self.neighbor_cache.reset_expiry_if_existing( + IpAddress::Ipv6(ipv6_repr.src_addr), + source_hardware_addr, + self.now, + ); + } self.process_nxt_hdr( sockets, diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs index f8c8afb69..7eed974e8 100644 --- a/src/iface/interface/mod.rs +++ b/src/iface/interface/mod.rs @@ -785,12 +785,12 @@ impl InterfaceInner { Ok(IpVersion::Ipv4) => { let ipv4_packet = check!(Ipv4Packet::new_checked(ip_payload)); - self.process_ipv4(sockets, meta, &ipv4_packet, frag) + self.process_ipv4(sockets, meta, HardwareAddress::Ip, &ipv4_packet, frag) } #[cfg(feature = "proto-ipv6")] Ok(IpVersion::Ipv6) => { let ipv6_packet = check!(Ipv6Packet::new_checked(ip_payload)); - self.process_ipv6(sockets, meta, &ipv6_packet) + self.process_ipv6(sockets, meta, HardwareAddress::Ip, &ipv6_packet) } // Drop all other traffic. _ => None, diff --git a/src/iface/interface/sixlowpan.rs b/src/iface/interface/sixlowpan.rs index 3caa446f5..a89934f1f 100644 --- a/src/iface/interface/sixlowpan.rs +++ b/src/iface/interface/sixlowpan.rs @@ -99,7 +99,15 @@ impl InterfaceInner { } }; - self.process_ipv6(sockets, meta, &check!(Ipv6Packet::new_checked(payload))) + self.process_ipv6( + sockets, + meta, + match ieee802154_repr.src_addr { + Some(s) => HardwareAddress::Ieee802154(s), + None => HardwareAddress::Ieee802154(Ieee802154Address::Absent), + }, + &check!(Ipv6Packet::new_checked(payload)), + ) } #[cfg(feature = "proto-sixlowpan-fragmentation")] diff --git a/src/iface/interface/tests/ipv4.rs b/src/iface/interface/tests/ipv4.rs index 475911bed..1c394f2a9 100644 --- a/src/iface/interface/tests/ipv4.rs +++ b/src/iface/interface/tests/ipv4.rs @@ -91,6 +91,7 @@ fn test_no_icmp_no_unicast(#[case] medium: Medium) { iface.inner.process_ipv4( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &frame, &mut iface.fragments ), @@ -152,6 +153,7 @@ fn test_icmp_error_no_payload(#[case] medium: Medium) { iface.inner.process_ipv4( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &frame, &mut iface.fragments ), @@ -397,6 +399,7 @@ fn test_handle_ipv4_broadcast(#[case] medium: Medium) { iface.inner.process_ipv4( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &frame, &mut iface.fragments ), @@ -828,6 +831,7 @@ fn test_raw_socket_no_reply(#[case] medium: Medium) { iface.inner.process_ipv4( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &frame, &mut iface.fragments ), @@ -925,6 +929,7 @@ fn test_raw_socket_with_udp_socket(#[case] medium: Medium) { iface.inner.process_ipv4( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &frame, &mut iface.fragments ), diff --git a/src/iface/interface/tests/ipv6.rs b/src/iface/interface/tests/ipv6.rs index c891ecc34..785daba11 100644 --- a/src/iface/interface/tests/ipv6.rs +++ b/src/iface/interface/tests/ipv6.rs @@ -86,6 +86,7 @@ fn any_ip(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), None @@ -98,6 +99,7 @@ fn any_ip(#[case] medium: Medium) { .process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ) .is_some()); @@ -125,6 +127,7 @@ fn multicast_source_address(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -173,6 +176,7 @@ fn hop_by_hop_skip_with_icmp(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -208,6 +212,7 @@ fn hop_by_hop_discard_with_icmp(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -262,6 +267,7 @@ fn hop_by_hop_discard_param_problem(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -319,6 +325,7 @@ fn hop_by_hop_discard_with_multicast(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -378,6 +385,7 @@ fn imcp_empty_echo_request(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -438,6 +446,7 @@ fn icmp_echo_request(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -485,6 +494,7 @@ fn icmp_echo_reply_as_input(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -533,6 +543,7 @@ fn unknown_proto_with_multicast_dst_address(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -582,6 +593,7 @@ fn unknown_proto(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -626,6 +638,7 @@ fn ndsic_neighbor_advertisement_ethernet(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -682,6 +695,7 @@ fn ndsic_neighbor_advertisement_ethernet_multicast_addr(#[case] medium: Medium) iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response @@ -734,6 +748,7 @@ fn ndsic_neighbor_advertisement_ieee802154(#[case] medium: Medium) { iface.inner.process_ipv6( &mut sockets, PacketMeta::default(), + HardwareAddress::default(), &Ipv6Packet::new_checked(&data[..]).unwrap() ), response diff --git a/src/iface/neighbor.rs b/src/iface/neighbor.rs index 543d78c98..dcef2bb87 100644 --- a/src/iface/neighbor.rs +++ b/src/iface/neighbor.rs @@ -63,13 +63,20 @@ impl Cache { } } - pub fn reset_expiry_if_existing(&mut self, protocol_addr: IpAddress, timestamp: Instant) { + pub fn reset_expiry_if_existing( + &mut self, + protocol_addr: IpAddress, + source_hardware_addr: HardwareAddress, + timestamp: Instant, + ) { if let Some(Neighbor { expires_at, - hardware_addr: _, + hardware_addr, }) = self.storage.get_mut(&protocol_addr) { - *expires_at = timestamp + Self::ENTRY_LIFETIME; + if source_hardware_addr == *hardware_addr { + *expires_at = timestamp + Self::ENTRY_LIFETIME; + } } } diff --git a/src/wire/mod.rs b/src/wire/mod.rs index a552b63b4..c197afed9 100644 --- a/src/wire/mod.rs +++ b/src/wire/mod.rs @@ -323,6 +323,30 @@ pub enum HardwareAddress { Ieee802154(Ieee802154Address), } +#[cfg(any( + feature = "medium-ip", + feature = "medium-ethernet", + feature = "medium-ieee802154" +))] +#[cfg(test)] +impl Default for HardwareAddress { + fn default() -> Self { + #![allow(unreachable_code)] + #[cfg(feature = "medium-ethernet")] + { + return Self::Ethernet(EthernetAddress::default()); + } + #[cfg(feature = "medium-ip")] + { + return Self::Ip; + } + #[cfg(feature = "medium-ieee802154")] + { + Self::Ieee802154(Ieee802154Address::default()) + } + } +} + #[cfg(any( feature = "medium-ip", feature = "medium-ethernet", From 8e97c95f23719f8b5291663a213737cbc86ca455 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Mon, 5 Aug 2024 11:23:20 +0200 Subject: [PATCH 03/25] Update default neighbor cache size to 8 entries --- Cargo.toml | 4 ++-- build.rs | 2 +- gen_config.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 576cf175b..a505a214a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -151,11 +151,11 @@ iface-max-sixlowpan-address-context-count-1024 = [] iface-neighbor-cache-count-1 = [] iface-neighbor-cache-count-2 = [] iface-neighbor-cache-count-3 = [] -iface-neighbor-cache-count-4 = [] # Default +iface-neighbor-cache-count-4 = [] iface-neighbor-cache-count-5 = [] iface-neighbor-cache-count-6 = [] iface-neighbor-cache-count-7 = [] -iface-neighbor-cache-count-8 = [] +iface-neighbor-cache-count-8 = [] # Default iface-neighbor-cache-count-16 = [] iface-neighbor-cache-count-32 = [] iface-neighbor-cache-count-64 = [] diff --git a/build.rs b/build.rs index 1ee723eb7..e1746d23f 100644 --- a/build.rs +++ b/build.rs @@ -9,7 +9,7 @@ static CONFIGS: &[(&str, usize)] = &[ ("IFACE_MAX_ADDR_COUNT", 2), ("IFACE_MAX_MULTICAST_GROUP_COUNT", 4), ("IFACE_MAX_SIXLOWPAN_ADDRESS_CONTEXT_COUNT", 4), - ("IFACE_NEIGHBOR_CACHE_COUNT", 4), + ("IFACE_NEIGHBOR_CACHE_COUNT", 8), ("IFACE_MAX_ROUTE_COUNT", 2), ("FRAGMENTATION_BUFFER_SIZE", 1500), ("ASSEMBLER_MAX_SEGMENT_COUNT", 4), diff --git a/gen_config.py b/gen_config.py index e33f74606..1407ca2d6 100644 --- a/gen_config.py +++ b/gen_config.py @@ -30,7 +30,7 @@ def feature(name, default, min, max, pow2=None): feature("iface_max_addr_count", default=2, min=1, max=8) feature("iface_max_multicast_group_count", default=4, min=1, max=1024, pow2=8) feature("iface_max_sixlowpan_address_context_count", default=4, min=1, max=1024, pow2=8) -feature("iface_neighbor_cache_count", default=4, min=1, max=1024, pow2=8) +feature("iface_neighbor_cache_count", default=8, min=1, max=1024, pow2=8) feature("iface_max_route_count", default=2, min=1, max=1024, pow2=8) feature("fragmentation_buffer_size", default=1500, min=256, max=65536, pow2=True) feature("assembler_max_segment_count", default=4, min=1, max=32, pow2=4) From 8f06109a4a5deaffe22d2609e0eaac7a5193973f Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Mon, 5 Aug 2024 11:22:12 +0200 Subject: [PATCH 04/25] Fix wrong feature flag for UDP broadcast test The UDP broadcast test was using the wrong feature flag. This commit fixes the feature flag to use the correct one. This must have slipped through the cracks when I updated the tests to use rstest. --- src/iface/interface/tests/mod.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/iface/interface/tests/mod.rs b/src/iface/interface/tests/mod.rs index d1ee45fe3..3cb5e103c 100644 --- a/src/iface/interface/tests/mod.rs +++ b/src/iface/interface/tests/mod.rs @@ -61,11 +61,15 @@ fn test_new_panic() { Interface::new(config, &mut device, Instant::ZERO); } +#[cfg(feature = "socket-udp")] #[rstest] -#[cfg(feature = "default")] -fn test_handle_udp_broadcast( - #[values(Medium::Ip, Medium::Ethernet, Medium::Ieee802154)] medium: Medium, -) { +#[case::ip(Medium::Ip)] +#[cfg(feature = "medium-ip")] +#[case::ethernet(Medium::Ethernet)] +#[cfg(feature = "medium-ethernet")] +#[case::ieee802154(Medium::Ieee802154)] +#[cfg(feature = "medium-ieee802154")] +fn test_handle_udp_broadcast(#[case] medium: Medium) { use crate::socket::udp; use crate::wire::IpEndpoint; From 94d7ab8e4dd227915f8cf6d530567e8008b51fa5 Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Mon, 5 Aug 2024 12:02:15 +0200 Subject: [PATCH 05/25] Remove unknown target-arch for TUNSETIFF syscalls The endianness of the target architecture cannot be specified using target_arch. The endianness can be specified using target_endian. The target description for mipsel-unknown-linux-gnu is found here: https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/targets/mipsel_unknown_linux_gnu.rs In this specification, the arch is set to "mips" and not "mipsel". --- src/phy/sys/linux.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/phy/sys/linux.rs b/src/phy/sys/linux.rs index c73eb4f5d..6c3388e3e 100644 --- a/src/phy/sys/linux.rs +++ b/src/phy/sys/linux.rs @@ -9,12 +9,12 @@ pub const ETH_P_IEEE802154: libc::c_short = 0x00F6; // https://github.com/golang/sys/blob/master/unix/zerrors_linux_.go pub const TUNSETIFF: libc::c_ulong = if cfg!(any( target_arch = "mips", + all(target_arch = "mips", target_endian = "little"), target_arch = "mips64", - target_arch = "mips64el", - target_arch = "mipsel", + all(target_arch = "mips64", target_endian = "little"), target_arch = "powerpc", target_arch = "powerpc64", - target_arch = "powerpc64le", + all(target_arch = "powerpc64", target_endian = "little"), target_arch = "sparc64" )) { 0x800454CA From 86e56ad16279988ab4fd905312fb62d8f8d5ecaa Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Mon, 5 Aug 2024 12:22:49 +0200 Subject: [PATCH 06/25] Remove warning for `fuzzing` feature The warning is added in 1.80.0, but can be disabled by updating the Cargo.toml file. --- Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 576cf175b..5a9a5fb7e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,9 @@ license = "0BSD" # ensure that the correct features are enabled. autoexamples = false +[lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(fuzzing)'] } + [dependencies] managed = { version = "0.8", default-features = false, features = ["map"] } byteorder = { version = "1.0", default-features = false } From 4055a19bc3a0838ac1799f8248f06793b57fdacc Mon Sep 17 00:00:00 2001 From: Lucas Bollen Date: Tue, 23 Apr 2024 15:41:31 +0200 Subject: [PATCH 07/25] Change mutability of `RxToken`'s `consume` argument. This drops the requirement that we can mutate the receive buffer, which is not always te case. --- src/phy/fault_injector.rs | 2 +- src/phy/fuzz_injector.rs | 10 +++++++--- src/phy/loopback.rs | 6 +++--- src/phy/mod.rs | 8 ++++---- src/phy/pcap_writer.rs | 2 +- src/phy/raw_socket.rs | 6 +++--- src/phy/tracer.rs | 2 +- src/phy/tuntap_interface.rs | 6 +++--- src/tests.rs | 6 +++--- 9 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/phy/fault_injector.rs b/src/phy/fault_injector.rs index fffe11a26..6a9091004 100644 --- a/src/phy/fault_injector.rs +++ b/src/phy/fault_injector.rs @@ -274,7 +274,7 @@ pub struct RxToken<'a> { impl<'a> phy::RxToken for RxToken<'a> { fn consume(self, f: F) -> R where - F: FnOnce(&mut [u8]) -> R, + F: FnOnce(&[u8]) -> R, { f(self.buf) } diff --git a/src/phy/fuzz_injector.rs b/src/phy/fuzz_injector.rs index 6769d8ec0..f0b53c850 100644 --- a/src/phy/fuzz_injector.rs +++ b/src/phy/fuzz_injector.rs @@ -1,3 +1,6 @@ +extern crate alloc; +use alloc::vec::Vec; + use crate::phy::{self, Device, DeviceCapabilities}; use crate::time::Instant; @@ -92,11 +95,12 @@ pub struct RxToken<'a, Rx: phy::RxToken, F: Fuzzer + 'a> { impl<'a, Rx: phy::RxToken, FRx: Fuzzer> phy::RxToken for RxToken<'a, Rx, FRx> { fn consume(self, f: F) -> R where - F: FnOnce(&mut [u8]) -> R, + F: FnOnce(&[u8]) -> R, { self.token.consume(|buffer| { - self.fuzzer.fuzz_packet(buffer); - f(buffer) + let mut new_buffer: Vec = buffer.to_vec(); + self.fuzzer.fuzz_packet(&mut new_buffer); + f(&mut new_buffer) }) } diff --git a/src/phy/loopback.rs b/src/phy/loopback.rs index b382ab627..cabce2c34 100644 --- a/src/phy/loopback.rs +++ b/src/phy/loopback.rs @@ -61,11 +61,11 @@ pub struct RxToken { } impl phy::RxToken for RxToken { - fn consume(mut self, f: F) -> R + fn consume(self, f: F) -> R where - F: FnOnce(&mut [u8]) -> R, + F: FnOnce(&[u8]) -> R, { - f(&mut self.buffer) + f(&self.buffer) } } diff --git a/src/phy/mod.rs b/src/phy/mod.rs index c3845d980..3354d42f6 100644 --- a/src/phy/mod.rs +++ b/src/phy/mod.rs @@ -62,11 +62,11 @@ impl phy::Device for StmPhy { struct StmPhyRxToken<'a>(&'a mut [u8]); impl<'a> phy::RxToken for StmPhyRxToken<'a> { - fn consume(mut self, f: F) -> R - where F: FnOnce(&mut [u8]) -> R + fn consume(self, f: F) -> R + where F: FnOnce(& [u8]) -> R { // TODO: receive packet into buffer - let result = f(&mut self.0); + let result = f(&self.0); println!("rx called"); result } @@ -372,7 +372,7 @@ pub trait RxToken { /// packet bytes as argument. fn consume(self, f: F) -> R where - F: FnOnce(&mut [u8]) -> R; + F: FnOnce(&[u8]) -> R; /// The Packet ID associated with the frame received by this [`RxToken`] fn meta(&self) -> PacketMeta { diff --git a/src/phy/pcap_writer.rs b/src/phy/pcap_writer.rs index aadf2a27f..671e89780 100644 --- a/src/phy/pcap_writer.rs +++ b/src/phy/pcap_writer.rs @@ -219,7 +219,7 @@ pub struct RxToken<'a, Rx: phy::RxToken, S: PcapSink> { } impl<'a, Rx: phy::RxToken, S: PcapSink> phy::RxToken for RxToken<'a, Rx, S> { - fn consume R>(self, f: F) -> R { + fn consume R>(self, f: F) -> R { self.token.consume(|buffer| { match self.mode { PcapMode::Both | PcapMode::RxOnly => self diff --git a/src/phy/raw_socket.rs b/src/phy/raw_socket.rs index 19c5b98d2..0cb7752d9 100644 --- a/src/phy/raw_socket.rs +++ b/src/phy/raw_socket.rs @@ -104,11 +104,11 @@ pub struct RxToken { } impl phy::RxToken for RxToken { - fn consume(mut self, f: F) -> R + fn consume(self, f: F) -> R where - F: FnOnce(&mut [u8]) -> R, + F: FnOnce(&[u8]) -> R, { - f(&mut self.buffer[..]) + f(&self.buffer[..]) } } diff --git a/src/phy/tracer.rs b/src/phy/tracer.rs index 48e60ec2b..999a9523b 100644 --- a/src/phy/tracer.rs +++ b/src/phy/tracer.rs @@ -94,7 +94,7 @@ pub struct RxToken { impl phy::RxToken for RxToken { fn consume(self, f: F) -> R where - F: FnOnce(&mut [u8]) -> R, + F: FnOnce(&[u8]) -> R, { self.token.consume(|buffer| { (self.writer)( diff --git a/src/phy/tuntap_interface.rs b/src/phy/tuntap_interface.rs index 32a28dbb4..45c96f508 100644 --- a/src/phy/tuntap_interface.rs +++ b/src/phy/tuntap_interface.rs @@ -93,11 +93,11 @@ pub struct RxToken { } impl phy::RxToken for RxToken { - fn consume(mut self, f: F) -> R + fn consume(self, f: F) -> R where - F: FnOnce(&mut [u8]) -> R, + F: FnOnce(&[u8]) -> R, { - f(&mut self.buffer[..]) + f(&self.buffer[..]) } } diff --git a/src/tests.rs b/src/tests.rs index ec026ab64..b9eb740be 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -120,11 +120,11 @@ pub struct RxToken { } impl phy::RxToken for RxToken { - fn consume(mut self, f: F) -> R + fn consume(self, f: F) -> R where - F: FnOnce(&mut [u8]) -> R, + F: FnOnce(&[u8]) -> R, { - f(&mut self.buffer) + f(&self.buffer) } } From ee1a9e59ac9565f712b3b47a69cbc8f0412e27c3 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Sun, 11 Aug 2024 13:37:00 +0200 Subject: [PATCH 08/25] `extern crate alloc` was added accidentally breaking downstream `no_std` --- src/phy/fuzz_injector.rs | 4 +--- src/phy/mod.rs | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/phy/fuzz_injector.rs b/src/phy/fuzz_injector.rs index f0b53c850..dbb9fe68b 100644 --- a/src/phy/fuzz_injector.rs +++ b/src/phy/fuzz_injector.rs @@ -1,8 +1,6 @@ -extern crate alloc; -use alloc::vec::Vec; - use crate::phy::{self, Device, DeviceCapabilities}; use crate::time::Instant; +use alloc::vec::Vec; // This could be fixed once associated consts are stable. const MTU: usize = 1536; diff --git a/src/phy/mod.rs b/src/phy/mod.rs index 3354d42f6..d008164e6 100644 --- a/src/phy/mod.rs +++ b/src/phy/mod.rs @@ -97,6 +97,7 @@ use crate::time::Instant; mod sys; mod fault_injector; +#[cfg(feature = "alloc")] mod fuzz_injector; #[cfg(feature = "alloc")] mod loopback; @@ -117,6 +118,7 @@ mod tuntap_interface; pub use self::sys::wait; pub use self::fault_injector::FaultInjector; +#[cfg(feature = "alloc")] pub use self::fuzz_injector::{FuzzInjector, Fuzzer}; #[cfg(feature = "alloc")] pub use self::loopback::Loopback; From 485f29cccc747087237bfd709bc6fbf94d06dd22 Mon Sep 17 00:00:00 2001 From: David Young Date: Fri, 16 Aug 2024 21:23:22 +0100 Subject: [PATCH 09/25] Make Address:v6() constructor const --- src/wire/ip.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/wire/ip.rs b/src/wire/ip.rs index b782d84e0..1d1c63d64 100644 --- a/src/wire/ip.rs +++ b/src/wire/ip.rs @@ -104,7 +104,16 @@ impl Address { /// Create an address wrapping an IPv6 address with the given octets. #[cfg(feature = "proto-ipv6")] #[allow(clippy::too_many_arguments)] - pub fn v6(a0: u16, a1: u16, a2: u16, a3: u16, a4: u16, a5: u16, a6: u16, a7: u16) -> Address { + pub const fn v6( + a0: u16, + a1: u16, + a2: u16, + a3: u16, + a4: u16, + a5: u16, + a6: u16, + a7: u16, + ) -> Address { Address::Ipv6(Ipv6Address::new(a0, a1, a2, a3, a4, a5, a6, a7)) } From 4990fb979d5d8d0114a04fe8ceddb14fddb734d5 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 20 Aug 2024 00:00:05 +0200 Subject: [PATCH 10/25] multicast: use a single map for both ipv4 and ipv6. --- src/iface/interface/igmp.rs | 77 +++++++++++++++++++------------------ src/iface/interface/mod.rs | 20 ++++------ 2 files changed, 47 insertions(+), 50 deletions(-) diff --git a/src/iface/interface/igmp.rs b/src/iface/interface/igmp.rs index fcfea32d2..d120a463d 100644 --- a/src/iface/interface/igmp.rs +++ b/src/iface/interface/igmp.rs @@ -39,20 +39,22 @@ impl Interface { where D: Device + ?Sized, { + let addr = addr.into(); self.inner.now = timestamp; - match addr.into() { + let is_not_new = self + .inner + .multicast_groups + .insert(addr, ()) + .map_err(|_| MulticastError::GroupTableFull)? + .is_some(); + if is_not_new { + return Ok(false); + } + + match addr { IpAddress::Ipv4(addr) => { - let is_not_new = self - .inner - .ipv4_multicast_groups - .insert(addr, ()) - .map_err(|_| MulticastError::GroupTableFull)? - .is_some(); - if is_not_new { - Ok(false) - } else if let Some(pkt) = self.inner.igmp_report_packet(IgmpVersion::Version2, addr) - { + if let Some(pkt) = self.inner.igmp_report_packet(IgmpVersion::Version2, addr) { // Send initial membership report let tx_token = device .transmit(timestamp) @@ -71,19 +73,10 @@ impl Interface { #[cfg(feature = "proto-ipv6")] IpAddress::Ipv6(addr) => { // Build report packet containing this new address - let report_record = &[MldAddressRecordRepr::new( + if let Some(pkt) = self.inner.mldv2_report_packet(&[MldAddressRecordRepr::new( MldRecordType::ChangeToInclude, addr, - )]; - let is_not_new = self - .inner - .ipv6_multicast_groups - .insert(addr, ()) - .map_err(|_| MulticastError::GroupTableFull)? - .is_some(); - if is_not_new { - Ok(false) - } else if let Some(pkt) = self.inner.mldv2_report_packet(report_record) { + )]) { // Send initial membership report let tx_token = device .transmit(timestamp) @@ -117,14 +110,16 @@ impl Interface { where D: Device + ?Sized, { + let addr = addr.into(); self.inner.now = timestamp; + let was_not_present = self.inner.multicast_groups.remove(&addr).is_none(); + if was_not_present { + return Ok(false); + } - match addr.into() { + match addr { IpAddress::Ipv4(addr) => { - let was_not_present = self.inner.ipv4_multicast_groups.remove(&addr).is_none(); - if was_not_present { - Ok(false) - } else if let Some(pkt) = self.inner.igmp_leave_packet(addr) { + if let Some(pkt) = self.inner.igmp_leave_packet(addr) { // Send group leave packet let tx_token = device .transmit(timestamp) @@ -142,14 +137,10 @@ impl Interface { } #[cfg(feature = "proto-ipv6")] IpAddress::Ipv6(addr) => { - let report_record = &[MldAddressRecordRepr::new( + if let Some(pkt) = self.inner.mldv2_report_packet(&[MldAddressRecordRepr::new( MldRecordType::ChangeToExclude, addr, - )]; - let was_not_present = self.inner.ipv6_multicast_groups.remove(&addr).is_none(); - if was_not_present { - Ok(false) - } else if let Some(pkt) = self.inner.mldv2_report_packet(report_record) { + )]) { // Send group leave packet let tx_token = device .transmit(timestamp) @@ -210,10 +201,14 @@ impl Interface { } if self.inner.now >= timeout => { let addr = self .inner - .ipv4_multicast_groups + .multicast_groups .iter() - .nth(next_index) - .map(|(addr, ())| *addr); + .filter_map(|(addr, _)| match addr { + IpAddress::Ipv4(addr) => Some(*addr), + #[allow(unreachable_patterns)] + _ => None, + }) + .nth(next_index); match addr { Some(addr) => { @@ -280,15 +275,21 @@ impl InterfaceInner { if group_addr.is_unspecified() && ipv4_repr.dst_addr == Ipv4Address::MULTICAST_ALL_SYSTEMS { + let ipv4_multicast_group_count = self + .multicast_groups + .keys() + .filter(|a| matches!(a, IpAddress::Ipv4(_))) + .count(); + // Are we member in any groups? - if self.ipv4_multicast_groups.iter().next().is_some() { + if ipv4_multicast_group_count != 0 { let interval = match version { IgmpVersion::Version1 => Duration::from_millis(100), IgmpVersion::Version2 => { // No dependence on a random generator // (see [#24](https://github.com/m-labs/smoltcp/issues/24)) // but at least spread reports evenly across max_resp_time. - let intervals = self.ipv4_multicast_groups.len() as u32 + 1; + let intervals = ipv4_multicast_group_count as u32 + 1; max_resp_time / intervals } }; diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs index 7eed974e8..4090327ad 100644 --- a/src/iface/interface/mod.rs +++ b/src/iface/interface/mod.rs @@ -111,10 +111,8 @@ pub struct InterfaceInner { ip_addrs: Vec, any_ip: bool, routes: Routes, - #[cfg(feature = "proto-igmp")] - ipv4_multicast_groups: LinearMap, - #[cfg(feature = "proto-ipv6")] - ipv6_multicast_groups: LinearMap, + #[cfg(any(feature = "proto-igmp", feature = "proto-ipv6"))] + multicast_groups: LinearMap, /// When to report for (all or) the next multicast group membership via IGMP #[cfg(feature = "proto-igmp")] igmp_report_state: IgmpReportState, @@ -226,10 +224,8 @@ impl Interface { routes: Routes::new(), #[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))] neighbor_cache: NeighborCache::new(), - #[cfg(feature = "proto-igmp")] - ipv4_multicast_groups: LinearMap::new(), - #[cfg(feature = "proto-ipv6")] - ipv6_multicast_groups: LinearMap::new(), + #[cfg(any(feature = "proto-igmp", feature = "proto-ipv6"))] + multicast_groups: LinearMap::new(), #[cfg(feature = "proto-igmp")] igmp_report_state: IgmpReportState::Inactive, #[cfg(feature = "medium-ieee802154")] @@ -753,17 +749,18 @@ impl InterfaceInner { /// If built without feature `proto-igmp` this function will /// always return `false` when using IPv4. fn has_multicast_group>(&self, addr: T) -> bool { - match addr.into() { + let addr = addr.into(); + match addr { #[cfg(feature = "proto-igmp")] IpAddress::Ipv4(key) => { key == Ipv4Address::MULTICAST_ALL_SYSTEMS - || self.ipv4_multicast_groups.get(&key).is_some() + || self.multicast_groups.get(&addr).is_some() } #[cfg(feature = "proto-ipv6")] IpAddress::Ipv6(key) => { key == Ipv6Address::LINK_LOCAL_ALL_NODES || self.has_solicited_node(key) - || self.ipv6_multicast_groups.get(&key).is_some() + || self.multicast_groups.get(&addr).is_some() } #[cfg(feature = "proto-rpl")] IpAddress::Ipv6(Ipv6Address::LINK_LOCAL_ALL_RPL_NODES) => true, @@ -784,7 +781,6 @@ impl InterfaceInner { #[cfg(feature = "proto-ipv4")] Ok(IpVersion::Ipv4) => { let ipv4_packet = check!(Ipv4Packet::new_checked(ip_payload)); - self.process_ipv4(sockets, meta, HardwareAddress::Ip, &ipv4_packet, frag) } #[cfg(feature = "proto-ipv6")] From bde1a1ee6dc4b0191214292fce91bc3ad1e8d2b4 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 21 Aug 2024 00:39:31 +0200 Subject: [PATCH 11/25] tcp: do not ignore fin if segment is partially outside the window. --- src/socket/tcp.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 946d6de88..f944875ad 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -1710,9 +1710,9 @@ impl<'a> Socket<'a> { let mut control = repr.control; control = control.quash_psh(); - // If a FIN is received at the end of the current segment but the start of the segment - // is not at the start of the receive window, disregard this FIN. - if control == TcpControl::Fin && window_start != segment_start { + // If a FIN is received at the end of the current segment, but + // we have a hole in the assembler before the current segment, disregard this FIN. + if control == TcpControl::Fin && window_start < segment_start { tcp_trace!("ignoring FIN because we don't have full data yet. window_start={} segment_start={}", window_start, segment_start); control = TcpControl::None; } @@ -4237,6 +4237,50 @@ mod test { .unwrap(); } + #[test] + fn test_established_receive_partially_outside_window_fin() { + let mut s = socket_established(); + + send!( + s, + TcpRepr { + seq_number: REMOTE_SEQ + 1, + ack_number: Some(LOCAL_SEQ + 1), + payload: &b"abc"[..], + ..SEND_TEMPL + } + ); + + s.recv(|data| { + assert_eq!(data, b"abc"); + (3, ()) + }) + .unwrap(); + + // Peer decides to retransmit (perhaps because the ACK was lost) + // and also pushed data, and sent a FIN. + send!( + s, + TcpRepr { + seq_number: REMOTE_SEQ + 1, + ack_number: Some(LOCAL_SEQ + 1), + control: TcpControl::Fin, + payload: &b"abcdef"[..], + ..SEND_TEMPL + } + ); + + s.recv(|data| { + assert_eq!(data, b"def"); + (3, ()) + }) + .unwrap(); + + // We should accept the FIN, because even though the last packet was partially + // outside the receive window, there is no hole after adding its data to the assembler. + assert_eq!(s.state, State::CloseWait); + } + #[test] fn test_established_send_wrap() { let mut s = socket_established(); From 957a39000cc1fe784ed0838b4d0fdf193cf1a2e6 Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Fri, 30 Aug 2024 09:43:40 +0200 Subject: [PATCH 12/25] Update license copyright holder Update the copyright holder in the license file. --- LICENSE-0BSD.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE-0BSD.txt b/LICENSE-0BSD.txt index 427fafaca..f4d92fe72 100644 --- a/LICENSE-0BSD.txt +++ b/LICENSE-0BSD.txt @@ -1,4 +1,4 @@ -Copyright (C) 2016 whitequark@whitequark.org +Copyright (C) smoltcp contributors Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted. From c76f32fa6c8034c3fc181965380ffa369156946e Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Wed, 11 Sep 2024 11:08:14 +0200 Subject: [PATCH 13/25] Use own src address for ARP and NDISC Solicits When looking up the destination link-layer address for a given destination address, we need to send an ARP or ICMPv6 Neighbor Solicitation if the destination link-layer address is not in the cache. When transmitting an ARP or ICMPv6 Neighbor Solicitation packet, use the source address of the interface instead of the source address of the packet that is tried to be sent. By using the source address of the interface, we are sure that a response will be received on the same interface. --- src/iface/interface/mod.rs | 30 ++++++++++-------------------- src/iface/interface/tests/ipv4.rs | 3 --- src/iface/interface/tests/ipv6.rs | 1 - 3 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs index 7eed974e8..47b2ae006 100644 --- a/src/iface/interface/mod.rs +++ b/src/iface/interface/mod.rs @@ -898,7 +898,6 @@ impl InterfaceInner { fn lookup_hardware_addr( &mut self, tx_token: Tx, - src_addr: &IpAddress, dst_addr: &IpAddress, fragmenter: &mut Fragmenter, ) -> Result<(HardwareAddress, Tx), DispatchError> @@ -966,11 +965,9 @@ impl InterfaceInner { _ => (), // XXX } - match (src_addr, dst_addr) { + match dst_addr { #[cfg(all(feature = "medium-ethernet", feature = "proto-ipv4"))] - (&IpAddress::Ipv4(src_addr), IpAddress::Ipv4(dst_addr)) - if matches!(self.caps.medium, Medium::Ethernet) => - { + IpAddress::Ipv4(dst_addr) if matches!(self.caps.medium, Medium::Ethernet) => { net_debug!( "address {} not in neighbor cache, sending ARP request", dst_addr @@ -980,7 +977,9 @@ impl InterfaceInner { let arp_repr = ArpRepr::EthernetIpv4 { operation: ArpOperation::Request, source_hardware_addr: src_hardware_addr, - source_protocol_addr: src_addr, + source_protocol_addr: self + .get_source_address_ipv4(&dst_addr) + .ok_or(DispatchError::NoRoute)?, target_hardware_addr: EthernetAddress::BROADCAST, target_protocol_addr: dst_addr, }; @@ -999,7 +998,7 @@ impl InterfaceInner { } #[cfg(feature = "proto-ipv6")] - (&IpAddress::Ipv6(src_addr), IpAddress::Ipv6(dst_addr)) => { + IpAddress::Ipv6(dst_addr) => { net_debug!( "address {} not in neighbor cache, sending Neighbor Solicitation", dst_addr @@ -1012,7 +1011,7 @@ impl InterfaceInner { let packet = Packet::new_ipv6( Ipv6Repr { - src_addr, + src_addr: self.get_source_address_ipv6(&dst_addr), dst_addr: dst_addr.solicited_node(), next_header: IpProtocol::Icmpv6, payload_len: solicit.buffer_len(), @@ -1059,12 +1058,8 @@ impl InterfaceInner { #[cfg(feature = "medium-ieee802154")] if matches!(self.caps.medium, Medium::Ieee802154) { - let (addr, tx_token) = self.lookup_hardware_addr( - tx_token, - &ip_repr.src_addr(), - &ip_repr.dst_addr(), - frag, - )?; + let (addr, tx_token) = + self.lookup_hardware_addr(tx_token, &ip_repr.dst_addr(), frag)?; let addr = addr.ieee802154_or_panic(); self.dispatch_ieee802154(addr, tx_token, meta, packet, frag); @@ -1091,12 +1086,7 @@ impl InterfaceInner { #[cfg(feature = "medium-ethernet")] let (dst_hardware_addr, mut tx_token) = match self.caps.medium { Medium::Ethernet => { - match self.lookup_hardware_addr( - tx_token, - &ip_repr.src_addr(), - &ip_repr.dst_addr(), - frag, - )? { + match self.lookup_hardware_addr(tx_token, &ip_repr.dst_addr(), frag)? { (HardwareAddress::Ethernet(addr), tx_token) => (addr, tx_token), (_, _) => unreachable!(), } diff --git a/src/iface/interface/tests/ipv4.rs b/src/iface/interface/tests/ipv4.rs index 1c394f2a9..c2c8ce460 100644 --- a/src/iface/interface/tests/ipv4.rs +++ b/src/iface/interface/tests/ipv4.rs @@ -456,7 +456,6 @@ fn test_handle_valid_arp_request(#[case] medium: Medium) { assert_eq!( iface.inner.lookup_hardware_addr( MockTxToken, - &IpAddress::Ipv4(local_ip_addr), &IpAddress::Ipv4(remote_ip_addr), &mut iface.fragmenter, ), @@ -505,7 +504,6 @@ fn test_handle_other_arp_request(#[case] medium: Medium) { assert_eq!( iface.inner.lookup_hardware_addr( MockTxToken, - &IpAddress::Ipv4(Ipv4Address([0x7f, 0x00, 0x00, 0x01])), &IpAddress::Ipv4(remote_ip_addr), &mut iface.fragmenter, ), @@ -564,7 +562,6 @@ fn test_arp_flush_after_update_ip(#[case] medium: Medium) { assert_eq!( iface.inner.lookup_hardware_addr( MockTxToken, - &IpAddress::Ipv4(local_ip_addr), &IpAddress::Ipv4(remote_ip_addr), &mut iface.fragmenter, ), diff --git a/src/iface/interface/tests/ipv6.rs b/src/iface/interface/tests/ipv6.rs index 785daba11..f6e214e76 100644 --- a/src/iface/interface/tests/ipv6.rs +++ b/src/iface/interface/tests/ipv6.rs @@ -834,7 +834,6 @@ fn test_handle_valid_ndisc_request(#[case] medium: Medium) { assert_eq!( iface.inner.lookup_hardware_addr( MockTxToken, - &IpAddress::Ipv6(local_ip_addr), &IpAddress::Ipv6(remote_ip_addr), &mut iface.fragmenter, ), From 4739cc7fc6828ff0b588620e3e423cbbb4fe1af9 Mon Sep 17 00:00:00 2001 From: Jamie Bird Date: Thu, 12 Sep 2024 11:28:27 +0100 Subject: [PATCH 14/25] Fix: Prevent panic in DNS Socket when server list exceeds max count - Truncate the servers list to DNS_MAX_SERVER_COUNT to prevent panics. - Ensure only the first `DNS_MAX_SERVER_COUNT` servers are used when constructing the `Socket`. - This prevents overflow issues when the provided server list is larger than the allowed maximum. --- src/socket/dns.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/socket/dns.rs b/src/socket/dns.rs index 3b9fb6555..21f40e1a3 100644 --- a/src/socket/dns.rs +++ b/src/socket/dns.rs @@ -1,3 +1,4 @@ +use core::cmp::min; #[cfg(feature = "async")] use core::task::Waker; @@ -149,15 +150,15 @@ pub struct Socket<'a> { impl<'a> Socket<'a> { /// Create a DNS socket. /// - /// # Panics - /// - /// Panics if `servers.len() > MAX_SERVER_COUNT` + /// Truncates the server list if `servers.len() > MAX_SERVER_COUNT` pub fn new(servers: &[IpAddress], queries: Q) -> Socket<'a> where Q: Into>>, { + let truncated_servers = &servers[..min(servers.len(), DNS_MAX_SERVER_COUNT)]; + Socket { - servers: Vec::from_slice(servers).unwrap(), + servers: Vec::from_slice(truncated_servers).unwrap(), queries: queries.into(), hop_limit: None, } @@ -165,11 +166,14 @@ impl<'a> Socket<'a> { /// Update the list of DNS servers, will replace all existing servers /// - /// # Panics - /// - /// Panics if `servers.len() > MAX_SERVER_COUNT` + /// Truncates the server list if `servers.len() > MAX_SERVER_COUNT` pub fn update_servers(&mut self, servers: &[IpAddress]) { - self.servers = Vec::from_slice(servers).unwrap(); + if servers.len() > DNS_MAX_SERVER_COUNT { + net_trace!("Max DNS Servers exceeded. Increase MAX_SERVER_COUNT"); + self.servers = Vec::from_slice(&servers[..DNS_MAX_SERVER_COUNT]).unwrap(); + } else { + self.servers = Vec::from_slice(servers).unwrap(); + } } /// Return the time-to-live (IPv4) or hop limit (IPv6) value used in outgoing packets. From 115fd06f03df2df01768c909556ccef1cadc84ff Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Thu, 12 Sep 2024 14:52:10 +0200 Subject: [PATCH 15/25] No panic if no valid src addr for DNS query Don't panic if there is no valid source address for the DNS query. The state of the query is set to `Failed` instead and the query is abandoned. --- src/socket/dns.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/socket/dns.rs b/src/socket/dns.rs index 3b9fb6555..1e35270c8 100644 --- a/src/socket/dns.rs +++ b/src/socket/dns.rs @@ -612,7 +612,15 @@ impl<'a> Socket<'a> { }; let dst_addr = servers[pq.server_idx]; - let src_addr = cx.get_source_address(&dst_addr).unwrap(); // TODO remove unwrap + let src_addr = match cx.get_source_address(&dst_addr) { + Some(src_addr) => src_addr, + None => { + net_trace!("no source address for destination {}", dst_addr); + q.set_state(State::Failure); + continue; + } + }; + let ip_repr = IpRepr::new( src_addr, dst_addr, From 34b1fa505bb69dcd5fa531c540380530db14c46a Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 12 Sep 2024 01:02:10 +0200 Subject: [PATCH 16/25] iface: do not require device and timestamp for multicast join/leave. Instead of eagerly sending the join/leave packet when the user calls join/leave, we update internal state and send the packet when the interface is polled. Advantages: - If the device is exhausted, the packet gets sent later instead of just failing and returning an error to the user. - Makes the API consistent with everything else in smoltcp: operations only update internal state, poll is what sends/receives packets. - Enables wrappers to offer simpler APIs with less generics. See https://github.com/embassy-rs/embassy/pull/3329 for an example, which is my original motivation. --- examples/multicast.rs | 6 +- examples/multicast6.rs | 2 +- src/iface/interface/igmp.rs | 244 +++++++++++++++--------------- src/iface/interface/mod.rs | 29 +++- src/iface/interface/tests/ipv4.rs | 10 +- src/iface/interface/tests/ipv6.rs | 14 +- 6 files changed, 160 insertions(+), 145 deletions(-) diff --git a/examples/multicast.rs b/examples/multicast.rs index ea89a2e93..44162d6d3 100644 --- a/examples/multicast.rs +++ b/examples/multicast.rs @@ -82,11 +82,7 @@ fn main() { // Join a multicast group to receive mDNS traffic iface - .join_multicast_group( - &mut device, - Ipv4Address::from_bytes(&MDNS_GROUP), - Instant::now(), - ) + .join_multicast_group(Ipv4Address::from_bytes(&MDNS_GROUP)) .unwrap(); loop { diff --git a/examples/multicast6.rs b/examples/multicast6.rs index 814c4fe1e..46e4e7bd7 100644 --- a/examples/multicast6.rs +++ b/examples/multicast6.rs @@ -66,7 +66,7 @@ fn main() { // Join a multicast group iface - .join_multicast_group(&mut device, Ipv6Address::from_parts(&GROUP), Instant::now()) + .join_multicast_group(Ipv6Address::from_parts(&GROUP)) .unwrap(); loop { diff --git a/src/iface/interface/igmp.rs b/src/iface/interface/igmp.rs index d120a463d..e5506a98e 100644 --- a/src/iface/interface/igmp.rs +++ b/src/iface/interface/igmp.rs @@ -4,8 +4,6 @@ use super::*; #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum MulticastError { - /// The hardware device transmit buffer is full. Try again later. - Exhausted, /// The table of joined multicast groups is already full. GroupTableFull, /// Cannot join/leave the given multicast group. @@ -15,7 +13,6 @@ pub enum MulticastError { impl core::fmt::Display for MulticastError { fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { match self { - MulticastError::Exhausted => write!(f, "Exhausted"), MulticastError::GroupTableFull => write!(f, "GroupTableFull"), MulticastError::Unaddressable => write!(f, "Unaddressable"), } @@ -27,138 +24,52 @@ impl std::error::Error for MulticastError {} impl Interface { /// Add an address to a list of subscribed multicast IP addresses. - /// - /// Returns `Ok(announce_sent)` if the address was added successfully, where `announce_sent` - /// indicates whether an initial immediate announcement has been sent. - pub fn join_multicast_group>( + pub fn join_multicast_group>( &mut self, - device: &mut D, addr: T, - timestamp: Instant, - ) -> Result - where - D: Device + ?Sized, - { + ) -> Result<(), MulticastError> { let addr = addr.into(); - self.inner.now = timestamp; - - let is_not_new = self - .inner - .multicast_groups - .insert(addr, ()) - .map_err(|_| MulticastError::GroupTableFull)? - .is_some(); - if is_not_new { - return Ok(false); + if !addr.is_multicast() { + return Err(MulticastError::Unaddressable); } - match addr { - IpAddress::Ipv4(addr) => { - if let Some(pkt) = self.inner.igmp_report_packet(IgmpVersion::Version2, addr) { - // Send initial membership report - let tx_token = device - .transmit(timestamp) - .ok_or(MulticastError::Exhausted)?; - - // NOTE(unwrap): packet destination is multicast, which is always routable and doesn't require neighbor discovery. - self.inner - .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) - .unwrap(); - - Ok(true) - } else { - Ok(false) - } - } - #[cfg(feature = "proto-ipv6")] - IpAddress::Ipv6(addr) => { - // Build report packet containing this new address - if let Some(pkt) = self.inner.mldv2_report_packet(&[MldAddressRecordRepr::new( - MldRecordType::ChangeToInclude, - addr, - )]) { - // Send initial membership report - let tx_token = device - .transmit(timestamp) - .ok_or(MulticastError::Exhausted)?; - - // NOTE(unwrap): packet destination is multicast, which is always routable and doesn't require neighbor discovery. - self.inner - .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) - .unwrap(); - - Ok(true) - } else { - Ok(false) - } - } - #[allow(unreachable_patterns)] - _ => Err(MulticastError::Unaddressable), + if let Some(state) = self.inner.multicast_groups.get_mut(&addr) { + *state = match state { + MulticastGroupState::Joining => MulticastGroupState::Joining, + MulticastGroupState::Joined => MulticastGroupState::Joined, + MulticastGroupState::Leaving => MulticastGroupState::Joined, + }; + } else { + self.inner + .multicast_groups + .insert(addr, MulticastGroupState::Joining) + .map_err(|_| MulticastError::GroupTableFull)?; } + Ok(()) } /// Remove an address from the subscribed multicast IP addresses. - /// - /// Returns `Ok(leave_sent)` if the address was removed successfully, where `leave_sent` - /// indicates whether an immediate leave packet has been sent. - pub fn leave_multicast_group>( + pub fn leave_multicast_group>( &mut self, - device: &mut D, addr: T, - timestamp: Instant, - ) -> Result - where - D: Device + ?Sized, - { + ) -> Result<(), MulticastError> { let addr = addr.into(); - self.inner.now = timestamp; - let was_not_present = self.inner.multicast_groups.remove(&addr).is_none(); - if was_not_present { - return Ok(false); + if !addr.is_multicast() { + return Err(MulticastError::Unaddressable); } - match addr { - IpAddress::Ipv4(addr) => { - if let Some(pkt) = self.inner.igmp_leave_packet(addr) { - // Send group leave packet - let tx_token = device - .transmit(timestamp) - .ok_or(MulticastError::Exhausted)?; - - // NOTE(unwrap): packet destination is multicast, which is always routable and doesn't require neighbor discovery. - self.inner - .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) - .unwrap(); - - Ok(true) - } else { - Ok(false) - } + if let Some(state) = self.inner.multicast_groups.get_mut(&addr) { + let delete; + (*state, delete) = match state { + MulticastGroupState::Joining => (MulticastGroupState::Joined, true), + MulticastGroupState::Joined => (MulticastGroupState::Leaving, false), + MulticastGroupState::Leaving => (MulticastGroupState::Leaving, false), + }; + if delete { + self.inner.multicast_groups.remove(&addr); } - #[cfg(feature = "proto-ipv6")] - IpAddress::Ipv6(addr) => { - if let Some(pkt) = self.inner.mldv2_report_packet(&[MldAddressRecordRepr::new( - MldRecordType::ChangeToExclude, - addr, - )]) { - // Send group leave packet - let tx_token = device - .transmit(timestamp) - .ok_or(MulticastError::Exhausted)?; - - // NOTE(unwrap): packet destination is multicast, which is always routable and doesn't require neighbor discovery. - self.inner - .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) - .unwrap(); - - Ok(true) - } else { - Ok(false) - } - } - #[allow(unreachable_patterns)] - _ => Err(MulticastError::Unaddressable), } + Ok(()) } /// Check whether the interface listens to given destination multicast IP address. @@ -166,12 +77,101 @@ impl Interface { self.inner.has_multicast_group(addr) } - /// Depending on `igmp_report_state` and the therein contained - /// timeouts, send IGMP membership reports. - pub(crate) fn igmp_egress(&mut self, device: &mut D) -> bool + /// Do multicast egress. + /// + /// - Send join/leave packets according to the multicast group state. + /// - Depending on `igmp_report_state` and the therein contained + /// timeouts, send IGMP membership reports. + pub(crate) fn multicast_egress(&mut self, device: &mut D) -> bool where D: Device + ?Sized, { + // Process multicast joins. + while let Some((&addr, _)) = self + .inner + .multicast_groups + .iter() + .find(|(_, &state)| state == MulticastGroupState::Joining) + { + match addr { + IpAddress::Ipv4(addr) => { + if let Some(pkt) = self.inner.igmp_report_packet(IgmpVersion::Version2, addr) { + let Some(tx_token) = device.transmit(self.inner.now) else { + break; + }; + + // NOTE(unwrap): packet destination is multicast, which is always routable and doesn't require neighbor discovery. + self.inner + .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) + .unwrap(); + } + } + #[cfg(feature = "proto-ipv6")] + IpAddress::Ipv6(addr) => { + if let Some(pkt) = self.inner.mldv2_report_packet(&[MldAddressRecordRepr::new( + MldRecordType::ChangeToInclude, + addr, + )]) { + let Some(tx_token) = device.transmit(self.inner.now) else { + break; + }; + + // NOTE(unwrap): packet destination is multicast, which is always routable and doesn't require neighbor discovery. + self.inner + .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) + .unwrap(); + } + } + } + + // NOTE(unwrap): this is always replacing an existing entry, so it can't fail due to the map being full. + self.inner + .multicast_groups + .insert(addr, MulticastGroupState::Joined) + .unwrap(); + } + + // Process multicast leaves. + while let Some((&addr, _)) = self + .inner + .multicast_groups + .iter() + .find(|(_, &state)| state == MulticastGroupState::Leaving) + { + match addr { + IpAddress::Ipv4(addr) => { + if let Some(pkt) = self.inner.igmp_leave_packet(addr) { + let Some(tx_token) = device.transmit(self.inner.now) else { + break; + }; + + // NOTE(unwrap): packet destination is multicast, which is always routable and doesn't require neighbor discovery. + self.inner + .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) + .unwrap(); + } + } + #[cfg(feature = "proto-ipv6")] + IpAddress::Ipv6(addr) => { + if let Some(pkt) = self.inner.mldv2_report_packet(&[MldAddressRecordRepr::new( + MldRecordType::ChangeToExclude, + addr, + )]) { + let Some(tx_token) = device.transmit(self.inner.now) else { + break; + }; + + // NOTE(unwrap): packet destination is multicast, which is always routable and doesn't require neighbor discovery. + self.inner + .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) + .unwrap(); + } + } + } + + self.inner.multicast_groups.remove(&addr); + } + match self.inner.igmp_report_state { IgmpReportState::ToSpecificQuery { version, diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs index 870e88e61..b2cbdc399 100644 --- a/src/iface/interface/mod.rs +++ b/src/iface/interface/mod.rs @@ -82,6 +82,16 @@ pub struct Interface { fragmenter: Fragmenter, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum MulticastGroupState { + /// Joining group, we have to send the join packet. + Joining, + /// We've already sent the join packet, we have nothing to do. + Joined, + /// We want to leave the group, we have to send a leave packet. + Leaving, +} + /// The device independent part of an Ethernet network interface. /// /// Separating the device from the data required for processing and dispatching makes @@ -112,7 +122,7 @@ pub struct InterfaceInner { any_ip: bool, routes: Routes, #[cfg(any(feature = "proto-igmp", feature = "proto-ipv6"))] - multicast_groups: LinearMap, + multicast_groups: LinearMap, /// When to report for (all or) the next multicast group membership via IGMP #[cfg(feature = "proto-igmp")] igmp_report_state: IgmpReportState, @@ -437,7 +447,7 @@ impl Interface { #[cfg(feature = "proto-igmp")] { - readiness_may_have_changed |= self.igmp_egress(device); + readiness_may_have_changed |= self.multicast_egress(device); } readiness_may_have_changed @@ -749,18 +759,29 @@ impl InterfaceInner { /// If built without feature `proto-igmp` this function will /// always return `false` when using IPv4. fn has_multicast_group>(&self, addr: T) -> bool { + /// Return false if we don't have the multicast group, + /// or we're leaving it. + fn wanted_state(x: Option<&MulticastGroupState>) -> bool { + match x { + None => false, + Some(MulticastGroupState::Joining) => true, + Some(MulticastGroupState::Joined) => true, + Some(MulticastGroupState::Leaving) => false, + } + } + let addr = addr.into(); match addr { #[cfg(feature = "proto-igmp")] IpAddress::Ipv4(key) => { key == Ipv4Address::MULTICAST_ALL_SYSTEMS - || self.multicast_groups.get(&addr).is_some() + || wanted_state(self.multicast_groups.get(&addr)) } #[cfg(feature = "proto-ipv6")] IpAddress::Ipv6(key) => { key == Ipv6Address::LINK_LOCAL_ALL_NODES || self.has_solicited_node(key) - || self.multicast_groups.get(&addr).is_some() + || wanted_state(self.multicast_groups.get(&addr)) } #[cfg(feature = "proto-rpl")] IpAddress::Ipv6(Ipv6Address::LINK_LOCAL_ALL_RPL_NODES) => true, diff --git a/src/iface/interface/tests/ipv4.rs b/src/iface/interface/tests/ipv4.rs index c2c8ce460..c9e100fc8 100644 --- a/src/iface/interface/tests/ipv4.rs +++ b/src/iface/interface/tests/ipv4.rs @@ -702,10 +702,9 @@ fn test_handle_igmp(#[case] medium: Medium) { // Join multicast groups let timestamp = Instant::ZERO; for group in &groups { - iface - .join_multicast_group(&mut device, *group, timestamp) - .unwrap(); + iface.join_multicast_group(*group).unwrap(); } + iface.poll(timestamp, &mut device, &mut sockets); let reports = recv_igmp(&mut device, timestamp); assert_eq!(reports.len(), 2); @@ -745,10 +744,9 @@ fn test_handle_igmp(#[case] medium: Medium) { // Leave multicast groups let timestamp = Instant::ZERO; for group in &groups { - iface - .leave_multicast_group(&mut device, *group, timestamp) - .unwrap(); + iface.leave_multicast_group(*group).unwrap(); } + iface.poll(timestamp, &mut device, &mut sockets); let leaves = recv_igmp(&mut device, timestamp); assert_eq!(leaves.len(), 2); diff --git a/src/iface/interface/tests/ipv6.rs b/src/iface/interface/tests/ipv6.rs index f6e214e76..620712c9c 100644 --- a/src/iface/interface/tests/ipv6.rs +++ b/src/iface/interface/tests/ipv6.rs @@ -1289,7 +1289,7 @@ fn test_join_ipv6_multicast_group(#[case] medium: Medium) { .collect::>() } - let (mut iface, _sockets, mut device) = setup(medium); + let (mut iface, mut sockets, mut device) = setup(medium); let groups = [ Ipv6Address::from_parts(&[0xff05, 0, 0, 0, 0, 0, 0, 0x00fb]), @@ -1299,12 +1299,12 @@ fn test_join_ipv6_multicast_group(#[case] medium: Medium) { let timestamp = Instant::from_millis(0); for &group in &groups { - iface - .join_multicast_group(&mut device, group, timestamp) - .unwrap(); + iface.join_multicast_group(group).unwrap(); assert!(iface.has_multicast_group(group)); } assert!(iface.has_multicast_group(Ipv6Address::LINK_LOCAL_ALL_NODES)); + iface.poll(timestamp, &mut device, &mut sockets); + assert!(iface.has_multicast_group(Ipv6Address::LINK_LOCAL_ALL_NODES)); let reports = recv_icmpv6(&mut device, timestamp); assert_eq!(reports.len(), 2); @@ -1374,9 +1374,9 @@ fn test_join_ipv6_multicast_group(#[case] medium: Medium) { } ); - iface - .leave_multicast_group(&mut device, group_addr, timestamp) - .unwrap(); + iface.leave_multicast_group(group_addr).unwrap(); + assert!(!iface.has_multicast_group(group_addr)); + iface.poll(timestamp, &mut device, &mut sockets); assert!(!iface.has_multicast_group(group_addr)); } } From 07f09911bcf36057c5232405cf2687799ed3ec5a Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 12 Sep 2024 16:16:08 +0200 Subject: [PATCH 17/25] multicast: use single Cargo feature for both ipv4 and ipv6. The multicast feature was somewhat broken before, there was no way to do IPv6-only multicast since a lot of multicast code was gated behind `proto-igmp` which force-enables ipv4. Solution: - Added feature `multicast` which enables multicast for ipv4 and/or ipv6, whatever the user has enabled. - Removed feature `proto-igmp`. IGMP support in `wire` is always built if ipv4 is enabled. This causes no bloat if unused, and the increase in compile time is negligible. `iface` only uses it if the `multicast` feature is enabled. --- Cargo.toml | 9 +- ci.sh | 12 +- src/iface/interface/ipv4.rs | 45 +--- src/iface/interface/mod.rs | 76 ++----- src/iface/interface/{igmp.rs => multicast.rs} | 199 ++++++++++++++---- src/iface/interface/tests/ipv4.rs | 4 +- src/iface/interface/tests/mod.rs | 4 +- src/iface/mod.rs | 4 +- src/iface/packet.rs | 22 +- src/wire/mod.rs | 4 +- 10 files changed, 197 insertions(+), 182 deletions(-) rename src/iface/interface/{igmp.rs => multicast.rs} (66%) diff --git a/Cargo.toml b/Cargo.toml index 2f3e266bd..9d42b06a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,7 +50,6 @@ defmt = ["dep:defmt", "heapless/defmt-03"] "proto-ipv4" = [] "proto-ipv4-fragmentation" = ["proto-ipv4", "_proto-fragmentation"] -"proto-igmp" = ["proto-ipv4"] "proto-dhcpv4" = ["proto-ipv4"] "proto-ipv6" = [] "proto-ipv6-hbh" = ["proto-ipv6"] @@ -64,6 +63,8 @@ defmt = ["dep:defmt", "heapless/defmt-03"] "proto-ipsec-ah" = [] "proto-ipsec-esp" = [] +"multicast" = [] + "socket" = [] "socket-raw" = ["socket"] "socket-udp" = ["socket"] @@ -96,10 +97,10 @@ default = [ "std", "log", # needed for `cargo test --no-default-features --features default` :/ "medium-ethernet", "medium-ip", "medium-ieee802154", "phy-raw_socket", "phy-tuntap_interface", - "proto-ipv4", "proto-igmp", "proto-dhcpv4", "proto-ipv6", "proto-dns", + "proto-ipv4", "proto-dhcpv4", "proto-ipv6", "proto-dns", "proto-ipv4-fragmentation", "proto-sixlowpan-fragmentation", "socket-raw", "socket-icmp", "socket-udp", "socket-tcp", "socket-dhcpv4", "socket-dns", "socket-mdns", - "packetmeta-id", "async" + "packetmeta-id", "async", "multicast" ] # Private features @@ -301,7 +302,7 @@ required-features = ["std", "log", "medium-ethernet", "proto-ipv4", "socket-tcp" [[example]] name = "multicast" -required-features = ["std", "medium-ethernet", "medium-ip", "phy-tuntap_interface", "proto-ipv4", "proto-igmp", "socket-udp"] +required-features = ["std", "medium-ethernet", "medium-ip", "phy-tuntap_interface", "proto-ipv4", "multicast", "socket-udp"] [[example]] name = "multicast6" diff --git a/ci.sh b/ci.sh index 9a9cc1f65..d8a6cf680 100755 --- a/ci.sh +++ b/ci.sh @@ -18,10 +18,10 @@ FEATURES_TEST=( "std,medium-ethernet,phy-raw_socket,proto-ipv6,socket-udp,socket-dns" "std,medium-ethernet,phy-tuntap_interface,proto-ipv6,socket-udp" "std,medium-ethernet,proto-ipv4,proto-ipv4-fragmentation,socket-raw,socket-dns" - "std,medium-ethernet,proto-ipv4,proto-igmp,socket-raw,socket-dns" + "std,medium-ethernet,proto-ipv4,multicast,socket-raw,socket-dns" "std,medium-ethernet,proto-ipv4,socket-udp,socket-tcp,socket-dns" "std,medium-ethernet,proto-ipv4,proto-dhcpv4,socket-udp" - "std,medium-ethernet,medium-ip,medium-ieee802154,proto-ipv6,proto-igmp,proto-rpl,socket-udp,socket-dns" + "std,medium-ethernet,medium-ip,medium-ieee802154,proto-ipv6,multicast,proto-rpl,socket-udp,socket-dns" "std,medium-ethernet,proto-ipv6,socket-tcp" "std,medium-ethernet,medium-ip,proto-ipv4,socket-icmp,socket-tcp" "std,medium-ip,proto-ipv6,socket-icmp,socket-tcp" @@ -29,7 +29,7 @@ FEATURES_TEST=( "std,medium-ieee802154,proto-sixlowpan,proto-sixlowpan-fragmentation,socket-udp" "std,medium-ieee802154,proto-rpl,proto-sixlowpan,proto-sixlowpan-fragmentation,socket-udp" "std,medium-ip,proto-ipv4,proto-ipv6,socket-tcp,socket-udp" - "std,medium-ethernet,medium-ip,medium-ieee802154,proto-ipv4,proto-ipv6,proto-igmp,proto-rpl,socket-raw,socket-udp,socket-tcp,socket-icmp,socket-dns,async" + "std,medium-ethernet,medium-ip,medium-ieee802154,proto-ipv4,proto-ipv6,multicast,proto-rpl,socket-raw,socket-udp,socket-tcp,socket-icmp,socket-dns,async" "std,medium-ieee802154,medium-ip,proto-ipv4,socket-raw" "std,medium-ethernet,proto-ipv4,proto-ipsec,socket-raw" ) @@ -39,9 +39,9 @@ FEATURES_TEST_NIGHTLY=( ) FEATURES_CHECK=( - "medium-ip,medium-ethernet,medium-ieee802154,proto-ipv6,proto-ipv6,proto-igmp,proto-dhcpv4,proto-ipsec,socket-raw,socket-udp,socket-tcp,socket-icmp,socket-dns,async" - "defmt,medium-ip,medium-ethernet,proto-ipv6,proto-ipv6,proto-igmp,proto-dhcpv4,socket-raw,socket-udp,socket-tcp,socket-icmp,socket-dns,async" - "defmt,alloc,medium-ip,medium-ethernet,proto-ipv6,proto-ipv6,proto-igmp,proto-dhcpv4,socket-raw,socket-udp,socket-tcp,socket-icmp,socket-dns,async" + "medium-ip,medium-ethernet,medium-ieee802154,proto-ipv6,proto-ipv6,multicast,proto-dhcpv4,proto-ipsec,socket-raw,socket-udp,socket-tcp,socket-icmp,socket-dns,async" + "defmt,medium-ip,medium-ethernet,proto-ipv6,proto-ipv6,multicast,proto-dhcpv4,socket-raw,socket-udp,socket-tcp,socket-icmp,socket-dns,async" + "defmt,alloc,medium-ip,medium-ethernet,proto-ipv6,proto-ipv6,multicast,proto-dhcpv4,socket-raw,socket-udp,socket-tcp,socket-icmp,socket-dns,async" ) test() { diff --git a/src/iface/interface/ipv4.rs b/src/iface/interface/ipv4.rs index e75159ea2..ef5766585 100644 --- a/src/iface/interface/ipv4.rs +++ b/src/iface/interface/ipv4.rs @@ -209,7 +209,7 @@ impl InterfaceInner { match ipv4_repr.next_header { IpProtocol::Icmp => self.process_icmpv4(sockets, ipv4_repr, ip_payload), - #[cfg(feature = "proto-igmp")] + #[cfg(feature = "multicast")] IpProtocol::Igmp => self.process_igmp(ipv4_repr, ip_payload), #[cfg(any(feature = "socket-udp", feature = "socket-dns"))] @@ -464,47 +464,4 @@ impl InterfaceInner { frag.ipv4.frag_offset += payload_len as u16; }) } - - #[cfg(feature = "proto-igmp")] - pub(super) fn igmp_report_packet<'any>( - &self, - version: IgmpVersion, - group_addr: Ipv4Address, - ) -> Option> { - let iface_addr = self.ipv4_addr()?; - let igmp_repr = IgmpRepr::MembershipReport { - group_addr, - version, - }; - let pkt = Packet::new_ipv4( - Ipv4Repr { - src_addr: iface_addr, - // Send to the group being reported - dst_addr: group_addr, - next_header: IpProtocol::Igmp, - payload_len: igmp_repr.buffer_len(), - hop_limit: 1, - // [#183](https://github.com/m-labs/smoltcp/issues/183). - }, - IpPayload::Igmp(igmp_repr), - ); - Some(pkt) - } - - #[cfg(feature = "proto-igmp")] - pub(super) fn igmp_leave_packet<'any>(&self, group_addr: Ipv4Address) -> Option> { - self.ipv4_addr().map(|iface_addr| { - let igmp_repr = IgmpRepr::LeaveGroup { group_addr }; - Packet::new_ipv4( - Ipv4Repr { - src_addr: iface_addr, - dst_addr: Ipv4Address::MULTICAST_ALL_ROUTERS, - next_header: IpProtocol::Igmp, - payload_len: igmp_repr.buffer_len(), - hop_limit: 1, - }, - IpPayload::Igmp(igmp_repr), - ) - }) - } } diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs index b2cbdc399..c850d3c8f 100644 --- a/src/iface/interface/mod.rs +++ b/src/iface/interface/mod.rs @@ -17,20 +17,17 @@ mod ipv6; #[cfg(feature = "proto-sixlowpan")] mod sixlowpan; -#[cfg(feature = "proto-igmp")] -mod igmp; +#[cfg(feature = "multicast")] +pub(crate) mod multicast; #[cfg(feature = "socket-tcp")] mod tcp; #[cfg(any(feature = "socket-udp", feature = "socket-dns"))] mod udp; -#[cfg(feature = "proto-igmp")] -pub use igmp::MulticastError; - use super::packet::*; use core::result::Result; -use heapless::{LinearMap, Vec}; +use heapless::Vec; #[cfg(feature = "_proto-fragmentation")] use super::fragmentation::FragKey; @@ -41,10 +38,7 @@ use super::fragmentation::{Fragmenter, FragmentsBuffer}; #[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))] use super::neighbor::{Answer as NeighborAnswer, Cache as NeighborCache}; use super::socket_set::SocketSet; -use crate::config::{ - IFACE_MAX_ADDR_COUNT, IFACE_MAX_MULTICAST_GROUP_COUNT, - IFACE_MAX_SIXLOWPAN_ADDRESS_CONTEXT_COUNT, -}; +use crate::config::{IFACE_MAX_ADDR_COUNT, IFACE_MAX_SIXLOWPAN_ADDRESS_CONTEXT_COUNT}; use crate::iface::Routes; use crate::phy::PacketMeta; use crate::phy::{ChecksumCapabilities, Device, DeviceCapabilities, Medium, RxToken, TxToken}; @@ -82,16 +76,6 @@ pub struct Interface { fragmenter: Fragmenter, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum MulticastGroupState { - /// Joining group, we have to send the join packet. - Joining, - /// We've already sent the join packet, we have nothing to do. - Joined, - /// We want to leave the group, we have to send a leave packet. - Leaving, -} - /// The device independent part of an Ethernet network interface. /// /// Separating the device from the data required for processing and dispatching makes @@ -121,11 +105,8 @@ pub struct InterfaceInner { ip_addrs: Vec, any_ip: bool, routes: Routes, - #[cfg(any(feature = "proto-igmp", feature = "proto-ipv6"))] - multicast_groups: LinearMap, - /// When to report for (all or) the next multicast group membership via IGMP - #[cfg(feature = "proto-igmp")] - igmp_report_state: IgmpReportState, + #[cfg(feature = "multicast")] + multicast: multicast::State, } /// Configuration structure used for creating a network interface. @@ -234,10 +215,8 @@ impl Interface { routes: Routes::new(), #[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))] neighbor_cache: NeighborCache::new(), - #[cfg(any(feature = "proto-igmp", feature = "proto-ipv6"))] - multicast_groups: LinearMap::new(), - #[cfg(feature = "proto-igmp")] - igmp_report_state: IgmpReportState::Inactive, + #[cfg(feature = "multicast")] + multicast: multicast::State::new(), #[cfg(feature = "medium-ieee802154")] sequence_no, #[cfg(feature = "medium-ieee802154")] @@ -445,10 +424,8 @@ impl Interface { let mut readiness_may_have_changed = self.socket_ingress(device, sockets); readiness_may_have_changed |= self.socket_egress(device, sockets); - #[cfg(feature = "proto-igmp")] - { - readiness_may_have_changed |= self.multicast_egress(device); - } + #[cfg(feature = "multicast")] + self.multicast_egress(device); readiness_may_have_changed } @@ -755,36 +732,23 @@ impl InterfaceInner { } /// Check whether the interface listens to given destination multicast IP address. - /// - /// If built without feature `proto-igmp` this function will - /// always return `false` when using IPv4. fn has_multicast_group>(&self, addr: T) -> bool { - /// Return false if we don't have the multicast group, - /// or we're leaving it. - fn wanted_state(x: Option<&MulticastGroupState>) -> bool { - match x { - None => false, - Some(MulticastGroupState::Joining) => true, - Some(MulticastGroupState::Joined) => true, - Some(MulticastGroupState::Leaving) => false, - } + let addr = addr.into(); + + #[cfg(feature = "multicast")] + if self.multicast.has_multicast_group(addr) { + return true; } - let addr = addr.into(); match addr { - #[cfg(feature = "proto-igmp")] - IpAddress::Ipv4(key) => { - key == Ipv4Address::MULTICAST_ALL_SYSTEMS - || wanted_state(self.multicast_groups.get(&addr)) - } + #[cfg(feature = "proto-ipv4")] + IpAddress::Ipv4(key) => key == Ipv4Address::MULTICAST_ALL_SYSTEMS, + #[cfg(feature = "proto-rpl")] + IpAddress::Ipv6(Ipv6Address::LINK_LOCAL_ALL_RPL_NODES) => true, #[cfg(feature = "proto-ipv6")] IpAddress::Ipv6(key) => { - key == Ipv6Address::LINK_LOCAL_ALL_NODES - || self.has_solicited_node(key) - || wanted_state(self.multicast_groups.get(&addr)) + key == Ipv6Address::LINK_LOCAL_ALL_NODES || self.has_solicited_node(key) } - #[cfg(feature = "proto-rpl")] - IpAddress::Ipv6(Ipv6Address::LINK_LOCAL_ALL_RPL_NODES) => true, #[allow(unreachable_patterns)] _ => false, } diff --git a/src/iface/interface/igmp.rs b/src/iface/interface/multicast.rs similarity index 66% rename from src/iface/interface/igmp.rs rename to src/iface/interface/multicast.rs index e5506a98e..d66ba4d17 100644 --- a/src/iface/interface/igmp.rs +++ b/src/iface/interface/multicast.rs @@ -1,4 +1,12 @@ -use super::*; +use core::result::Result; +use heapless::LinearMap; + +#[cfg(feature = "proto-ipv4")] +use super::{check, IpPayload, Packet}; +use super::{Interface, InterfaceInner}; +use crate::config::IFACE_MAX_MULTICAST_GROUP_COUNT; +use crate::phy::{Device, PacketMeta}; +use crate::wire::*; /// Error type for `join_multicast_group`, `leave_multicast_group`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -10,6 +18,60 @@ pub enum MulticastError { Unaddressable, } +#[cfg(feature = "proto-ipv4")] +pub(crate) enum IgmpReportState { + Inactive, + ToGeneralQuery { + version: IgmpVersion, + timeout: crate::time::Instant, + interval: crate::time::Duration, + next_index: usize, + }, + ToSpecificQuery { + version: IgmpVersion, + timeout: crate::time::Instant, + group: Ipv4Address, + }, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum GroupState { + /// Joining group, we have to send the join packet. + Joining, + /// We've already sent the join packet, we have nothing to do. + Joined, + /// We want to leave the group, we have to send a leave packet. + Leaving, +} + +pub(crate) struct State { + groups: LinearMap, + /// When to report for (all or) the next multicast group membership via IGMP + #[cfg(feature = "proto-ipv4")] + igmp_report_state: IgmpReportState, +} + +impl State { + pub(crate) fn new() -> Self { + Self { + groups: LinearMap::new(), + #[cfg(feature = "proto-ipv4")] + igmp_report_state: IgmpReportState::Inactive, + } + } + + pub(crate) fn has_multicast_group>(&self, addr: T) -> bool { + // Return false if we don't have the multicast group, + // or we're leaving it. + match self.groups.get(&addr.into()) { + None => false, + Some(GroupState::Joining) => true, + Some(GroupState::Joined) => true, + Some(GroupState::Leaving) => false, + } + } +} + impl core::fmt::Display for MulticastError { fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { match self { @@ -33,16 +95,17 @@ impl Interface { return Err(MulticastError::Unaddressable); } - if let Some(state) = self.inner.multicast_groups.get_mut(&addr) { + if let Some(state) = self.inner.multicast.groups.get_mut(&addr) { *state = match state { - MulticastGroupState::Joining => MulticastGroupState::Joining, - MulticastGroupState::Joined => MulticastGroupState::Joined, - MulticastGroupState::Leaving => MulticastGroupState::Joined, + GroupState::Joining => GroupState::Joining, + GroupState::Joined => GroupState::Joined, + GroupState::Leaving => GroupState::Joined, }; } else { self.inner - .multicast_groups - .insert(addr, MulticastGroupState::Joining) + .multicast + .groups + .insert(addr, GroupState::Joining) .map_err(|_| MulticastError::GroupTableFull)?; } Ok(()) @@ -58,15 +121,15 @@ impl Interface { return Err(MulticastError::Unaddressable); } - if let Some(state) = self.inner.multicast_groups.get_mut(&addr) { + if let Some(state) = self.inner.multicast.groups.get_mut(&addr) { let delete; (*state, delete) = match state { - MulticastGroupState::Joining => (MulticastGroupState::Joined, true), - MulticastGroupState::Joined => (MulticastGroupState::Leaving, false), - MulticastGroupState::Leaving => (MulticastGroupState::Leaving, false), + GroupState::Joining => (GroupState::Joined, true), + GroupState::Joined => (GroupState::Leaving, false), + GroupState::Leaving => (GroupState::Leaving, false), }; if delete { - self.inner.multicast_groups.remove(&addr); + self.inner.multicast.groups.remove(&addr); } } Ok(()) @@ -82,18 +145,20 @@ impl Interface { /// - Send join/leave packets according to the multicast group state. /// - Depending on `igmp_report_state` and the therein contained /// timeouts, send IGMP membership reports. - pub(crate) fn multicast_egress(&mut self, device: &mut D) -> bool + pub(crate) fn multicast_egress(&mut self, device: &mut D) where D: Device + ?Sized, { // Process multicast joins. while let Some((&addr, _)) = self .inner - .multicast_groups + .multicast + .groups .iter() - .find(|(_, &state)| state == MulticastGroupState::Joining) + .find(|(_, &state)| state == GroupState::Joining) { match addr { + #[cfg(feature = "proto-ipv4")] IpAddress::Ipv4(addr) => { if let Some(pkt) = self.inner.igmp_report_packet(IgmpVersion::Version2, addr) { let Some(tx_token) = device.transmit(self.inner.now) else { @@ -126,19 +191,22 @@ impl Interface { // NOTE(unwrap): this is always replacing an existing entry, so it can't fail due to the map being full. self.inner - .multicast_groups - .insert(addr, MulticastGroupState::Joined) + .multicast + .groups + .insert(addr, GroupState::Joined) .unwrap(); } // Process multicast leaves. while let Some((&addr, _)) = self .inner - .multicast_groups + .multicast + .groups .iter() - .find(|(_, &state)| state == MulticastGroupState::Leaving) + .find(|(_, &state)| state == GroupState::Leaving) { match addr { + #[cfg(feature = "proto-ipv4")] IpAddress::Ipv4(addr) => { if let Some(pkt) = self.inner.igmp_leave_packet(addr) { let Some(tx_token) = device.transmit(self.inner.now) else { @@ -169,10 +237,11 @@ impl Interface { } } - self.inner.multicast_groups.remove(&addr); + self.inner.multicast.groups.remove(&addr); } - match self.inner.igmp_report_state { + #[cfg(feature = "proto-ipv4")] + match self.inner.multicast.igmp_report_state { IgmpReportState::ToSpecificQuery { version, timeout, @@ -185,13 +254,9 @@ impl Interface { self.inner .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) .unwrap(); - } else { - return false; + self.inner.multicast.igmp_report_state = IgmpReportState::Inactive; } } - - self.inner.igmp_report_state = IgmpReportState::Inactive; - true } IgmpReportState::ToGeneralQuery { version, @@ -201,7 +266,8 @@ impl Interface { } if self.inner.now >= timeout => { let addr = self .inner - .multicast_groups + .multicast + .groups .iter() .filter_map(|(addr, _)| match addr { IpAddress::Ipv4(addr) => Some(*addr), @@ -224,28 +290,24 @@ impl Interface { &mut self.fragmenter, ) .unwrap(); - } else { - return false; + + let next_timeout = (timeout + interval).max(self.inner.now); + self.inner.multicast.igmp_report_state = + IgmpReportState::ToGeneralQuery { + version, + timeout: next_timeout, + interval, + next_index: next_index + 1, + }; } } - - let next_timeout = (timeout + interval).max(self.inner.now); - self.inner.igmp_report_state = IgmpReportState::ToGeneralQuery { - version, - timeout: next_timeout, - interval, - next_index: next_index + 1, - }; - true } - None => { - self.inner.igmp_report_state = IgmpReportState::Inactive; - false + self.inner.multicast.igmp_report_state = IgmpReportState::Inactive; } } } - _ => false, + _ => {} } } } @@ -256,11 +318,14 @@ impl InterfaceInner { /// Sets up `igmp_report_state` for responding to IGMP general/specific membership queries. /// Membership must not be reported immediately in order to avoid flooding the network /// after a query is broadcasted by a router; this is not currently done. + #[cfg(feature = "proto-ipv4")] pub(super) fn process_igmp<'frame>( &mut self, ipv4_repr: Ipv4Repr, ip_payload: &'frame [u8], ) -> Option> { + use crate::time::Duration; + let igmp_packet = check!(IgmpPacket::new_checked(ip_payload)); let igmp_repr = check!(IgmpRepr::parse(&igmp_packet)); @@ -276,7 +341,8 @@ impl InterfaceInner { && ipv4_repr.dst_addr == Ipv4Address::MULTICAST_ALL_SYSTEMS { let ipv4_multicast_group_count = self - .multicast_groups + .multicast + .groups .keys() .filter(|a| matches!(a, IpAddress::Ipv4(_))) .count(); @@ -293,7 +359,7 @@ impl InterfaceInner { max_resp_time / intervals } }; - self.igmp_report_state = IgmpReportState::ToGeneralQuery { + self.multicast.igmp_report_state = IgmpReportState::ToGeneralQuery { version, timeout: self.now + interval, interval, @@ -305,7 +371,7 @@ impl InterfaceInner { if self.has_multicast_group(group_addr) && ipv4_repr.dst_addr == group_addr { // Don't respond immediately let timeout = max_resp_time / 4; - self.igmp_report_state = IgmpReportState::ToSpecificQuery { + self.multicast.igmp_report_state = IgmpReportState::ToSpecificQuery { version, timeout: self.now + timeout, group: group_addr, @@ -321,4 +387,47 @@ impl InterfaceInner { None } + + #[cfg(feature = "proto-ipv4")] + fn igmp_report_packet<'any>( + &self, + version: IgmpVersion, + group_addr: Ipv4Address, + ) -> Option> { + let iface_addr = self.ipv4_addr()?; + let igmp_repr = IgmpRepr::MembershipReport { + group_addr, + version, + }; + let pkt = Packet::new_ipv4( + Ipv4Repr { + src_addr: iface_addr, + // Send to the group being reported + dst_addr: group_addr, + next_header: IpProtocol::Igmp, + payload_len: igmp_repr.buffer_len(), + hop_limit: 1, + // [#183](https://github.com/m-labs/smoltcp/issues/183). + }, + IpPayload::Igmp(igmp_repr), + ); + Some(pkt) + } + + #[cfg(feature = "proto-ipv4")] + fn igmp_leave_packet<'any>(&self, group_addr: Ipv4Address) -> Option> { + self.ipv4_addr().map(|iface_addr| { + let igmp_repr = IgmpRepr::LeaveGroup { group_addr }; + Packet::new_ipv4( + Ipv4Repr { + src_addr: iface_addr, + dst_addr: Ipv4Address::MULTICAST_ALL_ROUTERS, + next_header: IpProtocol::Igmp, + payload_len: igmp_repr.buffer_len(), + hop_limit: 1, + }, + IpPayload::Igmp(igmp_repr), + ) + }) + } } diff --git a/src/iface/interface/tests/ipv4.rs b/src/iface/interface/tests/ipv4.rs index c9e100fc8..85fb82c5e 100644 --- a/src/iface/interface/tests/ipv4.rs +++ b/src/iface/interface/tests/ipv4.rs @@ -659,9 +659,9 @@ fn test_icmpv4_socket(#[case] medium: Medium) { #[rstest] #[case(Medium::Ip)] -#[cfg(all(feature = "proto-igmp", feature = "medium-ip"))] +#[cfg(all(feature = "multicast", feature = "medium-ip"))] #[case(Medium::Ethernet)] -#[cfg(all(feature = "proto-igmp", feature = "medium-ethernet"))] +#[cfg(all(feature = "multicast", feature = "medium-ethernet"))] fn test_handle_igmp(#[case] medium: Medium) { fn recv_igmp( device: &mut crate::tests::TestingDevice, diff --git a/src/iface/interface/tests/mod.rs b/src/iface/interface/tests/mod.rs index 3cb5e103c..db8cc89dd 100644 --- a/src/iface/interface/tests/mod.rs +++ b/src/iface/interface/tests/mod.rs @@ -5,7 +5,7 @@ mod ipv6; #[cfg(feature = "proto-sixlowpan")] mod sixlowpan; -#[cfg(feature = "proto-igmp")] +#[allow(unused)] use std::vec::Vec; use crate::tests::setup; @@ -27,7 +27,7 @@ fn fill_slice(s: &mut [u8], val: u8) { } } -#[cfg(feature = "proto-igmp")] +#[allow(unused)] fn recv_all(device: &mut crate::tests::TestingDevice, timestamp: Instant) -> Vec> { let mut pkts = Vec::new(); while let Some((rx, _tx)) = device.receive(timestamp) { diff --git a/src/iface/mod.rs b/src/iface/mod.rs index 3076088a0..8abcbd6e6 100644 --- a/src/iface/mod.rs +++ b/src/iface/mod.rs @@ -16,8 +16,8 @@ mod socket_set; mod packet; -#[cfg(feature = "proto-igmp")] -pub use self::interface::MulticastError; +#[cfg(feature = "multicast")] +pub use self::interface::multicast::MulticastError; pub use self::interface::{Config, Interface, InterfaceInner as Context}; pub use self::route::{Route, RouteTableFull, Routes}; diff --git a/src/iface/packet.rs b/src/iface/packet.rs index 13f24256e..1ccc6766b 100644 --- a/src/iface/packet.rs +++ b/src/iface/packet.rs @@ -81,7 +81,7 @@ impl<'p> Packet<'p> { IpPayload::Icmpv4(icmpv4_repr) => { icmpv4_repr.emit(&mut Icmpv4Packet::new_unchecked(payload), &caps.checksum) } - #[cfg(feature = "proto-igmp")] + #[cfg(all(feature = "proto-ipv4", feature = "multicast"))] IpPayload::Igmp(igmp_repr) => igmp_repr.emit(&mut IgmpPacket::new_unchecked(payload)), #[cfg(feature = "proto-ipv6")] IpPayload::Icmpv6(icmpv6_repr) => { @@ -207,7 +207,7 @@ pub(crate) struct PacketV6<'p> { pub(crate) enum IpPayload<'p> { #[cfg(feature = "proto-ipv4")] Icmpv4(Icmpv4Repr<'p>), - #[cfg(feature = "proto-igmp")] + #[cfg(all(feature = "proto-ipv4", feature = "multicast"))] Igmp(IgmpRepr), #[cfg(feature = "proto-ipv6")] Icmpv6(Icmpv6Repr<'p>), @@ -235,7 +235,7 @@ impl<'p> IpPayload<'p> { Self::Icmpv6(_) => SixlowpanNextHeader::Uncompressed(IpProtocol::Icmpv6), #[cfg(feature = "proto-ipv6")] Self::HopByHopIcmpv6(_, _) => unreachable!(), - #[cfg(feature = "proto-igmp")] + #[cfg(all(feature = "proto-ipv4", feature = "multicast"))] Self::Igmp(_) => unreachable!(), #[cfg(feature = "socket-tcp")] Self::Tcp(_) => SixlowpanNextHeader::Uncompressed(IpProtocol::Tcp), @@ -259,19 +259,3 @@ pub(crate) fn icmp_reply_payload_len(len: usize, mtu: usize, header_len: usize) // - IP Header Size * 2 - ICMPv4 DstUnreachable hdr size len.min(mtu - header_len * 2 - 8) } - -#[cfg(feature = "proto-igmp")] -pub(crate) enum IgmpReportState { - Inactive, - ToGeneralQuery { - version: IgmpVersion, - timeout: crate::time::Instant, - interval: crate::time::Duration, - next_index: usize, - }, - ToSpecificQuery { - version: IgmpVersion, - timeout: crate::time::Instant, - group: Ipv4Address, - }, -} diff --git a/src/wire/mod.rs b/src/wire/mod.rs index c197afed9..64ac65323 100644 --- a/src/wire/mod.rs +++ b/src/wire/mod.rs @@ -93,7 +93,7 @@ mod icmpv4; mod icmpv6; #[cfg(feature = "medium-ieee802154")] pub mod ieee802154; -#[cfg(feature = "proto-igmp")] +#[cfg(feature = "proto-ipv4")] mod igmp; pub(crate) mod ip; #[cfg(feature = "proto-ipv4")] @@ -225,7 +225,7 @@ pub use self::icmpv4::{ TimeExceeded as Icmpv4TimeExceeded, }; -#[cfg(feature = "proto-igmp")] +#[cfg(feature = "proto-ipv4")] pub use self::igmp::{IgmpVersion, Packet as IgmpPacket, Repr as IgmpRepr}; #[cfg(feature = "proto-ipv6")] From 00aa2b457cb704fb4ba976af15946e1a80053f50 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 12 Sep 2024 17:10:03 +0200 Subject: [PATCH 18/25] Fix bench, check it in CI. --- benches/bench.rs | 1 + ci.sh | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/benches/bench.rs b/benches/bench.rs index 2738840c1..0920958d9 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -42,6 +42,7 @@ mod wire { sack_permitted: false, sack_ranges: [None, None, None], payload: &PAYLOAD_BYTES, + timestamp: None, }; let mut bytes = vec![0xa5; repr.buffer_len()]; diff --git a/ci.sh b/ci.sh index 9a9cc1f65..2b078af08 100755 --- a/ci.sh +++ b/ci.sh @@ -68,6 +68,12 @@ check() { for features in ${FEATURES_CHECK[@]}; do cargo +$version check --no-default-features --features "$features" done + + cargo +$version check --examples + + if [[ $version == "nightly" ]]; then + cargo +$version check --benches + fi } clippy() { From 3acd5111798fbd581c03f005a6d2383062b7106d Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 16 Sep 2024 19:34:41 +0200 Subject: [PATCH 19/25] tests: use two queues for TestingDevice. Currently TestingDevice is a loopback device, which causes problems if the iface transmits a packet then receives. The packet gets looped back to the interface instead of being handled by the test code. This commit changes TestingDevice to behave more like a "virtual wire" that communicates the interface with the testing code, with one queue for each direction. --- src/iface/interface/tests/ipv4.rs | 10 ++------- src/iface/interface/tests/mod.rs | 6 ++--- src/iface/interface/tests/sixlowpan.rs | 8 +++---- src/tests.rs | 31 +++++++++++++------------- 4 files changed, 23 insertions(+), 32 deletions(-) diff --git a/src/iface/interface/tests/ipv4.rs b/src/iface/interface/tests/ipv4.rs index 85fb82c5e..41e35301c 100644 --- a/src/iface/interface/tests/ipv4.rs +++ b/src/iface/interface/tests/ipv4.rs @@ -721,20 +721,14 @@ fn test_handle_igmp(#[case] medium: Medium) { } // General query - let timestamp = Instant::ZERO; const GENERAL_QUERY_BYTES: &[u8] = &[ 0x46, 0xc0, 0x00, 0x24, 0xed, 0xb4, 0x00, 0x00, 0x01, 0x02, 0x47, 0x43, 0xac, 0x16, 0x63, 0x04, 0xe0, 0x00, 0x00, 0x01, 0x94, 0x04, 0x00, 0x00, 0x11, 0x64, 0xec, 0x8f, 0x00, 0x00, 0x00, 0x00, 0x02, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, ]; - { - // Transmit GENERAL_QUERY_BYTES into loopback - let tx_token = device.transmit(timestamp).unwrap(); - tx_token.consume(GENERAL_QUERY_BYTES.len(), |buffer| { - buffer.copy_from_slice(GENERAL_QUERY_BYTES); - }); - } + device.rx_queue.push_back(GENERAL_QUERY_BYTES.to_vec()); + // Trigger processing until all packets received through the // loopback have been processed, including responses to // GENERAL_QUERY_BYTES. Therefore `recv_all()` would return 0 diff --git a/src/iface/interface/tests/mod.rs b/src/iface/interface/tests/mod.rs index db8cc89dd..fe98ddd3d 100644 --- a/src/iface/interface/tests/mod.rs +++ b/src/iface/interface/tests/mod.rs @@ -30,10 +30,8 @@ fn fill_slice(s: &mut [u8], val: u8) { #[allow(unused)] fn recv_all(device: &mut crate::tests::TestingDevice, timestamp: Instant) -> Vec> { let mut pkts = Vec::new(); - while let Some((rx, _tx)) = device.receive(timestamp) { - rx.consume(|pkt| { - pkts.push(pkt.to_vec()); - }); + while let Some(pkt) = device.tx_queue.pop_front() { + pkts.push(pkt) } pkts } diff --git a/src/iface/interface/tests/sixlowpan.rs b/src/iface/interface/tests/sixlowpan.rs index b80024a8b..14a4c46ae 100644 --- a/src/iface/interface/tests/sixlowpan.rs +++ b/src/iface/interface/tests/sixlowpan.rs @@ -244,7 +244,7 @@ fn test_echo_request_sixlowpan_128_bytes() { ); assert_eq!( - device.queue.pop_front().unwrap(), + device.tx_queue.pop_front().unwrap(), &[ 0x41, 0xcc, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0xc0, 0xb0, 0x5, 0x4e, 0x7a, 0x11, 0x3a, 0x92, 0xfc, 0x48, 0xc2, @@ -261,7 +261,7 @@ fn test_echo_request_sixlowpan_128_bytes() { iface.poll(Instant::now(), &mut device, &mut sockets); assert_eq!( - device.queue.pop_front().unwrap(), + device.tx_queue.pop_front().unwrap(), &[ 0x41, 0xcc, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0xe0, 0xb0, 0x5, 0x4e, 0xf, 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, @@ -415,7 +415,7 @@ In at rhoncus tortor. Cras blandit tellus diam, varius vestibulum nibh commodo n iface.poll(Instant::now(), &mut device, &mut sockets); assert_eq!( - device.queue.pop_front().unwrap(), + device.tx_queue.pop_front().unwrap(), &[ 0x41, 0xcc, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0xc0, 0xb4, 0x5, 0x4e, 0x7e, 0x40, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @@ -430,7 +430,7 @@ In at rhoncus tortor. Cras blandit tellus diam, varius vestibulum nibh commodo n ); assert_eq!( - device.queue.pop_front().unwrap(), + device.tx_queue.pop_front().unwrap(), &[ 0x41, 0xcc, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0x2, 0xe0, 0xb4, 0x5, 0x4e, 0xf, 0x6f, 0x72, 0x74, 0x6f, 0x72, 0x2e, diff --git a/src/tests.rs b/src/tests.rs index b9eb740be..165bd52f7 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,4 +1,8 @@ +use std::collections::VecDeque; + use crate::iface::*; +use crate::phy::{self, Device, DeviceCapabilities, Medium}; +use crate::time::Instant; use crate::wire::*; pub(crate) fn setup<'a>(medium: Medium) -> (Interface, SocketSet<'a>, TestingDevice) { @@ -49,16 +53,11 @@ pub(crate) fn setup<'a>(medium: Medium) -> (Interface, SocketSet<'a>, TestingDev (iface, SocketSet::new(vec![]), device) } -use heapless::Deque; -use heapless::Vec; - -use crate::phy::{self, Device, DeviceCapabilities, Medium}; -use crate::time::Instant; - /// A testing device. #[derive(Debug)] pub struct TestingDevice { - pub(crate) queue: Deque, 4>, + pub(crate) tx_queue: VecDeque>, + pub(crate) rx_queue: VecDeque>, max_transmission_unit: usize, medium: Medium, } @@ -71,7 +70,8 @@ impl TestingDevice { /// in FIFO order. pub fn new(medium: Medium) -> Self { TestingDevice { - queue: Deque::new(), + tx_queue: VecDeque::new(), + rx_queue: VecDeque::new(), max_transmission_unit: match medium { #[cfg(feature = "medium-ethernet")] Medium::Ethernet => 1514, @@ -98,10 +98,10 @@ impl Device for TestingDevice { } fn receive(&mut self, _timestamp: Instant) -> Option<(Self::RxToken<'_>, Self::TxToken<'_>)> { - self.queue.pop_front().map(move |buffer| { + self.rx_queue.pop_front().map(move |buffer| { let rx = RxToken { buffer }; let tx = TxToken { - queue: &mut self.queue, + queue: &mut self.tx_queue, }; (rx, tx) }) @@ -109,14 +109,14 @@ impl Device for TestingDevice { fn transmit(&mut self, _timestamp: Instant) -> Option> { Some(TxToken { - queue: &mut self.queue, + queue: &mut self.tx_queue, }) } } #[doc(hidden)] pub struct RxToken { - buffer: Vec, + buffer: Vec, } impl phy::RxToken for RxToken { @@ -131,7 +131,7 @@ impl phy::RxToken for RxToken { #[doc(hidden)] #[derive(Debug)] pub struct TxToken<'a> { - queue: &'a mut Deque, 4>, + queue: &'a mut VecDeque>, } impl<'a> phy::TxToken for TxToken<'a> { @@ -139,10 +139,9 @@ impl<'a> phy::TxToken for TxToken<'a> { where F: FnOnce(&mut [u8]) -> R, { - let mut buffer = Vec::new(); - buffer.resize(len, 0).unwrap(); + let mut buffer = vec![0; len]; let result = f(&mut buffer); - self.queue.push_back(buffer).unwrap(); + self.queue.push_back(buffer); result } } From dd43c8f189178b0ab3bda798ed8578b5b0a6f094 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 16 Sep 2024 19:43:34 +0200 Subject: [PATCH 20/25] iface: make poll() process all packets, add fine-grained poll functions. This makes `.poll()` behave the same as before #954. Users affected by DoS concerns can use the finer-grained egress-only and single-packet-ingress-only fns. --- src/iface/interface/ipv4.rs | 9 +- src/iface/interface/mod.rs | 192 ++++++++++++++++++++++--------- src/iface/interface/multicast.rs | 5 +- src/iface/interface/sixlowpan.rs | 13 +-- 4 files changed, 145 insertions(+), 74 deletions(-) diff --git a/src/iface/interface/ipv4.rs b/src/iface/interface/ipv4.rs index ef5766585..2e93a336e 100644 --- a/src/iface/interface/ipv4.rs +++ b/src/iface/interface/ipv4.rs @@ -7,17 +7,14 @@ impl Interface { /// processed or emitted, and thus, whether the readiness of any socket might /// have changed. #[cfg(feature = "proto-ipv4-fragmentation")] - pub(super) fn ipv4_egress(&mut self, device: &mut D) -> bool - where - D: Device + ?Sized, - { + pub(super) fn ipv4_egress(&mut self, device: &mut (impl Device + ?Sized)) { // Reset the buffer when we transmitted everything. if self.fragmenter.finished() { self.fragmenter.reset(); } if self.fragmenter.is_empty() { - return false; + return; } let pkt = &self.fragmenter; @@ -25,10 +22,8 @@ impl Interface { if let Some(tx_token) = device.transmit(self.inner.now) { self.inner .dispatch_ipv4_frag(tx_token, &mut self.fragmenter); - return true; } } - false } } diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs index c850d3c8f..0a03cb9b5 100644 --- a/src/iface/interface/mod.rs +++ b/src/iface/interface/mod.rs @@ -65,6 +65,44 @@ macro_rules! check { } use check; +/// Result returned by [`Interface::poll`]. +/// +/// This contains information on whether socket states might have changed. +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum PollResult { + /// Socket state is guaranteed to not have changed. + None, + /// You should check the state of sockets again for received data or completion of operations. + SocketStateChanged, +} + +/// Result returned by [`Interface::poll_ingress_single`]. +/// +/// This contains information on whether a packet was processed or not, +/// and whether it might've affected socket states. +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum PollIngressSingleResult { + /// No packet was processed. You don't need to call [`Interface::poll_ingress_single`] + /// again, until more packets arrive. + /// + /// Socket state is guaranteed to not have changed. + None, + /// A packet was processed. + /// + /// There may be more packets in the device's RX queue, so you should call [`Interface::poll_ingress_single`] again. + /// + /// Socket state is guaranteed to not have changed. + PacketProcessed, + /// A packet was processed, which might have caused socket state to change. + /// + /// There may be more packets in the device's RX queue, so you should call [`Interface::poll_ingress_single`] again. + /// + /// You should check the state of sockets again for received data or completion of operations. + SocketStateChanged, +} + /// A network interface. /// /// The network interface logically owns a number of other data structures; to avoid @@ -150,10 +188,7 @@ impl Interface { /// # Panics /// This function panics if the [`Config::hardware_address`] does not match /// the medium of the device. - pub fn new(config: Config, device: &mut D, now: Instant) -> Self - where - D: Device + ?Sized, - { + pub fn new(config: Config, device: &mut (impl Device + ?Sized), now: Instant) -> Self { let caps = device.capabilities(); assert_eq!( config.hardware_addr.medium(), @@ -375,59 +410,107 @@ impl Interface { self.fragments.reassembly_timeout = timeout; } - /// Transmit packets queued in the given sockets, and receive packets queued + /// Transmit packets queued in the sockets, and receive packets queued /// in the device. /// - /// This function returns a boolean value indicating whether any packets were - /// processed or emitted, and thus, whether the readiness of any socket might - /// have changed. + /// This function returns a value indicating whether the state of any socket + /// might have changed. + /// + /// ## DoS warning /// - /// # Note - /// This function performs a bounded amount of work per call to avoid - /// starving other tasks of CPU time. If it returns true, there may still be - /// packets to be received or transmitted. Depending on system design, - /// calling this function in a loop may cause a denial of service if - /// packets cannot be processed faster than they arrive. - pub fn poll( + /// This function processes all packets in the device's queue. This can + /// be an unbounded amount of work if packets arrive faster than they're + /// processed. + /// + /// If this is a concern for your application (i.e. your environment doesn't + /// have preemptive scheduling, or `poll()` is called from a main loop where + /// other important things are processed), you may use the lower-level methods + /// [`poll_egress()`](Self::poll_egress) and [`poll_ingress_single()`](Self::poll_ingress_single). + /// This allows you to insert yields or process other events between processing + /// individual ingress packets. + pub fn poll( &mut self, timestamp: Instant, - device: &mut D, + device: &mut (impl Device + ?Sized), sockets: &mut SocketSet<'_>, - ) -> bool - where - D: Device + ?Sized, - { + ) -> PollResult { self.inner.now = timestamp; + let mut res = PollResult::None; + #[cfg(feature = "_proto-fragmentation")] self.fragments.assembler.remove_expired(timestamp); + // Process ingress while there's packets available. + loop { + match self.socket_ingress(device, sockets) { + PollIngressSingleResult::None => break, + PollIngressSingleResult::PacketProcessed => {} + PollIngressSingleResult::SocketStateChanged => res = PollResult::SocketStateChanged, + } + } + + // Process egress. + match self.poll_egress(timestamp, device, sockets) { + PollResult::None => {} + PollResult::SocketStateChanged => res = PollResult::SocketStateChanged, + } + + res + } + + /// Transmit packets queued in the sockets. + /// + /// This function returns a value indicating whether the state of any socket + /// might have changed. + /// + /// This is guaranteed to always perform a bounded amount of work. + pub fn poll_egress( + &mut self, + timestamp: Instant, + device: &mut (impl Device + ?Sized), + sockets: &mut SocketSet<'_>, + ) -> PollResult { + self.inner.now = timestamp; + match self.inner.caps.medium { #[cfg(feature = "medium-ieee802154")] - Medium::Ieee802154 => - { + Medium::Ieee802154 => { #[cfg(feature = "proto-sixlowpan-fragmentation")] - if self.sixlowpan_egress(device) { - return true; - } + self.sixlowpan_egress(device); } #[cfg(any(feature = "medium-ethernet", feature = "medium-ip"))] - _ => - { + _ => { #[cfg(feature = "proto-ipv4-fragmentation")] - if self.ipv4_egress(device) { - return true; - } + self.ipv4_egress(device); } } - let mut readiness_may_have_changed = self.socket_ingress(device, sockets); - readiness_may_have_changed |= self.socket_egress(device, sockets); - #[cfg(feature = "multicast")] self.multicast_egress(device); - readiness_may_have_changed + self.socket_egress(device, sockets) + } + + /// Process one incoming packet queued in the device. + /// + /// Returns a value indicating: + /// - whether a packet was processed, in which case you have to call this method again in case there's more packets queued. + /// - whether the state of any socket might have changed. + /// + /// Since it processes at most one packet, this is guaranteed to always perform a bounded amount of work. + pub fn poll_ingress_single( + &mut self, + timestamp: Instant, + device: &mut (impl Device + ?Sized), + sockets: &mut SocketSet<'_>, + ) -> PollIngressSingleResult { + self.inner.now = timestamp; + + #[cfg(feature = "_proto-fragmentation")] + self.fragments.assembler.remove_expired(timestamp); + + self.socket_ingress(device, sockets) } /// Return a _soft deadline_ for calling [poll] the next time. @@ -480,20 +563,19 @@ impl Interface { } } - fn socket_ingress(&mut self, device: &mut D, sockets: &mut SocketSet<'_>) -> bool - where - D: Device + ?Sized, - { - let mut processed_any = false; - + fn socket_ingress( + &mut self, + device: &mut (impl Device + ?Sized), + sockets: &mut SocketSet<'_>, + ) -> PollIngressSingleResult { let Some((rx_token, tx_token)) = device.receive(self.inner.now) else { - return processed_any; + return PollIngressSingleResult::None; }; let rx_meta = rx_token.meta(); rx_token.consume(|frame| { if frame.is_empty() { - return; + return PollIngressSingleResult::PacketProcessed; } match self.inner.caps.medium { @@ -543,16 +625,22 @@ impl Interface { } } } - processed_any = true; - }); - processed_any + // TODO: Propagate the PollIngressSingleResult from deeper. + // There's many received packets that we process but can't cause sockets + // to change state. For example IP fragments, multicast stuff, ICMP pings + // if they dont't match any raw socket... + // We should return `PacketProcessed` for these to save the user from + // doing useless socket polls. + PollIngressSingleResult::SocketStateChanged + }) } - fn socket_egress(&mut self, device: &mut D, sockets: &mut SocketSet<'_>) -> bool - where - D: Device + ?Sized, - { + fn socket_egress( + &mut self, + device: &mut (impl Device + ?Sized), + sockets: &mut SocketSet<'_>, + ) -> PollResult { let _caps = device.capabilities(); enum EgressError { @@ -560,7 +648,7 @@ impl Interface { Dispatch, } - let mut emitted_any = false; + let mut result = PollResult::None; for item in sockets.items_mut() { if !item .meta @@ -581,7 +669,7 @@ impl Interface { .dispatch_ip(t, meta, response, &mut self.fragmenter) .map_err(|_| EgressError::Dispatch)?; - emitted_any = true; + result = PollResult::SocketStateChanged; Ok(()) }; @@ -663,7 +751,7 @@ impl Interface { Ok(()) => {} } } - emitted_any + result } } diff --git a/src/iface/interface/multicast.rs b/src/iface/interface/multicast.rs index d66ba4d17..7542a12f4 100644 --- a/src/iface/interface/multicast.rs +++ b/src/iface/interface/multicast.rs @@ -145,10 +145,7 @@ impl Interface { /// - Send join/leave packets according to the multicast group state. /// - Depending on `igmp_report_state` and the therein contained /// timeouts, send IGMP membership reports. - pub(crate) fn multicast_egress(&mut self, device: &mut D) - where - D: Device + ?Sized, - { + pub(crate) fn multicast_egress(&mut self, device: &mut (impl Device + ?Sized)) { // Process multicast joins. while let Some((&addr, _)) = self .inner diff --git a/src/iface/interface/sixlowpan.rs b/src/iface/interface/sixlowpan.rs index a89934f1f..5cdfa3315 100644 --- a/src/iface/interface/sixlowpan.rs +++ b/src/iface/interface/sixlowpan.rs @@ -7,22 +7,15 @@ pub(crate) const MAX_DECOMPRESSED_LEN: usize = 1500; impl Interface { /// Process fragments that still need to be sent for 6LoWPAN packets. - /// - /// This function returns a boolean value indicating whether any packets were - /// processed or emitted, and thus, whether the readiness of any socket might - /// have changed. #[cfg(feature = "proto-sixlowpan-fragmentation")] - pub(super) fn sixlowpan_egress(&mut self, device: &mut D) -> bool - where - D: Device + ?Sized, - { + pub(super) fn sixlowpan_egress(&mut self, device: &mut (impl Device + ?Sized)) { // Reset the buffer when we transmitted everything. if self.fragmenter.finished() { self.fragmenter.reset(); } if self.fragmenter.is_empty() { - return false; + return; } let pkt = &self.fragmenter; @@ -30,10 +23,8 @@ impl Interface { if let Some(tx_token) = device.transmit(self.inner.now) { self.inner .dispatch_ieee802154_frag(tx_token, &mut self.fragmenter); - return true; } } - false } /// Get the 6LoWPAN address contexts. From b94dd2dd8804501f66687e93f748e295e6633e4e Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Wed, 18 Sep 2024 09:02:13 +0200 Subject: [PATCH 21/25] Update actions/checkout to v4 --- .github/workflows/coverage.yml | 2 +- .github/workflows/fuzz.yml | 2 +- .github/workflows/rustfmt.yaml | 2 +- .github/workflows/test.yml | 25 +++++-------------------- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 1d03357b7..6c2ee2c6d 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -8,7 +8,7 @@ jobs: coverage: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Install `cargo llvm-cov` uses: taiki-e/install-action@cargo-llvm-cov - name: Run Coverage diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index e7a787afa..68d1ba0c7 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -10,7 +10,7 @@ jobs: env: RUSTUP_TOOLCHAIN: nightly steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Install cargo-fuzz run: cargo install cargo-fuzz - name: Fuzz diff --git a/.github/workflows/rustfmt.yaml b/.github/workflows/rustfmt.yaml index 107164af5..b7155894c 100644 --- a/.github/workflows/rustfmt.yaml +++ b/.github/workflows/rustfmt.yaml @@ -7,6 +7,6 @@ jobs: fmt: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Check fmt run: cargo fmt -- --check diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ee6774c98..4a13ca959 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,28 +15,28 @@ jobs: check-msrv: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Run Checks MSRV run: ./ci.sh check msrv test-msrv: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Run Tests MSRV run: ./ci.sh test msrv clippy: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Run Clippy run: ./ci.sh clippy test-stable: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Run Tests stable run: ./ci.sh test stable @@ -44,21 +44,6 @@ jobs: runs-on: ubuntu-22.04 continue-on-error: true steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Run Tests nightly run: ./ci.sh test nightly - - #check-stable: - #runs-on: ubuntu-22.04 - #steps: - #- uses: actions/checkout@v2 - #- name: Run Tests - #run: ./ci.sh check stable - - #check-nightly: - #runs-on: ubuntu-22.04 - #continue-on-error: true - #steps: - #- uses: actions/checkout@v2 - #- name: Run Tests - #run: ./ci.sh check nightly From 336e7e5935bdf7e85ff8f2d4c43315cdd49344de Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Thu, 19 Sep 2024 13:13:19 +0800 Subject: [PATCH 22/25] Export `PollResult` --- src/iface/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/iface/mod.rs b/src/iface/mod.rs index 8abcbd6e6..5b028d055 100644 --- a/src/iface/mod.rs +++ b/src/iface/mod.rs @@ -18,7 +18,9 @@ mod packet; #[cfg(feature = "multicast")] pub use self::interface::multicast::MulticastError; -pub use self::interface::{Config, Interface, InterfaceInner as Context}; +pub use self::interface::{ + Config, Interface, InterfaceInner as Context, PollIngressSingleResult, PollResult, +}; pub use self::route::{Route, RouteTableFull, Routes}; pub use self::socket_set::{SocketHandle, SocketSet, SocketStorage}; From 805eed84473ababca496054d7ebb9ba65e931992 Mon Sep 17 00:00:00 2001 From: XOR-op <17672363+XOR-op@users.noreply.github.com> Date: Sat, 21 Sep 2024 23:31:27 -0400 Subject: [PATCH 23/25] fix: internal sACK flag not updated for client socket --- src/socket/tcp.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index f944875ad..988bce905 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -1806,6 +1806,7 @@ impl<'a> Socket<'a> { self.remote_seq_no = repr.seq_number + 1; self.remote_last_seq = self.local_seq_no + 1; self.remote_last_ack = Some(repr.seq_number); + self.remote_has_sack = repr.sack_permitted; self.remote_win_scale = repr.window_scale; // Remote doesn't support window scaling, don't do it. if self.remote_win_scale.is_none() { From ffcb62e11923a7b51460ff6a0b3ced6c4d7fe14e Mon Sep 17 00:00:00 2001 From: XOR-op <17672363+XOR-op@users.noreply.github.com> Date: Sun, 22 Sep 2024 17:50:38 -0400 Subject: [PATCH 24/25] test(tcp): add coverage for remote_has_sack in SYN-SENT state --- src/socket/tcp.rs | 57 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 988bce905..46f1b36f9 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -3725,6 +3725,63 @@ mod test { assert_eq!(s.state, State::Closed); } + #[test] + fn test_syn_sent_sack_option() { + let mut s = socket_syn_sent(); + recv!( + s, + [TcpRepr { + control: TcpControl::Syn, + seq_number: LOCAL_SEQ, + ack_number: None, + max_seg_size: Some(BASE_MSS), + window_scale: Some(0), + sack_permitted: true, + ..RECV_TEMPL + }] + ); + send!( + s, + TcpRepr { + control: TcpControl::Syn, + seq_number: REMOTE_SEQ, + ack_number: Some(LOCAL_SEQ + 1), + max_seg_size: Some(BASE_MSS - 80), + window_scale: Some(0), + sack_permitted: true, + ..SEND_TEMPL + } + ); + assert!(s.remote_has_sack); + + let mut s = socket_syn_sent(); + recv!( + s, + [TcpRepr { + control: TcpControl::Syn, + seq_number: LOCAL_SEQ, + ack_number: None, + max_seg_size: Some(BASE_MSS), + window_scale: Some(0), + sack_permitted: true, + ..RECV_TEMPL + }] + ); + send!( + s, + TcpRepr { + control: TcpControl::Syn, + seq_number: REMOTE_SEQ, + ack_number: Some(LOCAL_SEQ + 1), + max_seg_size: Some(BASE_MSS - 80), + window_scale: Some(0), + sack_permitted: false, + ..SEND_TEMPL + } + ); + assert!(!s.remote_has_sack); + } + #[test] fn test_syn_sent_win_scale_buffers() { for (buffer_size, shift_amt) in &[ From b59e494ae7d90e58a9d476098fb96b5d26c73066 Mon Sep 17 00:00:00 2001 From: d2weber <29163905+d2weber@users.noreply.github.com> Date: Tue, 10 Sep 2024 18:06:09 +0200 Subject: [PATCH 25/25] Fixes #982 --- .github/workflows/test.yml | 8 ++++++++ ci.sh | 14 ++++++++++++++ src/socket/tcp.rs | 16 +++++++--------- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a13ca959..a3e79ec6e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -47,3 +47,11 @@ jobs: - uses: actions/checkout@v4 - name: Run Tests nightly run: ./ci.sh test nightly + + test-build-16bit: + runs-on: ubuntu-22.04 + continue-on-error: true + steps: + - uses: actions/checkout@v4 + - name: Build for target with 16 bit pointer + run: ./ci.sh build_16bit diff --git a/ci.sh b/ci.sh index 109047719..eb9235f7e 100755 --- a/ci.sh +++ b/ci.sh @@ -82,6 +82,16 @@ clippy() { cargo +$MSRV clippy --tests --examples -- -D warnings } +build_16bit() { + rustup toolchain install nightly + rustup +nightly component add rust-src + + TARGET_WITH_16BIT_POINTER=msp430-none-elf + for features in ${FEATURES_CHECK[@]}; do + cargo +nightly build -Z build-std=core,alloc --target $TARGET_WITH_16BIT_POINTER --no-default-features --features=$features + done +} + coverage() { for features in ${FEATURES_TEST[@]}; do cargo llvm-cov --no-report --no-default-features --features "$features" @@ -121,6 +131,10 @@ if [[ $1 == "clippy" || $1 == "all" ]]; then clippy fi +if [[ $1 == "build_16bit" || $1 == "all" ]]; then + build_16bit +fi + if [[ $1 == "coverage" || $1 == "all" ]]; then coverage fi diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 46f1b36f9..9460fa5c0 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -5,7 +5,7 @@ use core::fmt::Display; #[cfg(feature = "async")] use core::task::Waker; -use core::{cmp, fmt, mem}; +use core::{fmt, mem}; #[cfg(feature = "async")] use crate::socket::WakerRegistration; @@ -510,6 +510,7 @@ impl<'a> Socket<'a> { // [...] the above constraints imply that 2 * the max window size must be less // than 2**31 [...] Thus, the shift count must be limited to 14 (which allows // windows of 2**30 = 1 Gbyte). + #[cfg(not(target_pointer_width = "16"))] // Prevent overflow if rx_capacity > (1 << 30) { panic!("receiving buffer too large, cannot exceed 1 GiB") } @@ -676,10 +677,7 @@ impl<'a> Socket<'a> { /// Used in internal calculations as well as packet generation. #[inline] fn scaled_window(&self) -> u16 { - cmp::min( - self.rx_buffer.window() >> self.remote_win_shift as usize, - (1 << 16) - 1, - ) as u16 + u16::try_from(self.rx_buffer.window() >> self.remote_win_shift).unwrap_or(u16::MAX) } /// Return the last window field value, including scaling according to RFC 1323. @@ -698,7 +696,7 @@ impl<'a> Socket<'a> { let last_win = (self.remote_last_win as usize) << self.remote_win_shift; let last_win_adjusted = last_ack + last_win - next_ack; - Some(cmp::min(last_win_adjusted >> self.remote_win_shift, (1 << 16) - 1) as u16) + Some(u16::try_from(last_win_adjusted >> self.remote_win_shift).unwrap_or(u16::MAX)) } /// Set the timeout duration. @@ -2335,7 +2333,7 @@ impl<'a> Socket<'a> { State::SynSent | State::SynReceived => { repr.control = TcpControl::Syn; // window len must NOT be scaled in SYNs. - repr.window_len = self.rx_buffer.window().min((1 << 16) - 1) as u16; + repr.window_len = u16::try_from(self.rx_buffer.window()).unwrap_or(u16::MAX); if self.state == State::SynSent { repr.ack_number = None; repr.window_scale = Some(self.remote_win_shift); @@ -3075,7 +3073,7 @@ mod test { ack_number: Some(REMOTE_SEQ + 1), max_seg_size: Some(BASE_MSS), window_scale: Some(*shift_amt), - window_len: cmp::min(*buffer_size, 65535) as u16, + window_len: u16::try_from(*buffer_size).unwrap_or(u16::MAX), ..RECV_TEMPL }] ); @@ -3810,7 +3808,7 @@ mod test { ack_number: None, max_seg_size: Some(BASE_MSS), window_scale: Some(*shift_amt), - window_len: cmp::min(*buffer_size, 65535) as u16, + window_len: u16::try_from(*buffer_size).unwrap_or(u16::MAX), sack_permitted: true, ..RECV_TEMPL }]