Conversation
3d9504d to
a126e2e
Compare
|
Having tried this out for a bit, this API is a little annoying because we also need unix equivalents to |
5f51026 to
485eee8
Compare
|
I've removed the FD passing bits for now and put them at https://github.com/talex5/eio/tree/fd-passing, which I'll submit separately once this is merged. So this PR is just adding a skeleton |
lib_eio/net.ml
Outdated
|
|
||
| type datagram = [ | ||
| | `Udp of Ipaddr.v4v6 * int | ||
| | `Unix_datagram of string |
There was a problem hiding this comment.
I'm still digesting this diff, but not sure why Unix_datagram is named like this in the portable core. We have unix domain sockets, and potentially Windows named pipes, so should this be a little more abstract?
There was a problem hiding this comment.
It could be. The unix(7) man-page says "The AF_UNIX (also known as AF_LOCAL) socket family ...", so perhaps Local_datagram would work?
I guess the question is whether it's useful to use Unix-domain sockets interchangeably with Windows named pipes. If so, they should share a name. If not, we can add another constructor for those.
There was a problem hiding this comment.
There's a somewhat orthogonal set of properties that each expose. Windows named pipes are unidirectional streams I think. Is the common property here that some things have a local name, and some are datagrams or streams.
There was a problem hiding this comment.
According to https://learn.microsoft.com/en-us/windows/win32/ipc/pipe-names, a pipe name is in the format \\ServerName\pipe\PipeName. If a Linux process wanted to connect to a Windows named pipe on another machine, that would be quite different to connecting to a Unix socket with the same name, so I think they should be separate. Probably something like Windows_pipe (server, local_name).
We could combine Unix and Unix_datagram I think. The only thing that distinguishes between them is getnameinfo to set the NI_DGRAM flag, but that isn't useful for Unix-domain sockets anyway.
I wonder if we should separate out Linux abstract socket addresses too. Unix "/tmp/sock" refers to a file, but Unix "\000/tmp/sock" doesn't. Or perhaps we shouldn't have a Unix network address at all, and instead add Path.bind and Path.connect. They are part of the mount namespace rather than the network namespace.
|
I've pushed a couple of minor changes:
If there are no objections I'll merge this tomorrow, so I can then submit the FD-passing PR. |
- Add `Eio_unix.Net` module and move network-related items from
`Eio_unix` to there.
- Replace `Eio_unix.socketpair` with
`Eio_unix.Net.socketpair_{stream,datagram}`. Previously, it took a
type argument (stream or datagram), but always returned a `stream_socket`.
- Add `Eio_unix.Net.import_socket_datagram`.
- Move sockaddr conversions to `eio.unix` so they can be shared.
- Implement `getnameinfo` in `eio.unix` to share it by default.
- `Eio_linux.Low_level.send_msg` no longer throws away the number of
bytes sent.
- Add `close` to all socket types.
- The destination in `send` is now optional.
- `send` now takes a iovec, not just a single buffer.
The stream API allows this and we tested it for datagrams using
that API due to the previous type confusion.
- Allow `Unix` socket paths as datagram addresses too.
- Add `send_msg` and `recv_msg` stubs to Eio_posix.
This avoids converting to bytes, and will be useful to add FD passing
later.
|
One implication of this change is that |
|
(For reference, this is the diff I applied to bring other libraries uptodate with this change avsm/eeww@dfdd8f2) |
|
Good point. At the moment, |
|
It would be cleaner to just pass a Flow.two_way I think, and leave closing to the connection handler. |
Sockets created by `accept_fork` can't be closed manually currently, so revert the part of ocaml-multicore#516 that said they could. We might want to revisit this later, for now put things back how they were. Reported by Anil Madhavapeddy.
Sockets created by `accept_fork` can't be closed manually currently, so revert the part of ocaml-multicore#516 that said they could. We might want to revisit this later, for now put things back how they were. Reported by Anil Madhavapeddy.
Sockets created by `accept_fork` can't be closed manually currently, so revert the part of ocaml-multicore#516 that said they could. We might want to revisit this later, but for now put things back how they were. Reported by Anil Madhavapeddy.
Sockets created by `accept_fork` can't be closed manually currently, so revert the part of ocaml-multicore#516 that said they could. We might want to revisit this later, but for now put things back how they were. Reported by Anil Madhavapeddy.
Sockets created by `accept_fork` can't be closed manually currently, so revert the part of ocaml-multicore#516 that said they could. We might want to revisit this later, but for now put things back how they were. Reported by Anil Madhavapeddy.
Sockets created by `accept_fork` can't be closed manually currently, so revert the part of ocaml-multicore#516 that said they could. We might want to revisit this later, but for now put things back how they were. Reported by Anil Madhavapeddy.
Sockets created by `accept_fork` can't be closed manually currently, so revert the part of ocaml-multicore#516 that said they could. We might want to revisit this later, but for now put things back how they were. Reported by Anil Madhavapeddy.
Sockets created by `accept_fork` can't be closed manually currently, so revert the part of ocaml-multicore#516 that said they could. We might want to revisit this later, but for now put things back how they were. Reported by Anil Madhavapeddy.
Eio_unix.Netmodule and move network-related items fromEio_unixto there.Eio_unix.socketpairwithEio_unix.Net.socketpair_{stream,datagram}. Previously, it took a type argument (stream or datagram), but always returned astream_socket.eio.unixso they can be shared.getnameinfoineio.unixto share it by default.Eio_linux.Low_level.send_msgno longer throws away the number of bytes sent.closeto all socket types.sendis now optional, for unconnected datagram sockets.sendnow takes a iovec, not just a single buffer. The stream API allows this and we tested it for datagrams using that API due to the previous type confusion.Unix_datagramsocket address type for completeness.send_msgandrecv_msgstubs to Eio_posix. This avoids converting to bytes, and will be useful to add FD passing later.I was planning to include FD passing in this FD, but it got too long so I've split that out for later.
Fixes #342.