feat(ip_recverr): ADD IP_RECVERR and IPV6_RECVERR support for linux #2421

RaghavendraRQ · 2025-10-21T10:11:47Z

On Linux, enables IP_RECVERR and IPV6_RECVERR socket options

This allows the socket to receive ICMP errors via the error queue, enabling faster detection of network unreachability instead of waiting for connection timeouts.

Features Implemented

Enable IP_RECVERR and IPV6_RECVERR socket options on Linux
Add error queue polling in recv path
Add IcmpError type for ICMP error reporting
Add tests for ICMP error handling (Not Completed)

Closes #2052

mxinden

Good work. I will take a more in-depth look.

quinn-udp/src/unix.rs

thomaseizinger

Gave this a first pass.

quinn-udp/src/lib.rs

quinn-udp/src/unix.rs

thomaseizinger · 2025-10-22T21:12:05Z

quinn-udp/src/unix.rs

+        if is_ip_err || is_ipv6_err {
+            let err_data = unsafe { cmsg::decode::<SockExtendedErr, libc::cmsghdr>(cmsg) };
+
+            let addr = unsafe {


Do we not have this code somewhere already in the codebase? Perhaps extracting a small utility function could make this a bit more concise.

I considered factoring it out, but this logic is fairly localized and only used here. so I’ll keep it inline for clarity.

quinn-udp/src/unix.rs

quinn-udp/tests/tests.rs

RaghavendraRQ · 2025-10-25T13:09:00Z

This is my first open-source contribution, so I understand there may be several mistakes. Thank you for taking the time to review my work and provide guidance—I really appreciate it.

thomaseizinger

Great progress. One big item to discuss.

thomaseizinger · 2025-10-25T20:31:16Z

quinn-udp/src/unix.rs

+        recv_err(socket.0)
+    }
+
+    #[cfg(not(target_os = "linux"))]


I think we should aim for a unified API across all platforms, even if they currently don't return a value. Otherwise code that uses this also needs to use a cfg to compile on multiple platforms.

quinn-udp/src/unix.rs

thomaseizinger · 2025-10-25T20:39:41Z

quinn-udp/src/unix.rs

+
+    let mut control = cmsg::Aligned([0u8; CMSG_LEN]);
+
+    // We don't need actual data, just the error info


Oh! I understand now. So to receive error info, we pass a flag to recvmsg but otherwise it is the same syscall. An alternative design would be to do this as part of the other recv flow and store the received ICMP messages in a (bounded) queue. That would be more efficient in terms of syscall usage, especially because most of the time, this syscall here is not going to return anything. Yet, an application wanting to handle this still needs to call this in a loop with the other recv, taking up CPU usage of the recv hot path.

Curious to hear what other people think about this.

Your suggestion makes sense to me.

Interesting. In case someone here knows, what is the intended use, or asked differently, how do other applications use it?

djc · 2025-10-26T12:26:31Z

quinn-udp/src/lib.rs

+struct SockExtendedErr {
+    ee_errno: u32,
+    ee_origin: u8,
+    ee_type: u8,
+    ee_code: u8,
+    ee_pad: u8,
+    ee_info: u32,
+    ee_data: u32,


Nit: without any compelling reason to the contrary (like shared standardization with other APIs), suggest naming this SocketExtendedError and dropping the common ee_ prefix on fields (which is a kind of stuttering).

Also, since these only seem relevant on Unix, suggest that these should live in the unix module?

Also, since these only seem relevant on Unix, suggest that these should live in the unix module?

I don't think you addressed this.

Also, please keep it these types near the bottom of the module to maintain the top-down ordering of items.

quinn-udp/src/lib.rs

djc · 2025-10-26T12:27:28Z

quinn-udp/src/lib.rs

+    HostUnreachable,
+    PortUnreachable,
+    PacketTooBig,
+    Other { icmp_type: u8, icmp_code: u8 },


Nit: drop the icmp_ prefix, which is pretty obvious for a type named IcmpErrorKind.

Since type is a reserved keyword in Rust, it can’t be used directly as a field name, We can use raw identifier instead r#type. Can i do that?

Seems okay to me.

djc · 2025-10-26T12:27:53Z

quinn-udp/src/lib.rs

+
+#[cfg(target_os = "linux")]
+impl IcmpErrorKind {
+    fn from_extended_err(err: &SockExtendedErr) -> Self {


It feels like this should be SocketExtendedError::kind().

Can we name the magic constants for type and code here with their common libc or whatever names?

djc · 2025-10-28T12:43:18Z

Please squash all your changes before pushing.

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

djc · 2025-10-29T08:55:16Z

Please squash all your changes before pushing.

Including merges.

RaghavendraRQ requested review from Ralith, djc, gretchenfrage and mxinden as code owners October 21, 2025 10:11

RaghavendraRQ force-pushed the linux-ip-recverr branch 4 times, most recently from 5338a62 to aa6bf63 Compare October 21, 2025 11:26

mxinden reviewed Oct 21, 2025

View reviewed changes

quinn-udp/src/unix.rs Show resolved Hide resolved

RaghavendraRQ force-pushed the linux-ip-recverr branch from 2addc3c to 125a041 Compare October 22, 2025 10:29

thomaseizinger reviewed Oct 23, 2025

View reviewed changes

RaghavendraRQ force-pushed the linux-ip-recverr branch 2 times, most recently from 032a179 to a9e7e2a Compare October 25, 2025 13:07

RaghavendraRQ force-pushed the linux-ip-recverr branch from a9e7e2a to c21cbe6 Compare October 25, 2025 13:09

thomaseizinger reviewed Oct 25, 2025

View reviewed changes

djc reviewed Oct 26, 2025

View reviewed changes

RaghavendraRQ force-pushed the linux-ip-recverr branch from 4333346 to fd00c2e Compare October 29, 2025 05:28

RaghavendraRQ force-pushed the linux-ip-recverr branch from fd00c2e to 7278847 Compare October 29, 2025 05:35

RaghavendraRQ force-pushed the linux-ip-recverr branch 2 times, most recently from 5252988 to 7278847 Compare November 3, 2025 10:13

Merge branch 'main' into linux-ip-recverr

48581e3


		let mut control = cmsg::Aligned([0u8; CMSG_LEN]);

		// We don't need actual data, just the error info

Uh oh!

feat(ip_recverr): ADD IP_RECVERR and IPV6_RECVERR support for linux #2421

Are you sure you want to change the base?

feat(ip_recverr): ADD IP_RECVERR and IPV6_RECVERR support for linux #2421

Conversation

RaghavendraRQ commented Oct 21, 2025

Uh oh!

mxinden left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thomaseizinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RaghavendraRQ commented Oct 25, 2025

Uh oh!

thomaseizinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thomaseizinger Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

djc commented Oct 28, 2025

Uh oh!

djc commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thomaseizinger Oct 25, 2025 •

edited

Loading