Re: [PATCH] S390: Call direct system calls for socket operations.

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

Hi Stefan,

Some comments below.

On 13-10-2015 07:48, Stefan Liebler wrote:
> Hi,
> 
> this patch calls direct system calls for socket operations in the same way as power does. The system calls were introduced in kernel commit https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=977108f89c989b1eeb5c8d938e1e71913391eb5f.
> There are no direct recv, send, accept syscalls available on s390. Thus
> recvfrom, sendto, accept4 are called instead of the socketcall. See recv.c, send.c, accept.c in sysdeps/unix/sysv/linux/s390 folder.
> 
> The socketcalls in syscalls.list for s390-64 are removed. They were never used on s390x.
> 
> Bye Stefan
> 
> ---
> 2015-10-13  Stefan Liebler  <stli@linux.vnet.ibm.com>
> 
>     * sysdeps/unix/sysv/linux/s390/kernel-features.h:
>     (__ASSUME_*_SYSCALL) Define new macros.
>     * sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list:
>     Remove socketcall syscalls.
>     * sysdeps/unix/sysv/linux/s390/accept.c: New File.
>     * sysdeps/unix/sysv/linux/s390/recv.c: Likewise.
>     * sysdeps/unix/sysv/linux/s390/send.c: Likewise.
> 
> 20151013_direct_socketcalls
> 
> 
> diff --git a/sysdeps/unix/sysv/linux/s390/accept.c b/sysdeps/unix/sysv/linux/s390/accept.c
> new file mode 100644
> index 0000000..c491690
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/s390/accept.c
> @@ -0,0 +1,40 @@
> +/* Copyright (C) 2015 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <signal.h>
> +#include <sys/socket.h>
> +
> +#include <sysdep-cancel.h>
> +#include <socketcall.h>
> +#include <sys/syscall.h>
> +#include <kernel-features.h>
> +
> +int
> +__libc_accept (int fd, __SOCKADDR_ARG addr, socklen_t *len)
> +{
> +#ifdef __ASSUME_ACCEPT4_SYSCALL
> +  /* There does not exist a direct accept syscall on s390, thus accept4 is used.
> +     The kernel also wraps this syscall with additional 0 parameters if called
> +     via socketcall.  */
> +  return SYSCALL_CANCEL (accept4, fd, addr.__sockaddr__, len, 0);

I would prefer to not add another arch-specific implementation, but instead
use something like __ASSUME_ACCEPT4_FOR_ACCEPT_SYSCALL and use accept4
directly in sysdeps/unix/sysv/linux/accept.c. Same for recv and send.

> +#else
> +  return SOCKETCALL_CANCEL (accept, fd, addr.__sockaddr__, len);
> +#endif
> +}
> +weak_alias (__libc_accept, accept)
> +libc_hidden_def (accept)
> diff --git a/sysdeps/unix/sysv/linux/s390/kernel-features.h b/sysdeps/unix/sysv/linux/s390/kernel-features.h
> index 96f73ef..899421f 100644
> --- a/sysdeps/unix/sysv/linux/s390/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/s390/kernel-features.h
> @@ -20,4 +20,29 @@
>  /* S/390 uses socketcall.  */
>  #define __ASSUME_SOCKETCALL		1
>  
> +/* Direct socketcalls available with kernel 4.3.  */
> +#if __LINUX_KERNEL_VERSION >= 0x040300
> +# define __ASSUME_RECVMMSG_SYSCALL     1
> +# define __ASSUME_SENDMMSG_SYSCALL     1
> +# define __ASSUME_SOCKET_SYSCALL       1
> +# define __ASSUME_SOCKETPAIR_SYSCALL   1
> +# define __ASSUME_BIND_SYSCALL         1
> +# define __ASSUME_CONNECT_SYSCALL      1
> +# define __ASSUME_LISTEN_SYSCALL       1
> +# define __ASSUME_ACCEPT4_SYSCALL      1
> +# define __ASSUME_GETSOCKOPT_SYSCALL   1
> +# define __ASSUME_SETSOCKOPT_SYSCALL   1
> +# define __ASSUME_GETSOCKNAME_SYSCALL  1
> +# define __ASSUME_GETPEERNAME_SYSCALL  1
> +# define __ASSUME_SENDTO_SYSCALL       1
> +# define __ASSUME_SENDMSG_SYSCALL      1
> +# define __ASSUME_RECVFROM_SYSCALL     1
> +# define __ASSUME_RECVMSG_SYSCALL      1
> +# define __ASSUME_SHUTDOWN_SYSCALL     1
> +
> +/* There are no direct recv, send, accept syscalls available on s390. Thus
> +   recvfrom, sendto, accept4 are called if available instead of the socketcall.
> +   See recv.c, send.c, accept.c in sysdeps/unix/sysv/linux/s390 folder.  */
> +#endif
> +
>  #include_next <kernel-features.h>
> diff --git a/sysdeps/unix/sysv/linux/s390/recv.c b/sysdeps/unix/sysv/linux/s390/recv.c
> new file mode 100644
> index 0000000..9c841fb
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/s390/recv.c
> @@ -0,0 +1,41 @@
> +/* Copyright (C) 2015 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <signal.h>
> +#include <sys/socket.h>
> +
> +#include <sysdep-cancel.h>
> +#include <socketcall.h>
> +#include <kernel-features.h>
> +#include <sys/syscall.h>
> +
> +ssize_t
> +__libc_recv (int fd, void *buf, size_t len, int flags)
> +{
> +#ifdef __ASSUME_RECVFROM_SYSCALL
> +  /* There does not exist a direct recv syscall on s390, thus recvfrom is used.
> +     The kernel also wraps this syscall with additional 0 parameters if called
> +     via socketcall.  */
> +  return SYSCALL_CANCEL (recvfrom, fd, buf, len, flags, NULL, NULL);
> +#else
> +  return SOCKETCALL_CANCEL (recv, fd, buf, len, flags);
> +#endif
> +}
> +weak_alias (__libc_recv, recv)
> +weak_alias (__libc_recv, __recv)
> +libc_hidden_weak (__recv)
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list b/sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list
> index 5b8c102..9f03d26 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list
> @@ -12,22 +12,3 @@ shmget		-	shmget		i:iii	__shmget	shmget
>  semop		-	semop		i:ipi	__semop		semop
>  semget		-	semget		i:iii	__semget	semget
>  semctl		-	semctl		i:iiii	__semctl	semctl
> -
> -# proper socket implementations:
> -accept		-	accept		Ci:iBN	__libc_accept	__accept accept
> -bind		-	bind		i:ipi	__bind		bind
> -connect		-	connect		Ci:ipi	__libc_connect	__connect connect
> -getpeername	-	getpeername	i:ipp	__getpeername	getpeername
> -getsockname	-	getsockname	i:ipp	__getsockname	getsockname
> -getsockopt	-	getsockopt	i:iiiBN	__getsockopt	getsockopt
> -listen		-	listen		i:ii	__listen	listen
> -recv		-	recv		Ci:ibni	__libc_recv	__recv recv
> -recvfrom	-	recvfrom	Ci:ibniBN	__libc_recvfrom	__recvfrom recvfrom
> -recvmsg		-	recvmsg		Ci:ipi	__libc_recvmsg	__recvmsg recvmsg
> -send		-	send		Ci:ibni	__libc_send	__send send
> -sendmsg		-	sendmsg		Ci:ipi	__libc_sendmsg	__sendmsg sendmsg
> -sendto		-	sendto		Ci:ibnibn	__libc_sendto	__sendto sendto
> -setsockopt	-	setsockopt	i:iiibn	__setsockopt	setsockopt
> -shutdown	-	shutdown	i:ii	__shutdown	shutdown
> -socket		-	socket		i:iii	__socket	socket
> -socketpair	-	socketpair	i:iiif	__socketpair	socketpair
> diff --git a/sysdeps/unix/sysv/linux/s390/send.c b/sysdeps/unix/sysv/linux/s390/send.c
> new file mode 100644
> index 0000000..e4a695b
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/s390/send.c
> @@ -0,0 +1,41 @@
> +/* Copyright (C) 2015 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <signal.h>
> +#include <sys/socket.h>
> +
> +#include <sysdep-cancel.h>
> +#include <socketcall.h>
> +#include <kernel-features.h>
> +#include <sys/syscall.h>
> +
> +ssize_t
> +__libc_send (int fd, const void *buf, size_t len, int flags)
> +{
> +#ifdef __ASSUME_SENDTO_SYSCALL
> +  /* There does not exist a direct send syscall on s390, thus sendto is used.
> +     The kernel also wraps this syscall with additional 0 parameters if called
> +     via socketcall.  */
> +  return SYSCALL_CANCEL (sendto, fd, buf, len, flags, NULL, 0);
> +#else
> +  return SOCKETCALL_CANCEL (send, fd, buf, len, flags);
> +#endif
> +}
> +weak_alias (__libc_send, send)
> +weak_alias (__libc_send, __send)
> +libc_hidden_def (__send)