Skip to content

Commit fd00c2e

Browse files
committed
feat(ip_recverr): Add ICMP error struct, start with linux
chore(ip_recverr): Testing with socket setup similar to cmsg in recv feat(ip_recverr): Initialised socket options for IPV4 and V6 feat(ip_recverr): Add error read queue function chore(ip_recverr): Add error handling after recieving normal packets test(ip_recverr): Start the test cases for IP_RECVERR test(ip_recverr): Complete the test cases for ip_recverr and ipv6 test(ip_recverr): handle expected send failure instead of panicking style: Format the files using rustfmt style(recv_err): Change the name of the unused function refactor(ip_recverr): change the ICMPErr to enum kind refactor(ip_recverr): COnversion from SockExtendedErr to ICMPErrKind refactor(ip_recverr): Remove ICMPErr from recv and add seperate recv_icmp_err test(ip_recverr): Change the test files following new method Apply suggestions from code review Co-authored-by: Thomas Eizinger <thomas@eizinger.io> refactor: address code review feedback - Use early returns to reduce nesting - Replace match with unwrap in tests - Add non-Linux fallback implementation fix(ip_recverr): Continue to next control message instead of return test(ip_recverr): Fix the socket binding error from OS, now sends proper network error fix(ip_recverr): rename IcmpError for non linux platforms fix(pacing): allow ±0ns tolerance in computes_pause_correctly test on i386 -Used Duration::abs_diff() instead of manual difference for cleaner and more robust duration comparison. -added inline formatting as required. build(deps): bump rustls-platform-verifier from 0.6.1 to 0.6.2 Bumps [rustls-platform-verifier](https://github.com/rustls/rustls-platform-verifier) from 0.6.1 to 0.6.2. - [Release notes](https://github.com/rustls/rustls-platform-verifier/releases) - [Changelog](https://github.com/rustls/rustls-platform-verifier/blob/main/CHANGELOG) - [Commits](rustls/rustls-platform-verifier@v/0.6.1...v/0.6.2) --- updated-dependencies: - dependency-name: rustls-platform-verifier dependency-version: 0.6.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> refactor(ip_recverr): Change SockExtendedError as suggested fix(ip_recverr): Add IcmpError for non linux platforms refactor(ip_recverr): Change to SockExtendedError Kind
1 parent fc5e219 commit fd00c2e

File tree

5 files changed

+308
-16
lines changed

5 files changed

+308
-16
lines changed

Cargo.lock

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

quinn-proto/src/connection/pacing.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,14 +273,18 @@ mod tests {
273273

274274
let pace_duration = Duration::from_nanos((TARGET_BURST_INTERVAL.as_nanos() * 4 / 5) as u64);
275275

276-
assert_eq!(
277-
pacer
278-
.delay(rtt, mtu as u64, mtu, window, old_instant)
279-
.expect("Send must be delayed")
280-
.duration_since(old_instant),
281-
pace_duration
282-
);
276+
let actual_delay = pacer
277+
.delay(rtt, mtu as u64, mtu, window, old_instant)
278+
.expect("Send must be delayed")
279+
.duration_since(old_instant);
280+
281+
let diff = actual_delay.abs_diff(pace_duration);
283282

283+
// Allow up to 2ns difference due to rounding
284+
assert!(
285+
diff < Duration::from_nanos(2),
286+
"expected ≈ {pace_duration:?}, got {actual_delay:?} (diff {diff:?})"
287+
);
284288
// Refill half of the tokens
285289
assert_eq!(
286290
pacer.delay(

quinn-udp/src/lib.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ mod log {
8080
#[cfg(not(wasm_browser))]
8181
pub use imp::UdpSocketState;
8282

83+
8384
/// Number of UDP packets to send/receive at a time
8485
#[cfg(not(wasm_browser))]
8586
pub const BATCH_SIZE: usize = imp::BATCH_SIZE;
@@ -238,3 +239,24 @@ impl EcnCodepoint {
238239
})
239240
}
240241
}
242+
243+
#[cfg(target_os = "linux")]
244+
#[derive(Clone, Debug, Copy)]
245+
pub struct IcmpError {
246+
pub dst: SocketAddr,
247+
pub kind: IcmpErrorKind,
248+
}
249+
250+
#[cfg(not(target_os = "linux"))]
251+
pub struct IcmpError;
252+
253+
#[cfg(target_os = "linux")]
254+
#[derive(Clone, Debug, Copy, PartialEq, Eq)]
255+
pub enum IcmpErrorKind {
256+
NetworkUnreachable,
257+
HostUnreachable,
258+
PortUnreachable,
259+
PacketTooBig,
260+
Other { icmp_type: u8, icmp_code: u8 },
261+
}
262+

quinn-udp/src/unix.rs

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ use std::{
1414

1515
use socket2::SockRef;
1616

17+
#[cfg(target_os = "linux")]
18+
use crate::{IcmpError, IcmpErrorKind};
19+
1720
use super::{
1821
EcnCodepoint, IO_ERROR_LOG_INTERVAL, RecvMeta, Transmit, UdpSockRef, cmsg, log_sendmsg_error,
1922
};
@@ -33,6 +36,66 @@ pub(crate) struct msghdr_x {
3336
pub msg_datalen: usize,
3437
}
3538

39+
#[cfg(target_os = "linux")]
40+
#[repr(C)]
41+
#[derive(Clone, Copy, Debug)]
42+
pub(crate) struct SockExtendedError {
43+
pub errno: u32,
44+
pub origin: u8,
45+
pub r#type: u8,
46+
pub code: u8,
47+
pub pad: u8,
48+
pub info: u32,
49+
pub data: u32,
50+
}
51+
52+
#[cfg(target_os = "linux")]
53+
impl SockExtendedError {
54+
fn kind(&self) -> IcmpErrorKind {
55+
const ICMP_DEST_UNREACH: u8 = 3; // Type 3: Destination Unreachable
56+
const ICMP_NET_UNREACH: u8 = 0;
57+
const ICMP_HOST_UNREACH: u8 = 1;
58+
const ICMP_PORT_UNREACH: u8 = 3;
59+
const ICMP_FRAG_NEEDED: u8 = 4;
60+
61+
const ICMPV6_DEST_UNREACH: u8 = 1; // Type 1: Destination Unreachable for IPv6
62+
const ICMPV6_NO_ROUTE: u8 = 0;
63+
const ICMPV6_ADDR_UNREACH: u8 = 1;
64+
const ICMPV6_PORT_UNREACH: u8 = 4;
65+
66+
const ICMPV6_PACKET_TOO_BIG: u8 = 2;
67+
68+
match (self.origin, self.r#type, self.code) {
69+
(libc::SO_EE_ORIGIN_ICMP, ICMP_DEST_UNREACH, ICMP_NET_UNREACH) => {
70+
IcmpErrorKind::NetworkUnreachable
71+
}
72+
(libc::SO_EE_ORIGIN_ICMP, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH) => {
73+
IcmpErrorKind::HostUnreachable
74+
}
75+
(libc::SO_EE_ORIGIN_ICMP, ICMP_DEST_UNREACH, ICMP_PORT_UNREACH) => {
76+
IcmpErrorKind::PortUnreachable
77+
}
78+
(libc::SO_EE_ORIGIN_ICMP, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED) => {
79+
IcmpErrorKind::PacketTooBig
80+
}
81+
(libc::SO_EE_ORIGIN_ICMP6, ICMPV6_DEST_UNREACH, ICMPV6_NO_ROUTE) => {
82+
IcmpErrorKind::NetworkUnreachable
83+
}
84+
(libc::SO_EE_ORIGIN_ICMP6, ICMPV6_DEST_UNREACH, ICMPV6_ADDR_UNREACH) => {
85+
IcmpErrorKind::HostUnreachable
86+
}
87+
(libc::SO_EE_ORIGIN_ICMP6, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH) => {
88+
IcmpErrorKind::PortUnreachable
89+
}
90+
(libc::SO_EE_ORIGIN_ICMP6, ICMPV6_PACKET_TOO_BIG, _) => IcmpErrorKind::PacketTooBig,
91+
_ => IcmpErrorKind::Other {
92+
icmp_type: self.r#type,
93+
icmp_code: self.code,
94+
},
95+
}
96+
}
97+
}
98+
3699
#[cfg(apple_fast)]
37100
extern "C" {
38101
fn recvmsg_x(
@@ -122,6 +185,20 @@ impl UdpSocketState {
122185
}
123186
}
124187

188+
// Enable IP_RECVERR and IPV6_RECVERR for ICMP Errors
189+
#[cfg(target_os = "linux")]
190+
if is_ipv4 {
191+
if let Err(_err) =
192+
set_socket_option(&*io, libc::IPPROTO_IP, libc::IP_RECVERR, OPTION_ON)
193+
{
194+
crate::log::debug!("Failed to enable IP_RECVERR: {}", _err);
195+
}
196+
} else if let Err(_err) =
197+
set_socket_option(&*io, libc::IPPROTO_IPV6, libc::IPV6_RECVERR, OPTION_ON)
198+
{
199+
crate::log::debug!("Failed to enable IPV6_RECVERR: {}", _err);
200+
}
201+
125202
let mut may_fragment = false;
126203
#[cfg(any(target_os = "linux", target_os = "android"))]
127204
{
@@ -233,6 +310,16 @@ impl UdpSocketState {
233310
recv(socket.0, bufs, meta)
234311
}
235312

313+
#[cfg(target_os = "linux")]
314+
pub fn recv_icmp_err(&self, socket: UdpSockRef<'_>) -> io::Result<Option<IcmpError>> {
315+
recv_err(socket.0)
316+
}
317+
318+
#[cfg(not(target_os = "linux"))]
319+
pub fn recv_icmp_err(&self, socket: UdpSockRef<'_>) -> io::Result<Option<IcmpError>> {
320+
recv_err(socket.0)
321+
}
322+
236323
/// The maximum amount of segments which can be transmitted if a platform
237324
/// supports Generic Send Offload (GSO).
238325
///
@@ -500,6 +587,7 @@ fn recv(io: SockRef<'_>, bufs: &mut [IoSliceMut<'_>], meta: &mut [RecvMeta]) ->
500587
for i in 0..(msg_count as usize) {
501588
meta[i] = decode_recv(&names[i], &hdrs[i].msg_hdr, hdrs[i].msg_len as usize)?;
502589
}
590+
503591
Ok(msg_count as usize)
504592
}
505593

@@ -795,6 +883,99 @@ fn decode_recv(
795883
})
796884
}
797885

886+
#[cfg(target_os = "linux")]
887+
fn recv_err(io: SockRef<'_>) -> io::Result<Option<IcmpError>> {
888+
use std::mem;
889+
890+
let fd = io.as_raw_fd();
891+
892+
let mut control = cmsg::Aligned([0u8; CMSG_LEN]);
893+
894+
// We don't need actual data, just the error info
895+
let mut iovec = libc::iovec {
896+
iov_base: std::ptr::null_mut(),
897+
iov_len: 0,
898+
};
899+
900+
let mut addr_storage: libc::sockaddr_storage = unsafe { mem::zeroed() };
901+
902+
// Have followed the previous declarations
903+
let mut hdr: libc::msghdr = unsafe { mem::zeroed() };
904+
hdr.msg_name = &mut addr_storage as *mut _ as *mut _;
905+
hdr.msg_namelen = mem::size_of::<libc::sockaddr_storage>() as libc::socklen_t;
906+
hdr.msg_iov = &mut iovec;
907+
hdr.msg_iovlen = 1;
908+
hdr.msg_control = control.0.as_mut_ptr() as *mut _;
909+
hdr.msg_controllen = CMSG_LEN as _;
910+
911+
let ret = unsafe { libc::recvmsg(fd, &mut hdr, libc::MSG_ERRQUEUE) };
912+
913+
if ret < 0 {
914+
let err = io::Error::last_os_error();
915+
// EAGAIN/EWOULDBLOCK means no error in queue - this is normal
916+
if err.kind() == io::ErrorKind::WouldBlock {
917+
return Ok(None);
918+
}
919+
return Err(err);
920+
}
921+
922+
let cmsg_iter = unsafe { cmsg::Iter::new(&hdr) };
923+
924+
for cmsg in cmsg_iter {
925+
const IP_RECVERR: libc::c_int = 11;
926+
const IPV6_RECVERR: libc::c_int = 25;
927+
928+
let is_ipv4_err = cmsg.cmsg_level == libc::IPPROTO_IP && cmsg.cmsg_type == IP_RECVERR;
929+
let is_ipv6_err = cmsg.cmsg_level == libc::IPPROTO_IPV6 && cmsg.cmsg_type == IPV6_RECVERR;
930+
931+
if !is_ipv4_err && !is_ipv6_err {
932+
continue;
933+
}
934+
935+
let err_data = unsafe { cmsg::decode::<SockExtendedError, libc::cmsghdr>(cmsg) };
936+
937+
let dst = unsafe {
938+
let addr_ptr = &addr_storage as *const _ as *const libc::sockaddr;
939+
match (*addr_ptr).sa_family as i32 {
940+
libc::AF_INET => {
941+
let addr_in = &*(addr_ptr as *const libc::sockaddr_in);
942+
SocketAddr::V4(std::net::SocketAddrV4::new(
943+
std::net::Ipv4Addr::from(u32::from_be(addr_in.sin_addr.s_addr)),
944+
u16::from_be(addr_in.sin_port),
945+
))
946+
}
947+
libc::AF_INET6 => {
948+
let addr_in6 = &*(addr_ptr as *const libc::sockaddr_in6);
949+
SocketAddr::V6(std::net::SocketAddrV6::new(
950+
std::net::Ipv6Addr::from(addr_in6.sin6_addr.s6_addr),
951+
u16::from_be(addr_in6.sin6_port),
952+
addr_in6.sin6_flowinfo,
953+
addr_in6.sin6_scope_id,
954+
))
955+
}
956+
_ => {
957+
crate::log::warn!(
958+
"Ignoring ICMP error with unknown address family: {}",
959+
addr_storage.ss_family
960+
);
961+
continue;
962+
}
963+
}
964+
};
965+
966+
return Ok(Some(IcmpError {
967+
dst,
968+
kind: err_data.kind(),
969+
}));
970+
}
971+
Ok(None)
972+
}
973+
974+
#[cfg(not(target_os = "linux"))]
975+
fn recv_err(_io: SockRef<'_>) -> io::Result<Option<IcmpError>> {
976+
Ok(None)
977+
}
978+
798979
#[cfg(not(apple_slow))]
799980
// Chosen somewhat arbitrarily; might benefit from additional tuning.
800981
pub(crate) const BATCH_SIZE: usize = 32;

0 commit comments

Comments
 (0)