Skip to content

Commit

Permalink
fix(udp): make GRO (i.e. URO) optional, off by default
Browse files Browse the repository at this point in the history
We have multiple bug reports for URO on Windows, including non-ARM. See:

- #2041
- https://bugzilla.mozilla.org/show_bug.cgi?id=1916558

Instead of enabling GRO on Windows by default, this commit changes the default
to off, adding a `set_gro` to optionally enable it.
  • Loading branch information
mxinden authored and djc committed Dec 17, 2024
1 parent 85011ef commit 6ee883a
Showing 1 changed file with 22 additions and 11 deletions.
33 changes: 22 additions & 11 deletions quinn-udp/src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,34 @@ impl UdpSocketState {
)?;
}

// Opportunistically try to enable GRO
_ = set_socket_option(
&*socket.0,
WinSock::IPPROTO_UDP,
WinSock::UDP_RECV_MAX_COALESCED_SIZE,
// u32 per
// https://learn.microsoft.com/en-us/windows/win32/winsock/ipproto-udp-socket-options.
// Choice of 2^16 - 1 inspired by msquic.
u16::MAX as u32,
);

let now = Instant::now();
Ok(Self {
last_send_error: Mutex::new(now.checked_sub(2 * IO_ERROR_LOG_INTERVAL).unwrap_or(now)),
})
}

/// Enable or disable receive offloading.
///
/// Also referred to as UDP Receive Segment Coalescing Offload (URO) on Windows.
///
/// <https://learn.microsoft.com/en-us/windows-hardware/drivers/network/udp-rsc-offload>
///
/// Disabled by default on Windows due to <https://github.com/quinn-rs/quinn/issues/2041>.
pub fn set_gro(&self, socket: UdpSockRef<'_>, enable: bool) -> io::Result<()> {
set_socket_option(
&*socket.0,
WinSock::IPPROTO_UDP,
WinSock::UDP_RECV_MAX_COALESCED_SIZE,
match enable {
// u32 per
// https://learn.microsoft.com/en-us/windows/win32/winsock/ipproto-udp-socket-options.
// Choice of 2^16 - 1 inspired by msquic.
true => u16::MAX as u32,
false => 0,
},
)
}

/// Sends a [`Transmit`] on the given socket.
///
/// This function will only ever return errors of kind [`io::ErrorKind::WouldBlock`].
Expand Down

0 comments on commit 6ee883a

Please sign in to comment.