Re: [PATCHv6 1/3] tun: export underlying socket
On Tuesday 03 November 2009, Arnd Bergmann wrote: index 3f5fd52..404abe0 100644 --- a/include/linux/if_tun.h +++ b/include/linux/if_tun.h @@ -86,4 +86,18 @@ struct tun_filter { __u8 addr[0][ETH_ALEN]; }; +#ifdef __KERNEL__ +#if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE) +struct socket *tun_get_socket(struct file *); +#else +#include linux/err.h +#include linux/errno.h +struct file; +struct socket; +static inline struct socket *tun_get_socket(struct file *f) +{ + return ERR_PTR(-EINVAL); +} +#endif /* CONFIG_TUN */ +#endif /* __KERNEL__ */ #endif /* __IF_TUN_H */ Is this a leftover from testing? Exporting the function for !__KERNEL__ seems pointless. Michael, you didn't reply on this comment and the code is still there in v8. Do you actually need this? What for? Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCHv6 1/3] tun: export underlying socket
On Wed, Nov 04, 2009 at 07:09:05PM +0100, Arnd Bergmann wrote: On Tuesday 03 November 2009, Arnd Bergmann wrote: index 3f5fd52..404abe0 100644 --- a/include/linux/if_tun.h +++ b/include/linux/if_tun.h @@ -86,4 +86,18 @@ struct tun_filter { __u8 addr[0][ETH_ALEN]; }; +#ifdef __KERNEL__ +#if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE) +struct socket *tun_get_socket(struct file *); +#else +#include linux/err.h +#include linux/errno.h +struct file; +struct socket; +static inline struct socket *tun_get_socket(struct file *f) +{ + return ERR_PTR(-EINVAL); +} +#endif /* CONFIG_TUN */ +#endif /* __KERNEL__ */ #endif /* __IF_TUN_H */ Is this a leftover from testing? Exporting the function for !__KERNEL__ seems pointless. Michael, you didn't reply on this comment and the code is still there in v8. Do you actually need this? What for? Arnd Sorry, missed the question. If you look closely it is not exported for !__KERNEL__ at all. The stub is for when CONFIG_TUN is undefined. Maybe I'll add a comment near #else, even though this is a bit strange since the #if is just 2 lines above it. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCHv6 1/3] tun: export underlying socket
On Monday 02 November 2009, Michael S. Tsirkin wrote: Tun device looks similar to a packet socket in that both pass complete frames from/to userspace. This patch fills in enough fields in the socket underlying tun driver to support sendmsg/recvmsg operations, and message flags MSG_TRUNC and MSG_DONTWAIT, and exports access to this socket to modules. Regular read/write behaviour is unchanged. This way, code using raw sockets to inject packets into a physical device, can support injecting packets into host network stack almost without modification. First user of this interface will be vhost virtualization accelerator. You mentioned before that you wanted to export the socket using some ioctl function returning an open file descriptor, which seemed to be a cleaner approach than this one. What was your reason for changing? index 3f5fd52..404abe0 100644 --- a/include/linux/if_tun.h +++ b/include/linux/if_tun.h @@ -86,4 +86,18 @@ struct tun_filter { __u8 addr[0][ETH_ALEN]; }; +#ifdef __KERNEL__ +#if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE) +struct socket *tun_get_socket(struct file *); +#else +#include linux/err.h +#include linux/errno.h +struct file; +struct socket; +static inline struct socket *tun_get_socket(struct file *f) +{ + return ERR_PTR(-EINVAL); +} +#endif /* CONFIG_TUN */ +#endif /* __KERNEL__ */ #endif /* __IF_TUN_H */ Is this a leftover from testing? Exporting the function for !__KERNEL__ seems pointless. Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCHv6 1/3] tun: export underlying socket
On Tue, Nov 03, 2009 at 01:12:33PM +0100, Arnd Bergmann wrote: On Monday 02 November 2009, Michael S. Tsirkin wrote: Tun device looks similar to a packet socket in that both pass complete frames from/to userspace. This patch fills in enough fields in the socket underlying tun driver to support sendmsg/recvmsg operations, and message flags MSG_TRUNC and MSG_DONTWAIT, and exports access to this socket to modules. Regular read/write behaviour is unchanged. This way, code using raw sockets to inject packets into a physical device, can support injecting packets into host network stack almost without modification. First user of this interface will be vhost virtualization accelerator. You mentioned before that you wanted to export the socket using some ioctl function returning an open file descriptor, which seemed to be a cleaner approach than this one. Note that a similar feature can be implemented on top of tun_get_socket, as seen from patch below. What was your reason for changing? It turns out socket structure is really bound to specific a file, so we can not have 2 files referencing the same socket. Instead, as I say above, it's possible to make sendmsg/recvmsg work on tap file directly. For vhost, the advantage of such a feature over using tun_get_socket directly would be that vhost module won't depend on tun module then. I have implemented this (patch below), but decided to go with the simple thing first. Since no userspace-visible changes are involved, let's do this by small steps: it will be easier to figure out when vhost is upstream. --- Note: patch below aplies on top of patch tun: export underlying socket. It is not intended for merge yet. net: convert tun device to socket Add callback to file_ops to retrieve socket from file structure. Use this to make tun character device accept sendmsg/recvmsg calls. Signed-off-by: Michael S. Tsirkin m...@redhat.com diff --git a/drivers/net/tun.c b/drivers/net/tun.c index b58095a..53e1806 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1405,7 +1405,8 @@ static const struct file_operations tun_fops = { .unlocked_ioctl = tun_chr_ioctl, .open = tun_chr_open, .release = tun_chr_close, - .fasync = tun_chr_fasync + .fasync = tun_chr_fasync, + .get_socket = tun_get_socket, }; static struct miscdevice tun_miscdev = { diff --git a/include/linux/fs.h b/include/linux/fs.h index 2620a8c..f2b381f 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1506,6 +1506,9 @@ struct file_operations { ssize_t (*splice_write)(struct pipe_inode_info *, struct file *, loff_t *, size_t, unsigned int); ssize_t (*splice_read)(struct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int); int (*setlease)(struct file *, long, struct file_lock **); +#ifdef CONFIG_NET + struct socket *(*get_socket)(struct file *file); +#endif }; struct inode_operations { diff --git a/net/socket.c b/net/socket.c index 9dff31c..700efcb 100644 --- a/net/socket.c +++ b/net/socket.c @@ -119,6 +119,11 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos, struct pipe_inode_info *pipe, size_t len, unsigned int flags); +static struct socket *sock_get_socket(struct file *file) +{ + return file-private_data; /* set in sock_map_fd */ +} + /* * Socket files have a set of 'special' operations as well as the generic file ones. These don't appear * in the operation structures but are done directly via the socketcall() multiplexor. @@ -141,6 +146,7 @@ static const struct file_operations socket_file_ops = { .sendpage = sock_sendpage, .splice_write = generic_splice_sendpage, .splice_read = sock_splice_read, + .get_socket = sock_get_socket, }; /* @@ -416,8 +422,8 @@ int sock_map_fd(struct socket *sock, int flags) static struct socket *sock_from_file(struct file *file, int *err) { - if (file-f_op == socket_file_ops) - return file-private_data; /* set in sock_map_fd */ + if (file-f_op-get_socket) + return file-f_op-get_socket(file); *err = -ENOTSOCK; return NULL; -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCHv6 1/3] tun: export underlying socket
On Tuesday 03 November 2009, Michael S. Tsirkin wrote: What was your reason for changing? It turns out socket structure is really bound to specific a file, so we can not have 2 files referencing the same socket. Instead, as I say above, it's possible to make sendmsg/recvmsg work on tap file directly. Ah, I see. I have implemented this (patch below), but decided to go with the simple thing first. Since no userspace-visible changes are involved, let's do this by small steps: it will be easier to figure out when vhost is upstream. This may even make it easier for me to do the same with macvtap if I resume work on that. @@ -416,8 +422,8 @@ int sock_map_fd(struct socket *sock, int flags) static struct socket *sock_from_file(struct file *file, int *err) { - if (file-f_op == socket_file_ops) - return file-private_data; /* set in sock_map_fd */ + if (file-f_op-get_socket) + return file-f_op-get_socket(file); *err = -ENOTSOCK; Or maybe do both (socket_file_ops and get_socket), to avoid an indirect function call. Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization