network splice receive v3
Subsystems (5)
RFC PATCH 1/1 network splice receive v3
Hi,
Here's an updated implementation of tcp network splice receive support. It actually works for me now, no data corruption seen.
For the original announcement and how to test it, see:
http://marc.info/?l=linux-netdev&m=118103093400770&w=2
The splice core changes needed to support this are now merged in 2.6.22-git, so the patchset shrinks to just two patches - one for adding a release hook, and one for the networking changes.
The code is also available in the splice-net branch here:
git://git.kernel.dk/data/git/linux-2.6-block.git splice-net
There's a third experimental patch in there that allows vmsplice directly to user memory, that still needs some work though.
Comments, testing welcome!
Signature · 1 line
Jens Axboe
Quoted 1 line
From
e59a68f2d7d261b301960b97659910aab8e3d776Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Mon, 11 Jun 2007 13:00:32 +0200
Subject: [PATCH] splice: don't assume regular pages in splice_to_pipe()
Allow caller to pass in a release function, there might be other resources that need releasing as well. Needed for network receive.
--- fs/splice.c | 9 ++++++++- include/linux/splice.h | 1 + 2 files changed, 9 insertions(+), 1 deletions(-)
fs/splice.c +8 -1
@@ -254,11 +254,16 @@ ssize_t splice_to_pipe(struct pipe_inode_info *pipe, }while (page_nr < spd_pages) -page_cache_release (spd->pages[page_nr++]); + spd->spd_release (spd, page_nr++);return ret; } +static void spd_release_page (struct splice_pipe_desc *spd,unsigned int i) +{ +page_cache_release (spd->pages[i]); +} + static int __generic_file_splice_read(struct file *in, loff_t *ppos, struct pipe_inode_info *pipe, size_t len,
@@ -277,6 +282,7 @@ __generic_file_splice_read(struct file *in, loff_t *ppos, .partial = partial, .flags = flags, .ops = &page_cache_pipe_buf_ops, + .spd_release = spd_release_page, }; index = *ppos >>PAGE_CACHE_SHIFT ;
@@ -1674,6 +1680,7 @@ static long vmsplice_to_pipe(struct file *file, const struct iovec __user *iov, .partial = partial, .flags = flags, .ops = &user_page_pipe_buf_ops, + .spd_release = spd_release_page, }; pipe =pipe_info (file->f_path.dentry->d_inode);
include/linux/splice.h +1 -0
@@ -54,6 +54,7 @@ struct splice_pipe_desc {int nr_pages;/* number of pages in map */ unsigned int flags;/* splice flags */ const struct pipe_buf_operations *ops;/* ops associated with output pipe */ +void (*spd_release)(struct splice_pipe_desc *,unsigned int ); }; typedef int (splice_actor)(struct pipe_inode_info *, struct pipe_buffer *,
Signature · 1 line
1.5.3.rc0.90.gbaa79
Quoted 1 line
From
b62e4a5a3e3220702e837e556427972dc591ff59Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Wed, 20 Jun 2007 09:54:14 +0200
Subject: [PATCH] TCP splice receive support
Support for network splice receive.
--- include/linux/net.h | 3 + include/linux/skbuff.h | 5 + include/net/tcp.h | 3 + net/core/skbuff.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++ net/ipv4/af_inet.c | 1 + net/ipv4/tcp.c | 129 +++++++++++++++++++++++++ net/socket.c | 13 +++ 7 files changed, 400 insertions(+), 0 deletions(-)
include/linux/net.h +3 -0
@@ -19,6 +19,7 @@#define _LINUX_NET_H#include <linux/wait.h> +#include <linux/splice.h> #include <asm/socket.h> struct poll_table_struct ;
@@ -165,6 +166,8 @@ struct proto_ops { struct vm_area_struct * vma);ssize_t (*sendpage) (struct socket *sock,struct page *page,int offset,size_t size,int flags); +ssize_t (*splice_read)(struct socket *sock,loff_t *ppos, +struct pipe_inode_info *pipe,size_t len,unsigned int flags); };struct net_proto_family {
include/linux/skbuff.h +5 -0
@@ -1504,6 +1504,11 @@ extern int skb_store_bits(struct sk_buff *skb, int offset,extern __wsum skb_copy_and_csum_bits (const struct sk_buff *skb,int offset,u8 *to,int len,__wsum csum); +extern int skb_splice_bits (struct sk_buff *skb, +unsigned int offset, +struct pipe_inode_info *pipe, +unsigned int len, +unsigned int flags);extern void skb_copy_and_csum_dev (const struct sk_buff *skb,u8 *to);extern void skb_split (struct sk_buff *skb,struct sk_buff *skb1,const u32 len);
include/net/tcp.h +3 -0
@@ -308,6 +308,9 @@ extern int tcp_twsk_unique(struct sock *sk,extern void tcp_twsk_destructor (struct sock *sk); +extern ssize_t tcp_splice_read (struct socket *sk,loff_t *ppos, +struct pipe_inode_info *pipe,size_t len,unsigned int flags); +static inline void tcp_dec_quickack_mode (struct sock *sk,const unsigned int pkts) {
net/core/skbuff.c +246 -0
264 lines not on this page · raw message
net/ipv4/af_inet.c +1 -0
@@ -835,6 +835,7 @@ const struct proto_ops inet_stream_ops = { .recvmsg = sock_common_recvmsg, .mmap = sock_no_mmap, .sendpage = tcp_sendpage, + .splice_read = tcp_splice_read, #ifdef CONFIG_COMPAT .compat_setsockopt = compat_sock_common_setsockopt, .compat_getsockopt = compat_sock_common_getsockopt,
net/ipv4/tcp.c +129 -0
159 lines not on this page · raw message
net/socket.c +13 -0
@@ -111,6 +111,9 @@ static long compat_sock_ioctl(struct file *file,static int sock_fasync (int fd,struct file *filp,int on);static ssize_t sock_sendpage (struct file *file,struct page *page,int offset,size_t size,loff_t *ppos,int more); +static ssize_t sock_splice_read (struct file *file,loff_t *ppos, +struct pipe_inode_info *pipe,size_t len, +unsigned int flags);/* * Socket files have a set of 'special' operations as well as the generic file ones. These don't appear
@@ -133,6 +136,7 @@ static const struct file_operations socket_file_ops = { .fasync = sock_fasync, .sendpage = sock_sendpage, .splice_write = generic_splice_sendpage, + .splice_read = sock_splice_read, };/*
@@ -691,6 +695,15 @@ static ssize_t sock_sendpage(struct file *file, struct page *page,return sock->ops->sendpage (sock, page, offset, size, flags); } +static ssize_t sock_splice_read (struct file *file,loff_t *ppos, +struct pipe_inode_info *pipe,size_t len, +unsigned int flags) +{ +struct socket *sock = file->private_data; + +return sock->ops->splice_read (sock, ppos, pipe, len, flags); +} +static struct sock_iocb *alloc_sock_iocb (struct kiocb *iocb,struct sock_iocb *siocb) {
Signature · 1 line
1.5.3.rc0.90.gbaa79
Quoted 11 lines · Jens Axboe
Subject: [PATCH] splice: don't assume regular pages in
splice_to_pipe()Allow caller to pass in a release function, there might be other resources that need releasing as well. Needed for network receive.
fs/splice.c +0
-0
fs/splice.c ssize_t splice_to_pipe(struct pipe_inode_info *pipe, · line 257
Quoted 5 lines
fs/splice.c +1
-1@@ -254,11 +254,16 @@ ssize_t splice_to_pipe(struct pipe_inode_info *pipe, }while (page_nr < spd_pages) -page_cache_release (spd->pages[page_nr++]); + spd->spd_release (spd, page_nr++);
Rather than requiring the caller set this, shouldn't we just
allow it? Especially given there is only one non-page user?
while (page_nr < spd_pages)
- page_cache_release(spd->pages[page_nr++]);
+ if (spd->spd_release)
+ spd->spd_release(spd, page_nr++);
+ else
+ page_cache_release(spd->pages[page_nr++]);
Joel
Signature · 9 lines
"Any man who is under 30, and is not a liberal, has not heart;
and any man who is over 30, and is not a conservative, has no brains."
- Sir Winston Churchill
Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127Quoted 21 lines · Joel Becker
Quoted 11 lines · Jens Axboe
Subject: [PATCH] splice: don't assume regular pages in
splice_to_pipe()Allow caller to pass in a release function, there might be other resources that need releasing as well. Needed for network receive.
fs/splice.c +0
-0fs/splice.c ssize_t splice_to_pipe(struct pipe_inode_info *pipe, · line 257
Quoted 5 lines
fs/splice.c +1
-1@@ -254,11 +254,16 @@ ssize_t splice_to_pipe(struct pipe_inode_info *pipe, }while (page_nr < spd_pages) -page_cache_release (spd->pages[page_nr++]); + spd->spd_release (spd, page_nr++);Rather than requiring the caller set this, shouldn't we just
allow it? Especially given there is only one non-page user?
Quoted 6 lines
while (
page_nr<spd_pages)
-page_cache_release(spd->pages[page_nr++]);
+ if (spd->spd_release)
+spd->spd_release(spd, page_nr++);
+ else
+page_cache_release(spd->pages[page_nr++]);
Certainly possible, I think it's cleaner with it always being set though. If it grows other out-of-splice.c users, then your change may be a good idea though.
Signature · 1 line
Jens Axboe
Quoted 1 line · Jens Axboe
Hi,
Hi Jens.
Quoted 11 lines
Here's an updated implementation of tcp network splice receive support. It actually works for me now, no data corruption seen.
For the original announcement and how to test it, see:
http://marc.info/?l=linux-netdev&m=118103093400770&w=2
The splice core changes needed to support this are now merged in 2.6.22-git, so the patchset shrinks to just two patches - one for adding a release hook, and one for the networking changes.
Quoted 8 lines
The code is also available in the splice-net branch here:
git://git.kernel.dk/data/git/linux-2.6-block.gitsplice-netThere's a third experimental patch in there that allows vmsplice directly to user memory, that still needs some work though.
Comments, testing welcome!
It looks like you included all bits we found in the previous runs, so likely it will work good, but so far I have conflicts merging todays git and your tree in include/linux/splice.h, fs/ext2/file.c, fs/splice.c and mm/filemap_xip.c. This can be a problem with my tree though. It really looks like the last tree we tested, so if you think additional one will not hurt, feel free to ping, so I will completely rebase testing tree.
Quoted 2 lines
Signature · 1 line
Jens Axboe
Signature · 1 line
Evgeniy Polyakov
Quoted 23 lines · Evgeniy Polyakov
Quoted 1 line · Jens Axboe
Hi,
Hi Jens.
Quoted 18 lines
Here's an updated implementation of tcp network splice receive support. It actually works for me now, no data corruption seen.
For the original announcement and how to test it, see:
http://marc.info/?l=linux-netdev&m=118103093400770&w=2
The splice core changes needed to support this are now merged in 2.6.22-git, so the patchset shrinks to just two patches - one for adding a release hook, and one for the networking changes.
The code is also available in the splice-net branch here:
git://git.kernel.dk/data/git/linux-2.6-block.gitsplice-netThere's a third experimental patch in there that allows vmsplice directly to user memory, that still needs some work though.
Quoted 6 lines
Quoted 1 line
Comments, testing welcome!
It looks like you included all bits we found in the previous runs, so likely it will work good, but so far I have conflicts merging todays git and your tree in
include/linux/splice.h,fs/ext2/file.c,fs/splice.candmm/filemap_xip.c. This can be a problem with my tree though.
Hmm, the patch should apply directly to the tree as of when I posted this original mail, or any later one. I just tried a rebase, and it rebased fine on top of the current -git as well. So I think the issue is with your tree, sorry!
Quoted 3 lines
It really looks like the last tree we tested, so if you think additional one will not hurt, feel free to ping, so I will completely rebase testing tree.
It would be great if you could retest! There are some minor changes in there, and some extra testing definitely will not hurt.
Signature · 1 line
Jens Axboe
Hi.
Quoted 6 lines · Jens Axboe
Quoted 3 lines
It really looks like the last tree we tested, so if you think additional one will not hurt, feel free to ping, so I will completely rebase testing tree.
It would be great if you could retest! There are some minor changes in there, and some extra testing definitely will not hurt.
I've just tested it with 2.6.22 (e1c1e98d2a3f57b22a0d4136c8160e54404aa437 commit) and did not found any problems - after qute big files were transferred there is no observed previously skb leak, no crashes (quite a few debug options are turned on in config) and files are correct on both peers, so it works good.
Quoted 2 lines
Signature · 1 line
Jens Axboe
Signature · 1 line
Evgeniy Polyakov
Hello.
In article <[email protected]> (at Wed, 11 Jul 2007 11:19:27 +0200), Jens Axboe <[email protected]> says:
net/ipv4/af_inet.c const struct proto_ops inet_stream_ops = { · line 841
Quoted 8 lines
net/ipv4/af_inet.c +1
-0@@ -835,6 +835,7 @@ const struct proto_ops inet_stream_ops = { .recvmsg = sock_common_recvmsg, .mmap = sock_no_mmap, .sendpage = tcp_sendpage, + .splice_read = tcp_splice_read, #ifdef CONFIG_COMPAT .compat_setsockopt = compat_sock_common_setsockopt, .compat_getsockopt = compat_sock_common_getsockopt,
Please add similar bits in net/ipv6/af_inet6.c
unless there are any dependency on IPv4.
(And if there are, it is not good.)
--yoshfuji
Quoted 4 lines · On Thu, Jul 19 2007, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
Hello.
In article <[email protected]> (at Wed, 11 Jul 2007 11:19:27 +0200), Jens Axboe <[email protected]> says:
Quoted 12 lines
net/ipv4/af_inet.c const struct proto_ops inet_stream_ops = { · line 841
Quoted 8 lines
net/ipv4/af_inet.c +1
-0@@ -835,6 +835,7 @@ const struct proto_ops inet_stream_ops = { .recvmsg = sock_common_recvmsg, .mmap = sock_no_mmap, .sendpage = tcp_sendpage, + .splice_read = tcp_splice_read, #ifdef CONFIG_COMPAT .compat_setsockopt = compat_sock_common_setsockopt, .compat_getsockopt = compat_sock_common_getsockopt,Please add similar bits in
net/ipv6/af_inet6.c
unless there are any dependency on IPv4.
(And if there are, it is not good.)
There are no specific ipv4 depedencies, it's just an oversight. So thanks for the clue, I'll add it!
Signature · 1 line
Jens Axboe