Skip to content

Commit

Permalink
Do not set tunnel config MSS/MTU on non-Android systems
Browse files Browse the repository at this point in the history
IP_USER_MTU does not have the intended effect of limiting MSS size.
  • Loading branch information
dlon committed Sep 21, 2024
1 parent 8475b05 commit 9bbbd65
Showing 1 changed file with 7 additions and 38 deletions.
45 changes: 7 additions & 38 deletions talpid-tunnel-config-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,11 @@ pub type RelayConfigService = proto::ephemeral_peer_client::EphemeralPeerClient<
pub const CONFIG_SERVICE_PORT: u16 = 1337;

/// MTU to set on the tunnel config client socket. We want a low value to prevent fragmentation.
/// This is needed for two reasons:
/// 1. Especially on Android, we've found that the real MTU is often lower than the default MTU, and
/// we cannot lower it further. This causes the outer packets to be dropped. Also, MTU detection
/// will likely occur after the PQ handshake, so we cannot assume that the MTU is already
/// correctly configured.
/// 2. MH + PQ on macOS has connection issues during the handshake due to PF blocking packet
/// fragments for not having a port. In the longer term this might be fixed by allowing the
/// handshake to work even if there is fragmentation.
#[cfg(not(target_os = "ios"))]
/// Especially on Android, we've found that the real MTU is often lower than the default MTU, and
/// we cannot lower it further. This causes the outer packets to be dropped. Also, MTU detection
/// will likely occur after the PQ handshake, so we cannot assume that the MTU is already
/// correctly configured.
#[cfg(target_os = "android")]
const CONFIG_CLIENT_MTU: u16 = 576;

pub struct EphemeralPeer {
Expand Down Expand Up @@ -253,10 +249,7 @@ async fn new_client(addr: Ipv4Addr) -> Result<RelayConfigService, Error> {
.connect_with_connector(service_fn(move |_| async move {
let sock = TcpSocket::new_v4()?;

#[cfg(target_os = "windows")]
try_set_tcp_sock_mtu(sock.as_raw_socket(), CONFIG_CLIENT_MTU);

#[cfg(not(target_os = "windows"))]
#[cfg(target_os = "android")]
try_set_tcp_sock_mtu(&addr, sock.as_raw_fd(), CONFIG_CLIENT_MTU);

sock.connect(SocketAddr::new(addr, CONFIG_SERVICE_PORT))
Expand All @@ -268,31 +261,7 @@ async fn new_client(addr: Ipv4Addr) -> Result<RelayConfigService, Error> {
Ok(RelayConfigService::new(conn))
}

#[cfg(windows)]
fn try_set_tcp_sock_mtu(sock: RawSocket, mtu: u16) {
let mtu = u32::from(mtu);
log::debug!("Config client socket MTU: {mtu}");

let raw_sock = usize::try_from(sock).unwrap();

let result = unsafe {
setsockopt(
raw_sock,
IPPROTO_IP,
IP_USER_MTU,
&mtu as *const _ as _,
std::ffi::c_int::try_from(std::mem::size_of_val(&mtu)).unwrap(),
)
};
if result != 0 {
log::error!(
"Failed to set user MTU on config client socket: {}",
std::io::Error::last_os_error()
);
}
}

#[cfg(not(any(target_os = "windows", target_os = "ios")))]
#[cfg(target_os = "android")]
fn try_set_tcp_sock_mtu(dest: &IpAddr, sock: RawFd, mut mtu: u16) {
const IPV4_HEADER_SIZE: u16 = 20;
const IPV6_HEADER_SIZE: u16 = 40;
Expand Down

0 comments on commit 9bbbd65

Please sign in to comment.