feat: Allow changing the UDP send/receive buffer sizes

[larseggert]

Add UdpSocketState::set_send_buffer_size and UdpSocketState::set_recv_buffer_size, which allow resizing the respective socket buffers.

CC @mxinden

[mxinden]

Long story short, I am in favor of this patch.

Increasing the UDP socket send and receive buffer can have a positive performance impact. That said, the additional memory footprint might not be an option for all quinn-udp users. Thus an opt-in set of functions on UdpSocketState seems ideal to me.

Long term, once we gathered more experience, we might want to bump the buffer sizes by default.

[mxinden]

Worth having the code below use the new functions?

quinn/perf/src/lib.rs

Lines 28 to 49 in 434c358

	socket
	.set_send_buffer_size(send_buffer_size)
	.context("send buffer size")?;
	socket
	.set_recv_buffer_size(recv_buffer_size)
	.context("recv buffer size")?;
	let buf_size = socket.send_buffer_size().context("send buffer size")?;
	if buf_size < send_buffer_size {
	warn!(
	"Unable to set desired send buffer size. Desired: {}, Actual: {}",
	send_buffer_size, buf_size
	);
	}
	let buf_size = socket.recv_buffer_size().context("recv buffer size")?;
	if buf_size < recv_buffer_size {
	warn!(
	"Unable to set desired recv buffer size. Desired: {}, Actual: {}",
	recv_buffer_size, buf_size
	);
	}

[mxinden]

Changes look good to me.

CI is failing, e.g. on Windows with:

---- socket_buffers stdout ----

thread 'socket_buffers' panicked at quinn-udp\tests\tests.rs:211:58:
called `Result::unwrap()` on an `Err` value: Os { code: 10022, kind: InvalidInput, message: "An invalid argument was supplied." }

[thomaseizinger]

I have played with this before as well and noticed some interesting behaviour on several platforms. Linux for example will give you however much buffer size it can if you request a really large size and not fail the function.

I am not sure if this is common knowledge around socket APIs :)

Maybe incorporating a log like the one pointed out by @mxinden would be worth it? Maybe not on warn but on debug? Or we check and return our own error so users can log it themselves?

I am curious to learn about any performance gains you get from this. In my testing, this wasn't a bottleneck but that was also ~6 months ago and I've landed other perf improvements in our codebase since.

[larseggert]

I am thinking that I will change the test to check that a "set" operation results in any change of the buffer size in the right direction (increase or decrease).

[larseggert]

I can't figure out why UdpSocketState::new fails on WIndows 🤷

[mxinden]

Do you know which of the syscalls in UdpSocketState::new fails?

quinn/quinn-udp/src/windows.rs

Lines 31 to 120 in 434c358

	pub fn new(socket: UdpSockRef<'_>) -> io::Result<Self> {
	assert!(
	CMSG_LEN
	>= WinSock::CMSGHDR::cmsg_space(mem::size_of::<WinSock::IN6_PKTINFO>())
	+ WinSock::CMSGHDR::cmsg_space(mem::size_of::<c_int>())
	+ WinSock::CMSGHDR::cmsg_space(mem::size_of::<u32>())
	);
	assert!(
	mem::align_of::<WinSock::CMSGHDR>() <= mem::align_of::<cmsg::Aligned<[u8; 0]>>(),
	"control message buffers will be misaligned"
	);
	socket.0.set_nonblocking(true)?;
	let addr = socket.0.local_addr()?;
	let is_ipv6 = addr.as_socket_ipv6().is_some();
	let v6only = unsafe {
	let mut result: u32 = 0;
	let mut len = mem::size_of_val(&result) as i32;
	let rc = WinSock::getsockopt(
	socket.0.as_raw_socket() as _,
	WinSock::IPPROTO_IPV6,
	WinSock::IPV6_V6ONLY as _,
	&mut result as *mut _ as _,
	&mut len,
	);
	if rc == -1 {
	return Err(io::Error::last_os_error());
	}
	result != 0
	};
	let is_ipv4 = addr.as_socket_ipv4().is_some() || !v6only;
	// We don't support old versions of Windows that do not enable access to `WSARecvMsg()`
	if WSARECVMSG_PTR.is_none() {
	return Err(io::Error::new(
	io::ErrorKind::Unsupported,
	"network stack does not support WSARecvMsg function",
	));
	}
	if is_ipv4 {
	set_socket_option(
	&*socket.0,
	WinSock::IPPROTO_IP,
	WinSock::IP_DONTFRAGMENT,
	OPTION_ON,
	)?;
	set_socket_option(
	&*socket.0,
	WinSock::IPPROTO_IP,
	WinSock::IP_PKTINFO,
	OPTION_ON,
	)?;
	set_socket_option(
	&*socket.0,
	WinSock::IPPROTO_IP,
	WinSock::IP_RECVECN,
	OPTION_ON,
	)?;
	}
	if is_ipv6 {
	set_socket_option(
	&*socket.0,
	WinSock::IPPROTO_IPV6,
	WinSock::IPV6_DONTFRAG,
	OPTION_ON,
	)?;
	set_socket_option(
	&*socket.0,
	WinSock::IPPROTO_IPV6,
	WinSock::IPV6_PKTINFO,
	OPTION_ON,
	)?;
	set_socket_option(
	&*socket.0,
	WinSock::IPPROTO_IPV6,
	WinSock::IPV6_RECVECN,
	OPTION_ON,
	)?;
	}
	let now = Instant::now();
	Ok(Self {
	last_send_error: Mutex::new(now.checked_sub(2 * IO_ERROR_LOG_INTERVAL).unwrap_or(now)),
	})
	}

[larseggert]

Do you know which of the syscalls in UdpSocketState::new fails?

thread 'socket_buffers' panicked at quinn-udp\src\windows.rs:44:42:
https://github.com/quinn-rs/quinn/actions/runs/13993457832/job/39182608862?pr=2179#step:6:350

Seems to be local_addr.

[larseggert]

Turns out one needs to bind both sockets on Windows.

[Ralith]

Don't forget fallback.rs

[djc]

LGTM. Can you squash your changes into a single commit?

Do we want a quinn-udp version bump so this can be published?

[djc]

Tiny nit: please remove the periods from the first sentences of docstrings.

[djc]

Tiny nit: sockstate -> sock_state or even socket_state.

[larseggert]

I'll squash Monday. I also want to do a follow-up with methods for reading the buffer sizes. Bump afterwards would be nice.

[djc]

Bump afterwards would be nice.

Just add a commit with a version bump when you're ready.