udp: refactor memory accounting
Subsystems (6)
v5 cover udp: refactor memory accounting
This patch series refactor the udp memory accounting, replacing the generic implementation with a custom one, in order to remove the needs for locking the socket on the enqueue and dequeue operations. The socket backlog usage is dropped, as well.
The first patch factor out pieces of some queue and memory management socket helpers, so that they can later be used by the udp memory accounting functions.
The second patch adds the memory account helpers, without using them. The third patch replacse the old rx memory accounting path for udp over ipv4 and udp over ipv6. In kernel UDP users are updated, as well.
The memory accounting schema is described in detail in the individual patch commit message.
The performance gain depends on the specific scenario; with few flows (and little contention in the original code) the differences are in the noise range, while with several flows contending the same socket, the measured speed-up is relevant (e.g. even over 100% in case of extreme contention)
v4 -> v5:
- use the receive queue spin lock to protect the memory accounting
- several minor clean-up
v3 -> v4:
- simplified the locking schema, always use a plain spinlock
v2 -> v3:
- do not set the now unsed backlog_rcv callback
v1 -> v2:
- changed slighly the memory accounting schema, we now perform lazy reclaim
- fixed forward_alloc updating issue
- fixed memory counter integer overflows
Paolo Abeni (3):
net/socket: factor out helpers for memory and queue manipulation
udp: implement memory accounting helpers
udp: use it's own memory accounting schema
include/net/sock.h | 4 ++ include/net/udp.h | 4 ++ net/core/datagram.c | 36 +++++++----- net/core/sock.c | 64 ++++++++++++++------- net/ipv4/udp.c | 151 ++++++++++++++++++++++++++++++++++++++------------ net/ipv6/udp.c | 34 +++--------- net/sunrpc/svcsock.c | 20 +++++-- net/sunrpc/xprtsock.c | 2 +- 8 files changed, 216 insertions(+), 99 deletions(-)
Signature · 6 lines
1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): cover.1476979679.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
PATCH v5 2/3 udp: implement memory accounting helpers
Avoid using the generic helpers.
Use the receive queue spin lock to protect the memory accounting operation, both on enqueue and on dequeue.
On dequeue perform partial memory reclaiming, trying to leave a quantum of forward allocated memory.
On enqueue use a custom helper, to allow some optimizations:
- use a plain spin_lock() variant instead of the slightly costly spin_lock_irqsave(),
- avoid dst_force check, since the calling code has already dropped the skb dst
The above needs custom memory reclaiming on shutdown, provided by the udp_destruct_sock().
v4 -> v5:
- replace the mem_lock with the receive queue spin lock
- ensure that the bh is always allowed to enqueue at least a skb, even if sk_rcvbuf is exceeded
v3 -> v4:
- reworked memory accunting, simplifying the schema
- provide an helper for both memory scheduling and enqueuing
v1 -> v2:
- use a udp specific destrctor to perform memory reclaiming
- remove a couple of helpers, unneeded after the above cleanup
- do not reclaim memory on dequeue if not under memory pressure
- reworked the fwd accounting schema to avoid potential integer overflow
--- include/net/udp.h | 4 ++ net/ipv4/udp.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+)
include/net/udp.h +4 -0
@@ -246,6 +246,9 @@ static inline __be16 udp_flow_src_port(struct net *net, struct sk_buff *skb, }/* net/ipv4/udp.c */ +void skb_consume_udp (struct sock *sk,struct sk_buff *skb,int len); +int __udp_enqueue_schedule_skb (struct sock *sk,struct sk_buff *skb); +void udp_v4_early_demux (struct sk_buff *skb); int udp_get_port(struct sock *sk, unsigned short snum, int (*saddr_cmp)(const struct sock *,
@@ -258,6 +261,7 @@ int udp_get_port(struct sock *sk, unsigned short snum,void udp4_hwcsum (struct sk_buff *skb,__be32 src,__be32 dst);int udp_rcv (struct sk_buff *skb);int udp_ioctl (struct sock *sk,int cmd,unsigned long arg); +int udp_init_sock (struct sock *sk);int udp_disconnect (struct sock *sk,int flags);unsigned int udp_poll (struct file *file,struct socket *sock,poll_table *wait); struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
net/ipv4/udp.c +108 -0
114 lines not on this page · raw message
Signature · 6 lines
1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): d932905c2581df9415449216eceeed1566290e3e.1476979679.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Re: [PATCH net-next v5 2/3] udp: implement memory accounting helpers
Quoted 12 lines · Paolo Abeni
net/ipv4/udp.c +12
-0+ +int __udp_enqueue_schedule_skb (struct sock *sk,struct sk_buff *skb) +{ +struct sk_buff_head *list = &sk->sk_receive_queue; +int rmem, delta, amt, err = -ENOMEM ; +int size = skb->truesize; + +/* try to avoid the costly atomic add/sub pair when the receive +* queue is full; always allow at least a packet +*/ + rmem =atomic_read (&sk->sk_rmem_alloc); +if (rmem && (rmem + size > sk->sk_rcvbuf))
net/ipv4/udp.c int udp_sendpage(struct sock *sk, struct page *page, int offset, · line 1216
Quoted 10 lines
net/ipv4/udp.c +10
-0+ goto drop; + +/* we drop only if the receive buf is full and the receive +* queue contains some other skb +*/ + rmem =atomic_add_return (size, &sk->sk_rmem_alloc); +if ((rmem > sk->sk_rcvbuf) && (rmem > size)) + goto uncharge_drop; + +skb_orphan (skb);
Minor point :
UDP should already have orphaned skbs ? (it uses skb_steal_sock())
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 1476997834.7065.35.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org
Re: [PATCH net-next v5 2/3] udp: implement memory accounting helpers
Quoted 27 lines · Eric Dumazet
Quoted 12 lines · Paolo Abeni
net/ipv4/udp.c +12
-0+ +int __udp_enqueue_schedule_skb (struct sock *sk,struct sk_buff *skb) +{ +struct sk_buff_head *list = &sk->sk_receive_queue; +int rmem, delta, amt, err = -ENOMEM ; +int size = skb->truesize; + +/* try to avoid the costly atomic add/sub pair when the receive +* queue is full; always allow at least a packet +*/ + rmem =atomic_read (&sk->sk_rmem_alloc); +if (rmem && (rmem + size > sk->sk_rcvbuf))net/ipv4/udp.c int udp_sendpage(struct sock *sk, struct page *page, int offset, · line 1216
Quoted 10 lines
net/ipv4/udp.c +10
-0+ goto drop; + +/* we drop only if the receive buf is full and the receive +* queue contains some other skb +*/ + rmem =atomic_add_return (size, &sk->sk_rmem_alloc); +if ((rmem > sk->sk_rcvbuf) && (rmem > size)) + goto uncharge_drop; + +skb_orphan (skb);Minor point :
Quoted 1 line
UDP should already have orphaned skbs ? (it uses
skb_steal_sock())
You are right, the skb_orphan() call is redundant.
I'll send the v6 soon.
Thank you,
Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v5 3/3 udp: use it's own memory accounting schema
In reply to a message not in the archiveCompletely avoid default sock memory accounting and replace it with udp-specific accounting.
Since the new memory accounting model encapsulates completely the required locking, remove the socket lock on both enqueue and dequeue, and avoid using the backlog on enqueue.
Be sure to clean-up rx queue memory on socket destruction, using udp its own sk_destruct.
Tested using pktgen with random src port, 64 bytes packet, wire-speed on a 10G link as sender and udp_sink as the receiver, using an l4 tuple rxhash to stress the contention, and one or more udp_sink instances with reuseport.
nr readers Kpps (vanilla) Kpps (patched) 1 170 440 3 1250 2150 6 3000 3650 9 4200 4450 12 5700 6250
v4 -> v5:
- avoid unneeded test in first_packet_length
v3 -> v4:
- remove useless sk_rcvqueues_full() call
v2 -> v3:
- do not set the now unsed backlog_rcv callback
v1 -> v2:
- add memory pressure support
- fixed dropwatch accounting for ipv6
--- net/ipv4/udp.c | 43 ++++++++----------------------------------- net/ipv6/udp.c | 34 ++++++++-------------------------- net/sunrpc/svcsock.c | 20 ++++++++++++++++---- net/sunrpc/xprtsock.c | 2 +- 4 files changed, 33 insertions(+), 66 deletions(-)
net/ipv4/udp.c +8 -35
@@ -1309,13 +1309,7 @@ static int first_packet_length(struct sock *sk) res = skb ? skb->len :-1 ;spin_unlock_bh (&rcvq->lock); -if (!skb_queue_empty (&list_kill)) { -bool slow =lock_sock_fast (sk); - -__skb_queue_purge (&list_kill); -sk_mem_reclaim_partial (sk); -unlock_sock_fast (sk, slow); - } +__skb_queue_purge (&list_kill);return res; }
@@ -1364,7 +1358,6 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,int err;int is_udplite =IS_UDPLITE (sk);bool checksum_valid = false; -bool slow;if (flags &MSG_ERRQUEUE )return ip_recv_error (sk, msg, len, addr_len);
@@ -1405,13 +1398,12 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock, }if (unlikely (err)) { -trace_kfree_skb (skb, udp_recvmsg);if (!peeked) {atomic_inc (&sk->sk_drops);UDP_INC_STATS (sock_net (sk),UDP_MIB_INERRORS , is_udplite); } -skb_free_datagram_locked (sk, skb); +kfree_skb (skb);return err; }
@@ -1436,16 +1428,15 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,if (flags &MSG_TRUNC ) err = ulen; -__skb_free_datagram_locked (sk, skb, peeking ? -err : err); +skb_consume_udp (sk, skb, peeking ? -err : err);return err; csum_copy_err: - slow =lock_sock_fast (sk); -if (!skb_kill_datagram (sk, skb, flags)) { +if (!__sk_queue_drop_skb (sk, skb, flags)) {UDP_INC_STATS (sock_net (sk),UDP_MIB_CSUMERRORS , is_udplite);UDP_INC_STATS (sock_net (sk),UDP_MIB_INERRORS , is_udplite); } -unlock_sock_fast (sk, slow); +kfree_skb (skb);/* starting over for a new packet, but check if we need to yield */ cond_resched ();
@@ -1564,7 +1555,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)sk_incoming_cpu_update (sk); } - rc =__sock_queue_rcv_skb (sk, skb); + rc =__udp_enqueue_schedule_skb (sk, skb);if (rc <0 ) {int is_udplite =IS_UDPLITE (sk);
@@ -1579,7 +1570,6 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) }return 0 ; - }static struct static_key udp_encap_needed__read_mostly ;
@@ -1601,7 +1591,6 @@ void udp_encap_enable(void)int udp_queue_rcv_skb (struct sock *sk,struct sk_buff *skb) {struct udp_sock *up =udp_sk (sk); -int rc;int is_udplite =IS_UDPLITE (sk);/*
@@ -1688,25 +1677,9 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) goto drop;udp_csum_pull_header (skb); -if (sk_rcvqueues_full (sk, sk->sk_rcvbuf)) { -__UDP_INC_STATS (sock_net (sk),UDP_MIB_RCVBUFERRORS , - is_udplite); - goto drop; - } - - rc =0 ;ipv4_pktinfo_prepare (sk, skb); -bh_lock_sock (sk); -if (!sock_owned_by_user (sk)) - rc =__udp_queue_rcv_skb (sk, skb); -else if (sk_add_backlog (sk, skb, sk->sk_rcvbuf)) { -bh_unlock_sock (sk); - goto drop; - } -bh_unlock_sock (sk); - -return rc; +return __udp_queue_rcv_skb (sk, skb); csum_error:__UDP_INC_STATS (sock_net (sk),UDP_MIB_CSUMERRORS , is_udplite);
@@ -2316,13 +2289,13 @@ struct proto udp_prot = { .connect = ip4_datagram_connect, .disconnect = udp_disconnect, .ioctl = udp_ioctl, + .init = udp_init_sock, .destroy = udp_destroy_sock, .setsockopt = udp_setsockopt, .getsockopt = udp_getsockopt, .sendmsg = udp_sendmsg, .recvmsg = udp_recvmsg, .sendpage = udp_sendpage, - .backlog_rcv = __udp_queue_rcv_skb, .release_cb = ip4_datagram_release_cb, .hash = udp_lib_hash, .unhash = udp_lib_unhash,
net/ipv6/udp.c +8 -26
@@ -334,7 +334,6 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,int is_udplite =IS_UDPLITE (sk);bool checksum_valid = false;int is_udp4; -bool slow;if (flags &MSG_ERRQUEUE )return ipv6_recv_error (sk, msg, len, addr_len);
@@ -378,7 +377,6 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, goto csum_copy_err; }if (unlikely (err)) { -trace_kfree_skb (skb, udpv6_recvmsg);if (!peeked) {atomic_inc (&sk->sk_drops);if (is_udp4)
@@ -388,7 +386,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,UDP6_INC_STATS (sock_net (sk),UDP_MIB_INERRORS , is_udplite); } -skb_free_datagram_locked (sk, skb); +kfree_skb (skb);return err; }if (!peeked) {
@@ -437,12 +435,11 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,if (flags &MSG_TRUNC ) err = ulen; -__skb_free_datagram_locked (sk, skb, peeking ? -err : err); +skb_consume_udp (sk, skb, peeking ? -err : err);return err; csum_copy_err: - slow =lock_sock_fast (sk); -if (!skb_kill_datagram (sk, skb, flags)) { +if (!__sk_queue_drop_skb (sk, skb, flags)) {if (is_udp4) {UDP_INC_STATS (sock_net (sk),UDP_MIB_CSUMERRORS , is_udplite);
@@ -455,7 +452,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, UDP_MIB_INERRORS, is_udplite); } } -unlock_sock_fast (sk, slow); +kfree_skb (skb);/* starting over for a new packet, but check if we need to yield */ cond_resched ();
@@ -523,7 +520,7 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)sk_incoming_cpu_update (sk); } - rc =__sock_queue_rcv_skb (sk, skb); + rc =__udp_enqueue_schedule_skb (sk, skb);if (rc <0 ) {int is_udplite =IS_UDPLITE (sk);
@@ -535,6 +532,7 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)kfree_skb (skb);return -1 ; } +return 0 ; }
@@ -556,7 +554,6 @@ void udpv6_encap_enable(void)int udpv6_queue_rcv_skb (struct sock *sk,struct sk_buff *skb) {struct udp_sock *up =udp_sk (sk); -int rc;int is_udplite =IS_UDPLITE (sk);if (!xfrm6_policy_check (sk,XFRM_POLICY_IN , skb))
@@ -622,25 +619,10 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) goto drop;udp_csum_pull_header (skb); -if (sk_rcvqueues_full (sk, sk->sk_rcvbuf)) { -__UDP6_INC_STATS (sock_net (sk), -UDP_MIB_RCVBUFERRORS , is_udplite); - goto drop; - }skb_dst_drop (skb); -bh_lock_sock (sk); - rc =0 ; -if (!sock_owned_by_user (sk)) - rc =__udpv6_queue_rcv_skb (sk, skb); -else if (sk_add_backlog (sk, skb, sk->sk_rcvbuf)) { -bh_unlock_sock (sk); - goto drop; - } -bh_unlock_sock (sk); - -return rc; +return __udpv6_queue_rcv_skb (sk, skb); csum_error:__UDP6_INC_STATS (sock_net (sk),UDP_MIB_CSUMERRORS , is_udplite);
@@ -1433,12 +1415,12 @@ struct proto udpv6_prot = { .connect = ip6_datagram_connect, .disconnect = udp_disconnect, .ioctl = udp_ioctl, + .init = udp_init_sock, .destroy = udpv6_destroy_sock, .setsockopt = udpv6_setsockopt, .getsockopt = udpv6_getsockopt, .sendmsg = udpv6_sendmsg, .recvmsg = udpv6_recvmsg, - .backlog_rcv = __udpv6_queue_rcv_skb, .release_cb = ip6_datagram_release_cb, .hash = udp_lib_hash, .unhash = udp_lib_unhash,
net/sunrpc/svcsock.c +16 -4
@@ -39,6 +39,7 @@#include <net/checksum.h> #include <net/ip.h> #include <net/ipv6.h> +#include <net/udp.h> #include <net/tcp.h> #include <net/tcp_states.h> #include <asm/uaccess.h>
@@ -129,6 +130,18 @@ static void svc_release_skb(struct svc_rqst *rqstp) } } +static void svc_release_udp_skb (struct svc_rqst *rqstp) +{ +struct sk_buff *skb = rqstp->rq_xprt_ctxt; + +if (skb) { + rqstp->rq_xprt_ctxt =NULL ; + +dprintk ("svc: service %p, releasing skb %p\n" , rqstp, skb); +consume_skb (skb); + } +} +union svc_pktinfo_u {struct in_pktinfo pkti;struct in6_pktinfo pkti6;
@@ -575,7 +588,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp) goto out_free; }local_bh_enable (); -skb_free_datagram_locked (svsk->sk_sk, skb); +consume_skb (skb); }else {/* we can use it in-place */ rqstp->rq_arg.head[0 ].iov_base = skb->data;
@@ -602,8 +615,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)return len; out_free: -trace_kfree_skb (skb, svc_udp_recvfrom); -skb_free_datagram_locked (svsk->sk_sk, skb); +kfree_skb (skb);return 0 ; }
@@ -660,7 +672,7 @@ static struct svc_xprt *svc_udp_create(struct svc_serv *serv, .xpo_create = svc_udp_create, .xpo_recvfrom = svc_udp_recvfrom, .xpo_sendto = svc_udp_sendto, - .xpo_release_rqst = svc_release_skb, + .xpo_release_rqst = svc_release_udp_skb, .xpo_detach = svc_sock_detach, .xpo_free = svc_sock_free, .xpo_prep_reply_hdr = svc_udp_prep_reply_hdr,
net/sunrpc/xprtsock.c +1 -1
@@ -1074,7 +1074,7 @@ static void xs_udp_data_receive(struct sock_xprt *transport) skb =skb_recv_datagram (sk,0 ,1 , &err);if (skb !=NULL ) {xs_udp_data_read_skb (&transport->xprt, sk, skb); -skb_free_datagram_locked (sk, skb); +consume_skb (skb);continue ; }if (!test_and_clear_bit (XPRT_SOCK_DATA_READY , &transport->sock_state))
Signature · 6 lines
1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Abeni
maintainerPATCH v5 1/3 net/socket: factor out helpers for memory and queue manipulation
In reply to Paolo AbeniBasic sock operations that udp code can use with its own memory accounting schema. No functional change is introduced in the existing APIs.
v4 -> v5:
- avoid whitespace changes
v2 -> v4:
- avoid exporting __sock_enqueue_skb
v1 -> v2:
- avoid export sock_rmem_free
--- include/net/sock.h | 4 ++++ net/core/datagram.c | 36 ++++++++++++++++++------------ net/core/sock.c | 64 +++++++++++++++++++++++++++++++++++++---------------- 3 files changed, 71 insertions(+), 33 deletions(-)
include/net/sock.h +4 -0
@@ -1274,7 +1274,9 @@ static inline struct inode *SOCK_INODE(struct socket *socket)/* * Functions for memory accounting */ +int __sk_mem_raise_allocated (struct sock *sk,int size,int amt,int kind);int __sk_mem_schedule (struct sock *sk,int size,int kind); +void __sk_mem_reduce_allocated (struct sock *sk,int amount);void __sk_mem_reclaim (struct sock *sk,int amount);#define SK_MEM_QUANTUM ((int)PAGE_SIZE)
@@ -1950,6 +1952,8 @@ void sk_reset_timer(struct sock *sk, struct timer_list *timer,void sk_stop_timer (struct sock *sk,struct timer_list *timer); +int __sk_queue_drop_skb (struct sock *sk,struct sk_buff *skb, +unsigned int flags);int __sock_queue_rcv_skb (struct sock *sk,struct sk_buff *skb);int sock_queue_rcv_skb (struct sock *sk,struct sk_buff *skb);
net/core/datagram.c +22 -14
@@ -323,6 +323,27 @@ void __skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb, int len) }EXPORT_SYMBOL (__skb_free_datagram_locked); +int __sk_queue_drop_skb (struct sock *sk,struct sk_buff *skb, +unsigned int flags) +{ +int err =0 ; + +if (flags &MSG_PEEK ) { + err = -ENOENT ; +spin_lock_bh (&sk->sk_receive_queue.lock); +if (skb ==skb_peek (&sk->sk_receive_queue)) { +__skb_unlink (skb, &sk->sk_receive_queue); +atomic_dec (&skb->users); + err =0 ; + } +spin_unlock_bh (&sk->sk_receive_queue.lock); + } + +atomic_inc (&sk->sk_drops); +return err; +} +EXPORT_SYMBOL (__sk_queue_drop_skb); +/** * skb_kill_datagram - Free a datagram skbuff forcibly * @sk: socket
@@ -346,23 +367,10 @@ void __skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb, int len)int skb_kill_datagram (struct sock *sk,struct sk_buff *skb,unsigned int flags) { -int err =0 ; - -if (flags &MSG_PEEK ) { - err = -ENOENT ; -spin_lock_bh (&sk->sk_receive_queue.lock); -if (skb ==skb_peek (&sk->sk_receive_queue)) { -__skb_unlink (skb, &sk->sk_receive_queue); -atomic_dec (&skb->users); - err =0 ; - } -spin_unlock_bh (&sk->sk_receive_queue.lock); - } +int err =__sk_queue_drop_skb (sk, skb, flags);kfree_skb (skb); -atomic_inc (&sk->sk_drops);sk_mem_reclaim_partial (sk); -return err; }EXPORT_SYMBOL (skb_kill_datagram);
net/core/sock.c +45 -19
102 lines not on this page · raw message
Signature · 1 line
1.8.3.1