Skip to content

Commit 7278847

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 <[email protected]> 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] <[email protected]> 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 7278847

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: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,3 +238,23 @@ impl EcnCodepoint {
238238
})
239239
}
240240
}
241+
242+
#[cfg(target_os = "linux")]
243+
#[derive(Clone, Debug, Copy)]
244+
pub struct IcmpError {
245+
pub dst: SocketAddr,
246+
pub kind: IcmpErrorKind,
247+
}
248+
249+
#[cfg(not(target_os = "linux"))]
250+
pub struct IcmpError;
251+
252+
#[cfg(target_os = "linux")]
253+
#[derive(Clone, Debug, Copy, PartialEq, Eq)]
254+
pub enum IcmpErrorKind {
255+
NetworkUnreachable,
256+
HostUnreachable,
257+
PortUnreachable,
258+
PacketTooBig,
259+
Other { icmp_type: u8, icmp_code: u8 },
260+
}

quinn-udp/src/unix.rs

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

1515
use socket2::SockRef;
1616

17+
use crate::IcmpError;
18+
19+
#[cfg(target_os = "linux")]
20+
use crate::IcmpErrorKind;
21+
1722
use super::{
1823
EcnCodepoint, IO_ERROR_LOG_INTERVAL, RecvMeta, Transmit, UdpSockRef, cmsg, log_sendmsg_error,
1924
};
@@ -33,6 +38,66 @@ pub(crate) struct msghdr_x {
3338
pub msg_datalen: usize,
3439
}
3540

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

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

315+
#[cfg(target_os = "linux")]
316+
pub fn recv_icmp_err(&self, socket: UdpSockRef<'_>) -> io::Result<Option<IcmpError>> {
317+
recv_err(socket.0)
318+
}
319+
320+
#[cfg(not(target_os = "linux"))]
321+
pub fn recv_icmp_err(&self, socket: UdpSockRef<'_>) -> io::Result<Option<IcmpError>> {
322+
recv_err(socket.0)
323+
}
324+
236325
/// The maximum amount of segments which can be transmitted if a platform
237326
/// supports Generic Send Offload (GSO).
238327
///
@@ -500,6 +589,7 @@ fn recv(io: SockRef<'_>, bufs: &mut [IoSliceMut<'_>], meta: &mut [RecvMeta]) ->
500589
for i in 0..(msg_count as usize) {
501590
meta[i] = decode_recv(&names[i], &hdrs[i].msg_hdr, hdrs[i].msg_len as usize)?;
502591
}
592+
503593
Ok(msg_count as usize)
504594
}
505595

@@ -795,6 +885,99 @@ fn decode_recv(
795885
})
796886
}
797887

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

0 commit comments

Comments
 (0)