From d7c71d7b49e65466a08fef260c519ed014777f86 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Sat, 30 Dec 2023 21:27:30 -0500 Subject: [PATCH] TCP: Coalesce more ACKs in DelayedAcknowledge. First, we don't need to generate ACKs for every other segment received, only every second full-size segment or within 500ms, as the comment notes. So check the receive window size before deciding to send an ACK immediately. Second, let the timeout routine handle sending the ACK even in the immediate invocation case. This way, we don't spend time in receive routines waiting for the send path locks, and also multiple packets received in quick succession will have one ACK generated instead of many. Also, following the previous commit, the timeout routine will avoid generating duplicate ACKs now. In the case where a duplicate ACK really needs to be generated, DelayedAcknowledge won't be used anyway. Inspired by ambroff's remarks and patch in comment:14 of #18203. Greatly reduces the number of ACKs generated, and increases throughput due to less duplicate ACKs causing congestion logic to kick in. Change-Id: I37991464b1a802aceb3e2b453df8dc4cb2e14ce5 Reviewed-on: https://review.haiku-os.org/c/haiku/+/7284 Reviewed-by: Alex von Gluck IV Reviewed-by: waddlesplash --- .../network/protocols/tcp/TCPEndpoint.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/add-ons/kernel/network/protocols/tcp/TCPEndpoint.cpp b/src/add-ons/kernel/network/protocols/tcp/TCPEndpoint.cpp index c68822f78f..4473b87782 100644 --- a/src/add-ons/kernel/network/protocols/tcp/TCPEndpoint.cpp +++ b/src/add-ons/kernel/network/protocols/tcp/TCPEndpoint.cpp @@ -1134,15 +1134,19 @@ TCPEndpoint::IsLocal() const status_t TCPEndpoint::DelayedAcknowledge() { - if (gStackModule->cancel_timer(&fDelayedAcknowledgeTimer)) { - // timer was active, send an ACK now (with the exception above, - // we send every other ACK) - T(TimerSet(this, "delayed ack", -1)); - return SendAcknowledge(true); + // ACKs "MUST" be generated within 500ms of the first unACKed packet, and + // "SHOULD" be for at least every second full-size segment. (RFC 5681 ยง 4.2) + + bigtime_t delay = TCP_DELAYED_ACKNOWLEDGE_TIMEOUT; + if ((fReceiveNext - fLastAcknowledgeSent) >= (fReceiveMaxSegmentSize * 2)) { + // Trigger an immediate timeout rather than invoking Send directly, + // allowing multiple invocations to be coalesced. + delay = 0; + } else if (gStackModule->is_timer_active(&fDelayedAcknowledgeTimer)) { + return B_OK; } - gStackModule->set_timer(&fDelayedAcknowledgeTimer, - TCP_DELAYED_ACKNOWLEDGE_TIMEOUT); + gStackModule->set_timer(&fDelayedAcknowledgeTimer, delay); T(TimerSet(this, "delayed ack", TCP_DELAYED_ACKNOWLEDGE_TIMEOUT)); return B_OK; }