files: Rework send/recv-fds to be more generic

Submitted by Pavel Emelianov on Nov. 22, 2016, 5:03 p.m.

Details

Message ID 58347A7B.9090900@virtuozzo.com
State Accepted
Series "files: Rework send/recv-fds to be more generic"
Commit 4bb1d370db4ccbd6463205e4ea489b2392666ad9
Headers show

Commit Message

Pavel Emelianov Nov. 22, 2016, 5:03 p.m.
Remove getting opts from descriptors out from scm engine,
this stuff is pure criu thing, so make it collect the data.

The tricky change here is that parasite code needs memory
to keep fd_opts on. The memory is taken from parasite args
region, which is now bigger than it used to be. But that's
not a big deal, as previously this space was allocated on
the parasite stack (!, but with smaller chunks).

On the other hand, now we have one memcpy less, as opts are
put directly into the destination buffer.

Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
---
 compel/plugins/std/infect.c |  1 -
 compel/src/lib/infect.c     |  2 -
 criu/include/parasite.h     | 16 ++++++--
 criu/include/util-pie.h     |  2 -
 criu/parasite-syscall.c     |  2 +-
 criu/pie/parasite.c         | 60 +++++++++++++++++++++++++++++-
 include/common/scm-code.c   | 90 +++++++++++----------------------------------
 include/common/scm.h        | 32 +++-------------
 8 files changed, 98 insertions(+), 107 deletions(-)

Patch hide | download patch | download mbox

diff --git a/compel/plugins/std/infect.c b/compel/plugins/std/infect.c
index 4d06814..13bb006 100644
--- a/compel/plugins/std/infect.c
+++ b/compel/plugins/std/infect.c
@@ -1,7 +1,6 @@ 
 #include <compel/plugins/std/syscall.h>
 #include <compel/plugins/std/infect.h>
 
-#define SCM_FDSET_HAS_OPTS
 #include "common/scm.h"
 #include "uapi/compel/plugins/plugin-fds.h"
 #include "uapi/compel/plugins/std/string.h"
diff --git a/compel/src/lib/infect.c b/compel/src/lib/infect.c
index b88449e..699312f 100644
--- a/compel/src/lib/infect.c
+++ b/compel/src/lib/infect.c
@@ -32,8 +32,6 @@ 
 #define __sys(foo)	foo
 #define __memcpy	memcpy
 
-#define SCM_FDSET_HAS_OPTS
-
 #include "common/scm.h"
 #include "common/scm-code.c"
 
diff --git a/criu/include/parasite.h b/criu/include/parasite.h
index 21db35d..2763e74 100644
--- a/criu/include/parasite.h
+++ b/criu/include/parasite.h
@@ -193,13 +193,21 @@  struct parasite_drain_fd {
 	s32	fds[0];
 };
 
+struct fd_opts {
+	char flags;
+	struct {
+		uint32_t uid;
+		uint32_t euid;
+		uint32_t signum;
+		uint32_t pid_type;
+		uint32_t pid;
+	} fown;
+};
+
 static inline int drain_fds_size(struct parasite_drain_fd *dfds)
 {
 	int nr_fds = min((int)PARASITE_MAX_FDS, dfds->nr_fds);
-
-	BUILD_BUG_ON(sizeof(*dfds) + PARASITE_MAX_FDS * sizeof(dfds->fds[0]) > PAGE_SIZE);
-
-	return sizeof(dfds) + nr_fds * sizeof(dfds->fds[0]);
+	return sizeof(*dfds) + nr_fds * (sizeof(dfds->fds[0]) + sizeof(struct fd_opts));
 }
 
 struct parasite_tty_args {
diff --git a/criu/include/util-pie.h b/criu/include/util-pie.h
index 3c81849..31975dd 100644
--- a/criu/include/util-pie.h
+++ b/criu/include/util-pie.h
@@ -11,8 +11,6 @@ 
 #define SO_PEEK_OFF            42
 #endif
 
-#define SCM_FDSET_HAS_OPTS
-
 #include "common/scm.h"
 
 extern int open_detach_mount(char *dir);
diff --git a/criu/parasite-syscall.c b/criu/parasite-syscall.c
index 8984d70..305d3fe 100644
--- a/criu/parasite-syscall.c
+++ b/criu/parasite-syscall.c
@@ -471,7 +471,7 @@  int parasite_drain_fds_seized(struct parasite_ctl *ctl,
 	}
 
 	sk = compel_rpc_sock(ctl);
-	ret = recv_fds(sk, lfds, nr_fds, opts);
+	ret = recv_fds(sk, lfds, nr_fds, opts, sizeof(struct fd_opts));
 	if (ret)
 		pr_err("Can't retrieve FDs from socket\n");
 
diff --git a/criu/pie/parasite.c b/criu/pie/parasite.c
index 37934df..9c7f3fa 100644
--- a/criu/pie/parasite.c
+++ b/criu/pie/parasite.c
@@ -280,13 +280,71 @@  grps_err:
 	return -1;
 }
 
+static int fill_fds_opts(struct parasite_drain_fd *fds, struct fd_opts *opts)
+{
+	int i;
+
+	for (i = 0; i < fds->nr_fds; i++) {
+		int flags, fd = fds->fds[i], ret;
+		struct fd_opts *p = opts + i;
+		struct f_owner_ex owner_ex;
+		uint32_t v[2];
+
+		flags = sys_fcntl(fd, F_GETFD, 0);
+		if (flags < 0) {
+			pr_err("fcntl(%d, F_GETFD) -> %d\n", fd, flags);
+			return -1;
+		}
+
+		p->flags = (char)flags;
+
+		ret = sys_fcntl(fd, F_GETOWN_EX, (long)&owner_ex);
+		if (ret) {
+			pr_err("fcntl(%d, F_GETOWN_EX) -> %d\n", fd, ret);
+			return -1;
+		}
+
+		/*
+		 * Simple case -- nothing is changed.
+		 */
+		if (owner_ex.pid == 0) {
+			p->fown.pid = 0;
+			continue;
+		}
+
+		ret = sys_fcntl(fd, F_GETOWNER_UIDS, (long)&v);
+		if (ret) {
+			pr_err("fcntl(%d, F_GETOWNER_UIDS) -> %d\n", fd, ret);
+			return -1;
+		}
+
+		p->fown.uid	 = v[0];
+		p->fown.euid	 = v[1];
+		p->fown.pid_type = owner_ex.type;
+		p->fown.pid	 = owner_ex.pid;
+	}
+
+	return 0;
+}
+
 static int drain_fds(struct parasite_drain_fd *args)
 {
 	int ret, tsock;
+	struct fd_opts *opts;
+
+	/*
+	 * See the drain_fds_size() in criu code, the memory
+	 * for this args is ensured to be large enough to keep
+	 * an array of fd_opts at the tail.
+	 */
+	opts = ((void *)args) + sizeof(*args) + args->nr_fds * sizeof(args->fds[0]);
+	ret = fill_fds_opts(args, opts);
+	if (ret)
+		return ret;
 
 	tsock = parasite_get_rpc_sock();
 	ret = send_fds(tsock, NULL, 0,
-		       args->fds, args->nr_fds, true);
+		       args->fds, args->nr_fds, opts, sizeof(struct fd_opts));
 	if (ret)
 		pr_err("send_fds failed (%d)\n", ret);
 
diff --git a/include/common/scm-code.c b/include/common/scm-code.c
index 59a330a..504c972 100644
--- a/include/common/scm-code.c
+++ b/include/common/scm-code.c
@@ -6,24 +6,24 @@ 
 #error "The __memcpy macro is required"
 #endif
 
-#ifdef SCM_FDSET_HAS_OPTS
-#define OPTS_LEN(_flags, _nr)	(_flags ? sizeof(struct fd_opts) * (_nr) : 1)
-#define OPTS_BUF(_fdset)	((_fdset)->opts)
-#else
-#define OPTS_LEN(_flags, _nr)	(1)
-#define OPTS_BUF(_fdset)	(&(_fdset)->dummy)
-#endif
-
-static void scm_fdset_init_chunk(struct scm_fdset *fdset, int nr_fds, bool with_flags)
+static void scm_fdset_init_chunk(struct scm_fdset *fdset, int nr_fds,
+		void *data, unsigned ch_size)
 {
 	struct cmsghdr *cmsg;
+	static char dummy;
 
 	fdset->hdr.msg_controllen = CMSG_LEN(sizeof(int) * nr_fds);
 
 	cmsg		= CMSG_FIRSTHDR(&fdset->hdr);
 	cmsg->cmsg_len	= fdset->hdr.msg_controllen;
 
-	fdset->iov.iov_len = OPTS_LEN(with_flags, nr_fds);
+	if (data) {
+		fdset->iov.iov_base = data;
+		fdset->iov.iov_len = nr_fds * ch_size;
+	} else {
+		fdset->iov.iov_base = &dummy;
+		fdset->iov.iov_len = 1;
+	}
 }
 
 static int *scm_fdset_init(struct scm_fdset *fdset, struct sockaddr_un *saddr,
@@ -33,7 +33,7 @@  static int *scm_fdset_init(struct scm_fdset *fdset, struct sockaddr_un *saddr,
 
 	BUILD_BUG_ON(sizeof(fdset->msg_buf) < (CMSG_SPACE(sizeof(int) * CR_SCM_MAX_FD)));
 
-	fdset->iov.iov_base		= OPTS_BUF(fdset);
+	fdset->iov.iov_base		= (void *)0xdeadbeef;
 
 	fdset->hdr.msg_iov		= &fdset->iov;
 	fdset->hdr.msg_iovlen		= 1;
@@ -52,7 +52,7 @@  static int *scm_fdset_init(struct scm_fdset *fdset, struct sockaddr_un *saddr,
 }
 
 int send_fds(int sock, struct sockaddr_un *saddr, int len,
-		int *fds, int nr_fds, bool with_flags)
+		int *fds, int nr_fds, void *data, unsigned ch_size)
 {
 	struct scm_fdset fdset;
 	int *cmsg_data;
@@ -61,68 +61,21 @@  int send_fds(int sock, struct sockaddr_un *saddr, int len,
 	cmsg_data = scm_fdset_init(&fdset, saddr, len);
 	for (i = 0; i < nr_fds; i += min_fd) {
 		min_fd = min(CR_SCM_MAX_FD, nr_fds - i);
-		scm_fdset_init_chunk(&fdset, min_fd, with_flags);
+		scm_fdset_init_chunk(&fdset, min_fd, data, ch_size);
 		__memcpy(cmsg_data, &fds[i], sizeof(int) * min_fd);
 
-#ifdef SCM_FDSET_HAS_OPTS
-		if (with_flags) {
-			int j;
-
-			for (j = 0; j < min_fd; j++) {
-				int flags, fd = fds[i + j];
-				struct fd_opts *p = fdset.opts + j;
-				struct f_owner_ex owner_ex;
-				uint32_t v[2];
-
-				flags = __sys(fcntl)(fd, F_GETFD, 0);
-				if (flags < 0) {
-					pr_err("fcntl(%d, F_GETFD) -> %d\n", fd, flags);
-					return -1;
-				}
-
-				p->flags = (char)flags;
-
-				ret = __sys(fcntl)(fd, F_GETOWN_EX, (long)&owner_ex);
-				if (ret) {
-					pr_err("fcntl(%d, F_GETOWN_EX) -> %d\n", fd, ret);
-					return -1;
-				}
-
-				/*
-				 * Simple case -- nothing is changed.
-				 */
-				if (owner_ex.pid == 0) {
-					p->fown.pid = 0;
-					continue;
-				}
-
-				ret = __sys(fcntl)(fd, F_GETOWNER_UIDS, (long)&v);
-				if (ret) {
-					pr_err("fcntl(%d, F_GETOWNER_UIDS) -> %d\n", fd, ret);
-					return -1;
-				}
-
-				p->fown.uid	 = v[0];
-				p->fown.euid	 = v[1];
-				p->fown.pid_type = owner_ex.type;
-				p->fown.pid	 = owner_ex.pid;
-			}
-		}
-#endif
-
 		ret = __sys(sendmsg)(sock, &fdset.hdr, 0);
 		if (ret <= 0)
 			return ret ? : -1;
+
+		if (data)
+			data += min_fd * ch_size;
 	}
 
 	return 0;
 }
 
-#ifdef SCM_FDSET_HAS_OPTS
-int recv_fds(int sock, int *fds, int nr_fds, struct fd_opts *opts)
-#else
-int recv_fds(int sock, int *fds, int nr_fds, char *opts)
-#endif
+int recv_fds(int sock, int *fds, int nr_fds, void *data, unsigned ch_size)
 {
 	struct scm_fdset fdset;
 	struct cmsghdr *cmsg;
@@ -133,7 +86,7 @@  int recv_fds(int sock, int *fds, int nr_fds, char *opts)
 	cmsg_data = scm_fdset_init(&fdset, NULL, 0);
 	for (i = 0; i < nr_fds; i += min_fd) {
 		min_fd = min(CR_SCM_MAX_FD, nr_fds - i);
-		scm_fdset_init_chunk(&fdset, min_fd, opts != NULL);
+		scm_fdset_init_chunk(&fdset, min_fd, data, ch_size);
 
 		ret = __sys(recvmsg)(sock, &fdset.hdr, 0);
 		if (ret <= 0)
@@ -159,11 +112,10 @@  int recv_fds(int sock, int *fds, int nr_fds, char *opts)
 
 		if (unlikely(min_fd <= 0))
 			return -1;
+
 		__memcpy(&fds[i], cmsg_data, sizeof(int) * min_fd);
-#ifdef SCM_FDSET_HAS_OPTS
-		if (opts)
-			__memcpy(opts + i, fdset.opts, sizeof(struct fd_opts) * min_fd);
-#endif
+		if (data)
+			data += ch_size * min_fd;
 	}
 
 	return 0;
diff --git a/include/common/scm.h b/include/common/scm.h
index f7bc5e6..3874d1c 100644
--- a/include/common/scm.h
+++ b/include/common/scm.h
@@ -15,28 +15,10 @@ 
 #define CR_SCM_MSG_SIZE		(1024)
 #define CR_SCM_MAX_FD		(252)
 
-#ifdef SCM_FDSET_HAS_OPTS
-struct fd_opts {
-	char flags;
-	struct {
-		uint32_t uid;
-		uint32_t euid;
-		uint32_t signum;
-		uint32_t pid_type;
-		uint32_t pid;
-	} fown;
-};
-#endif
-
 struct scm_fdset {
 	struct msghdr	hdr;
 	struct iovec	iov;
 	char		msg_buf[CR_SCM_MSG_SIZE];
-#ifdef SCM_FDSET_HAS_OPTS
-	struct fd_opts	opts[CR_SCM_MAX_FD];
-#else
-	char		dummy;
-#endif
 };
 
 #ifndef F_GETOWNER_UIDS
@@ -44,24 +26,20 @@  struct scm_fdset {
 #endif
 
 extern int send_fds(int sock, struct sockaddr_un *saddr, int len,
-		    int *fds, int nr_fds, bool with_flags);
-
-#ifdef SCM_FDSET_HAS_OPTS
-extern int recv_fds(int sock, int *fds, int nr_fds, struct fd_opts *opts);
-#else
-extern int recv_fds(int sock, int *fds, int nr_fds, char *opts);
-#endif
+		int *fds, int nr_fds, void *data, unsigned ch_size);
+extern int recv_fds(int sock, int *fds, int nr_fds,
+		void *data, unsigned ch_size);
 
 static inline int send_fd(int sock, struct sockaddr_un *saddr, int saddr_len, int fd)
 {
-	return send_fds(sock, saddr, saddr_len, &fd, 1, false);
+	return send_fds(sock, saddr, saddr_len, &fd, 1, NULL, 0);
 }
 
 static inline int recv_fd(int sock)
 {
 	int fd, ret;
 
-	ret = recv_fds(sock, &fd, 1, NULL);
+	ret = recv_fds(sock, &fd, 1, NULL, 0);
 	if (ret)
 		return -1;