[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <549DB660.8040606@fb.com>
Date: Fri, 26 Dec 2014 14:26:24 -0500
From: Alex Gartrell <agartrell@...com>
To: Jason Wang <jasowang@...hat.com>, <davem@...emloft.net>,
<herbert@...dor.apana.org.au>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<kernel-team@...com>
Subject: Re: [PATCH net-next 1/2] socket: Allow external sockets to use socket
syscalls
Hello Jason,
Thanks again for your comments.
On 12/26/14 4:45 AM, Jason Wang wrote:
>> @@ -388,6 +388,7 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
>> sock->file = file;
>> file->f_flags = O_RDWR | (flags & O_NONBLOCK);
>> file->private_data = sock;
>> + file->private_data_is_socket = true;
>
> This is only safe if all user of sock_alloc_file() have full support for
> each method in proto_ops.
This doesn't change anything in the invocation of the syscalls, as every
file allocated through this path already passes the "is_socket" test due
to the (sadly missing here)
file = alloc_file(..., &socket_file_ops)
above, so this makes things no less safe than they were.
Of course we also need to implement these proto_ops for the tun device
in the subsequent patch to make it work.
>> return file;
>> }
>> EXPORT_SYMBOL(sock_alloc_file);
>> @@ -411,7 +412,7 @@ static int sock_map_fd(struct socket *sock, int flags)
>>
>> struct socket *sock_from_file(struct file *file, int *err)
>> {
>> - if (file->f_op == &socket_file_ops)
>> + if (file->private_data_is_socket)
>> return file->private_data; /* set in sock_map_fd */
>>
>> *err = -ENOTSOCK;
>
> Not sure it's the best method, how about a dedicated f_op to do this?
So like a get_socket operation? That would simplify this a lot,
certainly (we wouldn't need to move private_data around in the tun driver).
Thanks,
--
Alex Gartrell <agartrell@...com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists