Re: [RFC v2] net: Introduce recvmmsg socket syscall

[Paul Moore <paul.moore@hp.com>]

On Wednesday 10 June 2009 11:40:22 pm Arnaldo Carvalho de Melo wrote:
> diff --git a/net/socket.c b/net/socket.c
> index 791d71a..f9f1e20 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -702,6 +702,28 @@ int sock_recvmsg(struct socket *sock, struct msghdr
> *msg, return ret;
>  }
>
> +static int sock_recvmsg_nosec(struct socket *sock, struct msghdr *msg,
> +			      size_t size, int flags)
> +{
> +	struct kiocb iocb;
> +	struct sock_iocb siocb;
> +	int ret;
> +
> +	init_sync_kiocb(&iocb, NULL);
> +	iocb.private = &siocb;
> +
> +	siocb.sock = sock;
> +	siocb.scm = NULL;
> +	siocb.msg = msg;
> +	siocb.size = size;
> +	siocb.flags = flags;
> +
> +	ret = sock->ops->recvmsg(&iocb, sock, msg, size, flags);
> +	if (-EIOCBQUEUED == ret)
> +		ret = wait_on_sync_kiocb(&iocb);
> +	return ret;
> +}

Hmmm, in an effort to reduce duplicated code how about updating 
__sock_recvmsg() to something like the following:

static inline int __sock_recvmsg(struct kiocb *iocb, struct socket *sock,
                                 struct msghdr *msg, size_t size, int flags)
{
        int err;

        err = security_socket_recvmsg(...);
        if (err)
                return err;

        return sock_recvmsg_nosec(...);
}

The only real difference is that now the *_kiocb() functions get called and I 
have no clue if that is good or bad but it is different :)

>  	/*
> @@ -2018,46 +2029,47 @@ SYSCALL_DEFINE3(recvmsg, int, fd, struct msghdr
> __user *, msg, *      kernel msghdr to use the kernel address space)
>  	 */
>
> -	uaddr = (__force void __user *)msg_sys.msg_name;
> +	uaddr = (__force void __user *)msg_sys->msg_name;
>  	uaddr_len = COMPAT_NAMELEN(msg);
>  	if (MSG_CMSG_COMPAT & flags) {
> -		err = verify_compat_iovec(&msg_sys, iov,
> +		err = verify_compat_iovec(msg_sys, iov,
>  					  (struct sockaddr *)&addr,
>  					  VERIFY_WRITE);
>  	} else
> -		err = verify_iovec(&msg_sys, iov,
> +		err = verify_iovec(msg_sys, iov,
>  				   (struct sockaddr *)&addr,
>  				   VERIFY_WRITE);
>  	if (err < 0)
>  		goto out_freeiov;
>  	total_len = err;
>
> -	cmsg_ptr = (unsigned long)msg_sys.msg_control;
> -	msg_sys.msg_flags = flags & (MSG_CMSG_CLOEXEC|MSG_CMSG_COMPAT);
> +	cmsg_ptr = (unsigned long)msg_sys->msg_control;
> +	msg_sys->msg_flags = flags & (MSG_CMSG_CLOEXEC|MSG_CMSG_COMPAT);
>
>  	if (sock->file->f_flags & O_NONBLOCK)
>  		flags |= MSG_DONTWAIT;
> -	err = sock_recvmsg(sock, &msg_sys, total_len, flags);
> +	err = (nosec ? sock_recvmsg_nosec : sock_recvmsg)(sock, msg_sys,
> +							  total_len, flags);

Perhaps I'm just being nit-picky here but why not this (it is much easier on 
my eyes at least <g>):

	if (nosec)
		err = sock_recvmsg_nosec(...);
	else
		err = sock_recvmsg(...);

-- 
paul moore
linux @ hp