-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Implement sock_accept
and basic networking
#3711
Implement sock_accept
and basic networking
#3711
Conversation
Subscribe to Label Actioncc @kubkon
This issue or pull request has been labeled: "wasi"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
598194d
to
e6121ae
Compare
998f051
to
e16a969
Compare
@haraldh Is this still a draft? |
0f85778
to
60109b5
Compare
sock_accept
and basic networkingsock_accept
and basic networking
@npmccallum now it's ready for a review :) |
60109b5
to
fb76e65
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Just one potential question:
UnixStream(cap_std::os::unix::net::UnixStream), | ||
#[cfg(unix)] | ||
UnixListener(cap_std::os::unix::net::UnixListener), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option here would be to make Socket
be a simple struct that holds an OwnedFd
, and uses rustix::net
calls to implement the functions rather than cap_std::net
/std::net
. Rustix even supports the socket APIs on Windows, so this ought to work. And you could probably unify the "stream" vs. "listener" implementations below. That said, either way seems fine for now, so feel free to make a judgement call here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I didn't want to change all the other parts too much.
What you are proposing can be done in a separate PR, which will change much more, I guess.
With the Socket
abstraction, I think I didn't make the "user" facing API incompatible with your proposal.
fb76e65
to
ba5fa61
Compare
Changed one more thing to make async fn get_fdflags(&self) -> Result<FdFlags, Error> {
- Err(Error::badf())
+ let fdflags = self.0.as_filelike().get_fd_flags()?;
+ Ok(from_sysif_fdflags(fdflags))
} |
5cc7139
to
0d4b911
Compare
With the addition of `sock_accept()` in `wasi-0.11.0`, wasmtime can now implement basic networking for pre-opened sockets. For Windows `AsHandle` was replaced with `AsRawHandleOrSocket` to cope with the duality of Handles and Sockets. For Unix a `wasi_cap_std_sync::net::Socket` enum was created to handle the {Tcp,Unix}{Listener,Stream} more efficiently in `WasiCtxBuilder::preopened_socket()`. The addition of that many `WasiFile` implementors was mainly necessary, because of the difference in the `num_ready_bytes()` function. A known issue is Windows now busy polling on sockets, because except for `stdin`, nothing is querying the status of windows handles/sockets. Another know issue on Windows, is that there is no crate providing support for `fcntl(fd, F_GETFL, 0)` on a socket. Signed-off-by: Harald Hoyer <harald@profian.com>
0d4b911
to
1b3ce21
Compare
wasi: implement `sock_accept` and enable networking With the addition of `sock_accept()` to snapshot1, simple networking via a passed `TcpListener` is possible. This PR implements the basics to make a simple server work. See also: * [wasmtime tracking issue](bytecodealliance/wasmtime#3730) * [wasmtime PR](bytecodealliance/wasmtime#3711) TODO: * [ ] Discussion of `SocketAddr` return value for `::accept()` ```rust Ok(( TcpStream::from_inner(unsafe { Socket::from_raw_fd(fd as _) }), // WASI has no concept of SocketAddr yet // return an unspecified IPv4Addr SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), 0), )) ```
wasi: implement `sock_accept` and enable networking With the addition of `sock_accept()` to snapshot1, simple networking via a passed `TcpListener` is possible. This PR implements the basics to make a simple server work. See also: * [wasmtime tracking issue](bytecodealliance/wasmtime#3730) * [wasmtime PR](bytecodealliance/wasmtime#3711) TODO: * [ ] Discussion of `SocketAddr` return value for `::accept()` ```rust Ok(( TcpStream::from_inner(unsafe { Socket::from_raw_fd(fd as _) }), // WASI has no concept of SocketAddr yet // return an unspecified IPv4Addr SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), 0), )) ```
All right.. rust-lang PR merged 🥳 |
@sunfishcode Is this ready for merge? |
Yes, looks good to me! |
With * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Signed-off-by: Harald Hoyer <harald@profian.com>
With * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Signed-off-by: Harald Hoyer <harald@profian.com>
With * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Signed-off-by: Harald Hoyer <harald@profian.com>
With * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Signed-off-by: Harald Hoyer <harald@profian.com>
With * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Signed-off-by: Harald Hoyer <harald@profian.com>
Based on tokio-rs#1395 And with * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Co-authored-by: Thomas de Zeeuw <thomasdezeeuw@gmail.com> Signed-off-by: Harald Hoyer <harald@profian.com>
Based on tokio-rs#1395 And with * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Co-authored-by: Thomas de Zeeuw <thomasdezeeuw@gmail.com> Signed-off-by: Harald Hoyer <harald@profian.com>
Based on #1395 And with * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Co-authored-by: Thomas de Zeeuw <thomasdezeeuw@gmail.com> Signed-off-by: Harald Hoyer <harald@profian.com>
Based on tokio-rs#1395 And with * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Co-authored-by: Thomas de Zeeuw <thomasdezeeuw@gmail.com> Signed-off-by: Harald Hoyer <harald@profian.com>
Based on tokio-rs#1395 And with * bytecodealliance/wasmtime#3711 * rust-lang/rust#93158 merged, mio can have limited support for networking for the `wasm32-wasi` target. Co-authored-by: Thomas de Zeeuw <thomasdezeeuw@gmail.com> Signed-off-by: Harald Hoyer <harald@profian.com>
Together with rust-lang/rust#93158 this enables basic networking in rust.
See #3730 for the tracking issue.
With the addition of
sock_accept()
inwasi-0.11.0
, wasmtime can now implement basic networking for pre-opened sockets.For Windows
AsHandle
was replaced withAsRawHandleOrSocket
to cope with the duality of Handles and Sockets.For Unix a
wasi_cap_std_sync::net::Socket
enum was created to handle the {Tcp,Unix}{Listener,Stream} more efficiently inWasiCtxBuilder::preopened_socket()
.The addition of that many
WasiFile
implementors was mainly necessary, because of the difference in thenum_ready_bytes()
function.A known issue is Windows now busy polling on sockets, because except for
stdin
, nothing is querying the status of windows handles/sockets.Another know issue on Windows, is that there is no crate providing support for
fcntl(fd, F_GETFL, 0)
for sockets, soWasiFile::get_fdflags()
always returns 0.Todo