udp: scalability improvements
Subsystems (4)
Paolo Abeni
maintainerv2 cover udp: scalability improvements
This patch series implement an idea suggested by Eric Dumazet to reduce the contention of the udp sk_receive_queue lock when the socket is under flood.
An ancillary queue is added to the udp socket, and the socket always tries first to read packets from such queue. If it's empty, we splice the content from sk_receive_queue into the ancillary queue.
The first patch introduces some helpers to keep the udp code small, and the following two implement the ancillary queue strategy. The code is split to hopefully help the reviewing process.
The measured overall gain under udp flood is up to the 30% depending on the numa layout and the number of ingress queue used by the relevant nic.
The performance numbers have been gathered using pktgen as sender, with 64 bytes packets, random src port on a host b2b connected via a 10Gbs link with the dut.
The receiver used the udp_sink program by Jesper [1] and an h/w l4 rx hash on the ingress nic, so that the number of ingress nic rx queues hit by the udp traffic could be controlled via ethtool -L.
The udp_sink program was bound to the first idle cpu, to get more stable numbers.
On a single numa node receiver:
nic rx queues vanilla patched kernel 1 1820 kpps 1900 kpps 2 1950 kpps 2500 kpps 16 1670 kpps 2120 kpps
When using a single nic rx queue, busy polling was also enabled, elsewhere, in the above scenario, the bh processing becomes the bottle-neck and this produces large artifacts in the measured performances (e.g. improving the udp sink run time, decreases the overall tput, since more action from the scheduler comes into play).
[1] https://github.com/netoptimizer/network-testing/blob/master/src/udp_sink.c
v1 -> v2:
Patches 1/3 and 2/3 are unchanged, in patch 3/3 the rx_queue_lock_held param of udp_rmem_release() is now a bool.
Paolo Abeni (3):
net/sock: factor out dequeue/peek with offset code
udp: use a separate rx queue for packet reception
udp: keep the sk_receive_queue held when splicing
include/linux/skbuff.h | 7 +++ include/linux/udp.h | 3 + include/net/sock.h | 4 +- include/net/udp.h | 9 +-- include/net/udplite.h | 2 +- net/core/datagram.c | 90 +++++++++++++++------------ net/ipv4/udp.c | 162 +++++++++++++++++++++++++++++++++++++++++++------ net/ipv6/udp.c | 3 +- 8 files changed, 211 insertions(+), 69 deletions(-)
Signature · 1 line
2.9.3
Paolo Abeni
maintainerPATCH v2 1/3 net/sock: factor out dequeue/peek with offset code
And update __sk_queue_drop_skb() to work on the specified queue. This will help the udp protocol to use an additional private rx queue in a later patch.
--- include/linux/skbuff.h | 7 ++++ include/net/sock.h | 4 +-- net/core/datagram.c | 90 ++++++++++++++++++++++++++++---------------------- 3 files changed, 60 insertions(+), 41 deletions(-)
include/linux/skbuff.h +7 -0
@@ -3056,6 +3056,13 @@ static inline void skb_frag_list_init(struct sk_buff *skb)int __skb_wait_for_more_packets (struct sock *sk,int *err,long *timeo_p,const struct sk_buff *skb); +struct sk_buff *__skb_try_recv_from_queue (struct sock *sk, +struct sk_buff_head *queue, +unsigned int flags, +void (*destructor)(struct sock *sk, +struct sk_buff *skb), +int *peeked,int *off,int *err, +struct sk_buff **last); struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned flags, void (*destructor)(struct sock *sk, struct sk_buff *skb),
include/net/sock.h +2 -2
@@ -2035,8 +2035,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 __sk_queue_drop_skb (struct sock *sk,struct sk_buff_head *sk_queue, +struct sk_buff *skb,unsigned int flags,void (*destructor)(struct sock *sk,struct sk_buff *skb));int __sock_queue_rcv_skb (struct sock *sk,struct sk_buff *skb);
net/core/datagram.c +51 -39
133 lines not on this page · raw message
Signature · 1 line
2.9.3
Re: [net-next,v2,1/3] net/sock: factor out dequeue/peek with offset code
Quoted 3 lines · Paolo Abeni
And update
__sk_queue_drop_skb()to work on the specified queue. This will help the udp protocol to use an additional private rx queue in a later patch.
CRIU tests fails with this patch:
recvmsg(14, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="packet dgram right\0", iov_len=212960}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, MSG_PEEK|MSG_DONTWAIT) = 19 <0.000048> writev(9, [{iov_base="\4\0\0\0", iov_len=4}, {iov_base="\10\t\20\23", iov_len=4}], 2) = 8 <0.000085>write(9, "packet dgram right\0", 19) = 19 <0.000062>recvmsg(14, {msg_namelen=0}, MSG_PEEK|MSG_DONTWAIT) = -1 EFAULT (Bad address) <0.000046>
without this patch, strace looks like this:
g(14, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="packet dgram right\0", iov_len=212960}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, MSG_PEEK|MSG_DONTWAIT) = 19 <0.000024> writev(9, [{iov_base="\4\0\0\0", iov_len=4}, {iov_base="\10\t\20\23", iov_len=4}], 2) = 8 <0.000037>write(9, "packet dgram right\0", 19) = 19 <0.000030>
recvmsg(14, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="packet dgram left\0", iov_len=212960}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, MSG_PEEK|MSG_DONTWAIT) = 18 <0.000023> writev(9, [{iov_base="\4\0\0\0", iov_len=4}, {iov_base="\10\t\20\22", iov_len=4}], 2) = 8 <0.000030>write(9, "packet dgram left\0", 18) = 18 <0.000030>recvmsg(14, {msg_namelen=0}, MSG_PEEK|MSG_DONTWAIT) = -1 EAGAIN (Resource temporarily unavailable) <0.000023>
https://travis-ci.org/avagin/criu/jobs/232990442
Quoted 184 lines
Signed-off-by Paolo Abeni <[email protected]>Acked-by Eric Dumazet <[email protected]>--- include/linux/skbuff.h | 7 ++++ include/net/sock.h | 4 +-- net/core/datagram.c | 90 ++++++++++++++++++++++++++++---------------------- 3 files changed, 60 insertions(+), 41 deletions(-)include/linux/skbuff.h +7
-0@@ -3056,6 +3056,13 @@ static inline void skb_frag_list_init(struct sk_buff *skb)int __skb_wait_for_more_packets (struct sock *sk,int *err,long *timeo_p,const struct sk_buff *skb); +struct sk_buff *__skb_try_recv_from_queue (struct sock *sk, +struct sk_buff_head *queue, +unsigned int flags, +void (*destructor)(struct sock *sk, +struct sk_buff *skb), +int *peeked,int *off,int *err, +struct sk_buff **last); struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned flags, void (*destructor)(struct sock *sk, struct sk_buff *skb),include/net/sock.h +2
-2@@ -2035,8 +2035,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 __sk_queue_drop_skb (struct sock *sk,struct sk_buff_head *sk_queue, +struct sk_buff *skb,unsigned int flags,void (*destructor)(struct sock *sk,struct sk_buff *skb));int __sock_queue_rcv_skb (struct sock *sk,struct sk_buff *skb);net/core/datagram.c +51
-39@@ -161,6 +161,43 @@ static struct sk_buff *skb_set_peeked(struct sk_buff *skb)return skb; } +struct sk_buff *__skb_try_recv_from_queue (struct sock *sk, +struct sk_buff_head *queue, +unsigned int flags, +void (*destructor)(struct sock *sk, +struct sk_buff *skb), +int *peeked,int *off,int *err, +struct sk_buff **last) +{ +struct sk_buff *skb; + + *last = queue->prev; + skb_queue_walk(queue, skb) { +if (flags &MSG_PEEK ) { +if (*off >= skb->len && (skb->len || *off || + skb->peeked)) { + *off -= skb->len; +continue ; + } +if (!skb->len) { + skb =skb_set_peeked (skb); +if (unlikely (IS_ERR (skb))) { + *err =PTR_ERR (skb); +return skb; + } + } + *peeked =1 ; +atomic_inc (&skb->users); + }else { +__skb_unlink (skb, queue); +if (destructor) +destructor (sk, skb); + } +return skb; + } +return NULL ; +} +/** * __skb_try_recv_datagram - Receive a datagram skbuff * @sk: socket @@ -216,46 +253,20 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags, *peeked =0 ; do { + int _off = *off; + /* Again only user level code calls this function, so nothing * interrupt level will suddenly eat the receive_queue. * * Look at current nfs client by the way... * However, this function was correct in any case. 8) */ - int _off = *off; - - *last = (struct sk_buff *)queue; spin_lock_irqsave(&queue->lock, cpu_flags); - skb_queue_walk(queue, skb) { - *last = skb; - if (flags & MSG_PEEK) { - if (_off >= skb->len && (skb->len || _off || - skb->peeked)) { - _off -= skb->len; - continue; - } - if (!skb->len) { - skb = skb_set_peeked(skb); - if (IS_ERR(skb)) { - error = PTR_ERR(skb); - spin_unlock_irqrestore(&queue->lock, - cpu_flags); - goto no_packet; - } - } - *peeked = 1; - atomic_inc(&skb->users); - } else { - __skb_unlink(skb, queue); - if (destructor) - destructor(sk, skb); - } - spin_unlock_irqrestore(&queue->lock, cpu_flags); - *off = _off; - return skb; - } - + skb = __skb_try_recv_from_queue(sk, queue, flags, destructor, + peeked, &_off, err, last); spin_unlock_irqrestore(&queue->lock, cpu_flags); + if (skb) + return skb; if (!sk_can_busy_loop(sk)) break;@@ -335,8 +346,8 @@ 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 __sk_queue_drop_skb (struct sock *sk,struct sk_buff_head *sk_queue, +struct sk_buff *skb,unsigned int flags,void (*destructor)(struct sock *sk,struct sk_buff *skb)) {@@ -344,15 +355,15 @@ int __sk_queue_drop_skb(struct sock *sk, struct sk_buff *skb,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); +spin_lock_bh (&sk_queue->lock); +if (skb ==skb_peek (sk_queue)) { +__skb_unlink (skb, sk_queue);atomic_dec (&skb->users);if (destructor)destructor (sk, skb); err =0 ; } -spin_unlock_bh (&sk->sk_receive_queue.lock); +spin_unlock_bh (&sk_queue->lock); }atomic_inc (&sk->sk_drops);@@ -383,7 +394,8 @@ EXPORT_SYMBOL(__sk_queue_drop_skb);int skb_kill_datagram (struct sock *sk,struct sk_buff *skb,unsigned int flags) { -int err =__sk_queue_drop_skb (sk, skb, flags,NULL ); +int err =__sk_queue_drop_skb (sk, &sk->sk_receive_queue, skb, flags, +NULL );kfree_skb (skb);sk_mem_reclaim_partial (sk);
Paolo Abeni
maintainerPATCH v2 2/3 udp: use a separate rx queue for packet reception
In reply to Paolo Abeniunder udp flood the sk_receive_queue spinlock is heavily contended. This patch try to reduce the contention on such lock adding a second receive queue to the udp sockets; recvmsg() looks first in such queue and, only if empty, tries to fetch the data from sk_receive_queue. The latter is spliced into the newly added queue every time the receive path has to acquire the sk_receive_queue lock.
The accounting of forward allocated memory is still protected with the sk_receive_queue lock, so udp_rmem_release() needs to acquire both locks when the forward deficit is flushed.
On specific scenarios we can end up acquiring and releasing the sk_receive_queue lock multiple times; that will be covered by the next patch
--- include/linux/udp.h | 3 ++ include/net/udp.h | 9 +--- include/net/udplite.h | 2 +- net/ipv4/udp.c | 138 ++++++++++++++++++++++++++++++++++++++++++++------ net/ipv6/udp.c | 3 +- 5 files changed, 131 insertions(+), 24 deletions(-)
include/linux/udp.h +3 -0
@@ -80,6 +80,9 @@ struct udp_sock {struct sk_buff *skb, int nhoff); +/* udp_recvmsg try to use this before splicing sk_receive_queue */ +struct sk_buff_head reader_queue____cacheline_aligned_in_smp ; +/* This field is dirtied by udp_recvmsg() */ int forward_deficit; };
include/net/udp.h +2 -7
@@ -249,13 +249,8 @@ void udp_destruct_sock(struct sock *sk);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_skb_destructor (struct sock *sk,struct sk_buff *skb); -static inline struct sk_buff * -__skb_recv_udp (struct sock *sk,unsigned int flags,int noblock,int *peeked, -int *off,int *err) -{ -return __skb_recv_datagram (sk, flags | (noblock ?MSG_DONTWAIT :0 ), - udp_skb_destructor, peeked, off, err); -} +struct sk_buff *__skb_recv_udp (struct sock *sk,unsigned int flags, +int noblock,int *peeked,int *off,int *err);static inline struct sk_buff *skb_recv_udp (struct sock *sk,unsigned int flags,int noblock,int *err) {
include/net/udplite.h +1 -1
@@ -26,8 +26,8 @@ static __inline__ int udplite_getfrag(void *from, char *to, int offset,/* Designate sk as UDP-Lite socket */ static inline int udplite_sk_init (struct sock *sk) { +udp_init_sock (sk);udp_sk (sk)->pcflag =UDPLITE_BIT ; - sk->sk_destruct = udp_destruct_sock;return 0 ; }
net/ipv4/udp.c +123 -15
214 lines not on this page · raw message
net/ipv6/udp.c +2 -1
@@ -455,7 +455,8 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,return err; csum_copy_err: -if (!__sk_queue_drop_skb (sk, skb, flags, udp_skb_destructor)) { +if (!__sk_queue_drop_skb (sk, &udp_sk (sk)->reader_queue, skb, flags, + udp_skb_destructor)) {if (is_udp4) {UDP_INC_STATS (sock_net (sk),UDP_MIB_CSUMERRORS , is_udplite);
Signature · 1 line
2.9.3
Paolo Abeni
maintainerPATCH v2 3/3 udp: keep the sk_receive_queue held when splicing
In reply to Paolo AbeniOn packet reception, when we are forced to splice the sk_receive_queue, we can keep the related lock held, so that we can avoid re-acquiring it, if fwd memory scheduling is required.
v1 -> v2:
the rx_queue_lock_held param in udp_rmem_release() is now a bool
--- net/ipv4/udp.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-)
net/ipv4/udp.c +26 -10
@@ -1164,7 +1164,8 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset, }/* fully reclaim rmem/fwd memory allocated for skb */ -static void udp_rmem_release (struct sock *sk,int size,int partial) +static void udp_rmem_release (struct sock *sk,int size,int partial, +bool rx_queue_lock_held) {struct udp_sock *up =udp_sk (sk);struct sk_buff_head *sk_queue;
@@ -1181,9 +1182,13 @@ static void udp_rmem_release(struct sock *sk, int size, int partial) } up->forward_deficit =0 ; -/* acquire the sk_receive_queue for fwd allocated memory scheduling */ +/* acquire the sk_receive_queue for fwd allocated memory scheduling, +* if the called don't held it already +*/ sk_queue = &sk->sk_receive_queue; -spin_lock (&sk_queue->lock); +if (!rx_queue_lock_held) +spin_lock (&sk_queue->lock); + sk->sk_forward_alloc += size; amt = (sk->sk_forward_alloc - partial) & ~(SK_MEM_QUANTUM -1 );
@@ -1197,7 +1202,8 @@ static void udp_rmem_release(struct sock *sk, int size, int partial)/* this can save us from acquiring the rx queue lock on next receive */ skb_queue_splice_tail_init (sk_queue, &up->reader_queue); -spin_unlock (&sk_queue->lock); +if (!rx_queue_lock_held) +spin_unlock (&sk_queue->lock); }/* Note: called with reader_queue.lock held.
@@ -1207,10 +1213,16 @@ static void udp_rmem_release(struct sock *sk, int size, int partial)*/ void udp_skb_destructor (struct sock *sk,struct sk_buff *skb) { -udp_rmem_release (sk, skb->dev_scratch,1 ); +udp_rmem_release (sk, skb->dev_scratch,1 , false); }EXPORT_SYMBOL (udp_skb_destructor); +/* as above, but the caller held the rx queue lock, too */ +void udp_skb_dtor_locked (struct sock *sk,struct sk_buff *skb) +{ +udp_rmem_release (sk, skb->dev_scratch,1 , true); +} +/* Idea of busylocks is to let producers grab an extra spinlock * to relieve pressure on the receive_queue spinlock shared by consumer. * Under flood, this means that only one producer can be in line
@@ -1325,7 +1337,7 @@ void udp_destruct_sock(struct sock *sk) total += skb->truesize;kfree_skb (skb); } -udp_rmem_release (sk, total,0 ); +udp_rmem_release (sk, total,0 , true);inet_sock_destruct (sk); }
@@ -1397,7 +1409,7 @@ static int first_packet_length(struct sock *sk) } res = skb ? skb->len :-1 ;if (total) -udp_rmem_release (sk, total,1 ); +udp_rmem_release (sk, total,1 , false);spin_unlock_bh (&rcvq->lock);return res; }
@@ -1471,16 +1483,20 @@ struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags, goto busy_check; } -/* refill the reader queue and walk it again */ +/* refill the reader queue and walk it again +* keep both queues locked to avoid re-acquiring +* the sk_receive_queue lock if fwd memory scheduling +* is needed. +*/ _off = *off;spin_lock (&sk_queue->lock);skb_queue_splice_tail_init (sk_queue, queue); -spin_unlock (&sk_queue->lock); skb =__skb_try_recv_from_queue (sk, queue, flags, - udp_skb_destructor, + udp_skb_dtor_locked, peeked, &_off, err, &last); +spin_unlock (&sk_queue->lock);spin_unlock_bh (&queue->lock);if (skb) { *off = _off;
Signature · 1 line
2.9.3
Re: [PATCH net-next v2 3/3] udp: keep the sk_receive_queue held when splicing
Quoted 10 lines · Paolo Abeni
On packet reception, when we are forced to splice the
sk_receive_queue, we can keep the related lock held, so that we can avoid re-acquiring it, if fwd memory scheduling is required.v1 -> v2:
therx_queue_lock_heldparam inudp_rmem_release()is now a boolSigned-off-by Paolo Abeni <[email protected]>
David Miller
maintainerFrom: Paolo Abeni <[email protected]>
Date: Tue, 16 May 2017 11:20:12 +0200
Quoted 3 lines
This patch series implement an idea suggested by Eric Dumazet to reduce the contention of the udp
sk_receive_queuelock when the socket is under flood.
Series applied, thanks a lot.