Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed.
@ 2026-11-05 21:51 Kuniyuki Iwashima
  2026-11-05 21:51 ` [PATCH v2 net-next 1/2] tcp: " Kuniyuki Iwashima
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-05 21:51 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, Matthieu Baerts, Mat Martineau,
	David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Geliang Tang, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev

When RX HWtstamp is enabled on NIC, skb_hwtstamps(skb)->hwtstamp
is set.  It is also set when one socket enables software timestamp.

Then, recvmsg() always calls tcp_recv_timestamp() even when
sockets do not need timestamp.

Since it is called after release_sock() and touches sk->sk_tsflags,
which shares the same cacheline with sk->sk_forward_alloc, it stalls:

   0.31 │ 403:   test   %al,%al
   0.02 │      ↓ je     4a0
  12.40 │        test   $0x2,%bpl        ; if (cmsg_flags & TCP_CMSG_TS)
        │      ↓ je     48f
  11.59 │        mov    0x60(%r14),%r15  ; sock_flag(sk, SOCK_TSTAMP_NEW)
  12.25 │        mov    0x170(%r14),%ebx ; READ_ONCE(sk->sk_tsflags)
  20.70 │        mov    0xd0(%rsp),%rdi  ; if (tss->ts[0]) {
   0.39 │        test   %rdi,%rdi
        │      ↓ je     44c

Commit f0e70409b7eb ("net: avoid unconditionally touching
sk_tsflags on RX") introduced SOCK_TIMESTAMPING_ANY to avoid the
issue for UDP.

This series checks it to avoid touching sk->sk_tsflags unnecessarily.

With the change, tcp_rr -T 128 -F 10000 gains ~3% more throughput
on AMD EPYC 7B12 Zen 2 Rome (2 sockets, 256 vCPUs).


Changes:
  v2:
    * Patch 1
      * Mention software tstamp
      * s/sk_want_rx_tstamps/tcp_want_rx_tstamp_cmsg/ and
        moved to include/net/tcp.h
      * Use const in tcp_want_rx_tstamps()

  v1: https://lore.kernel.org/netdev/20261104165025.1026357-1-kuniyu@google.com/


Kuniyuki Iwashima (2):
  tcp: Don't call tcp_update_recv_tstamps() if not needed.
  mptcp: Don't call tcp_update_recv_tstamps() if not needed.

 include/net/tcp.h    |  7 +++++++
 net/ipv4/tcp.c       | 13 ++++++++++---
 net/mptcp/protocol.c |  3 ++-
 3 files changed, 19 insertions(+), 4 deletions(-)

-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 net-next 1/2] tcp: Don't call tcp_update_recv_tstamps() if not needed.
  2026-11-05 21:51 [PATCH v2 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed Kuniyuki Iwashima
@ 2026-11-05 21:51 ` Kuniyuki Iwashima
  2026-11-05 21:51 ` [PATCH v2 net-next 2/2] mptcp: " Kuniyuki Iwashima
  2026-11-10 23:50 ` [PATCH v2 net-next 0/2] tcp/mptcp: " patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-05 21:51 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, Matthieu Baerts, Mat Martineau,
	David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Geliang Tang, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev

When RX HWtstamp is enabled on NIC, skb_hwtstamps(skb)->hwtstamp
is set, which sets TCP_SKB_CB(skb)->has_rxtstamp to true.

In addition, TCP_SKB_CB(skb)->has_rxtstamp is also set from
skb->tstamp, which is set when one socket (e.g. tcpdump, PTP
daemon, etc) enables software RX timestamps (netstamp_needed_key).

Then, tcp_recvmsg_locked() always calls tcp_update_recv_tstamps()
and sets TCP_CMSG_TS to cmsg_flags so that later tcp_recvmsg()
calls tcp_recv_timestamp() after release_sock().

If a socket does not have SOCK_RCVTSTAMP in sk->sk_flags or any
flags set in sk->sk_tsflags, tcp_recv_timestamp() does nothing.

However, it is not free because sk->sk_tsflags shares the same
cacheline with sk->sk_forward_alloc, which is always modified
under lock_sock()/bh_lock_sock(), causing cacheline bouncing.

   0.31 │ 403:   test   %al,%al
   0.02 │      ↓ je     4a0
  12.40 │        test   $0x2,%bpl        ; if (cmsg_flags & TCP_CMSG_TS)
        │      ↓ je     48f
  11.59 │        mov    0x60(%r14),%r15  ; sock_flag(sk, SOCK_TSTAMP_NEW)
  12.25 │        mov    0x170(%r14),%ebx ; READ_ONCE(sk->sk_tsflags)
  20.70 │        mov    0xd0(%rsp),%rdi  ; if (tss->ts[0]) {
   0.39 │        test   %rdi,%rdi
        │      ↓ je     44c

Also, sk->sk_flags is on the cold cacheline in tcp_recvmsg().

Commit f0e70409b7eb ("net: avoid unconditionally touching
sk_tsflags on RX") introduced SOCK_TIMESTAMPING_ANY to avoid
such false sharing.

Let's check SOCK_RCVTSTAMP and SOCK_TIMESTAMPING_ANY under
lock_sock().

We still need to touch sk->sk_flags, but with this change,
tcp_rr -T 128 -F 10000 gains ~3% more throughput on AMD EPYC
7B12 Zen 2 Rome (2 sockets, 256 vCPUs).

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v2:
  * Mention software tstamp
  * s/sk_want_rx_tstamps/tcp_want_rx_tstamp_cmsg/ and
    moved to include/net/tcp.h
  * Use const in tcp_want_rx_tstamp()
---
 include/net/tcp.h |  7 +++++++
 net/ipv4/tcp.c    | 13 ++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 3560adddf821..f265ae06bb55 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -517,6 +517,13 @@ int tcp_set_rcvlowat(struct sock *sk, int val);
 void tcp_set_rcvbuf(struct sock *sk, int val);
 int tcp_set_window_clamp(struct sock *sk, int val);
 
+static inline bool tcp_want_rx_tstamp_cmsg(const struct sock *sk)
+{
+	return READ_ONCE(sk->sk_flags) &
+		((1UL << SOCK_RCVTSTAMP) |
+		 (1UL << SOCK_TIMESTAMPING_ANY));
+}
+
 static inline void
 tcp_update_recv_tstamps(struct sk_buff *skb,
 			struct scm_timestamping_internal *tss)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b9de7c302ba8..a1f89c1fa2ae 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1878,6 +1878,13 @@ void tcp_set_rcvbuf(struct sock *sk, int val)
 	tcp_set_window_clamp(sk, tcp_win_from_space(sk, val));
 }
 
+static bool tcp_want_rx_tstamp(const struct sock *sk,
+			       const struct sk_buff *skb)
+{
+	return TCP_SKB_CB(skb)->has_rxtstamp &&
+		tcp_want_rx_tstamp_cmsg(sk);
+}
+
 #ifdef CONFIG_MMU
 static const struct vm_operations_struct tcp_vm_ops = {
 };
@@ -2069,7 +2076,7 @@ static int tcp_zc_handle_leftover(struct tcp_zerocopy_receive *zc,
 		offset = *seq - TCP_SKB_CB(skb)->seq;
 	} else {
 		skb = tcp_recv_skb(sk, *seq, &offset);
-		if (TCP_SKB_CB(skb)->has_rxtstamp) {
+		if (tcp_want_rx_tstamp(sk, skb)) {
 			tcp_update_recv_tstamps(skb, tss);
 			zc->msg_flags |= TCP_CMSG_TS;
 		}
@@ -2280,7 +2287,7 @@ static int tcp_zerocopy_receive(struct sock *sk,
 			if (!skb_frags_readable(skb))
 				break;
 
-			if (TCP_SKB_CB(skb)->has_rxtstamp) {
+			if (tcp_want_rx_tstamp(sk, skb)) {
 				tcp_update_recv_tstamps(skb, tss);
 				zc->msg_flags |= TCP_CMSG_TS;
 			}
@@ -2893,7 +2900,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
 			tcp_fast_path_check(sk);
 		}
 
-		if (TCP_SKB_CB(skb)->has_rxtstamp) {
+		if (tcp_want_rx_tstamp(sk, skb)) {
 			tcp_update_recv_tstamps(skb, tss);
 			*cmsg_flags |= TCP_CMSG_TS;
 		}
-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 net-next 2/2] mptcp: Don't call tcp_update_recv_tstamps() if not needed.
  2026-11-05 21:51 [PATCH v2 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed Kuniyuki Iwashima
  2026-11-05 21:51 ` [PATCH v2 net-next 1/2] tcp: " Kuniyuki Iwashima
@ 2026-11-05 21:51 ` Kuniyuki Iwashima
  2026-11-09 12:24   ` Matthieu Baerts
  2026-11-10 23:50 ` [PATCH v2 net-next 0/2] tcp/mptcp: " patchwork-bot+netdevbpf
  2 siblings, 1 reply; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-05 21:51 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, Matthieu Baerts, Mat Martineau,
	David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Geliang Tang, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev

As with TCP, __mptcp_recvmsg_mskq() also always triggers
tcp_recv_timestamp() if skb has hwtstamp.

Accessing sk->sk_tsflags incurs unnecessary costs for sockets
that do not need RX tstamp.

Let's check SOCK_RCVTSTAMP and SOCK_TIMESTAMPING_ANY under
lock_sock().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/mptcp/protocol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7a22e9388e0e..797797ce4e88 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2189,7 +2189,8 @@ static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
 			}
 		}
 
-		if (MPTCP_SKB_CB(skb)->has_rxtstamp) {
+		if (MPTCP_SKB_CB(skb)->has_rxtstamp &&
+		    tcp_want_rx_tstamp_cmsg(sk)) {
 			tcp_update_recv_tstamps(skb, tss);
 			*cmsg_flags |= MPTCP_CMSG_TS;
 		}
-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 net-next 2/2] mptcp: Don't call tcp_update_recv_tstamps() if not needed.
  2026-11-05 21:51 ` [PATCH v2 net-next 2/2] mptcp: " Kuniyuki Iwashima
@ 2026-11-09 12:24   ` Matthieu Baerts
  0 siblings, 0 replies; 5+ messages in thread
From: Matthieu Baerts @ 2026-11-09 12:24 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Eric Dumazet, Neal Cardwell, Mat Martineau, David S . Miller,
	Jakub Kicinski, Paolo Abeni, Geliang Tang, Simon Horman,
	Kuniyuki Iwashima, netdev

Hi Kuniyuki,

On 05/11/2026 22:51, Kuniyuki Iwashima wrote:
> As with TCP, __mptcp_recvmsg_mskq() also always triggers
> tcp_recv_timestamp() if skb has hwtstamp.
> 
> Accessing sk->sk_tsflags incurs unnecessary costs for sockets
> that do not need RX tstamp.
> 
> Let's check SOCK_RCVTSTAMP and SOCK_TIMESTAMPING_ANY under
> lock_sock().

Thank you for also fixing MPTCP!

> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  net/mptcp/protocol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 7a22e9388e0e..797797ce4e88 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2189,7 +2189,8 @@ static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
>  			}
>  		}
>  
> -		if (MPTCP_SKB_CB(skb)->has_rxtstamp) {
> +		if (MPTCP_SKB_CB(skb)->has_rxtstamp &&
> +		    tcp_want_rx_tstamp_cmsg(sk)) {

It is "strange" to call a TCP helper on an MPTCP socket (and not on a
TCP subflow), and that's why I preferred the previous name (sk_*). But I
understand you got asked to change that name, so better not to change it
again just for MPTCP I suppose :)

Acked-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed.
  2026-11-05 21:51 [PATCH v2 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed Kuniyuki Iwashima
  2026-11-05 21:51 ` [PATCH v2 net-next 1/2] tcp: " Kuniyuki Iwashima
  2026-11-05 21:51 ` [PATCH v2 net-next 2/2] mptcp: " Kuniyuki Iwashima
@ 2026-11-10 23:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-11-10 23:50 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: edumazet, ncardwell, matttbe, martineau, davem, kuba, pabeni,
	geliang, horms, kuni1840, netdev

Hello:

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

On Thu,  5 Nov 2026 21:51:44 +0000 you wrote:
> When RX HWtstamp is enabled on NIC, skb_hwtstamps(skb)->hwtstamp
> is set.  It is also set when one socket enables software timestamp.
> 
> Then, recvmsg() always calls tcp_recv_timestamp() even when
> sockets do not need timestamp.
> 
> Since it is called after release_sock() and touches sk->sk_tsflags,
> which shares the same cacheline with sk->sk_forward_alloc, it stalls:
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/2] tcp: Don't call tcp_update_recv_tstamps() if not needed.
    https://git.kernel.org/netdev/net-next/c/f8143639f2e6
  - [v2,net-next,2/2] mptcp: Don't call tcp_update_recv_tstamps() if not needed.
    https://git.kernel.org/netdev/net-next/c/59e3434a73c8

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-10 23:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-05 21:51 [PATCH v2 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed Kuniyuki Iwashima
2026-11-05 21:51 ` [PATCH v2 net-next 1/2] tcp: " Kuniyuki Iwashima
2026-11-05 21:51 ` [PATCH v2 net-next 2/2] mptcp: " Kuniyuki Iwashima
2026-11-09 12:24   ` Matthieu Baerts
2026-11-10 23:50 ` [PATCH v2 net-next 0/2] tcp/mptcp: " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox