From 76248f0e44addc0287a0090fb8d8a6b5c91c6c73 Mon Sep 17 00:00:00 2001 From: Dave Bakker Date: Thu, 14 Sep 2023 10:47:39 +0200 Subject: [PATCH 1/6] Allow backlog size to be set before initial listen --- .../wasi-sockets-tests/src/bin/tcp_v4.rs | 2 + .../wasi-sockets-tests/src/bin/tcp_v6.rs | 2 + crates/wasi/src/preview2/host/tcp.rs | 39 +++++++++++++++---- crates/wasi/src/preview2/tcp.rs | 22 +++++++++++ 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs b/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs index c0eb0afdc473..c5974ce8dd1b 100644 --- a/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs +++ b/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs @@ -23,6 +23,8 @@ fn main() { let sock = tcp_create_socket::create_tcp_socket(IpAddressFamily::Ipv4).unwrap(); + tcp::set_listen_backlog_size(sock, 32).unwrap(); + let addr = IpSocketAddress::Ipv4(Ipv4SocketAddress { port: 0, // use any free port address: (127, 0, 0, 1), // localhost diff --git a/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs b/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs index 36e465f0fac7..0823f376f092 100644 --- a/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs +++ b/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs @@ -23,6 +23,8 @@ fn main() { let sock = tcp_create_socket::create_tcp_socket(IpAddressFamily::Ipv6).unwrap(); + tcp::set_listen_backlog_size(sock, 32).unwrap(); + let addr = IpSocketAddress::Ipv6(Ipv6SocketAddress { port: 0, // use any free port address: (0, 0, 0, 0, 0, 0, 0, 1), // localhost diff --git a/crates/wasi/src/preview2/host/tcp.rs b/crates/wasi/src/preview2/host/tcp.rs index acdca7ce018c..2f3b8c7c2fa8 100644 --- a/crates/wasi/src/preview2/host/tcp.rs +++ b/crates/wasi/src/preview2/host/tcp.rs @@ -166,7 +166,7 @@ impl tcp::Host for T { socket .tcp_socket() .as_socketlike_view::() - .listen(None)?; + .listen(Some(socket.listen_backlog_size))?; socket.tcp_state = HostTcpState::ListenStarted; @@ -303,16 +303,39 @@ impl tcp::Host for T { this: tcp::TcpSocket, value: u64, ) -> Result<(), network::Error> { - let table = self.table(); - let socket = table.get_tcp_socket(this)?; + const MIN_BACKLOG: i32 = 1; + const MAX_BACKLOG: i32 = i32::MAX; // OS'es will most likely limit it down even further. + + let table = self.table_mut(); + let socket = table.get_tcp_socket_mut(this)?; + + // Silently clamp backlog size. This is OK for us to do, because operating systems do this too. + let value = value.try_into().unwrap_or(i32::MAX).clamp(MIN_BACKLOG, MAX_BACKLOG); match socket.tcp_state { - HostTcpState::Listening => {} - _ => return Err(ErrorCode::NotInProgress.into()), - } + HostTcpState::Default + | HostTcpState::BindStarted + | HostTcpState::Bound => { + // Socket not listening yet. Stash value for first invocation to `listen`. + socket.listen_backlog_size = value; - let value = value.try_into().map_err(|_| ErrorCode::OutOfMemory)?; - Ok(rustix::net::listen(socket.tcp_socket(), value)?) + Ok(()) + }, + HostTcpState::Listening => { + // Try to update the backlog by calling `listen` again. + // Not all platforms support this. We'll only update our own value if the OS supports changing the backlog size after the fact. + + rustix::net::listen(socket.tcp_socket(), value).map_err(|_| ErrorCode::AlreadyListening)?; + + socket.listen_backlog_size = value; + + Ok(()) + }, + HostTcpState::Connected => Err(ErrorCode::AlreadyConnected.into()), + HostTcpState::Connecting + | HostTcpState::ConnectReady + | HostTcpState::ListenStarted => Err(ErrorCode::ConcurrencyConflict.into()), + } } fn keep_alive(&mut self, this: tcp::TcpSocket) -> Result { diff --git a/crates/wasi/src/preview2/tcp.rs b/crates/wasi/src/preview2/tcp.rs index a563a6e09b3b..a9c79f1b0776 100644 --- a/crates/wasi/src/preview2/tcp.rs +++ b/crates/wasi/src/preview2/tcp.rs @@ -51,6 +51,9 @@ pub(crate) struct HostTcpSocket { /// The current state in the bind/listen/accept/connect progression. pub(crate) tcp_state: HostTcpState, + + /// The desired listen queue size. + pub(crate) listen_backlog_size: i32, } /// The inner reference-counted state of a `HostTcpSocket`. @@ -59,6 +62,23 @@ pub(crate) struct HostTcpSocketInner { } impl HostTcpSocket { + + // The following DEFAULT_BACKLOG logic is from + // + // at revision defa2456246a8272ceace9c1cdccdf2e4c36175e. + + // The 3DS doesn't support a big connection backlog. Sometimes + // it allows up to about 37, but other times it doesn't even + // accept 32. There may be a global limitation causing this. + #[cfg(target_os = "horizon")] + const DEFAULT_BACKLOG: i32 = 20; + + // The default for all other platforms + #[cfg(not(target_os = "horizon"))] + const DEFAULT_BACKLOG: i32 = 128; + + + /// Create a new socket in the given family. pub fn new(family: AddressFamily) -> io::Result { // Create a new host socket and set it to non-blocking, which is needed @@ -77,6 +97,7 @@ impl HostTcpSocket { tcp_socket: tokio_tcp_socket, }), tcp_state: HostTcpState::Default, + listen_backlog_size: Self::DEFAULT_BACKLOG, }) } @@ -98,6 +119,7 @@ impl HostTcpSocket { tcp_socket: tokio_tcp_socket, }), tcp_state: HostTcpState::Default, + listen_backlog_size: Self::DEFAULT_BACKLOG, }) } From efd87fed70d04c5c92aed34ce3c5e886fd006fff Mon Sep 17 00:00:00 2001 From: Dave Bakker Date: Thu, 14 Sep 2023 11:06:38 +0200 Subject: [PATCH 2/6] Move set_listen_backlog_size into example_body --- crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs | 2 -- crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs | 2 -- crates/test-programs/wasi-sockets-tests/src/lib.rs | 2 ++ 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs b/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs index 38341c285c33..2388cb074afe 100644 --- a/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs +++ b/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v4.rs @@ -9,8 +9,6 @@ fn main() { let sock = tcp_create_socket::create_tcp_socket(IpAddressFamily::Ipv4).unwrap(); - tcp::set_listen_backlog_size(sock, 32).unwrap(); - let addr = IpSocketAddress::Ipv4(Ipv4SocketAddress { port: 0, // use any free port address: (127, 0, 0, 1), // localhost diff --git a/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs b/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs index 5a4812ef3256..b5ff8358cc09 100644 --- a/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs +++ b/crates/test-programs/wasi-sockets-tests/src/bin/tcp_v6.rs @@ -9,8 +9,6 @@ fn main() { let sock = tcp_create_socket::create_tcp_socket(IpAddressFamily::Ipv6).unwrap(); - tcp::set_listen_backlog_size(sock, 32).unwrap(); - let addr = IpSocketAddress::Ipv6(Ipv6SocketAddress { port: 0, // use any free port address: (0, 0, 0, 0, 0, 0, 0, 1), // localhost diff --git a/crates/test-programs/wasi-sockets-tests/src/lib.rs b/crates/test-programs/wasi-sockets-tests/src/lib.rs index fb06654c313a..04aa400f2837 100644 --- a/crates/test-programs/wasi-sockets-tests/src/lib.rs +++ b/crates/test-programs/wasi-sockets-tests/src/lib.rs @@ -65,6 +65,8 @@ pub fn example_body(net: tcp::Network, sock: tcp::TcpSocket, family: network::Ip let sub = tcp::subscribe(sock); + tcp::set_listen_backlog_size(sock, 32).unwrap(); + tcp::start_listen(sock).unwrap(); wait(sub); tcp::finish_listen(sock).unwrap(); From cc705219f113b259e366799f0150f7e26ec01cb4 Mon Sep 17 00:00:00 2001 From: Dave Bakker Date: Thu, 14 Sep 2023 11:48:19 +0200 Subject: [PATCH 3/6] Format code --- crates/wasi/src/preview2/host/tcp.rs | 22 ++++++++++++---------- crates/wasi/src/preview2/tcp.rs | 5 +---- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/crates/wasi/src/preview2/host/tcp.rs b/crates/wasi/src/preview2/host/tcp.rs index c0cdf598c7b2..c88b89ec19ff 100644 --- a/crates/wasi/src/preview2/host/tcp.rs +++ b/crates/wasi/src/preview2/host/tcp.rs @@ -309,31 +309,33 @@ impl tcp::Host for T { let socket = table.get_tcp_socket_mut(this)?; // Silently clamp backlog size. This is OK for us to do, because operating systems do this too. - let value = value.try_into().unwrap_or(i32::MAX).clamp(MIN_BACKLOG, MAX_BACKLOG); + let value = value + .try_into() + .unwrap_or(i32::MAX) + .clamp(MIN_BACKLOG, MAX_BACKLOG); match socket.tcp_state { - HostTcpState::Default - | HostTcpState::BindStarted - | HostTcpState::Bound => { + HostTcpState::Default | HostTcpState::BindStarted | HostTcpState::Bound => { // Socket not listening yet. Stash value for first invocation to `listen`. socket.listen_backlog_size = value; Ok(()) - }, + } HostTcpState::Listening => { // Try to update the backlog by calling `listen` again. // Not all platforms support this. We'll only update our own value if the OS supports changing the backlog size after the fact. - rustix::net::listen(socket.tcp_socket(), value).map_err(|_| ErrorCode::AlreadyListening)?; + rustix::net::listen(socket.tcp_socket(), value) + .map_err(|_| ErrorCode::AlreadyListening)?; socket.listen_backlog_size = value; Ok(()) - }, + } HostTcpState::Connected => Err(ErrorCode::AlreadyConnected.into()), - HostTcpState::Connecting - | HostTcpState::ConnectReady - | HostTcpState::ListenStarted => Err(ErrorCode::ConcurrencyConflict.into()), + HostTcpState::Connecting | HostTcpState::ConnectReady | HostTcpState::ListenStarted => { + Err(ErrorCode::ConcurrencyConflict.into()) + } } } diff --git a/crates/wasi/src/preview2/tcp.rs b/crates/wasi/src/preview2/tcp.rs index e186a903efc4..a2c97933ce64 100644 --- a/crates/wasi/src/preview2/tcp.rs +++ b/crates/wasi/src/preview2/tcp.rs @@ -50,7 +50,7 @@ pub(crate) struct HostTcpSocket { /// The current state in the bind/listen/accept/connect progression. pub(crate) tcp_state: HostTcpState, - + /// The desired listen queue size. pub(crate) listen_backlog_size: i32, } @@ -217,7 +217,6 @@ impl HostOutputStream for TcpWriteStream { } impl HostTcpSocket { - // The following DEFAULT_BACKLOG logic is from // // at revision defa2456246a8272ceace9c1cdccdf2e4c36175e. @@ -232,8 +231,6 @@ impl HostTcpSocket { #[cfg(not(target_os = "horizon"))] const DEFAULT_BACKLOG: i32 = 128; - - /// Create a new socket in the given family. pub fn new(family: AddressFamily) -> io::Result { // Create a new host socket and set it to non-blocking, which is needed From 879823df046db935bf860df054aa18b5e951dc34 Mon Sep 17 00:00:00 2001 From: Dave Bakker Date: Sat, 16 Sep 2023 22:21:19 +0200 Subject: [PATCH 4/6] Let cap_net_ext handle the default value. --- crates/wasi/src/preview2/host/tcp.rs | 6 +++--- crates/wasi/src/preview2/tcp.rs | 20 +++----------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/crates/wasi/src/preview2/host/tcp.rs b/crates/wasi/src/preview2/host/tcp.rs index c88b89ec19ff..c9f84f20c2f4 100644 --- a/crates/wasi/src/preview2/host/tcp.rs +++ b/crates/wasi/src/preview2/host/tcp.rs @@ -161,7 +161,7 @@ impl tcp::Host for T { socket .tcp_socket() .as_socketlike_view::() - .listen(Some(socket.listen_backlog_size))?; + .listen(socket.listen_backlog_size)?; socket.tcp_state = HostTcpState::ListenStarted; @@ -317,7 +317,7 @@ impl tcp::Host for T { match socket.tcp_state { HostTcpState::Default | HostTcpState::BindStarted | HostTcpState::Bound => { // Socket not listening yet. Stash value for first invocation to `listen`. - socket.listen_backlog_size = value; + socket.listen_backlog_size = Some(value); Ok(()) } @@ -328,7 +328,7 @@ impl tcp::Host for T { rustix::net::listen(socket.tcp_socket(), value) .map_err(|_| ErrorCode::AlreadyListening)?; - socket.listen_backlog_size = value; + socket.listen_backlog_size = Some(value); Ok(()) } diff --git a/crates/wasi/src/preview2/tcp.rs b/crates/wasi/src/preview2/tcp.rs index a2c97933ce64..5052174dfb9d 100644 --- a/crates/wasi/src/preview2/tcp.rs +++ b/crates/wasi/src/preview2/tcp.rs @@ -51,8 +51,8 @@ pub(crate) struct HostTcpSocket { /// The current state in the bind/listen/accept/connect progression. pub(crate) tcp_state: HostTcpState, - /// The desired listen queue size. - pub(crate) listen_backlog_size: i32, + /// The desired listen queue size. Set to None to use the system's default. + pub(crate) listen_backlog_size: Option, } pub(crate) struct TcpReadStream { @@ -217,20 +217,6 @@ impl HostOutputStream for TcpWriteStream { } impl HostTcpSocket { - // The following DEFAULT_BACKLOG logic is from - // - // at revision defa2456246a8272ceace9c1cdccdf2e4c36175e. - - // The 3DS doesn't support a big connection backlog. Sometimes - // it allows up to about 37, but other times it doesn't even - // accept 32. There may be a global limitation causing this. - #[cfg(target_os = "horizon")] - const DEFAULT_BACKLOG: i32 = 20; - - // The default for all other platforms - #[cfg(not(target_os = "horizon"))] - const DEFAULT_BACKLOG: i32 = 128; - /// Create a new socket in the given family. pub fn new(family: AddressFamily) -> io::Result { // Create a new host socket and set it to non-blocking, which is needed @@ -255,7 +241,7 @@ impl HostTcpSocket { Ok(Self { inner: Arc::new(stream), tcp_state: HostTcpState::Default, - listen_backlog_size: Self::DEFAULT_BACKLOG, + listen_backlog_size: None, }) } From 4b3fe853e6ea2445b079c0556ae3d7f9a8ca39c6 Mon Sep 17 00:00:00 2001 From: Dave Bakker Date: Tue, 19 Sep 2023 19:57:16 +0200 Subject: [PATCH 5/6] retrigger checks From abaabd23a7c035e05a5691598a6c9b0d64eb076b Mon Sep 17 00:00:00 2001 From: Dave Bakker Date: Tue, 3 Oct 2023 18:28:22 +0200 Subject: [PATCH 6/6] Got lost while pulling in changes from main. --- crates/wasi/src/preview2/host/tcp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasi/src/preview2/host/tcp.rs b/crates/wasi/src/preview2/host/tcp.rs index 0e2559cb7ec7..479ae44dc63c 100644 --- a/crates/wasi/src/preview2/host/tcp.rs +++ b/crates/wasi/src/preview2/host/tcp.rs @@ -159,7 +159,7 @@ impl crate::preview2::host::tcp::tcp::HostTcpSocket for T { socket .tcp_socket() .as_socketlike_view::() - .listen(None)?; + .listen(socket.listen_backlog_size)?; socket.tcp_state = TcpState::ListenStarted;