[PATCH v2 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed.

[PATCH v2 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed.

[Kuniyuki Iwashima]

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

[PATCH v2 net-next 1/2] tcp: Don't call tcp_update_recv_tstamps() if not needed.

[Kuniyuki Iwashima]

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

[PATCH v2 net-next 2/2] mptcp: Don't call tcp_update_recv_tstamps() if not needed.

[Kuniyuki Iwashima]

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