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

Enable IP_BOUND_IF on illumos and Solaris #561

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hawkw
Copy link

@hawkw hawkw commented Feb 26, 2025

The IP_BOUND_IF socket option, which is wrapped by the
Socket::bind_device_by_index_{v4,v6} and
Socket::device_index_{v4,v6} is available on SunOS-like systems, such
as illumos and Solaris, as well as macOS-like systems. However, these
APIs are currently cfg-flagged to only be available on macOS-like
systems.

This commit changes the cfg attributes to also enable these APIs on
illumos and Solaris.

Fixes #560

The `IP_BOUND_IF` socket option, which is wrapped by the
`Socket::bind_device_by_index_{v4,v6}` and
`Socket::device_index_{v4,v6}` is available on SunOS-like systems, such
as illumos and Solaris, as well as macOS-like systems. However, these
APIs are currently cfg-flagged to only be available on macOS-like
systems.

This commit changes the cfg attributes to also enable these APIs on
illumos and Solaris.

Fixes rust-lang#560
@hawkw
Copy link
Author

hawkw commented Feb 26, 2025

Ugh, it looks like libc also doesn't expose these socket options on illumos or Solaris. That's annoying...

Comment on lines +1394 to +1396
// TODO: remove this once https://github.com/rust-lang/libc/pull/4287 merges
#[cfg(all(feature = "all", any(target_os = "illumos", target_os = "solaris")))]
const IP_BOUND_IF: libc::c_int = 0x41;
Copy link
Author

Choose a reason for hiding this comment

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

This is a bit of a bummer; I have an upstream PR rust-lang/libc#4287 to add these constants in libc, but wanted to test the change without that. I don't have a strong opinion on whether it's important to wait to pick these up from libc or if it's okay to just merge with a local definition and use them from libc in the future, but I imagine the maintainers will have an opinion on this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The libc pr is already merged, so let's wait for a release. I really don't want to maintain constants :)

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I'm happy to wait --- this was just intended to allow testing the change temporarily. Once there's a new libc release, I'll pick it up and update this.

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.

Socket::bind_device_by_index_{v4,v6} should be available on illumos/Solaris
2 participants