Skip to content

RFC 2292 struct and constant definitions#4954

Open
Skyb0rg007 wants to merge 1 commit intorust-lang:mainfrom
Skyb0rg007:icmpv6-filters
Open

RFC 2292 struct and constant definitions#4954
Skyb0rg007 wants to merge 1 commit intorust-lang:mainfrom
Skyb0rg007:icmpv6-filters

Conversation

@Skyb0rg007
Copy link
Copy Markdown

@Skyb0rg007 Skyb0rg007 commented Jan 30, 2026

Description

This PR adds the structs and constants as defined by RFC 2292 - Advanced Sockets API for IPv6.
These constants are available for use in get/setsockopt on supported Unix platforms.
The operations defined on the icmp6_filter struct are defined as C macros, so the implementations are not included here.

Sources

https://github.com/bminor/glibc/blob/04e750e75b73957cf1c791535a3f4319534a52fc/inet/netinet/icmp6.h
https://github.com/kraj/musl/blob/kraj/master/include/netinet/icmp6.h
https://github.com/freebsd/freebsd-src/blob/9ae367d11de8abbdf53884836c9ba30908c5c8db/sys/netinet/icmp6.h

Also in the RFC: https://datatracker.ietf.org/doc/html/rfc2292#section-3.2

Checklist

Unfortunately I could not get the libc-tests working locally, so this PR is marked as draft.

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot rustbot added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Jan 30, 2026
@Skyb0rg007 Skyb0rg007 force-pushed the icmpv6-filters branch 15 times, most recently from b8f20e4 to 16f3cea Compare February 3, 2026 20:32
@Skyb0rg007
Copy link
Copy Markdown
Author

Alright, I need help. On *BSD platforms (FreeBSD, macOS, etc), struct in6_addr is defined as the union of uint32_t[4], uint16_t[8], and uint8_t[16], so Rust encodes it as [u8; 16] with repr(align(4)).

But the nd_(router|neighbor)_(solicit|advert) structures are marked as __packed (so their C alignment is 1), even though they include a in6_addr. Rust won't allow using repr(packed) on a struct containing a repr(align(...)) attribute, so I'm stuck.

Linux doesn't mark these structs as __packed, so there's no issue there.

@Skyb0rg007 Skyb0rg007 force-pushed the icmpv6-filters branch 13 times, most recently from 742cf17 to 6d97ed6 Compare February 4, 2026 20:49
@rustbot

This comment has been minimized.

@Skyb0rg007
Copy link
Copy Markdown
Author

I am most interested in the addition of the ICMP6_FILTER constant to libc. If it would help with review and acceptance, I can create a separate PR with just that single change, and leave the other RFC constants and structures for later PRs.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@Skyb0rg007 Skyb0rg007 force-pushed the icmpv6-filters branch 2 times, most recently from 8700997 to 7f5c749 Compare February 18, 2026 21:25
@Skyb0rg007
Copy link
Copy Markdown
Author

I see that some C macros such as the CMSG_ macros were added this package as const functions. It may make sense to add the ICMP6_FILTER_ macros too, especially since the icmp6_filter struct’s field name is different on Solaris.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

This PR adds the icmp6_filter struct and constants as defined
by RFC 2292 - Advanced Sockets API for IPv6.
These constants are available for use in get/setsockopt
on supported Unix platforms.
The operations defined on the icmp6_filter struct are defined
as C macros, so the implementations are not included here.
See the RFC for definitions and use.
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 26, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Copy link
Copy Markdown
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, thank you for the changes here. The contents themselves look good, but I think they should be moved out of the root unix module.

View changes since this review

Comment on lines +256 to +259
#[cfg(not(any(target_os = "linux", target_os = "emscripten")))]
pub ip6r0_reserved: u32,
#[cfg(any(target_os = "linux", target_os = "emscripten"))]
pub ip6r0_reserved: u8,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For any reserved/padding fields, please make them private and wrap them in Padding

Comment on lines +284 to +287
#[cfg(not(target_os = "solaris"))]
pub icmp6_filt: [u32; 8],
#[cfg(target_os = "solaris")]
pub __icmp6_filt: [u32; 8],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This struct should be opaque, can you make the fields private?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think enabling these in the unix module is too high level; do we know anything about AIX, fuchsia, redox, etc?

Instead, could you put these into the linux_like and bsd modules? That will mean some duplication, but that's probably fine because it will simplify some of the repetitive padding config.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What would be ideal here is to instead put these in src/new/common/bsd/{icmp6.rs,ip6.rs} and src/new/common/linux_like/{icmp6.rs,ip6.rs}, to match the header structure. The reexport them in src/new/{bionic_libc,musl,glibc,openbsd,freebsd,...}/{icmp6.rs,ip6.rs}, similar to what is done with unistd, and update src/new/mod.rs.

I won't require that because this has already been waiting a while, but eventually everything will move to that new structure so anything to help that is appreciated.

Comment on lines +240 to +250
#[cfg_attr(
any(
target_os = "android",
target_os = "freebsd",
target_os = "openbsd",
target_os = "dragonfly",
target_os = "macos"
),
repr(packed)
)]
#[cfg_attr(any(target_os = "linux", target_os = "emscripten"), repr(align(4)))]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add links to the PR description to the headers for these other platforms that have the specific alignment requirements? (Looks like only glibc/musl/freebsd are there)

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 1, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author stable-nominated This PR should be considered for cherry-pick to libc's stable release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants