feat: add support for DSCP and TTL / Hop Limit #2425

crisidev · 2024-06-03T17:44:34Z

What does this PR do

This PR improves the support for setting and retrieving of IP headers values for

Incoming packets IPv4 TTL (IP_RECVTTL)
Incoming packets IPv6 Hop Limit (IPV6_RECVHOPLIMIT)
Outgoing and incoming packets IPv4 DSCP (IP_TOS)
Outgoing and incoming packets IPv6 DSCP (IPV6_RECVTCLASS)

It includes the socket options and the appropriate control messages for sendmsg and recvmsg.

Note: I am not 100% sure I understand correctly the cfg values and feature flags of nix, so they could be set wrongly in the PR.

Checklist:

I have read CONTRIBUTING.md
I have written necessary tests and rustdoc comments
A change log has been added if this PR modifies nix's API

* Support IP_RECVTTL and IPV6_RECVHOPLIMIT socket options and related control messages for recvmsg. * Support setting DSCP in control messages for both sendmsg and recvmsg. Signed-off-by: Bigo <[email protected]>

test/sys/test_socket.rs

src/sys/socket/sockopt.rs

SteveLauC · 2024-06-10T11:57:10Z

src/sys/socket/mod.rs

@@ -987,6 +1013,30 @@ impl ControlMessageOwned {
                let content_type = unsafe { ptr::read_unaligned(p as *const u8) };
                ControlMessageOwned::TlsGetRecordType(content_type.into())
            },
+            #[cfg(any(linux_android, target_os = "freebsd"))]
+            #[cfg(feature = "net")]
+            (libc::IPPROTO_IP, libc::IP_TTL) => {


On FreeBSD (and other BSDs, maybe), this should be:

Suggested change

(libc::IPPROTO_IP, libc::IP_TTL) => {

(libc::IPPROTO_IP, libc::IP_RECVTTL) => {

Sorry for being unclear, on FreeBSD, this should check IP_RECVTTL, on Linux, IP_TTL should be used.

Is the suggestion to have 2 different blocks, one for Linux and one for FreeBSD?

Yes

This is documented in man 7 ip:

Some BSD sockets implementations also provide an IP_RECVTTL option, but an ancillary message with type IP_RECVTTL is passed with the incoming packet. This is different from the IP_TTL option used in Linux.

also in the FreeBSD manual:

If the IP_RECVTTL option is enabled on a SOCK_DGRAM socket, the
recvmsg(2) call will return the IP TTL (time to live) field for a UDP
datagram. The msg_control field in the msghdr structure points to a
buffer that contains a cmsghdr structure followed by the TTL. The cms-
ghdr fields have the following values:

cmsg_len = CMSG_LEN(sizeof(u_char)) cmsg_level = IPPROTO_IP cmsg_type = IP_RECVTTL

src/sys/socket/mod.rs

test/sys/test_socket.rs

SteveLauC · 2024-06-10T12:30:51Z

test/sys/test_socket.rs

@@ -2544,6 +2544,166 @@ fn test_recvmsg_rxq_ovfl() {
    assert_eq!(drop_counter, 1);
 }

+#[cfg(target_os = "linux")]
+#[cfg(feature = "net")]
+#[cfg_attr(qemu, ignore)]


So these tests do not work under QEMU? Nix's cross CI tests use qemu under the hood

I think some of them (maybe all) will probably work.. I'll validate once I am done with addressing your comments. If they do not work, I'll try to document why.

SteveLauC · 2024-06-10T12:33:18Z

Hi, thanks for the PR! Sorry for the late response!

Note: I am not 100% sure I understand correctly the cfg values and feature flags of nix, so they could be set wrongly in the PR.

For the cfg for targets (OSes), see

nix/build.rs

Lines 20 to 27 in 5fde28e

    
           // cfg aliases we would like to use 
        
           apple_targets: { any(ios, macos, watchos, tvos, visionos) }, 
        
           bsd: { any(freebsd, dragonfly, netbsd, openbsd, apple_targets) }, 
        
           bsd_without_apple: { any(freebsd, dragonfly, netbsd, openbsd) }, 
        
           linux_android: { any(android, linux) }, 
        
           freebsdlike: { any(dragonfly, freebsd) }, 
        
           netbsdlike: { any(netbsd, openbsd) }, 
        
           solarish: { any(illumos, solaris) },

crisidev · 2024-06-11T09:40:03Z

Hi, thanks for the PR! Sorry for the late response!

Thanks a lot for the review and don't worry about the timing :)

I'll address the comments and publish a new version!

Note: I am not 100% sure I understand correctly the cfg values and feature flags of nix, so they could be set wrongly in the PR.

For the cfg for targets (OSes), see

nix/build.rs

Lines 20 to 27 in 5fde28e

// cfg aliases we would like to use

apple_targets: { any(ios, macos, watchos, tvos, visionos) },

bsd: { any(freebsd, dragonfly, netbsd, openbsd, apple_targets) },

bsd_without_apple: { any(freebsd, dragonfly, netbsd, openbsd) },

linux_android: { any(android, linux) },

freebsdlike: { any(dragonfly, freebsd) },

netbsdlike: { any(netbsd, openbsd) },

solarish: { any(illumos, solaris) },

Co-authored-by: SteveLauC <[email protected]>

crisidev · 2024-06-14T13:46:57Z

The code we have right now does not build on freebsd 14 because

error[E0531]: cannot find unit struct, unit variant or constant `IP_RECVTTL` in crate `libc`
    --> src/sys/socket/mod.rs:1018:38
     |
1018 |             (libc::IPPROTO_IP, libc::IP_RECVTTL) => {
     |                                      ^^^^^^^^^^ help: a constant with a similar name exists: `IP_RECVTOS`
     |
    ::: /.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.155/src/unix/bsd/freebsdlike/freebsd/mod.rs:3685:1
     |
3685 | pub const IP_RECVTOS: ::c_int = 68;
     | ----------------------------- similarly named constant `IP_RECVTOS` defined here

I think we need to enable RECVTTL only on linux.

SteveLauC · 2024-06-15T08:39:09Z

The code we have right now does not build on freebsd 14 because

error[E0531]: cannot find unit struct, unit variant or constant `IP_RECVTTL` in crate `libc`
    --> src/sys/socket/mod.rs:1018:38
     |
1018 |             (libc::IPPROTO_IP, libc::IP_RECVTTL) => {
     |                                      ^^^^^^^^^^ help: a constant with a similar name exists: `IP_RECVTOS`
     |
    ::: /.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.155/src/unix/bsd/freebsdlike/freebsd/mod.rs:3685:1
     |
3685 | pub const IP_RECVTOS: ::c_int = 68;
     | ----------------------------- similarly named constant `IP_RECVTOS` defined here

I think we need to enable RECVTTL only on linux.

On FreeBSD, IP_RECVTTL should be used, it is available on FreeBSD, our CI says it does not exist because the Rust libc crate does not include it, we need to add this constant to the libc crate.

Update: PR filed rust-lang/libc#3750

Update: PR merged, we can use the libc from git now:

libc = { git = "https://github.com/rust-lang/libc", branch = "libc-0.2", features = ["extra_traits"] }

SteveLauC · 2024-06-15T08:02:36Z

src/sys/socket/sockopt.rs

@@ -406,7 +406,7 @@ sockopt_impl!(
 #[cfg(feature = "net")]
 sockopt_impl!(
    #[cfg_attr(docsrs, doc(cfg(feature = "net")))]
-    /// Set or receive the Type-Of-Service (TOS) field that is
+    /// Set or receivethe Type-Of-Service (TOS) field that is


Need a blank space here:

Suggested change

/// Set or receivethe Type-Of-Service (TOS) field that is

/// Set or receive the Type-Of-Service (TOS) field that is

SteveLauC · 2024-06-15T08:28:59Z

src/sys/socket/mod.rs

@@ -987,6 +1013,30 @@ impl ControlMessageOwned {
                let content_type = unsafe { ptr::read_unaligned(p as *const u8) };
                ControlMessageOwned::TlsGetRecordType(content_type.into())
            },
+            #[cfg(any(linux_android, target_os = "freebsd"))]
+            #[cfg(feature = "net")]
+            (libc::IPPROTO_IP, libc::IP_TTL) => {


Sorry for being unclear, on FreeBSD, this should check IP_RECVTTL, on Linux, IP_TTL should be used.

Is the suggestion to have 2 different blocks, one for Linux and one for FreeBSD?

Yes

This is documented in man 7 ip:

Some BSD sockets implementations also provide an IP_RECVTTL option, but an ancillary message with type IP_RECVTTL is passed with the incoming packet. This is different from the IP_TTL option used in Linux.

also in the FreeBSD manual:

If the IP_RECVTTL option is enabled on a SOCK_DGRAM socket, the
recvmsg(2) call will return the IP TTL (time to live) field for a UDP
datagram. The msg_control field in the msghdr structure points to a
buffer that contains a cmsghdr structure followed by the TTL. The cms-
ghdr fields have the following values:

cmsg_len = CMSG_LEN(sizeof(u_char)) cmsg_level = IPPROTO_IP cmsg_type = IP_RECVTTL

SteveLauC · 2024-06-15T08:33:03Z

src/sys/socket/mod.rs

+            },
+            #[cfg(any(linux_android, target_os = "freebsd"))]
+            #[cfg(feature = "net")]
+            (libc::IPPROTO_IP, libc::IP_TOS) => {


That's right, sorry for being unclear.

SteveLauC · 2024-06-15T09:03:21Z

src/sys/socket/mod.rs

@@ -1137,7 +1193,7 @@ pub enum ControlMessage<'a> {
    #[cfg_attr(docsrs, doc(cfg(feature = "net")))]
    Ipv6HopLimit(&'a libc::c_int),

-    /// SO_RXQ_OVFL indicates that an unsigned 32 bit value
+        /// SO_RXQ_OVFL indicates that an unsigned 32 bit value


A change made by accident:)

crisidev · 2024-06-17T11:49:50Z

I am moving this back to draft, while I find some time to finish to implement the changes and fix all the tests.

crisidev added 2 commits June 3, 2024 18:38

feat: add support for DSCP and TTL / Hop Limit

8a030dc

* Support IP_RECVTTL and IPV6_RECVHOPLIMIT socket options and related control messages for recvmsg. * Support setting DSCP in control messages for both sendmsg and recvmsg. Signed-off-by: Bigo <[email protected]>

Add PR changelog

27cac60

crisidev marked this pull request as ready for review June 3, 2024 17:45

crisidev added 5 commits June 3, 2024 18:49

This is not supported on Freebsd

4faed1b

IPV6_RECVTCLASS not supported on freebsd

342aa6b

Properly limit IPV6_RECVTCLASS

84fc5b1

Properly limit IP_TOS

72173b5

Restrict everything to target_os linux

64efe4e

crisidev marked this pull request as draft June 3, 2024 18:07

crisidev added 2 commits June 4, 2024 12:17

...

ee761a8

Protect bind

8f52422

crisidev marked this pull request as ready for review June 4, 2024 14:55

crisidev commented Jun 4, 2024

View reviewed changes

test/sys/test_socket.rs Show resolved Hide resolved

SteveLauC reviewed Jun 10, 2024

View reviewed changes

crisidev and others added 3 commits June 14, 2024 12:54

Apply suggestions from code review

be4a2de

Co-authored-by: SteveLauC <[email protected]>

Address PR comments

6e34f92

Run cargo fmt

13b2107

SteveLauC reviewed Jun 15, 2024

View reviewed changes

SteveLauC mentioned this pull request Jun 15, 2024

feat: IP_RECVTTL/IPV6_RECVHOPLIMIT for FreeBSD/DragonFlyBSD rust-lang/libc#3750

Merged

SteveLauC reviewed Jun 15, 2024

View reviewed changes

crisidev marked this pull request as draft June 17, 2024 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for DSCP and TTL / Hop Limit #2425

feat: add support for DSCP and TTL / Hop Limit #2425

crisidev commented Jun 3, 2024 •

edited

Loading

SteveLauC Jun 10, 2024

SteveLauC Jun 15, 2024 •

edited

Loading

SteveLauC Jun 10, 2024 •

edited

Loading

crisidev Jun 14, 2024

SteveLauC commented Jun 10, 2024 •

edited

Loading

crisidev commented Jun 11, 2024

crisidev commented Jun 14, 2024

SteveLauC commented Jun 15, 2024 •

edited

Loading

SteveLauC Jun 15, 2024

SteveLauC Jun 15, 2024 •

edited

Loading

SteveLauC Jun 15, 2024

SteveLauC Jun 15, 2024

crisidev commented Jun 17, 2024

	(libc::IPPROTO_IP, libc::IP_TTL) => {
	(libc::IPPROTO_IP, libc::IP_RECVTTL) => {

	/// Set or receivethe Type-Of-Service (TOS) field that is
	/// Set or receive the Type-Of-Service (TOS) field that is

feat: add support for DSCP and TTL / Hop Limit #2425

Are you sure you want to change the base?

feat: add support for DSCP and TTL / Hop Limit #2425

Conversation

crisidev commented Jun 3, 2024 • edited Loading

What does this PR do

Checklist:

SteveLauC Jun 10, 2024

Choose a reason for hiding this comment

SteveLauC Jun 15, 2024 • edited Loading

Choose a reason for hiding this comment

SteveLauC Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

crisidev Jun 14, 2024

Choose a reason for hiding this comment

SteveLauC commented Jun 10, 2024 • edited Loading

crisidev commented Jun 11, 2024

crisidev commented Jun 14, 2024

SteveLauC commented Jun 15, 2024 • edited Loading

SteveLauC Jun 15, 2024

Choose a reason for hiding this comment

SteveLauC Jun 15, 2024 • edited Loading

Choose a reason for hiding this comment

SteveLauC Jun 15, 2024

Choose a reason for hiding this comment

SteveLauC Jun 15, 2024

Choose a reason for hiding this comment

crisidev commented Jun 17, 2024

crisidev commented Jun 3, 2024 •

edited

Loading

SteveLauC Jun 15, 2024 •

edited

Loading

SteveLauC Jun 10, 2024 •

edited

Loading

SteveLauC commented Jun 10, 2024 •

edited

Loading

SteveLauC commented Jun 15, 2024 •

edited

Loading

SteveLauC Jun 15, 2024 •

edited

Loading