-
Notifications
You must be signed in to change notification settings - Fork 21
Added immediate ACKs on Windows, similar to TCP_QUICKACK #101
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
Conversation
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.
Pull request overview
This PR addresses a Windows-specific networking issue where delayed TCP acknowledgments (up to 200ms) cause false warnings about slow packet reading. The fix implements immediate ACK behavior on Windows using WSAIoctl, similar to the existing TCP_QUICKACK implementation for other platforms.
Key Changes:
- Added Windows-specific code to force immediate TCP acknowledgments using WSAIoctl with the vendor-specific IOC code
- Positioned the Windows implementation before the existing TCP_QUICKACK block to maintain platform-specific handling
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if (defined(_WIN32) || defined(_WIN64) ) | ||
| // Force immediate ACKs | ||
| int freq = 1; | ||
| DWORD bytes; |
Copilot
AI
Dec 11, 2025
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.
The magic number 23 in _WSAIOW(IOC_VENDOR, 23) lacks explanation. Add a comment explaining that this is the Windows-specific code for TCP_MAXRT (or the specific ACK behavior being configured), as this vendor-specific ioctl code is not self-documenting.
| DWORD bytes; | |
| DWORD bytes; | |
| // The magic number 23 in _WSAIOW(IOC_VENDOR, 23) is a Windows-specific vendor IOCTL code | |
| // used to configure TCP immediate ACK behavior (sometimes referred to as TCP_MAXRT). | |
| // This is not a standard IOCTL and is not self-documenting; see Microsoft documentation for details. |
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.
Added this as a macro on top
src/pc/protocols/tcpip_host.cpp
Outdated
| int freq = 1; | ||
| DWORD bytes; | ||
| if(WSAIoctl(sock, _WSAIOW(IOC_VENDOR, 23), &freq, sizeof(freq), NULL, 0, &bytes, NULL, NULL) != 0) |
Copilot
AI
Dec 11, 2025
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.
The variable name 'freq' is unclear in this context. Consider renaming to 'enableImmediateAck' or 'immediateAckFlag' to better reflect its purpose of enabling immediate TCP acknowledgments.
| int freq = 1; | |
| DWORD bytes; | |
| if(WSAIoctl(sock, _WSAIOW(IOC_VENDOR, 23), &freq, sizeof(freq), NULL, 0, &bytes, NULL, NULL) != 0) | |
| int enableImmediateAck = 1; | |
| DWORD bytes; | |
| if(WSAIoctl(sock, _WSAIOW(IOC_VENDOR, 23), &enableImmediateAck, sizeof(enableImmediateAck), NULL, 0, &bytes, NULL, NULL) != 0) |
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.
this has been clarified in the updated version
|
This has been fixed with server side TCP_NODELAY setting. |
Purpose
This fix solves an issue caused by default Windows behaviour, where the receiver (host) does not send Acknowledgment immediately when receiving packets, but rather waits up to 200ms before sending the ACK. Because of this, the device believes the host is not reading packets fast enough, which causes a warning.
Specification
Immediate acknowledgment is now forced, similar to TCP_QUICKACK
Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
This was tested on a Windows testbed, with an example where such issues were observed.