[PATCH net-next 0/2] tcp: tcp_tx_timestamp() fix

[PATCH net-next 0/2] tcp: tcp_tx_timestamp() fix

[Eric Dumazet]

Fix an old bug in tcp_tx_timestamp().

Add one corresponding packetdrill test.

Eric Dumazet (2):
  tcp: tcp_tx_timestamp() must look at the rtx queue
  selftest: packetdrill: add tcp_timestamping_tcp_tx_timestamp_bug.pkt

 net/ipv4/tcp.c                                |  3 +
 .../tcp_timestamping_tcp_tx_timestamp_bug.pkt | 70 +++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_timestamping_tcp_tx_timestamp_bug.pkt

-- 
2.53.0.rc1.217.geba53bf80e-goog

[PATCH net-next 1/2] tcp: tcp_tx_timestamp() must look at the rtx queue

[Eric Dumazet]

tcp_tx_timestamp() is only called at the end of tcp_sendmsg_locked()
before the final tcp_push().

By the time it is called, it is possible all the copied data
has been sent already (transmit queue is empty).

If this is the case, use the last skb in the rtx queue.

Fixes: 75c119afe14f ("tcp: implement rb-tree based retransmit queue")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 22b7ec192272f680709be171184fdc3b2df813fb..595329fd2c439290141c4ab5a3a4fa7084dbd5b5 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -501,6 +501,9 @@ static void tcp_tx_timestamp(struct sock *sk, struct sockcm_cookie *sockc)
 	struct sk_buff *skb = tcp_write_queue_tail(sk);
 	u32 tsflags = sockc->tsflags;
 
+	if (unlikely(!skb))
+	        skb = skb_rb_last(&sk->tcp_rtx_queue);
+
 	if (tsflags && skb) {
 		struct skb_shared_info *shinfo = skb_shinfo(skb);
 		struct tcp_skb_cb *tcb = TCP_SKB_CB(skb);
-- 
2.53.0.rc1.217.geba53bf80e-goog

Re: [PATCH net-next 1/2] tcp: tcp_tx_timestamp() must look at the rtx queue

[Jason Xing]

On Tue, Jan 27, 2026 at 8:38 PM Eric Dumazet <edumazet@google.com> wrote:
>
> tcp_tx_timestamp() is only called at the end of tcp_sendmsg_locked()
> before the final tcp_push().
>
> By the time it is called, it is possible all the copied data
> has been sent already (transmit queue is empty).
>
> If this is the case, use the last skb in the rtx queue.
>
> Fixes: 75c119afe14f ("tcp: implement rb-tree based retransmit queue")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>

Great, I failed to spot this case... It could be reproduced when
'wait_for_space' is triggered. I strongly feel that more packetdrill
tests are needed to observe the behavior of socket timestamping.

+ Willem

Thanks,
Jason

[PATCH net-next 2/2] selftest: packetdrill: add tcp_timestamping_tcp_tx_timestamp_bug.pkt

[Eric Dumazet]

Test tcp_tx_timestamp() behavior after ("tcp: tcp_tx_timestamp()
must look at the rtx queue").

Without the fix, this new test fails like this:

tcp_timestamping_tcp_tx_timestamp_bug.pkt:55: runtime error in recvmsg call: Expected result 0 but got -1 with errno 11 (Resource temporarily unavailable)

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 .../tcp_timestamping_tcp_tx_timestamp_bug.pkt | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_timestamping_tcp_tx_timestamp_bug.pkt

diff --git a/tools/testing/selftests/net/packetdrill/tcp_timestamping_tcp_tx_timestamp_bug.pkt b/tools/testing/selftests/net/packetdrill/tcp_timestamping_tcp_tx_timestamp_bug.pkt
new file mode 100644
index 0000000000000000000000000000000000000000..95a1957a2cf9c8d52946174c01241b1a911e4e02
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_timestamping_tcp_tx_timestamp_bug.pkt
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test after "tcp: tcp_tx_timestamp() must look at the rtx queue"
+
+// This test is about receiving the SCM_TSTAMP_ACK,
+// we do not care about its SCM_TIMESTAMPING precision.
+--tolerance_usecs=1000000
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_min_tso_segs=70
+`
+
+// Create a socket and set it to non-blocking.
+    0	socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0	fcntl(3, F_GETFL) = 0x2 (flags O_RDWR)
+   +0	fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+
+// Establish connection and verify that there was no error.
+   +0	connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)
+   +0	> S 0:0(0) <mss 1460,sackOK,TS val 100 ecr 0,nop,wscale 8>
++.010	< S. 0:0(0) ack 1 win 65535 <mss 1000,sackOK,TS val 700 ecr 100,nop,wscale 7>
+   +0	> . 1:1(0) ack 1 <nop,nop,TS val 200 ecr 700>
+   +0	getsockopt(3, SOL_SOCKET, SO_ERROR, [0], [4]) = 0
+   +0	setsockopt(3, SOL_SOCKET, SO_SNDBUF, [30000], 4) = 0
+
+   +0   write(3, ..., 9880) = 9880
+   +0   > P. 1:9881(9880) ack 1 <nop,nop,TS val 200 ecr 700>
++.010   < . 1:1(0) ack 9881 win 10000 <nop,nop,TS val 701 ecr 200>
+
+   +0   write(3, ..., 19760) = 19760
+   +0   > P. 9881:29641(19760) ack 1 <nop,nop,TS val 201 ecr 701>
++.010   < . 1:1(0) ack 29641 win 10000 <nop,nop,TS val 702 ecr 201>
+
+   +0   write(3, ..., 39520) = 39520
+   +0   > P. 29641:69161(39520) ack 1 <nop,nop,TS val 202 ecr 702>
++.010   < . 1:1(0) ack 69161 win 10000 <nop,nop,TS val 703 ecr 202>
+
+// One more write to increase cwnd
+   +0	write(3, ..., 79040) = 79040
+   +0	> P. 69161:108681(39520) ack 1 <nop,nop,TS val 203 ecr 703>
+   +0	> P. 108681:148201(39520) ack 1 <nop,nop,TS val 203 ecr 703>
++.010	< . 1:1(0) ack 148201 win 1000 <nop,nop,TS val 704 ecr 203>
+
+   +0	setsockopt(3, SOL_SOCKET, SO_TIMESTAMPING,
+		   [SOF_TIMESTAMPING_TX_ACK | SOF_TIMESTAMPING_SOFTWARE |
+		    SOF_TIMESTAMPING_OPT_ID], 4) = 0
+
+// We have one write filling one skb
+// last byte can not be stored because of our small SO_SNDBUF
+   +0	write(3, ..., 65209) = 65208
+   +0	> P. 148201:213409(65208) ack 1 <nop,nop,TS val 204 ecr 704>
++.010	< . 1:1(0) ack 213409 win 1000 <nop,nop,TS val 705 ecr 204>
+
+// SCM_TSTAMP_ACK should be received after the last ack at
+// t=60ms.
+   +0	recvmsg(3, {msg_name(...)=...,
+		    msg_iov(1)=[{...,0}],
+                    msg_flags=MSG_ERRQUEUE|MSG_TRUNC,
+                    msg_control=[
+			{cmsg_level=SOL_SOCKET,
+			 cmsg_type=SCM_TIMESTAMPING,
+			 cmsg_data={scm_sec=0,scm_nsec=60000000}},
+			{cmsg_level=CMSG_LEVEL_IP,
+			 cmsg_type=CMSG_TYPE_RECVERR,
+			 cmsg_data={ee_errno=ENOMSG,
+				    ee_origin=SO_EE_ORIGIN_TIMESTAMPING,
+				    ee_type=0,
+				    ee_code=0,
+				    ee_info=SCM_TSTAMP_ACK,
+				    ee_data=65207}}
+		    ]}, MSG_ERRQUEUE) = 0
-- 
2.53.0.rc1.217.geba53bf80e-goog

Re: [PATCH net-next 2/2] selftest: packetdrill: add tcp_timestamping_tcp_tx_timestamp_bug.pkt

[Jason Xing]

On Tue, Jan 27, 2026 at 8:39 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Test tcp_tx_timestamp() behavior after ("tcp: tcp_tx_timestamp()
> must look at the rtx queue").
>
> Without the fix, this new test fails like this:
>
> tcp_timestamping_tcp_tx_timestamp_bug.pkt:55: runtime error in recvmsg call: Expected result 0 but got -1 with errno 11 (Resource temporarily unavailable)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Thanks for the test!

Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>

+Willem

Thanks,
Jason

Re: [PATCH net-next 0/2] tcp: tcp_tx_timestamp() fix

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 27 Jan 2026 12:38:26 +0000 you wrote:
> Fix an old bug in tcp_tx_timestamp().
> 
> Add one corresponding packetdrill test.
> 
> Eric Dumazet (2):
>   tcp: tcp_tx_timestamp() must look at the rtx queue
>   selftest: packetdrill: add tcp_timestamping_tcp_tx_timestamp_bug.pkt
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] tcp: tcp_tx_timestamp() must look at the rtx queue
    https://git.kernel.org/netdev/net-next/c/838eb9687691
  - [net-next,2/2] selftest: packetdrill: add tcp_timestamping_tcp_tx_timestamp_bug.pkt
    https://git.kernel.org/netdev/net-next/c/6080d525aba8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html