diff --git a/AGENTS.md b/AGENTS.md index 04e5bc5c7b..17ee4884ea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -166,6 +166,19 @@ ocsf_emit!(event); - If you change sandbox infrastructure, ensure the relevant sandbox e2e path succeeds. +## Network Sockets + +- On latency-sensitive TCP streams, disable Nagle's algorithm so small + request/response frames don't stall on delayed ACKs. Use + `openshell_core::net::set_tcp_nodelay_best_effort` on an accepted or + already-connected stream, or `openshell_core::net::connect_tcp_nodelay_best_effort` + when dialing. +- This applies to loopback/localhost TCP too — the delayed-ACK stall is a timer + behavior, not wire latency. +- You should skip it for unix domain sockets (no Nagle). It's not critical for + test-only connections, though using it on any non-UDS TCP stream — tests + included — is fine and preferred. + ## Commits - Always use [Conventional Commits](https://www.conventionalcommits.org/) format for commit messages diff --git a/crates/openshell-cli/src/edge_tunnel.rs b/crates/openshell-cli/src/edge_tunnel.rs index 814e245f3c..e9b1a92668 100644 --- a/crates/openshell-cli/src/edge_tunnel.rs +++ b/crates/openshell-cli/src/edge_tunnel.rs @@ -26,6 +26,7 @@ use futures::stream::{SplitSink, SplitStream}; use futures::{SinkExt, StreamExt}; use miette::{IntoDiagnostic, Result}; +use openshell_core::net::set_tcp_nodelay_best_effort; use std::net::SocketAddr; use std::sync::Arc; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -101,6 +102,7 @@ async fn accept_loop(listener: TcpListener, config: Arc) { match listener.accept().await { Ok((stream, peer)) => { debug!(peer = %peer, "accepted local tunnel connection"); + set_tcp_nodelay_best_effort(&stream); let config = Arc::clone(&config); tokio::spawn(async move { if let Err(e) = handle_connection(stream, &config).await { @@ -174,6 +176,20 @@ async fn open_ws(config: &TunnelConfig) -> Result Some(tcp), + MaybeTlsStream::Rustls(tls) => Some(tls.get_ref().0), + // `MaybeTlsStream` is #[non_exhaustive]; surface any future/unknown + // variant so a silent TCP_NODELAY miss doesn't go unnoticed. + _ => { + debug!("edge tunnel: unrecognized MaybeTlsStream variant; skipping TCP_NODELAY"); + None + } + }; + if let Some(tcp) = tcp { + set_tcp_nodelay_best_effort(tcp); + } + debug!( status = %response.status(), "WebSocket connected to edge" diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 4843475e5d..1e417f3709 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -31,6 +31,7 @@ use miette::{IntoDiagnostic, Result, WrapErr, miette}; use openshell_bootstrap::{ GatewayMetadata, clear_last_sandbox_if_matches, get_gateway_metadata, save_last_sandbox, }; +use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::proto::ProviderProfileCategory; use openshell_core::proto::{ ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, AttachSandboxProviderRequest, @@ -1504,6 +1505,7 @@ pub async fn service_forward_tcp( let (socket, peer) = accepted .into_diagnostic() .wrap_err("failed to accept local forward connection")?; + set_tcp_nodelay_best_effort(&socket); let mut client = client.clone(); let sandbox_id = sandbox_id.clone(); let target_host = target_host.to_string(); diff --git a/crates/openshell-cli/src/tls.rs b/crates/openshell-cli/src/tls.rs index 10df401a5b..2eadafc71a 100644 --- a/crates/openshell-cli/src/tls.rs +++ b/crates/openshell-cli/src/tls.rs @@ -3,6 +3,7 @@ use miette::{IntoDiagnostic, Result, WrapErr}; use openshell_core::auth::EdgeAuthInterceptor; +use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::proto::inference_client::InferenceClient; use openshell_core::proto::open_shell_client::OpenShellClient; use rustls::{ @@ -295,6 +296,7 @@ impl tower::Service for InsecureTlsConnector { let port = uri.port_u16().unwrap_or(443); let addr = format!("{host}:{port}"); let tcp = tokio::net::TcpStream::connect(addr).await?; + set_tcp_nodelay_best_effort(&tcp); let server_name = ServerName::try_from(host)?; let tls_stream = tls_connector.connect(server_name, tcp).await?; Ok(hyper_util::rt::TokioIo::new(tls_stream)) diff --git a/crates/openshell-core/src/net.rs b/crates/openshell-core/src/net.rs index 3f14a397b8..a9bbc23217 100644 --- a/crates/openshell-core/src/net.rs +++ b/crates/openshell-core/src/net.rs @@ -1,17 +1,23 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! Network IP classification utilities shared across `OpenShell` crates. +//! Shared networking utilities for `OpenShell` crates. //! -//! These helpers enforce the always-blocked IP invariant (loopback, link-local, -//! unspecified) and the broader internal-IP classification (adds RFC 1918 and -//! ULA). They are used by: +//! The IP-classification helpers enforce the always-blocked IP invariant +//! (loopback, link-local, unspecified) and the broader internal-IP +//! classification (adds RFC 1918 and ULA). They are used by: //! - The sandbox proxy for runtime SSRF enforcement //! - The mechanistic mapper for proposal filtering //! - The gateway server for defense-in-depth validation on approval +//! +//! The socket tuning helpers ([`set_tcp_nodelay_best_effort`], +//! [`connect_tcp_nodelay_best_effort`]) help avoid the known latency +//! imposed by conflict between Nagle's algorithm and delayed ACK behaviors. +//! use ipnet::{IpNet, Ipv4Net, Ipv6Net}; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; +use tokio::net::TcpStream; /// Check if a hostname is a known cloud metadata hostname that resolves to an /// always-blocked metadata service. @@ -279,6 +285,27 @@ fn is_internal_v4(v4: Ipv4Addr) -> bool { false } +/// Enable `TCP_NODELAY` on a stream, logging (not returning) any failure. +/// +/// Disabling Nagle's algorithm keeps small writes from waiting on delayed ACKs. +/// It's a latency optimization: if it fails the connection still works, just a +/// bit slower, so there is nothing for the caller to act on — we log and move on. +pub fn set_tcp_nodelay_best_effort(stream: &TcpStream) { + if let Err(e) = stream.set_nodelay(true) { + tracing::debug!(error = %e, "failed to set TCP_NODELAY"); + } +} + +/// Connect to `addrs`, then enable `TCP_NODELAY` on a best-effort basis, propagating +/// any errors from `TcpStream::connect`. +/// +/// The returned stream is not *guaranteed* to have `TCP_NODELAY` set. +pub async fn connect_tcp_nodelay_best_effort(addrs: &[SocketAddr]) -> std::io::Result { + let stream = TcpStream::connect(addrs).await?; + set_tcp_nodelay_best_effort(&stream); + Ok(stream) +} + #[cfg(test)] mod tests { use super::*; @@ -699,4 +726,31 @@ mod tests { let v6 = Ipv4Addr::new(100, 64, 0, 1).to_ipv6_mapped(); assert!(is_internal_ip(IpAddr::V6(v6))); } + + // -- tcp_nodelay helpers -- + + #[tokio::test] + async fn set_tcp_nodelay_best_effort_enables_nodelay() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("local addr"); + let stream = TcpStream::connect(addr).await.expect("connect"); + + set_tcp_nodelay_best_effort(&stream); + assert!(stream.nodelay().expect("query TCP_NODELAY")); + } + + #[tokio::test] + async fn connect_tcp_nodelay_best_effort_sets_nodelay() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("local addr"); + + let stream = connect_tcp_nodelay_best_effort(&[addr]) + .await + .expect("connect"); + assert!(stream.nodelay().expect("query TCP_NODELAY")); + } } diff --git a/crates/openshell-sandbox/src/metadata_server.rs b/crates/openshell-sandbox/src/metadata_server.rs index aff1b5418a..2d434ca4ac 100644 --- a/crates/openshell-sandbox/src/metadata_server.rs +++ b/crates/openshell-sandbox/src/metadata_server.rs @@ -12,6 +12,7 @@ //! that needs an instance metadata emulator can implement the trait. use miette::Result; +use openshell_core::net::set_tcp_nodelay_best_effort; use std::future::Future; use std::net::SocketAddr; use std::sync::Arc; @@ -70,6 +71,9 @@ pub async fn run( match listener.accept().await { Ok((stream, _addr)) => { + // Small-request IMDS-style endpoint an agent polls for + // credentials/identity — disable Nagle to avoid delayed-ACK stalls. + set_tcp_nodelay_best_effort(&stream); let handler = handler.clone(); tokio::spawn(async move { if let Err(e) = handle_connection(handler.as_ref(), stream).await { diff --git a/crates/openshell-sdk/src/edge_tunnel.rs b/crates/openshell-sdk/src/edge_tunnel.rs index 5ced5fc354..ac7d8d4e9b 100644 --- a/crates/openshell-sdk/src/edge_tunnel.rs +++ b/crates/openshell-sdk/src/edge_tunnel.rs @@ -26,6 +26,7 @@ use crate::error::{Result, SdkError}; use futures::stream::{SplitSink, SplitStream}; use futures::{SinkExt, StreamExt}; +use openshell_core::net::set_tcp_nodelay_best_effort; use std::net::SocketAddr; use std::sync::Arc; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -100,6 +101,7 @@ async fn accept_loop(listener: TcpListener, config: Arc) { match listener.accept().await { Ok((stream, peer)) => { debug!(peer = %peer, "accepted local tunnel connection"); + set_tcp_nodelay_best_effort(&stream); let config = Arc::clone(&config); tokio::spawn(async move { if let Err(e) = handle_connection(stream, &config).await { @@ -175,6 +177,20 @@ async fn open_ws(config: &TunnelConfig) -> Result Some(tcp), + MaybeTlsStream::Rustls(tls) => Some(tls.get_ref().0), + // `MaybeTlsStream` is #[non_exhaustive]; surface any future/unknown + // variant so a silent TCP_NODELAY miss doesn't go unnoticed. + _ => { + debug!("edge tunnel: unrecognized MaybeTlsStream variant; skipping TCP_NODELAY"); + None + } + }; + if let Some(tcp) = tcp { + set_tcp_nodelay_best_effort(tcp); + } + debug!( status = %response.status(), "WebSocket connected to edge" diff --git a/crates/openshell-sdk/src/transport.rs b/crates/openshell-sdk/src/transport.rs index f5610db6a2..9b25ced28c 100644 --- a/crates/openshell-sdk/src/transport.rs +++ b/crates/openshell-sdk/src/transport.rs @@ -10,6 +10,7 @@ use crate::config::{AuthConfig, ClientConfig}; use crate::edge_tunnel; use crate::error::{Result, SdkError}; +use openshell_core::net::set_tcp_nodelay_best_effort; use rustls::{ client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}, pki_types::{CertificateDer, ServerName, UnixTime}, @@ -232,6 +233,7 @@ impl tower::Service for InsecureTlsConnector { let port = uri.port_u16().unwrap_or(443); let addr = format!("{host}:{port}"); let tcp = tokio::net::TcpStream::connect(addr).await?; + set_tcp_nodelay_best_effort(&tcp); let server_name = ServerName::try_from(host)?; let tls_stream = tls_connector.connect(server_name, tcp).await?; Ok(hyper_util::rt::TokioIo::new(tls_stream)) diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 0d6a4e277f..9afbbb17f4 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -12,6 +12,7 @@ use crate::ServerState; use crate::persistence::{ObjectLabels, ObjectType, WriteCondition, generate_name}; use futures::future; +use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::proto::{ AttachSandboxProviderRequest, AttachSandboxProviderResponse, CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, DeleteSandboxRequest, DeleteSandboxResponse, @@ -1823,6 +1824,9 @@ async fn run_interactive_exec_with_russh( let stream = TcpStream::connect(("127.0.0.1", local_proxy_port)) .await .map_err(|e| Status::internal(format!("failed to connect to ssh proxy: {e}")))?; + // russh client end of the loopback exec bridge — disable Nagle so keystroke + // and PTY tinygrams don't stall on delayed ACKs. + set_tcp_nodelay_best_effort(&stream); let config = Arc::new(exec_ssh_client_config()); let mut client = russh::client::connect_stream(config, stream, SandboxSshClientHandler) @@ -1956,6 +1960,9 @@ async fn start_single_use_ssh_proxy_over_relay( warn!("SSH relay proxy: failed to accept local connection"); return; }; + // Loopback bridge for interactive SSH exec (keystrokes, line-buffered + // PTY output) — disable Nagle so tinygrams don't stall on delayed ACKs. + set_tcp_nodelay_best_effort(&client_conn); let _ = tokio::io::copy_bidirectional(&mut client_conn, &mut relay_stream).await; }); @@ -1998,6 +2005,9 @@ async fn run_exec_with_russh( let stream = TcpStream::connect(("127.0.0.1", local_proxy_port)) .await .map_err(|e| Status::internal(format!("failed to connect to ssh proxy: {e}")))?; + // russh client end of the loopback exec bridge — disable Nagle so keystroke + // and PTY tinygrams don't stall on delayed ACKs. + set_tcp_nodelay_best_effort(&stream); let config = Arc::new(exec_ssh_client_config()); let mut client = russh::client::connect_stream(config, stream, SandboxSshClientHandler) diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index f2967a833d..b73c1e5a99 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -54,6 +54,7 @@ pub mod tracing_bus; mod ws_tunnel; use metrics_exporter_prometheus::PrometheusBuilder; +use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::{ComputeDriverKind, Config, Error, ObjectLabels, Result}; use openshell_supervisor_middleware::MiddlewareRegistry; use std::collections::HashMap; @@ -614,6 +615,8 @@ async fn serve_gateway_listener( } }; + set_tcp_nodelay_best_effort(&stream); + spawn_gateway_connection( stream, addr, diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index 6e9c48220b..37df246e76 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -11,7 +11,10 @@ use crate::upstream_proxy::{self, UpstreamProxyConfig}; use miette::{IntoDiagnostic, Result}; use openshell_core::activity::{ActivitySender, try_record_activity}; use openshell_core::denial::DenialEvent; -use openshell_core::net::{is_always_blocked_ip, is_internal_ip, is_link_local_ip}; +use openshell_core::net::{ + connect_tcp_nodelay_best_effort, is_always_blocked_ip, is_internal_ip, is_link_local_ip, + set_tcp_nodelay_best_effort, +}; use openshell_core::policy::ProxyPolicy; use openshell_core::provider_credentials::ProviderCredentialState; use openshell_core::secrets::{self, SecretResolver, rewrite_header_line_checked}; @@ -316,6 +319,7 @@ impl ProxyHandle { Ok((stream, _addr)) => { consecutive_resource_errors = 0; consecutive_unknown_errors = 0; + set_tcp_nodelay_best_effort(&stream); let opa = opa_engine.clone(); let cache = identity_cache.clone(); let spid = entrypoint_pid.clone(); @@ -3153,13 +3157,13 @@ async fn dial_upstream( } upstream_proxy::ProxyDecision::Direct(direct_addrs) => { Ok(upstream_proxy::PrefixedStream::without_prefix( - TcpStream::connect(&direct_addrs[..]).await?, + connect_tcp_nodelay_best_effort(&direct_addrs[..]).await?, )) } }; } Ok(upstream_proxy::PrefixedStream::without_prefix( - TcpStream::connect(addrs).await?, + connect_tcp_nodelay_best_effort(addrs).await?, )) } @@ -4845,7 +4849,7 @@ async fn handle_forward_proxy( // directly: only TLS (CONNECT) tunnels chain through the corporate // proxy, since plain-HTTP forwarding would need absolute-form requests // rather than a CONNECT tunnel. - let dial_result = TcpStream::connect(addrs.as_slice()).await; + let dial_result = connect_tcp_nodelay_best_effort(addrs.as_slice()).await; let mut upstream = match dial_result { Ok(s) => s, Err(e) => { diff --git a/crates/openshell-supervisor-network/src/upstream_proxy.rs b/crates/openshell-supervisor-network/src/upstream_proxy.rs index 85e57c14c5..411a547759 100644 --- a/crates/openshell-supervisor-network/src/upstream_proxy.rs +++ b/crates/openshell-supervisor-network/src/upstream_proxy.rs @@ -52,6 +52,7 @@ use std::task::{Context, Poll}; use std::time::Duration; use base64::Engine as _; +use openshell_core::net::set_tcp_nodelay_best_effort; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt, ReadBuf}; use tokio::net::TcpStream; use tracing::debug; @@ -798,6 +799,7 @@ async fn connect_via_inner( target: ConnectTarget, ) -> std::io::Result { let mut stream = TcpStream::connect((endpoint.host.as_str(), endpoint.port)).await?; + set_tcp_nodelay_best_effort(&stream); let target = match target { ConnectTarget::Ip(IpAddr::V6(ip)) => format!("[{ip}]:{port}"), diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 7fa6482a99..d13bffc9a0 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -14,6 +14,7 @@ use crate::sandbox; use miette::{IntoDiagnostic, Result}; use nix::pty::{Winsize, openpty}; use nix::unistd::setsid; +use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::policy::SandboxPolicy; use openshell_core::provider_credentials::ProviderCredentialState; use openshell_ocsf::{ @@ -664,13 +665,17 @@ pub async fn connect_in_netns( .await .map_err(|_| std::io::Error::other("netns connect thread panicked"))??; std_stream.set_nonblocking(true)?; - return tokio::net::TcpStream::from_std(std_stream); + let stream = tokio::net::TcpStream::from_std(std_stream)?; + set_tcp_nodelay_best_effort(&stream); + return Ok(stream); } #[cfg(not(target_os = "linux"))] let _ = netns_fd; - tokio::net::TcpStream::connect(addr).await + let stream = tokio::net::TcpStream::connect(addr).await?; + set_tcp_nodelay_best_effort(&stream); + Ok(stream) } #[derive(Clone)] @@ -1349,6 +1354,20 @@ mod tests { use super::*; use std::process::Stdio; + /// Regression test: the direct-tcpip connect path sets `TCP_NODELAY`. + #[tokio::test] + async fn connect_in_netns_sets_tcp_nodelay() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind listener"); + let addr = listener.local_addr().expect("local addr"); + + let stream = connect_in_netns(&addr.to_string(), None) + .await + .expect("connect"); + assert!(stream.nodelay().expect("query TCP_NODELAY")); + } + #[cfg(unix)] fn file_mode(path: &Path) -> u32 { use std::os::unix::fs::PermissionsExt; diff --git a/crates/openshell-supervisor-process/src/supervisor_session.rs b/crates/openshell-supervisor-process/src/supervisor_session.rs index 63fcad4c7a..6cdc9e7d6c 100644 --- a/crates/openshell-supervisor-process/src/supervisor_session.rs +++ b/crates/openshell-supervisor-process/src/supervisor_session.rs @@ -33,6 +33,7 @@ use tokio_stream::StreamExt; use tracing::{debug, warn}; use openshell_core::grpc_client; +use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::transport_errors::is_expected_transport_close_status; const INITIAL_BACKOFF: Duration = Duration::from_secs(1); @@ -745,10 +746,14 @@ async fn connect_tcp_target( .await .map_err(|_| "netns tcp connect thread panicked")??; stream.set_nonblocking(true)?; - return Ok(tokio::net::TcpStream::from_std(stream)?); + let stream = tokio::net::TcpStream::from_std(stream)?; + set_tcp_nodelay_best_effort(&stream); + return Ok(stream); } - Ok(tokio::net::TcpStream::connect((host.as_str(), port)).await?) + let stream = tokio::net::TcpStream::connect((host.as_str(), port)).await?; + set_tcp_nodelay_best_effort(&stream); + Ok(stream) } #[cfg(not(target_os = "linux"))] @@ -757,7 +762,9 @@ async fn connect_tcp_target( port: u16, _netns_fd: Option, ) -> Result> { - Ok(tokio::net::TcpStream::connect((host.as_str(), port)).await?) + let stream = tokio::net::TcpStream::connect((host.as_str(), port)).await?; + set_tcp_nodelay_best_effort(&stream); + Ok(stream) } #[cfg(test)] @@ -799,6 +806,20 @@ mod target_tests { } } + /// Regression test: the TCP relay connect path sets `TCP_NODELAY`. + #[tokio::test] + async fn connect_tcp_target_sets_tcp_nodelay() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind listener"); + let addr = listener.local_addr().expect("local addr"); + + let stream = connect_tcp_target(addr.ip().to_string(), addr.port(), None) + .await + .expect("connect"); + assert!(stream.nodelay().expect("query TCP_NODELAY")); + } + #[test] fn tcp_target_allows_loopback_hosts() { validate_tcp_target(&tcp("127.0.0.1", 8080)).expect("ipv4 loopback");