* [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
@ 2013-04-01 15:49 Petar Jovanovic
2013-04-10 18:17 ` Petar Jovanovic
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Petar Jovanovic @ 2013-04-01 15:49 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, riku.voipio, petar.jovanovic, aurelien, rth
From: Petar Jovanovic <petar.jovanovic@imgtec.com>
Previous implementation has failed to take into account different value of
SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
The same conversion has to be applied both for do_socket and do_socketpair,
so the code has been isolated in a static inline function.
The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
these are MIPS, SPARC and ALPHA now.
enum sock_type in linux-user/socket.h has been extended to include
TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
The patch also includes necessary code style changes (tab to spaces) in the
header file in the MIPS #ifdef block touched by the change.
Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
---
v2:
- the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
- values for sock_type are defined for SPARC and ALPHA in socket.h
linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
linux-user/syscall.c | 45 +++++----
2 files changed, 195 insertions(+), 98 deletions(-)
diff --git a/linux-user/socket.h b/linux-user/socket.h
index 339cae5..d2b05dc 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -1,91 +1,104 @@
#if defined(TARGET_MIPS)
- // MIPS special values for constants
-
- /*
- * For setsockopt(2)
- *
- * This defines are ABI conformant as far as Linux supports these ...
- */
- #define TARGET_SOL_SOCKET 0xffff
-
- #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
- #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
- #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
- SIGPIPE when they die. */
- #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
- #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
- broadcast messages. */
- #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
- socket to transmit pending data. */
- #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
- #if 0
- To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
- #endif
-
- #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
- #define TARGET_SO_STYLE SO_TYPE /* Synonym */
- #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
- #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
- #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
- #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
- #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
- #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
- #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
- #define TARGET_SO_ACCEPTCONN 0x1009
-
- /* linux-specific, might as well be the same as on i386 */
- #define TARGET_SO_NO_CHECK 11
- #define TARGET_SO_PRIORITY 12
- #define TARGET_SO_BSDCOMPAT 14
+ /* MIPS special values for constants */
+
+ /*
+ * For setsockopt(2)
+ *
+ * This defines are ABI conformant as far as Linux supports these ...
+ */
+ #define TARGET_SOL_SOCKET 0xffff
+
+ #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
+ #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
+ #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
+ SIGPIPE when they die. */
+ #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
+ #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
+ broadcast messages. */
+ #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
+ * socket to transmit pending data.
+ */
+ #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
+ */
+ #if 0
+ /* To add: Allow local address and port reuse. */
+ #define TARGET_SO_REUSEPORT 0x0200
+ #endif
+
+ #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
+ #define TARGET_SO_STYLE SO_TYPE /* Synonym */
+ #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
+ #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
+ #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
+ #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
+ #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
+ #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
+ #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
+ #define TARGET_SO_ACCEPTCONN 0x1009
- #define TARGET_SO_PASSCRED 17
- #define TARGET_SO_PEERCRED 18
+ /* linux-specific, might as well be the same as on i386 */
+ #define TARGET_SO_NO_CHECK 11
+ #define TARGET_SO_PRIORITY 12
+ #define TARGET_SO_BSDCOMPAT 14
- /* Security levels - as per NRL IPv6 - don't actually do anything */
- #define TARGET_SO_SECURITY_AUTHENTICATION 22
- #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
- #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
+ #define TARGET_SO_PASSCRED 17
+ #define TARGET_SO_PEERCRED 18
- #define TARGET_SO_BINDTODEVICE 25
+ /* Security levels - as per NRL IPv6 - don't actually do anything */
+ #define TARGET_SO_SECURITY_AUTHENTICATION 22
+ #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
+ #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
- /* Socket filtering */
- #define TARGET_SO_ATTACH_FILTER 26
- #define TARGET_SO_DETACH_FILTER 27
+ #define TARGET_SO_BINDTODEVICE 25
- #define TARGET_SO_PEERNAME 28
- #define TARGET_SO_TIMESTAMP 29
- #define SCM_TIMESTAMP SO_TIMESTAMP
-
- #define TARGET_SO_PEERSEC 30
- #define TARGET_SO_SNDBUFFORCE 31
- #define TARGET_SO_RCVBUFFORCE 33
-
- /** sock_type - Socket types
- *
- * Please notice that for binary compat reasons MIPS has to
- * override the enum sock_type in include/linux/net.h, so
- * we define ARCH_HAS_SOCKET_TYPES here.
- *
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- * For writing rarp and other similar things on the user level.
- */
- enum sock_type {
- TARGET_SOCK_DGRAM = 1,
- TARGET_SOCK_STREAM = 2,
- TARGET_SOCK_RAW = 3,
- TARGET_SOCK_RDM = 4,
- TARGET_SOCK_SEQPACKET = 5,
- TARGET_SOCK_DCCP = 6,
- TARGET_SOCK_PACKET = 10,
- };
-
- #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
+ /* Socket filtering */
+ #define TARGET_SO_ATTACH_FILTER 26
+ #define TARGET_SO_DETACH_FILTER 27
+
+ #define TARGET_SO_PEERNAME 28
+ #define TARGET_SO_TIMESTAMP 29
+ #define SCM_TIMESTAMP SO_TIMESTAMP
+
+ #define TARGET_SO_PEERSEC 30
+ #define TARGET_SO_SNDBUFFORCE 31
+ #define TARGET_SO_RCVBUFFORCE 33
+
+ /** sock_type - Socket types
+ *
+ * Please notice that for binary compat reasons MIPS has to
+ * override the enum sock_type in include/linux/net.h, so
+ * we define ARCH_HAS_SOCKET_TYPES here.
+ *
+ * @SOCK_DGRAM - datagram (conn.less) socket
+ * @SOCK_STREAM - stream (connection) socket
+ * @SOCK_RAW - raw socket
+ * @SOCK_RDM - reliably-delivered message
+ * @SOCK_SEQPACKET - sequential packet socket
+ * @SOCK_DCCP - Datagram Congestion Control Protocol socket
+ * @SOCK_PACKET - linux specific way of getting packets at the dev level.
+ * For writing rarp and other similar things on the user
+ * level.
+ * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
+ * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
+ */
+
+ #define ARCH_HAS_SOCKET_TYPES 1
+
+ enum sock_type {
+ TARGET_SOCK_DGRAM = 1,
+ TARGET_SOCK_STREAM = 2,
+ TARGET_SOCK_RAW = 3,
+ TARGET_SOCK_RDM = 4,
+ TARGET_SOCK_SEQPACKET = 5,
+ TARGET_SOCK_DCCP = 6,
+ TARGET_SOCK_PACKET = 10,
+ TARGET_SOCK_CLOEXEC = 02000000,
+ TARGET_SOCK_NONBLOCK = 0200,
+ };
+
+ #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
+ #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
#elif defined(TARGET_ALPHA)
@@ -156,8 +169,81 @@
/* Instruct lower device to use last 4-bytes of skb data as FCS */
#define TARGET_SO_NOFCS 43
+ /** sock_type - Socket types
+ *
+ * Please notice that for binary compat reasons ALPHA has to
+ * override the enum sock_type in include/linux/net.h, so
+ * we define ARCH_HAS_SOCKET_TYPES here.
+ *
+ * @SOCK_DGRAM - datagram (conn.less) socket
+ * @SOCK_STREAM - stream (connection) socket
+ * @SOCK_RAW - raw socket
+ * @SOCK_RDM - reliably-delivered message
+ * @SOCK_SEQPACKET - sequential packet socket
+ * @SOCK_DCCP - Datagram Congestion Control Protocol socket
+ * @SOCK_PACKET - linux specific way of getting packets at the dev level.
+ * For writing rarp and other similar things on the user
+ * level.
+ * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
+ * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
+ */
+
+ #define ARCH_HAS_SOCKET_TYPES 1
+
+ enum sock_type {
+ TARGET_SOCK_STREAM = 1,
+ TARGET_SOCK_DGRAM = 2,
+ TARGET_SOCK_RAW = 3,
+ TARGET_SOCK_RDM = 4,
+ TARGET_SOCK_SEQPACKET = 5,
+ TARGET_SOCK_DCCP = 6,
+ TARGET_SOCK_PACKET = 10,
+ TARGET_SOCK_CLOEXEC = 010000000,
+ TARGET_SOCK_NONBLOCK = 010000000000,
+ };
+
+ #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
+ #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
#else
+#if defined(TARGET_SPARC)
+ /** sock_type - Socket types
+ *
+ * Please notice that for binary compat reasons SPARC has to
+ * override the enum sock_type in include/linux/net.h, so
+ * we define ARCH_HAS_SOCKET_TYPES here.
+ *
+ * @SOCK_DGRAM - datagram (conn.less) socket
+ * @SOCK_STREAM - stream (connection) socket
+ * @SOCK_RAW - raw socket
+ * @SOCK_RDM - reliably-delivered message
+ * @SOCK_SEQPACKET - sequential packet socket
+ * @SOCK_DCCP - Datagram Congestion Control Protocol socket
+ * @SOCK_PACKET - linux specific way of getting packets at the dev level.
+ * For writing rarp and other similar things on the user
+ * level.
+ * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
+ * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
+ */
+
+ #define ARCH_HAS_SOCKET_TYPES 1
+
+ enum sock_type {
+ TARGET_SOCK_STREAM = 1,
+ TARGET_SOCK_DGRAM = 2,
+ TARGET_SOCK_RAW = 3,
+ TARGET_SOCK_RDM = 4,
+ TARGET_SOCK_SEQPACKET = 5,
+ TARGET_SOCK_DCCP = 6,
+ TARGET_SOCK_PACKET = 10,
+ TARGET_SOCK_CLOEXEC = 020000000,
+ TARGET_SOCK_NONBLOCK = 040000,
+ };
+
+ #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
+ #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
+#endif
+
/* For setsockopt(2) */
#define TARGET_SOL_SOCKET 1
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ee82a2d..5b87c9d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
free(vec);
}
-/* do_socket() Must return target values and target errnos. */
-static abi_long do_socket(int domain, int type, int protocol)
+#if defined(ARCH_HAS_SOCKET_TYPES)
+static inline void target_to_host_sock_type(int *type)
{
-#if defined(TARGET_MIPS)
- switch(type) {
+ int host_type = 0;
+ int target_type = *type;
+
+ switch (target_type & TARGET_SOCK_TYPE_MASK) {
case TARGET_SOCK_DGRAM:
- type = SOCK_DGRAM;
+ host_type = SOCK_DGRAM;
break;
case TARGET_SOCK_STREAM:
- type = SOCK_STREAM;
- break;
- case TARGET_SOCK_RAW:
- type = SOCK_RAW;
+ host_type = SOCK_STREAM;
break;
- case TARGET_SOCK_RDM:
- type = SOCK_RDM;
- break;
- case TARGET_SOCK_SEQPACKET:
- type = SOCK_SEQPACKET;
- break;
- case TARGET_SOCK_PACKET:
- type = SOCK_PACKET;
+ default:
+ host_type = target_type & TARGET_SOCK_TYPE_MASK;
break;
}
+ if (target_type & TARGET_SOCK_CLOEXEC) {
+ host_type |= SOCK_CLOEXEC;
+ }
+ if (target_type & TARGET_SOCK_NONBLOCK) {
+ host_type |= SOCK_NONBLOCK;
+ }
+ *type = host_type;
+}
+#endif
+
+/* do_socket() Must return target values and target errnos. */
+static abi_long do_socket(int domain, int type, int protocol)
+{
+#if defined(ARCH_HAS_SOCKET_TYPES)
+ target_to_host_sock_type(&type);
#endif
if (domain == PF_NETLINK)
return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
@@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
int tab[2];
abi_long ret;
+#if defined(ARCH_HAS_SOCKET_TYPES)
+ target_to_host_sock_type(&type);
+#endif
ret = get_errno(socketpair(domain, type, protocol, tab));
if (!is_error(ret)) {
if (put_user_s32(tab[0], target_tab_addr)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-04-01 15:49 [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion Petar Jovanovic
@ 2013-04-10 18:17 ` Petar Jovanovic
2013-04-15 13:47 ` Aurelien Jarno
2013-06-27 17:21 ` Peter Maydell
2 siblings, 0 replies; 14+ messages in thread
From: Petar Jovanovic @ 2013-04-10 18:17 UTC (permalink / raw)
To: Petar Jovanovic, qemu-devel@nongnu.org
Cc: blauwirbel@gmail.com, riku.voipio@linaro.org,
aurelien@aurel32.net, rth@twiddle.net
ping
http://patchwork.ozlabs.org/patch/232770/
________________________________________
From: Petar Jovanovic [petar.jovanovic@rt-rk.com]
Sent: Monday, April 01, 2013 5:49 PM
To: qemu-devel@nongnu.org
Cc: Petar Jovanovic; riku.voipio@linaro.org; aurelien@aurel32.net; rth@twiddle.net; blauwirbel@gmail.com
Subject: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
From: Petar Jovanovic <petar.jovanovic@imgtec.com>
Previous implementation has failed to take into account different value of
SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
The same conversion has to be applied both for do_socket and do_socketpair,
so the code has been isolated in a static inline function.
The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
these are MIPS, SPARC and ALPHA now.
enum sock_type in linux-user/socket.h has been extended to include
TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
The patch also includes necessary code style changes (tab to spaces) in the
header file in the MIPS #ifdef block touched by the change.
Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
---
v2:
- the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
- values for sock_type are defined for SPARC and ALPHA in socket.h
linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
linux-user/syscall.c | 45 +++++----
2 files changed, 195 insertions(+), 98 deletions(-)
diff --git a/linux-user/socket.h b/linux-user/socket.h
index 339cae5..d2b05dc 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -1,91 +1,104 @@
#if defined(TARGET_MIPS)
- // MIPS special values for constants
-
- /*
- * For setsockopt(2)
- *
- * This defines are ABI conformant as far as Linux supports these ...
- */
- #define TARGET_SOL_SOCKET 0xffff
-
- #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
- #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
- #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
- SIGPIPE when they die. */
- #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
- #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
- broadcast messages. */
- #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
- socket to transmit pending data. */
- #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
- #if 0
- To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
- #endif
-
- #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
- #define TARGET_SO_STYLE SO_TYPE /* Synonym */
- #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
- #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
- #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
- #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
- #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
- #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
- #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
- #define TARGET_SO_ACCEPTCONN 0x1009
-
- /* linux-specific, might as well be the same as on i386 */
- #define TARGET_SO_NO_CHECK 11
- #define TARGET_SO_PRIORITY 12
- #define TARGET_SO_BSDCOMPAT 14
+ /* MIPS special values for constants */
+
+ /*
+ * For setsockopt(2)
+ *
+ * This defines are ABI conformant as far as Linux supports these ...
+ */
+ #define TARGET_SOL_SOCKET 0xffff
+
+ #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
+ #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
+ #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
+ SIGPIPE when they die. */
+ #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
+ #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
+ broadcast messages. */
+ #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
+ * socket to transmit pending data.
+ */
+ #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
+ */
+ #if 0
+ /* To add: Allow local address and port reuse. */
+ #define TARGET_SO_REUSEPORT 0x0200
+ #endif
+
+ #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
+ #define TARGET_SO_STYLE SO_TYPE /* Synonym */
+ #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
+ #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
+ #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
+ #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
+ #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
+ #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
+ #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
+ #define TARGET_SO_ACCEPTCONN 0x1009
- #define TARGET_SO_PASSCRED 17
- #define TARGET_SO_PEERCRED 18
+ /* linux-specific, might as well be the same as on i386 */
+ #define TARGET_SO_NO_CHECK 11
+ #define TARGET_SO_PRIORITY 12
+ #define TARGET_SO_BSDCOMPAT 14
- /* Security levels - as per NRL IPv6 - don't actually do anything */
- #define TARGET_SO_SECURITY_AUTHENTICATION 22
- #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
- #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
+ #define TARGET_SO_PASSCRED 17
+ #define TARGET_SO_PEERCRED 18
- #define TARGET_SO_BINDTODEVICE 25
+ /* Security levels - as per NRL IPv6 - don't actually do anything */
+ #define TARGET_SO_SECURITY_AUTHENTICATION 22
+ #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
+ #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
- /* Socket filtering */
- #define TARGET_SO_ATTACH_FILTER 26
- #define TARGET_SO_DETACH_FILTER 27
+ #define TARGET_SO_BINDTODEVICE 25
- #define TARGET_SO_PEERNAME 28
- #define TARGET_SO_TIMESTAMP 29
- #define SCM_TIMESTAMP SO_TIMESTAMP
-
- #define TARGET_SO_PEERSEC 30
- #define TARGET_SO_SNDBUFFORCE 31
- #define TARGET_SO_RCVBUFFORCE 33
-
- /** sock_type - Socket types
- *
- * Please notice that for binary compat reasons MIPS has to
- * override the enum sock_type in include/linux/net.h, so
- * we define ARCH_HAS_SOCKET_TYPES here.
- *
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- * For writing rarp and other similar things on the user level.
- */
- enum sock_type {
- TARGET_SOCK_DGRAM = 1,
- TARGET_SOCK_STREAM = 2,
- TARGET_SOCK_RAW = 3,
- TARGET_SOCK_RDM = 4,
- TARGET_SOCK_SEQPACKET = 5,
- TARGET_SOCK_DCCP = 6,
- TARGET_SOCK_PACKET = 10,
- };
-
- #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
+ /* Socket filtering */
+ #define TARGET_SO_ATTACH_FILTER 26
+ #define TARGET_SO_DETACH_FILTER 27
+
+ #define TARGET_SO_PEERNAME 28
+ #define TARGET_SO_TIMESTAMP 29
+ #define SCM_TIMESTAMP SO_TIMESTAMP
+
+ #define TARGET_SO_PEERSEC 30
+ #define TARGET_SO_SNDBUFFORCE 31
+ #define TARGET_SO_RCVBUFFORCE 33
+
+ /** sock_type - Socket types
+ *
+ * Please notice that for binary compat reasons MIPS has to
+ * override the enum sock_type in include/linux/net.h, so
+ * we define ARCH_HAS_SOCKET_TYPES here.
+ *
+ * @SOCK_DGRAM - datagram (conn.less) socket
+ * @SOCK_STREAM - stream (connection) socket
+ * @SOCK_RAW - raw socket
+ * @SOCK_RDM - reliably-delivered message
+ * @SOCK_SEQPACKET - sequential packet socket
+ * @SOCK_DCCP - Datagram Congestion Control Protocol socket
+ * @SOCK_PACKET - linux specific way of getting packets at the dev level.
+ * For writing rarp and other similar things on the user
+ * level.
+ * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
+ * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
+ */
+
+ #define ARCH_HAS_SOCKET_TYPES 1
+
+ enum sock_type {
+ TARGET_SOCK_DGRAM = 1,
+ TARGET_SOCK_STREAM = 2,
+ TARGET_SOCK_RAW = 3,
+ TARGET_SOCK_RDM = 4,
+ TARGET_SOCK_SEQPACKET = 5,
+ TARGET_SOCK_DCCP = 6,
+ TARGET_SOCK_PACKET = 10,
+ TARGET_SOCK_CLOEXEC = 02000000,
+ TARGET_SOCK_NONBLOCK = 0200,
+ };
+
+ #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
+ #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
#elif defined(TARGET_ALPHA)
@@ -156,8 +169,81 @@
/* Instruct lower device to use last 4-bytes of skb data as FCS */
#define TARGET_SO_NOFCS 43
+ /** sock_type - Socket types
+ *
+ * Please notice that for binary compat reasons ALPHA has to
+ * override the enum sock_type in include/linux/net.h, so
+ * we define ARCH_HAS_SOCKET_TYPES here.
+ *
+ * @SOCK_DGRAM - datagram (conn.less) socket
+ * @SOCK_STREAM - stream (connection) socket
+ * @SOCK_RAW - raw socket
+ * @SOCK_RDM - reliably-delivered message
+ * @SOCK_SEQPACKET - sequential packet socket
+ * @SOCK_DCCP - Datagram Congestion Control Protocol socket
+ * @SOCK_PACKET - linux specific way of getting packets at the dev level.
+ * For writing rarp and other similar things on the user
+ * level.
+ * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
+ * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
+ */
+
+ #define ARCH_HAS_SOCKET_TYPES 1
+
+ enum sock_type {
+ TARGET_SOCK_STREAM = 1,
+ TARGET_SOCK_DGRAM = 2,
+ TARGET_SOCK_RAW = 3,
+ TARGET_SOCK_RDM = 4,
+ TARGET_SOCK_SEQPACKET = 5,
+ TARGET_SOCK_DCCP = 6,
+ TARGET_SOCK_PACKET = 10,
+ TARGET_SOCK_CLOEXEC = 010000000,
+ TARGET_SOCK_NONBLOCK = 010000000000,
+ };
+
+ #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
+ #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
#else
+#if defined(TARGET_SPARC)
+ /** sock_type - Socket types
+ *
+ * Please notice that for binary compat reasons SPARC has to
+ * override the enum sock_type in include/linux/net.h, so
+ * we define ARCH_HAS_SOCKET_TYPES here.
+ *
+ * @SOCK_DGRAM - datagram (conn.less) socket
+ * @SOCK_STREAM - stream (connection) socket
+ * @SOCK_RAW - raw socket
+ * @SOCK_RDM - reliably-delivered message
+ * @SOCK_SEQPACKET - sequential packet socket
+ * @SOCK_DCCP - Datagram Congestion Control Protocol socket
+ * @SOCK_PACKET - linux specific way of getting packets at the dev level.
+ * For writing rarp and other similar things on the user
+ * level.
+ * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
+ * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
+ */
+
+ #define ARCH_HAS_SOCKET_TYPES 1
+
+ enum sock_type {
+ TARGET_SOCK_STREAM = 1,
+ TARGET_SOCK_DGRAM = 2,
+ TARGET_SOCK_RAW = 3,
+ TARGET_SOCK_RDM = 4,
+ TARGET_SOCK_SEQPACKET = 5,
+ TARGET_SOCK_DCCP = 6,
+ TARGET_SOCK_PACKET = 10,
+ TARGET_SOCK_CLOEXEC = 020000000,
+ TARGET_SOCK_NONBLOCK = 040000,
+ };
+
+ #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
+ #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
+#endif
+
/* For setsockopt(2) */
#define TARGET_SOL_SOCKET 1
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ee82a2d..5b87c9d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
free(vec);
}
-/* do_socket() Must return target values and target errnos. */
-static abi_long do_socket(int domain, int type, int protocol)
+#if defined(ARCH_HAS_SOCKET_TYPES)
+static inline void target_to_host_sock_type(int *type)
{
-#if defined(TARGET_MIPS)
- switch(type) {
+ int host_type = 0;
+ int target_type = *type;
+
+ switch (target_type & TARGET_SOCK_TYPE_MASK) {
case TARGET_SOCK_DGRAM:
- type = SOCK_DGRAM;
+ host_type = SOCK_DGRAM;
break;
case TARGET_SOCK_STREAM:
- type = SOCK_STREAM;
- break;
- case TARGET_SOCK_RAW:
- type = SOCK_RAW;
+ host_type = SOCK_STREAM;
break;
- case TARGET_SOCK_RDM:
- type = SOCK_RDM;
- break;
- case TARGET_SOCK_SEQPACKET:
- type = SOCK_SEQPACKET;
- break;
- case TARGET_SOCK_PACKET:
- type = SOCK_PACKET;
+ default:
+ host_type = target_type & TARGET_SOCK_TYPE_MASK;
break;
}
+ if (target_type & TARGET_SOCK_CLOEXEC) {
+ host_type |= SOCK_CLOEXEC;
+ }
+ if (target_type & TARGET_SOCK_NONBLOCK) {
+ host_type |= SOCK_NONBLOCK;
+ }
+ *type = host_type;
+}
+#endif
+
+/* do_socket() Must return target values and target errnos. */
+static abi_long do_socket(int domain, int type, int protocol)
+{
+#if defined(ARCH_HAS_SOCKET_TYPES)
+ target_to_host_sock_type(&type);
#endif
if (domain == PF_NETLINK)
return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
@@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
int tab[2];
abi_long ret;
+#if defined(ARCH_HAS_SOCKET_TYPES)
+ target_to_host_sock_type(&type);
+#endif
ret = get_errno(socketpair(domain, type, protocol, tab));
if (!is_error(ret)) {
if (put_user_s32(tab[0], target_tab_addr)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-04-01 15:49 [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion Petar Jovanovic
2013-04-10 18:17 ` Petar Jovanovic
@ 2013-04-15 13:47 ` Aurelien Jarno
2013-04-23 0:15 ` Petar Jovanovic
2013-06-27 17:21 ` Peter Maydell
2 siblings, 1 reply; 14+ messages in thread
From: Aurelien Jarno @ 2013-04-15 13:47 UTC (permalink / raw)
To: Petar Jovanovic; +Cc: blauwirbel, riku.voipio, rth, qemu-devel, petar.jovanovic
On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>
> Previous implementation has failed to take into account different value of
> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
> The same conversion has to be applied both for do_socket and do_socketpair,
> so the code has been isolated in a static inline function.
> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
> these are MIPS, SPARC and ALPHA now.
>
> enum sock_type in linux-user/socket.h has been extended to include
> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
> The patch also includes necessary code style changes (tab to spaces) in the
> header file in the MIPS #ifdef block touched by the change.
>
> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
> ---
> v2:
>
> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
> - values for sock_type are defined for SPARC and ALPHA in socket.h
>
> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
> linux-user/syscall.c | 45 +++++----
> 2 files changed, 195 insertions(+), 98 deletions(-)
>
> diff --git a/linux-user/socket.h b/linux-user/socket.h
> index 339cae5..d2b05dc 100644
> --- a/linux-user/socket.h
> +++ b/linux-user/socket.h
> @@ -1,91 +1,104 @@
>
> #if defined(TARGET_MIPS)
> - // MIPS special values for constants
> -
> - /*
> - * For setsockopt(2)
> - *
> - * This defines are ABI conformant as far as Linux supports these ...
> - */
> - #define TARGET_SOL_SOCKET 0xffff
> -
> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
> - SIGPIPE when they die. */
> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
> - broadcast messages. */
> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
> - socket to transmit pending data. */
> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
> - #if 0
> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
> - #endif
> -
> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
> - #define TARGET_SO_ACCEPTCONN 0x1009
> -
> - /* linux-specific, might as well be the same as on i386 */
> - #define TARGET_SO_NO_CHECK 11
> - #define TARGET_SO_PRIORITY 12
> - #define TARGET_SO_BSDCOMPAT 14
> + /* MIPS special values for constants */
> +
> + /*
> + * For setsockopt(2)
> + *
> + * This defines are ABI conformant as far as Linux supports these ...
> + */
> + #define TARGET_SOL_SOCKET 0xffff
> +
> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
> + SIGPIPE when they die. */
> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
> + broadcast messages. */
> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
> + * socket to transmit pending data.
> + */
> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
> + */
> + #if 0
> + /* To add: Allow local address and port reuse. */
> + #define TARGET_SO_REUSEPORT 0x0200
> + #endif
> +
> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
> + #define TARGET_SO_ACCEPTCONN 0x1009
>
> - #define TARGET_SO_PASSCRED 17
> - #define TARGET_SO_PEERCRED 18
> + /* linux-specific, might as well be the same as on i386 */
> + #define TARGET_SO_NO_CHECK 11
> + #define TARGET_SO_PRIORITY 12
> + #define TARGET_SO_BSDCOMPAT 14
>
> - /* Security levels - as per NRL IPv6 - don't actually do anything */
> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
> + #define TARGET_SO_PASSCRED 17
> + #define TARGET_SO_PEERCRED 18
>
> - #define TARGET_SO_BINDTODEVICE 25
> + /* Security levels - as per NRL IPv6 - don't actually do anything */
> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>
> - /* Socket filtering */
> - #define TARGET_SO_ATTACH_FILTER 26
> - #define TARGET_SO_DETACH_FILTER 27
> + #define TARGET_SO_BINDTODEVICE 25
>
> - #define TARGET_SO_PEERNAME 28
> - #define TARGET_SO_TIMESTAMP 29
> - #define SCM_TIMESTAMP SO_TIMESTAMP
> -
> - #define TARGET_SO_PEERSEC 30
> - #define TARGET_SO_SNDBUFFORCE 31
> - #define TARGET_SO_RCVBUFFORCE 33
> -
> - /** sock_type - Socket types
> - *
> - * Please notice that for binary compat reasons MIPS has to
> - * override the enum sock_type in include/linux/net.h, so
> - * we define ARCH_HAS_SOCKET_TYPES here.
> - *
> - * @SOCK_DGRAM - datagram (conn.less) socket
> - * @SOCK_STREAM - stream (connection) socket
> - * @SOCK_RAW - raw socket
> - * @SOCK_RDM - reliably-delivered message
> - * @SOCK_SEQPACKET - sequential packet socket
> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> - * For writing rarp and other similar things on the user level.
> - */
> - enum sock_type {
> - TARGET_SOCK_DGRAM = 1,
> - TARGET_SOCK_STREAM = 2,
> - TARGET_SOCK_RAW = 3,
> - TARGET_SOCK_RDM = 4,
> - TARGET_SOCK_SEQPACKET = 5,
> - TARGET_SOCK_DCCP = 6,
> - TARGET_SOCK_PACKET = 10,
> - };
> -
> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
> + /* Socket filtering */
> + #define TARGET_SO_ATTACH_FILTER 26
> + #define TARGET_SO_DETACH_FILTER 27
> +
> + #define TARGET_SO_PEERNAME 28
> + #define TARGET_SO_TIMESTAMP 29
> + #define SCM_TIMESTAMP SO_TIMESTAMP
> +
> + #define TARGET_SO_PEERSEC 30
> + #define TARGET_SO_SNDBUFFORCE 31
> + #define TARGET_SO_RCVBUFFORCE 33
> +
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons MIPS has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_DGRAM = 1,
> + TARGET_SOCK_STREAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 02000000,
> + TARGET_SOCK_NONBLOCK = 0200,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>
> #elif defined(TARGET_ALPHA)
>
> @@ -156,8 +169,81 @@
> /* Instruct lower device to use last 4-bytes of skb data as FCS */
> #define TARGET_SO_NOFCS 43
>
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons ALPHA has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_STREAM = 1,
> + TARGET_SOCK_DGRAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 010000000,
> + TARGET_SOCK_NONBLOCK = 010000000000,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
> #else
>
> +#if defined(TARGET_SPARC)
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons SPARC has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_STREAM = 1,
> + TARGET_SOCK_DGRAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 020000000,
> + TARGET_SOCK_NONBLOCK = 040000,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
> +#endif
> +
> /* For setsockopt(2) */
> #define TARGET_SOL_SOCKET 1
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ee82a2d..5b87c9d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
> free(vec);
> }
>
> -/* do_socket() Must return target values and target errnos. */
> -static abi_long do_socket(int domain, int type, int protocol)
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> +static inline void target_to_host_sock_type(int *type)
> {
> -#if defined(TARGET_MIPS)
> - switch(type) {
> + int host_type = 0;
> + int target_type = *type;
> +
> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
> case TARGET_SOCK_DGRAM:
> - type = SOCK_DGRAM;
> + host_type = SOCK_DGRAM;
> break;
> case TARGET_SOCK_STREAM:
> - type = SOCK_STREAM;
> - break;
> - case TARGET_SOCK_RAW:
> - type = SOCK_RAW;
> + host_type = SOCK_STREAM;
> break;
> - case TARGET_SOCK_RDM:
> - type = SOCK_RDM;
> - break;
> - case TARGET_SOCK_SEQPACKET:
> - type = SOCK_SEQPACKET;
> - break;
> - case TARGET_SOCK_PACKET:
> - type = SOCK_PACKET;
> + default:
> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
> break;
> }
> + if (target_type & TARGET_SOCK_CLOEXEC) {
> + host_type |= SOCK_CLOEXEC;
> + }
> + if (target_type & TARGET_SOCK_NONBLOCK) {
> + host_type |= SOCK_NONBLOCK;
> + }
> + *type = host_type;
The problem I see with this way of handling the flags is that if the
kernel later adds a new flag value, QEMU will silently drop it. It means
that if the glibc tries to see it will think the kernel (and here QEMU)
supports it (as no error is returned), and will not use to the fallback
code. For that each processed flag should probably be removed from
target_type and it should be 0 at the end of the function.
Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
supported by the glibc, but silently dropped by QEMU.
That said I agree it's purely hypothetical, as no new flags has been
added recently or is planned to be added. We can also decide to fix that
when a new flag is added. What the other people (the Cc:ed one) think?
> +}
> +#endif
> +
> +/* do_socket() Must return target values and target errnos. */
> +static abi_long do_socket(int domain, int type, int protocol)
> +{
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> + target_to_host_sock_type(&type);
> #endif
> if (domain == PF_NETLINK)
> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
> int tab[2];
> abi_long ret;
>
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> + target_to_host_sock_type(&type);
> +#endif
> ret = get_errno(socketpair(domain, type, protocol, tab));
> if (!is_error(ret)) {
> if (put_user_s32(tab[0], target_tab_addr)
Otherwise the patch looks fine.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-04-15 13:47 ` Aurelien Jarno
@ 2013-04-23 0:15 ` Petar Jovanovic
2013-04-29 13:14 ` Petar Jovanovic
2013-04-29 14:56 ` Andreas Färber
0 siblings, 2 replies; 14+ messages in thread
From: Petar Jovanovic @ 2013-04-23 0:15 UTC (permalink / raw)
To: Aurelien Jarno, Petar Jovanovic
Cc: blauwirbel@gmail.com, riku.voipio@linaro.org,
qemu-devel@nongnu.org, rth@twiddle.net
Thanks Aurelien for your comments.
What others think? Can we submit this version of the patch? Riku? Richard, Blue?
Petar
________________________________________
From: Aurelien Jarno [aurelien@aurel32.net]
Sent: Monday, April 15, 2013 3:47 PM
To: Petar Jovanovic
Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>
> Previous implementation has failed to take into account different value of
> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
> The same conversion has to be applied both for do_socket and do_socketpair,
> so the code has been isolated in a static inline function.
> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
> these are MIPS, SPARC and ALPHA now.
>
> enum sock_type in linux-user/socket.h has been extended to include
> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
> The patch also includes necessary code style changes (tab to spaces) in the
> header file in the MIPS #ifdef block touched by the change.
>
> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
> ---
> v2:
>
> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
> - values for sock_type are defined for SPARC and ALPHA in socket.h
>
> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
> linux-user/syscall.c | 45 +++++----
> 2 files changed, 195 insertions(+), 98 deletions(-)
>
> diff --git a/linux-user/socket.h b/linux-user/socket.h
> index 339cae5..d2b05dc 100644
> --- a/linux-user/socket.h
> +++ b/linux-user/socket.h
> @@ -1,91 +1,104 @@
>
> #if defined(TARGET_MIPS)
> - // MIPS special values for constants
> -
> - /*
> - * For setsockopt(2)
> - *
> - * This defines are ABI conformant as far as Linux supports these ...
> - */
> - #define TARGET_SOL_SOCKET 0xffff
> -
> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
> - SIGPIPE when they die. */
> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
> - broadcast messages. */
> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
> - socket to transmit pending data. */
> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
> - #if 0
> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
> - #endif
> -
> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
> - #define TARGET_SO_ACCEPTCONN 0x1009
> -
> - /* linux-specific, might as well be the same as on i386 */
> - #define TARGET_SO_NO_CHECK 11
> - #define TARGET_SO_PRIORITY 12
> - #define TARGET_SO_BSDCOMPAT 14
> + /* MIPS special values for constants */
> +
> + /*
> + * For setsockopt(2)
> + *
> + * This defines are ABI conformant as far as Linux supports these ...
> + */
> + #define TARGET_SOL_SOCKET 0xffff
> +
> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
> + SIGPIPE when they die. */
> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
> + broadcast messages. */
> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
> + * socket to transmit pending data.
> + */
> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
> + */
> + #if 0
> + /* To add: Allow local address and port reuse. */
> + #define TARGET_SO_REUSEPORT 0x0200
> + #endif
> +
> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
> + #define TARGET_SO_ACCEPTCONN 0x1009
>
> - #define TARGET_SO_PASSCRED 17
> - #define TARGET_SO_PEERCRED 18
> + /* linux-specific, might as well be the same as on i386 */
> + #define TARGET_SO_NO_CHECK 11
> + #define TARGET_SO_PRIORITY 12
> + #define TARGET_SO_BSDCOMPAT 14
>
> - /* Security levels - as per NRL IPv6 - don't actually do anything */
> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
> + #define TARGET_SO_PASSCRED 17
> + #define TARGET_SO_PEERCRED 18
>
> - #define TARGET_SO_BINDTODEVICE 25
> + /* Security levels - as per NRL IPv6 - don't actually do anything */
> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>
> - /* Socket filtering */
> - #define TARGET_SO_ATTACH_FILTER 26
> - #define TARGET_SO_DETACH_FILTER 27
> + #define TARGET_SO_BINDTODEVICE 25
>
> - #define TARGET_SO_PEERNAME 28
> - #define TARGET_SO_TIMESTAMP 29
> - #define SCM_TIMESTAMP SO_TIMESTAMP
> -
> - #define TARGET_SO_PEERSEC 30
> - #define TARGET_SO_SNDBUFFORCE 31
> - #define TARGET_SO_RCVBUFFORCE 33
> -
> - /** sock_type - Socket types
> - *
> - * Please notice that for binary compat reasons MIPS has to
> - * override the enum sock_type in include/linux/net.h, so
> - * we define ARCH_HAS_SOCKET_TYPES here.
> - *
> - * @SOCK_DGRAM - datagram (conn.less) socket
> - * @SOCK_STREAM - stream (connection) socket
> - * @SOCK_RAW - raw socket
> - * @SOCK_RDM - reliably-delivered message
> - * @SOCK_SEQPACKET - sequential packet socket
> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> - * For writing rarp and other similar things on the user level.
> - */
> - enum sock_type {
> - TARGET_SOCK_DGRAM = 1,
> - TARGET_SOCK_STREAM = 2,
> - TARGET_SOCK_RAW = 3,
> - TARGET_SOCK_RDM = 4,
> - TARGET_SOCK_SEQPACKET = 5,
> - TARGET_SOCK_DCCP = 6,
> - TARGET_SOCK_PACKET = 10,
> - };
> -
> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
> + /* Socket filtering */
> + #define TARGET_SO_ATTACH_FILTER 26
> + #define TARGET_SO_DETACH_FILTER 27
> +
> + #define TARGET_SO_PEERNAME 28
> + #define TARGET_SO_TIMESTAMP 29
> + #define SCM_TIMESTAMP SO_TIMESTAMP
> +
> + #define TARGET_SO_PEERSEC 30
> + #define TARGET_SO_SNDBUFFORCE 31
> + #define TARGET_SO_RCVBUFFORCE 33
> +
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons MIPS has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_DGRAM = 1,
> + TARGET_SOCK_STREAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 02000000,
> + TARGET_SOCK_NONBLOCK = 0200,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>
> #elif defined(TARGET_ALPHA)
>
> @@ -156,8 +169,81 @@
> /* Instruct lower device to use last 4-bytes of skb data as FCS */
> #define TARGET_SO_NOFCS 43
>
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons ALPHA has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_STREAM = 1,
> + TARGET_SOCK_DGRAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 010000000,
> + TARGET_SOCK_NONBLOCK = 010000000000,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
> #else
>
> +#if defined(TARGET_SPARC)
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons SPARC has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_STREAM = 1,
> + TARGET_SOCK_DGRAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 020000000,
> + TARGET_SOCK_NONBLOCK = 040000,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
> +#endif
> +
> /* For setsockopt(2) */
> #define TARGET_SOL_SOCKET 1
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ee82a2d..5b87c9d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
> free(vec);
> }
>
> -/* do_socket() Must return target values and target errnos. */
> -static abi_long do_socket(int domain, int type, int protocol)
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> +static inline void target_to_host_sock_type(int *type)
> {
> -#if defined(TARGET_MIPS)
> - switch(type) {
> + int host_type = 0;
> + int target_type = *type;
> +
> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
> case TARGET_SOCK_DGRAM:
> - type = SOCK_DGRAM;
> + host_type = SOCK_DGRAM;
> break;
> case TARGET_SOCK_STREAM:
> - type = SOCK_STREAM;
> - break;
> - case TARGET_SOCK_RAW:
> - type = SOCK_RAW;
> + host_type = SOCK_STREAM;
> break;
> - case TARGET_SOCK_RDM:
> - type = SOCK_RDM;
> - break;
> - case TARGET_SOCK_SEQPACKET:
> - type = SOCK_SEQPACKET;
> - break;
> - case TARGET_SOCK_PACKET:
> - type = SOCK_PACKET;
> + default:
> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
> break;
> }
> + if (target_type & TARGET_SOCK_CLOEXEC) {
> + host_type |= SOCK_CLOEXEC;
> + }
> + if (target_type & TARGET_SOCK_NONBLOCK) {
> + host_type |= SOCK_NONBLOCK;
> + }
> + *type = host_type;
The problem I see with this way of handling the flags is that if the
kernel later adds a new flag value, QEMU will silently drop it. It means
that if the glibc tries to see it will think the kernel (and here QEMU)
supports it (as no error is returned), and will not use to the fallback
code. For that each processed flag should probably be removed from
target_type and it should be 0 at the end of the function.
Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
supported by the glibc, but silently dropped by QEMU.
That said I agree it's purely hypothetical, as no new flags has been
added recently or is planned to be added. We can also decide to fix that
when a new flag is added. What the other people (the Cc:ed one) think?
> +}
> +#endif
> +
> +/* do_socket() Must return target values and target errnos. */
> +static abi_long do_socket(int domain, int type, int protocol)
> +{
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> + target_to_host_sock_type(&type);
> #endif
> if (domain == PF_NETLINK)
> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
> int tab[2];
> abi_long ret;
>
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> + target_to_host_sock_type(&type);
> +#endif
> ret = get_errno(socketpair(domain, type, protocol, tab));
> if (!is_error(ret)) {
> if (put_user_s32(tab[0], target_tab_addr)
Otherwise the patch looks fine.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-04-23 0:15 ` Petar Jovanovic
@ 2013-04-29 13:14 ` Petar Jovanovic
2013-04-29 14:56 ` Andreas Färber
1 sibling, 0 replies; 14+ messages in thread
From: Petar Jovanovic @ 2013-04-29 13:14 UTC (permalink / raw)
To: riku.voipio@linaro.org, rth@twiddle.net, blauwirbel@gmail.com
Cc: qemu-devel@nongnu.org, Aurelien Jarno, Petar Jovanovic
ping
http://patchwork.ozlabs.org/patch/232770/
________________________________________
From: Petar Jovanovic
Sent: Tuesday, April 23, 2013 2:15 AM
To: Aurelien Jarno; Petar Jovanovic
Cc: qemu-devel@nongnu.org; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
Subject: RE: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
Thanks Aurelien for your comments.
What others think? Can we submit this version of the patch? Riku? Richard, Blue?
Petar
________________________________________
From: Aurelien Jarno [aurelien@aurel32.net]
Sent: Monday, April 15, 2013 3:47 PM
To: Petar Jovanovic
Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>
> Previous implementation has failed to take into account different value of
> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
> The same conversion has to be applied both for do_socket and do_socketpair,
> so the code has been isolated in a static inline function.
> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
> these are MIPS, SPARC and ALPHA now.
>
> enum sock_type in linux-user/socket.h has been extended to include
> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
> The patch also includes necessary code style changes (tab to spaces) in the
> header file in the MIPS #ifdef block touched by the change.
>
> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
> ---
> v2:
>
> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
> - values for sock_type are defined for SPARC and ALPHA in socket.h
>
> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
> linux-user/syscall.c | 45 +++++----
> 2 files changed, 195 insertions(+), 98 deletions(-)
>
> diff --git a/linux-user/socket.h b/linux-user/socket.h
> index 339cae5..d2b05dc 100644
> --- a/linux-user/socket.h
> +++ b/linux-user/socket.h
> @@ -1,91 +1,104 @@
>
> #if defined(TARGET_MIPS)
> - // MIPS special values for constants
> -
> - /*
> - * For setsockopt(2)
> - *
> - * This defines are ABI conformant as far as Linux supports these ...
> - */
> - #define TARGET_SOL_SOCKET 0xffff
> -
> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
> - SIGPIPE when they die. */
> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
> - broadcast messages. */
> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
> - socket to transmit pending data. */
> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
> - #if 0
> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
> - #endif
> -
> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
> - #define TARGET_SO_ACCEPTCONN 0x1009
> -
> - /* linux-specific, might as well be the same as on i386 */
> - #define TARGET_SO_NO_CHECK 11
> - #define TARGET_SO_PRIORITY 12
> - #define TARGET_SO_BSDCOMPAT 14
> + /* MIPS special values for constants */
> +
> + /*
> + * For setsockopt(2)
> + *
> + * This defines are ABI conformant as far as Linux supports these ...
> + */
> + #define TARGET_SOL_SOCKET 0xffff
> +
> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
> + SIGPIPE when they die. */
> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
> + broadcast messages. */
> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
> + * socket to transmit pending data.
> + */
> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
> + */
> + #if 0
> + /* To add: Allow local address and port reuse. */
> + #define TARGET_SO_REUSEPORT 0x0200
> + #endif
> +
> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
> + #define TARGET_SO_ACCEPTCONN 0x1009
>
> - #define TARGET_SO_PASSCRED 17
> - #define TARGET_SO_PEERCRED 18
> + /* linux-specific, might as well be the same as on i386 */
> + #define TARGET_SO_NO_CHECK 11
> + #define TARGET_SO_PRIORITY 12
> + #define TARGET_SO_BSDCOMPAT 14
>
> - /* Security levels - as per NRL IPv6 - don't actually do anything */
> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
> + #define TARGET_SO_PASSCRED 17
> + #define TARGET_SO_PEERCRED 18
>
> - #define TARGET_SO_BINDTODEVICE 25
> + /* Security levels - as per NRL IPv6 - don't actually do anything */
> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>
> - /* Socket filtering */
> - #define TARGET_SO_ATTACH_FILTER 26
> - #define TARGET_SO_DETACH_FILTER 27
> + #define TARGET_SO_BINDTODEVICE 25
>
> - #define TARGET_SO_PEERNAME 28
> - #define TARGET_SO_TIMESTAMP 29
> - #define SCM_TIMESTAMP SO_TIMESTAMP
> -
> - #define TARGET_SO_PEERSEC 30
> - #define TARGET_SO_SNDBUFFORCE 31
> - #define TARGET_SO_RCVBUFFORCE 33
> -
> - /** sock_type - Socket types
> - *
> - * Please notice that for binary compat reasons MIPS has to
> - * override the enum sock_type in include/linux/net.h, so
> - * we define ARCH_HAS_SOCKET_TYPES here.
> - *
> - * @SOCK_DGRAM - datagram (conn.less) socket
> - * @SOCK_STREAM - stream (connection) socket
> - * @SOCK_RAW - raw socket
> - * @SOCK_RDM - reliably-delivered message
> - * @SOCK_SEQPACKET - sequential packet socket
> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> - * For writing rarp and other similar things on the user level.
> - */
> - enum sock_type {
> - TARGET_SOCK_DGRAM = 1,
> - TARGET_SOCK_STREAM = 2,
> - TARGET_SOCK_RAW = 3,
> - TARGET_SOCK_RDM = 4,
> - TARGET_SOCK_SEQPACKET = 5,
> - TARGET_SOCK_DCCP = 6,
> - TARGET_SOCK_PACKET = 10,
> - };
> -
> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
> + /* Socket filtering */
> + #define TARGET_SO_ATTACH_FILTER 26
> + #define TARGET_SO_DETACH_FILTER 27
> +
> + #define TARGET_SO_PEERNAME 28
> + #define TARGET_SO_TIMESTAMP 29
> + #define SCM_TIMESTAMP SO_TIMESTAMP
> +
> + #define TARGET_SO_PEERSEC 30
> + #define TARGET_SO_SNDBUFFORCE 31
> + #define TARGET_SO_RCVBUFFORCE 33
> +
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons MIPS has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_DGRAM = 1,
> + TARGET_SOCK_STREAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 02000000,
> + TARGET_SOCK_NONBLOCK = 0200,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>
> #elif defined(TARGET_ALPHA)
>
> @@ -156,8 +169,81 @@
> /* Instruct lower device to use last 4-bytes of skb data as FCS */
> #define TARGET_SO_NOFCS 43
>
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons ALPHA has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_STREAM = 1,
> + TARGET_SOCK_DGRAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 010000000,
> + TARGET_SOCK_NONBLOCK = 010000000000,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
> #else
>
> +#if defined(TARGET_SPARC)
> + /** sock_type - Socket types
> + *
> + * Please notice that for binary compat reasons SPARC has to
> + * override the enum sock_type in include/linux/net.h, so
> + * we define ARCH_HAS_SOCKET_TYPES here.
> + *
> + * @SOCK_DGRAM - datagram (conn.less) socket
> + * @SOCK_STREAM - stream (connection) socket
> + * @SOCK_RAW - raw socket
> + * @SOCK_RDM - reliably-delivered message
> + * @SOCK_SEQPACKET - sequential packet socket
> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> + * For writing rarp and other similar things on the user
> + * level.
> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> + */
> +
> + #define ARCH_HAS_SOCKET_TYPES 1
> +
> + enum sock_type {
> + TARGET_SOCK_STREAM = 1,
> + TARGET_SOCK_DGRAM = 2,
> + TARGET_SOCK_RAW = 3,
> + TARGET_SOCK_RDM = 4,
> + TARGET_SOCK_SEQPACKET = 5,
> + TARGET_SOCK_DCCP = 6,
> + TARGET_SOCK_PACKET = 10,
> + TARGET_SOCK_CLOEXEC = 020000000,
> + TARGET_SOCK_NONBLOCK = 040000,
> + };
> +
> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
> +#endif
> +
> /* For setsockopt(2) */
> #define TARGET_SOL_SOCKET 1
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ee82a2d..5b87c9d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
> free(vec);
> }
>
> -/* do_socket() Must return target values and target errnos. */
> -static abi_long do_socket(int domain, int type, int protocol)
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> +static inline void target_to_host_sock_type(int *type)
> {
> -#if defined(TARGET_MIPS)
> - switch(type) {
> + int host_type = 0;
> + int target_type = *type;
> +
> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
> case TARGET_SOCK_DGRAM:
> - type = SOCK_DGRAM;
> + host_type = SOCK_DGRAM;
> break;
> case TARGET_SOCK_STREAM:
> - type = SOCK_STREAM;
> - break;
> - case TARGET_SOCK_RAW:
> - type = SOCK_RAW;
> + host_type = SOCK_STREAM;
> break;
> - case TARGET_SOCK_RDM:
> - type = SOCK_RDM;
> - break;
> - case TARGET_SOCK_SEQPACKET:
> - type = SOCK_SEQPACKET;
> - break;
> - case TARGET_SOCK_PACKET:
> - type = SOCK_PACKET;
> + default:
> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
> break;
> }
> + if (target_type & TARGET_SOCK_CLOEXEC) {
> + host_type |= SOCK_CLOEXEC;
> + }
> + if (target_type & TARGET_SOCK_NONBLOCK) {
> + host_type |= SOCK_NONBLOCK;
> + }
> + *type = host_type;
The problem I see with this way of handling the flags is that if the
kernel later adds a new flag value, QEMU will silently drop it. It means
that if the glibc tries to see it will think the kernel (and here QEMU)
supports it (as no error is returned), and will not use to the fallback
code. For that each processed flag should probably be removed from
target_type and it should be 0 at the end of the function.
Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
supported by the glibc, but silently dropped by QEMU.
That said I agree it's purely hypothetical, as no new flags has been
added recently or is planned to be added. We can also decide to fix that
when a new flag is added. What the other people (the Cc:ed one) think?
> +}
> +#endif
> +
> +/* do_socket() Must return target values and target errnos. */
> +static abi_long do_socket(int domain, int type, int protocol)
> +{
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> + target_to_host_sock_type(&type);
> #endif
> if (domain == PF_NETLINK)
> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
> int tab[2];
> abi_long ret;
>
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> + target_to_host_sock_type(&type);
> +#endif
> ret = get_errno(socketpair(domain, type, protocol, tab));
> if (!is_error(ret)) {
> if (put_user_s32(tab[0], target_tab_addr)
Otherwise the patch looks fine.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-04-23 0:15 ` Petar Jovanovic
2013-04-29 13:14 ` Petar Jovanovic
@ 2013-04-29 14:56 ` Andreas Färber
2013-04-30 1:20 ` Petar Jovanovic
1 sibling, 1 reply; 14+ messages in thread
From: Andreas Färber @ 2013-04-29 14:56 UTC (permalink / raw)
To: Petar Jovanovic
Cc: Alexander Graf, riku.voipio@linaro.org, Petar Jovanovic,
qemu-devel@nongnu.org, blauwirbel@gmail.com, Aurelien Jarno,
rth@twiddle.net
Am 23.04.2013 02:15, schrieb Petar Jovanovic:
> Thanks Aurelien for your comments.
>
> What others think? Can we submit this version of the patch? Riku? Richard, Blue?
No objection but isn't that a frequent issue that mappings may need to
be extended from time to time? The way I've seen that handled is on a
case by case basis mapping from one known value to another, with
defaulting to whatever form of error reporting appropriate. Here it
seems that some cases were dropped and we are now defaulting to taking
the literal value where no difference is known. This may lead to silent
errors, whereas an abort as other extreme may prohibit use cases with no
value difference between host and target.
Andreas
> ________________________________________
> From: Aurelien Jarno [aurelien@aurel32.net]
> Sent: Monday, April 15, 2013 3:47 PM
> To: Petar Jovanovic
> Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
> Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
>> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>
>> Previous implementation has failed to take into account different value of
>> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
>> The same conversion has to be applied both for do_socket and do_socketpair,
>> so the code has been isolated in a static inline function.
>> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
>> these are MIPS, SPARC and ALPHA now.
>>
>> enum sock_type in linux-user/socket.h has been extended to include
>> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
>> The patch also includes necessary code style changes (tab to spaces) in the
>> header file in the MIPS #ifdef block touched by the change.
>>
>> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
>> ---
>> v2:
>>
>> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
>> - values for sock_type are defined for SPARC and ALPHA in socket.h
>>
>> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
>> linux-user/syscall.c | 45 +++++----
>> 2 files changed, 195 insertions(+), 98 deletions(-)
>>
>> diff --git a/linux-user/socket.h b/linux-user/socket.h
>> index 339cae5..d2b05dc 100644
>> --- a/linux-user/socket.h
>> +++ b/linux-user/socket.h
>> @@ -1,91 +1,104 @@
>>
>> #if defined(TARGET_MIPS)
>> - // MIPS special values for constants
>> -
>> - /*
>> - * For setsockopt(2)
>> - *
>> - * This defines are ABI conformant as far as Linux supports these ...
>> - */
>> - #define TARGET_SOL_SOCKET 0xffff
>> -
>> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> - SIGPIPE when they die. */
>> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> - broadcast messages. */
>> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> - socket to transmit pending data. */
>> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
>> - #if 0
>> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
>> - #endif
>> -
>> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> - #define TARGET_SO_ACCEPTCONN 0x1009
>> -
>> - /* linux-specific, might as well be the same as on i386 */
>> - #define TARGET_SO_NO_CHECK 11
>> - #define TARGET_SO_PRIORITY 12
>> - #define TARGET_SO_BSDCOMPAT 14
>> + /* MIPS special values for constants */
>> +
>> + /*
>> + * For setsockopt(2)
>> + *
>> + * This defines are ABI conformant as far as Linux supports these ...
>> + */
>> + #define TARGET_SOL_SOCKET 0xffff
>> +
>> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> + SIGPIPE when they die. */
>> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> + broadcast messages. */
>> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> + * socket to transmit pending data.
>> + */
>> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
>> + */
>> + #if 0
>> + /* To add: Allow local address and port reuse. */
>> + #define TARGET_SO_REUSEPORT 0x0200
>> + #endif
>> +
>> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> + #define TARGET_SO_ACCEPTCONN 0x1009
>>
>> - #define TARGET_SO_PASSCRED 17
>> - #define TARGET_SO_PEERCRED 18
>> + /* linux-specific, might as well be the same as on i386 */
>> + #define TARGET_SO_NO_CHECK 11
>> + #define TARGET_SO_PRIORITY 12
>> + #define TARGET_SO_BSDCOMPAT 14
>>
>> - /* Security levels - as per NRL IPv6 - don't actually do anything */
>> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>> + #define TARGET_SO_PASSCRED 17
>> + #define TARGET_SO_PEERCRED 18
>>
>> - #define TARGET_SO_BINDTODEVICE 25
>> + /* Security levels - as per NRL IPv6 - don't actually do anything */
>> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>
>> - /* Socket filtering */
>> - #define TARGET_SO_ATTACH_FILTER 26
>> - #define TARGET_SO_DETACH_FILTER 27
>> + #define TARGET_SO_BINDTODEVICE 25
>>
>> - #define TARGET_SO_PEERNAME 28
>> - #define TARGET_SO_TIMESTAMP 29
>> - #define SCM_TIMESTAMP SO_TIMESTAMP
>> -
>> - #define TARGET_SO_PEERSEC 30
>> - #define TARGET_SO_SNDBUFFORCE 31
>> - #define TARGET_SO_RCVBUFFORCE 33
>> -
>> - /** sock_type - Socket types
>> - *
>> - * Please notice that for binary compat reasons MIPS has to
>> - * override the enum sock_type in include/linux/net.h, so
>> - * we define ARCH_HAS_SOCKET_TYPES here.
>> - *
>> - * @SOCK_DGRAM - datagram (conn.less) socket
>> - * @SOCK_STREAM - stream (connection) socket
>> - * @SOCK_RAW - raw socket
>> - * @SOCK_RDM - reliably-delivered message
>> - * @SOCK_SEQPACKET - sequential packet socket
>> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> - * For writing rarp and other similar things on the user level.
>> - */
>> - enum sock_type {
>> - TARGET_SOCK_DGRAM = 1,
>> - TARGET_SOCK_STREAM = 2,
>> - TARGET_SOCK_RAW = 3,
>> - TARGET_SOCK_RDM = 4,
>> - TARGET_SOCK_SEQPACKET = 5,
>> - TARGET_SOCK_DCCP = 6,
>> - TARGET_SOCK_PACKET = 10,
>> - };
>> -
>> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
>> + /* Socket filtering */
>> + #define TARGET_SO_ATTACH_FILTER 26
>> + #define TARGET_SO_DETACH_FILTER 27
>> +
>> + #define TARGET_SO_PEERNAME 28
>> + #define TARGET_SO_TIMESTAMP 29
>> + #define SCM_TIMESTAMP SO_TIMESTAMP
>> +
>> + #define TARGET_SO_PEERSEC 30
>> + #define TARGET_SO_SNDBUFFORCE 31
>> + #define TARGET_SO_RCVBUFFORCE 33
>> +
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons MIPS has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_DGRAM = 1,
>> + TARGET_SOCK_STREAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 02000000,
>> + TARGET_SOCK_NONBLOCK = 0200,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>
>> #elif defined(TARGET_ALPHA)
>>
>> @@ -156,8 +169,81 @@
>> /* Instruct lower device to use last 4-bytes of skb data as FCS */
>> #define TARGET_SO_NOFCS 43
>>
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons ALPHA has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 010000000,
>> + TARGET_SOCK_NONBLOCK = 010000000000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> #else
>>
>> +#if defined(TARGET_SPARC)
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons SPARC has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 020000000,
>> + TARGET_SOCK_NONBLOCK = 040000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> +#endif
>> +
>> /* For setsockopt(2) */
>> #define TARGET_SOL_SOCKET 1
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index ee82a2d..5b87c9d 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>> free(vec);
>> }
>>
>> -/* do_socket() Must return target values and target errnos. */
>> -static abi_long do_socket(int domain, int type, int protocol)
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> +static inline void target_to_host_sock_type(int *type)
>> {
>> -#if defined(TARGET_MIPS)
>> - switch(type) {
>> + int host_type = 0;
>> + int target_type = *type;
>> +
>> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
>> case TARGET_SOCK_DGRAM:
>> - type = SOCK_DGRAM;
>> + host_type = SOCK_DGRAM;
>> break;
>> case TARGET_SOCK_STREAM:
>> - type = SOCK_STREAM;
>> - break;
>> - case TARGET_SOCK_RAW:
>> - type = SOCK_RAW;
>> + host_type = SOCK_STREAM;
>> break;
>> - case TARGET_SOCK_RDM:
>> - type = SOCK_RDM;
>> - break;
>> - case TARGET_SOCK_SEQPACKET:
>> - type = SOCK_SEQPACKET;
>> - break;
>> - case TARGET_SOCK_PACKET:
>> - type = SOCK_PACKET;
>> + default:
>> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
>> break;
>> }
>> + if (target_type & TARGET_SOCK_CLOEXEC) {
>> + host_type |= SOCK_CLOEXEC;
>> + }
>> + if (target_type & TARGET_SOCK_NONBLOCK) {
>> + host_type |= SOCK_NONBLOCK;
>> + }
>> + *type = host_type;
>
> The problem I see with this way of handling the flags is that if the
> kernel later adds a new flag value, QEMU will silently drop it. It means
> that if the glibc tries to see it will think the kernel (and here QEMU)
> supports it (as no error is returned), and will not use to the fallback
> code. For that each processed flag should probably be removed from
> target_type and it should be 0 at the end of the function.
>
> Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
> supported by the glibc, but silently dropped by QEMU.
>
> That said I agree it's purely hypothetical, as no new flags has been
> added recently or is planned to be added. We can also decide to fix that
> when a new flag is added. What the other people (the Cc:ed one) think?
>
>> +}
>> +#endif
>> +
>> +/* do_socket() Must return target values and target errnos. */
>> +static abi_long do_socket(int domain, int type, int protocol)
>> +{
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> #endif
>> if (domain == PF_NETLINK)
>> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
>> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
>> int tab[2];
>> abi_long ret;
>>
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> +#endif
>> ret = get_errno(socketpair(domain, type, protocol, tab));
>> if (!is_error(ret)) {
>> if (put_user_s32(tab[0], target_tab_addr)
>
> Otherwise the patch looks fine.
>
>
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> aurelien@aurel32.net http://www.aurel32.net
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-04-29 14:56 ` Andreas Färber
@ 2013-04-30 1:20 ` Petar Jovanovic
2013-05-07 23:16 ` Petar Jovanovic
0 siblings, 1 reply; 14+ messages in thread
From: Petar Jovanovic @ 2013-04-30 1:20 UTC (permalink / raw)
To: Andreas Färber
Cc: Alexander Graf, riku.voipio@linaro.org, Petar Jovanovic,
qemu-devel@nongnu.org, blauwirbel@gmail.com, Aurelien Jarno,
rth@twiddle.net
My assumption was that if new socket values are added in future, they will likely be the same for all platforms, so they can be supported without adding new lines of code. Here we convert the values known to be different, we leave the values known to be the same, and for the incorrect values - we pass them to (host) kernel as they are in belief that kernel would return error. We could handle that part and check for incorrect value before passing it to the kernel, but in that case we bring more kernel logic to QEMU for the cases that are not likely to happen.
Petar
________________________________________
From: Andreas Färber [afaerber@suse.de]
Sent: Monday, April 29, 2013 4:56 PM
To: Petar Jovanovic
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
Subject: Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
Am 23.04.2013 02:15, schrieb Petar Jovanovic:
> Thanks Aurelien for your comments.
>
> What others think? Can we submit this version of the patch? Riku? Richard, Blue?
No objection but isn't that a frequent issue that mappings may need to
be extended from time to time? The way I've seen that handled is on a
case by case basis mapping from one known value to another, with
defaulting to whatever form of error reporting appropriate. Here it
seems that some cases were dropped and we are now defaulting to taking
the literal value where no difference is known. This may lead to silent
errors, whereas an abort as other extreme may prohibit use cases with no
value difference between host and target.
Andreas
> ________________________________________
> From: Aurelien Jarno [aurelien@aurel32.net]
> Sent: Monday, April 15, 2013 3:47 PM
> To: Petar Jovanovic
> Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
> Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
>> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>
>> Previous implementation has failed to take into account different value of
>> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
>> The same conversion has to be applied both for do_socket and do_socketpair,
>> so the code has been isolated in a static inline function.
>> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
>> these are MIPS, SPARC and ALPHA now.
>>
>> enum sock_type in linux-user/socket.h has been extended to include
>> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
>> The patch also includes necessary code style changes (tab to spaces) in the
>> header file in the MIPS #ifdef block touched by the change.
>>
>> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
>> ---
>> v2:
>>
>> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
>> - values for sock_type are defined for SPARC and ALPHA in socket.h
>>
>> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
>> linux-user/syscall.c | 45 +++++----
>> 2 files changed, 195 insertions(+), 98 deletions(-)
>>
>> diff --git a/linux-user/socket.h b/linux-user/socket.h
>> index 339cae5..d2b05dc 100644
>> --- a/linux-user/socket.h
>> +++ b/linux-user/socket.h
>> @@ -1,91 +1,104 @@
>>
>> #if defined(TARGET_MIPS)
>> - // MIPS special values for constants
>> -
>> - /*
>> - * For setsockopt(2)
>> - *
>> - * This defines are ABI conformant as far as Linux supports these ...
>> - */
>> - #define TARGET_SOL_SOCKET 0xffff
>> -
>> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> - SIGPIPE when they die. */
>> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> - broadcast messages. */
>> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> - socket to transmit pending data. */
>> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
>> - #if 0
>> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
>> - #endif
>> -
>> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> - #define TARGET_SO_ACCEPTCONN 0x1009
>> -
>> - /* linux-specific, might as well be the same as on i386 */
>> - #define TARGET_SO_NO_CHECK 11
>> - #define TARGET_SO_PRIORITY 12
>> - #define TARGET_SO_BSDCOMPAT 14
>> + /* MIPS special values for constants */
>> +
>> + /*
>> + * For setsockopt(2)
>> + *
>> + * This defines are ABI conformant as far as Linux supports these ...
>> + */
>> + #define TARGET_SOL_SOCKET 0xffff
>> +
>> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> + SIGPIPE when they die. */
>> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> + broadcast messages. */
>> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> + * socket to transmit pending data.
>> + */
>> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
>> + */
>> + #if 0
>> + /* To add: Allow local address and port reuse. */
>> + #define TARGET_SO_REUSEPORT 0x0200
>> + #endif
>> +
>> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> + #define TARGET_SO_ACCEPTCONN 0x1009
>>
>> - #define TARGET_SO_PASSCRED 17
>> - #define TARGET_SO_PEERCRED 18
>> + /* linux-specific, might as well be the same as on i386 */
>> + #define TARGET_SO_NO_CHECK 11
>> + #define TARGET_SO_PRIORITY 12
>> + #define TARGET_SO_BSDCOMPAT 14
>>
>> - /* Security levels - as per NRL IPv6 - don't actually do anything */
>> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>> + #define TARGET_SO_PASSCRED 17
>> + #define TARGET_SO_PEERCRED 18
>>
>> - #define TARGET_SO_BINDTODEVICE 25
>> + /* Security levels - as per NRL IPv6 - don't actually do anything */
>> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>
>> - /* Socket filtering */
>> - #define TARGET_SO_ATTACH_FILTER 26
>> - #define TARGET_SO_DETACH_FILTER 27
>> + #define TARGET_SO_BINDTODEVICE 25
>>
>> - #define TARGET_SO_PEERNAME 28
>> - #define TARGET_SO_TIMESTAMP 29
>> - #define SCM_TIMESTAMP SO_TIMESTAMP
>> -
>> - #define TARGET_SO_PEERSEC 30
>> - #define TARGET_SO_SNDBUFFORCE 31
>> - #define TARGET_SO_RCVBUFFORCE 33
>> -
>> - /** sock_type - Socket types
>> - *
>> - * Please notice that for binary compat reasons MIPS has to
>> - * override the enum sock_type in include/linux/net.h, so
>> - * we define ARCH_HAS_SOCKET_TYPES here.
>> - *
>> - * @SOCK_DGRAM - datagram (conn.less) socket
>> - * @SOCK_STREAM - stream (connection) socket
>> - * @SOCK_RAW - raw socket
>> - * @SOCK_RDM - reliably-delivered message
>> - * @SOCK_SEQPACKET - sequential packet socket
>> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> - * For writing rarp and other similar things on the user level.
>> - */
>> - enum sock_type {
>> - TARGET_SOCK_DGRAM = 1,
>> - TARGET_SOCK_STREAM = 2,
>> - TARGET_SOCK_RAW = 3,
>> - TARGET_SOCK_RDM = 4,
>> - TARGET_SOCK_SEQPACKET = 5,
>> - TARGET_SOCK_DCCP = 6,
>> - TARGET_SOCK_PACKET = 10,
>> - };
>> -
>> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
>> + /* Socket filtering */
>> + #define TARGET_SO_ATTACH_FILTER 26
>> + #define TARGET_SO_DETACH_FILTER 27
>> +
>> + #define TARGET_SO_PEERNAME 28
>> + #define TARGET_SO_TIMESTAMP 29
>> + #define SCM_TIMESTAMP SO_TIMESTAMP
>> +
>> + #define TARGET_SO_PEERSEC 30
>> + #define TARGET_SO_SNDBUFFORCE 31
>> + #define TARGET_SO_RCVBUFFORCE 33
>> +
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons MIPS has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_DGRAM = 1,
>> + TARGET_SOCK_STREAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 02000000,
>> + TARGET_SOCK_NONBLOCK = 0200,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>
>> #elif defined(TARGET_ALPHA)
>>
>> @@ -156,8 +169,81 @@
>> /* Instruct lower device to use last 4-bytes of skb data as FCS */
>> #define TARGET_SO_NOFCS 43
>>
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons ALPHA has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 010000000,
>> + TARGET_SOCK_NONBLOCK = 010000000000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> #else
>>
>> +#if defined(TARGET_SPARC)
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons SPARC has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 020000000,
>> + TARGET_SOCK_NONBLOCK = 040000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> +#endif
>> +
>> /* For setsockopt(2) */
>> #define TARGET_SOL_SOCKET 1
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index ee82a2d..5b87c9d 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>> free(vec);
>> }
>>
>> -/* do_socket() Must return target values and target errnos. */
>> -static abi_long do_socket(int domain, int type, int protocol)
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> +static inline void target_to_host_sock_type(int *type)
>> {
>> -#if defined(TARGET_MIPS)
>> - switch(type) {
>> + int host_type = 0;
>> + int target_type = *type;
>> +
>> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
>> case TARGET_SOCK_DGRAM:
>> - type = SOCK_DGRAM;
>> + host_type = SOCK_DGRAM;
>> break;
>> case TARGET_SOCK_STREAM:
>> - type = SOCK_STREAM;
>> - break;
>> - case TARGET_SOCK_RAW:
>> - type = SOCK_RAW;
>> + host_type = SOCK_STREAM;
>> break;
>> - case TARGET_SOCK_RDM:
>> - type = SOCK_RDM;
>> - break;
>> - case TARGET_SOCK_SEQPACKET:
>> - type = SOCK_SEQPACKET;
>> - break;
>> - case TARGET_SOCK_PACKET:
>> - type = SOCK_PACKET;
>> + default:
>> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
>> break;
>> }
>> + if (target_type & TARGET_SOCK_CLOEXEC) {
>> + host_type |= SOCK_CLOEXEC;
>> + }
>> + if (target_type & TARGET_SOCK_NONBLOCK) {
>> + host_type |= SOCK_NONBLOCK;
>> + }
>> + *type = host_type;
>
> The problem I see with this way of handling the flags is that if the
> kernel later adds a new flag value, QEMU will silently drop it. It means
> that if the glibc tries to see it will think the kernel (and here QEMU)
> supports it (as no error is returned), and will not use to the fallback
> code. For that each processed flag should probably be removed from
> target_type and it should be 0 at the end of the function.
>
> Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
> supported by the glibc, but silently dropped by QEMU.
>
> That said I agree it's purely hypothetical, as no new flags has been
> added recently or is planned to be added. We can also decide to fix that
> when a new flag is added. What the other people (the Cc:ed one) think?
>
>> +}
>> +#endif
>> +
>> +/* do_socket() Must return target values and target errnos. */
>> +static abi_long do_socket(int domain, int type, int protocol)
>> +{
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> #endif
>> if (domain == PF_NETLINK)
>> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
>> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
>> int tab[2];
>> abi_long ret;
>>
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> +#endif
>> ret = get_errno(socketpair(domain, type, protocol, tab));
>> if (!is_error(ret)) {
>> if (put_user_s32(tab[0], target_tab_addr)
>
> Otherwise the patch looks fine.
>
>
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> aurelien@aurel32.net http://www.aurel32.net
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-04-30 1:20 ` Petar Jovanovic
@ 2013-05-07 23:16 ` Petar Jovanovic
2013-05-22 13:41 ` Petar Jovanovic
2013-05-27 10:49 ` Petar Jovanovic
0 siblings, 2 replies; 14+ messages in thread
From: Petar Jovanovic @ 2013-05-07 23:16 UTC (permalink / raw)
To: riku.voipio@linaro.org, qemu-devel@nongnu.org
Cc: Alexander Graf, Petar Jovanovic, blauwirbel@gmail.com,
Andreas Färber, Aurelien Jarno, rth@twiddle.net
ping
http://patchwork.ozlabs.org/patch/232770/
________________________________________
From: Petar Jovanovic
Sent: Tuesday, April 30, 2013 3:20 AM
To: Andreas Färber
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
My assumption was that if new socket values are added in future, they will
likely be the same for all platforms, so they can be supported without
adding new lines of code. Here we convert the values known to be different,
we leave the values known to be the same, and for the incorrect values - we
pass them to (host) kernel as they are in belief that kernel would return
error. We could handle that part and check for incorrect value before
passing it to the kernel, but in that case we bring more kernel logic to
QEMU for the cases that are not likely to happen.
Petar
________________________________________
From: Andreas Färber [afaerber@suse.de]
Sent: Monday, April 29, 2013 4:56 PM
To: Petar Jovanovic
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
Subject: Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
Am 23.04.2013 02:15, schrieb Petar Jovanovic:
> Thanks Aurelien for your comments.
>
> What others think? Can we submit this version of the patch? Riku? Richard, Blue?
No objection but isn't that a frequent issue that mappings may need to
be extended from time to time? The way I've seen that handled is on a
case by case basis mapping from one known value to another, with
defaulting to whatever form of error reporting appropriate. Here it
seems that some cases were dropped and we are now defaulting to taking
the literal value where no difference is known. This may lead to silent
errors, whereas an abort as other extreme may prohibit use cases with no
value difference between host and target.
Andreas
> ________________________________________
> From: Aurelien Jarno [aurelien@aurel32.net]
> Sent: Monday, April 15, 2013 3:47 PM
> To: Petar Jovanovic
> Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
> Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
>> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>
>> Previous implementation has failed to take into account different value of
>> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
>> The same conversion has to be applied both for do_socket and do_socketpair,
>> so the code has been isolated in a static inline function.
>> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
>> these are MIPS, SPARC and ALPHA now.
>>
>> enum sock_type in linux-user/socket.h has been extended to include
>> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
>> The patch also includes necessary code style changes (tab to spaces) in the
>> header file in the MIPS #ifdef block touched by the change.
>>
>> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
>> ---
>> v2:
>>
>> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
>> - values for sock_type are defined for SPARC and ALPHA in socket.h
>>
>> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
>> linux-user/syscall.c | 45 +++++----
>> 2 files changed, 195 insertions(+), 98 deletions(-)
>>
>> diff --git a/linux-user/socket.h b/linux-user/socket.h
>> index 339cae5..d2b05dc 100644
>> --- a/linux-user/socket.h
>> +++ b/linux-user/socket.h
>> @@ -1,91 +1,104 @@
>>
>> #if defined(TARGET_MIPS)
>> - // MIPS special values for constants
>> -
>> - /*
>> - * For setsockopt(2)
>> - *
>> - * This defines are ABI conformant as far as Linux supports these ...
>> - */
>> - #define TARGET_SOL_SOCKET 0xffff
>> -
>> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> - SIGPIPE when they die. */
>> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> - broadcast messages. */
>> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> - socket to transmit pending data. */
>> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
>> - #if 0
>> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
>> - #endif
>> -
>> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> - #define TARGET_SO_ACCEPTCONN 0x1009
>> -
>> - /* linux-specific, might as well be the same as on i386 */
>> - #define TARGET_SO_NO_CHECK 11
>> - #define TARGET_SO_PRIORITY 12
>> - #define TARGET_SO_BSDCOMPAT 14
>> + /* MIPS special values for constants */
>> +
>> + /*
>> + * For setsockopt(2)
>> + *
>> + * This defines are ABI conformant as far as Linux supports these ...
>> + */
>> + #define TARGET_SOL_SOCKET 0xffff
>> +
>> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> + SIGPIPE when they die. */
>> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> + broadcast messages. */
>> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> + * socket to transmit pending data.
>> + */
>> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
>> + */
>> + #if 0
>> + /* To add: Allow local address and port reuse. */
>> + #define TARGET_SO_REUSEPORT 0x0200
>> + #endif
>> +
>> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> + #define TARGET_SO_ACCEPTCONN 0x1009
>>
>> - #define TARGET_SO_PASSCRED 17
>> - #define TARGET_SO_PEERCRED 18
>> + /* linux-specific, might as well be the same as on i386 */
>> + #define TARGET_SO_NO_CHECK 11
>> + #define TARGET_SO_PRIORITY 12
>> + #define TARGET_SO_BSDCOMPAT 14
>>
>> - /* Security levels - as per NRL IPv6 - don't actually do anything */
>> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>> + #define TARGET_SO_PASSCRED 17
>> + #define TARGET_SO_PEERCRED 18
>>
>> - #define TARGET_SO_BINDTODEVICE 25
>> + /* Security levels - as per NRL IPv6 - don't actually do anything */
>> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>
>> - /* Socket filtering */
>> - #define TARGET_SO_ATTACH_FILTER 26
>> - #define TARGET_SO_DETACH_FILTER 27
>> + #define TARGET_SO_BINDTODEVICE 25
>>
>> - #define TARGET_SO_PEERNAME 28
>> - #define TARGET_SO_TIMESTAMP 29
>> - #define SCM_TIMESTAMP SO_TIMESTAMP
>> -
>> - #define TARGET_SO_PEERSEC 30
>> - #define TARGET_SO_SNDBUFFORCE 31
>> - #define TARGET_SO_RCVBUFFORCE 33
>> -
>> - /** sock_type - Socket types
>> - *
>> - * Please notice that for binary compat reasons MIPS has to
>> - * override the enum sock_type in include/linux/net.h, so
>> - * we define ARCH_HAS_SOCKET_TYPES here.
>> - *
>> - * @SOCK_DGRAM - datagram (conn.less) socket
>> - * @SOCK_STREAM - stream (connection) socket
>> - * @SOCK_RAW - raw socket
>> - * @SOCK_RDM - reliably-delivered message
>> - * @SOCK_SEQPACKET - sequential packet socket
>> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> - * For writing rarp and other similar things on the user level.
>> - */
>> - enum sock_type {
>> - TARGET_SOCK_DGRAM = 1,
>> - TARGET_SOCK_STREAM = 2,
>> - TARGET_SOCK_RAW = 3,
>> - TARGET_SOCK_RDM = 4,
>> - TARGET_SOCK_SEQPACKET = 5,
>> - TARGET_SOCK_DCCP = 6,
>> - TARGET_SOCK_PACKET = 10,
>> - };
>> -
>> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
>> + /* Socket filtering */
>> + #define TARGET_SO_ATTACH_FILTER 26
>> + #define TARGET_SO_DETACH_FILTER 27
>> +
>> + #define TARGET_SO_PEERNAME 28
>> + #define TARGET_SO_TIMESTAMP 29
>> + #define SCM_TIMESTAMP SO_TIMESTAMP
>> +
>> + #define TARGET_SO_PEERSEC 30
>> + #define TARGET_SO_SNDBUFFORCE 31
>> + #define TARGET_SO_RCVBUFFORCE 33
>> +
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons MIPS has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_DGRAM = 1,
>> + TARGET_SOCK_STREAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 02000000,
>> + TARGET_SOCK_NONBLOCK = 0200,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>
>> #elif defined(TARGET_ALPHA)
>>
>> @@ -156,8 +169,81 @@
>> /* Instruct lower device to use last 4-bytes of skb data as FCS */
>> #define TARGET_SO_NOFCS 43
>>
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons ALPHA has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 010000000,
>> + TARGET_SOCK_NONBLOCK = 010000000000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> #else
>>
>> +#if defined(TARGET_SPARC)
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons SPARC has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 020000000,
>> + TARGET_SOCK_NONBLOCK = 040000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> +#endif
>> +
>> /* For setsockopt(2) */
>> #define TARGET_SOL_SOCKET 1
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index ee82a2d..5b87c9d 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>> free(vec);
>> }
>>
>> -/* do_socket() Must return target values and target errnos. */
>> -static abi_long do_socket(int domain, int type, int protocol)
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> +static inline void target_to_host_sock_type(int *type)
>> {
>> -#if defined(TARGET_MIPS)
>> - switch(type) {
>> + int host_type = 0;
>> + int target_type = *type;
>> +
>> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
>> case TARGET_SOCK_DGRAM:
>> - type = SOCK_DGRAM;
>> + host_type = SOCK_DGRAM;
>> break;
>> case TARGET_SOCK_STREAM:
>> - type = SOCK_STREAM;
>> - break;
>> - case TARGET_SOCK_RAW:
>> - type = SOCK_RAW;
>> + host_type = SOCK_STREAM;
>> break;
>> - case TARGET_SOCK_RDM:
>> - type = SOCK_RDM;
>> - break;
>> - case TARGET_SOCK_SEQPACKET:
>> - type = SOCK_SEQPACKET;
>> - break;
>> - case TARGET_SOCK_PACKET:
>> - type = SOCK_PACKET;
>> + default:
>> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
>> break;
>> }
>> + if (target_type & TARGET_SOCK_CLOEXEC) {
>> + host_type |= SOCK_CLOEXEC;
>> + }
>> + if (target_type & TARGET_SOCK_NONBLOCK) {
>> + host_type |= SOCK_NONBLOCK;
>> + }
>> + *type = host_type;
>
> The problem I see with this way of handling the flags is that if the
> kernel later adds a new flag value, QEMU will silently drop it. It means
> that if the glibc tries to see it will think the kernel (and here QEMU)
> supports it (as no error is returned), and will not use to the fallback
> code. For that each processed flag should probably be removed from
> target_type and it should be 0 at the end of the function.
>
> Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
> supported by the glibc, but silently dropped by QEMU.
>
> That said I agree it's purely hypothetical, as no new flags has been
> added recently or is planned to be added. We can also decide to fix that
> when a new flag is added. What the other people (the Cc:ed one) think?
>
>> +}
>> +#endif
>> +
>> +/* do_socket() Must return target values and target errnos. */
>> +static abi_long do_socket(int domain, int type, int protocol)
>> +{
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> #endif
>> if (domain == PF_NETLINK)
>> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
>> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
>> int tab[2];
>> abi_long ret;
>>
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> +#endif
>> ret = get_errno(socketpair(domain, type, protocol, tab));
>> if (!is_error(ret)) {
>> if (put_user_s32(tab[0], target_tab_addr)
>
> Otherwise the patch looks fine.
>
>
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> aurelien@aurel32.net http://www.aurel32.net
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-05-07 23:16 ` Petar Jovanovic
@ 2013-05-22 13:41 ` Petar Jovanovic
2013-05-27 10:49 ` Petar Jovanovic
1 sibling, 0 replies; 14+ messages in thread
From: Petar Jovanovic @ 2013-05-22 13:41 UTC (permalink / raw)
To: riku.voipio@linaro.org, qemu-devel@nongnu.org
Cc: Alexander Graf, Petar Jovanovic, blauwirbel@gmail.com,
Andreas Färber, Aurelien Jarno, rth@twiddle.net
ping
________________________________________
From: Petar Jovanovic
Sent: Wednesday, May 08, 2013 1:16 AM
To: riku.voipio@linaro.org; qemu-devel@nongnu.org
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; rth@twiddle.net; Alexander Graf; Andreas Färber
Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
ping
http://patchwork.ozlabs.org/patch/232770/
________________________________________
From: Petar Jovanovic
Sent: Tuesday, April 30, 2013 3:20 AM
To: Andreas Färber
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
My assumption was that if new socket values are added in future, they will
likely be the same for all platforms, so they can be supported without
adding new lines of code. Here we convert the values known to be different,
we leave the values known to be the same, and for the incorrect values - we
pass them to (host) kernel as they are in belief that kernel would return
error. We could handle that part and check for incorrect value before
passing it to the kernel, but in that case we bring more kernel logic to
QEMU for the cases that are not likely to happen.
Petar
________________________________________
From: Andreas Färber [afaerber@suse.de]
Sent: Monday, April 29, 2013 4:56 PM
To: Petar Jovanovic
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
Subject: Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
Am 23.04.2013 02:15, schrieb Petar Jovanovic:
> Thanks Aurelien for your comments.
>
> What others think? Can we submit this version of the patch? Riku? Richard, Blue?
No objection but isn't that a frequent issue that mappings may need to
be extended from time to time? The way I've seen that handled is on a
case by case basis mapping from one known value to another, with
defaulting to whatever form of error reporting appropriate. Here it
seems that some cases were dropped and we are now defaulting to taking
the literal value where no difference is known. This may lead to silent
errors, whereas an abort as other extreme may prohibit use cases with no
value difference between host and target.
Andreas
> ________________________________________
> From: Aurelien Jarno [aurelien@aurel32.net]
> Sent: Monday, April 15, 2013 3:47 PM
> To: Petar Jovanovic
> Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
> Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
>> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>
>> Previous implementation has failed to take into account different value of
>> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
>> The same conversion has to be applied both for do_socket and do_socketpair,
>> so the code has been isolated in a static inline function.
>> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
>> these are MIPS, SPARC and ALPHA now.
>>
>> enum sock_type in linux-user/socket.h has been extended to include
>> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
>> The patch also includes necessary code style changes (tab to spaces) in the
>> header file in the MIPS #ifdef block touched by the change.
>>
>> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
>> ---
>> v2:
>>
>> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
>> - values for sock_type are defined for SPARC and ALPHA in socket.h
>>
>> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
>> linux-user/syscall.c | 45 +++++----
>> 2 files changed, 195 insertions(+), 98 deletions(-)
>>
>> diff --git a/linux-user/socket.h b/linux-user/socket.h
>> index 339cae5..d2b05dc 100644
>> --- a/linux-user/socket.h
>> +++ b/linux-user/socket.h
>> @@ -1,91 +1,104 @@
>>
>> #if defined(TARGET_MIPS)
>> - // MIPS special values for constants
>> -
>> - /*
>> - * For setsockopt(2)
>> - *
>> - * This defines are ABI conformant as far as Linux supports these ...
>> - */
>> - #define TARGET_SOL_SOCKET 0xffff
>> -
>> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> - SIGPIPE when they die. */
>> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> - broadcast messages. */
>> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> - socket to transmit pending data. */
>> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
>> - #if 0
>> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
>> - #endif
>> -
>> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> - #define TARGET_SO_ACCEPTCONN 0x1009
>> -
>> - /* linux-specific, might as well be the same as on i386 */
>> - #define TARGET_SO_NO_CHECK 11
>> - #define TARGET_SO_PRIORITY 12
>> - #define TARGET_SO_BSDCOMPAT 14
>> + /* MIPS special values for constants */
>> +
>> + /*
>> + * For setsockopt(2)
>> + *
>> + * This defines are ABI conformant as far as Linux supports these ...
>> + */
>> + #define TARGET_SOL_SOCKET 0xffff
>> +
>> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> + SIGPIPE when they die. */
>> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> + broadcast messages. */
>> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> + * socket to transmit pending data.
>> + */
>> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
>> + */
>> + #if 0
>> + /* To add: Allow local address and port reuse. */
>> + #define TARGET_SO_REUSEPORT 0x0200
>> + #endif
>> +
>> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> + #define TARGET_SO_ACCEPTCONN 0x1009
>>
>> - #define TARGET_SO_PASSCRED 17
>> - #define TARGET_SO_PEERCRED 18
>> + /* linux-specific, might as well be the same as on i386 */
>> + #define TARGET_SO_NO_CHECK 11
>> + #define TARGET_SO_PRIORITY 12
>> + #define TARGET_SO_BSDCOMPAT 14
>>
>> - /* Security levels - as per NRL IPv6 - don't actually do anything */
>> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>> + #define TARGET_SO_PASSCRED 17
>> + #define TARGET_SO_PEERCRED 18
>>
>> - #define TARGET_SO_BINDTODEVICE 25
>> + /* Security levels - as per NRL IPv6 - don't actually do anything */
>> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>
>> - /* Socket filtering */
>> - #define TARGET_SO_ATTACH_FILTER 26
>> - #define TARGET_SO_DETACH_FILTER 27
>> + #define TARGET_SO_BINDTODEVICE 25
>>
>> - #define TARGET_SO_PEERNAME 28
>> - #define TARGET_SO_TIMESTAMP 29
>> - #define SCM_TIMESTAMP SO_TIMESTAMP
>> -
>> - #define TARGET_SO_PEERSEC 30
>> - #define TARGET_SO_SNDBUFFORCE 31
>> - #define TARGET_SO_RCVBUFFORCE 33
>> -
>> - /** sock_type - Socket types
>> - *
>> - * Please notice that for binary compat reasons MIPS has to
>> - * override the enum sock_type in include/linux/net.h, so
>> - * we define ARCH_HAS_SOCKET_TYPES here.
>> - *
>> - * @SOCK_DGRAM - datagram (conn.less) socket
>> - * @SOCK_STREAM - stream (connection) socket
>> - * @SOCK_RAW - raw socket
>> - * @SOCK_RDM - reliably-delivered message
>> - * @SOCK_SEQPACKET - sequential packet socket
>> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> - * For writing rarp and other similar things on the user level.
>> - */
>> - enum sock_type {
>> - TARGET_SOCK_DGRAM = 1,
>> - TARGET_SOCK_STREAM = 2,
>> - TARGET_SOCK_RAW = 3,
>> - TARGET_SOCK_RDM = 4,
>> - TARGET_SOCK_SEQPACKET = 5,
>> - TARGET_SOCK_DCCP = 6,
>> - TARGET_SOCK_PACKET = 10,
>> - };
>> -
>> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
>> + /* Socket filtering */
>> + #define TARGET_SO_ATTACH_FILTER 26
>> + #define TARGET_SO_DETACH_FILTER 27
>> +
>> + #define TARGET_SO_PEERNAME 28
>> + #define TARGET_SO_TIMESTAMP 29
>> + #define SCM_TIMESTAMP SO_TIMESTAMP
>> +
>> + #define TARGET_SO_PEERSEC 30
>> + #define TARGET_SO_SNDBUFFORCE 31
>> + #define TARGET_SO_RCVBUFFORCE 33
>> +
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons MIPS has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_DGRAM = 1,
>> + TARGET_SOCK_STREAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 02000000,
>> + TARGET_SOCK_NONBLOCK = 0200,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>
>> #elif defined(TARGET_ALPHA)
>>
>> @@ -156,8 +169,81 @@
>> /* Instruct lower device to use last 4-bytes of skb data as FCS */
>> #define TARGET_SO_NOFCS 43
>>
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons ALPHA has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 010000000,
>> + TARGET_SOCK_NONBLOCK = 010000000000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> #else
>>
>> +#if defined(TARGET_SPARC)
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons SPARC has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 020000000,
>> + TARGET_SOCK_NONBLOCK = 040000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> +#endif
>> +
>> /* For setsockopt(2) */
>> #define TARGET_SOL_SOCKET 1
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index ee82a2d..5b87c9d 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>> free(vec);
>> }
>>
>> -/* do_socket() Must return target values and target errnos. */
>> -static abi_long do_socket(int domain, int type, int protocol)
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> +static inline void target_to_host_sock_type(int *type)
>> {
>> -#if defined(TARGET_MIPS)
>> - switch(type) {
>> + int host_type = 0;
>> + int target_type = *type;
>> +
>> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
>> case TARGET_SOCK_DGRAM:
>> - type = SOCK_DGRAM;
>> + host_type = SOCK_DGRAM;
>> break;
>> case TARGET_SOCK_STREAM:
>> - type = SOCK_STREAM;
>> - break;
>> - case TARGET_SOCK_RAW:
>> - type = SOCK_RAW;
>> + host_type = SOCK_STREAM;
>> break;
>> - case TARGET_SOCK_RDM:
>> - type = SOCK_RDM;
>> - break;
>> - case TARGET_SOCK_SEQPACKET:
>> - type = SOCK_SEQPACKET;
>> - break;
>> - case TARGET_SOCK_PACKET:
>> - type = SOCK_PACKET;
>> + default:
>> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
>> break;
>> }
>> + if (target_type & TARGET_SOCK_CLOEXEC) {
>> + host_type |= SOCK_CLOEXEC;
>> + }
>> + if (target_type & TARGET_SOCK_NONBLOCK) {
>> + host_type |= SOCK_NONBLOCK;
>> + }
>> + *type = host_type;
>
> The problem I see with this way of handling the flags is that if the
> kernel later adds a new flag value, QEMU will silently drop it. It means
> that if the glibc tries to see it will think the kernel (and here QEMU)
> supports it (as no error is returned), and will not use to the fallback
> code. For that each processed flag should probably be removed from
> target_type and it should be 0 at the end of the function.
>
> Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
> supported by the glibc, but silently dropped by QEMU.
>
> That said I agree it's purely hypothetical, as no new flags has been
> added recently or is planned to be added. We can also decide to fix that
> when a new flag is added. What the other people (the Cc:ed one) think?
>
>> +}
>> +#endif
>> +
>> +/* do_socket() Must return target values and target errnos. */
>> +static abi_long do_socket(int domain, int type, int protocol)
>> +{
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> #endif
>> if (domain == PF_NETLINK)
>> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
>> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
>> int tab[2];
>> abi_long ret;
>>
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> +#endif
>> ret = get_errno(socketpair(domain, type, protocol, tab));
>> if (!is_error(ret)) {
>> if (put_user_s32(tab[0], target_tab_addr)
>
> Otherwise the patch looks fine.
>
>
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> aurelien@aurel32.net http://www.aurel32.net
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-05-07 23:16 ` Petar Jovanovic
2013-05-22 13:41 ` Petar Jovanovic
@ 2013-05-27 10:49 ` Petar Jovanovic
2013-05-30 13:28 ` Riku Voipio
1 sibling, 1 reply; 14+ messages in thread
From: Petar Jovanovic @ 2013-05-27 10:49 UTC (permalink / raw)
To: riku.voipio@linaro.org, qemu-devel@nongnu.org
Cc: Alexander Graf, Petar Jovanovic, blauwirbel@gmail.com,
Andreas Färber, Aurelien Jarno, rth@twiddle.net
Can anyone take a look at this and commit it if there are no other change requests?
Thank you.
________________________________________
From: Petar Jovanovic
Sent: Wednesday, May 08, 2013 1:16 AM
To: riku.voipio@linaro.org; qemu-devel@nongnu.org
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; rth@twiddle.net; Alexander Graf; Andreas Färber
Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
ping
http://patchwork.ozlabs.org/patch/232770/
________________________________________
From: Petar Jovanovic
Sent: Tuesday, April 30, 2013 3:20 AM
To: Andreas Färber
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
My assumption was that if new socket values are added in future, they will
likely be the same for all platforms, so they can be supported without
adding new lines of code. Here we convert the values known to be different,
we leave the values known to be the same, and for the incorrect values - we
pass them to (host) kernel as they are in belief that kernel would return
error. We could handle that part and check for incorrect value before
passing it to the kernel, but in that case we bring more kernel logic to
QEMU for the cases that are not likely to happen.
Petar
________________________________________
From: Andreas Färber [afaerber@suse.de]
Sent: Monday, April 29, 2013 4:56 PM
To: Petar Jovanovic
Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
Subject: Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
Am 23.04.2013 02:15, schrieb Petar Jovanovic:
> Thanks Aurelien for your comments.
>
> What others think? Can we submit this version of the patch? Riku? Richard, Blue?
No objection but isn't that a frequent issue that mappings may need to
be extended from time to time? The way I've seen that handled is on a
case by case basis mapping from one known value to another, with
defaulting to whatever form of error reporting appropriate. Here it
seems that some cases were dropped and we are now defaulting to taking
the literal value where no difference is known. This may lead to silent
errors, whereas an abort as other extreme may prohibit use cases with no
value difference between host and target.
Andreas
> ________________________________________
> From: Aurelien Jarno [aurelien@aurel32.net]
> Sent: Monday, April 15, 2013 3:47 PM
> To: Petar Jovanovic
> Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
> Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
>> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>
>> Previous implementation has failed to take into account different value of
>> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
>> The same conversion has to be applied both for do_socket and do_socketpair,
>> so the code has been isolated in a static inline function.
>> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
>> these are MIPS, SPARC and ALPHA now.
>>
>> enum sock_type in linux-user/socket.h has been extended to include
>> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
>> The patch also includes necessary code style changes (tab to spaces) in the
>> header file in the MIPS #ifdef block touched by the change.
>>
>> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
>> ---
>> v2:
>>
>> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
>> - values for sock_type are defined for SPARC and ALPHA in socket.h
>>
>> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
>> linux-user/syscall.c | 45 +++++----
>> 2 files changed, 195 insertions(+), 98 deletions(-)
>>
>> diff --git a/linux-user/socket.h b/linux-user/socket.h
>> index 339cae5..d2b05dc 100644
>> --- a/linux-user/socket.h
>> +++ b/linux-user/socket.h
>> @@ -1,91 +1,104 @@
>>
>> #if defined(TARGET_MIPS)
>> - // MIPS special values for constants
>> -
>> - /*
>> - * For setsockopt(2)
>> - *
>> - * This defines are ABI conformant as far as Linux supports these ...
>> - */
>> - #define TARGET_SOL_SOCKET 0xffff
>> -
>> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> - SIGPIPE when they die. */
>> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> - broadcast messages. */
>> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> - socket to transmit pending data. */
>> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
>> - #if 0
>> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
>> - #endif
>> -
>> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> - #define TARGET_SO_ACCEPTCONN 0x1009
>> -
>> - /* linux-specific, might as well be the same as on i386 */
>> - #define TARGET_SO_NO_CHECK 11
>> - #define TARGET_SO_PRIORITY 12
>> - #define TARGET_SO_BSDCOMPAT 14
>> + /* MIPS special values for constants */
>> +
>> + /*
>> + * For setsockopt(2)
>> + *
>> + * This defines are ABI conformant as far as Linux supports these ...
>> + */
>> + #define TARGET_SOL_SOCKET 0xffff
>> +
>> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>> + SIGPIPE when they die. */
>> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>> + broadcast messages. */
>> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>> + * socket to transmit pending data.
>> + */
>> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
>> + */
>> + #if 0
>> + /* To add: Allow local address and port reuse. */
>> + #define TARGET_SO_REUSEPORT 0x0200
>> + #endif
>> +
>> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>> + #define TARGET_SO_ACCEPTCONN 0x1009
>>
>> - #define TARGET_SO_PASSCRED 17
>> - #define TARGET_SO_PEERCRED 18
>> + /* linux-specific, might as well be the same as on i386 */
>> + #define TARGET_SO_NO_CHECK 11
>> + #define TARGET_SO_PRIORITY 12
>> + #define TARGET_SO_BSDCOMPAT 14
>>
>> - /* Security levels - as per NRL IPv6 - don't actually do anything */
>> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>> + #define TARGET_SO_PASSCRED 17
>> + #define TARGET_SO_PEERCRED 18
>>
>> - #define TARGET_SO_BINDTODEVICE 25
>> + /* Security levels - as per NRL IPv6 - don't actually do anything */
>> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
>> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>
>> - /* Socket filtering */
>> - #define TARGET_SO_ATTACH_FILTER 26
>> - #define TARGET_SO_DETACH_FILTER 27
>> + #define TARGET_SO_BINDTODEVICE 25
>>
>> - #define TARGET_SO_PEERNAME 28
>> - #define TARGET_SO_TIMESTAMP 29
>> - #define SCM_TIMESTAMP SO_TIMESTAMP
>> -
>> - #define TARGET_SO_PEERSEC 30
>> - #define TARGET_SO_SNDBUFFORCE 31
>> - #define TARGET_SO_RCVBUFFORCE 33
>> -
>> - /** sock_type - Socket types
>> - *
>> - * Please notice that for binary compat reasons MIPS has to
>> - * override the enum sock_type in include/linux/net.h, so
>> - * we define ARCH_HAS_SOCKET_TYPES here.
>> - *
>> - * @SOCK_DGRAM - datagram (conn.less) socket
>> - * @SOCK_STREAM - stream (connection) socket
>> - * @SOCK_RAW - raw socket
>> - * @SOCK_RDM - reliably-delivered message
>> - * @SOCK_SEQPACKET - sequential packet socket
>> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> - * For writing rarp and other similar things on the user level.
>> - */
>> - enum sock_type {
>> - TARGET_SOCK_DGRAM = 1,
>> - TARGET_SOCK_STREAM = 2,
>> - TARGET_SOCK_RAW = 3,
>> - TARGET_SOCK_RDM = 4,
>> - TARGET_SOCK_SEQPACKET = 5,
>> - TARGET_SOCK_DCCP = 6,
>> - TARGET_SOCK_PACKET = 10,
>> - };
>> -
>> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
>> + /* Socket filtering */
>> + #define TARGET_SO_ATTACH_FILTER 26
>> + #define TARGET_SO_DETACH_FILTER 27
>> +
>> + #define TARGET_SO_PEERNAME 28
>> + #define TARGET_SO_TIMESTAMP 29
>> + #define SCM_TIMESTAMP SO_TIMESTAMP
>> +
>> + #define TARGET_SO_PEERSEC 30
>> + #define TARGET_SO_SNDBUFFORCE 31
>> + #define TARGET_SO_RCVBUFFORCE 33
>> +
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons MIPS has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_DGRAM = 1,
>> + TARGET_SOCK_STREAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 02000000,
>> + TARGET_SOCK_NONBLOCK = 0200,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>
>> #elif defined(TARGET_ALPHA)
>>
>> @@ -156,8 +169,81 @@
>> /* Instruct lower device to use last 4-bytes of skb data as FCS */
>> #define TARGET_SO_NOFCS 43
>>
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons ALPHA has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 010000000,
>> + TARGET_SOCK_NONBLOCK = 010000000000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> #else
>>
>> +#if defined(TARGET_SPARC)
>> + /** sock_type - Socket types
>> + *
>> + * Please notice that for binary compat reasons SPARC has to
>> + * override the enum sock_type in include/linux/net.h, so
>> + * we define ARCH_HAS_SOCKET_TYPES here.
>> + *
>> + * @SOCK_DGRAM - datagram (conn.less) socket
>> + * @SOCK_STREAM - stream (connection) socket
>> + * @SOCK_RAW - raw socket
>> + * @SOCK_RDM - reliably-delivered message
>> + * @SOCK_SEQPACKET - sequential packet socket
>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>> + * For writing rarp and other similar things on the user
>> + * level.
>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>> + */
>> +
>> + #define ARCH_HAS_SOCKET_TYPES 1
>> +
>> + enum sock_type {
>> + TARGET_SOCK_STREAM = 1,
>> + TARGET_SOCK_DGRAM = 2,
>> + TARGET_SOCK_RAW = 3,
>> + TARGET_SOCK_RDM = 4,
>> + TARGET_SOCK_SEQPACKET = 5,
>> + TARGET_SOCK_DCCP = 6,
>> + TARGET_SOCK_PACKET = 10,
>> + TARGET_SOCK_CLOEXEC = 020000000,
>> + TARGET_SOCK_NONBLOCK = 040000,
>> + };
>> +
>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>> +#endif
>> +
>> /* For setsockopt(2) */
>> #define TARGET_SOL_SOCKET 1
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index ee82a2d..5b87c9d 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>> free(vec);
>> }
>>
>> -/* do_socket() Must return target values and target errnos. */
>> -static abi_long do_socket(int domain, int type, int protocol)
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> +static inline void target_to_host_sock_type(int *type)
>> {
>> -#if defined(TARGET_MIPS)
>> - switch(type) {
>> + int host_type = 0;
>> + int target_type = *type;
>> +
>> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
>> case TARGET_SOCK_DGRAM:
>> - type = SOCK_DGRAM;
>> + host_type = SOCK_DGRAM;
>> break;
>> case TARGET_SOCK_STREAM:
>> - type = SOCK_STREAM;
>> - break;
>> - case TARGET_SOCK_RAW:
>> - type = SOCK_RAW;
>> + host_type = SOCK_STREAM;
>> break;
>> - case TARGET_SOCK_RDM:
>> - type = SOCK_RDM;
>> - break;
>> - case TARGET_SOCK_SEQPACKET:
>> - type = SOCK_SEQPACKET;
>> - break;
>> - case TARGET_SOCK_PACKET:
>> - type = SOCK_PACKET;
>> + default:
>> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
>> break;
>> }
>> + if (target_type & TARGET_SOCK_CLOEXEC) {
>> + host_type |= SOCK_CLOEXEC;
>> + }
>> + if (target_type & TARGET_SOCK_NONBLOCK) {
>> + host_type |= SOCK_NONBLOCK;
>> + }
>> + *type = host_type;
>
> The problem I see with this way of handling the flags is that if the
> kernel later adds a new flag value, QEMU will silently drop it. It means
> that if the glibc tries to see it will think the kernel (and here QEMU)
> supports it (as no error is returned), and will not use to the fallback
> code. For that each processed flag should probably be removed from
> target_type and it should be 0 at the end of the function.
>
> Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
> supported by the glibc, but silently dropped by QEMU.
>
> That said I agree it's purely hypothetical, as no new flags has been
> added recently or is planned to be added. We can also decide to fix that
> when a new flag is added. What the other people (the Cc:ed one) think?
>
>> +}
>> +#endif
>> +
>> +/* do_socket() Must return target values and target errnos. */
>> +static abi_long do_socket(int domain, int type, int protocol)
>> +{
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> #endif
>> if (domain == PF_NETLINK)
>> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
>> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
>> int tab[2];
>> abi_long ret;
>>
>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>> + target_to_host_sock_type(&type);
>> +#endif
>> ret = get_errno(socketpair(domain, type, protocol, tab));
>> if (!is_error(ret)) {
>> if (put_user_s32(tab[0], target_tab_addr)
>
> Otherwise the patch looks fine.
>
>
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> aurelien@aurel32.net http://www.aurel32.net
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-05-27 10:49 ` Petar Jovanovic
@ 2013-05-30 13:28 ` Riku Voipio
2013-05-30 13:32 ` Petar Jovanovic
0 siblings, 1 reply; 14+ messages in thread
From: Riku Voipio @ 2013-05-30 13:28 UTC (permalink / raw)
To: Petar Jovanovic
Cc: Alexander Graf, qemu-devel@nongnu.org, blauwirbel@gmail.com,
Petar Jovanovic, Andreas Färber, Aurelien Jarno,
rth@twiddle.net
Hi,
On 27 May 2013 13:49, Petar Jovanovic <Petar.Jovanovic@imgtec.com> wrote:
> Can anyone take a look at this and commit it if there are no other change requests?
I thought Aurelian had an issue with this patch, but it seems you explained your
position well. I'll get this included in my next pull.
Riku
> ________________________________________
> From: Petar Jovanovic
> Sent: Wednesday, May 08, 2013 1:16 AM
> To: riku.voipio@linaro.org; qemu-devel@nongnu.org
> Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; rth@twiddle.net; Alexander Graf; Andreas Färber
> Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> ping
>
> http://patchwork.ozlabs.org/patch/232770/
> ________________________________________
> From: Petar Jovanovic
> Sent: Tuesday, April 30, 2013 3:20 AM
> To: Andreas Färber
> Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
> Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> My assumption was that if new socket values are added in future, they will
> likely be the same for all platforms, so they can be supported without
> adding new lines of code. Here we convert the values known to be different,
> we leave the values known to be the same, and for the incorrect values - we
> pass them to (host) kernel as they are in belief that kernel would return
> error. We could handle that part and check for incorrect value before
> passing it to the kernel, but in that case we bring more kernel logic to
> QEMU for the cases that are not likely to happen.
>
> Petar
> ________________________________________
> From: Andreas Färber [afaerber@suse.de]
> Sent: Monday, April 29, 2013 4:56 PM
> To: Petar Jovanovic
> Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
> Subject: Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> Am 23.04.2013 02:15, schrieb Petar Jovanovic:
>> Thanks Aurelien for your comments.
>>
>> What others think? Can we submit this version of the patch? Riku? Richard, Blue?
>
> No objection but isn't that a frequent issue that mappings may need to
> be extended from time to time? The way I've seen that handled is on a
> case by case basis mapping from one known value to another, with
> defaulting to whatever form of error reporting appropriate. Here it
> seems that some cases were dropped and we are now defaulting to taking
> the literal value where no difference is known. This may lead to silent
> errors, whereas an abort as other extreme may prohibit use cases with no
> value difference between host and target.
>
> Andreas
>
>> ________________________________________
>> From: Aurelien Jarno [aurelien@aurel32.net]
>> Sent: Monday, April 15, 2013 3:47 PM
>> To: Petar Jovanovic
>> Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
>> Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>>
>> On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
>>> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>>
>>> Previous implementation has failed to take into account different value of
>>> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
>>> The same conversion has to be applied both for do_socket and do_socketpair,
>>> so the code has been isolated in a static inline function.
>>> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
>>> these are MIPS, SPARC and ALPHA now.
>>>
>>> enum sock_type in linux-user/socket.h has been extended to include
>>> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
>>> The patch also includes necessary code style changes (tab to spaces) in the
>>> header file in the MIPS #ifdef block touched by the change.
>>>
>>> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>> ---
>>> v2:
>>>
>>> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
>>> - values for sock_type are defined for SPARC and ALPHA in socket.h
>>>
>>> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
>>> linux-user/syscall.c | 45 +++++----
>>> 2 files changed, 195 insertions(+), 98 deletions(-)
>>>
>>> diff --git a/linux-user/socket.h b/linux-user/socket.h
>>> index 339cae5..d2b05dc 100644
>>> --- a/linux-user/socket.h
>>> +++ b/linux-user/socket.h
>>> @@ -1,91 +1,104 @@
>>>
>>> #if defined(TARGET_MIPS)
>>> - // MIPS special values for constants
>>> -
>>> - /*
>>> - * For setsockopt(2)
>>> - *
>>> - * This defines are ABI conformant as far as Linux supports these ...
>>> - */
>>> - #define TARGET_SOL_SOCKET 0xffff
>>> -
>>> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>>> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>>> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>>> - SIGPIPE when they die. */
>>> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>>> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>>> - broadcast messages. */
>>> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>>> - socket to transmit pending data. */
>>> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
>>> - #if 0
>>> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
>>> - #endif
>>> -
>>> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>>> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>>> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>>> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>>> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>>> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>>> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>>> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>>> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>>> - #define TARGET_SO_ACCEPTCONN 0x1009
>>> -
>>> - /* linux-specific, might as well be the same as on i386 */
>>> - #define TARGET_SO_NO_CHECK 11
>>> - #define TARGET_SO_PRIORITY 12
>>> - #define TARGET_SO_BSDCOMPAT 14
>>> + /* MIPS special values for constants */
>>> +
>>> + /*
>>> + * For setsockopt(2)
>>> + *
>>> + * This defines are ABI conformant as far as Linux supports these ...
>>> + */
>>> + #define TARGET_SOL_SOCKET 0xffff
>>> +
>>> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>>> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>>> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>>> + SIGPIPE when they die. */
>>> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>>> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>>> + broadcast messages. */
>>> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>>> + * socket to transmit pending data.
>>> + */
>>> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
>>> + */
>>> + #if 0
>>> + /* To add: Allow local address and port reuse. */
>>> + #define TARGET_SO_REUSEPORT 0x0200
>>> + #endif
>>> +
>>> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>>> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>>> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>>> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>>> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>>> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>>> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>>> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>>> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>>> + #define TARGET_SO_ACCEPTCONN 0x1009
>>>
>>> - #define TARGET_SO_PASSCRED 17
>>> - #define TARGET_SO_PEERCRED 18
>>> + /* linux-specific, might as well be the same as on i386 */
>>> + #define TARGET_SO_NO_CHECK 11
>>> + #define TARGET_SO_PRIORITY 12
>>> + #define TARGET_SO_BSDCOMPAT 14
>>>
>>> - /* Security levels - as per NRL IPv6 - don't actually do anything */
>>> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
>>> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>>> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>> + #define TARGET_SO_PASSCRED 17
>>> + #define TARGET_SO_PEERCRED 18
>>>
>>> - #define TARGET_SO_BINDTODEVICE 25
>>> + /* Security levels - as per NRL IPv6 - don't actually do anything */
>>> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
>>> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>>> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>>
>>> - /* Socket filtering */
>>> - #define TARGET_SO_ATTACH_FILTER 26
>>> - #define TARGET_SO_DETACH_FILTER 27
>>> + #define TARGET_SO_BINDTODEVICE 25
>>>
>>> - #define TARGET_SO_PEERNAME 28
>>> - #define TARGET_SO_TIMESTAMP 29
>>> - #define SCM_TIMESTAMP SO_TIMESTAMP
>>> -
>>> - #define TARGET_SO_PEERSEC 30
>>> - #define TARGET_SO_SNDBUFFORCE 31
>>> - #define TARGET_SO_RCVBUFFORCE 33
>>> -
>>> - /** sock_type - Socket types
>>> - *
>>> - * Please notice that for binary compat reasons MIPS has to
>>> - * override the enum sock_type in include/linux/net.h, so
>>> - * we define ARCH_HAS_SOCKET_TYPES here.
>>> - *
>>> - * @SOCK_DGRAM - datagram (conn.less) socket
>>> - * @SOCK_STREAM - stream (connection) socket
>>> - * @SOCK_RAW - raw socket
>>> - * @SOCK_RDM - reliably-delivered message
>>> - * @SOCK_SEQPACKET - sequential packet socket
>>> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>>> - * For writing rarp and other similar things on the user level.
>>> - */
>>> - enum sock_type {
>>> - TARGET_SOCK_DGRAM = 1,
>>> - TARGET_SOCK_STREAM = 2,
>>> - TARGET_SOCK_RAW = 3,
>>> - TARGET_SOCK_RDM = 4,
>>> - TARGET_SOCK_SEQPACKET = 5,
>>> - TARGET_SOCK_DCCP = 6,
>>> - TARGET_SOCK_PACKET = 10,
>>> - };
>>> -
>>> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
>>> + /* Socket filtering */
>>> + #define TARGET_SO_ATTACH_FILTER 26
>>> + #define TARGET_SO_DETACH_FILTER 27
>>> +
>>> + #define TARGET_SO_PEERNAME 28
>>> + #define TARGET_SO_TIMESTAMP 29
>>> + #define SCM_TIMESTAMP SO_TIMESTAMP
>>> +
>>> + #define TARGET_SO_PEERSEC 30
>>> + #define TARGET_SO_SNDBUFFORCE 31
>>> + #define TARGET_SO_RCVBUFFORCE 33
>>> +
>>> + /** sock_type - Socket types
>>> + *
>>> + * Please notice that for binary compat reasons MIPS has to
>>> + * override the enum sock_type in include/linux/net.h, so
>>> + * we define ARCH_HAS_SOCKET_TYPES here.
>>> + *
>>> + * @SOCK_DGRAM - datagram (conn.less) socket
>>> + * @SOCK_STREAM - stream (connection) socket
>>> + * @SOCK_RAW - raw socket
>>> + * @SOCK_RDM - reliably-delivered message
>>> + * @SOCK_SEQPACKET - sequential packet socket
>>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>>> + * For writing rarp and other similar things on the user
>>> + * level.
>>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>>> + */
>>> +
>>> + #define ARCH_HAS_SOCKET_TYPES 1
>>> +
>>> + enum sock_type {
>>> + TARGET_SOCK_DGRAM = 1,
>>> + TARGET_SOCK_STREAM = 2,
>>> + TARGET_SOCK_RAW = 3,
>>> + TARGET_SOCK_RDM = 4,
>>> + TARGET_SOCK_SEQPACKET = 5,
>>> + TARGET_SOCK_DCCP = 6,
>>> + TARGET_SOCK_PACKET = 10,
>>> + TARGET_SOCK_CLOEXEC = 02000000,
>>> + TARGET_SOCK_NONBLOCK = 0200,
>>> + };
>>> +
>>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>>
>>> #elif defined(TARGET_ALPHA)
>>>
>>> @@ -156,8 +169,81 @@
>>> /* Instruct lower device to use last 4-bytes of skb data as FCS */
>>> #define TARGET_SO_NOFCS 43
>>>
>>> + /** sock_type - Socket types
>>> + *
>>> + * Please notice that for binary compat reasons ALPHA has to
>>> + * override the enum sock_type in include/linux/net.h, so
>>> + * we define ARCH_HAS_SOCKET_TYPES here.
>>> + *
>>> + * @SOCK_DGRAM - datagram (conn.less) socket
>>> + * @SOCK_STREAM - stream (connection) socket
>>> + * @SOCK_RAW - raw socket
>>> + * @SOCK_RDM - reliably-delivered message
>>> + * @SOCK_SEQPACKET - sequential packet socket
>>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>>> + * For writing rarp and other similar things on the user
>>> + * level.
>>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>>> + */
>>> +
>>> + #define ARCH_HAS_SOCKET_TYPES 1
>>> +
>>> + enum sock_type {
>>> + TARGET_SOCK_STREAM = 1,
>>> + TARGET_SOCK_DGRAM = 2,
>>> + TARGET_SOCK_RAW = 3,
>>> + TARGET_SOCK_RDM = 4,
>>> + TARGET_SOCK_SEQPACKET = 5,
>>> + TARGET_SOCK_DCCP = 6,
>>> + TARGET_SOCK_PACKET = 10,
>>> + TARGET_SOCK_CLOEXEC = 010000000,
>>> + TARGET_SOCK_NONBLOCK = 010000000000,
>>> + };
>>> +
>>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>> #else
>>>
>>> +#if defined(TARGET_SPARC)
>>> + /** sock_type - Socket types
>>> + *
>>> + * Please notice that for binary compat reasons SPARC has to
>>> + * override the enum sock_type in include/linux/net.h, so
>>> + * we define ARCH_HAS_SOCKET_TYPES here.
>>> + *
>>> + * @SOCK_DGRAM - datagram (conn.less) socket
>>> + * @SOCK_STREAM - stream (connection) socket
>>> + * @SOCK_RAW - raw socket
>>> + * @SOCK_RDM - reliably-delivered message
>>> + * @SOCK_SEQPACKET - sequential packet socket
>>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>>> + * For writing rarp and other similar things on the user
>>> + * level.
>>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>>> + */
>>> +
>>> + #define ARCH_HAS_SOCKET_TYPES 1
>>> +
>>> + enum sock_type {
>>> + TARGET_SOCK_STREAM = 1,
>>> + TARGET_SOCK_DGRAM = 2,
>>> + TARGET_SOCK_RAW = 3,
>>> + TARGET_SOCK_RDM = 4,
>>> + TARGET_SOCK_SEQPACKET = 5,
>>> + TARGET_SOCK_DCCP = 6,
>>> + TARGET_SOCK_PACKET = 10,
>>> + TARGET_SOCK_CLOEXEC = 020000000,
>>> + TARGET_SOCK_NONBLOCK = 040000,
>>> + };
>>> +
>>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>> +#endif
>>> +
>>> /* For setsockopt(2) */
>>> #define TARGET_SOL_SOCKET 1
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index ee82a2d..5b87c9d 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>>> free(vec);
>>> }
>>>
>>> -/* do_socket() Must return target values and target errnos. */
>>> -static abi_long do_socket(int domain, int type, int protocol)
>>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>>> +static inline void target_to_host_sock_type(int *type)
>>> {
>>> -#if defined(TARGET_MIPS)
>>> - switch(type) {
>>> + int host_type = 0;
>>> + int target_type = *type;
>>> +
>>> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
>>> case TARGET_SOCK_DGRAM:
>>> - type = SOCK_DGRAM;
>>> + host_type = SOCK_DGRAM;
>>> break;
>>> case TARGET_SOCK_STREAM:
>>> - type = SOCK_STREAM;
>>> - break;
>>> - case TARGET_SOCK_RAW:
>>> - type = SOCK_RAW;
>>> + host_type = SOCK_STREAM;
>>> break;
>>> - case TARGET_SOCK_RDM:
>>> - type = SOCK_RDM;
>>> - break;
>>> - case TARGET_SOCK_SEQPACKET:
>>> - type = SOCK_SEQPACKET;
>>> - break;
>>> - case TARGET_SOCK_PACKET:
>>> - type = SOCK_PACKET;
>>> + default:
>>> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
>>> break;
>>> }
>>> + if (target_type & TARGET_SOCK_CLOEXEC) {
>>> + host_type |= SOCK_CLOEXEC;
>>> + }
>>> + if (target_type & TARGET_SOCK_NONBLOCK) {
>>> + host_type |= SOCK_NONBLOCK;
>>> + }
>>> + *type = host_type;
>>
>> The problem I see with this way of handling the flags is that if the
>> kernel later adds a new flag value, QEMU will silently drop it. It means
>> that if the glibc tries to see it will think the kernel (and here QEMU)
>> supports it (as no error is returned), and will not use to the fallback
>> code. For that each processed flag should probably be removed from
>> target_type and it should be 0 at the end of the function.
>>
>> Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
>> supported by the glibc, but silently dropped by QEMU.
>>
>> That said I agree it's purely hypothetical, as no new flags has been
>> added recently or is planned to be added. We can also decide to fix that
>> when a new flag is added. What the other people (the Cc:ed one) think?
>>
>>> +}
>>> +#endif
>>> +
>>> +/* do_socket() Must return target values and target errnos. */
>>> +static abi_long do_socket(int domain, int type, int protocol)
>>> +{
>>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>>> + target_to_host_sock_type(&type);
>>> #endif
>>> if (domain == PF_NETLINK)
>>> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
>>> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
>>> int tab[2];
>>> abi_long ret;
>>>
>>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>>> + target_to_host_sock_type(&type);
>>> +#endif
>>> ret = get_errno(socketpair(domain, type, protocol, tab));
>>> if (!is_error(ret)) {
>>> if (put_user_s32(tab[0], target_tab_addr)
>>
>> Otherwise the patch looks fine.
>>
>>
>> --
>> Aurelien Jarno GPG: 1024D/F1BCDB73
>> aurelien@aurel32.net http://www.aurel32.net
>>
>>
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-05-30 13:28 ` Riku Voipio
@ 2013-05-30 13:32 ` Petar Jovanovic
0 siblings, 0 replies; 14+ messages in thread
From: Petar Jovanovic @ 2013-05-30 13:32 UTC (permalink / raw)
To: Riku Voipio
Cc: Alexander Graf, qemu-devel@nongnu.org, blauwirbel@gmail.com,
Petar Jovanovic, Andreas Färber, Aurelien Jarno,
rth@twiddle.net
Thanks.
Petar
________________________________________
From: Riku Voipio [riku.voipio@linaro.org]
Sent: Thursday, May 30, 2013 3:28 PM
To: Petar Jovanovic
Cc: qemu-devel@nongnu.org; Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; rth@twiddle.net; Alexander Graf; Andreas Färber
Subject: Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
Hi,
On 27 May 2013 13:49, Petar Jovanovic <Petar.Jovanovic@imgtec.com> wrote:
> Can anyone take a look at this and commit it if there are no other change requests?
I thought Aurelian had an issue with this patch, but it seems you explained your
position well. I'll get this included in my next pull.
Riku
> ________________________________________
> From: Petar Jovanovic
> Sent: Wednesday, May 08, 2013 1:16 AM
> To: riku.voipio@linaro.org; qemu-devel@nongnu.org
> Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; rth@twiddle.net; Alexander Graf; Andreas Färber
> Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> ping
>
> http://patchwork.ozlabs.org/patch/232770/
> ________________________________________
> From: Petar Jovanovic
> Sent: Tuesday, April 30, 2013 3:20 AM
> To: Andreas Färber
> Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
> Subject: RE: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> My assumption was that if new socket values are added in future, they will
> likely be the same for all platforms, so they can be supported without
> adding new lines of code. Here we convert the values known to be different,
> we leave the values known to be the same, and for the incorrect values - we
> pass them to (host) kernel as they are in belief that kernel would return
> error. We could handle that part and check for incorrect value before
> passing it to the kernel, but in that case we bring more kernel logic to
> QEMU for the cases that are not likely to happen.
>
> Petar
> ________________________________________
> From: Andreas Färber [afaerber@suse.de]
> Sent: Monday, April 29, 2013 4:56 PM
> To: Petar Jovanovic
> Cc: Aurelien Jarno; Petar Jovanovic; blauwirbel@gmail.com; riku.voipio@linaro.org; qemu-devel@nongnu.org; rth@twiddle.net; Alexander Graf
> Subject: Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>
> Am 23.04.2013 02:15, schrieb Petar Jovanovic:
>> Thanks Aurelien for your comments.
>>
>> What others think? Can we submit this version of the patch? Riku? Richard, Blue?
>
> No objection but isn't that a frequent issue that mappings may need to
> be extended from time to time? The way I've seen that handled is on a
> case by case basis mapping from one known value to another, with
> defaulting to whatever form of error reporting appropriate. Here it
> seems that some cases were dropped and we are now defaulting to taking
> the literal value where no difference is known. This may lead to silent
> errors, whereas an abort as other extreme may prohibit use cases with no
> value difference between host and target.
>
> Andreas
>
>> ________________________________________
>> From: Aurelien Jarno [aurelien@aurel32.net]
>> Sent: Monday, April 15, 2013 3:47 PM
>> To: Petar Jovanovic
>> Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; rth@twiddle.net; blauwirbel@gmail.com
>> Subject: Re: [PATCH v2] linux-user: improve target_to_host_sock_type conversion
>>
>> On Mon, Apr 01, 2013 at 05:49:39PM +0200, Petar Jovanovic wrote:
>>> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>>
>>> Previous implementation has failed to take into account different value of
>>> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
>>> The same conversion has to be applied both for do_socket and do_socketpair,
>>> so the code has been isolated in a static inline function.
>>> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
>>> these are MIPS, SPARC and ALPHA now.
>>>
>>> enum sock_type in linux-user/socket.h has been extended to include
>>> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
>>> The patch also includes necessary code style changes (tab to spaces) in the
>>> header file in the MIPS #ifdef block touched by the change.
>>>
>>> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
>>> ---
>>> v2:
>>>
>>> - the patch defines ARCH_HAS_SOCKET_TYPES for MIPS, SPARC and ALPHA
>>> - values for sock_type are defined for SPARC and ALPHA in socket.h
>>>
>>> linux-user/socket.h | 248 +++++++++++++++++++++++++++++++++-----------------
>>> linux-user/syscall.c | 45 +++++----
>>> 2 files changed, 195 insertions(+), 98 deletions(-)
>>>
>>> diff --git a/linux-user/socket.h b/linux-user/socket.h
>>> index 339cae5..d2b05dc 100644
>>> --- a/linux-user/socket.h
>>> +++ b/linux-user/socket.h
>>> @@ -1,91 +1,104 @@
>>>
>>> #if defined(TARGET_MIPS)
>>> - // MIPS special values for constants
>>> -
>>> - /*
>>> - * For setsockopt(2)
>>> - *
>>> - * This defines are ABI conformant as far as Linux supports these ...
>>> - */
>>> - #define TARGET_SOL_SOCKET 0xffff
>>> -
>>> - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>>> - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>>> - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>>> - SIGPIPE when they die. */
>>> - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>>> - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>>> - broadcast messages. */
>>> - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>>> - socket to transmit pending data. */
>>> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */
>>> - #if 0
>>> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */
>>> - #endif
>>> -
>>> - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>>> - #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>>> - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>>> - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>>> - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>>> - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>>> - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>>> - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>>> - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>>> - #define TARGET_SO_ACCEPTCONN 0x1009
>>> -
>>> - /* linux-specific, might as well be the same as on i386 */
>>> - #define TARGET_SO_NO_CHECK 11
>>> - #define TARGET_SO_PRIORITY 12
>>> - #define TARGET_SO_BSDCOMPAT 14
>>> + /* MIPS special values for constants */
>>> +
>>> + /*
>>> + * For setsockopt(2)
>>> + *
>>> + * This defines are ABI conformant as far as Linux supports these ...
>>> + */
>>> + #define TARGET_SOL_SOCKET 0xffff
>>> +
>>> + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */
>>> + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */
>>> + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send
>>> + SIGPIPE when they die. */
>>> + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */
>>> + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of
>>> + broadcast messages. */
>>> + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable
>>> + * socket to transmit pending data.
>>> + */
>>> + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band.
>>> + */
>>> + #if 0
>>> + /* To add: Allow local address and port reuse. */
>>> + #define TARGET_SO_REUSEPORT 0x0200
>>> + #endif
>>> +
>>> + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */
>>> + #define TARGET_SO_STYLE SO_TYPE /* Synonym */
>>> + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */
>>> + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */
>>> + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */
>>> + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */
>>> + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */
>>> + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */
>>> + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */
>>> + #define TARGET_SO_ACCEPTCONN 0x1009
>>>
>>> - #define TARGET_SO_PASSCRED 17
>>> - #define TARGET_SO_PEERCRED 18
>>> + /* linux-specific, might as well be the same as on i386 */
>>> + #define TARGET_SO_NO_CHECK 11
>>> + #define TARGET_SO_PRIORITY 12
>>> + #define TARGET_SO_BSDCOMPAT 14
>>>
>>> - /* Security levels - as per NRL IPv6 - don't actually do anything */
>>> - #define TARGET_SO_SECURITY_AUTHENTICATION 22
>>> - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>>> - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>> + #define TARGET_SO_PASSCRED 17
>>> + #define TARGET_SO_PEERCRED 18
>>>
>>> - #define TARGET_SO_BINDTODEVICE 25
>>> + /* Security levels - as per NRL IPv6 - don't actually do anything */
>>> + #define TARGET_SO_SECURITY_AUTHENTICATION 22
>>> + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
>>> + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24
>>>
>>> - /* Socket filtering */
>>> - #define TARGET_SO_ATTACH_FILTER 26
>>> - #define TARGET_SO_DETACH_FILTER 27
>>> + #define TARGET_SO_BINDTODEVICE 25
>>>
>>> - #define TARGET_SO_PEERNAME 28
>>> - #define TARGET_SO_TIMESTAMP 29
>>> - #define SCM_TIMESTAMP SO_TIMESTAMP
>>> -
>>> - #define TARGET_SO_PEERSEC 30
>>> - #define TARGET_SO_SNDBUFFORCE 31
>>> - #define TARGET_SO_RCVBUFFORCE 33
>>> -
>>> - /** sock_type - Socket types
>>> - *
>>> - * Please notice that for binary compat reasons MIPS has to
>>> - * override the enum sock_type in include/linux/net.h, so
>>> - * we define ARCH_HAS_SOCKET_TYPES here.
>>> - *
>>> - * @SOCK_DGRAM - datagram (conn.less) socket
>>> - * @SOCK_STREAM - stream (connection) socket
>>> - * @SOCK_RAW - raw socket
>>> - * @SOCK_RDM - reliably-delivered message
>>> - * @SOCK_SEQPACKET - sequential packet socket
>>> - * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>>> - * For writing rarp and other similar things on the user level.
>>> - */
>>> - enum sock_type {
>>> - TARGET_SOCK_DGRAM = 1,
>>> - TARGET_SOCK_STREAM = 2,
>>> - TARGET_SOCK_RAW = 3,
>>> - TARGET_SOCK_RDM = 4,
>>> - TARGET_SOCK_SEQPACKET = 5,
>>> - TARGET_SOCK_DCCP = 6,
>>> - TARGET_SOCK_PACKET = 10,
>>> - };
>>> -
>>> - #define TARGET_SOCK_MAX (SOCK_PACKET + 1)
>>> + /* Socket filtering */
>>> + #define TARGET_SO_ATTACH_FILTER 26
>>> + #define TARGET_SO_DETACH_FILTER 27
>>> +
>>> + #define TARGET_SO_PEERNAME 28
>>> + #define TARGET_SO_TIMESTAMP 29
>>> + #define SCM_TIMESTAMP SO_TIMESTAMP
>>> +
>>> + #define TARGET_SO_PEERSEC 30
>>> + #define TARGET_SO_SNDBUFFORCE 31
>>> + #define TARGET_SO_RCVBUFFORCE 33
>>> +
>>> + /** sock_type - Socket types
>>> + *
>>> + * Please notice that for binary compat reasons MIPS has to
>>> + * override the enum sock_type in include/linux/net.h, so
>>> + * we define ARCH_HAS_SOCKET_TYPES here.
>>> + *
>>> + * @SOCK_DGRAM - datagram (conn.less) socket
>>> + * @SOCK_STREAM - stream (connection) socket
>>> + * @SOCK_RAW - raw socket
>>> + * @SOCK_RDM - reliably-delivered message
>>> + * @SOCK_SEQPACKET - sequential packet socket
>>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>>> + * For writing rarp and other similar things on the user
>>> + * level.
>>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>>> + */
>>> +
>>> + #define ARCH_HAS_SOCKET_TYPES 1
>>> +
>>> + enum sock_type {
>>> + TARGET_SOCK_DGRAM = 1,
>>> + TARGET_SOCK_STREAM = 2,
>>> + TARGET_SOCK_RAW = 3,
>>> + TARGET_SOCK_RDM = 4,
>>> + TARGET_SOCK_SEQPACKET = 5,
>>> + TARGET_SOCK_DCCP = 6,
>>> + TARGET_SOCK_PACKET = 10,
>>> + TARGET_SOCK_CLOEXEC = 02000000,
>>> + TARGET_SOCK_NONBLOCK = 0200,
>>> + };
>>> +
>>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>>
>>> #elif defined(TARGET_ALPHA)
>>>
>>> @@ -156,8 +169,81 @@
>>> /* Instruct lower device to use last 4-bytes of skb data as FCS */
>>> #define TARGET_SO_NOFCS 43
>>>
>>> + /** sock_type - Socket types
>>> + *
>>> + * Please notice that for binary compat reasons ALPHA has to
>>> + * override the enum sock_type in include/linux/net.h, so
>>> + * we define ARCH_HAS_SOCKET_TYPES here.
>>> + *
>>> + * @SOCK_DGRAM - datagram (conn.less) socket
>>> + * @SOCK_STREAM - stream (connection) socket
>>> + * @SOCK_RAW - raw socket
>>> + * @SOCK_RDM - reliably-delivered message
>>> + * @SOCK_SEQPACKET - sequential packet socket
>>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>>> + * For writing rarp and other similar things on the user
>>> + * level.
>>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>>> + */
>>> +
>>> + #define ARCH_HAS_SOCKET_TYPES 1
>>> +
>>> + enum sock_type {
>>> + TARGET_SOCK_STREAM = 1,
>>> + TARGET_SOCK_DGRAM = 2,
>>> + TARGET_SOCK_RAW = 3,
>>> + TARGET_SOCK_RDM = 4,
>>> + TARGET_SOCK_SEQPACKET = 5,
>>> + TARGET_SOCK_DCCP = 6,
>>> + TARGET_SOCK_PACKET = 10,
>>> + TARGET_SOCK_CLOEXEC = 010000000,
>>> + TARGET_SOCK_NONBLOCK = 010000000000,
>>> + };
>>> +
>>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>> #else
>>>
>>> +#if defined(TARGET_SPARC)
>>> + /** sock_type - Socket types
>>> + *
>>> + * Please notice that for binary compat reasons SPARC has to
>>> + * override the enum sock_type in include/linux/net.h, so
>>> + * we define ARCH_HAS_SOCKET_TYPES here.
>>> + *
>>> + * @SOCK_DGRAM - datagram (conn.less) socket
>>> + * @SOCK_STREAM - stream (connection) socket
>>> + * @SOCK_RAW - raw socket
>>> + * @SOCK_RDM - reliably-delivered message
>>> + * @SOCK_SEQPACKET - sequential packet socket
>>> + * @SOCK_DCCP - Datagram Congestion Control Protocol socket
>>> + * @SOCK_PACKET - linux specific way of getting packets at the dev level.
>>> + * For writing rarp and other similar things on the user
>>> + * level.
>>> + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
>>> + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
>>> + */
>>> +
>>> + #define ARCH_HAS_SOCKET_TYPES 1
>>> +
>>> + enum sock_type {
>>> + TARGET_SOCK_STREAM = 1,
>>> + TARGET_SOCK_DGRAM = 2,
>>> + TARGET_SOCK_RAW = 3,
>>> + TARGET_SOCK_RDM = 4,
>>> + TARGET_SOCK_SEQPACKET = 5,
>>> + TARGET_SOCK_DCCP = 6,
>>> + TARGET_SOCK_PACKET = 10,
>>> + TARGET_SOCK_CLOEXEC = 020000000,
>>> + TARGET_SOCK_NONBLOCK = 040000,
>>> + };
>>> +
>>> + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
>>> + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */
>>> +#endif
>>> +
>>> /* For setsockopt(2) */
>>> #define TARGET_SOL_SOCKET 1
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index ee82a2d..5b87c9d 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>>> free(vec);
>>> }
>>>
>>> -/* do_socket() Must return target values and target errnos. */
>>> -static abi_long do_socket(int domain, int type, int protocol)
>>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>>> +static inline void target_to_host_sock_type(int *type)
>>> {
>>> -#if defined(TARGET_MIPS)
>>> - switch(type) {
>>> + int host_type = 0;
>>> + int target_type = *type;
>>> +
>>> + switch (target_type & TARGET_SOCK_TYPE_MASK) {
>>> case TARGET_SOCK_DGRAM:
>>> - type = SOCK_DGRAM;
>>> + host_type = SOCK_DGRAM;
>>> break;
>>> case TARGET_SOCK_STREAM:
>>> - type = SOCK_STREAM;
>>> - break;
>>> - case TARGET_SOCK_RAW:
>>> - type = SOCK_RAW;
>>> + host_type = SOCK_STREAM;
>>> break;
>>> - case TARGET_SOCK_RDM:
>>> - type = SOCK_RDM;
>>> - break;
>>> - case TARGET_SOCK_SEQPACKET:
>>> - type = SOCK_SEQPACKET;
>>> - break;
>>> - case TARGET_SOCK_PACKET:
>>> - type = SOCK_PACKET;
>>> + default:
>>> + host_type = target_type & TARGET_SOCK_TYPE_MASK;
>>> break;
>>> }
>>> + if (target_type & TARGET_SOCK_CLOEXEC) {
>>> + host_type |= SOCK_CLOEXEC;
>>> + }
>>> + if (target_type & TARGET_SOCK_NONBLOCK) {
>>> + host_type |= SOCK_NONBLOCK;
>>> + }
>>> + *type = host_type;
>>
>> The problem I see with this way of handling the flags is that if the
>> kernel later adds a new flag value, QEMU will silently drop it. It means
>> that if the glibc tries to see it will think the kernel (and here QEMU)
>> supports it (as no error is returned), and will not use to the fallback
>> code. For that each processed flag should probably be removed from
>> target_type and it should be 0 at the end of the function.
>>
>> Imagine for example SOCK_NONBLOCK is a new feature, it starts to be
>> supported by the glibc, but silently dropped by QEMU.
>>
>> That said I agree it's purely hypothetical, as no new flags has been
>> added recently or is planned to be added. We can also decide to fix that
>> when a new flag is added. What the other people (the Cc:ed one) think?
>>
>>> +}
>>> +#endif
>>> +
>>> +/* do_socket() Must return target values and target errnos. */
>>> +static abi_long do_socket(int domain, int type, int protocol)
>>> +{
>>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>>> + target_to_host_sock_type(&type);
>>> #endif
>>> if (domain == PF_NETLINK)
>>> return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
>>> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
>>> int tab[2];
>>> abi_long ret;
>>>
>>> +#if defined(ARCH_HAS_SOCKET_TYPES)
>>> + target_to_host_sock_type(&type);
>>> +#endif
>>> ret = get_errno(socketpair(domain, type, protocol, tab));
>>> if (!is_error(ret)) {
>>> if (put_user_s32(tab[0], target_tab_addr)
>>
>> Otherwise the patch looks fine.
>>
>>
>> --
>> Aurelien Jarno GPG: 1024D/F1BCDB73
>> aurelien@aurel32.net http://www.aurel32.net
>>
>>
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-04-01 15:49 [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion Petar Jovanovic
2013-04-10 18:17 ` Petar Jovanovic
2013-04-15 13:47 ` Aurelien Jarno
@ 2013-06-27 17:21 ` Peter Maydell
2013-07-01 0:48 ` Petar Jovanovic
2 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2013-06-27 17:21 UTC (permalink / raw)
To: Petar Jovanovic
Cc: riku.voipio, qemu-devel, petar.jovanovic, blauwirbel, aurelien,
rth
On 1 April 2013 16:49, Petar Jovanovic <petar.jovanovic@rt-rk.com> wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>
> Previous implementation has failed to take into account different value of
> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
> The same conversion has to be applied both for do_socket and do_socketpair,
> so the code has been isolated in a static inline function.
> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
> these are MIPS, SPARC and ALPHA now.
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> +static inline void target_to_host_sock_type(int *type)
> {
[etc]
I apologise for the hugely late review on this patch, but
this looks odd. Whether we need to translate SOCK_* constants
doesn't just depend on the target architecture, but also on
the host. (Consider running QEMU on MIPS and emulating x86.)
I think that we need to make sure that we define TARGET_SOCK_*
for all architectures (ie with a bit in the common section
that goes #ifndef ARCH_HAS_SOCKET_TYPES [default stuff]),
and then unconditionally define and call target_to_host_sock_type
in syscall.c.
thanks
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
2013-06-27 17:21 ` Peter Maydell
@ 2013-07-01 0:48 ` Petar Jovanovic
0 siblings, 0 replies; 14+ messages in thread
From: Petar Jovanovic @ 2013-07-01 0:48 UTC (permalink / raw)
To: Peter Maydell, Petar Jovanovic
Cc: blauwirbel@gmail.com, riku.voipio@linaro.org,
qemu-devel@nongnu.org, aurelien@aurel32.net, rth@twiddle.net
________________________________________
From: Peter Maydell [peter.maydell@linaro.org]
Sent: Thursday, June 27, 2013 7:21 PM
To: Petar Jovanovic
Cc: qemu-devel@nongnu.org; blauwirbel@gmail.com; riku.voipio@linaro.org; Petar Jovanovic; aurelien@aurel32.net; rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion
On 1 April 2013 16:49, Petar Jovanovic <petar.jovanovic@rt-rk.com> wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
>
> Previous implementation has failed to take into account different value of
> SOCK_NONBLOCK on target and host, and existence of SOCK_CLOEXEC.
> The same conversion has to be applied both for do_socket and do_socketpair,
> so the code has been isolated in a static inline function.
> The change is valid for architectures that define ARCH_HAS_SOCKET_TYPES,
> these are MIPS, SPARC and ALPHA now.
> +#if defined(ARCH_HAS_SOCKET_TYPES)
> +static inline void target_to_host_sock_type(int *type)
> {
> [etc]
> I apologise for the hugely late review on this patch, but
> this looks odd. Whether we need to translate SOCK_* constants
> doesn't just depend on the target architecture, but also on
> the host. (Consider running QEMU on MIPS and emulating x86.)
>
> I think that we need to make sure that we define TARGET_SOCK_*
> for all architectures (ie with a bit in the common section
> that goes #ifndef ARCH_HAS_SOCKET_TYPES [default stuff]),
> and then unconditionally define and call target_to_host_sock_type
> in syscall.c.
>
> thanks
> -- PMM
I have just resubmitted the patch with this addition included.
Check for [PATCH v3].
Thanks.
Petar
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-07-01 0:49 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-01 15:49 [Qemu-devel] [PATCH v2] linux-user: improve target_to_host_sock_type conversion Petar Jovanovic
2013-04-10 18:17 ` Petar Jovanovic
2013-04-15 13:47 ` Aurelien Jarno
2013-04-23 0:15 ` Petar Jovanovic
2013-04-29 13:14 ` Petar Jovanovic
2013-04-29 14:56 ` Andreas Färber
2013-04-30 1:20 ` Petar Jovanovic
2013-05-07 23:16 ` Petar Jovanovic
2013-05-22 13:41 ` Petar Jovanovic
2013-05-27 10:49 ` Petar Jovanovic
2013-05-30 13:28 ` Riku Voipio
2013-05-30 13:32 ` Petar Jovanovic
2013-06-27 17:21 ` Peter Maydell
2013-07-01 0:48 ` Petar Jovanovic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).