Skip to content
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

Add Unix.TCP_QUICKACK #13133

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Conversation

rbjorklin
Copy link

@rbjorklin rbjorklin commented Apr 28, 2024

If accepted this PR adds the TCP_QUICKACK socket option.

TCP_QUICKACK can be of use on low latency networks where a socket buffer may not fill up in the hard coded 200ms window. Setting TCP_QUICKACK will cause acknowledgements to be sent immediately.

Short version: set TCP_QUICKACK. If you find a case where that makes things worse, let me know.

John Nagle

PS. This PR was heavily inspired by #9869 and I'm not convinced I did everything correctly. For example it is not clear to me why the TCP_NODELAY macro exists in sockopt_unix.c but not in sockopt_win32.c and because I don't understand I opted to not add the macro to sockopt_win32.c.

EDIT: The build failed without the macro in sockopt_win32.c so I ended up adding it after all.

@rbjorklin rbjorklin force-pushed the add-tcp-quickack branch 2 times, most recently from 58830ea to 0d16c14 Compare April 29, 2024 01:57
@MisterDA
Copy link
Contributor

I went looking around and found a discussion on the dotnet runtime about adding this feature Socket support for TCP_QUICKACK. They decided against it, because there was no good cross-platform support, and couldn't settle on a portable way to expose the feature to their users.

@rbjorklin
Copy link
Author

For what it's worth this is available at the system level when using systemd >= 253.

It is also possible to modify existing routes to force quickack for any connection using said route.

ip -4 route show scope link | while read route ; do 
    ip route change ${route} quickack 1
done

@gasche
Copy link
Member

gasche commented May 15, 2024

I wonder who would want to make a decision on this question. Maybe @xavierleroy or @damiendoligez?

@rbjorklin
Copy link
Author

Given that workarounds exist I would be fine with this being closed as “won’t do”.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants