Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add FreeBSD IP_RECVDSTADDR support #1447

Merged
merged 1 commit into from
Dec 2, 2022
Merged

add FreeBSD IP_RECVDSTADDR support #1447

merged 1 commit into from
Dec 2, 2022

Conversation

lucifer9
Copy link
Contributor

Fixes #1142

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Any chance you could add a FreeBSD case to CI so this doesn't bitrot?

quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
@lucifer9
Copy link
Contributor Author

There's no FreeBSD runner image. So I try to use vmactions/freebsd-vm to run tests.

quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
Ralith
Ralith previously approved these changes Nov 30, 2022
quinn-udp/src/unix.rs Outdated Show resolved Hide resolved
djc
djc previously approved these changes Dec 1, 2022
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing up our FreeBSD support (and adding a CI test for it)!

I had a bit of a hard time reviewing this because it seems to have grown to encompass a bit more than just RECVDSTADDR. If I understand correctly, there are few things here:

  • Set IPV6_RECVPKTINFO on FreeBSD as well (not just on Linux)
  • Set IP_RECVDSTADDR on FreeBSD and macOS only for IPv4
  • Fix a warning about unused state: &UdpState which we didn't notice before
  • Allow different types for the 3rd argument to sendmmsg()/recvmmsg()

It might be nice add some comments about how FreeBSD compares to Linux here, which is left implicit, for the reader to decode from the code. Additionally, IMO the expected spelling for encode_srcip would be encode_src_ip -- might be nice to fix that up.

…n FreeBSD & macOS, set IP_RECVDSTADDR on FreeBSD and macOS only for IPv4, fix a warning about unused state: &UdpState, allow different types for the 3rd argument to sendmmsg()/recvmmsg()
@lucifer9
Copy link
Contributor Author

lucifer9 commented Dec 2, 2022

Thank you for fixing up our FreeBSD support (and adding a CI test for it)!

I had a bit of a hard time reviewing this because it seems to have grown to encompass a bit more than just RECVDSTADDR. If I understand correctly, there are few things here:

  • Set IPV6_RECVPKTINFO on FreeBSD as well (not just on Linux)
  • Set IP_RECVDSTADDR on FreeBSD and macOS only for IPv4
  • Fix a warning about unused state: &UdpState which we didn't notice before
  • Allow different types for the 3rd argument to sendmmsg()/recvmmsg()

It might be nice add some comments about how FreeBSD compares to Linux here, which is left implicit, for the reader to decode from the code. Additionally, IMO the expected spelling for encode_srcip would be encode_src_ip -- might be nice to fix that up.

I added a few comments to clarify these parts related to FreeBSD and macOS, hope they be useful. And I have fixed the spelling😊

@djc djc merged commit 166e0fb into quinn-rs:main Dec 2, 2022
@asomers
Copy link

asomers commented Dec 2, 2022

BTW, the easiest way to do CI on FreeBSD for Github-hosted projects is with https://cirrus-ci.com/ .

@lucifer9 lucifer9 deleted the freebsd branch December 3, 2022 09:03
@littlesum
Copy link

Fixes #1142

I see you fix this bug in freebsd, but it still not work in openbsd, is there any possible help fix it in openbsd

sur# cargo build
    Updating crates.io index
  Downloaded matchers v0.1.0
  Downloaded num_threads v0.1.6
  Downloaded heck v0.4.0
  Downloaded thread_local v1.1.4
  Downloaded nu-ansi-term v0.46.0
  Downloaded bytes v1.3.0
  Downloaded anyhow v1.0.66
  Downloaded tracing-subscriber v0.3.16
  Downloaded hdrhistogram v7.5.2
  Downloaded tokio v1.23.0
  Downloaded time-core v0.1.0
  Downloaded rustc-hash v1.1.0
  Downloaded termcolor v1.1.3
  Downloaded proc-macro-error v1.0.4
  Downloaded strsim v0.10.0
  Downloaded time-macros v0.2.6
  Downloaded yasna v0.5.0
  Downloaded time v0.3.17
  Downloaded clap_derive v3.2.18
  Downloaded sharded-slab v0.1.4
  Downloaded signal-hook-registry v1.4.0
  Downloaded overload v0.1.1
  Downloaded rcgen v0.10.0
  Downloaded proc-macro-error-attr v1.0.4
  Downloaded pem v1.1.0
  Downloaded 25 crates (1.7 MB) in 5.49s
   Compiling proc-macro2 v1.0.47
   Compiling unicode-ident v1.0.5
   Compiling quote v1.0.21
   Compiling libc v0.2.138
   Compiling syn v1.0.105
   Compiling autocfg v1.1.0
   Compiling once_cell v1.16.0
   Compiling cfg-if v1.0.0
   Compiling cc v1.0.77
   Compiling untrusted v0.7.1
   Compiling spin v0.5.2
   Compiling version_check v0.9.4
   Compiling base64 v0.13.1
   Compiling pin-project-lite v0.2.9
   Compiling itoa v1.0.4
   Compiling log v0.4.17
   Compiling rustls v0.20.7
   Compiling ppv-lite86 v0.2.17
   Compiling thiserror v1.0.37
   Compiling openssl-probe v0.1.5
   Compiling regex-syntax v0.6.28
   Compiling tinyvec_macros v0.1.0
   Compiling time-core v0.1.0
   Compiling num_threads v0.1.6
   Compiling rustc-hash v1.1.0
   Compiling bytes v1.3.0
   Compiling lazy_static v1.4.0
   Compiling heck v0.4.0
   Compiling hashbrown v0.12.3
   Compiling serde_derive v1.0.150
   Compiling overload v0.1.1
   Compiling anyhow v1.0.66
   Compiling os_str_bytes v6.4.1
   Compiling strsim v0.10.0
   Compiling textwrap v0.16.0
   Compiling serde v1.0.150
   Compiling termcolor v1.1.3
   Compiling byteorder v1.4.3
   Compiling bitflags v1.3.2
   Compiling serde_json v1.0.89
   Compiling ryu v1.0.11
   Compiling tracing-core v0.1.30
   Compiling thread_local v1.1.4
   Compiling slab v0.4.7
   Compiling tokio v1.23.0
   Compiling num-traits v0.2.15
   Compiling indexmap v1.9.2
   Compiling proc-macro-error-attr v1.0.4
   Compiling proc-macro-error v1.0.4
   Compiling rustls-pemfile v1.0.1
   Compiling pem v1.1.0
   Compiling ring v0.16.20
   Compiling tinyvec v1.6.0
   Compiling sharded-slab v0.1.4
   Compiling regex-automata v0.1.10
   Compiling regex v1.7.0
   Compiling nu-ansi-term v0.46.0
   Compiling clap_lex v0.2.4
   Compiling rustls-native-certs v0.6.2
   Compiling matchers v0.1.0
   Compiling getrandom v0.2.8
   Compiling time v0.3.17
   Compiling socket2 v0.4.7
   Compiling num_cpus v1.14.0
   Compiling signal-hook-registry v1.4.0
   Compiling atty v0.2.14
   Compiling mio v0.8.5
   Compiling rand_core v0.6.4
   Compiling yasna v0.5.0
   Compiling hdrhistogram v7.5.2
   Compiling rand_chacha v0.3.1
   Compiling sct v0.7.0
   Compiling webpki v0.22.0
   Compiling rcgen v0.10.0
   Compiling rand v0.8.5
   Compiling tracing-attributes v0.1.23
   Compiling thiserror-impl v1.0.37
   Compiling tokio-macros v1.8.2
   Compiling clap_derive v3.2.18
   Compiling tracing v0.1.37
   Compiling clap v3.2.23
   Compiling tracing-subscriber v0.3.16
   Compiling quinn-proto v0.9.2 (/home/build/quinn-0.9.3/quinn-proto)
   Compiling quinn-udp v0.3.2 (/home/build/quinn-0.9.3/quinn-udp)
error[E0425]: cannot find value `IP_RECVTOS` in crate `libc`
    --> quinn-udp/src/unix.rs:94:75
     |
94   |         if let Err(err) = set_socket_option(&*io, libc::IPPROTO_IP, libc::IP_RECVTOS, OPTION_ON) {
     |                                                                           ^^^^^^^^^^ help: a constant with a similar name exists: `IP_RECVIF`
     |
    ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/netbsdlike/openbsd/mod.rs:1068:1
     |
1068 | pub const IP_RECVIF: ::c_int = 30;
     | ---------------------------------- similarly named constant `IP_RECVIF` defined here

error[E0412]: cannot find type `mmsghdr` in crate `libc`
   --> quinn-udp/src/unix.rs:166:26
    |
166 |       let mut msgs: [libc::mmsghdr; BATCH_SIZE] = unsafe { mem::zeroed() };
    |                            ^^^^^^^ help: a struct with a similar name exists: `cmsghdr`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:10:1
    |
10  | / s! {
11  | |     pub struct sockaddr {
12  | |         pub sa_len: u8,
13  | |         pub sa_family: sa_family_t,
...   |
124 | |     }
125 | | }
    | |_- similarly named struct `cmsghdr` defined here

error[E0425]: cannot find function `sendmmsg` in crate `libc`
   --> quinn-udp/src/unix.rs:197:32
    |
197 |         let n = unsafe { libc::sendmmsg(io.as_raw_fd(), msgs.as_mut_ptr(), num_transmits as _, 0) };
    |                                ^^^^^^^^ help: a function with a similar name exists: `sendmsg`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:743:5
    |
743 |     pub fn sendmsg(fd: ::c_int, msg: *const ::msghdr, flags: ::c_int) -> ::ssize_t;
    |     ------------------------------------------------------------------------------- similarly named function `sendmsg` defined here

error[E0412]: cannot find type `mmsghdr` in crate `libc`
   --> quinn-udp/src/unix.rs:296:50
    |
296 |       let mut hdrs = unsafe { mem::zeroed::<[libc::mmsghdr; BATCH_SIZE]>() };
    |                                                    ^^^^^^^ help: a struct with a similar name exists: `cmsghdr`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:10:1
    |
10  | / s! {
11  | |     pub struct sockaddr {
12  | |         pub sa_len: u8,
13  | |         pub sa_family: sa_family_t,
...   |
124 | |     }
125 | | }
    | |_- similarly named struct `cmsghdr` defined here

error[E0425]: cannot find function `recvmmsg` in crate `libc`
   --> quinn-udp/src/unix.rs:308:19
    |
308 |             libc::recvmmsg(
    |                   ^^^^^^^^ help: a function with a similar name exists: `recvmsg`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:748:5
    |
748 |     pub fn recvmsg(fd: ::c_int, msg: *mut ::msghdr, flags: ::c_int) -> ::ssize_t;
    |     ----------------------------------------------------------------------------- similarly named function `recvmsg` defined here

error[E0531]: cannot find unit struct, unit variant or constant `IP_RECVTOS` in crate `libc`
    --> quinn-udp/src/unix.rs:472:73
     |
472  |             (libc::IPPROTO_IP, libc::IP_TOS) | (libc::IPPROTO_IP, libc::IP_RECVTOS) => unsafe {
     |                                                                         ^^^^^^^^^^ help: a constant with a similar name exists: `IP_RECVIF`
     |
    ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/netbsdlike/openbsd/mod.rs:1068:1
     |
1068 | pub const IP_RECVIF: ::c_int = 30;
     | ---------------------------------- similarly named constant `IP_RECVIF` defined here

Some errors have detailed explanations: E0412, E0425, E0531.
For more information about an error, try `rustc --explain E0412`.
error: could not compile `quinn-udp` due to 6 previous errors
warning: build failed, waiting for other jobs to finish...
sur#
sur# cargo build
   Compiling quinn-udp v0.3.2 (/home/build/quinn-0.9.3/quinn-udp)
error[E0425]: cannot find value `IP_RECVTOS` in crate `libc`
    --> quinn-udp/src/unix.rs:94:75
     |
94   |         if let Err(err) = set_socket_option(&*io, libc::IPPROTO_IP, libc::IP_RECVTOS, OPTION_ON) {
     |                                                                           ^^^^^^^^^^ help: a constant with a similar name exists: `IP_RECVIF`
     |
    ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/netbsdlike/openbsd/mod.rs:1068:1
     |
1068 | pub const IP_RECVIF: ::c_int = 30;
     | ---------------------------------- similarly named constant `IP_RECVIF` defined here

error[E0412]: cannot find type `mmsghdr` in crate `libc`
   --> quinn-udp/src/unix.rs:166:26
    |
166 |       let mut msgs: [libc::mmsghdr; BATCH_SIZE] = unsafe { mem::zeroed() };
    |                            ^^^^^^^ help: a struct with a similar name exists: `cmsghdr`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:10:1
    |
10  | / s! {
11  | |     pub struct sockaddr {
12  | |         pub sa_len: u8,
13  | |         pub sa_family: sa_family_t,
...   |
124 | |     }
125 | | }
    | |_- similarly named struct `cmsghdr` defined here

error[E0425]: cannot find function `sendmmsg` in crate `libc`
   --> quinn-udp/src/unix.rs:197:32
    |
197 |         let n = unsafe { libc::sendmmsg(io.as_raw_fd(), msgs.as_mut_ptr(), num_transmits as _, 0) };
    |                                ^^^^^^^^ help: a function with a similar name exists: `sendmsg`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:743:5
    |
743 |     pub fn sendmsg(fd: ::c_int, msg: *const ::msghdr, flags: ::c_int) -> ::ssize_t;
    |     ------------------------------------------------------------------------------- similarly named function `sendmsg` defined here

error[E0412]: cannot find type `mmsghdr` in crate `libc`
   --> quinn-udp/src/unix.rs:296:50
    |
296 |       let mut hdrs = unsafe { mem::zeroed::<[libc::mmsghdr; BATCH_SIZE]>() };
    |                                                    ^^^^^^^ help: a struct with a similar name exists: `cmsghdr`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:10:1
    |
10  | / s! {
11  | |     pub struct sockaddr {
12  | |         pub sa_len: u8,
13  | |         pub sa_family: sa_family_t,
...   |
124 | |     }
125 | | }
    | |_- similarly named struct `cmsghdr` defined here

error[E0425]: cannot find function `recvmmsg` in crate `libc`
   --> quinn-udp/src/unix.rs:308:19
    |
308 |             libc::recvmmsg(
    |                   ^^^^^^^^ help: a function with a similar name exists: `recvmsg`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:748:5
    |
748 |     pub fn recvmsg(fd: ::c_int, msg: *mut ::msghdr, flags: ::c_int) -> ::ssize_t;
    |     ----------------------------------------------------------------------------- similarly named function `recvmsg` defined here

error[E0531]: cannot find unit struct, unit variant or constant `IP_RECVTOS` in crate `libc`
    --> quinn-udp/src/unix.rs:472:73
     |
472  |             (libc::IPPROTO_IP, libc::IP_TOS) | (libc::IPPROTO_IP, libc::IP_RECVTOS) => unsafe {
     |                                                                         ^^^^^^^^^^ help: a constant with a similar name exists: `IP_RECVIF`
     |
    ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/netbsdlike/openbsd/mod.rs:1068:1
     |
1068 | pub const IP_RECVIF: ::c_int = 30;
     | ---------------------------------- similarly named constant `IP_RECVIF` defined here

Some errors have detailed explanations: E0412, E0425, E0531.
For more information about an error, try `rustc --explain E0412`.
error: could not compile `quinn-udp` due to 6 previous errors
sur#
sur# cargo build
   Compiling quinn-udp v0.3.2 (/home/build/quinn-0.9.3/quinn-udp)
error[E0425]: cannot find value `IP_RECVTOS` in crate `libc`
    --> quinn-udp/src/unix.rs:94:75
     |
94   |         if let Err(err) = set_socket_option(&*io, libc::IPPROTO_IP, libc::IP_RECVTOS, OPTION_ON) {
     |                                                                           ^^^^^^^^^^ help: a constant with a similar name exists: `IP_RECVIF`
     |
    ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/netbsdlike/openbsd/mod.rs:1068:1
     |
1068 | pub const IP_RECVIF: ::c_int = 30;
     | ---------------------------------- similarly named constant `IP_RECVIF` defined here

error[E0412]: cannot find type `mmsghdr` in crate `libc`
   --> quinn-udp/src/unix.rs:166:26
    |
166 |       let mut msgs: [libc::mmsghdr; BATCH_SIZE] = unsafe { mem::zeroed() };
    |                            ^^^^^^^ help: a struct with a similar name exists: `cmsghdr`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:10:1
    |
10  | / s! {
11  | |     pub struct sockaddr {
12  | |         pub sa_len: u8,
13  | |         pub sa_family: sa_family_t,
...   |
124 | |     }
125 | | }
    | |_- similarly named struct `cmsghdr` defined here

error[E0425]: cannot find function `sendmmsg` in crate `libc`
   --> quinn-udp/src/unix.rs:197:32
    |
197 |         let n = unsafe { libc::sendmmsg(io.as_raw_fd(), msgs.as_mut_ptr(), num_transmits as _, 0) };
    |                                ^^^^^^^^ help: a function with a similar name exists: `sendmsg`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:743:5
    |
743 |     pub fn sendmsg(fd: ::c_int, msg: *const ::msghdr, flags: ::c_int) -> ::ssize_t;
    |     ------------------------------------------------------------------------------- similarly named function `sendmsg` defined here

error[E0412]: cannot find type `mmsghdr` in crate `libc`
   --> quinn-udp/src/unix.rs:296:50
    |
296 |       let mut hdrs = unsafe { mem::zeroed::<[libc::mmsghdr; BATCH_SIZE]>() };
    |                                                    ^^^^^^^ help: a struct with a similar name exists: `cmsghdr`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:10:1
    |
10  | / s! {
11  | |     pub struct sockaddr {
12  | |         pub sa_len: u8,
13  | |         pub sa_family: sa_family_t,
...   |
124 | |     }
125 | | }
    | |_- similarly named struct `cmsghdr` defined here

error[E0425]: cannot find function `recvmmsg` in crate `libc`
   --> quinn-udp/src/unix.rs:308:19
    |
308 |             libc::recvmmsg(
    |                   ^^^^^^^^ help: a function with a similar name exists: `recvmsg`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/mod.rs:748:5
    |
748 |     pub fn recvmsg(fd: ::c_int, msg: *mut ::msghdr, flags: ::c_int) -> ::ssize_t;
    |     ----------------------------------------------------------------------------- similarly named function `recvmsg` defined here

error[E0531]: cannot find unit struct, unit variant or constant `IP_RECVTOS` in crate `libc`
    --> quinn-udp/src/unix.rs:472:73
     |
472  |             (libc::IPPROTO_IP, libc::IP_TOS) | (libc::IPPROTO_IP, libc::IP_RECVTOS) => unsafe {
     |                                                                         ^^^^^^^^^^ help: a constant with a similar name exists: `IP_RECVIF`
     |
    ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/unix/bsd/netbsdlike/openbsd/mod.rs:1068:1
     |
1068 | pub const IP_RECVIF: ::c_int = 30;
     | ---------------------------------- similarly named constant `IP_RECVIF` defined here

Some errors have detailed explanations: E0412, E0425, E0531.
For more information about an error, try `rustc --explain E0412`.
error: could not compile `quinn-udp` due to 6 previous errors

@djc
Copy link
Member

djc commented Dec 13, 2022

Let's discuss OpenBSD in #1469 (@lucifer9 any relevant expertise you have would be much appreciated!).

@djc djc mentioned this pull request May 8, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails to build on FreeBSD
5 participants