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

[PATCH v1 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.

Then, recvmsg() always calls tcp_recv_timestamp() even when the
socket does 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).

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/sock.h   |  7 +++++++
 net/ipv4/tcp.c       | 12 +++++++++---
 net/mptcp/protocol.c |  3 ++-
 3 files changed, 18 insertions(+), 4 deletions(-)

-- 
2.56.0.385.gac671c819a-goog

[PATCH v1 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.

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>
---
 include/net/sock.h |  7 +++++++
 net/ipv4/tcp.c     | 12 +++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 173b37b2b8bb..ba310182e6cc 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1063,6 +1063,13 @@ static inline bool sock_flag(const struct sock *sk, enum sock_flags flag)
 	return test_bit(flag, &sk->sk_flags);
 }
 
+static inline bool sk_want_rx_tstamps(const struct sock *sk)
+{
+	return READ_ONCE(sk->sk_flags) &
+		((1UL << SOCK_RCVTSTAMP) |
+		 (1UL << SOCK_TIMESTAMPING_ANY));
+}
+
 #ifdef CONFIG_NET
 DECLARE_STATIC_KEY_FALSE(memalloc_socks_key);
 static inline int sk_memalloc_socks(void)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b9de7c302ba8..4166be09b826 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1878,6 +1878,12 @@ 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_tstamps(struct sock *sk, struct sk_buff *skb)
+{
+	return TCP_SKB_CB(skb)->has_rxtstamp &&
+		sk_want_rx_tstamps(sk);
+}
+
 #ifdef CONFIG_MMU
 static const struct vm_operations_struct tcp_vm_ops = {
 };
@@ -2069,7 +2075,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_tstamps(sk, skb)) {
 			tcp_update_recv_tstamps(skb, tss);
 			zc->msg_flags |= TCP_CMSG_TS;
 		}
@@ -2280,7 +2286,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_tstamps(sk, skb)) {
 				tcp_update_recv_tstamps(skb, tss);
 				zc->msg_flags |= TCP_CMSG_TS;
 			}
@@ -2893,7 +2899,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_tstamps(sk, skb)) {
 			tcp_update_recv_tstamps(skb, tss);
 			*cmsg_flags |= TCP_CMSG_TS;
 		}
-- 
2.56.0.385.gac671c819a-goog

[PATCH v1 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..e632c7de779f 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 &&
+		    sk_want_rx_tstamps(sk)) {
 			tcp_update_recv_tstamps(skb, tss);
 			*cmsg_flags |= MPTCP_CMSG_TS;
 		}
-- 
2.56.0.385.gac671c819a-goog

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

[Naman Gulati]

On Wed, Nov 4, 2026 at 9:14 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> When RX HWtstamp is enabled on NIC, skb_hwtstamps(skb)->hwtstamp
> is set, which sets TCP_SKB_CB(skb)->has_rxtstamp to true.
>
> 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>
> ---
>  include/net/sock.h |  7 +++++++
>  net/ipv4/tcp.c     | 12 +++++++++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 173b37b2b8bb..ba310182e6cc 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1063,6 +1063,13 @@ static inline bool sock_flag(const struct sock *sk, enum sock_flags flag)
>         return test_bit(flag, &sk->sk_flags);
>  }
>

Small suggestion for safety: SOCK_RCVTSTAMP/SOCK_RCVTSTAMP_ANY are set
by SO_TIMESTAMP{ING} sockopts. Datagram/raw sockets also support
ioctl(SIOCGSTAMP{NS}) which don't return the RX timestamp over cmsg
but trigger updates to sk->sk_stamp, future improper reuse of this
function may miss those paths. Maybe rename to something like
sk_want_cmsg_rx_tstamp (or add a comment describing the scope of this
check).

> +static inline bool sk_want_rx_tstamps(const struct sock *sk)
> +{
> +       return READ_ONCE(sk->sk_flags) &
> +               ((1UL << SOCK_RCVTSTAMP) |
> +                (1UL << SOCK_TIMESTAMPING_ANY));
> +}
> +
>  #ifdef CONFIG_NET
>  DECLARE_STATIC_KEY_FALSE(memalloc_socks_key);
>  static inline int sk_memalloc_socks(void)
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index b9de7c302ba8..4166be09b826 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1878,6 +1878,12 @@ 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_tstamps(struct sock *sk, struct sk_buff *skb)
> +{
> +       return TCP_SKB_CB(skb)->has_rxtstamp &&
> +               sk_want_rx_tstamps(sk);
> +}
> +
>  #ifdef CONFIG_MMU
>  static const struct vm_operations_struct tcp_vm_ops = {
>  };
> @@ -2069,7 +2075,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_tstamps(sk, skb)) {
>                         tcp_update_recv_tstamps(skb, tss);
>                         zc->msg_flags |= TCP_CMSG_TS;
>                 }
> @@ -2280,7 +2286,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_tstamps(sk, skb)) {
>                                 tcp_update_recv_tstamps(skb, tss);
>                                 zc->msg_flags |= TCP_CMSG_TS;
>                         }
> @@ -2893,7 +2899,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_tstamps(sk, skb)) {
>                         tcp_update_recv_tstamps(skb, tss);
>                         *cmsg_flags |= TCP_CMSG_TS;
>                 }
> --
> 2.56.0.385.gac671c819a-goog
>
>

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

[Eric Dumazet]

Le mer. 4 nov. 2026 à 17:50, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> When RX HWtstamp is enabled on NIC, skb_hwtstamps(skb)->hwtstamp
> is set, which sets TCP_SKB_CB(skb)->has_rxtstamp to true.
>

Note that has_rxtstamp is also set when skb->tstamp is not zero,
which is the case for all incoming packets as soon as one socket
on the host enabled software RX timestamps (netstamp_needed_key),
think tcpdump or a PTP daemon.

So this patch helps more hosts than the changelog suggests,
so it might be worth mentioning it.


> 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>
> ---
>  include/net/sock.h |  7 +++++++
>  net/ipv4/tcp.c     | 12 +++++++++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 173b37b2b8bb..ba310182e6cc 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1063,6 +1063,13 @@ static inline bool sock_flag(const struct sock *sk, enum sock_flags flag)
>         return test_bit(flag, &sk->sk_flags);
>  }
>
> +static inline bool sk_want_rx_tstamps(const struct sock *sk)
> +{
> +       return READ_ONCE(sk->sk_flags) &
> +               ((1UL << SOCK_RCVTSTAMP) |
> +                (1UL << SOCK_TIMESTAMPING_ANY));
> +}
> +


I agree with Naman, this name is too generic for sock.h.

This only mirrors the conditions used in tcp_recv_timestamp()
to decide if a cmsg is needed, and is only used by TCP and MPTCP.

What about moving it to include/net/tcp.h, with a name like
tcp_want_rx_tstamp_cmsg() ?

> +static bool tcp_want_rx_tstamps(struct sock *sk, struct sk_buff *skb)

Otherwise the logic looks good to me.

Thanks.

>  #ifdef CONFIG_NET
>  DECLARE_STATIC_KEY_FALSE(memalloc_socks_key);
>  static inline int sk_memalloc_socks(void)
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index b9de7c302ba8..4166be09b826 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1878,6 +1878,12 @@ 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_tstamps(struct sock *sk, struct sk_buff *skb)

nit: const struct sock *sk, const struct sk_buff *skb

> +{
> +       return TCP_SKB_CB(skb)->has_rxtstamp &&
> +               sk_want_rx_tstamps(sk);
> +}
> +
>  #ifdef CONFIG_MMU
>  static const struct vm_operations_struct tcp_vm_ops = {
>  };
> @@ -2069,7 +2075,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_tstamps(sk, skb)) {
>                         tcp_update_recv_tstamps(skb, tss);
>                         zc->msg_flags |= TCP_CMSG_TS;
>                 }
> @@ -2280,7 +2286,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_tstamps(sk, skb)) {
>                                 tcp_update_recv_tstamps(skb, tss);
>                                 zc->msg_flags |= TCP_CMSG_TS;
>                         }
> @@ -2893,7 +2899,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_tstamps(sk, skb)) {
>                         tcp_update_recv_tstamps(skb, tss);
>                         *cmsg_flags |= TCP_CMSG_TS;
>                 }
> --
> 2.56.0.385.gac671c819a-goog
>

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

[Kuniyuki Iwashima]

On Wed, Nov 4, 2026 at 10:41 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le mer. 4 nov. 2026 à 17:50, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
> >
> > When RX HWtstamp is enabled on NIC, skb_hwtstamps(skb)->hwtstamp
> > is set, which sets TCP_SKB_CB(skb)->has_rxtstamp to true.
> >
>
> Note that has_rxtstamp is also set when skb->tstamp is not zero,
> which is the case for all incoming packets as soon as one socket
> on the host enabled software RX timestamps (netstamp_needed_key),
> think tcpdump or a PTP daemon.
>
> So this patch helps more hosts than the changelog suggests,
> so it might be worth mentioning it.

Indeed, will meioned the case.

>
>
> > 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>
> > ---
> >  include/net/sock.h |  7 +++++++
> >  net/ipv4/tcp.c     | 12 +++++++++---
> >  2 files changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 173b37b2b8bb..ba310182e6cc 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -1063,6 +1063,13 @@ static inline bool sock_flag(const struct sock *sk, enum sock_flags flag)
> >         return test_bit(flag, &sk->sk_flags);
> >  }
> >
> > +static inline bool sk_want_rx_tstamps(const struct sock *sk)
> > +{
> > +       return READ_ONCE(sk->sk_flags) &
> > +               ((1UL << SOCK_RCVTSTAMP) |
> > +                (1UL << SOCK_TIMESTAMPING_ANY));
> > +}
> > +
>
>
> I agree with Naman, this name is too generic for sock.h.
>
> This only mirrors the conditions used in tcp_recv_timestamp()
> to decide if a cmsg is needed, and is only used by TCP and MPTCP.
>
> What about moving it to include/net/tcp.h, with a name like
> tcp_want_rx_tstamp_cmsg() ?

Sounds good.

>
> > +static bool tcp_want_rx_tstamps(struct sock *sk, struct sk_buff *skb)
>
> Otherwise the logic looks good to me.
>
> Thanks.
>
> >  #ifdef CONFIG_NET
> >  DECLARE_STATIC_KEY_FALSE(memalloc_socks_key);
> >  static inline int sk_memalloc_socks(void)
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index b9de7c302ba8..4166be09b826 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -1878,6 +1878,12 @@ 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_tstamps(struct sock *sk, struct sk_buff *skb)
>
> nit: const struct sock *sk, const struct sk_buff *skb

Will add const.

Thank you both !

>
> > +{
> > +       return TCP_SKB_CB(skb)->has_rxtstamp &&
> > +               sk_want_rx_tstamps(sk);
> > +}
> > +
> >  #ifdef CONFIG_MMU
> >  static const struct vm_operations_struct tcp_vm_ops = {
> >  };
> > @@ -2069,7 +2075,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_tstamps(sk, skb)) {
> >                         tcp_update_recv_tstamps(skb, tss);
> >                         zc->msg_flags |= TCP_CMSG_TS;
> >                 }
> > @@ -2280,7 +2286,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_tstamps(sk, skb)) {
> >                                 tcp_update_recv_tstamps(skb, tss);
> >                                 zc->msg_flags |= TCP_CMSG_TS;
> >                         }
> > @@ -2893,7 +2899,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_tstamps(sk, skb)) {
> >                         tcp_update_recv_tstamps(skb, tss);
> >                         *cmsg_flags |= TCP_CMSG_TS;
> >                 }
> > --
> > 2.56.0.385.gac671c819a-goog
> >