From aba9add87379e021fd9d8a9f5718a62d2707a0bf Mon Sep 17 00:00:00 2001 From: zhangjio21 Date: Fri, 1 Nov 2024 19:31:07 +0800 Subject: [PATCH 01/21] fix bug --- src/iface/interface/mod.rs | 2 +- src/iface/interface/tests/ipv4.rs | 76 +++++++++++++++++++++++++++++++ src/lib.rs | 2 +- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs index cfac7f205..4df6d7378 100644 --- a/src/iface/interface/mod.rs +++ b/src/iface/interface/mod.rs @@ -1201,7 +1201,7 @@ impl InterfaceInner { #[cfg(feature = "proto-ipv4")] IpRepr::Ipv4(repr) => { // If we have an IPv4 packet, then we need to check if we need to fragment it. - if total_ip_len > self.caps.max_transmission_unit { + if total_ip_len > self.caps.ip_mtu() { #[cfg(feature = "proto-ipv4-fragmentation")] { net_debug!("start fragmentation"); diff --git a/src/iface/interface/tests/ipv4.rs b/src/iface/interface/tests/ipv4.rs index d3b8c61e8..a29bd20f8 100644 --- a/src/iface/interface/tests/ipv4.rs +++ b/src/iface/interface/tests/ipv4.rs @@ -759,6 +759,82 @@ fn test_handle_igmp(#[case] medium: Medium) { } } +#[rstest] +#[case(Medium::Ip)] +#[cfg(all(feature = "proto-ipv4-fragmentation", feature = "medium-ip"))] +#[case(Medium::Ethernet)] +#[cfg(all(feature = "proto-ipv4-fragmentation", feature = "medium-ethernet"))] +fn test_packet_len(#[case] medium: Medium) { + use crate::config::FRAGMENTATION_BUFFER_SIZE; + + let (mut iface, _, _) = setup(medium); + + struct TestTxToken { + max_transmission_unit: usize, + } + + impl TxToken for TestTxToken { + fn consume(self, len: usize, f: F) -> R + where + F: FnOnce(&mut [u8]) -> R, + { + net_debug!("TxToken get len: {}", len); + assert!(len <= self.max_transmission_unit); + let mut junk = [0; 1536]; + f(&mut junk[..len]) + } + } + + iface.inner.neighbor_cache.fill( + IpAddress::Ipv4(Ipv4Address::new(127, 0, 0, 1)), + HardwareAddress::Ethernet(EthernetAddress::from_bytes(&[ + 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, + ])), + Instant::ZERO, + ); + + for ip_packet_len in [ + 100, + iface.inner.ip_mtu(), + iface.inner.ip_mtu() + 1, + FRAGMENTATION_BUFFER_SIZE, + ] { + net_debug!("ip_packet_len: {}", ip_packet_len); + + let mut ip_repr = Ipv4Repr { + src_addr: Ipv4Address::new(127, 0, 0, 1), + dst_addr: Ipv4Address::new(127, 0, 0, 1), + next_header: IpProtocol::Udp, + payload_len: 0, + hop_limit: 64, + }; + let udp_repr = UdpRepr { + src_port: 12345, + dst_port: 54321, + }; + + let ip_packet_payload_len = ip_packet_len - ip_repr.buffer_len(); + let udp_packet_payload_len = ip_packet_payload_len - udp_repr.header_len(); + ip_repr.payload_len = ip_packet_payload_len; + + let udp_packet_payload = vec![1; udp_packet_payload_len]; + let ip_payload = IpPayload::Udp(udp_repr, &udp_packet_payload); + let ip_packet = Packet::new_ipv4(ip_repr, ip_payload); + + assert_eq!( + iface.inner.dispatch_ip( + TestTxToken { + max_transmission_unit: iface.inner.caps.max_transmission_unit + }, + PacketMeta::default(), + ip_packet, + &mut iface.fragmenter, + ), + Ok(()) + ); + } +} + #[rstest] #[case(Medium::Ip)] #[cfg(all(feature = "socket-raw", feature = "medium-ip"))] diff --git a/src/lib.rs b/src/lib.rs index 8f2ba3b37..4b1073501 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -136,7 +136,7 @@ pub mod config { pub const DNS_MAX_NAME_SIZE: usize = 255; pub const DNS_MAX_RESULT_COUNT: usize = 1; pub const DNS_MAX_SERVER_COUNT: usize = 1; - pub const FRAGMENTATION_BUFFER_SIZE: usize = 1500; + pub const FRAGMENTATION_BUFFER_SIZE: usize = 4096; pub const IFACE_MAX_ADDR_COUNT: usize = 8; pub const IFACE_MAX_MULTICAST_GROUP_COUNT: usize = 4; pub const IFACE_MAX_ROUTE_COUNT: usize = 4; From 4b79e3395ec72fab034e9dc0d8147fe3722bdc05 Mon Sep 17 00:00:00 2001 From: Koen Zandberg Date: Fri, 8 Nov 2024 15:59:23 +0100 Subject: [PATCH 02/21] feat: Add MLDv2 Listener Query response support This add support for responding to MLDv2 Listener Queries. Both general queries and multicast specific queries are supported. --- src/iface/interface/ipv6.rs | 10 +++ src/iface/interface/multicast.rs | 107 ++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/src/iface/interface/ipv6.rs b/src/iface/interface/ipv6.rs index 96e999f7e..ca8c2ef35 100644 --- a/src/iface/interface/ipv6.rs +++ b/src/iface/interface/ipv6.rs @@ -422,6 +422,16 @@ impl InterfaceInner { #[cfg(feature = "medium-ip")] Medium::Ip => None, }, + #[cfg(feature = "multicast")] + Icmpv6Repr::Mld(repr) => match repr { + // [RFC 3810 ยง 6.2], reception checks + MldRepr::Query { .. } + if ip_repr.hop_limit == 1 && ip_repr.src_addr.is_link_local() => + { + self.process_mldv2(ip_repr, repr) + } + _ => None, + }, // Don't report an error if a packet with unknown type // has been handled by an ICMP socket diff --git a/src/iface/interface/multicast.rs b/src/iface/interface/multicast.rs index 68b1c7750..cf7b1914b 100644 --- a/src/iface/interface/multicast.rs +++ b/src/iface/interface/multicast.rs @@ -1,7 +1,7 @@ use core::result::Result; use heapless::LinearMap; -#[cfg(feature = "proto-ipv4")] +#[cfg(any(feature = "proto-ipv4", feature = "proto-ipv6"))] use super::{check, IpPayload, Packet}; use super::{Interface, InterfaceInner}; use crate::config::IFACE_MAX_MULTICAST_GROUP_COUNT; @@ -34,6 +34,18 @@ pub(crate) enum IgmpReportState { }, } +#[cfg(feature = "proto-ipv6")] +pub(crate) enum MldReportState { + Inactive, + ToGeneralQuery { + timeout: crate::time::Instant, + }, + ToSpecificQuery { + group: Ipv6Address, + timeout: crate::time::Instant, + }, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum GroupState { /// Joining group, we have to send the join packet. @@ -49,6 +61,8 @@ pub(crate) struct State { /// When to report for (all or) the next multicast group membership via IGMP #[cfg(feature = "proto-ipv4")] igmp_report_state: IgmpReportState, + #[cfg(feature = "proto-ipv6")] + mld_report_state: MldReportState, } impl State { @@ -57,6 +71,8 @@ impl State { groups: LinearMap::new(), #[cfg(feature = "proto-ipv4")] igmp_report_state: IgmpReportState::Inactive, + #[cfg(feature = "proto-ipv6")] + mld_report_state: MldReportState::Inactive, } } @@ -306,6 +322,46 @@ impl Interface { } _ => {} } + #[cfg(feature = "proto-ipv6")] + match self.inner.multicast.mld_report_state { + MldReportState::ToGeneralQuery { timeout } if self.inner.now >= timeout => { + let records = self + .inner + .multicast + .groups + .iter() + .filter_map(|(addr, _)| match addr { + IpAddress::Ipv6(addr) => Some(MldAddressRecordRepr::new( + MldRecordType::ModeIsExclude, + *addr, + )), + #[allow(unreachable_patterns)] + _ => None, + }) + .collect::>(); + if let Some(pkt) = self.inner.mldv2_report_packet(&records) { + if let Some(tx_token) = device.transmit(self.inner.now) { + self.inner + .dispatch_ip(tx_token, PacketMeta::default(), pkt, &mut self.fragmenter) + .unwrap(); + }; + }; + self.inner.multicast.mld_report_state = MldReportState::Inactive; + } + MldReportState::ToSpecificQuery { group, timeout } if self.inner.now >= timeout => { + let record = MldAddressRecordRepr::new(MldRecordType::ModeIsExclude, group); + if let Some(pkt) = self.inner.mldv2_report_packet(&[record]) { + if let Some(tx_token) = device.transmit(self.inner.now) { + // 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.mld_report_state = MldReportState::Inactive; + } + _ => {} + } } } @@ -425,4 +481,53 @@ impl InterfaceInner { ) }) } + + /// Host duties of the **MLDv2** protocol. + /// + /// Sets up `mld_report_state` for responding to MLD 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; Currently the delay is fixed and not randomized. + #[cfg(feature = "proto-ipv6")] + pub(super) fn process_mldv2<'frame>( + &mut self, + ip_repr: Ipv6Repr, + repr: MldRepr<'frame>, + ) -> Option> { + match repr { + MldRepr::Query { + mcast_addr, + max_resp_code, + .. + } => { + // Do not respont immediately to the query + let delay = crate::time::Duration::from_millis(max_resp_code.into()) / 3; + // General query + if mcast_addr.is_unspecified() + && (ip_repr.dst_addr == IPV6_LINK_LOCAL_ALL_NODES + || self.has_ip_addr(ip_repr.dst_addr)) + { + let ipv6_multicast_group_count = self + .multicast + .groups + .keys() + .filter(|a| matches!(a, IpAddress::Ipv6(_))) + .count(); + if ipv6_multicast_group_count != 0 { + self.multicast.mld_report_state = MldReportState::ToGeneralQuery { + timeout: self.now + delay, + }; + } + } + if self.has_multicast_group(mcast_addr) && ip_repr.dst_addr == mcast_addr { + self.multicast.mld_report_state = MldReportState::ToSpecificQuery { + group: mcast_addr, + timeout: self.now + delay, + }; + } + None + } + MldRepr::Report { .. } => None, + MldRepr::ReportRecordReprs { .. } => None, + } + } } From 9ed4e3a1515597cd7ac98769694a1abd576ed98b Mon Sep 17 00:00:00 2001 From: Astro Date: Sat, 9 Nov 2024 23:08:03 +0100 Subject: [PATCH 03/21] tests: s/ndsic/ndisc/ --- src/iface/interface/tests/ipv6.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/iface/interface/tests/ipv6.rs b/src/iface/interface/tests/ipv6.rs index adc058fd3..f67737b31 100644 --- a/src/iface/interface/tests/ipv6.rs +++ b/src/iface/interface/tests/ipv6.rs @@ -601,7 +601,7 @@ fn unknown_proto(#[case] medium: Medium) { #[rstest] #[case::ethernet(Medium::Ethernet)] #[cfg(feature = "medium-ethernet")] -fn ndsic_neighbor_advertisement_ethernet(#[case] medium: Medium) { +fn ndisc_neighbor_advertisement_ethernet(#[case] medium: Medium) { let data = [ 0x60, 0x0, 0x0, 0x0, 0x0, 0x20, 0x3a, 0xff, 0xfd, 0xbe, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0xfd, 0xbe, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @@ -656,7 +656,7 @@ fn ndsic_neighbor_advertisement_ethernet(#[case] medium: Medium) { #[rstest] #[case::ethernet(Medium::Ethernet)] #[cfg(feature = "medium-ethernet")] -fn ndsic_neighbor_advertisement_ethernet_multicast_addr(#[case] medium: Medium) { +fn ndisc_neighbor_advertisement_ethernet_multicast_addr(#[case] medium: Medium) { let data = [ 0x60, 0x0, 0x0, 0x0, 0x0, 0x20, 0x3a, 0xff, 0xfd, 0xbe, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0xfd, 0xbe, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @@ -711,7 +711,7 @@ fn ndsic_neighbor_advertisement_ethernet_multicast_addr(#[case] medium: Medium) #[rstest] #[case::ieee802154(Medium::Ieee802154)] #[cfg(feature = "medium-ieee802154")] -fn ndsic_neighbor_advertisement_ieee802154(#[case] medium: Medium) { +fn ndisc_neighbor_advertisement_ieee802154(#[case] medium: Medium) { let data = [ 0x60, 0x0, 0x0, 0x0, 0x0, 0x28, 0x3a, 0xff, 0xfd, 0xbe, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0xfd, 0xbe, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, From 41be841b47716de0c36b6471f2c218d388217a3a Mon Sep 17 00:00:00 2001 From: Koen Zandberg Date: Wed, 13 Nov 2024 11:42:14 +0100 Subject: [PATCH 04/21] fixup! feat: Add MLDv2 Listener Query response support --- src/iface/interface/multicast.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/iface/interface/multicast.rs b/src/iface/interface/multicast.rs index cf7b1914b..6f67c3d9c 100644 --- a/src/iface/interface/multicast.rs +++ b/src/iface/interface/multicast.rs @@ -499,8 +499,10 @@ impl InterfaceInner { max_resp_code, .. } => { - // Do not respont immediately to the query - let delay = crate::time::Duration::from_millis(max_resp_code.into()) / 3; + // Do not respont immediately to the query, but wait a random time + let delay = crate::time::Duration::from_millis( + (self.rand.rand_u16() % max_resp_code).into(), + ); // General query if mcast_addr.is_unspecified() && (ip_repr.dst_addr == IPV6_LINK_LOCAL_ALL_NODES From 593a664e6f0e2cb28c8ee2a2ae427e3c3de94dbf Mon Sep 17 00:00:00 2001 From: Koen Zandberg Date: Wed, 13 Nov 2024 11:43:07 +0100 Subject: [PATCH 05/21] fixup! fixup! feat: Add MLDv2 Listener Query response support --- src/iface/interface/tests/ipv6.rs | 184 ++++++++++++++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/src/iface/interface/tests/ipv6.rs b/src/iface/interface/tests/ipv6.rs index adc058fd3..08530b5b9 100644 --- a/src/iface/interface/tests/ipv6.rs +++ b/src/iface/interface/tests/ipv6.rs @@ -1378,3 +1378,187 @@ fn test_join_ipv6_multicast_group(#[case] medium: Medium) { assert!(!iface.has_multicast_group(group_addr)); } } + +#[rstest] +#[case(Medium::Ethernet)] +#[cfg(all(feature = "multicast", feature = "medium-ethernet"))] +fn test_handle_valid_multicast_query(#[case] medium: Medium) { + fn recv_icmpv6( + device: &mut crate::tests::TestingDevice, + timestamp: Instant, + ) -> std::vec::Vec>> { + let caps = device.capabilities(); + recv_all(device, timestamp) + .iter() + .filter_map(|frame| { + let ipv6_packet = match caps.medium { + #[cfg(feature = "medium-ethernet")] + Medium::Ethernet => { + let eth_frame = EthernetFrame::new_checked(frame).ok()?; + Ipv6Packet::new_checked(eth_frame.payload()).ok()? + } + #[cfg(feature = "medium-ip")] + Medium::Ip => Ipv6Packet::new_checked(&frame[..]).ok()?, + #[cfg(feature = "medium-ieee802154")] + Medium::Ieee802154 => todo!(), + }; + let buf = ipv6_packet.into_inner().to_vec(); + Some(Ipv6Packet::new_unchecked(buf)) + }) + .collect::>() + } + + let (mut iface, mut sockets, mut device) = setup(medium); + + let mut timestamp = Instant::ZERO; + + let mut eth_bytes = vec![0u8; 86]; + + let local_ip_addr = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 101); + let remote_ip_addr = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 100); + let remote_hw_addr = EthernetAddress([0x52, 0x54, 0x00, 0x00, 0x00, 0x00]); + let query_ip_addr = Ipv6Address::new(0xff02, 0, 0, 0, 0, 0, 0, 0x1234); + + iface.join_multicast_group(query_ip_addr).unwrap(); + iface + .join_multicast_group(local_ip_addr.solicited_node()) + .unwrap(); + + iface.poll(timestamp, &mut device, &mut sockets); + // flush multicast reports from the join_multicast_group calls + recv_icmpv6(&mut device, timestamp); + + let queries = [ + // General query, expect both multicast addresses back + ( + Ipv6Address::UNSPECIFIED, + IPV6_LINK_LOCAL_ALL_NODES, + vec![query_ip_addr, local_ip_addr.solicited_node()], + ), + // Address specific query, expect only the queried address back + (query_ip_addr, query_ip_addr, vec![query_ip_addr]), + ]; + + for (mcast_query, address, _results) in queries.iter() { + let query = Icmpv6Repr::Mld(MldRepr::Query { + max_resp_code: 1000, + mcast_addr: *mcast_query, + s_flag: false, + qrv: 1, + qqic: 60, + num_srcs: 0, + data: &[0, 0, 0, 0], + }); + + let ip_repr = IpRepr::Ipv6(Ipv6Repr { + src_addr: remote_ip_addr, + dst_addr: *address, + next_header: IpProtocol::Icmpv6, + hop_limit: 1, + payload_len: query.buffer_len(), + }); + + let mut frame = EthernetFrame::new_unchecked(&mut eth_bytes); + frame.set_dst_addr(EthernetAddress([0x33, 0x33, 0x00, 0x00, 0x00, 0x00])); + frame.set_src_addr(remote_hw_addr); + frame.set_ethertype(EthernetProtocol::Ipv6); + ip_repr.emit(frame.payload_mut(), &ChecksumCapabilities::default()); + query.emit( + &remote_ip_addr, + address, + &mut Icmpv6Packet::new_unchecked(&mut frame.payload_mut()[ip_repr.header_len()..]), + &ChecksumCapabilities::default(), + ); + + iface.inner.process_ethernet( + &mut sockets, + PacketMeta::default(), + frame.into_inner(), + &mut iface.fragments, + ); + + timestamp += crate::time::Duration::from_millis(1000); + iface.poll(timestamp, &mut device, &mut sockets); + } + + let reports = recv_icmpv6(&mut device, timestamp); + assert_eq!(reports.len(), queries.len()); + + let caps = device.capabilities(); + let checksum_caps = &caps.checksum; + for ((_mcast_query, _address, results), ipv6_packet) in queries.iter().zip(reports) { + let buf = ipv6_packet.into_inner(); + let ipv6_packet = Ipv6Packet::new_unchecked(buf.as_slice()); + + let ipv6_repr = Ipv6Repr::parse(&ipv6_packet).unwrap(); + let ip_payload = ipv6_packet.payload(); + assert_eq!(ipv6_repr.dst_addr, IPV6_LINK_LOCAL_ALL_MLDV2_ROUTERS); + + // The first 2 octets of this payload hold the next-header indicator and the + // Hop-by-Hop header length (in 8-octet words, minus 1). The remaining 6 octets + // hold the Hop-by-Hop PadN and Router Alert options. + let hbh_header = Ipv6HopByHopHeader::new_checked(&ip_payload[..8]).unwrap(); + let hbh_repr = Ipv6HopByHopRepr::parse(&hbh_header).unwrap(); + + assert_eq!(hbh_repr.options.len(), 3); + assert_eq!( + hbh_repr.options[0], + Ipv6OptionRepr::Unknown { + type_: Ipv6OptionType::Unknown(IpProtocol::Icmpv6.into()), + length: 0, + data: &[], + } + ); + assert_eq!( + hbh_repr.options[1], + Ipv6OptionRepr::RouterAlert(Ipv6OptionRouterAlert::MulticastListenerDiscovery) + ); + assert_eq!(hbh_repr.options[2], Ipv6OptionRepr::PadN(0)); + + let icmpv6_packet = + Icmpv6Packet::new_checked(&ip_payload[hbh_repr.buffer_len()..]).unwrap(); + let icmpv6_repr = Icmpv6Repr::parse( + &ipv6_packet.src_addr(), + &ipv6_packet.dst_addr(), + &icmpv6_packet, + checksum_caps, + ) + .unwrap(); + + let record_data = match icmpv6_repr { + Icmpv6Repr::Mld(MldRepr::Report { + nr_mcast_addr_rcrds, + data, + }) => { + assert_eq!(nr_mcast_addr_rcrds, results.len() as u16); + data + } + other => panic!("unexpected icmpv6_repr: {:?}", other), + }; + + let mut record_reprs = Vec::new(); + let mut payload = record_data; + + // FIXME: parsing multiple address records should be done by the MLD code + while !payload.is_empty() { + let record = MldAddressRecord::new_checked(payload).unwrap(); + let mut record_repr = MldAddressRecordRepr::parse(&record).unwrap(); + payload = record_repr.payload; + record_repr.payload = &[]; + record_reprs.push(record_repr); + } + + let expected_records = results + .iter() + .map(|addr| MldAddressRecordRepr { + num_srcs: 0, + mcast_addr: *addr, + record_type: MldRecordType::ModeIsExclude, + aux_data_len: 0, + payload: &[], + }) + .collect::>(); + + assert_eq!(record_reprs, expected_records); + } +} From ae587fc221a638d223b450a4a353328838f92c10 Mon Sep 17 00:00:00 2001 From: Will Bicks Date: Tue, 26 Nov 2024 11:39:08 -0500 Subject: [PATCH 06/21] docs: Improve UdpMetadata doc comments. Fix local_address doc to indicate purpose on outgoing datagrams. Add rustdoc comment to endpoint field. --- src/socket/udp.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/socket/udp.rs b/src/socket/udp.rs index 1e09116f3..b1a843904 100644 --- a/src/socket/udp.rs +++ b/src/socket/udp.rs @@ -14,8 +14,10 @@ use crate::wire::{IpAddress, IpEndpoint, IpListenEndpoint, IpProtocol, IpRepr, U #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub struct UdpMetadata { + /// The IP endpoint from which an incoming datagram was received, or to which an outgoing + /// datagram will be sent. pub endpoint: IpEndpoint, - /// The IP address to which an incoming datagram was sent, or to which an outgoing datagram + /// The IP address to which an incoming datagram was sent, or from which an outgoing datagram /// will be sent. Incoming datagrams always have this set. On outgoing datagrams, if it is not /// set, and the socket is not bound to a single address anyway, a suitable address will be /// determined using the algorithms of RFC 6724 (candidate source address selection) or some From 2d5a2009593a8f1a5bf05075ec076191e77fa3db Mon Sep 17 00:00:00 2001 From: Koen Zandberg Date: Thu, 14 Nov 2024 07:54:26 +0100 Subject: [PATCH 07/21] feat(ipv6): Extend with sol node mcast check --- src/wire/ipv6.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/wire/ipv6.rs b/src/wire/ipv6.rs index 8dc05c51d..24904b60a 100644 --- a/src/wire/ipv6.rs +++ b/src/wire/ipv6.rs @@ -125,6 +125,11 @@ pub(crate) trait AddressExt { /// `x_` prefix is to avoid a collision with the still-unstable method in `core::ip`. fn x_multicast_scope(&self) -> MulticastScope; + /// Query whether the IPv6 address is a [solicited-node multicast address]. + /// + /// [Solicited-node multicast address]: https://datatracker.ietf.org/doc/html/rfc4291#section-2.7.1 + fn is_solicited_node_multicast(&self) -> bool; + /// If `self` is a CIDR-compatible subnet mask, return `Some(prefix_len)`, /// where `prefix_len` is the number of leading zeroes. Return `None` otherwise. fn prefix_len(&self) -> Option; @@ -193,6 +198,13 @@ impl AddressExt for Address { } } + fn is_solicited_node_multicast(&self) -> bool { + self.octets()[0..13] + == [ + 0xff, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0xFF, + ] + } + fn prefix_len(&self) -> Option { let mut ones = true; let mut prefix_len = 0; @@ -680,6 +692,8 @@ pub(crate) mod test { const UNIQUE_LOCAL_ADDR: Address = Address::new(0xfd00, 0, 0, 201, 1, 1, 1, 1); const GLOBAL_UNICAST_ADDR: Address = Address::new(0x2001, 0xdb8, 0x3, 0, 0, 0, 0, 1); + const TEST_SOL_NODE_MCAST_ADDR: Address = Address::new(0xff02, 0, 0, 0, 0, 1, 0xff01, 101); + #[test] fn test_basic_multicast() { assert!(!LINK_LOCAL_ALL_ROUTERS.is_unspecified()); @@ -688,12 +702,14 @@ pub(crate) mod test { assert!(!LINK_LOCAL_ALL_ROUTERS.is_loopback()); assert!(!LINK_LOCAL_ALL_ROUTERS.x_is_unique_local()); assert!(!LINK_LOCAL_ALL_ROUTERS.is_global_unicast()); + assert!(!LINK_LOCAL_ALL_ROUTERS.is_solicited_node_multicast()); assert!(!LINK_LOCAL_ALL_NODES.is_unspecified()); assert!(LINK_LOCAL_ALL_NODES.is_multicast()); assert!(!LINK_LOCAL_ALL_NODES.is_link_local()); assert!(!LINK_LOCAL_ALL_NODES.is_loopback()); assert!(!LINK_LOCAL_ALL_NODES.x_is_unique_local()); assert!(!LINK_LOCAL_ALL_NODES.is_global_unicast()); + assert!(!LINK_LOCAL_ALL_NODES.is_solicited_node_multicast()); } #[test] @@ -704,6 +720,7 @@ pub(crate) mod test { assert!(!LINK_LOCAL_ADDR.is_loopback()); assert!(!LINK_LOCAL_ADDR.x_is_unique_local()); assert!(!LINK_LOCAL_ADDR.is_global_unicast()); + assert!(!LINK_LOCAL_ADDR.is_solicited_node_multicast()); } #[test] @@ -714,6 +731,7 @@ pub(crate) mod test { assert!(Address::LOCALHOST.is_loopback()); assert!(!Address::LOCALHOST.x_is_unique_local()); assert!(!Address::LOCALHOST.is_global_unicast()); + assert!(!Address::LOCALHOST.is_solicited_node_multicast()); } #[test] @@ -724,6 +742,7 @@ pub(crate) mod test { assert!(!UNIQUE_LOCAL_ADDR.is_loopback()); assert!(UNIQUE_LOCAL_ADDR.x_is_unique_local()); assert!(!UNIQUE_LOCAL_ADDR.is_global_unicast()); + assert!(!UNIQUE_LOCAL_ADDR.is_solicited_node_multicast()); } #[test] @@ -734,6 +753,18 @@ pub(crate) mod test { assert!(!GLOBAL_UNICAST_ADDR.is_loopback()); assert!(!GLOBAL_UNICAST_ADDR.x_is_unique_local()); assert!(GLOBAL_UNICAST_ADDR.is_global_unicast()); + assert!(!GLOBAL_UNICAST_ADDR.is_solicited_node_multicast()); + } + + #[test] + fn test_sollicited_node_multicast() { + assert!(!TEST_SOL_NODE_MCAST_ADDR.is_unspecified()); + assert!(TEST_SOL_NODE_MCAST_ADDR.is_multicast()); + assert!(!TEST_SOL_NODE_MCAST_ADDR.is_link_local()); + assert!(!TEST_SOL_NODE_MCAST_ADDR.is_loopback()); + assert!(!TEST_SOL_NODE_MCAST_ADDR.x_is_unique_local()); + assert!(!TEST_SOL_NODE_MCAST_ADDR.is_global_unicast()); + assert!(TEST_SOL_NODE_MCAST_ADDR.is_solicited_node_multicast()); } #[test] From 2a5f90ab0b664454d478c173cb0d28335f874553 Mon Sep 17 00:00:00 2001 From: Koen Zandberg Date: Thu, 14 Nov 2024 10:40:20 +0100 Subject: [PATCH 08/21] feat: Automatically join sol-node mcast addresses IPv6 over Ethernet should join the solicited-node multicast addresses required by the configured IPv6 addresses, as neighbor solicitations for these addresses have the solicited-node multicast address as destination address. This commit automatically leaves old solicited-node multicast addresses and joins the new set when the IP addresses on the interface are updated. --- src/iface/interface/mod.rs | 7 ++++++- src/iface/interface/multicast.rs | 34 +++++++++++++++++++++++++++++-- src/iface/interface/tests/ipv6.rs | 14 +++++++++---- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs index 4df6d7378..4a2458ab6 100644 --- a/src/iface/interface/mod.rs +++ b/src/iface/interface/mod.rs @@ -361,7 +361,12 @@ impl Interface { pub fn update_ip_addrs)>(&mut self, f: F) { f(&mut self.inner.ip_addrs); InterfaceInner::flush_neighbor_cache(&mut self.inner); - InterfaceInner::check_ip_addrs(&self.inner.ip_addrs) + InterfaceInner::check_ip_addrs(&self.inner.ip_addrs); + + #[cfg(all(feature = "proto-ipv6", feature = "multicast"))] + if self.inner.caps.medium == Medium::Ethernet { + self.update_solicited_node_groups(); + } } /// Check whether the interface has the given IP address assigned. diff --git a/src/iface/interface/multicast.rs b/src/iface/interface/multicast.rs index 6f67c3d9c..e79194a24 100644 --- a/src/iface/interface/multicast.rs +++ b/src/iface/interface/multicast.rs @@ -1,10 +1,10 @@ use core::result::Result; -use heapless::LinearMap; +use heapless::{LinearMap, Vec}; #[cfg(any(feature = "proto-ipv4", feature = "proto-ipv6"))] use super::{check, IpPayload, Packet}; use super::{Interface, InterfaceInner}; -use crate::config::IFACE_MAX_MULTICAST_GROUP_COUNT; +use crate::config::{IFACE_MAX_ADDR_COUNT, IFACE_MAX_MULTICAST_GROUP_COUNT}; use crate::phy::{Device, PacketMeta}; use crate::wire::*; @@ -156,6 +156,36 @@ impl Interface { self.inner.has_multicast_group(addr) } + #[cfg(feature = "proto-ipv6")] + pub(super) fn update_solicited_node_groups(&mut self) { + // Remove old solicited-node multicast addresses + let removals: Vec<_, IFACE_MAX_MULTICAST_GROUP_COUNT> = self + .inner + .multicast + .groups + .keys() + .filter_map(|group_addr| match group_addr { + IpAddress::Ipv6(address) + if address.is_solicited_node_multicast() + && self.inner.has_solicited_node(*address) => + { + Some(*group_addr) + } + _ => None, + }) + .collect(); + for removal in removals { + let _ = self.leave_multicast_group(removal); + } + + let cidrs: Vec = Vec::from_slice(self.ip_addrs()).unwrap(); + for cidr in cidrs { + if let IpCidr::Ipv6(cidr) = cidr { + let _ = self.join_multicast_group(cidr.address().solicited_node()); + } + } + } + /// Do multicast egress. /// /// - Send join/leave packets according to the multicast group state. diff --git a/src/iface/interface/tests/ipv6.rs b/src/iface/interface/tests/ipv6.rs index 75687e5c3..ca1df2def 100644 --- a/src/iface/interface/tests/ipv6.rs +++ b/src/iface/interface/tests/ipv6.rs @@ -1296,6 +1296,10 @@ fn test_join_ipv6_multicast_group(#[case] medium: Medium) { let timestamp = Instant::from_millis(0); + // Drain the unsolicited node multicast report from the device + iface.poll(timestamp, &mut device, &mut sockets); + let _ = recv_icmpv6(&mut device, timestamp); + for &group in &groups { iface.join_multicast_group(group).unwrap(); assert!(iface.has_multicast_group(group)); @@ -1372,10 +1376,12 @@ fn test_join_ipv6_multicast_group(#[case] medium: Medium) { } ); - 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)); + if !group_addr.is_solicited_node_multicast() { + 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 2be54073f1c9e298a5d7d80ad3a42b0355f47aa8 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 28 Nov 2024 00:56:34 +0100 Subject: [PATCH 09/21] remove multicast groups that we *don't* want as solicited node. --- src/iface/interface/multicast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iface/interface/multicast.rs b/src/iface/interface/multicast.rs index e79194a24..37ae1f3c1 100644 --- a/src/iface/interface/multicast.rs +++ b/src/iface/interface/multicast.rs @@ -167,7 +167,7 @@ impl Interface { .filter_map(|group_addr| match group_addr { IpAddress::Ipv6(address) if address.is_solicited_node_multicast() - && self.inner.has_solicited_node(*address) => + && !self.inner.has_solicited_node(*address) => { Some(*group_addr) } From e9cf1c51a59307a3c8ca1d0d6b24052ef6eb08de Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 28 Nov 2024 00:58:01 +0100 Subject: [PATCH 10/21] iface: make iterator chain a bit more readable. --- src/iface/interface/multicast.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/iface/interface/multicast.rs b/src/iface/interface/multicast.rs index 37ae1f3c1..45261a8fd 100644 --- a/src/iface/interface/multicast.rs +++ b/src/iface/interface/multicast.rs @@ -164,15 +164,8 @@ impl Interface { .multicast .groups .keys() - .filter_map(|group_addr| match group_addr { - IpAddress::Ipv6(address) - if address.is_solicited_node_multicast() - && !self.inner.has_solicited_node(*address) => - { - Some(*group_addr) - } - _ => None, - }) + .cloned() + .filter(|a| matches!(a, IpAddress::Ipv6(a) if a.is_solicited_node_multicast() && !self.inner.has_solicited_node(*a))) .collect(); for removal in removals { let _ = self.leave_multicast_group(removal); From e9bf78c69d7500e1bb1497118f8bd57aadaa90d9 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 28 Nov 2024 00:58:14 +0100 Subject: [PATCH 11/21] iface: add test for ipv6 solicited node autojoin. --- src/iface/interface/tests/ipv6.rs | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/iface/interface/tests/ipv6.rs b/src/iface/interface/tests/ipv6.rs index ca1df2def..dec4bfd36 100644 --- a/src/iface/interface/tests/ipv6.rs +++ b/src/iface/interface/tests/ipv6.rs @@ -1568,3 +1568,41 @@ fn test_handle_valid_multicast_query(#[case] medium: Medium) { assert_eq!(record_reprs, expected_records); } } + +#[rstest] +#[case(Medium::Ethernet)] +#[cfg(all(feature = "multicast", feature = "medium-ethernet"))] +fn test_solicited_node_multicast_autojoin(#[case] medium: Medium) { + let (mut iface, _, _) = setup(medium); + + let addr1 = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 1); + let addr2 = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 2); + + iface.update_ip_addrs(|ip_addrs| { + ip_addrs.clear(); + ip_addrs.push(IpCidr::new(addr1.into(), 64)).unwrap(); + }); + assert!(iface.has_multicast_group(addr1.solicited_node())); + assert!(!iface.has_multicast_group(addr2.solicited_node())); + + iface.update_ip_addrs(|ip_addrs| { + ip_addrs.clear(); + ip_addrs.push(IpCidr::new(addr2.into(), 64)).unwrap(); + }); + assert!(!iface.has_multicast_group(addr1.solicited_node())); + assert!(iface.has_multicast_group(addr2.solicited_node())); + + iface.update_ip_addrs(|ip_addrs| { + ip_addrs.clear(); + ip_addrs.push(IpCidr::new(addr1.into(), 64)).unwrap(); + ip_addrs.push(IpCidr::new(addr2.into(), 64)).unwrap(); + }); + assert!(iface.has_multicast_group(addr1.solicited_node())); + assert!(iface.has_multicast_group(addr2.solicited_node())); + + iface.update_ip_addrs(|ip_addrs| { + ip_addrs.clear(); + }); + assert!(!iface.has_multicast_group(addr1.solicited_node())); + assert!(!iface.has_multicast_group(addr2.solicited_node())); +} From 0385bad9332895292948ad3eb1ea2769af57d492 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 28 Nov 2024 01:05:06 +0100 Subject: [PATCH 12/21] iface: fix mld test. --- src/iface/interface/tests/ipv6.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/iface/interface/tests/ipv6.rs b/src/iface/interface/tests/ipv6.rs index dec4bfd36..5a75b5b8c 100644 --- a/src/iface/interface/tests/ipv6.rs +++ b/src/iface/interface/tests/ipv6.rs @@ -1420,15 +1420,12 @@ fn test_handle_valid_multicast_query(#[case] medium: Medium) { let mut eth_bytes = vec![0u8; 86]; - let local_ip_addr = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 101); + let local_ip_addr = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 1); let remote_ip_addr = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 100); let remote_hw_addr = EthernetAddress([0x52, 0x54, 0x00, 0x00, 0x00, 0x00]); let query_ip_addr = Ipv6Address::new(0xff02, 0, 0, 0, 0, 0, 0, 0x1234); iface.join_multicast_group(query_ip_addr).unwrap(); - iface - .join_multicast_group(local_ip_addr.solicited_node()) - .unwrap(); iface.poll(timestamp, &mut device, &mut sockets); // flush multicast reports from the join_multicast_group calls @@ -1439,7 +1436,7 @@ fn test_handle_valid_multicast_query(#[case] medium: Medium) { ( Ipv6Address::UNSPECIFIED, IPV6_LINK_LOCAL_ALL_NODES, - vec![query_ip_addr, local_ip_addr.solicited_node()], + vec![local_ip_addr.solicited_node(), query_ip_addr], ), // Address specific query, expect only the queried address back (query_ip_addr, query_ip_addr, vec![query_ip_addr]), From d2d647090d544b1e7c142571da9d55f7280f664b Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 28 Nov 2024 01:24:56 +0100 Subject: [PATCH 13/21] Release v0.12.0 --- CHANGELOG.md | 55 ++++++++++++++++++++++++++++++++++++++++++++++++---- Cargo.toml | 2 +- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2d3f3b1e..102a4ddb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -# Changelog + # Changelog All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), @@ -6,8 +6,54 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Changed -- iface: The `poll` function now only performs a single cycle of processing sockets ([#954](https://github.com/smoltcp-rs/smoltcp/pull/954)) +No unreleased changes yet. Please send PRs! + +## [0.12.0] - 2024-11-28 + +Almost a year in the making, the highlights of the release are the migration to `core::net` IP types, IPv6 multicast, TCP improvements, and many fixes. Smoltcp now connects your gadgets to the Internet better than ever. + +- Minimum Supported Rust Version (MSRV) bumped to 1.80. +- iface + - IPv6 multicast ([#914](https://github.com/smoltcp-rs/smoltcp/pull/914), [#976](https://github.com/smoltcp-rs/smoltcp/pull/976), [#988](https://github.com/smoltcp-rs/smoltcp/pull/988), [#1009](https://github.com/smoltcp-rs/smoltcp/pull/1009), [#1012](https://github.com/smoltcp-rs/smoltcp/pull/1012)) + - Add `poll_egress()` and `poll_ingress_single()` methods for finer-grained control of what and how many packets are processed. ([#954](https://github.com/smoltcp-rs/smoltcp/pull/954), [#991](https://github.com/smoltcp-rs/smoltcp/pull/991), [#993](https://github.com/smoltcp-rs/smoltcp/pull/993)) + - Multicast join/leave no longer requires access to device+timestamp. ([#985](https://github.com/smoltcp-rs/smoltcp/pull/985)) + - Reset expiry of entries in the neighbor cache on packet reception ([#966](https://github.com/smoltcp-rs/smoltcp/pull/966)) + - Honor `any_ip` for ARP ([#880](https://github.com/smoltcp-rs/smoltcp/pull/880)) + - Honor `any_ip` for IPv6 ([#900](https://github.com/smoltcp-rs/smoltcp/pull/900)) + - Use own source address for ARP and NDISC Solicitations ([#984](https://github.com/smoltcp-rs/smoltcp/pull/984)) + - fix panic when discarding HBH Option with multicast destination address ([#996](https://github.com/smoltcp-rs/smoltcp/pull/996)) + - fix panic with 6lowpan frag datagram_size < 40 ([#997](https://github.com/smoltcp-rs/smoltcp/pull/997)) + - fix panic if no suitable IPv6 src_addr is found ([#895](https://github.com/smoltcp-rs/smoltcp/pull/895)) + - Fix specific length IP packets not being fragmented ([#1008](https://github.com/smoltcp-rs/smoltcp/pull/1008)) +- tcp + - Add support for congestion control ([#907](https://github.com/smoltcp-rs/smoltcp/pull/907)) + - Add support for simultaneous open ([#1001](https://github.com/smoltcp-rs/smoltcp/pull/1001)) + - Add support for Timestamp option ([#939](https://github.com/smoltcp-rs/smoltcp/pull/939)) + - Send immediate ACKs after RMSS bytes of data ([#1002](https://github.com/smoltcp-rs/smoltcp/pull/1002)) + - Do not ignore FIN if segment is partially outside the window. ([#977](https://github.com/smoltcp-rs/smoltcp/pull/977)) + - Correctly set internal sACK flag for client sockets ([#995](https://github.com/smoltcp-rs/smoltcp/pull/995)) + - Only reset remote_last_ts if some data is enqueued ([#917](https://github.com/smoltcp-rs/smoltcp/pull/917)) + - Don't delay ACKs for significant window updates ([#935](https://github.com/smoltcp-rs/smoltcp/pull/935)) + - Add `listen_endpoint` getter ([#1005](https://github.com/smoltcp-rs/smoltcp/pull/1005)) +- socket + - UDP,ICMP,raw: Add `send_queue`/`recv_queue` ([#1003](https://github.com/smoltcp-rs/smoltcp/pull/1003)) + - ICMP: split ICMPv4/v6 accept and process ([#887](https://github.com/smoltcp-rs/smoltcp/pull/887)) + - UDP: Store local and use local address in metadata ([#904](https://github.com/smoltcp-rs/smoltcp/pull/904)) + - DNS: fix panic if server list is too long ([#986](https://github.com/smoltcp-rs/smoltcp/pull/986)) + - DNS: fix panic if no valid source address is found ([#987](https://github.com/smoltcp-rs/smoltcp/pull/987)) +- phy + - Change mutability of `RxToken`'s `consume` argument. ([#924](https://github.com/smoltcp-rs/smoltcp/pull/924)) + - Add support for NetBSD ([#883](https://github.com/smoltcp-rs/smoltcp/pull/883)) + - Add minimum support for iOS ([#896](https://github.com/smoltcp-rs/smoltcp/pull/896)) + - Add BPF support for FreeBSD ([#906](https://github.com/smoltcp-rs/smoltcp/pull/906)) + - disable checksums on loopback ([#919](https://github.com/smoltcp-rs/smoltcp/pull/919)) +- wire + - Use core::net types for IP addresses. ([#937](https://github.com/smoltcp-rs/smoltcp/pull/937), [#994](https://github.com/smoltcp-rs/smoltcp/pull/994)) + - Add missing exports in wire for DNS ([#891](https://github.com/smoltcp-rs/smoltcp/pull/891)) + - rename Scope to MulticastScope ([#898](https://github.com/smoltcp-rs/smoltcp/pull/898)) + - Re-export `dhcpv4::Flags` and `dhcpv4::OpCode` ([#901](https://github.com/smoltcp-rs/smoltcp/pull/901)) + - Make Address:v6() constructor const ([#975](https://github.com/smoltcp-rs/smoltcp/pull/975)) + - Ipv6RoutingHeader::clear_reserved: fix offsets for Type 2 routing headers. ([#882](https://github.com/smoltcp-rs/smoltcp/pull/882)) ## [0.11.0] - 2023-12-23 @@ -281,7 +327,8 @@ only processed when directed to the 255.255.255.255 address. ([377](https://gith - Use #[non_exhaustive] for enums and structs ([409](https://github.com/smoltcp-rs/smoltcp/pull/409), [411](https://github.com/smoltcp-rs/smoltcp/pull/411)) - Simplify lifetime parameters of sockets, SocketSet, EthernetInterface ([410](https://github.com/smoltcp-rs/smoltcp/pull/410), [413](https://github.com/smoltcp-rs/smoltcp/pull/413)) -[Unreleased]: https://github.com/smoltcp-rs/smoltcp/compare/v0.11.0...HEAD +[Unreleased]: https://github.com/smoltcp-rs/smoltcp/compare/v0.12.0...HEAD +[0.12.0]: https://github.com/smoltcp-rs/smoltcp/compare/v0.11.0...v0.12.0 [0.11.0]: https://github.com/smoltcp-rs/smoltcp/compare/v0.10.0...v0.11.0 [0.10.0]: https://github.com/smoltcp-rs/smoltcp/compare/v0.9.1...v0.10.0 [0.9.1]: https://github.com/smoltcp-rs/smoltcp/compare/v0.9.0...v0.9.1 diff --git a/Cargo.toml b/Cargo.toml index 15b8321e7..c3ae7d556 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "smoltcp" -version = "0.11.0" +version = "0.12.0" edition = "2021" rust-version = "1.80" authors = ["whitequark "] From 65ddcf5fb1a78c217d3b30f539e16ebcb9b7a165 Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Thu, 28 Nov 2024 11:28:01 +0100 Subject: [PATCH 14/21] update codecov CI to latest version --- .github/workflows/coverage.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 6c2ee2c6d..86d210239 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -14,8 +14,8 @@ jobs: - name: Run Coverage run: ./ci.sh coverage - name: Upload coverage to Codecov - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v5 with: - #token: ${{ secrets.CODECOV_TOKEN }} # not required for public repos + token: ${{ secrets.CODECOV_TOKEN }} files: lcov.info fail_ci_if_error: false From 8a8f3afcd51131e440ffdfaeb6c221de926fc3e1 Mon Sep 17 00:00:00 2001 From: tomDev5 Date: Mon, 23 Dec 2024 11:29:49 +0200 Subject: [PATCH 15/21] rustfmt --- src/phy/fault_injector.rs | 6 ++++-- src/phy/fuzz_injector.rs | 6 ++++-- src/phy/pcap_writer.rs | 6 ++++-- src/phy/raw_socket.rs | 6 ++++-- src/phy/tracer.rs | 6 ++++-- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/phy/fault_injector.rs b/src/phy/fault_injector.rs index 39ad0882c..1f30db549 100644 --- a/src/phy/fault_injector.rs +++ b/src/phy/fault_injector.rs @@ -196,10 +196,12 @@ impl FaultInjector { } impl Device for FaultInjector { - type RxToken<'a> = RxToken<'a> + type RxToken<'a> + = RxToken<'a> where Self: 'a; - type TxToken<'a> = TxToken<'a, D::TxToken<'a>> + type TxToken<'a> + = TxToken<'a, D::TxToken<'a>> where Self: 'a; diff --git a/src/phy/fuzz_injector.rs b/src/phy/fuzz_injector.rs index dbb9fe68b..5598488a0 100644 --- a/src/phy/fuzz_injector.rs +++ b/src/phy/fuzz_injector.rs @@ -47,10 +47,12 @@ where FTx: Fuzzer, FRx: Fuzzer, { - type RxToken<'a> = RxToken<'a, D::RxToken<'a>, FRx> + type RxToken<'a> + = RxToken<'a, D::RxToken<'a>, FRx> where Self: 'a; - type TxToken<'a> = TxToken<'a, D::TxToken<'a>, FTx> + type TxToken<'a> + = TxToken<'a, D::TxToken<'a>, FTx> where Self: 'a; diff --git a/src/phy/pcap_writer.rs b/src/phy/pcap_writer.rs index 671e89780..2ce23ff3f 100644 --- a/src/phy/pcap_writer.rs +++ b/src/phy/pcap_writer.rs @@ -165,10 +165,12 @@ impl Device for PcapWriter where S: PcapSink, { - type RxToken<'a> = RxToken<'a, D::RxToken<'a>, S> + type RxToken<'a> + = RxToken<'a, D::RxToken<'a>, S> where Self: 'a; - type TxToken<'a> = TxToken<'a, D::TxToken<'a>, S> + type TxToken<'a> + = TxToken<'a, D::TxToken<'a>, S> where Self: 'a; diff --git a/src/phy/raw_socket.rs b/src/phy/raw_socket.rs index 0cb7752d9..3c1ad1262 100644 --- a/src/phy/raw_socket.rs +++ b/src/phy/raw_socket.rs @@ -59,10 +59,12 @@ impl RawSocket { } impl Device for RawSocket { - type RxToken<'a> = RxToken + type RxToken<'a> + = RxToken where Self: 'a; - type TxToken<'a> = TxToken + type TxToken<'a> + = TxToken where Self: 'a; diff --git a/src/phy/tracer.rs b/src/phy/tracer.rs index 999a9523b..eb51fcc95 100644 --- a/src/phy/tracer.rs +++ b/src/phy/tracer.rs @@ -42,10 +42,12 @@ impl Tracer { } impl Device for Tracer { - type RxToken<'a> = RxToken> + type RxToken<'a> + = RxToken> where Self: 'a; - type TxToken<'a> = TxToken> + type TxToken<'a> + = TxToken> where Self: 'a; From 13774ff28d9420af621ef77d20e8b64813239cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20B=C3=B6rjesson?= Date: Sun, 15 Dec 2024 00:00:30 +0100 Subject: [PATCH 16/21] Restart retransmit timer on new data ACK --- src/socket/tcp.rs | 66 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index d50631de0..31e544dc7 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -337,19 +337,12 @@ impl Timer { fn set_for_retransmit(&mut self, timestamp: Instant, delay: Duration) { match *self { - Timer::Idle { .. } | Timer::FastRetransmit { .. } => { + Timer::Idle { .. } | Timer::FastRetransmit { .. } | Timer::Retransmit { .. } => { *self = Timer::Retransmit { expires_at: timestamp + delay, delay, } } - Timer::Retransmit { expires_at, delay } if timestamp >= expires_at => { - *self = Timer::Retransmit { - expires_at: timestamp + delay, - delay: delay * 2, - } - } - Timer::Retransmit { .. } => (), Timer::Close { .. } => (), } } @@ -1842,11 +1835,14 @@ impl<'a> Socket<'a> { self.timer.set_for_idle(cx.now(), self.keep_alive); } - // ACK packets in ESTABLISHED state reset the retransmit timer, - // except for duplicate ACK packets which preserve it. + // RFC 6298: (5.2) ACK of all outstanding data turn off the retransmit timer. + // (5.3) ACK of new data in ESTABLISHED state restart the retransmit timer. (State::Established, TcpControl::None) => { - if !self.timer.is_retransmit() || ack_all { + if ack_all { self.timer.set_for_idle(cx.now(), self.keep_alive); + } else if ack_len > 0 { + self.timer + .set_for_retransmit(cx.now(), self.rtte.retransmission_timeout()); } } @@ -2528,9 +2524,10 @@ impl<'a> Socket<'a> { .post_transmit(cx.now(), repr.segment_len()); } - if !self.seq_to_transmit(cx) && repr.segment_len() > 0 { - // If we've transmitted all data we could (and there was something at all, - // data or flag, to transmit, not just an ACK), wind up the retransmit timer. + if !self.seq_to_transmit(cx) && repr.segment_len() > 0 && !self.timer.is_retransmit() { + // RFC 6298: (5.1) If we've transmitted all data we could (and there was + // something at all, data or flag, to transmit, not just an ACK), start the + // retransmit timer if it is not already running. self.timer .set_for_retransmit(cx.now(), self.rtte.retransmission_timeout()); } @@ -5678,6 +5675,45 @@ mod test { recv_nothing!(s, time 1550); } + #[test] + fn test_retransmit_timer_restart_on_partial_ack() { + let mut s = socket_established(); + s.remote_mss = 6; + s.send_slice(b"abcdef012345").unwrap(); + + recv!(s, time 0, Ok(TcpRepr { + control: TcpControl::None, + seq_number: LOCAL_SEQ + 1, + ack_number: Some(REMOTE_SEQ + 1), + payload: &b"abcdef"[..], + ..RECV_TEMPL + }), exact); + recv!(s, time 0, Ok(TcpRepr { + control: TcpControl::Psh, + seq_number: LOCAL_SEQ + 1 + 6, + ack_number: Some(REMOTE_SEQ + 1), + payload: &b"012345"[..], + ..RECV_TEMPL + }), exact); + // Acknowledge the first packet + send!(s, time 600, TcpRepr { + seq_number: REMOTE_SEQ + 1, + ack_number: Some(LOCAL_SEQ + 1 + 6), + window_len: 6, + ..SEND_TEMPL + }); + // The ACK of the first packet should restart the retransmit timer and delay a retransmission. + recv_nothing!(s, time 1500); + // The second packet should be re-sent. + recv!(s, time 1600, Ok(TcpRepr { + control: TcpControl::Psh, + seq_number: LOCAL_SEQ + 1 + 6, + ack_number: Some(REMOTE_SEQ + 1), + payload: &b"012345"[..], + ..RECV_TEMPL + }), exact); + } + #[test] fn test_data_retransmit_bursts_half_ack_close() { let mut s = socket_established(); @@ -7794,7 +7830,7 @@ mod test { assert_eq!(r.should_retransmit(Instant::from_millis(1200)), None); assert_eq!( r.should_retransmit(Instant::from_millis(1301)), - Some(Duration::from_millis(300)) + Some(Duration::from_millis(200)) ); r.set_for_idle(Instant::from_millis(1301), None); assert_eq!(r.should_retransmit(Instant::from_millis(1350)), None); From 7736d4006752d6b5e23d54bb56ac9ce89e46d00a Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Fri, 27 Dec 2024 20:15:19 +0100 Subject: [PATCH 17/21] tcp: emsure line numbers in tests point to the actual failure. --- src/socket/tcp.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 31e544dc7..6c8f34727 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -2762,6 +2762,7 @@ mod test { } } + #[track_caller] fn recv(socket: &mut TestSocket, timestamp: Instant, mut f: F) where F: FnMut(Result), @@ -2787,6 +2788,7 @@ mod test { } } + #[track_caller] fn recv_nothing(socket: &mut TestSocket, timestamp: Instant) { socket.cx.set_now(timestamp); @@ -2799,6 +2801,7 @@ mod test { assert_eq!(result, Ok(())) } + #[collapse_debuginfo(yes)] macro_rules! send { ($socket:ident, $repr:expr) => (send!($socket, time 0, $repr)); @@ -2810,6 +2813,7 @@ mod test { (assert_eq!(send(&mut $socket, Instant::from_millis($time), &$repr), $result)); } + #[collapse_debuginfo(yes)] macro_rules! recv { ($socket:ident, [$( $repr:expr ),*]) => ({ $( recv!($socket, Ok($repr)); )* @@ -2830,11 +2834,13 @@ mod test { (recv(&mut $socket, Instant::from_millis($time), |repr| assert_eq!(repr, $result))); } + #[collapse_debuginfo(yes)] macro_rules! recv_nothing { ($socket:ident) => (recv_nothing!($socket, time 0)); ($socket:ident, time $time:expr) => (recv_nothing(&mut $socket, Instant::from_millis($time))); } + #[collapse_debuginfo(yes)] macro_rules! sanity { ($socket1:expr, $socket2:expr) => {{ let (s1, s2) = ($socket1, $socket2); From 9b9f60b5ba63ef614e5be0704d7dd77f871fa3ee Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Fri, 27 Dec 2024 20:18:44 +0100 Subject: [PATCH 18/21] tcp: fix retransmit exponential backoff, align to rfc6298. --- src/socket/tcp.rs | 216 ++++++++++++++++++++++++---------------------- 1 file changed, 112 insertions(+), 104 deletions(-) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 6c8f34727..19f18d635 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -141,23 +141,38 @@ impl fmt::Display for State { } } -// Conservative initial RTT estimate. -const RTTE_INITIAL_RTT: u32 = 300; -const RTTE_INITIAL_DEV: u32 = 100; +/// RFC 6298: (2.1) Until a round-trip time (RTT) measurement has been made for a +/// segment sent between the sender and receiver, the sender SHOULD +/// set RTO <- 1 second, +const RTTE_INITIAL_RTO: u32 = 1000; // Minimum "safety margin" for the RTO that kicks in when the // variance gets very low. const RTTE_MIN_MARGIN: u32 = 5; -const RTTE_MIN_RTO: u32 = 10; -const RTTE_MAX_RTO: u32 = 10000; +/// K, according to RFC 6298 +const RTTE_K: u32 = 4; + +// RFC 6298 (2.4): Whenever RTO is computed, if it is less than 1 second, then the +// RTO SHOULD be rounded up to 1 second. +const RTTE_MIN_RTO: u32 = 1000; + +// RFC 6298 (2.5) A maximum value MAY be placed on RTO provided it is at least 60 +// seconds +const RTTE_MAX_RTO: u32 = 60_000; #[derive(Debug, Clone, Copy)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] struct RttEstimator { + /// true if we have made at least one rtt measurement. + have_measurement: bool, // Using u32 instead of Duration to save space (Duration is i64) - rtt: u32, - deviation: u32, + /// Smoothed RTT + srtt: u32, + /// RTT variance. + rttvar: u32, + /// Retransmission Time-Out + rto: u32, timestamp: Option<(Instant, TcpSeqNumber)>, max_seq_sent: Option, rto_count: u8, @@ -166,8 +181,10 @@ struct RttEstimator { impl Default for RttEstimator { fn default() -> Self { Self { - rtt: RTTE_INITIAL_RTT, - deviation: RTTE_INITIAL_DEV, + have_measurement: false, + srtt: 0, // ignored, will be overwritten on first measurement. + rttvar: 0, // ignored, will be overwritten on first measurement. + rto: RTTE_INITIAL_RTO, timestamp: None, max_seq_sent: None, rto_count: 0, @@ -177,26 +194,34 @@ impl Default for RttEstimator { impl RttEstimator { fn retransmission_timeout(&self) -> Duration { - let margin = RTTE_MIN_MARGIN.max(self.deviation * 4); - let ms = (self.rtt + margin).clamp(RTTE_MIN_RTO, RTTE_MAX_RTO); - Duration::from_millis(ms as u64) + Duration::from_millis(self.rto as _) } fn sample(&mut self, new_rtt: u32) { - // "Congestion Avoidance and Control", Van Jacobson, Michael J. Karels, 1988 - self.rtt = (self.rtt * 7 + new_rtt + 7) / 8; - let diff = (self.rtt as i32 - new_rtt as i32).unsigned_abs(); - self.deviation = (self.deviation * 3 + diff + 3) / 4; + if self.have_measurement { + // RFC 6298 (2.3) When a subsequent RTT measurement R' is made, a host MUST set (...) + let diff = (self.srtt as i32 - new_rtt as i32).unsigned_abs(); + self.rttvar = (self.rttvar * 3 + diff).div_ceil(4); + self.srtt = (self.srtt * 7 + new_rtt).div_ceil(8); + } else { + // RFC 6298 (2.2) When the first RTT measurement R is made, the host MUST set (...) + self.have_measurement = true; + self.srtt = new_rtt; + self.rttvar = new_rtt / 2; + } + + // RFC 6298 (2.2), (2.3) + let margin = RTTE_MIN_MARGIN.max(self.rttvar * RTTE_K); + self.rto = (self.srtt + margin).clamp(RTTE_MIN_RTO, RTTE_MAX_RTO); self.rto_count = 0; - let rto = self.retransmission_timeout().total_millis(); tcp_trace!( - "rtte: sample={:?} rtt={:?} dev={:?} rto={:?}", + "rtte: sample={:?} srtt={:?} rttvar={:?} rto={:?}", new_rtt, - self.rtt, - self.deviation, - rto + self.srtt, + self.rttvar, + self.rto ); } @@ -228,23 +253,23 @@ impl RttEstimator { tcp_trace!("rtte: abort sampling due to retransmit"); } self.timestamp = None; - self.rto_count = self.rto_count.saturating_add(1); + + // RFC 6298 (5.5) The host MUST set RTO <- RTO * 2 ("back off the timer"). The + // maximum value discussed in (2.5) above may be used to provide + // an upper bound to this doubling operation. + self.rto = (self.rto * 2).min(RTTE_MAX_RTO); + tcp_trace!("rtte: doubling rto to {:?}", self.rto); + + // RFC 6298: a TCP implementation MAY clear SRTT and RTTVAR after + // backing off the timer multiple times as it is likely that the current + // SRTT and RTTVAR are bogus in this situation. Once SRTT and RTTVAR + // are cleared, they should be initialized with the next RTT sample + // taken per (2.2) rather than using (2.3). + self.rto_count += 1; if self.rto_count >= 3 { - // This happens in 2 scenarios: - // - The RTT is higher than the initial estimate - // - The network conditions change, suddenly making the RTT much higher - // In these cases, the estimator can get stuck, because it can't sample because - // all packets sent would incur a retransmit. To avoid this, force an estimate - // increase if we see 3 consecutive retransmissions without any successful sample. self.rto_count = 0; - self.rtt = RTTE_MAX_RTO.min(self.rtt * 2); - let rto = self.retransmission_timeout().total_millis(); - tcp_trace!( - "rtte: too many retransmissions, increasing: rtt={:?} dev={:?} rto={:?}", - self.rtt, - self.deviation, - rto - ); + self.have_measurement = false; + tcp_trace!("rtte: too many retransmissions, clearing srtt, rttvar."); } } } @@ -252,17 +277,10 @@ impl RttEstimator { #[derive(Debug, Clone, Copy, PartialEq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] enum Timer { - Idle { - keep_alive_at: Option, - }, - Retransmit { - expires_at: Instant, - delay: Duration, - }, + Idle { keep_alive_at: Option }, + Retransmit { expires_at: Instant }, FastRetransmit, - Close { - expires_at: Instant, - }, + Close { expires_at: Instant }, } const ACK_DELAY_DEFAULT: Duration = Duration::from_millis(10); @@ -284,13 +302,11 @@ impl Timer { } } - fn should_retransmit(&self, timestamp: Instant) -> Option { + fn should_retransmit(&self, timestamp: Instant) -> bool { match *self { - Timer::Retransmit { expires_at, delay } if timestamp >= expires_at => { - Some(timestamp - expires_at + delay) - } - Timer::FastRetransmit => Some(Duration::from_millis(0)), - _ => None, + Timer::Retransmit { expires_at } if timestamp >= expires_at => true, + Timer::FastRetransmit => true, + _ => false, } } @@ -340,7 +356,6 @@ impl Timer { Timer::Idle { .. } | Timer::FastRetransmit { .. } | Timer::Retransmit { .. } => { *self = Timer::Retransmit { expires_at: timestamp + delay, - delay, } } Timer::Close { .. } => (), @@ -2271,30 +2286,28 @@ impl<'a> Socket<'a> { // If a timeout expires, we should abort the connection. net_debug!("timeout exceeded"); self.set_state(State::Closed); - } else if !self.seq_to_transmit(cx) { - if let Some(retransmit_delta) = self.timer.should_retransmit(cx.now()) { - // If a retransmit timer expired, we should resend data starting at the last ACK. - net_debug!("retransmitting at t+{}", retransmit_delta); - - // Rewind "last sequence number sent", as if we never - // had sent them. This will cause all data in the queue - // to be sent again. - self.remote_last_seq = self.local_seq_no; + } else if !self.seq_to_transmit(cx) && self.timer.should_retransmit(cx.now()) { + // If a retransmit timer expired, we should resend data starting at the last ACK. + net_debug!("retransmitting"); - // Clear the `should_retransmit` state. If we can't retransmit right - // now for whatever reason (like zero window), this avoids an - // infinite polling loop where `poll_at` returns `Now` but `dispatch` - // can't actually do anything. - self.timer.set_for_idle(cx.now(), self.keep_alive); + // Rewind "last sequence number sent", as if we never + // had sent them. This will cause all data in the queue + // to be sent again. + self.remote_last_seq = self.local_seq_no; - // Inform RTTE, so that it can avoid bogus measurements. - self.rtte.on_retransmit(); + // Clear the `should_retransmit` state. If we can't retransmit right + // now for whatever reason (like zero window), this avoids an + // infinite polling loop where `poll_at` returns `Now` but `dispatch` + // can't actually do anything. + self.timer.set_for_idle(cx.now(), self.keep_alive); - // Inform the congestion controller that we're retransmitting. - self.congestion_controller - .inner_mut() - .on_retransmit(cx.now()); - } + // Inform RTTE, so that it can avoid bogus measurements. + self.rtte.on_retransmit(); + + // Inform the congestion controller that we're retransmitting. + self.congestion_controller + .inner_mut() + .on_retransmit(cx.now()); } // Decide whether we're sending a packet. @@ -2735,6 +2748,7 @@ mod test { } } + #[track_caller] fn send( socket: &mut TestSocket, timestamp: Instant, @@ -5709,9 +5723,9 @@ mod test { ..SEND_TEMPL }); // The ACK of the first packet should restart the retransmit timer and delay a retransmission. - recv_nothing!(s, time 1500); + recv_nothing!(s, time 2399); // The second packet should be re-sent. - recv!(s, time 1600, Ok(TcpRepr { + recv!(s, time 2400, Ok(TcpRepr { control: TcpControl::Psh, seq_number: LOCAL_SEQ + 1 + 6, ack_number: Some(REMOTE_SEQ + 1), @@ -5770,7 +5784,7 @@ mod test { max_seg_size: Some(BASE_MSS), ..RECV_TEMPL })); - recv!(s, time 750, Ok(TcpRepr { // retransmit + recv!(s, time 1050, Ok(TcpRepr { // retransmit control: TcpControl::Syn, seq_number: LOCAL_SEQ, ack_number: Some(REMOTE_SEQ + 1), @@ -5891,18 +5905,18 @@ mod test { payload: &b"ABCDEF"[..], ..RECV_TEMPL })); // also dropped - recv!(s, time 2000, Ok(TcpRepr { + recv!(s, time 3000, Ok(TcpRepr { seq_number: LOCAL_SEQ + 1, ack_number: Some(REMOTE_SEQ + 1), payload: &b"abcdef"[..], ..RECV_TEMPL })); // retransmission - send!(s, time 2005, TcpRepr { + send!(s, time 3005, TcpRepr { seq_number: REMOTE_SEQ + 1, ack_number: Some(LOCAL_SEQ + 1 + 6 + 6), ..SEND_TEMPL }); // acknowledgement of both segments - recv!(s, time 2010, Ok(TcpRepr { + recv!(s, time 3010, Ok(TcpRepr { seq_number: LOCAL_SEQ + 1 + 6 + 6, ack_number: Some(REMOTE_SEQ + 1), payload: &b"ABCDEF"[..], @@ -6908,11 +6922,11 @@ mod test { #[test] fn test_established_timeout() { let mut s = socket_established(); - s.set_timeout(Some(Duration::from_millis(1000))); + s.set_timeout(Some(Duration::from_millis(2000))); recv_nothing!(s, time 250); assert_eq!( s.socket.poll_at(&mut s.cx), - PollAt::Time(Instant::from_millis(1250)) + PollAt::Time(Instant::from_millis(2250)) ); s.send_slice(b"abcdef").unwrap(); assert_eq!(s.socket.poll_at(&mut s.cx), PollAt::Now); @@ -6924,9 +6938,9 @@ mod test { })); assert_eq!( s.socket.poll_at(&mut s.cx), - PollAt::Time(Instant::from_millis(955)) + PollAt::Time(Instant::from_millis(1255)) ); - recv!(s, time 955, Ok(TcpRepr { + recv!(s, time 1255, Ok(TcpRepr { seq_number: LOCAL_SEQ + 1, ack_number: Some(REMOTE_SEQ + 1), payload: &b"abcdef"[..], @@ -6934,9 +6948,9 @@ mod test { })); assert_eq!( s.socket.poll_at(&mut s.cx), - PollAt::Time(Instant::from_millis(1255)) + PollAt::Time(Instant::from_millis(2255)) ); - recv!(s, time 1255, Ok(TcpRepr { + recv!(s, time 2255, Ok(TcpRepr { control: TcpControl::Rst, seq_number: LOCAL_SEQ + 1 + 6, ack_number: Some(REMOTE_SEQ + 1), @@ -7822,24 +7836,18 @@ mod test { fn test_timer_retransmit() { const RTO: Duration = Duration::from_millis(100); let mut r = Timer::new(); - assert_eq!(r.should_retransmit(Instant::from_secs(1)), None); + assert!(!r.should_retransmit(Instant::from_secs(1))); r.set_for_retransmit(Instant::from_millis(1000), RTO); - assert_eq!(r.should_retransmit(Instant::from_millis(1000)), None); - assert_eq!(r.should_retransmit(Instant::from_millis(1050)), None); - assert_eq!( - r.should_retransmit(Instant::from_millis(1101)), - Some(Duration::from_millis(101)) - ); + assert!(!r.should_retransmit(Instant::from_millis(1000))); + assert!(!r.should_retransmit(Instant::from_millis(1050))); + assert!(r.should_retransmit(Instant::from_millis(1101))); r.set_for_retransmit(Instant::from_millis(1101), RTO); - assert_eq!(r.should_retransmit(Instant::from_millis(1101)), None); - assert_eq!(r.should_retransmit(Instant::from_millis(1150)), None); - assert_eq!(r.should_retransmit(Instant::from_millis(1200)), None); - assert_eq!( - r.should_retransmit(Instant::from_millis(1301)), - Some(Duration::from_millis(200)) - ); + assert!(!r.should_retransmit(Instant::from_millis(1101))); + assert!(!r.should_retransmit(Instant::from_millis(1150))); + assert!(!r.should_retransmit(Instant::from_millis(1200))); + assert!(r.should_retransmit(Instant::from_millis(1301))); r.set_for_idle(Instant::from_millis(1301), None); - assert_eq!(r.should_retransmit(Instant::from_millis(1350)), None); + assert!(!r.should_retransmit(Instant::from_millis(1350))); } #[test] @@ -7847,12 +7855,12 @@ mod test { let mut r = RttEstimator::default(); let rtos = &[ - 751, 766, 755, 731, 697, 656, 613, 567, 523, 484, 445, 411, 378, 350, 322, 299, 280, - 261, 243, 229, 215, 206, 197, 188, + 6000, 5000, 4252, 3692, 3272, 2956, 2720, 2540, 2408, 2308, 2232, 2176, 2132, 2100, + 2076, 2060, 2048, 2036, 2028, 2024, 2020, 2016, 2012, 2012, ]; for &rto in rtos { - r.sample(100); + r.sample(2000); assert_eq!(r.retransmission_timeout(), Duration::from_millis(rto)); } } From e82698919340a99059c5b667679f11c7c5970a66 Mon Sep 17 00:00:00 2001 From: tomDev5 Date: Fri, 27 Dec 2024 20:19:59 +0100 Subject: [PATCH 19/21] tcp: add retransmission exponential backoff test. --- src/socket/tcp.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index 19f18d635..42f99175f 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -6411,6 +6411,38 @@ mod test { recv_nothing!(s); } + #[test] + fn test_retransmit_exponential_backoff() { + let mut s = socket_established(); + s.send_slice(b"abcdef").unwrap(); + recv!(s, time 0, Ok(TcpRepr { + seq_number: LOCAL_SEQ + 1, + ack_number: Some(REMOTE_SEQ + 1), + payload: &b"abcdef"[..], + ..RECV_TEMPL + })); + + let expected_retransmission_instant = s.rtte.retransmission_timeout().total_millis() as i64; + recv_nothing!(s, time expected_retransmission_instant - 1); + recv!(s, time expected_retransmission_instant, Ok(TcpRepr { + seq_number: LOCAL_SEQ + 1, + ack_number: Some(REMOTE_SEQ + 1), + payload: &b"abcdef"[..], + ..RECV_TEMPL + })); + + // "current time" is expected_retransmission_instant, and we want to wait 2 * retransmission timeout + let expected_retransmission_instant = 3 * expected_retransmission_instant; + + recv_nothing!(s, time expected_retransmission_instant - 1); + recv!(s, time expected_retransmission_instant, Ok(TcpRepr { + seq_number: LOCAL_SEQ + 1, + ack_number: Some(REMOTE_SEQ + 1), + payload: &b"abcdef"[..], + ..RECV_TEMPL + })); + } + // =========================================================================================// // Tests for window management. // =========================================================================================// From 31fda8f7cd111d2e1b1b87d694442aa62eb82a82 Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Fri, 3 Jan 2025 11:43:31 +0100 Subject: [PATCH 20/21] fix lladdr parse panic The `RawHardwareAddress::parse()` method panics if the length of the address is invalid. More specific, for an Ethernet address, the length should exactly be 6 bytes, and for an IEEE 802.15.4 address, the length should exactly be 8 bytes. Previously, we only checked if the size of the input was at least the size of the link layer address. Since `Ethernet::from_bytes()` does a copy_from_slice, the length of the input should be exactly 6 bytes. A panic can be triggered when the length is for example 7 bytes, while the `medium-ethernet` and `medium-ieee802154` features are enabled. This commit fixes the panic by checking if the length of the input is exactly the size of the link layer address. This panic was discovered by people from Radically Open Security. Signed-off-by: Thibaut Vandervelden --- src/wire/mod.rs | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/wire/mod.rs b/src/wire/mod.rs index a6db11c3d..478f0cffc 100644 --- a/src/wire/mod.rs +++ b/src/wire/mod.rs @@ -478,6 +478,10 @@ pub struct RawHardwareAddress { #[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))] impl RawHardwareAddress { + /// Create a new `RawHardwareAddress` from a byte slice. + /// + /// # Panics + /// Panics if `addr.len() > MAX_HARDWARE_ADDRESS_LEN`. pub fn from_bytes(addr: &[u8]) -> Self { let mut data = [0u8; MAX_HARDWARE_ADDRESS_LEN]; data[..addr.len()].copy_from_slice(addr); @@ -504,7 +508,7 @@ impl RawHardwareAddress { match medium { #[cfg(feature = "medium-ethernet")] Medium::Ethernet => { - if self.len() < 6 { + if self.len() != 6 { return Err(Error); } Ok(HardwareAddress::Ethernet(EthernetAddress::from_bytes( @@ -513,7 +517,7 @@ impl RawHardwareAddress { } #[cfg(feature = "medium-ieee802154")] Medium::Ieee802154 => { - if self.len() < 8 { + if self.len() != 8 { return Err(Error); } Ok(HardwareAddress::Ieee802154(Ieee802154Address::from_bytes( @@ -559,3 +563,37 @@ impl From for RawHardwareAddress { Self::from_bytes(addr.as_bytes()) } } + +#[cfg(test)] +mod tests { + use super::*; + use rstest::rstest; + + #[rstest] + #[cfg(feature = "medium-ethernet")] + #[case((Medium::Ethernet, &[0u8; 6][..]), Ok(HardwareAddress::Ethernet(EthernetAddress([0, 0, 0, 0, 0, 0]))))] + #[cfg(feature = "medium-ethernet")] + #[case((Medium::Ethernet, &[1u8; 5][..]), Err(Error))] + #[cfg(feature = "medium-ethernet")] + #[case((Medium::Ethernet, &[1u8; 7][..]), Err(Error))] + #[cfg(feature = "medium-ieee802154")] + #[case((Medium::Ieee802154, &[0u8; 8][..]), Ok(HardwareAddress::Ieee802154(Ieee802154Address::Extended([0, 0, 0, 0, 0, 0, 0, 0]))))] + #[cfg(feature = "medium-ieee802154")] + #[case((Medium::Ieee802154, &[1u8; 2][..]), Err(Error))] + #[cfg(feature = "medium-ieee802154")] + #[case((Medium::Ieee802154, &[1u8; 1][..]), Err(Error))] + fn parse_hardware_address( + #[case] input: (Medium, &[u8]), + #[case] expected: Result, + ) { + let (medium, input) = input; + + // NOTE: we check the length since `RawHardwareAddress::parse()` panics if the length is + // invalid. MAX_HARDWARE_ADDRESS_LEN is based on the medium, and depending on the feature + // flags, it can be different. + if input.len() < MAX_HARDWARE_ADDRESS_LEN { + let raw = RawHardwareAddress::from_bytes(input); + assert_eq!(raw.parse(medium), expected); + } + } +} From e77d02bab7bdc27fb16a88e4d25f507cdaff2b64 Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Fri, 3 Jan 2025 14:37:02 +0100 Subject: [PATCH 21/21] fix DHCPv4 panic when T1 < T2 < lease duration is not respected When receiving T1 and T2 values from the DHCP server, they must be in the order T1 < T2 < lease duration. If this order is not respected, the defaults should be used. This patch adds a check to ensure that the order is respected and uses the defaults otherwise. Signed-off-by: Thibaut Vandervelden --- src/socket/dhcpv4.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/socket/dhcpv4.rs b/src/socket/dhcpv4.rs index 72980a946..7d8c920ad 100644 --- a/src/socket/dhcpv4.rs +++ b/src/socket/dhcpv4.rs @@ -502,6 +502,8 @@ impl<'a> Socket<'a> { // Times T1 and T2 are configurable by the server through // options. T1 defaults to (0.5 * duration_of_lease). T2 // defaults to (0.875 * duration_of_lease). + // When receiving T1 and T2, they must be in the order: + // T1 < T2 < lease_duration let (renew_duration, rebind_duration) = match ( dhcp_repr .renew_duration @@ -510,8 +512,11 @@ impl<'a> Socket<'a> { .rebind_duration .map(|d| Duration::from_secs(d as u64)), ) { - (Some(renew_duration), Some(rebind_duration)) => (renew_duration, rebind_duration), - (None, None) => (lease_duration / 2, lease_duration * 7 / 8), + (Some(renew_duration), Some(rebind_duration)) + if renew_duration < rebind_duration && rebind_duration < lease_duration => + { + (renew_duration, rebind_duration) + } // RFC 2131 does not say what to do if only one value is // provided, so: @@ -519,7 +524,7 @@ impl<'a> Socket<'a> { // between T1 and the duration of the lease. If T1 is set to // the default (0.5 * duration_of_lease), then T2 will also // be set to the default (0.875 * duration_of_lease). - (Some(renew_duration), None) => ( + (Some(renew_duration), None) if renew_duration < lease_duration => ( renew_duration, renew_duration + (lease_duration - renew_duration) * 3 / 4, ), @@ -527,9 +532,16 @@ impl<'a> Socket<'a> { // If only T2 is provided, then T1 will be set to be // whichever is smaller of the default (0.5 * // duration_of_lease) or T2. - (None, Some(rebind_duration)) => { + (None, Some(rebind_duration)) if rebind_duration < lease_duration => { ((lease_duration / 2).min(rebind_duration), rebind_duration) } + + // Use the defaults if the following order is not met: + // T1 < T2 < lease_duration + (_, _) => { + net_debug!("using default T1 and T2 values since the provided values are invalid"); + (lease_duration / 2, lease_duration * 7 / 8) + } }; let renew_at = now + renew_duration; let rebind_at = now + rebind_duration;