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

Send immediate ACKs after RMSS bytes of data #1002

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

lrh2000
Copy link
Contributor

@lrh2000 lrh2000 commented Oct 19, 2024

Previously, we let the delayed ACK timer expire and sent an immediate ACK for every other segment received, as described in the following comments:

smoltcp/src/socket/tcp.rs

Lines 2067 to 2073 in 45fa984

// RFC1122 says "in a stream of full-sized segments there SHOULD be an ACK
// for at least every second segment".
// For now, we send an ACK every second received packet, full-sized or not.
AckDelayTimer::Waiting(_) => {
tcp_trace!("delayed ack timer already started, forcing expiry");
AckDelayTimer::Immediate
}

I have found this to be a bit aggressive when the MTU is large (for example, the MTU of the loopback device is 65536 bytes). In such scenarios, every other received segment can be significantly different from every two full-size segments. In short, this can result in too many or too frequent ACKs being sent.

Acroding to the latest RFC, RFC 9293 states:

An ACK SHOULD be generated for at least every second full-sized segment or 2*RMSS bytes of new data (where RMSS is the MSS specified by the TCP endpoint receiving the segments to be acknowledged, or the default value if not specified) (SHLD-19).

However, note that the RFC above only says "at least 2*RMSS bytes", which is not a hard requirement. In practice, I think that it is reasonable to follow the Linux kernel's empirical value of sending an ACK for every RMSS byte of new data. The Linux kernel implementation can be found here:

	/* More than one full frame received... */
	if (((tp->rcv_nxt - tp->rcv_wup) > inet_csk(sk)->icsk_ack.rcv_mss &&

@Dirbaio Dirbaio added this pull request to the merge queue Oct 20, 2024
Merged via the queue into smoltcp-rs:main with commit fe0b4d1 Oct 21, 2024
13 checks passed
@lrh2000 lrh2000 deleted the mtu-ack branch November 8, 2024 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants