* [PATCH net v2 0/2] tcp: correct timestamp echo for accepted old ACKs
@ 2026-09-24 22:44 Jeff Jo
2026-09-24 22:44 ` [PATCH net v2 1/2] tcp: refresh TS.Recent " Jeff Jo
2026-09-24 22:44 ` [PATCH net v2 2/2] selftests: net: check timestamp echo after an old ACK Jeff Jo
0 siblings, 2 replies; 5+ messages in thread
From: Jeff Jo @ 2026-09-24 22:44 UTC (permalink / raw)
To: netdev
Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
linux-kernel, linux-kselftest
Linux can acknowledge newly received data while echoing an outdated
TCP timestamp. This happens when a reordered packet fills a receive gap
but carries an older acknowledgment for traffic in the other direction.
If the sender uses this echo to measure round-trip time after a long idle
period, the stale timestamp can inflate its estimate and slow its sending.
Changes in v2, following Eric Dumazet's review:
- Remove the redundant SYN_RECV condition and unused flag accumulation.
- Replace the log-parsing wrapper with a plain packetdrill test that checks
the timestamp echo directly, using the existing selftest runner.
- Rebase on net fc6d80eb5044.
v1: https://lore.kernel.org/netdev/20260921222609.50824-4-jeffjo@openai.com/
In this example, S sends the reordered data and R is the Linux receiver
being patched. All packets shown belong to the same TCP connection, with
overlapping requests in both directions. Each illustrated request fits in
one TCP packet; request/response names describe application
messages, while ACK numbers acknowledge TCP bytes. The numbers are
illustrative: TS and echo use S's millisecond clock, and byte numbers are
relative to the first post-idle byte in each direction. ACK=N acknowledges
bytes before N.
Before idle:
S -> R: sender request 1, TS=999
R -> S: response to sender request 1, echo=999
S -> R: TCP ACK, TS=1000 (R saves timestamp 1000)
... 300 seconds idle ...
After idle (byte ranges include both ends):
S -> R: sender request 2, bytes 1-17, ACK=1, TS=301000
(delayed in the network)
R -> S: receiver request 1, bytes 1-17, ACK=1
(initiated by R while sender request 2 is still in flight)
S -> R: sender request 3, bytes 18-34, ACK=18, TS=301005
(arrives before sender request 2)
R -> S: TCP ACK=1, SACK for sender request 3, echo=1000
S -> R: original sender request 2 arrives, still ACK=1, TS=301000
R -> S: TCP ACK=35, echo=1000 (bug) or echo=301000 (fixed)
R initiates receiver request 1 while sender request 2 is still in flight;
its ACK=1 means it has not received sender request 2. S receives R's
request before sending sender request 3, so that packet carries ACK=18.
Sender request 3 reaches R first, making sender request 2's ACK=1 old.
The earlier ACK with SACK correctly echoes 1000 while the gap is open.
The bug is retaining 1000 in ACK=35 after the gap closes, instead of
echoing sender request 2's timestamp, 301000.
If S falls back to timestamp-based RTT measurement for ACK=35, subtracting
echo=1000 from its current timestamp (about 301000) produces a roughly
300-second RTT sample, mistakenly counting the idle period. The inflated
smoothed RTT lowers the sender's calculated pacing rate and, when pacing
is enforced, unnecessarily delays outgoing packets and slows the transfer.
Patch 1 refreshes the saved timestamp while preserving existing validation.
Patch 2 checks that the ACK closing the receive gap echoes timestamp
301000, in IPv4, IPv6 and IPv4-mapped IPv6. It requires packetdrill's merged
TSecr verification fix, commit 83f72d3f9085, linked below. Once CI uses a
packetdrill version containing that fix, the timestamp assertion will
reject the stale echo on an unfixed kernel and pass with this kernel fix.
Older packetdrill versions ignore the comparison and incorrectly pass both.
https://github.com/google/packetdrill/commit/83f72d3f9085d0e26eb4d206fe4d7cfab5b6d872
V1 passed 68 focused/control cases on normal and KASAN/UBSAN/lockdep
ARM64 kernels, with no new diagnostics from W=1 allyesconfig/allmodconfig
builds or Sparse. V2 passed the same checks with equivalent results.
Earlier C-socket repro on net 46bc52d13594: after 300 seconds idle, with
controlled reordering, retransmission and fq pacing, a 1 MiB transfer takes
22.02 seconds without the fix versus 0.38 seconds with it (one run per arm).
AI assistance: Codex generated and revised the fix, reproducers, selftest,
analysis and patch messages. The user directed the investigation, asked
for real-socket and upstream-kernel comparisons, and requested broader
testing. A separate Codex reviewer challenged the v1 code and evidence.
Sparse supplied static analysis.
Jeff Jo (2):
tcp: refresh TS.Recent for accepted old ACKs
selftests: net: check timestamp echo after an old ACK
net/ipv4/tcp_input.c | 6 +++++
.../net/packetdrill/tcp_old_ack_ts.pkt | 22 +++++++++++++++++++
2 files changed, 28 insertions(+)
create mode 100644 tools/testing/selftests/net/packetdrill/tcp_old_ack_ts.pkt
base-commit: fc6d80eb504458d6416b75a94188b268c95c6533
--
2.55.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH net v2 1/2] tcp: refresh TS.Recent for accepted old ACKs
2026-09-24 22:44 [PATCH net v2 0/2] tcp: correct timestamp echo for accepted old ACKs Jeff Jo
@ 2026-09-24 22:44 ` Jeff Jo
2026-09-25 0:57 ` Eric Dumazet
2026-09-24 22:44 ` [PATCH net v2 2/2] selftests: net: check timestamp echo after an old ACK Jeff Jo
1 sibling, 1 reply; 5+ messages in thread
From: Jeff Jo @ 2026-09-24 22:44 UTC (permalink / raw)
To: netdev
Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
linux-kernel, linux-kselftest
A TCP packet can carry new data while acknowledging traffic in the
opposite direction. With overlapping traffic in both directions, a
delayed packet's acknowledgment can be older than one Linux has already
accepted, even when that packet fills a gap in the received data.
Linux accepts the data, but tcp_ack() takes the old_ack path and skips
updating TS.Recent, the timestamp saved for outgoing acknowledgments.
The reply therefore echoes an older timestamp. If the sender uses this
echo to measure round-trip time after a long idle period, its estimate
includes the idle time and can reduce its sending rate.
Update TS.Recent in old_ack using tcp_replace_ts_recent(), before SACK
processing can trigger a transmission. This reuses the existing timestamp
and sequence checks, including PAWS protection against old duplicate
packets. ACK validation already rejects old ACKs in SYN_RECV before this
path, so no additional state check is needed.
Echoing the timestamp of the packet that fills the receive gap follows
RFC 7323 section 4.3. In a socket reproduction with 300 seconds idle,
controlled reordering and retransmission to exercise timestamp-based RTT
sampling, the sender's smoothed round-trip time was 37.5 seconds without
the fix and 15.5 ms with it.
Fixes: 12fb3dd9dc3c ("tcp: call tcp_replace_ts_recent() from tcp_ack()")
Assisted-by: LLM sparse
Signed-off-by: Jeff Jo <jeffjo@openai.com>
---
net/ipv4/tcp_input.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 92bc60716f33..99baf14afdfd 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4465,6 +4465,12 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
return 1;
old_ack:
+ /* An old ACK can carry new data. Update TS.Recent before SACK
+ * processing can trigger a retransmission.
+ */
+ if (flag & FLAG_UPDATE_TS_RECENT)
+ tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
+
/* If data was SACKed, tag it and see if we should send more data.
* If data was DSACKed, see if we can undo a cwnd reduction.
*/
--
2.55.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net v2 1/2] tcp: refresh TS.Recent for accepted old ACKs
2026-09-24 22:44 ` [PATCH net v2 1/2] tcp: refresh TS.Recent " Jeff Jo
@ 2026-09-25 0:57 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2026-09-25 0:57 UTC (permalink / raw)
To: Jeff Jo
Cc: netdev, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
linux-kernel, linux-kselftest
On Fri, Sep 25, 2026 at 12:45 AM Jeff Jo <jeffjo@openai.com> wrote:
>
> A TCP packet can carry new data while acknowledging traffic in the
> opposite direction. With overlapping traffic in both directions, a
> delayed packet's acknowledgment can be older than one Linux has already
> accepted, even when that packet fills a gap in the received data.
>
> Linux accepts the data, but tcp_ack() takes the old_ack path and skips
> updating TS.Recent, the timestamp saved for outgoing acknowledgments.
> The reply therefore echoes an older timestamp. If the sender uses this
> echo to measure round-trip time after a long idle period, its estimate
> includes the idle time and can reduce its sending rate.
>
> Update TS.Recent in old_ack using tcp_replace_ts_recent(), before SACK
> processing can trigger a transmission. This reuses the existing timestamp
> and sequence checks, including PAWS protection against old duplicate
> packets. ACK validation already rejects old ACKs in SYN_RECV before this
> path, so no additional state check is needed.
>
> Echoing the timestamp of the packet that fills the receive gap follows
> RFC 7323 section 4.3. In a socket reproduction with 300 seconds idle,
> controlled reordering and retransmission to exercise timestamp-based RTT
> sampling, the sender's smoothed round-trip time was 37.5 seconds without
> the fix and 15.5 ms with it.
>
> Fixes: 12fb3dd9dc3c ("tcp: call tcp_replace_ts_recent() from tcp_ack()")
> Assisted-by: LLM sparse
> Signed-off-by: Jeff Jo <jeffjo@openai.com>
> ---
> net/ipv4/tcp_input.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 92bc60716f33..99baf14afdfd 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4465,6 +4465,12 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
> return 1;
>
> old_ack:
> + /* An old ACK can carry new data. Update TS.Recent before SACK
> + * processing can trigger a retransmission.
> + */
> + if (flag & FLAG_UPDATE_TS_RECENT)
> + tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
> +
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net v2 2/2] selftests: net: check timestamp echo after an old ACK
2026-09-24 22:44 [PATCH net v2 0/2] tcp: correct timestamp echo for accepted old ACKs Jeff Jo
2026-09-24 22:44 ` [PATCH net v2 1/2] tcp: refresh TS.Recent " Jeff Jo
@ 2026-09-24 22:44 ` Jeff Jo
2026-09-25 0:58 ` Eric Dumazet
1 sibling, 1 reply; 5+ messages in thread
From: Jeff Jo @ 2026-09-24 22:44 UTC (permalink / raw)
To: netdev
Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
linux-kernel, linux-kselftest
Add a regression test for a gap-filling packet whose acknowledgment has
become old. Linux first sends data to the peer. Deliver two peer data
packets out of order: the later packet acknowledges Linux's data, while
the delayed packet still carries the earlier acknowledgment.
Require the ACK that closes the receive gap to echo the delayed packet's
timestamp, 301000. Without the fix, Linux accepts the data but still echoes
the previously saved timestamp, 1000. Also check that the application can
read all 34 bytes.
Use a large jump in peer timestamps to represent the idle interval, with
no real wait, loss or retransmission. The test directly checks the outgoing
timestamp echo. It requires packetdrill's merged TSecr verification fix
(linked below); older tools incorrectly pass on an unfixed kernel.
Use the existing packetdrill selftest runner for IPv4, IPv6 and
IPv4-mapped IPv6.
Link: https://github.com/google/packetdrill/commit/83f72d3f9085d0e26eb4d206fe4d7cfab5b6d872
Assisted-by: LLM
Signed-off-by: Jeff Jo <jeffjo@openai.com>
---
.../net/packetdrill/tcp_old_ack_ts.pkt | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 tools/testing/selftests/net/packetdrill/tcp_old_ack_ts.pkt
diff --git a/tools/testing/selftests/net/packetdrill/tcp_old_ack_ts.pkt b/tools/testing/selftests/net/packetdrill/tcp_old_ack_ts.pkt
new file mode 100644
index 000000000000..f072e5e4c449
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_old_ack_ts.pkt
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+// An out-of-order segment advances SND.UNA. The gap filler still carries
+// its original, older ACK. The reply must echo the gap filler's timestamp.
+--tcp_ts_tick_usecs=1000
+
+`./defaults.sh`
+
+0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
++0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
++0 bind(3, ..., ...) = 0
++0 listen(3, 1) = 0
+0.100 < S 0:0(0) win 20000 <mss 1000,sackOK,TS val 900 ecr 0>
++0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 100 ecr 900>
+0.200 < . 1:1(0) ack 1 win 20000 <nop,nop,TS val 1000 ecr 100>
++0 accept(3, ..., ...) = 4
+0.300 write(4, ..., 17) = 17
++0 > P. 1:18(17) ack 1 <nop,nop,TS val 300 ecr 1000>
+0.310 < P. 18:35(17) ack 18 win 20000 <nop,nop,TS val 301001 ecr 300>
++0 > . 18:18(0) ack 1 <nop,nop,TS val 310 ecr 1000,nop,nop,sack 18:35>
+0.320 < P. 1:18(17) ack 1 win 20000 <nop,nop,TS val 301000 ecr 100>
++0 > . 18:18(0) ack 35 <nop,nop,TS val 320 ecr 301000>
++0 read(4, ..., 34) = 34
--
2.55.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v2 2/2] selftests: net: check timestamp echo after an old ACK
2026-09-24 22:44 ` [PATCH net v2 2/2] selftests: net: check timestamp echo after an old ACK Jeff Jo
@ 2026-09-25 0:58 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2026-09-25 0:58 UTC (permalink / raw)
To: Jeff Jo
Cc: netdev, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
linux-kernel, linux-kselftest
On Fri, Sep 25, 2026 at 12:45 AM Jeff Jo <jeffjo@openai.com> wrote:
>
> Add a regression test for a gap-filling packet whose acknowledgment has
> become old. Linux first sends data to the peer. Deliver two peer data
> packets out of order: the later packet acknowledges Linux's data, while
> the delayed packet still carries the earlier acknowledgment.
>
> Require the ACK that closes the receive gap to echo the delayed packet's
> timestamp, 301000. Without the fix, Linux accepts the data but still echoes
> the previously saved timestamp, 1000. Also check that the application can
> read all 34 bytes.
>
> Use a large jump in peer timestamps to represent the idle interval, with
> no real wait, loss or retransmission. The test directly checks the outgoing
> timestamp echo. It requires packetdrill's merged TSecr verification fix
> (linked below); older tools incorrectly pass on an unfixed kernel.
>
> Use the existing packetdrill selftest runner for IPv4, IPv6 and
> IPv4-mapped IPv6.
>
> Link: https://github.com/google/packetdrill/commit/83f72d3f9085d0e26eb4d206fe4d7cfab5b6d872
> Assisted-by: LLM
> Signed-off-by: Jeff Jo <jeffjo@openai.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-25 0:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24 22:44 [PATCH net v2 0/2] tcp: correct timestamp echo for accepted old ACKs Jeff Jo
2026-09-24 22:44 ` [PATCH net v2 1/2] tcp: refresh TS.Recent " Jeff Jo
2026-09-25 0:57 ` Eric Dumazet
2026-09-24 22:44 ` [PATCH net v2 2/2] selftests: net: check timestamp echo after an old ACK Jeff Jo
2026-09-25 0:58 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®