[PATCH v2 02/15] serialise: Split functions user for serialisation from util.c

[David Gibson <david@gibson.dropbear.id.au>]

The read_all_buf() and write_all_buf() functions in util.c are
primarily used for serialising data structures to a stream during
migraiton.  We're going to have further use for such serialisation
when we add dynamic configuration updates, where we'll want to share
the code with the client program.

To make that easier move the functions into a new serialisation.c
file, and rename thematically.  While we're there add some macros for
the common idiom of sending all of a given variable using sizeof().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile    | 11 ++++---
 flow.c      |  5 +--
 migrate.c   |  9 +++---
 pcap.c      |  3 +-
 serialise.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 serialise.h | 17 +++++++++++
 tcp.c       | 19 ++++++------
 util.c      | 78 +++--------------------------------------------
 util.h      |  2 --
 9 files changed, 136 insertions(+), 96 deletions(-)
 create mode 100644 serialise.c
 create mode 100644 serialise.h

diff --git a/Makefile b/Makefile
index 513dc6c6..5b6891d7 100644
--- a/Makefile
+++ b/Makefile
@@ -40,8 +40,8 @@ FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
 PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c \
 	flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c \
 	log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c \
-	pif.c repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c \
-	udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c
+	pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c \
+	udp.c udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c
 QRAP_SRCS = qrap.c
 PASST_REPAIR_SRCS = passt-repair.c
 SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS)
@@ -51,9 +51,10 @@ MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1
 PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h epoll_ctl.h \
 	flow.h fwd.h flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h \
 	isolation.h lineread.h log.h migrate.h ndp.h netlink.h packet.h \
-	passt.h pasta.h pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h \
-	tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h \
-	udp_internal.h udp_vu.h util.h vhost_user.h virtio.h vu_common.h
+	passt.h pasta.h pcap.h pif.h repair.h serialise.h siphash.h tap.h tcp.h \
+	tcp_buf.h tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h \
+	udp_flow.h udp_internal.h udp_vu.h util.h vhost_user.h virtio.h \
+	vu_common.h
 HEADERS = $(PASST_HEADERS) seccomp.h
 
 C := \#include <sys/random.h>\nint main(){int a=getrandom(0, 0, 0);}
diff --git a/flow.c b/flow.c
index 4282da2e..7d3c5ae6 100644
--- a/flow.c
+++ b/flow.c
@@ -21,6 +21,7 @@
 #include "flow_table.h"
 #include "repair.h"
 #include "epoll_ctl.h"
+#include "serialise.h"
 
 const char *flow_state_str[] = {
 	[FLOW_STATE_FREE]	= "FREE",
@@ -1135,7 +1136,7 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage,
 	}
 
 	count = htonl(count);
-	if (write_all_buf(fd, &count, sizeof(count))) {
+	if (sewrite_var(fd, &count)) {
 		rc = errno;
 		err_perror("Can't send flow count (%u)", ntohl(count));
 		return flow_migrate_source_rollback(c, FLOW_MAX, rc);
@@ -1220,7 +1221,7 @@ int flow_migrate_target(struct ctx *c, const struct migrate_stage *stage,
 
 	(void)stage;
 
-	if (read_all_buf(fd, &count, sizeof(count)))
+	if (seread_var(fd, &count))
 		return errno;
 
 	count = ntohl(count);
diff --git a/migrate.c b/migrate.c
index 1e8858a3..50f494b4 100644
--- a/migrate.c
+++ b/migrate.c
@@ -24,6 +24,7 @@
 
 #include "migrate.h"
 #include "repair.h"
+#include "serialise.h"
 
 /* Magic identifier for migration data */
 #define MIGRATE_MAGIC		0xB1BB1D1B0BB1D1B0
@@ -64,7 +65,7 @@ static int seen_addrs_source_v2(struct ctx *c,
 
 	memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac));
 
-	if (write_all_buf(fd, &addrs, sizeof(addrs)))
+	if (sewrite_var(fd, &addrs))
 		return errno;
 
 	return 0;
@@ -85,7 +86,7 @@ static int seen_addrs_target_v2(struct ctx *c,
 
 	(void)stage;
 
-	if (read_all_buf(fd, &addrs, sizeof(addrs)))
+	if (seread_var(fd, &addrs))
 		return errno;
 
 	c->ip6.addr_seen = addrs.addr6;
@@ -146,7 +147,7 @@ static int migrate_source(struct ctx *c, int fd)
 	const struct migrate_stage *s;
 	int ret;
 
-	if (write_all_buf(fd, &header, sizeof(header))) {
+	if (sewrite_var(fd, &header)) {
 		ret = errno;
 		err("Can't send migration header: %s, abort", strerror_(ret));
 		return ret;
@@ -180,7 +181,7 @@ static const struct migrate_version *migrate_target_read_header(int fd)
 	uint32_t id, compat_id;
 	unsigned i;
 
-	if (read_all_buf(fd, &h, sizeof(h)))
+	if (seread_var(fd, &h))
 		return NULL;
 
 	id = ntohl(h.version);
diff --git a/pcap.c b/pcap.c
index 54fba5c4..291112b6 100644
--- a/pcap.c
+++ b/pcap.c
@@ -34,6 +34,7 @@
 #include "pcap.h"
 #include "iov.h"
 #include "tap.h"
+#include "serialise.h"
 
 #define PCAP_VERSION_MINOR 4
 
@@ -64,7 +65,7 @@ static void pcap_frame(const struct iovec *iov, size_t iovcnt,
 		.len = l2len
 	};
 
-	if (write_all_buf(pcap_fd, &h, sizeof(h)) < 0 ||
+	if (sewrite_var(pcap_fd, &h) < 0 ||
 	    write_remainder(pcap_fd, iov, iovcnt, offset) < 0)
 		debug_perror("Cannot log packet, length %zu", l2len);
 }
diff --git a/serialise.c b/serialise.c
new file mode 100644
index 00000000..68054722
--- /dev/null
+++ b/serialise.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/* PASST - Plug A Simple Socket Transport
+ *  for qemu/UNIX domain socket mode
+ *
+ * PASTA - Pack A Subtle Tap Abstraction
+ *  for network namespace/tap device mode
+ *
+ * serialise.c - Serialisation of data structures over bytestreams
+ *
+ * Copyright Red Hat
+ * Author: David Gibson <david@gibson.dropbear.id.au>
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <unistd.h>
+
+#include "serialise.h"
+
+/**
+ * seread_buf() - Fill a whole buffer from a file descriptor
+ * @fd:		File descriptor
+ * @buf:	Pointer to base of buffer
+ * @len:	Length of buffer
+ *
+ * Return: 0 on success, -1 on error (with errno set)
+ *
+ * #syscalls read
+ */
+int seread_buf(int fd, void *buf, size_t len)
+{
+	size_t left = len;
+	char *p = buf;
+
+	while (left) {
+		ssize_t rc;
+
+		assert(left <= len);
+
+		do
+			rc = read(fd, p, left);
+		while ((rc < 0) && errno == EINTR);
+
+		if (rc < 0)
+			return -1;
+
+		if (rc == 0) {
+			errno = ENODATA;
+			return -1;
+		}
+
+		p += rc;
+		left -= rc;
+	}
+	return 0;
+}
+
+/**
+ * sewrite_buf() - write all of a buffer to an fd
+ * @fd:		File descriptor
+ * @buf:	Pointer to base of buffer
+ * @len:	Length of buffer
+ *
+ * Return: 0 on success, -1 on error (with errno set)
+ *
+ * #syscalls write
+ */
+int sewrite_buf(int fd, const void *buf, size_t len)
+{
+	const char *p = buf;
+	size_t left = len;
+
+	while (left) {
+		ssize_t rc;
+
+		do
+			rc = write(fd, p, left);
+		while ((rc < 0) && errno == EINTR);
+
+		if (rc < 0)
+			return -1;
+
+		p += rc;
+		left -= rc;
+	}
+	return 0;
+}
diff --git a/serialise.h b/serialise.h
new file mode 100644
index 00000000..ec95287f
--- /dev/null
+++ b/serialise.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright Red Hat
+ * Author: David Gibson <david@gibson.dropbear.id.au>
+ */
+
+#ifndef SERIALISE_H
+#define SERIALISE_H
+
+#include <stddef.h>
+
+int seread_buf(int fd, void *buf, size_t len);
+int sewrite_buf(int fd, const void *buf, size_t len);
+
+#define seread_var(fd_, var_)	(seread_buf((fd_), (var_), sizeof(*(var_))))
+#define sewrite_var(fd_, var_)	(sewrite_buf((fd_), (var_), sizeof(*(var_))))
+
+#endif /* _SERIALISE_H */
diff --git a/tcp.c b/tcp.c
index b1458624..8649d2db 100644
--- a/tcp.c
+++ b/tcp.c
@@ -308,6 +308,7 @@
 #include "flow.h"
 #include "repair.h"
 #include "linux_dep.h"
+#include "serialise.h"
 
 #include "flow_table.h"
 #include "tcp_internal.h"
@@ -3472,7 +3473,7 @@ int tcp_flow_migrate_source(int fd, struct tcp_tap_conn *conn)
 	memcpy(&t.pif, conn->f.pif, sizeof(t.pif));
 	memcpy(&t.side, conn->f.side, sizeof(t.side));
 
-	if (write_all_buf(fd, &t, sizeof(t))) {
+	if (sewrite_var(fd, &t)) {
 		int rc = -errno;
 		err_perror("Can't write migration data, socket %i", conn->sock);
 		return rc;
@@ -3570,17 +3571,17 @@ int tcp_flow_migrate_source_ext(const struct ctx *c,
 	t->rcv_wnd	= htonl(t->rcv_wnd);
 	t->rcv_wup	= htonl(t->rcv_wup);
 
-	if (write_all_buf(fd, t, sizeof(*t))) {
+	if (sewrite_var(fd, t)) {
 		flow_perror(conn, "Failed to write extended data");
 		return -EIO;
 	}
 
-	if (write_all_buf(fd, tcp_migrate_snd_queue, ntohl(t->sndq))) {
+	if (sewrite_buf(fd, tcp_migrate_snd_queue, ntohl(t->sndq))) {
 		flow_perror(conn, "Failed to write send queue data");
 		return -EIO;
 	}
 
-	if (write_all_buf(fd, tcp_migrate_rcv_queue, ntohl(t->rcvq))) {
+	if (sewrite_buf(fd, tcp_migrate_rcv_queue, ntohl(t->rcvq))) {
 		flow_perror(conn, "Failed to write receive queue data");
 		return -EIO;
 	}
@@ -3595,7 +3596,7 @@ fail:
 	 */
 	t->tcpi_state = 0; /* Not defined: tell the target to skip this flow */
 
-	if (write_all_buf(fd, t, sizeof(*t))) {
+	if (sewrite_var(fd, t)) {
 		flow_perror(conn, "Failed to write extended data");
 		return -EIO;
 	}
@@ -3710,7 +3711,7 @@ int tcp_flow_migrate_target(struct ctx *c, int fd)
 		return 0;
 	}
 
-	if (read_all_buf(fd, &t, sizeof(t))) {
+	if (seread_var(fd, &t)) {
 		flow_perror(flow, "Failed to receive migration data");
 		flow_alloc_cancel(flow);
 		return -errno;
@@ -3774,7 +3775,7 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd
 	struct tcp_tap_transfer_ext t;
 	int s = conn->sock, rc;
 
-	if (read_all_buf(fd, &t, sizeof(t))) {
+	if (seread_var(fd, &t)) {
 		rc = -errno;
 		flow_perror(conn, "Failed to read extended data");
 		return rc;
@@ -3819,13 +3820,13 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd
 		return -EINVAL;
 	}
 
-	if (read_all_buf(fd, tcp_migrate_snd_queue, t.sndq)) {
+	if (seread_buf(fd, tcp_migrate_snd_queue, t.sndq)) {
 		rc = -errno;
 		flow_perror(conn, "Failed to read send queue data");
 		return rc;
 	}
 
-	if (read_all_buf(fd, tcp_migrate_rcv_queue, t.rcvq)) {
+	if (seread_buf(fd, tcp_migrate_rcv_queue, t.rcvq)) {
 		rc = -errno;
 		flow_perror(conn, "Failed to read receive queue data");
 		return rc;
diff --git a/util.c b/util.c
index 22318c00..c64a1a61 100644
--- a/util.c
+++ b/util.c
@@ -37,6 +37,7 @@
 #include "pcap.h"
 #include "epoll_ctl.h"
 #include "pasta.h"
+#include "serialise.h"
 #ifdef HAS_GETRANDOM
 #include <sys/random.h>
 #endif
@@ -799,37 +800,6 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
 #endif
 }
 
-/**
- * write_all_buf() - write all of a buffer to an fd
- * @fd:		File descriptor
- * @buf:	Pointer to base of buffer
- * @len:	Length of buffer
- *
- * Return: 0 on success, -1 on error (with errno set)
- *
- * #syscalls write
- */
-int write_all_buf(int fd, const void *buf, size_t len)
-{
-	const char *p = buf;
-	size_t left = len;
-
-	while (left) {
-		ssize_t rc;
-
-		do
-			rc = write(fd, p, left);
-		while ((rc < 0) && errno == EINTR);
-
-		if (rc < 0)
-			return -1;
-
-		p += rc;
-		left -= rc;
-	}
-	return 0;
-}
-
 /**
  * write_remainder() - write the tail of an IO vector to an fd
  * @fd:		File descriptor
@@ -850,8 +820,8 @@ int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip)
 
 		if (offset) {
 			/* Write the remainder of the partially written buffer */
-			if (write_all_buf(fd, (char *)iov[i].iov_base + offset,
-					  iov[i].iov_len - offset) < 0)
+			if (sewrite_buf(fd, (char *)iov[i].iov_base + offset,
+					iov[i].iov_len - offset) < 0)
 				return -1;
 			i++;
 		}
@@ -866,44 +836,6 @@ int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip)
 	return 0;
 }
 
-/**
- * read_all_buf() - Fill a whole buffer from a file descriptor
- * @fd:		File descriptor
- * @buf:	Pointer to base of buffer
- * @len:	Length of buffer
- *
- * Return: 0 on success, -1 on error (with errno set)
- *
- * #syscalls read
- */
-int read_all_buf(int fd, void *buf, size_t len)
-{
-	size_t left = len;
-	char *p = buf;
-
-	while (left) {
-		ssize_t rc;
-
-		assert(left <= len);
-
-		do
-			rc = read(fd, p, left);
-		while ((rc < 0) && errno == EINTR);
-
-		if (rc < 0)
-			return -1;
-
-		if (rc == 0) {
-			errno = ENODATA;
-			return -1;
-		}
-
-		p += rc;
-		left -= rc;
-	}
-	return 0;
-}
-
 /**
  * read_remainder() - Read the tail of an IO vector from a file descriptor
  * @fd:		File descriptor
@@ -926,8 +858,8 @@ int read_remainder(int fd, const struct iovec *iov, size_t cnt, size_t skip)
 		if (offset) {
 			assert(offset < iov[i].iov_len);
 			/* Read the remainder of the partially read buffer */
-			if (read_all_buf(fd, (char *)iov[i].iov_base + offset,
-					 iov[i].iov_len - offset) < 0)
+			if (seread_buf(fd, (char *)iov[i].iov_base + offset,
+				       iov[i].iov_len - offset) < 0)
 				return -1;
 			i++;
 		}
diff --git a/util.h b/util.h
index 2fc5cd74..cb669105 100644
--- a/util.h
+++ b/util.h
@@ -245,9 +245,7 @@ int fls(unsigned long x);
 int ilog2(unsigned long x);
 int write_file(const char *path, const char *buf);
 intmax_t read_file_integer(const char *path, intmax_t fallback);
-int write_all_buf(int fd, const void *buf, size_t len);
 int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip);
-int read_all_buf(int fd, void *buf, size_t len);
 int read_remainder(int fd, const struct iovec *iov, size_t cnt, size_t skip);
 void close_open_files(int argc, char **argv);
 bool snprintf_check(char *str, size_t size, const char *format, ...);
-- 
2.53.0