#13681: tcp: 2018: implementing TCP SACK option
-------------------------------------+----------------------------
Reporter: a-star | Owner: axeld
Type: enhancement | Status: closed
Priority: normal | Milestone: R1/beta3
Component: Network & Internet/TCP | Version: R1/Development
Resolution: fixed | Keywords: tcp, sack
Blocked By: | Blocking:
Platform: All |
-------------------------------------+----------------------------
Comment (by Skik):
TCP functionality breaks with TCP SACK option due to my company firewall
(SonicWall) thinking its an invalid Option #4 and dropping the packet.
Looking at reference packet captures
https://packetlife.net/media/captures/TCP_SACK.cap and local Linux packet
captures. The SACK option is sent before the TCP Timestamp option and used
as padding instead of the two NOP Options.
I reordered the operations and also added padding when there is no
SACK_PERMITTED. And TCP works now with my company firewall. This is
attached as a reference. I am sure there is a better way to handle it.
{{{
diff --git a/src/add-ons/kernel/network/protocols/tcp/tcp.cpp b/src/add-
ons/kernel/network/protocols/tcp/tcp.cpp
index 5ad4014dfc..5449a35b2e 100644
--- a/src/add-ons/kernel/network/protocols/tcp/tcp.cpp
+++ b/src/add-ons/kernel/network/protocols/tcp/tcp.cpp
@@ -106,13 +106,25 @@ add_options(tcp_segment_header &segment, uint8
*buffer, size_t bufferSize)
bump_option(option, length);
}
+ if ((segment.options & TCP_SACK_PERMITTED) != 0
+ && length + 2 <= bufferSize) {
+ option->kind = TCP_OPTION_SACK_PERMITTED;
+ option->length = 2;
+ bump_option(option, length);
+ }
+
if ((segment.options & TCP_HAS_TIMESTAMPS) != 0
- && length + 12 <= bufferSize) {
- // two NOPs so the timestamps get aligned to a 4 byte
boundary
+ && (segment.options & TCP_SACK_PERMITTED) == 0
+ && length + 2 <= bufferSize) {
+ // two NOPs so the timestamps get aligned to a 4 byte
boundary when no SACK
option->kind = TCP_OPTION_NOP;
bump_option(option, length);
option->kind = TCP_OPTION_NOP;
bump_option(option, length);
+ }
+
+ if ((segment.options & TCP_HAS_TIMESTAMPS) != 0
+ && length + 10 <= bufferSize) {
option->kind = TCP_OPTION_TIMESTAMP;
option->length = 10;
option->timestamp.value = htonl(segment.timestamp_value);
@@ -132,13 +144,6 @@ add_options(tcp_segment_header &segment, uint8
*buffer, size_t bufferSize)
bump_option(option, length);
}
- if ((segment.options & TCP_SACK_PERMITTED) != 0
- && length + 2 <= bufferSize) {
- option->kind = TCP_OPTION_SACK_PERMITTED;
- option->length = 2;
- bump_option(option, length);
- }
-
if (segment.sackCount > 0) {
int sackCount = ((int)(bufferSize - length) - 4) /
sizeof(tcp_sack);
if (sackCount > segment.sackCount)
@@ -435,15 +440,15 @@ tcp_options_length(tcp_segment_header& segment)
if (segment.max_segment_size > 0)
length += 4;
+ if (segment.options & TCP_SACK_PERMITTED)
+ length += 2;
+
if (segment.options & TCP_HAS_TIMESTAMPS)
- length += 12;
+ length += (segment.options & TCP_SACK_PERMITTED) ? 10 :
12;
if (segment.options & TCP_HAS_WINDOW_SCALE)
length += 4;
- if (segment.options & TCP_SACK_PERMITTED)
- length += 2;
-
if (segment.sackCount > 0) {
int sackCount = min_c((int)((kMaxOptionSize - length - 4)
/ sizeof(tcp_sack)), segment.sackCount);
}}}
--
Ticket URL: <https://dev.haiku-os.org/ticket/13681#comment:11>
Haiku <https://dev.haiku-os.org>
The Haiku operating system.
