-
Notifications
You must be signed in to change notification settings - Fork 681
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
support PKTINFO cmsgs for Linux/macOS/iOS #891
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please enable IPV6_PKTINFO
on FreeBSD, and check the other BSDs to see if any of them support it. Also, can you write a test for this? The cmsg code is devilishly difficult; it would be great to have some test coverage for every supported message type.
@@ -414,6 +414,14 @@ impl<'a> Iterator for CmsgIterator<'a> { | |||
Some(ControlMessage::ScmTimestamp( | |||
&*(cmsg_data.as_ptr() as *const _))) | |||
}, | |||
#[cfg(any(target_os = "linux", target_os = "android", target_os = "macos", target_os = "ios"))] | |||
(libc::IPPROTO_IPV6, libc::IPV6_PKTINFO) => unsafe { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FreeBSD also defines IPV6_PKTINFO
. I haven't checked on any of the other BSDs.
Agreed about tests, it is tricky code. Will do.
Yeah the other BSDs will take longer because I only merged macOS/Linux
libc constants so far just for the sake of speed. Would you like to hold
off on this whole thing until then? Or would you accept separate PRs
after I get the rest merged in libc?
|
I'll wait for the libc PR. libc usually merges stuff quickly. |
new libc PR submitted: rust-lang/libc#983 |
I'm curious if this is still in progress, because I also need PKTINFO. |
@@ -414,6 +414,14 @@ impl<'a> Iterator for CmsgIterator<'a> { | |||
Some(ControlMessage::ScmTimestamp( | |||
&*(cmsg_data.as_ptr() as *const _))) | |||
}, | |||
#[cfg(any(target_os = "linux", target_os = "android", target_os = "macos", target_os = "ios"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lists should be alphabetized (android, ios, linux, macos). This applies to all of your #[cfg
attributes.
@@ -33,6 +33,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). | |||
([#857](https://github.com/nix-rust/nix/pull/857)) | |||
- Added `request_code_write_int!` on FreeBSD/DragonFlyBSD | |||
([#833](https://github.com/nix-rust/nix/pull/833)) | |||
- Added PKTINFO cmsg support on Linux/macOS/iOS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to "Added `PKTINFO` support for `cmsg` on ... platforms" (note the backticks)
I've updated @mcginty original changes to work with the lastest master in my ipv6_pktinfo branch. How do I contribute them to this pull request? I only see options for creating a new pull request. |
@pusateri sorry I've been MIA on this PR. I'm happy to close this pull request in favor of a new one you open. You have my blessing to either include my commits in that PR or squash them so it's simply under your name 🙂 |
Thanks @mcinty but I don't need credit. Just would like to use this feature. I can push my changes back to your branch if you give me permission. I'm just not familiar with how to update someone else's pull request. |
I submitted a new pull request that updates the changes, adds tests without using pre-configured loopback addresses, adds IPv6_PKTINFO for FreeBSD, and sorts the cfg options. See #990. |
The libc changes have been merged (rust-lang/libc#980) now, so looks like it's finally time to submit a PR for this :).