diff --git a/src/common.rs b/src/common.rs index d289f8c8f..eada0adcc 100644 --- a/src/common.rs +++ b/src/common.rs @@ -2454,14 +2454,26 @@ pub fn is_udp_disabled() -> bool { pub const OPTION_ENABLE_KCP_CC: &str = "enable-kcp-congestion-control"; -// Default ON ("enable-" option2bool semantics); set "N" to fall back to the pure -// turbo profile (nc=1, no congestion window). +/// Whether to run KCP with its built-in congestion window (nc=0) instead of the pure turbo +/// profile (nc=1) it has always shipped with. +/// +/// Opt-in, deliberately: switching it on is a transport-behavior change for every session, and +/// which profile wins depends on why packets are being lost. +/// +/// - nc=1 never shrinks the send window. On a link that is genuinely congested it keeps pushing, +/// deepening the loss it is reacting to and crowding out other traffic on the same uplink. +/// - nc=0 adds KCP's congestion window, whose backoff is blunt: a fast retransmit halves it, but +/// an RTO sets `cwnd = 1` outright (ikcp.c) and the recovery slow-starts from one packet. On a +/// link with random loss but no congestion — Wi-Fi interference, a long-haul path — that reads +/// loss as congestion and can stall an interactive video stream for seconds. +/// +/// Neither is safely decidable from reasoning, and a loopback benchmark cannot settle it: with +/// no bottleneck queue there is no congestion to control, so it would flatter nc=1 by +/// construction. Until there is evidence from a shaped link or the field, keep the profile users +/// already run and let anyone who wants the other one ask for it. #[inline] pub fn get_kcp_cc_enabled() -> bool { - config::option2bool( - OPTION_ENABLE_KCP_CC, - &Config::get_option(OPTION_ENABLE_KCP_CC), - ) + Config::get_option(OPTION_ENABLE_KCP_CC) == "Y" } // this crate https://github.com/yoshd/stun-client supports nat type diff --git a/src/kcp_stream.rs b/src/kcp_stream.rs index 932f5bfb2..eead2a472 100644 --- a/src/kcp_stream.rs +++ b/src/kcp_stream.rs @@ -26,12 +26,11 @@ static KCP_RECV_ERR_LOG: hbb_common::log_throttle::LogThrottle = hbb_common::log_throttle::LogThrottle::new(KCP_IO_ERR_LOG_INTERVAL); impl KcpStream { - // Engage KCP's built-in congestion control (nc=0) unless disabled by option: pure turbo - // (nc=1) keeps blasting a full 1024-segment window through loss, which on constrained - // links amplifies brief loss into a spiral users experience as stalls or drops. This is - // sender-side only, so no wire negotiation is needed and either peer may run either - // profile. Requires kcp-sys from the `rustdesk-patches` branch, which wires the config - // factory into connection setup (on older revs the factory was stored but never consulted). + // Opt in to KCP's built-in congestion window (nc=0) instead of the pure turbo profile + // (nc=1) that has always shipped; see `get_kcp_cc_enabled` for why this is not the default. + // Sender-side only, so no wire negotiation is needed and either peer may run either profile. + // Requires kcp-sys from the `rustdesk-patches` branch, which wires the config factory into + // connection setup (on older revs the factory was stored but never consulted). fn apply_kcp_config(endpoint: &mut KcpEndpoint) { if crate::get_kcp_cc_enabled() { endpoint.set_kcp_config_factory(Box::new(|conv| {