tcp: several fixes for SACK handling:

* kMaxOptionSize: TCP header size is 60 bytes. process_options() would have
 accepted a longer header as permitted. add_options() would have possibly added
 more options or SACK entries as permitted.
* tcp_segment: reserve memory for sack entries.
* _SendQueued(): "fLastAcknowledgeSent <= fReceiveNext": also send SACK entries
 for received data when acknowledging a missing segment.
* _SendQueue(): after acknowledging, cancel the delayed acknowledgment timer.
* _Receive(): if calling Acknowledged() already triggered sending a segment
 including an acknowledgement (piggy-back), remove "immediate acknowledge"
 from the action. This helps to reduce empty ACKs for bidirectional streams.

Change-Id: I32808fbe549be0f5b25bcf4be17cd91a640b8ec4
Reviewed-on: https://review.haiku-os.org/c/haiku/+/3906
Reviewed-by: Adrien Destugues <pulkomandy@gmail.com>
This commit is contained in:
Jérôme Duval 2021-05-11 17:25:35 +02:00
parent a939e873ba
commit f1ec69628a
3 changed files with 26 additions and 18 deletions

View File

@ -1773,8 +1773,13 @@ TCPEndpoint::_Receive(tcp_segment_header& segment, net_buffer* buffer)
}
}
if (fState != CLOSED)
if (fState != CLOSED) {
tcp_sequence last = fLastAcknowledgeSent;
_Acknowledged(segment);
// we just sent an acknowledge, remove from action
if (last < fLastAcknowledgeSent)
action &= ~IMMEDIATE_ACKNOWLEDGE;
}
}
}
@ -2024,15 +2029,15 @@ TCPEndpoint::_SendQueued(bool force, uint32 sendWindow)
}
// SACK information is embedded with duplicate acknowledgements
if (!fReceiveQueue.IsContiguous() && fLastAcknowledgeSent == fReceiveNext
if (!fReceiveQueue.IsContiguous()
&& fLastAcknowledgeSent <= fReceiveNext
&& (fFlags & FLAG_OPTION_SACK_PERMITTED) != 0) {
segment.options |= TCP_HAS_SACK;
int maxSackCount = 4 - ((fFlags & FLAG_OPTION_TIMESTAMP) != 0);
segment.sacks = (tcp_sack*)calloc(maxSackCount, sizeof(tcp_sack));
if (segment.sacks != NULL)
segment.sackCount = fReceiveQueue.PopulateSackInfo(fReceiveNext, maxSackCount, segment.sacks);
else
segment.sackCount = 0;
int maxSackCount = MAX_SACK_BLKS
- ((fFlags & FLAG_OPTION_TIMESTAMP) != 0);
memset(segment.sacks, 0, sizeof(segment.sacks));
segment.sackCount = fReceiveQueue.PopulateSackInfo(fReceiveNext,
maxSackCount, segment.sacks);
}
if ((segment.flags & TCP_FLAG_SYNCHRONIZE) != 0
@ -2215,8 +2220,10 @@ TCPEndpoint::_SendQueued(bool force, uint32 sendWindow)
shouldStartRetransmitTimer = false;
}
if (segment.flags & TCP_FLAG_ACKNOWLEDGE)
if (segment.flags & TCP_FLAG_ACKNOWLEDGE) {
fLastAcknowledgeSent = segment.acknowledge;
gStackModule->cancel_timer(&fDelayedAcknowledgeTimer);
}
length -= segmentLength;
segment.flags &= ~(TCP_FLAG_SYNCHRONIZE | TCP_FLAG_RESET

View File

@ -53,8 +53,8 @@ static EndpointManager* sEndpointManagers[AF_MAX];
static rw_lock sEndpointManagersLock;
// The TCP header length is at most 64 bytes.
static const int kMaxOptionSize = 64 - sizeof(tcp_header);
// The TCP header length is at most 60 bytes (0xf * 4).
static const int kMaxOptionSize = 60 - sizeof(tcp_header);
/*! Returns an endpoint manager for the specified domain, if any.
@ -153,7 +153,6 @@ add_options(tcp_segment_header &segment, uint8 *buffer, size_t bufferSize)
option->length = 2 + sackCount * sizeof(tcp_sack);
memcpy(option->sack, segment.sacks, sackCount * sizeof(tcp_sack));
bump_option(option, length);
free(segment.sacks);
}
}
@ -221,12 +220,13 @@ process_options(tcp_segment_header &segment, net_buffer *buffer, size_t size)
case TCP_OPTION_SACK:
if (size >= option->length) {
segment.options |= TCP_HAS_SACK;
segment.sackCount = (option->length - 2) / sizeof(tcp_sack);
segment.sacks = option->sack;
segment.sackCount = min_c((option->length - 2)
/ sizeof(tcp_sack), MAX_SACK_BLKS);
for(int i = 0; i < segment.sackCount; ++i) {
segment.sacks[i].left_edge = ntohl(segment.sacks[i].left_edge);
segment.sacks[i].right_edge = ntohl(segment.sacks[i].right_edge);
segment.sacks[i].left_edge = ntohl(
option->sack[i].left_edge);
segment.sacks[i].right_edge = ntohl(
option->sack[i].right_edge);
}
}
break;

View File

@ -255,7 +255,8 @@ struct tcp_segment_header {
uint32 timestamp_value;
uint32 timestamp_reply;
tcp_sack *sacks;
#define MAX_SACK_BLKS 4
tcp_sack sacks[MAX_SACK_BLKS];
int sackCount;
uint32 options;