-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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 SyscallConn for minor protocols #28144
Comments
Thank you for sending the proposal and moving things forward. Indeed, if the required system calls and data structures don't match the ones used by the
I'm a little bit confused about this. If this proposal were to be accepted, what do we need the registry for? And if we would have it, what kind of mappings would it contain? The registry was going to be used by package net to implement I know it's pretty late in the cycle, but I would very much like to find a resolution for #15021 and similar issues, for Go 1.12. I am also willing to do whatever work might be necessary. |
It should not be a mandatory option, but we still see issues like "the net package doesn't provide a socket something-something I really need (or want)", perhaps there might be an option for the sake of convenience in the x/sys repository.
That's a good question. Sorry, I don't have any concrete idea yet. I enjoyed an idea of replacing the existing unix.Sockaddr interface and system call wrapper functions with something using generic facilities (interface and parametric polymorphism are orthogonal, sounds neat), but the lack of operational experiences (e.g., how to manage the distribution of contracts; along with the per-package basis under cmd/go control or having a dedicated package?) prevented me from moving forward. So, I mean, this issue smashes the necessity of #15021 but there perhaps might be a reason for keeping #15021 open. |
How is the proposed net.NewSyscallConn function different from the net.FileConn function? |
Currently, net.FileConn and net.FilePacketConn only work if the file descriptor refers to a socket that the net package knows how deal with. It would be nice if we could use other sockets through the runtime network poller. Using a non-blocking os.File is generally not sufficient, because in certain cases, we can't make the right system calls while making use of the poller. The proposed net.SyscallConn would help with this issue. Callers could wrap it, and use its attached syscall.RawConn to make system calls such as recvfrom(2) for I/O, while the underlying file descriptor is registered with the poller. |
Can you clarify why a non-blocking Also, can you clarify what you mea by "a socket that the net package knows how to deal with?" Am I correct in thinking that this is all about the runtime poller? We've been working toward having all non-blocking descriptors use the poller. It's not clear to me why we need to introduce a new type here. There may be some clear reason, but I don't see it. |
Simply the series of File{Conn,PacketConn,Listener} is designed for Unix variants and doesn't work well on Windows. There was (is?) a few attempts to implement the series of File API on Windows but didn't succeed. On Windows, the duplication and inheritance of socket handles behave a bit different from Unix variants and the attempts failed to pass the existing tests, IIRC. NewSyscallConn doesn't do socket descriptor or handle duplication. |
To allow users to implement protocol- or platform-dependent system calls like http://man7.org/linux/man-pages/man3/sctp_send.3.html. |
For instance, recvmsg on a netlink socket, with the help of the poller. Perhaps I am missing something, but I don't see how that could be done today.
For example, a netlink or SCTP socket. The net package does not support such sockets, and net.FileConn and net.FilePacketConn fail when passed such a file descriptor.
Yes, pretty much. It is about being able to execute system calls in the context of a syscall.RawConn, on a file descriptor that has been registered with the poller. As far as I can tell, this is currently not possible. |
Can we make |
The idea would be to use |
How do they fail? Sorry for asking dumb questions, but this code is already really complicated, and this proposal does not have the background information explaining why we need to take this approach. |
They fail like so: https://github.com/golang/go/blob/master/src/net/file_unix.go#L41-L53 This is called by both the FileConn and FilePacketConn code paths. |
Sorry, I'm not a Windows user and have no Windows stuff; see https://go-review.googlesource.com/c/go/+/8683 |
Alternative problem statement: there is currently no way to obtain, for an arbitrary file descriptor, an implementation of syscall.RawConn such that the associated file descriptor is registered with the poller. |
That should be another proposal or issue; please file a new issue if you can address the issue. |
What if we change Can somebody share a small piece of code that ought to work but doesn't, using |
This example uses https://play.golang.org/p/HQHSKBfJIl2 I'm not convinced that this code ought to work. Assume One option is to return an implementation of An alternative is to make All in all, I believe the proposed |
The new API turns an The first 4 methods are The last method As said before by @acln0
I don't think this is an "alternative problem". This is the problem that the proposal try to solve. With Go1.11, you can use an arbitrary fd to obtain an |
Sorry. Language barrier. I wanted "alternative problem statement" to mean "a different way of phrasing the problem". |
Sorry for the long conversation. I am still a little confused. The net package looks for specific socket families so that it can be sure it knows the Go type to return from net.Conn's LocalAddr and RemoteAddr methods. What should it return if it doesn't know the family? If you don't need LocalAddr and RemoteAddr, all that's left in net.Conn is Read, Write, Close, SetDeadline, SetReadDeadline, and SetWriteDeadline. Those are all now present in os.File. So if you don't need the addresses, it seems like you can now use an os.File directly. If you do need the addresses, how should they behave? Also, if we do need some way to provide hooks into LocalAddr and RemoteAddr, isn't that #15021? |
My understanding is that the fundamental problem which both #15021 and this bug try to solve is the following: we would like to perform system calls on socket file descriptors with the help of the runtime poller, and these system calls are not limited to Read and Write (which is why an os.File is not enough). For example, this netlink library calls recvmsg: https://github.com/mdlayher/netlink/blob/master/conn_linux.go#L140. When timeouts are needed for such calls, solutions such as vishvananda/netlink@17ea11b appear, which seem unpleasant and almost wrong. Go has a poller built-in. We should be able to make use of it. The current solution to the problem of "I want to use the poller when making a system call on the file descriptor of a socket" is to use syscall.RawConn, which is implemented in package net for the types of sockets the net package knows about. But there is no implementation of syscall.RawConn (one which makes use of the runtime poller) for socket types the net package does not know about directly. #15021 argues that the solution is to change the net package to offer some mechanism to register hooks into LocalAddr and RemoteAddr. This would answer the question of "What should it return if it doesn't know the family?". Using this mechanism, the net package would wrap sockets it doesn't (directly) know about in Conn or PacketConn implementations, via FileConn or FilePacketConn. Those implementations would use the network poller for the usual system calls, and would also implement syscall.RawConn for completeness. A demo of this was presented in https://go-review.googlesource.com/c/go/+/45790/, which I implemented in https://go-review.googlesource.com/c/go/+/136595. The approach in my CL was rightfully deemed ineffective, and this bug was filed instead, as a better solution for the problem. The point is, when callers want to make a specific system call on the file descriptor, they're going to want to use syscall.RawConn anyway, instead of the net.Conn or net.PacketConn methods. Instead of encumbering the net and syscall packages with managing hooks for addresses, this proposal argues that the net package should be changed to offer what I would call a minimal syscall.RawConn implementation. Managing addresses and choosing system calls remains entirely up to the caller. The caller can now implement net.Conn or net.PacketConn effectively, if they need to, because the new net.SyscallConn offers a syscall.RawConn which uses the poller. In conclusion, I think #15021 doesn't focus on the correct fundamental issue, because it gets bogged down in the address problem which your message alludes to. I think the addresses are a red herring. The key is syscall.RawConn. What this proposal asks for is a tiny wrapper around a poll.FD, which implements syscall.RawConn, because that is the minimal API to allow callers to implement anything else they would like in terms of it. |
Sorry, there's a lot to digest in your response. If we did submit your CL 136595, would that address the problem? You say it was rightly deemed ineffective but all I see is Mikio saying "looks like your approach cannot support sctp-like multipath transport protocols perfectly." That's not quite "ineffective" to me, just that there are even more obscure corner cases remaining. We really don't want to expose syscall.RawConn more than we already have. |
Hi all, I'm a bit hard to recognize who is "you." Probably it's better to add identifiers for distinction. Looks like this issue now has three streams:
Is my understanding (roughly) correct? If so, I will add a few comments to bridge the gap between (1) and (2)+(3) till the end of this year. |
There is another issue for adding |
This is a supplementary proposal for #15021, also a fix for #10565 and its variants.
If I understand correctly, the requirements described in the issues are simply like the following:
R1: accommodate various protocol- or platform-dependent connection setup, read and write system calls
R2: accommodate various protocol-dependent data, mostly protocol address structures
R3: make a pair of R1 and R2 work together with the runtime-integrated network poller
Unfortunately, #15021 just covers only R2 and R3 even though it prevents #22191 from moving forward. As far as I can see, common SCTP implementations provide a bit quaint system calls for its multipath-capable connection setup and data transmission. In addition, MPTCP and TCP fast open support on Darwin also takes the same approach; see sa_endpoints_t, sae_connid_t and connectx.
To cover all of the requirements, I propose to add a new struct type to the net package for turning user socket descriptors into the runtime-integrated network poller without supporting various system calls and data structures in the standard library.
The struct type has a method returning a syscall.RawConn so that users are able to implement any system call with any protocol-dependent data structure without worrying about the development speed of the standard library and x/net or x/sys repository. In addition, fixing #15021 in the x/sys repository would be a great help and this proposal would work well together with the fix for #15021 on x/sys repository.
The text was updated successfully, but these errors were encountered: