From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:25
From: Eric Dumazet <edumazet@google.com>
Mostly small improvements in this series.
The notable change is in "defer skb freeing after
socket lock is released" in recvmsg() (and RX zerocopy)
The idea is to try to let skb freeing to BH handler,
whenever possible, or at least perform the freeing
outside of the socket lock section, for much improved
performance. This idea can probably be extended
to other protocols.
Tests on a 100Gbit NIC
Max throughput for one TCP_STREAM flow, over 10 runs.
MTU : 1500 (1428 bytes of TCP payload per MSS)
Before: 55 Gbit
After: 66 Gbit
MTU : 4096+ (4096 bytes of TCP payload, plus TCP/IPv6 headers)
Before: 82 Gbit
After: 95 Gbit
Eric Dumazet (20):
tcp: minor optimization in tcp_add_backlog()
tcp: remove dead code in __tcp_v6_send_check()
tcp: small optimization in tcp_v6_send_check()
net: use sk_is_tcp() in more places
net: remove sk_route_forced_caps
net: remove sk_route_nocaps
ipv6: shrink struct ipcm6_cookie
net: shrink struct sock by 8 bytes
net: forward_alloc_get depends on CONFIG_MPTCP
net: cache align tcp_memory_allocated, tcp_sockets_allocated
tcp: small optimization in tcp recvmsg()
tcp: add RETPOLINE mitigation to sk_backlog_rcv
tcp: annotate data-races on tp->segs_in and tp->data_segs_in
tcp: annotate races around tp->urg_data
tcp: tp->urg_data is unlikely to be set
tcp: avoid indirect calls to sock_rfree
tcp: defer skb freeing after socket lock is released
tcp: check local var (timeo) before socket fields in one test
tcp: do not call tcp_cleanup_rbuf() if we have a backlog
net: move early demux fields close to sk_refcnt
include/linux/skbuff.h | 2 +
include/linux/skmsg.h | 6 ---
include/net/ip6_checksum.h | 12 ++---
include/net/ipv6.h | 4 +-
include/net/sock.h | 51 +++++++++++++--------
include/net/tcp.h | 18 +++++++-
net/core/skbuff.c | 6 +--
net/core/sock.c | 18 +++++---
net/ipv4/tcp.c | 91 ++++++++++++++++++++++++++------------
net/ipv4/tcp_input.c | 8 ++--
net/ipv4/tcp_ipv4.c | 10 ++---
net/ipv4/tcp_output.c | 2 +-
net/ipv4/udp.c | 2 +-
net/ipv6/ip6_output.c | 2 +-
net/ipv6/tcp_ipv6.c | 10 ++---
net/mptcp/protocol.c | 2 +-
16 files changed, 149 insertions(+), 95 deletions(-)
--
2.34.0.rc1.387.gb447b232ab-goog
From: Eric Dumazet <hidden> Date: 2021-11-15 20:08:32
From: Eric Dumazet <edumazet@google.com>
gso_size can be moved after tclass, to use an existing hole.
(8 bytes saved on 64bit arches)
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ipv6.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-15 20:09:31
From: Eric Dumazet <edumazet@google.com>
Instead of using a full netdev_features_t, we can use a single bit,
as sk_route_nocaps is only used to remove NETIF_F_GSO_MASK from
sk->sk_route_cap.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/sock.h | 11 +++++------
net/core/sock.c | 3 ++-
net/ipv4/tcp_ipv4.c | 4 ++--
net/ipv4/tcp_output.c | 2 +-
net/ipv6/ip6_output.c | 2 +-
5 files changed, 11 insertions(+), 11 deletions(-)
@@ -1359,7 +1359,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,#ifdef CONFIG_TCP_MD5SIG/* Calculate the MD5 hash, as we have all we need now */if(md5){-sk_nocaps_add(sk,NETIF_F_GSO_MASK);+sk_gso_disable(sk);tp->af_specific->calc_md5_hash(opts.hash_location,md5,sk,skb);}
From: Eric Dumazet <hidden> Date: 2021-11-15 20:09:37
From: Eric Dumazet <edumazet@google.com>
tcp_segs_in() can be called from BH, while socket spinlock
is held but socket owned by user, eventually reading these
fields from tcp_get_info()
Found by code inspection, no need to backport this patch
to older kernels.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/tcp.h | 8 ++++++--
net/ipv4/tcp.c | 6 ++++--
2 files changed, 10 insertions(+), 4 deletions(-)
@@ -3769,10 +3769,12 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)tcp_get_info_chrono_stats(tp,info);info->tcpi_segs_out=tp->segs_out;-info->tcpi_segs_in=tp->segs_in;++/* segs_in and data_segs_in can be updated from tcp_segs_in() from BH */+info->tcpi_segs_in=READ_ONCE(tp->segs_in);+info->tcpi_data_segs_in=READ_ONCE(tp->data_segs_in);info->tcpi_min_rtt=tcp_min_rtt(tp);-info->tcpi_data_segs_in=tp->data_segs_in;info->tcpi_data_segs_out=tp->data_segs_out;info->tcpi_delivery_rate_app_limited=tp->rate_app_limited?1:0;
From: Eric Dumazet <hidden> Date: 2021-11-15 20:09:37
From: Eric Dumazet <edumazet@google.com>
When reading large chunks of data, incoming packets might
be added to the backlog from BH.
tcp recvmsg() detects the backlog queue is not empty, and uses
a release_sock()/lock_sock() pair to process this backlog.
We now have __sk_flush_backlog() to perform this
a bit faster.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
@@ -2409,8 +2409,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,if(copied>=target){/* Do not sleep, just process backlog. */-release_sock(sk);-lock_sock(sk);+__sk_flush_backlog(sk);}else{sk_wait_data(sk,&timeo,last);}
From: Eric Dumazet <hidden> Date: 2021-11-15 20:09:37
From: Eric Dumazet <edumazet@google.com>
Use some unlikely() hints in the fast path.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 10 +++++-----
net/ipv4/tcp_input.c | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
@@ -1633,7 +1633,7 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,len=skb->len-offset;/* Stop reading if we hit a patch of urgent data */-if(tp->urg_data){+if(unlikely(tp->urg_data)){u32urg_offset=tp->urg_seq-seq;if(urg_offset<len)len=urg_offset;
@@ -2326,7 +2326,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,u32offset;/* Are we at urgent data? Stop if we have read anything or have SIGURG pending. */-if(tp->urg_data&&tp->urg_seq==*seq){+if(unlikely(tp->urg_data)&&tp->urg_seq==*seq){if(copied)break;if(signal_pending(current)){
@@ -2431,7 +2431,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,used=len;/* Do we have urgent data here? */-if(tp->urg_data){+if(unlikely(tp->urg_data)){u32urg_offset=tp->urg_seq-*seq;if(urg_offset<used){if(!urg_offset){
@@ -5604,11 +5604,11 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, const struct tcphdr *tstructtcp_sock*tp=tcp_sk(sk);/* Check if we get a new urgent pointer - normally not. */-if(th->urg)+if(unlikely(th->urg))tcp_check_urg(sk,th);/* Do we wait for any urgent data? - normally not... */-if(tp->urg_data==TCP_URG_NOTYET){+if(unlikely(tp->urg_data==TCP_URG_NOTYET)){u32ptr=tp->urg_seq-ntohl(th->seq)+(th->doff*4)-th->syn;
From: Eric Dumazet <hidden> Date: 2021-11-15 20:10:13
From: Eric Dumazet <edumazet@google.com>
tcp recvmsg() (or rx zerocopy) spends a fair amount of time
freeing skbs after their payload has been consumed.
A typical ~64KB GRO packet has to release ~45 page
references, eventually going to page allocator
for each of them.
Currently, this freeing is performed while socket lock
is held, meaning that there is a high chance that
BH handler has to queue incoming packets to tcp socket backlog.
This can cause additional latencies, because the user
thread has to process the backlog at release_sock() time,
and while doing so, additional frames can be added
by BH handler.
This patch adds logic to defer these frees after socket
lock is released, or directly from BH handler if possible.
Being able to free these skbs from BH handler helps a lot,
because this avoids the usual alloc/free assymetry,
when BH handler and user thread do not run on same cpu or
NUMA node.
One cpu can now be fully utilized for the kernel->user copy,
and another cpu is handling BH processing and skb/page
allocs/frees (assuming RFS is not forcing use of a single CPU)
Tested:
100Gbit NIC
Max throughput for one TCP_STREAM flow, over 10 runs
MTU : 1500
Before: 55 Gbit
After: 66 Gbit
MTU : 4096+(headers)
Before: 82 Gbit
After: 95 Gbit
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/skbuff.h | 2 ++
include/net/sock.h | 3 +++
include/net/tcp.h | 10 ++++++++++
net/ipv4/tcp.c | 27 +++++++++++++++++++++++++--
net/ipv4/tcp_ipv4.c | 1 +
net/ipv6/tcp_ipv6.c | 1 +
6 files changed, 42 insertions(+), 2 deletions(-)
@@ -1580,14 +1580,34 @@ void tcp_cleanup_rbuf(struct sock *sk, int copied)tcp_send_ack(sk);}+void__sk_defer_free_flush(structsock*sk)+{+structllist_node*head;+structsk_buff*skb,*n;++head=llist_del_all(&sk->defer_list);+llist_for_each_entry_safe(skb,n,head,ll_node){+prefetch(n);+skb_mark_not_on_list(skb);+__kfree_skb(skb);+}+}+EXPORT_SYMBOL(__sk_defer_free_flush);+staticvoidtcp_eat_recv_skb(structsock*sk,structsk_buff*skb){+__skb_unlink(skb,&sk->sk_receive_queue);if(likely(skb->destructor==sock_rfree)){sock_rfree(skb);skb->destructor=NULL;skb->sk=NULL;+if(!skb_queue_empty(&sk->sk_receive_queue)||+!llist_empty(&sk->defer_list)){+llist_add(&skb->ll_node,&sk->defer_list);+return;+}}-sk_eat_skb(sk,skb);+__kfree_skb(skb);}staticstructsk_buff*tcp_recv_skb(structsock*sk,u32seq,u32*off)
@@ -2422,6 +2442,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,/* Do not sleep, just process backlog. */__sk_flush_backlog(sk);}else{+sk_defer_free_flush(sk);sk_wait_data(sk,&timeo,last);}
@@ -2540,6 +2561,7 @@ int tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int nonblock,ret=tcp_recvmsg_locked(sk,msg,len,nonblock,flags,&tss,&cmsg_flags);release_sock(sk);+sk_defer_free_flush(sk);if(cmsg_flags&&ret>=0){if(cmsg_flags&TCP_CMSG_TS)
@@ -3065,7 +3087,7 @@ int tcp_disconnect(struct sock *sk, int flags)sk->sk_frag.page=NULL;sk->sk_frag.offset=0;}-+sk_defer_free_flush(sk);sk_error_report(sk);return0;}
@@ -4194,6 +4216,7 @@ static int do_tcp_getsockopt(struct sock *sk, int level,err=BPF_CGROUP_RUN_PROG_GETSOCKOPT_KERN(sk,level,optname,&zc,&len,err);release_sock(sk);+sk_defer_free_flush(sk);if(len>=offsetofend(structtcp_zerocopy_receive,msg_flags))gotozerocopy_rcv_cmsg;switch(len){
From: Eric Dumazet <hidden> Date: 2021-11-15 20:10:14
From: Eric Dumazet <edumazet@google.com>
sk_rx_dst/sk_rx_dst_ifindex/sk_rx_dst_cookie are read in early demux,
and currently spans two cache lines.
Moving them close to sk_refcnt makes more sense, as only one cache
line is needed.
New layout for this hot cache line is :
struct sock {
struct sock_common __sk_common; /* 0 0x88 */
/* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
struct dst_entry * sk_rx_dst; /* 0x88 0x8 */
int sk_rx_dst_ifindex; /* 0x90 0x4 */
u32 sk_rx_dst_cookie; /* 0x94 0x4 */
socket_lock_t sk_lock; /* 0x98 0x20 */
atomic_t sk_drops; /* 0xb8 0x4 */
int sk_rcvlowat; /* 0xbc 0x4 */
/* --- cacheline 3 boundary (192 bytes) --- */
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/sock.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-15 20:10:14
From: Eric Dumazet <edumazet@google.com>
Under pressure, tcp recvmsg() has logic to process the socket backlog,
but calls tcp_cleanup_rbuf() right before.
Avoiding sending ACK right before processing new segments makes
a lot of sense, as this decrease the number of ACK packets,
with no impact on effective ACK clocking.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
@@ -2436,12 +2436,11 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,}}-tcp_cleanup_rbuf(sk,copied);-if(copied>=target){/* Do not sleep, just process backlog. */__sk_flush_backlog(sk);}else{+tcp_cleanup_rbuf(sk,copied);sk_defer_free_flush(sk);sk_wait_data(sk,&timeo,last);}
On Mon, Nov 15, 2021 at 2:02 PM Eric Dumazet [off-list ref] wrote:
From: Eric Dumazet <edumazet@google.com>
Mostly small improvements in this series.
The notable change is in "defer skb freeing after
socket lock is released" in recvmsg() (and RX zerocopy)
The idea is to try to let skb freeing to BH handler,
whenever possible, or at least perform the freeing
outside of the socket lock section, for much improved
performance. This idea can probably be extended
to other protocols.
Tests on a 100Gbit NIC
Max throughput for one TCP_STREAM flow, over 10 runs.
MTU : 1500 (1428 bytes of TCP payload per MSS)
Before: 55 Gbit
After: 66 Gbit
MTU : 4096+ (4096 bytes of TCP payload, plus TCP/IPv6 headers)
Before: 82 Gbit
After: 95 Gbit
Acked-by: Soheil Hassas Yeganeh <redacted>
Wow, this is really impressive. I reviewed all the patches and I can't
point out any issues other than the typo that Arjun has pointed out.
Thank you Eric!
Eric Dumazet (20):
tcp: minor optimization in tcp_add_backlog()
tcp: remove dead code in __tcp_v6_send_check()
tcp: small optimization in tcp_v6_send_check()
net: use sk_is_tcp() in more places
net: remove sk_route_forced_caps
net: remove sk_route_nocaps
ipv6: shrink struct ipcm6_cookie
net: shrink struct sock by 8 bytes
net: forward_alloc_get depends on CONFIG_MPTCP
net: cache align tcp_memory_allocated, tcp_sockets_allocated
tcp: small optimization in tcp recvmsg()
tcp: add RETPOLINE mitigation to sk_backlog_rcv
tcp: annotate data-races on tp->segs_in and tp->data_segs_in
tcp: annotate races around tp->urg_data
tcp: tp->urg_data is unlikely to be set
tcp: avoid indirect calls to sock_rfree
tcp: defer skb freeing after socket lock is released
tcp: check local var (timeo) before socket fields in one test
tcp: do not call tcp_cleanup_rbuf() if we have a backlog
net: move early demux fields close to sk_refcnt
include/linux/skbuff.h | 2 +
include/linux/skmsg.h | 6 ---
include/net/ip6_checksum.h | 12 ++---
include/net/ipv6.h | 4 +-
include/net/sock.h | 51 +++++++++++++--------
include/net/tcp.h | 18 +++++++-
net/core/skbuff.c | 6 +--
net/core/sock.c | 18 +++++---
net/ipv4/tcp.c | 91 ++++++++++++++++++++++++++------------
net/ipv4/tcp_input.c | 8 ++--
net/ipv4/tcp_ipv4.c | 10 ++---
net/ipv4/tcp_output.c | 2 +-
net/ipv4/udp.c | 2 +-
net/ipv6/ip6_output.c | 2 +-
net/ipv6/tcp_ipv6.c | 10 ++---
net/mptcp/protocol.c | 2 +-
16 files changed, 149 insertions(+), 95 deletions(-)
--
2.34.0.rc1.387.gb447b232ab-goog
From: Paolo Abeni <pabeni@redhat.com> Date: 2021-11-15 21:43:48
Hello,
On Mon, 2021-11-15 at 15:37 -0500, Soheil Hassas Yeganeh wrote:
On Mon, Nov 15, 2021 at 2:02 PM Eric Dumazet [off-list ref] wrote:
quoted
From: Eric Dumazet <edumazet@google.com>
Mostly small improvements in this series.
The notable change is in "defer skb freeing after
socket lock is released" in recvmsg() (and RX zerocopy)
The idea is to try to let skb freeing to BH handler,
whenever possible, or at least perform the freeing
outside of the socket lock section, for much improved
performance. This idea can probably be extended
to other protocols.
Tests on a 100Gbit NIC
Max throughput for one TCP_STREAM flow, over 10 runs.
MTU : 1500 (1428 bytes of TCP payload per MSS)
Before: 55 Gbit
After: 66 Gbit
MTU : 4096+ (4096 bytes of TCP payload, plus TCP/IPv6 headers)
Before: 82 Gbit
After: 95 Gbit
Acked-by: Soheil Hassas Yeganeh <redacted>
Wow, this is really impressive. I reviewed all the patches and I can't
point out any issues other than the typo that Arjun has pointed out.
Thank you Eric!
Possibly there has been some issues with the ML while processing these
patches?!? only an handful of them reached patchwork (and my mailbox :)
(/me was just curious about the code ;)
Cheers,
Paolo
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-15 22:57:54
On Mon, Nov 15, 2021 at 1:40 PM Paolo Abeni [off-list ref] wrote:
Possibly there has been some issues with the ML while processing these
patches?!? only an handful of them reached patchwork (and my mailbox :)
Yeah, this sort of thing happens. Let's wait a bit before re-sending ?
Maybe too much traffic today on vger or gmail, I honestly do not know.
I will send the series privately to you in the meantime :)
From: Arjun Roy <hidden> Date: 2021-11-16 00:27:48
On Mon, Nov 15, 2021 at 11:03 AM Eric Dumazet [off-list ref] wrote:
From: Eric Dumazet <edumazet@google.com>
TCP uses sk_eat_skb() when skbs can be removed from receive queue.
However, the call so skb_orphan() from __kfree_skb() incurs
an indirect call so sock_rfee(), which is more expensive than
Possible typo : s/so/to/g ?
-Arjun
quoted hunk
a direct call, especially for CONFIG_RETPOLINE=y.
Add tcp_eat_recv_skb() function to make the call before
__kfree_skb().
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:08
From: Eric Dumazet <edumazet@google.com>
TCP uses sk_eat_skb() when skbs can be removed from receive queue.
However, the call so skb_orphan() from __kfree_skb() incurs
an indirect call so sock_rfee(), which is more expensive than
a direct call, especially for CONFIG_RETPOLINE=y.
Add tcp_eat_recv_skb() function to make the call before
__kfree_skb().
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:08
From: Eric Dumazet <edumazet@google.com>
Testing timeo before sk_err/sk_state/sk_shutdown makes more sense.
Modern applications use non-blocking IO, while a socket is terminated
only once during its life time.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:09
From: Eric Dumazet <edumazet@google.com>
(struct proto)->sk_forward_alloc is currently only used by MPTCP.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/sock.h | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:10
From: Eric Dumazet <edumazet@google.com>
We were only using one bit, and we can replace it by sk_is_tcp()
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/sock.h | 3 ---
net/core/sock.c | 4 +++-
net/ipv4/tcp.c | 1 -
3 files changed, 3 insertions(+), 5 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:11
From: Eric Dumazet <edumazet@google.com>
tcp_poll() and tcp_ioctl() are reading tp->urg_data without socket lock
owned.
Also, it is faster to first check tp->urg_data in tcp_poll(),
then tp->urg_seq == tp->copied_seq, because tp->urg_seq is
located in a different/cold cache line.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 17 +++++++++--------
net/ipv4/tcp_input.c | 4 ++--
2 files changed, 11 insertions(+), 10 deletions(-)
@@ -573,7 +574,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)}elsemask|=EPOLLOUT|EPOLLWRNORM;-if(tp->urg_data&TCP_URG_VALID)+if(urg_data&TCP_URG_VALID)mask|=EPOLLPRI;}elseif(state==TCP_SYN_SENT&&inet_sk(sk)->defer_connect){/* Active TCP fastopen socket with defer_connect
@@ -607,7 +608,7 @@ int tcp_ioctl(struct sock *sk, int cmd, unsigned long arg)unlock_sock_fast(sk,slow);break;caseSIOCATMARK:-answ=tp->urg_data&&+answ=READ_ONCE(tp->urg_data)&&READ_ONCE(tp->urg_seq)==READ_ONCE(tp->copied_seq);break;caseSIOCOUTQ:
@@ -1465,7 +1466,7 @@ static int tcp_recv_urg(struct sock *sk, struct msghdr *msg, int len, int flags)charc=tp->urg_data;if(!(flags&MSG_PEEK))-tp->urg_data=TCP_URG_READ;+WRITE_ONCE(tp->urg_data,TCP_URG_READ);/* Read urgent data. */msg->msg_flags|=MSG_OOB;
@@ -2959,7 +2960,7 @@ int tcp_disconnect(struct sock *sk, int flags)tcp_clear_xmit_timers(sk);__skb_queue_purge(&sk->sk_receive_queue);WRITE_ONCE(tp->copied_seq,tp->rcv_nxt);-tp->urg_data=0;+WRITE_ONCE(tp->urg_data,0);tcp_write_queue_purge(sk);tcp_fastopen_active_disable_ofo_check(sk);skb_rbtree_purge(&tp->out_of_order_queue);
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:12
From: Eric Dumazet <edumazet@google.com>
For some reason, I forgot to change __tcp_v6_send_check() at
the same time I removed (ip_summed == CHECKSUM_PARTIAL) check
in __tcp_v4_send_check()
Fixes: 98be9b12096f ("tcp: remove dead code after CHECKSUM_PARTIAL adoption")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip6_checksum.h | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
@@ -872,8 +872,7 @@ int sock_set_timestamping(struct sock *sk, int optname,if(val&SOF_TIMESTAMPING_OPT_ID&&!(sk->sk_tsflags&SOF_TIMESTAMPING_OPT_ID)){-if(sk->sk_protocol==IPPROTO_TCP&&-sk->sk_type==SOCK_STREAM){+if(sk_is_tcp(sk)){if((1<<sk->sk_state)&(TCPF_CLOSE|TCPF_LISTEN))return-EINVAL;
@@ -1370,8 +1369,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,caseSO_ZEROCOPY:if(sk->sk_family==PF_INET||sk->sk_family==PF_INET6){-if(!((sk->sk_type==SOCK_STREAM&&-sk->sk_protocol==IPPROTO_TCP)||+if(!(sk_is_tcp(sk)||(sk->sk_type==SOCK_DGRAM&&sk->sk_protocol==IPPROTO_UDP)))ret=-ENOTSUPP;
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:14
From: Eric Dumazet <edumazet@google.com>
Move sk_bind_phc next to sk_peer_lock to fill a hole.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/sock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:16
From: Eric Dumazet <edumazet@google.com>
tcp_memory_allocated and tcp_sockets_allocated often share
a common cache line, source of false sharing.
Also take care of udp_memory_allocated and mptcp_sockets_allocated.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 4 ++--
net/ipv4/udp.c | 2 +-
net/mptcp/protocol.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:22
From: Eric Dumazet <edumazet@google.com>
For TCP flows, inet6_sk(sk)->saddr has the same value
than sk->sk_v6_rcv_saddr.
Using sk->sk_v6_rcv_saddr increases data locality.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/tcp_ipv6.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
From: Eric Dumazet <hidden> Date: 2021-11-16 00:28:26
From: Eric Dumazet <edumazet@google.com>
If packet is going to be coalesced, sk_sndbuf/sk_rcvbuf values
are not used. Defer their access to the point we need them.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_ipv4.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-16 05:04:49
On Mon, Nov 15, 2021 at 1:47 PM Eric Dumazet [off-list ref] wrote:
On Mon, Nov 15, 2021 at 1:40 PM Paolo Abeni [off-list ref] wrote:
quotedquoted
Possibly there has been some issues with the ML while processing these
patches?!? only an handful of them reached patchwork (and my mailbox :)
Yeah, this sort of thing happens. Let's wait a bit before re-sending ?
Maybe too much traffic today on vger or gmail, I honestly do not know.
I will send the series privately to you in the meantime :)
From: David Ahern <hidden> Date: 2021-11-16 05:35:32
On 11/15/21 12:02 PM, Eric Dumazet wrote:
From: Eric Dumazet <edumazet@google.com>
For some reason, I forgot to change __tcp_v6_send_check() at
the same time I removed (ip_summed == CHECKSUM_PARTIAL) check
in __tcp_v4_send_check()
Fixes: 98be9b12096f ("tcp: remove dead code after CHECKSUM_PARTIAL adoption")
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-16 05:38:10
On Mon, Nov 15, 2021 at 6:48 PM David Ahern [off-list ref] wrote:
On 11/15/21 12:02 PM, Eric Dumazet wrote:
quoted
From: Eric Dumazet <edumazet@google.com>
For some reason, I forgot to change __tcp_v6_send_check() at
the same time I removed (ip_summed == CHECKSUM_PARTIAL) check
in __tcp_v4_send_check()
Fixes: 98be9b12096f ("tcp: remove dead code after CHECKSUM_PARTIAL adoption")
Given the Fixes, should this go one through -net?
It is only removing dead code, and going through net-next is not a big deal.
No real 'bug', just a way for me to point out that we had a similar
change in IPv4 for years and nothing bad happened.
From: Arjun Roy <hidden> Date: 2021-11-16 05:46:36
On Mon, Nov 15, 2021 at 6:06 PM Eric Dumazet [off-list ref] wrote:
On Mon, Nov 15, 2021 at 1:47 PM Eric Dumazet [off-list ref] wrote:
quoted
On Mon, Nov 15, 2021 at 1:40 PM Paolo Abeni [off-list ref] wrote:
quotedquoted
Possibly there has been some issues with the ML while processing these
patches?!? only an handful of them reached patchwork (and my mailbox :)
Yeah, this sort of thing happens. Let's wait a bit before re-sending ?
Maybe too much traffic today on vger or gmail, I honestly do not know.
I will send the series privately to you in the meantime :)
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-11-16 14:27:39
On Mon, 15 Nov 2021 11:02:46 -0800 Eric Dumazet wrote:
One cpu can now be fully utilized for the kernel->user copy,
and another cpu is handling BH processing and skb/page
allocs/frees (assuming RFS is not forcing use of a single CPU)
Are you saying the kernel->user copy is not under the socket lock
today? I'm working on getting the crypto & copy from under the socket
lock for ktls, and it looked like tcp does the copy under the lock.
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-16 15:07:17
On Tue, Nov 16, 2021 at 6:27 AM Jakub Kicinski [off-list ref] wrote:
On Mon, 15 Nov 2021 11:02:46 -0800 Eric Dumazet wrote:
quoted
One cpu can now be fully utilized for the kernel->user copy,
and another cpu is handling BH processing and skb/page
allocs/frees (assuming RFS is not forcing use of a single CPU)
Are you saying the kernel->user copy is not under the socket lock
today? I'm working on getting the crypto & copy from under the socket
lock for ktls, and it looked like tcp does the copy under the lock.
Copy is done currently with socket lock owned.
But each skb is freed one at a time, after its payload has been consumed.
Note that I am also working on performing the copy while still allowing BH
to process incoming packets.
This is a bit more complex, but I think it is doable.
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-11-16 15:21:01
On Tue, 16 Nov 2021 07:05:54 -0800 Eric Dumazet wrote:
On Tue, Nov 16, 2021 at 6:27 AM Jakub Kicinski [off-list ref] wrote:
quoted
On Mon, 15 Nov 2021 11:02:46 -0800 Eric Dumazet wrote:
quoted
One cpu can now be fully utilized for the kernel->user copy,
and another cpu is handling BH processing and skb/page
allocs/frees (assuming RFS is not forcing use of a single CPU)
Are you saying the kernel->user copy is not under the socket lock
today? I'm working on getting the crypto & copy from under the socket
lock for ktls, and it looked like tcp does the copy under the lock.
Copy is done currently with socket lock owned.
But each skb is freed one at a time, after its payload has been consumed.
Note that I am also working on performing the copy while still allowing BH
to process incoming packets.
This is a bit more complex, but I think it is doable.
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-16 15:22:39
On Tue, Nov 16, 2021 at 7:05 AM Eric Dumazet [off-list ref] wrote:
On Tue, Nov 16, 2021 at 6:27 AM Jakub Kicinski [off-list ref] wrote:
quoted
On Mon, 15 Nov 2021 11:02:46 -0800 Eric Dumazet wrote:
quoted
One cpu can now be fully utilized for the kernel->user copy,
and another cpu is handling BH processing and skb/page
allocs/frees (assuming RFS is not forcing use of a single CPU)
Are you saying the kernel->user copy is not under the socket lock
today? I'm working on getting the crypto & copy from under the socket
lock for ktls, and it looked like tcp does the copy under the lock.
Copy is done currently with socket lock owned.
But each skb is freed one at a time, after its payload has been consumed.
Note that I am also working on performing the copy while still allowing BH
to process incoming packets.
This is a bit more complex, but I think it is doable.
Here is the perf top profile on cpu used by user thread doing the
recvmsg(), at 96 Gbit/s
We no longer see skb freeing related costs, but we still see costs of
having to process the backlog.
81.06% [kernel] [k] copy_user_enhanced_fast_string
2.50% [kernel] [k] __skb_datagram_iter
2.25% [kernel] [k] _copy_to_iter
1.45% [kernel] [k] tcp_recvmsg_locked
1.39% [kernel] [k] tcp_rcv_established
0.93% [kernel] [k] skb_try_coalesce
0.79% [kernel] [k] sock_rfree
0.72% [kernel] [k] tcp_v6_do_rcv
0.57% [kernel] [k] skb_release_data
0.50% [kernel] [k] tcp_queue_rcv
0.43% [kernel] [k] __direct_call_clocksource_read1
0.43% [kernel] [k] __release_sock
0.39% [kernel] [k] _raw_spin_lock
0.25% [kernel] [k] __direct_call_hrtimer_clock_base_get_time1
0.20% [kernel] [k] __tcp_transmit_skb
0.19% [kernel] [k] __dev_queue_xmit
0.18% [kernel] [k] __tcp_select_window
0.18% [kernel] [k] _raw_spin_lock_bh
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-11-16 15:27:45
On Tue, 16 Nov 2021 07:22:02 -0800 Eric Dumazet wrote:
Here is the perf top profile on cpu used by user thread doing the
recvmsg(), at 96 Gbit/s
We no longer see skb freeing related costs, but we still see costs of
having to process the backlog.
81.06% [kernel] [k] copy_user_enhanced_fast_string
2.50% [kernel] [k] __skb_datagram_iter
2.25% [kernel] [k] _copy_to_iter
1.45% [kernel] [k] tcp_recvmsg_locked
1.39% [kernel] [k] tcp_rcv_established
Huh, somehow I assumed your 4k MTU numbers were with zero-copy :o
Out of curiosity - what's the softirq load with 4k? Do you have an
idea what the load is on the CPU consuming the data vs the softirq
processing with 1500B ?
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-16 16:46:54
On Tue, Nov 16, 2021 at 7:27 AM Jakub Kicinski [off-list ref] wrote:
On Tue, 16 Nov 2021 07:22:02 -0800 Eric Dumazet wrote:
quoted
Here is the perf top profile on cpu used by user thread doing the
recvmsg(), at 96 Gbit/s
We no longer see skb freeing related costs, but we still see costs of
having to process the backlog.
81.06% [kernel] [k] copy_user_enhanced_fast_string
2.50% [kernel] [k] __skb_datagram_iter
2.25% [kernel] [k] _copy_to_iter
1.45% [kernel] [k] tcp_recvmsg_locked
1.39% [kernel] [k] tcp_rcv_established
Huh, somehow I assumed your 4k MTU numbers were with zero-copy :o
Out of curiosity - what's the softirq load with 4k? Do you have an
idea what the load is on the CPU consuming the data vs the softirq
processing with 1500B ?
On my testing host,
4K MTU : processing ~2,600.000 packets per second in GRO and other parts
use about 60% of the core in BH.
(Some of this cost comes from a clang issue, and the csum_partial() one
I was working on last week)
NIC RX interrupts are firing about 25,000 times per second in this setup.
1500 MTU : processing ~ 5,800,000 packets per second uses one core in
BH (and also one core in recvmsg()),
We stay in NAPI mode (no IRQ rearming)
(That was with a TCP_STREAM run sustaining 70Gbit)
BH numbers also depend on IRQ coalescing parameters.
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-11-16 18:19:02
On Tue, 16 Nov 2021 08:46:37 -0800 Eric Dumazet wrote:
On my testing host,
4K MTU : processing ~2,600.000 packets per second in GRO and other parts
use about 60% of the core in BH.
(Some of this cost comes from a clang issue, and the csum_partial() one
I was working on last week)
NIC RX interrupts are firing about 25,000 times per second in this setup.
1500 MTU : processing ~ 5,800,000 packets per second uses one core in
BH (and also one core in recvmsg()),
We stay in NAPI mode (no IRQ rearming)
(That was with a TCP_STREAM run sustaining 70Gbit)
BH numbers also depend on IRQ coalescing parameters.
Very interesting, curious to see what not doing the copy under socket
lock will do to the 1.5k case.
Thanks a lot for sharing the detailed info!
From: David Ahern <hidden> Date: 2021-11-16 20:45:22
On 11/16/21 9:46 AM, Eric Dumazet wrote:
On Tue, Nov 16, 2021 at 7:27 AM Jakub Kicinski [off-list ref] wrote:
quoted
On Tue, 16 Nov 2021 07:22:02 -0800 Eric Dumazet wrote:
quoted
Here is the perf top profile on cpu used by user thread doing the
recvmsg(), at 96 Gbit/s
We no longer see skb freeing related costs, but we still see costs of
having to process the backlog.
81.06% [kernel] [k] copy_user_enhanced_fast_string
2.50% [kernel] [k] __skb_datagram_iter
2.25% [kernel] [k] _copy_to_iter
1.45% [kernel] [k] tcp_recvmsg_locked
1.39% [kernel] [k] tcp_rcv_established
Huh, somehow I assumed your 4k MTU numbers were with zero-copy :o
I thought the same. :-)
quoted
Out of curiosity - what's the softirq load with 4k? Do you have an
idea what the load is on the CPU consuming the data vs the softirq
processing with 1500B ?
On my testing host,
4K MTU : processing ~2,600.000 packets per second in GRO and other parts
use about 60% of the core in BH.
4kB or 4kB+hdr MTU? I ask because there is a subtle difference in the
size of the GRO packet which affects overall efficiency.
e.g., at 1500 MTU, 1448 MSS, a GRO packet has at most 45 segments for a
GRO size of 65212. At 4000 MTU, 3948 MSS, a GRO packet has at most 16
segments for a GRO packet size of 63220. I have noticed that 3300 MTU is
a bit of sweet spot with MLX5/ConnectX-5 at least - 20 segments and
65012 GRO packet without triggering nonlinear mode.
(Some of this cost comes from a clang issue, and the csum_partial() one
I was working on last week)
NIC RX interrupts are firing about 25,000 times per second in this setup.
1500 MTU : processing ~ 5,800,000 packets per second uses one core in
BH (and also one core in recvmsg()),
We stay in NAPI mode (no IRQ rearming)
(That was with a TCP_STREAM run sustaining 70Gbit)
BH numbers also depend on IRQ coalescing parameters.
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-16 21:35:46
On Tue, Nov 16, 2021 at 12:45 PM David Ahern [off-list ref] wrote:
On 11/16/21 9:46 AM, Eric Dumazet wrote:
quoted
On Tue, Nov 16, 2021 at 7:27 AM Jakub Kicinski [off-list ref] wrote:
quoted
On Tue, 16 Nov 2021 07:22:02 -0800 Eric Dumazet wrote:
quoted
Here is the perf top profile on cpu used by user thread doing the
recvmsg(), at 96 Gbit/s
We no longer see skb freeing related costs, but we still see costs of
having to process the backlog.
81.06% [kernel] [k] copy_user_enhanced_fast_string
2.50% [kernel] [k] __skb_datagram_iter
2.25% [kernel] [k] _copy_to_iter
1.45% [kernel] [k] tcp_recvmsg_locked
1.39% [kernel] [k] tcp_rcv_established
Huh, somehow I assumed your 4k MTU numbers were with zero-copy :o
I thought the same. :-)
quoted
quoted
Out of curiosity - what's the softirq load with 4k? Do you have an
idea what the load is on the CPU consuming the data vs the softirq
processing with 1500B ?
On my testing host,
4K MTU : processing ~2,600.000 packets per second in GRO and other parts
use about 60% of the core in BH.
4kB or 4kB+hdr MTU? I ask because there is a subtle difference in the
size of the GRO packet which affects overall efficiency.
e.g., at 1500 MTU, 1448 MSS, a GRO packet has at most 45 segments for a
GRO size of 65212. At 4000 MTU, 3948 MSS, a GRO packet has at most 16
segments for a GRO packet size of 63220. I have noticed that 3300 MTU is
a bit of sweet spot with MLX5/ConnectX-5 at least - 20 segments and
65012 GRO packet without triggering nonlinear mode.
We are using 4096 bytes of payload, to enable TCP RX zero copy if
receiver wants it.
(even if in this case I was using TCP_STREAM which does a standard recvmsg())
Yes, the TSO/GRO standard limit in this case is 15*4K = 61440, but also remember
we are working on BIG TCP packets, so we do not have to find a 'sweet spot' :)
With BIG TCP enabled, I am sending/receiving TSO/GRO packets with 45
4K segments, (184320 bytes of payload).
(But the results I gave in this thread were with standard TSO/GRO limits)
quoted
(Some of this cost comes from a clang issue, and the csum_partial() one
I was working on last week)
NIC RX interrupts are firing about 25,000 times per second in this setup.
1500 MTU : processing ~ 5,800,000 packets per second uses one core in
BH (and also one core in recvmsg()),
We stay in NAPI mode (no IRQ rearming)
(That was with a TCP_STREAM run sustaining 70Gbit)
BH numbers also depend on IRQ coalescing parameters.