* [PATCH v1 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed.
@ 2026-11-04 16:49 Kuniyuki Iwashima
2026-11-04 16:49 ` [PATCH v1 net-next 1/2] tcp: " Kuniyuki Iwashima
2026-11-04 16:49 ` [PATCH v1 net-next 2/2] mptcp: " Kuniyuki Iwashima
0 siblings, 2 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-04 16:49 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v1 net-next 1/2] tcp: Don't call tcp_update_recv_tstamps() if not needed. 2026-11-04 16:49 [PATCH v1 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed Kuniyuki Iwashima @ 2026-11-04 16:49 ` Kuniyuki Iwashima 2026-11-04 18:25 ` Naman Gulati 2026-11-04 18:41 ` Eric Dumazet 2026-11-04 16:49 ` [PATCH v1 net-next 2/2] mptcp: " Kuniyuki Iwashima 1 sibling, 2 replies; 6+ messages in thread From: Kuniyuki Iwashima @ 2026-11-04 16:49 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. 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 net-next 1/2] tcp: Don't call tcp_update_recv_tstamps() if not needed. 2026-11-04 16:49 ` [PATCH v1 net-next 1/2] tcp: " Kuniyuki Iwashima @ 2026-11-04 18:25 ` Naman Gulati 2026-11-04 18:41 ` Eric Dumazet 1 sibling, 0 replies; 6+ messages in thread From: Naman Gulati @ 2026-11-04 18:25 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: Eric Dumazet, Neal Cardwell, Matthieu Baerts, Mat Martineau, David S . Miller, Jakub Kicinski, Paolo Abeni, Geliang Tang, Simon Horman, Kuniyuki Iwashima, netdev 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 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 net-next 1/2] tcp: Don't call tcp_update_recv_tstamps() if not needed. 2026-11-04 16:49 ` [PATCH v1 net-next 1/2] tcp: " Kuniyuki Iwashima 2026-11-04 18:25 ` Naman Gulati @ 2026-11-04 18:41 ` Eric Dumazet 2026-11-04 19:02 ` Kuniyuki Iwashima 1 sibling, 1 reply; 6+ messages in thread From: Eric Dumazet @ 2026-11-04 18:41 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: Neal Cardwell, Matthieu Baerts, Mat Martineau, David S . Miller, Jakub Kicinski, Paolo Abeni, Geliang Tang, Simon Horman, Kuniyuki Iwashima, netdev 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 net-next 1/2] tcp: Don't call tcp_update_recv_tstamps() if not needed. 2026-11-04 18:41 ` Eric Dumazet @ 2026-11-04 19:02 ` Kuniyuki Iwashima 0 siblings, 0 replies; 6+ messages in thread From: Kuniyuki Iwashima @ 2026-11-04 19:02 UTC (permalink / raw) To: Eric Dumazet Cc: Neal Cardwell, Matthieu Baerts, Mat Martineau, David S . Miller, Jakub Kicinski, Paolo Abeni, Geliang Tang, Simon Horman, Kuniyuki Iwashima, netdev 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 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 net-next 2/2] mptcp: Don't call tcp_update_recv_tstamps() if not needed. 2026-11-04 16:49 [PATCH v1 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed Kuniyuki Iwashima 2026-11-04 16:49 ` [PATCH v1 net-next 1/2] tcp: " Kuniyuki Iwashima @ 2026-11-04 16:49 ` Kuniyuki Iwashima 1 sibling, 0 replies; 6+ messages in thread From: Kuniyuki Iwashima @ 2026-11-04 16:49 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..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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-04 19:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-04 16:49 [PATCH v1 net-next 0/2] tcp/mptcp: Don't call tcp_update_recv_tstamps() if not needed Kuniyuki Iwashima 2026-11-04 16:49 ` [PATCH v1 net-next 1/2] tcp: " Kuniyuki Iwashima 2026-11-04 18:25 ` Naman Gulati 2026-11-04 18:41 ` Eric Dumazet 2026-11-04 19:02 ` Kuniyuki Iwashima 2026-11-04 16:49 ` [PATCH v1 net-next 2/2] mptcp: " Kuniyuki Iwashima
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox