-
Notifications
You must be signed in to change notification settings - Fork 293
Added capability to set TCP ACK Frequency on Windows #638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2280,6 +2280,35 @@ impl Socket { | |
| ) | ||
| } | ||
| } | ||
|
|
||
| /// Sets the number of TCP segments that must be received | ||
| /// before the delayed ACK timer is ignored for this socket. | ||
| /// | ||
| /// Currently this is only controllable per socket on Windows. | ||
| /// | ||
| /// # Notes | ||
| /// | ||
| /// * On Windows, this will invoke the `SIO_TCP_SET_ACK_FREQUENCY` IOCTL | ||
| /// on the socket. | ||
| /// * This can be used to achieve results similiar to `TCP_QUICKACK` on | ||
| /// Windows by setting frequency to 1, but on a permanent basis. | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ``` | ||
| /// use socket2::{Socket, Domain, Type, Protocol}; | ||
| /// | ||
| /// # fn main() -> std::io::Result<()> { | ||
| /// let socket = Socket::new(Domain::IPV4, Type::STREAM, Some(Protocol::TCP))?; | ||
| /// | ||
| /// socket.set_tcp_ack_frequency(1)?; | ||
| /// # Ok(()) } | ||
| /// ``` | ||
| /// | ||
| #[cfg(all(feature = "all", windows))] | ||
| pub fn set_tcp_ack_frequency(&self, frequency: u8) -> io::Result<()> { | ||
| sys::set_tcp_ack_frequency(self.as_raw(), frequency) | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this option be retrieved as well? If so, can we have a getter for this? You can use the same pattern that we follow for other getters and setters.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately no, |
||
| } | ||
|
|
||
| impl Read for Socket { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ use windows_sys::Win32::Networking::WinSock::{ | |
| POLLERR, POLLHUP, POLLRDNORM, POLLWRNORM, SD_BOTH, SD_RECEIVE, SD_SEND, SIO_KEEPALIVE_VALS, | ||
| SOCKET_ERROR, WSABUF, WSAEMSGSIZE, WSAESHUTDOWN, WSAPOLLFD, WSAPROTOCOL_INFOW, | ||
| WSA_FLAG_NO_HANDLE_INHERIT, WSA_FLAG_OVERLAPPED, WSA_FLAG_REGISTERED_IO, | ||
| SIO_TCP_SET_ACK_FREQUENCY, TCP_ACK_FREQUENCY_PARAMETERS, | ||
| }; | ||
| #[cfg(feature = "all")] | ||
| use windows_sys::Win32::Networking::WinSock::{ | ||
|
|
@@ -798,6 +799,29 @@ pub(crate) fn set_tcp_keepalive(socket: RawSocket, keepalive: &TcpKeepalive) -> | |
| Ok(()) | ||
| } | ||
|
|
||
| pub(crate) fn set_tcp_ack_frequency(socket: RawSocket, frequency: u8) -> io::Result<()> { | ||
|
Thomasdezeeuw marked this conversation as resolved.
|
||
| let mut freq_params = TCP_ACK_FREQUENCY_PARAMETERS { | ||
| TcpDelayedAckFrequency: frequency, | ||
| }; | ||
|
|
||
| let mut out = 0; | ||
| syscall!( | ||
| WSAIoctl( | ||
| socket, | ||
| SIO_TCP_SET_ACK_FREQUENCY, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't find any documentation for this online, so I have no idea if this is correct or not.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the official documentation is indeed sparse and makes no comment on this feature and others. Surprising given this Funny enough, the only Microsoft link I can find is this which is from their Rust crate docs. |
||
| &mut freq_params as *mut _ as *mut _, | ||
| size_of::<TCP_ACK_FREQUENCY_PARAMETERS>() as _, | ||
| ptr::null_mut(), | ||
| 0, | ||
| &mut out, | ||
| ptr::null_mut(), | ||
| None, | ||
| ), | ||
| PartialEq::eq, | ||
| SOCKET_ERROR | ||
| ).map(|_| ()) | ||
| } | ||
|
|
||
| /// Caller must ensure `T` is the correct type for `level` and `optname`. | ||
| // NOTE: `optname` is actually `i32`, but all constants are `u32`. | ||
| pub(crate) unsafe fn getsockopt<T>(socket: RawSocket, level: c_int, optname: i32) -> io::Result<T> { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have examples for any of the methods and the rest of these docs don't really add anything in my opinion.
We can mention it uses
SIO_TCP_SET_ACK_FREQUENCY, e.g. something like the following:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was following the
set_tcp_keepalivemethod as precedent for this, but your point is well taken. Very few methods contain such verbose documentation. I will scale it back accordingly.