mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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

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®