-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Tracking Issue for feature(unix_socket_ancillary_data) #76915
Comments
#79753 changes just the (Edit: Hmm, actually changing the ancillary data too would require both a SocketAncillary and SocketAncillaryMut, like IoSlice. Probably not worth it there) |
@tadeokondrak I open a new PR #82589. |
I create a new PR to send and receive TTL for UdpSocket from the ancillary interface. #82858 |
unix: Fix feature(unix_socket_ancillary_data) on macos and other BSDs This adds support for CMSG handling on macOS and fixes it on OpenBSD and possibly other BSDs. When traversing the CMSG list, the previous code had an exception for Android where the next element after the last pointer could point to the first pointer instead of NULL. This is actually not specific to Android: the `libc::CMSG_NXTHDR` implementation for Linux and emscripten have a special case to return NULL when the length of the previous element is zero; most other implementations simply return the previous element plus a zero offset in this case. This MR makes the check non-optional which fixes CMSG handling and a possible endless loop on such systems; tested with file descriptor passing on OpenBSD, Linux, and macOS. This MR additionally adds `SocketAncillary::is_empty` because clippy is right that it should be added. This belongs to the `feature(unix_socket_ancillary_data)` tracking issue: rust-lang#76915 r? `@joshtriplett`
What has to be done to stabilize this API? |
I think the API still needs polish. All those niche socket features are tricky and right now it's probably not extensible enough for 3rd party libraries to fill platform-specific gaps such as flags or cmsgs only supported on one particular OS. These basically are wrappers for the Before stabilizing we should consider whether Or we could put the flags into Similar could be done for sending. Additionally the |
What about CMSG_SPACE? Without it, we're forced to either guess or probe the appropriate buffer size. https://man7.org/linux/man-pages/man3/cmsg.3.html SCM_RIGHTS example shows how it's used to compute the correct buffer size (with some tricky business wrt alignment). |
Hi @tv42, Is there a function, which calculate the alignment for a struct to a specific address in Rust? |
CMSG_SPACE includes alignment bytes, CMSG_LEN doesn't.
I don't know what this means. |
I thought that the alignment depends on the address, where you want to store the struct. |
What it means for CMGS_SPACE to include “alignment bytes” is that it rounds the size up to include a number of padding bytes to make sure the next header is properly aligned (to the alignment of cmsghdr). |
So, the address have to be a multiple of the return value of |
The address of the cmsghdr object has to be a multiple of |
Sorry, that I ask again. Does the macro |
As an example, consider the following two headers: let A = cmsghdr{ cmsg_len = 19, cmsg_level = X, cmsg_type = Y };
let B = cmsghdr{ cmsg_len = 22, cmsg_level = W, cmsg_type = Z }; If
Notice how header B is at improperly aligned address 19, which is not a multiple of the alignment of cmsghdr. By rounding up the lengths (assuming an alignment of 8 for this example), we get this:
Now, every header and each field in every header is at the aligned address it should be. |
Thanks, @chloekek. Currently, the code is using But, if |
Actually, the man page says that you should include the padding in cmsg_len:
This means that the receiver will indeed interpret those bytes as part of the data, I don’t think there’s any way around that? This would mean that, if |
Interesting, currently there is a test, which send one FD and I tested on a 64-bit system, where the alignment of |
What is suspicious is that CMSG_NXTHDR adds the alignment if necessary. This could mean that either:
Note that while you could test this, other Unix implementations (e.g. BSD) may behave differently from Linux in this regard. It would be best to check the Official Spec™ (POSIX?). As for file descriptors, the kernel will indeed divide the data length by the size of a file descriptor in order to compute how many file descriptors to send. And it seems that you indeed do have to set all of them to valid file descriptors, not to |
So to summarize:
|
Disable unix::net::ancillary on BSD. See rust-lang#76915 (comment)
Disable unix::net::ancillary on BSD. See rust-lang/rust#76915 (comment)
I'm interested in this feature and have time to work on it. However, the current API is very different from what I would expect:
How much flexibility is available for making changes to the API at this point? If I were to write an RFC with an API approximating the following, would that be a plausible next step toward stabilization? // ===== API generic to any UNIX platform
trait unix::net::SocketSend {
// equivalent to sendmsg()
fn send_message(..., Option<&mut AncillaryData>) -> io::Result<usize>;
}
trait unix::net::SocketRecv {
// equivalent to recvmsg()
fn receive_message(..., Option<&mut AncillaryData>) -> io::Result<usize>;
}
struct unix::net::AncillaryData;
fn control_messages<'a>(&'a self) -> impl Iter<ControlMessage<'a>>;
struct unix::net::ControlMessage<'a>;
fn level() -> c_int;
fn type() -> c_int;
fn data() -> &'a [u8]
// ===== Platform-specific API (Linux)
#[non_exhaustive]
enum linux::net::ControlMessage<'a> {
ScmRights(ScmRights<'a>),
ScmCredentials(ScmCredentials<'a>),
}
trait linux::net::ControlMessageExt: Sealed {
fn try_get(&self) -> Option<linux::net::ControlMessage<'a>>,
}
struct linux::net::ScmCredentials<'a>;
fn new(&unix::net::ControlMessage) -> Option<Self>;
struct linux::net::ScmRights<'a>;
fn new(&unix::net::ControlMessage) -> Option<Self>;
// ===== Platform-specific API (etc)
trait freebsd::net::ControlMessageExt: Sealed { ... }
trait macos::net::ControlMessageExt: Sealed { ... }
trait netbsd::net::ControlMessageExt: Sealed { ... } Advantages:
|
I found a potential critical concern with how
My own implementation of Unix domain sockets and ancillary data in the |
I wrote a draft RFC for a v2 of Unix socket ancillary data support: rust-lang/rfcs#3430 In my testing, that proposal solves all the known issues with the current implementation (platform-specific control messages, third-party library extensibility, and file descriptor ownership). I would be happy if folks interested in stabilized ancillary data support could take a look at that RFC. |
Are you planning to implement it too? Asking since I intended to work on this eventually. |
Yes, if that RFC is accepted then I plan to implement it. I already have it working in a local branch, so it'd just be a matter of adding docs and tests. |
The RFC looks like it's moving now (tagged
|
As far as I can see, the current implementation is unsound: it takes a user-provided buffer and performs unaligned reads and writes. Is this a known issue? |
253 seems to be a hardcoded value without any particular reasoning. Move it to a constant and use a rounder value of 128. There were also troubles regarding alignment: the API asks us to pass an arbitrary byte buffer and then performs unaligned reads/writes. Workaround that by aligning the buffer manually. For more information, see rust-lang/rust#76915 (comment)
this allows us to work on an existing ancillary buffer or re-use a SocketAncillary on different read buffers if we want, and as a by-product makes testing a lot easier
and:
this allows us to println!("{:?}", ancillary.messages()) which makes logging/ debugging a lot easier.
Many thanks |
My use case is I don't know exactly where in a stream ancillary data will appear - I'd like to use several reads to parse a message and aggregate ancillary data across them (in a single buffer) to be used at the end. It doesn't appear trivially possible with the current design. |
I am doing exactly this. By making the constructor public and adding the
length and truncated to the constructor, I can customise the mio behaviour
as I need by managing the buffer/length/truncated collection myself using
the mio io call and only later creating this class to do the unpacking
…On Sat, 4 May 2024, 12:50 Andrew Baxter, ***@***.***> wrote:
1. Would if be possible to make the constructor public?
My use case is I don't know exactly where in a stream ancillary data will
appear. I'd like to use several reads to parse a message and aggregate
ancillary data across them (in a single buffer) to be used at the end. It
doesn't appear trivially possible with the current design.
—
Reply to this email directly, view it on GitHub
<#76915 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ASLCFDRQSLPJPQPNENYEB2TZATDRHAVCNFSM4RS6TJ6KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBZGQYTGNJQGY2A>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Wouldn't it be better for |
Would it be better for |
at least sending it requires zero-initialization anyway. |
Is that the responsibility of the caller of, or of |
I see that it currently only includes |
Tracking issue for
feature(unix_socket_ancillary_data)
to extendUnixStream
andUnixDatagram
to send and receive ancillary data. For example file descriptors.Unresolved Questions
SocketAncillary
struct before stabilization to ensure it is compatible with all OSes.fuchsia
,haiku
,illumos
,ios
,macos
andsolaris
does not haveMSG_CMSG_CLOEXEC
constant inlibc
fuchsia
anduclibc
(x86_64
) does not havecmsghdr
struct inlibc
libc
does not haveucred
struct for the target OSemscripten
, butemscripten
has this struct in the standard C libraryImplementation history
PR #69864, #82589, #83374
The text was updated successfully, but these errors were encountered: