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

proposal: net: add ability to read OOB data without discarding a byte #32465

Closed
APTy opened this issue Jun 6, 2019 · 15 comments
Closed

proposal: net: add ability to read OOB data without discarding a byte #32465

APTy opened this issue Jun 6, 2019 · 15 comments

Comments

@APTy
Copy link

APTy commented Jun 6, 2019

Problem

Currently, (*UnixConn) ReadMsgUnix() reads a message from a unix socket, and, if that socket is a stream, it will always read at least one byte from the stream.

From UnixConn.ReadMsgUnix:

Note that if len(b) == 0 and len(oob) > 0, this function will still
read (and discard) 1 byte from the connection.

For applications that use ReadMsgUnix to receive ancillary, out-of-bound data and don't want to disrupt the data sent in-bound, this isn't a useful API.

Proposed Solution

The underlying syscall, recvmsg(2) allows setting the flags param to MSG_PEEK, which accomplishes this task. Unfortunately, flags are not part of the ReadMsgUnix API. To solve these, I see two options:

  1. Add an API that accepts flags:
func (c *UnixConn) ReadMsgUnixFlags(b, oob []byte, flags int) (...)
  1. Add an API exclusively for peeking:
func (c *UnixConn) PeekMsgUnix(b, oob []byte) (...)

Notes

Since the list of flags is fairly short, option (2) could be acceptable.

           MSG_OOB        process out-of-band data
           MSG_PEEK       peek at incoming message
           MSG_WAITALL    wait for full request or error
@APTy APTy changed the title net/unix: Add ability to peak messages net/unix: Add ability to peek received messages Jun 6, 2019
@APTy
Copy link
Author

APTy commented Jun 6, 2019

BTW, users could implement this today by calling syscall.Recvmsg(sockfd, b, oob, unix.MSG_PEEK) themselves, but there are some internal polling mechanisms that would have to be re-implemented as well, which isn't ideal.

@ianlancetaylor ianlancetaylor changed the title net/unix: Add ability to peek received messages proposal: net: add ability to read OOB data without discarding a byte Jun 6, 2019
@gopherbot gopherbot added this to the Proposal milestone Jun 6, 2019
@mdlayher
Copy link
Member

mdlayher commented Jun 6, 2019

You can use https://golang.org/pkg/net/#UnixConn.SyscallConn to perform a raw read on the file descriptor with whichever syscall you like, AND you'll still have runtime network poller support.

Have you tried this option?

@mdlayher
Copy link
Member

mdlayher commented Jun 6, 2019

An example: https://github.com/mdlayher/raw/blob/a54781e5f38fd990ef58ac93981e1a9767d39693/raw_linux.go#L332

@APTy
Copy link
Author

APTy commented Jun 7, 2019

Oh that's pretty nice. I hadn't seen that one yet - I'll try it out. Thanks!

@rsc
Copy link
Contributor

rsc commented Jun 11, 2019

These details are pretty faded in my memory right now. Can someone explain why the 1 byte is disappearing? As far as I can see, the poller is given the read argument p unmodified, so if you ask to read 0 bytes, the kernel gets asked for 0 bytes. Is the kernel discarding the 1 byte? I don't see where Go is.

@tv42
Copy link

tv42 commented Jun 13, 2019

@rsc As far I know, historically OOB data wasn't sent alone, and one would "flush it out" by sending a dummy byte. I think syscall.Recvmsg explicitly accommodates this old cruft:

// receive at least one normal byte
if sockType != SOCK_DGRAM {
iov.Base = &dummy
iov.SetLen(1)
}

@tv42
Copy link

tv42 commented Jun 13, 2019

To pass file descriptors or credentials over a SOCK_STREAM socket, you must to send or receive at least one byte of nonancillary data in the same sendmsg(2) or recvmsg(2) call.

http://man7.org/linux/man-pages/man7/unix.7.html

@APTy
Copy link
Author

APTy commented Jun 13, 2019

Can someone explain why the 1 byte is disappearing?

This looks like an implementation detail of syscall.Recvmsg:

	msg.Iov = &iov
	msg.Iovlen = 1
	if n, err = recvmsg(fd, &msg, flags); err != nil {
		return
	}

The linux Recvmsg implementation reads at minimum one byte. This is likely because, as @tv42 mentions, ancillary data must be sent (and received) along with other data. Still, if a user wants to only read the ancillary data, without interrupting data in the stream (say, a gRPC interceptor), this is really undesirable behavior.

@tv42
Copy link

tv42 commented Jun 14, 2019

This is likely because, as @tv42 mentions, ancillary data must be sent (and received) along with other data.

Still, if a user wants to only read the ancillary data, without interrupting data in the stream (say, a gRPC interceptor),

Pick one or the other, you're self-contradicting.

You're also asking for behavior that is against a "must" in the relevant man page ("must [...] receive at least one byte of nonancillary data in the same sendmsg(2) or recvmsg(2) call").

Now, it might be possible to let go of this legacy kludge, but not with this api. Changing Recvmsg behavior here would break existing callers that expect it to eat the dummy byte silently.

(Has anyone tested writing/reading OOB data without a dummy byte, with raw syscalls? Linux, macOS, BSDs? If it doesn't work, this whole discussion is moot.)

@APTy
Copy link
Author

APTy commented Jun 14, 2019

To read ancillary data without interrupting the data stream, users can pass the MSG_PEEK flag to recvmsg. This is just a matter of exposing that functionality at the net.UnixConn layer.

@mdlayher
Copy link
Member

I'm believe the correct option at this point is to use the SyscallConn method to perform the appropriate system call with the appropriate flags. I'm not sure it's worth adding stdlib API for this.

@APTy
Copy link
Author

APTy commented Jun 14, 2019

🤷‍♂Eh there isn't always a correct option in situations like this, but I can understand a preference by lib maintainers to reduce API bloat even at the expense of extra functionality in the net/unix lib (especially when there are lower-level alternatives). Thanks for the help all

@rsc
Copy link
Contributor

rsc commented Jun 25, 2019

@APTy, did you try using SyscallConn? Did it work for you?
If so, we should probably take @mdlayher's advice and not add anything,
except maybe a comment on syscall.Recvmsg pointing at SyscallConn.
(We have very little evidence this is a frequent need. As @tv42 points out, most of the time you do want to take the dummy byte with the OOB data.)

@rsc
Copy link
Contributor

rsc commented Aug 27, 2019

There have been no updates since #32465 (comment). Based on that and @mdlayher's suggestion above, this seems like a likely decline.

Leaving open for a week for final comments.

@ianlancetaylor
Copy link
Contributor

Marked this last week as likely decline w/ call for last comments (#32465 (comment)).
Declining now.

@golang golang locked and limited conversation to collaborators Sep 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants