[PATCH 00/18] More pesto preliminaries

[PATCH 00/18] More pesto preliminaries

[David Gibson]

The number of preliminary patches in the pesto series has grown to the
point that I think it's worth trying to get a bunch merged, while we
polish the rest.

Changes from pesto series v3
 * Extracted just the preliminary patches, not introducing pesto
   itself
 * Several small revisions based on Stefano's feedback
 * Added several extra bits of preliminary rework.

David Gibson (18):
  conf: runas can be const
  fwd: Comparing rule can be const
  vhost_user: Fix assorted minor cppcheck warnings
  serialise: Split functions user for serialisation from util.c
  serialise: Add helpers for serialising unsigned integers
  fwd: Move selecting correct scan bitmap into fwd_sync_one()
  fwd: Look up rule index in fwd_sync_one()
  fwd: Store forwarding tables indexed by (origin) pif
  fwd: Allow FWD_DUAL_STACK_ANY flag to be passed directly to
    fwd_rule_add()
  fwd, conf: Expose ephemeral ports as bitmap rather than function
  conf: Don't bother complaining about overlapping excluded ranges
  conf: Move check for mapping port 0 to caller
  conf: Move check for disabled interfaces earlier
  conf: Remove redundant warning when SO_BINDTODEVICE is unavailable
  pif: Limit pif names to IFNAMSIZ (16) bytes
  ip: Define a bound for the string returned by ipproto_name()
  bitmap: Split bitmap helper functions into their own module
  fwd: Split forwading rule specification from its implementation state

 Makefile     |  24 +++----
 bitmap.c     |  99 ++++++++++++++++++++++++++++
 bitmap.h     |  24 +++++++
 conf.c       | 113 ++++++++++++++++----------------
 flow.c       |  35 +++++-----
 fwd.c        | 177 ++++++++++++++++++++++++++-------------------------
 fwd.h        |  38 +++--------
 fwd_rule.h   |  44 +++++++++++++
 ip.c         |  18 ++++--
 ip.h         |   2 +
 migrate.c    |   1 +
 passt.h      |   6 +-
 pcap.c       |   1 +
 pif.c        |   2 +-
 pif.h        |   4 +-
 serialise.c  | 123 +++++++++++++++++++++++++++++++++++
 serialise.h  |  21 ++++++
 tcp.c        |   1 +
 util.c       | 154 +-------------------------------------------
 util.h       |  12 ----
 vhost_user.c |  16 +++--
 virtio.h     |   2 +-
 vu_common.c  |   2 +-
 23 files changed, 535 insertions(+), 384 deletions(-)
 create mode 100644 bitmap.c
 create mode 100644 bitmap.h
 create mode 100644 fwd_rule.h
 create mode 100644 serialise.c
 create mode 100644 serialise.h

-- 
2.53.0

[PATCH 01/18] conf: runas can be const

[David Gibson]

Both the runas variable in conf() and the parameter to conf_ugid() can be
a const pointer.  For some reason cppcheck doesn't catch this now, but does
after some upcoming rearrangements for sharing code with the dynamic update
tool.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/conf.c b/conf.c
index 940fb9e9..b1ebb4a4 100644
--- a/conf.c
+++ b/conf.c
@@ -1320,7 +1320,7 @@ static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid)
  * @uid:	User ID, set on success
  * @gid:	Group ID, set on success
  */
-static void conf_ugid(char *runas, uid_t *uid, gid_t *gid)
+static void conf_ugid(const char *runas, uid_t *uid, gid_t *gid)
 {
 	/* If user has specified --runas, that takes precedence... */
 	if (runas) {
@@ -1561,8 +1561,8 @@ void conf(struct ctx *c, int argc, char **argv)
 	uint8_t prefix_len_from_opt = 0;
 	unsigned int ifi4 = 0, ifi6 = 0;
 	const char *logfile = NULL;
+	const char *runas = NULL;
 	size_t logsize = 0;
-	char *runas = NULL;
 	long fd_tap_opt;
 	int name, ret;
 	uid_t uid;
-- 
2.53.0

[PATCH 02/18] fwd: Comparing rule can be const

[David Gibson]

This pointer stepping through the array of existing rules can be const.
For some reason cppcheck doesn't catch this at the moment, but does with
certain changes we're looking at for the dynamic configuration client.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fwd.c b/fwd.c
index f3b4bf2a..31058194 100644
--- a/fwd.c
+++ b/fwd.c
@@ -362,7 +362,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 	/* Check for any conflicting entries */
 	for (i = 0; i < fwd->count; i++) {
 		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
-		struct fwd_rule *rule = &fwd->rules[i];
+		const struct fwd_rule *rule = &fwd->rules[i];
 
 		if (proto != rule->proto)
 			/* Non-conflicting protocols */
-- 
2.53.0

[PATCH 03/18] vhost_user: Fix assorted minor cppcheck warnings

[David Gibson]

This fixes a batch of minor incorrect format specifier and "could be const"
warnings in the vhost_user code.  For some very strange reason, cppcheck
doesn't catch these errors right now, but does after code rearrangements
we're making for the dynamic forwarding update stuff.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 vhost_user.c | 16 +++++++++-------
 virtio.h     |  2 +-
 vu_common.c  |  2 +-
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/vhost_user.c b/vhost_user.c
index 75665ec6..f062badd 100644
--- a/vhost_user.c
+++ b/vhost_user.c
@@ -440,11 +440,13 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev,
 
 	debug("vhost-user nregions: %u", memory->nregions);
 	for (i = 0; i < vdev->memory.nregions; i++) {
-		struct vhost_user_memory_region *msg_region = &memory->regions[i];
 		struct vu_dev_region *dev_region = &vdev->memory.regions[i];
+		const struct vhost_user_memory_region *msg_region;
 		const void *mmap_addr;
 
-		debug("vhost-user region %d", i);
+		msg_region = &memory->regions[i];
+
+		debug("vhost-user region %u", i);
 		debug("    guest_phys_addr: 0x%016"PRIx64,
 		      msg_region->guest_phys_addr);
 		debug("    memory_size:     0x%016"PRIx64,
@@ -479,7 +481,7 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev,
 	for (i = 0; i < VHOST_USER_MAX_VQS; i++) {
 		if (vdev->vq[i].vring.desc) {
 			if (map_ring(vdev, &vdev->vq[i]))
-				die("remapping queue %d during setmemtable", i);
+				die("remapping queue %u during setmemtable", i);
 		}
 	}
 
@@ -763,8 +765,8 @@ static void vu_set_watch(const struct vu_dev *vdev, int idx)
  */
 static void vu_check_queue_msg_file(struct vhost_user_msg *vmsg)
 {
+	unsigned idx = vmsg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
 	bool nofd = vmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK;
-	int idx = vmsg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
 
 	if (idx >= VHOST_USER_MAX_VQS)
 		die("Invalid vhost-user queue index: %u", idx);
@@ -1010,11 +1012,11 @@ static bool vu_set_device_state_fd_exec(struct vu_dev *vdev,
 		die("Invalid device_state_fd message");
 
 	if (phase != VHOST_USER_TRANSFER_STATE_PHASE_STOPPED)
-		die("Invalid device_state_fd phase: %d", phase);
+		die("Invalid device_state_fd phase: %u", phase);
 
 	if (direction != VHOST_USER_TRANSFER_STATE_DIRECTION_SAVE &&
 	    direction != VHOST_USER_TRANSFER_STATE_DIRECTION_LOAD)
-		die("Invalid device_state_fd direction: %d", direction);
+		die("Invalid device_state_fd direction: %u", direction);
 
 	migrate_request(vdev->context, vmsg->fds[0],
 			direction == VHOST_USER_TRANSFER_STATE_DIRECTION_LOAD);
@@ -1047,7 +1049,7 @@ static bool vu_check_device_state_exec(struct vu_dev *vdev,
  */
 void vu_init(struct ctx *c)
 {
-	int i;
+	unsigned i;
 
 	c->vdev = &vdev_storage;
 	c->vdev->context = c;
diff --git a/virtio.h b/virtio.h
index c7e447d5..8f2ae068 100644
--- a/virtio.h
+++ b/virtio.h
@@ -90,7 +90,7 @@ struct vu_dev_region {
 	uint64_t mmap_addr;
 };
 
-#define VHOST_USER_MAX_VQS 2
+#define VHOST_USER_MAX_VQS 2U
 
 /*
  * Set a reasonable maximum number of ram slots, which will be supported by
diff --git a/vu_common.c b/vu_common.c
index 92381cd3..13b1e510 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -256,7 +256,7 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)
 			      ARRAY_SIZE(in_sg), &in_total, size, &total);
 	if (elem_cnt == 0 || total < size) {
 		debug("vu_send_single: no space to send the data "
-		      "elem_cnt %d size %zd", elem_cnt, total);
+		      "elem_cnt %d size %zu", elem_cnt, total);
 		goto err;
 	}
 
-- 
2.53.0

[PATCH 04/18] serialise: Split functions user for serialisation from util.c

[David Gibson]

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 serialise.c
file, and rename thematically.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile    | 11 ++++---
 flow.c      |  1 +
 migrate.c   |  1 +
 pcap.c      |  1 +
 serialise.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 serialise.h | 14 +++++++++
 tcp.c       |  1 +
 util.c      | 70 +-----------------------------------------
 util.h      |  2 --
 9 files changed, 113 insertions(+), 76 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..25a6f1a3 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",
diff --git a/migrate.c b/migrate.c
index 1e8858a3..8937b85f 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
diff --git a/pcap.c b/pcap.c
index 54fba5c4..a026f17e 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
 
diff --git a/serialise.c b/serialise.c
new file mode 100644
index 00000000..f162eeb6
--- /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"
+
+/**
+ * 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;
+}
+
+/**
+ * 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;
+}
diff --git a/serialise.h b/serialise.h
new file mode 100644
index 00000000..251c772a
--- /dev/null
+++ b/serialise.h
@@ -0,0 +1,14 @@
+/* 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 read_all_buf(int fd, void *buf, size_t len);
+int write_all_buf(int fd, const void *buf, size_t len);
+
+#endif /* SERIALISE_H */
diff --git a/tcp.c b/tcp.c
index b1458624..8ea9be84 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"
diff --git a/util.c b/util.c
index 22318c00..faa2c6a4 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
@@ -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
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

[PATCH 05/18] serialise: Add helpers for serialising unsigned integers

[David Gibson]

Add helpers to serialise/deserialise unsigned integers, handling endian
conversion so that the stream format is consistent regardless of host
endiannness.  Currently we only use this on one place: sending the number
of flows during migration.  We're going to have more use for this as we
add dynamic configuration updates, so these will become more useful.

For now we only need a 32-bit version, however define the functions with
a macro so we can easily add other integer widths when we need them.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 flow.c      | 10 ++++------
 serialise.c | 35 +++++++++++++++++++++++++++++++++++
 serialise.h |  7 +++++++
 3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/flow.c b/flow.c
index 25a6f1a3..c84857b2 100644
--- a/flow.c
+++ b/flow.c
@@ -1135,10 +1135,9 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage,
 			count++;
 	}
 
-	count = htonl(count);
-	if (write_all_buf(fd, &count, sizeof(count))) {
+	if (write_u32(fd, count)) {
 		rc = errno;
-		err_perror("Can't send flow count (%u)", ntohl(count));
+		err_perror("Can't send flow count (%u)", count);
 		return flow_migrate_source_rollback(c, FLOW_MAX, rc);
 	}
 
@@ -1151,7 +1150,7 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage,
 	debug("Stop listen()s");
 	fwd_listen_close(&c->fwd_in);
 
-	debug("Sending %u flows", ntohl(count));
+	debug("Sending %u flows", count);
 
 	if (!count)
 		return 0;
@@ -1221,10 +1220,9 @@ int flow_migrate_target(struct ctx *c, const struct migrate_stage *stage,
 
 	(void)stage;
 
-	if (read_all_buf(fd, &count, sizeof(count)))
+	if (read_u32(fd, &count))
 		return errno;
 
-	count = ntohl(count);
 	debug("Receiving %u flows", count);
 
 	if (!count)
diff --git a/serialise.c b/serialise.c
index f162eeb6..944e7414 100644
--- a/serialise.c
+++ b/serialise.c
@@ -13,7 +13,9 @@
  */
 
 #include <assert.h>
+#include <endian.h>
 #include <errno.h>
+#include <stdint.h>
 #include <unistd.h>
 
 #include "serialise.h"
@@ -86,3 +88,36 @@ int write_all_buf(int fd, const void *buf, size_t len)
 	}
 	return 0;
 }
+
+/**
+ * read_uXXX() - Receive a uXXX value from an fd
+ * @fd:		File descriptor to read from
+ * @valp:	Pointer to variable to update with read value
+ *
+ * Return: 0 on success, -1 on error
+ */
+/**
+ * write_uXXX() - Send a uXXX value to an fd
+ * @fd:		File descriptor to write to
+ * @val:	Value to send
+ *
+ * Return: 0 on success, -1 on error
+ */
+#define SERIALISE_UINT(bits)						\
+	int read_u##bits(int fd, uint##bits##_t *val)			\
+	{								\
+		uint##bits##_t beval;					\
+		if (read_all_buf(fd, &beval, sizeof(beval)) < 0)	\
+			return -1;					\
+		*val = be##bits##toh(beval);				\
+		return 0;						\
+	}								\
+	int write_u##bits(int fd, uint##bits##_t val)			\
+	{								\
+		uint##bits##_t beval = htobe##bits(val);		\
+		return write_all_buf(fd, &beval, sizeof(beval));	\
+	}
+
+SERIALISE_UINT(32)
+
+#undef SERIALISE_UINT
diff --git a/serialise.h b/serialise.h
index 251c772a..a88f3dee 100644
--- a/serialise.h
+++ b/serialise.h
@@ -7,8 +7,15 @@
 #define SERIALISE_H
 
 #include <stddef.h>
+#include <stdint.h>
 
 int read_all_buf(int fd, void *buf, size_t len);
 int write_all_buf(int fd, const void *buf, size_t len);
 
+#define SERIALISE_UINT_DECL(bits)					\
+	int read_u##bits(int fd, uint##bits##_t *val);			\
+	int write_u##bits(int fd, uint##bits##_t val);
+
+SERIALISE_UINT_DECL(32)
+
 #endif /* SERIALISE_H */
-- 
2.53.0

[PATCH 06/18] fwd: Move selecting correct scan bitmap into fwd_sync_one()

[David Gibson]

Currently fwd_listen_sync_() determines which of the two port scanned
bitmaps is the correct one for a rule before passing it into
fwd_sync_one().  However, fwd_sync_one() is already looking at the rule
internals so is better placed to do this.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/fwd.c b/fwd.c
index 31058194..74260a32 100644
--- a/fwd.c
+++ b/fwd.c
@@ -508,16 +508,18 @@ void fwd_rules_print(const struct fwd_table *fwd)
  * @fwd:	Forwarding table
  * @rule:	Forwarding rule
  * @pif:	Interface to create listening sockets for
- * @scanmap:	Bitmap of ports to listen for on FWD_SCAN entries
+ * @tcp:	Bitmap of TCP ports to listen for on FWD_SCAN entries
+ * @udp:	Bitmap of UDP ports to listen for on FWD_SCAN entries
  *
  * Return: 0 on success, -1 on failure
  */
 static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
 			const struct fwd_rule *rule, uint8_t pif,
-			const uint8_t *scanmap)
+			const uint8_t *tcp, const uint8_t *udp)
 {
 	const union inany_addr *addr = fwd_rule_addr(rule);
 	const char *ifname = rule->ifname;
+	const uint8_t *map = NULL;
 	bool bound_one = false;
 	unsigned port, idx;
 
@@ -528,12 +530,19 @@ static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
 
 	idx = rule - fwd->rules;
 	assert(idx < MAX_FWD_RULES);
-	assert(!(rule->flags & FWD_SCAN && !scanmap));
-	
+
+	if (rule->flags & FWD_SCAN) {
+		if (rule->proto == IPPROTO_TCP)
+			map = tcp;
+		else if (rule->proto == IPPROTO_UDP)
+			map = udp;
+		assert(map);
+	}
+
 	for (port = rule->first; port <= rule->last; port++) {
 		int fd = rule->socks[port - rule->first];
 
-		if ((rule->flags & FWD_SCAN) && !bitmap_isset(scanmap, port)) {
+		if (map && !bitmap_isset(map, port)) {
 			/* We don't want to listen on this port */
 			if (fd >= 0) {
 				/* We already are, so stop */
@@ -619,15 +628,8 @@ static int fwd_listen_sync_(void *arg)
 		ns_enter(a->c);
 
 	for (i = 0; i < a->fwd->count; i++) {
-		const uint8_t *scanmap = NULL;
-
-		if (a->fwd->rules[i].proto == IPPROTO_TCP)
-			scanmap = a->tcpmap;
-		else if (a->fwd->rules[i].proto == IPPROTO_UDP)
-			scanmap = a->udpmap;
-
 		a->ret = fwd_sync_one(a->c, a->fwd, &a->fwd->rules[i],
-				      a->pif, scanmap);
+				      a->pif, a->tcpmap, a->udpmap);
 		if (a->ret < 0)
 			break;
 	}
-- 
2.53.0

[PATCH 07/18] fwd: Look up rule index in fwd_sync_one()

[David Gibson]

Currently we have the slightly silly setup that fwd_listen_sync_() looks
up the pointer to a rule based on its index, then fwd_sync_one() turns it
back into an index.  Avoid this by passing the index directly into
fwd_sync_one().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fwd.c b/fwd.c
index 74260a32..f47d666d 100644
--- a/fwd.c
+++ b/fwd.c
@@ -506,7 +506,7 @@ void fwd_rules_print(const struct fwd_table *fwd)
 /** fwd_sync_one() - Create or remove listening sockets for a forward entry
  * @c:		Execution context
  * @fwd:	Forwarding table
- * @rule:	Forwarding rule
+ * @idx:	Rule index
  * @pif:	Interface to create listening sockets for
  * @tcp:	Bitmap of TCP ports to listen for on FWD_SCAN entries
  * @udp:	Bitmap of UDP ports to listen for on FWD_SCAN entries
@@ -514,23 +514,21 @@ void fwd_rules_print(const struct fwd_table *fwd)
  * Return: 0 on success, -1 on failure
  */
 static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
-			const struct fwd_rule *rule, uint8_t pif,
+			unsigned idx, uint8_t pif,
 			const uint8_t *tcp, const uint8_t *udp)
 {
+	const struct fwd_rule *rule = &fwd->rules[idx];
 	const union inany_addr *addr = fwd_rule_addr(rule);
 	const char *ifname = rule->ifname;
 	const uint8_t *map = NULL;
 	bool bound_one = false;
-	unsigned port, idx;
+	unsigned port;
 
 	assert(pif_is_socket(pif));
 
 	if (!*ifname)
 		ifname = NULL;
 
-	idx = rule - fwd->rules;
-	assert(idx < MAX_FWD_RULES);
-
 	if (rule->flags & FWD_SCAN) {
 		if (rule->proto == IPPROTO_TCP)
 			map = tcp;
@@ -628,8 +626,8 @@ static int fwd_listen_sync_(void *arg)
 		ns_enter(a->c);
 
 	for (i = 0; i < a->fwd->count; i++) {
-		a->ret = fwd_sync_one(a->c, a->fwd, &a->fwd->rules[i],
-				      a->pif, a->tcpmap, a->udpmap);
+		a->ret = fwd_sync_one(a->c, a->fwd, i, a->pif,
+				      a->tcpmap, a->udpmap);
 		if (a->ret < 0)
 			break;
 	}
-- 
2.53.0

[PATCH 08/18] fwd: Store forwarding tables indexed by (origin) pif

[David Gibson]

Currently we store the inbound (PIF_HOST) and outbound (PIF_SPLICE)
forwarding tables in separate fields of struct ctx.  In a number of places
this requires somewhat awkward if or switch constructs to select the
right table for updates.  Conceptually simplify that by using an index of
forwarding tables by pif, which as a bonus keeps track generically which
pifs have implemented forwarding tables so far.

For now this doesn't simplify a lot textually, because many places that
need this also have other special cases to apply by pif.  It does simplify
a few crucial places though, and we expect it will become more useful as
the flexibility of the forwarding table is improved.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c  | 58 +++++++++++++++++++++++++++++++-------------------
 flow.c  | 24 +++++++++------------
 fwd.c   | 65 ++++++++++++++++++++++++++++++---------------------------
 fwd.h   |  4 ++--
 passt.h |  6 ++----
 5 files changed, 85 insertions(+), 72 deletions(-)

diff --git a/conf.c b/conf.c
index b1ebb4a4..6ca61b74 100644
--- a/conf.c
+++ b/conf.c
@@ -1252,11 +1252,17 @@ dns6:
 		}
 	}
 
-	info("Inbound forwarding:");
-	fwd_rules_print(&c->fwd_in);
-	if (c->mode == MODE_PASTA) {
-		info("Outbound forwarding:");
-		fwd_rules_print(&c->fwd_out);
+	for (i = 0; i < PIF_NUM_TYPES; i++) {
+		const char *dir = "Outbound";
+
+		if (!c->fwd[i])
+			continue;
+
+		if (i == PIF_HOST)
+			dir = "Inbound";
+
+		info("%s forwarding rules (%s):", dir, pif_name(i));
+		fwd_rules_print(c->fwd[i]);
 	}
 }
 
@@ -2154,18 +2160,24 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	/* Forwarding options can be parsed now, after IPv4/IPv6 settings */
 	fwd_probe_ephemeral();
+	fwd_rule_init(c);
 	optind = 0;
 	do {
 		name = getopt_long(argc, argv, optstring, options, NULL);
 
-		if (name == 't')
-			conf_ports(c, name, optarg, &c->fwd_in, &tcp_in_mode);
-		else if (name == 'u')
-			conf_ports(c, name, optarg, &c->fwd_in, &udp_in_mode);
-		else if (name == 'T')
-			conf_ports(c, name, optarg, &c->fwd_out, &tcp_out_mode);
-		else if (name == 'U')
-			conf_ports(c, name, optarg, &c->fwd_out, &udp_out_mode);
+		if (name == 't') {
+			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
+				   &tcp_in_mode);
+		} else if (name == 'u') {
+			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
+				   &udp_in_mode);
+		} else if (name == 'T') {
+			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
+				   &tcp_out_mode);
+		} else if (name == 'U') {
+			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
+				   &udp_out_mode);
+		}
 	} while (name != -1);
 
 	if (c->mode == MODE_PASTA)
@@ -2224,20 +2236,24 @@ void conf(struct ctx *c, int argc, char **argv)
 		udp_out_mode = fwd_default;
 
 	if (tcp_in_mode == FWD_MODE_AUTO) {
-		conf_ports_range_except(c, 't', "auto", &c->fwd_in, NULL, NULL,
-					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
+		conf_ports_range_except(c, 't', "auto", c->fwd[PIF_HOST],
+					NULL, NULL, 1, NUM_PORTS - 1, NULL, 1,
+					FWD_SCAN);
 	}
 	if (tcp_out_mode == FWD_MODE_AUTO) {
-		conf_ports_range_except(c, 'T', "auto", &c->fwd_out, NULL, "lo",
-					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
+		conf_ports_range_except(c, 'T', "auto", c->fwd[PIF_SPLICE],
+					NULL, "lo", 1, NUM_PORTS - 1, NULL, 1,
+					FWD_SCAN);
 	}
 	if (udp_in_mode == FWD_MODE_AUTO) {
-		conf_ports_range_except(c, 'u', "auto", &c->fwd_in, NULL, NULL,
-					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
+		conf_ports_range_except(c, 'u', "auto", c->fwd[PIF_HOST],
+					NULL, NULL, 1, NUM_PORTS - 1, NULL, 1,
+					FWD_SCAN);
 	}
 	if (udp_out_mode == FWD_MODE_AUTO) {
-		conf_ports_range_except(c, 'U', "auto", &c->fwd_out, NULL, "lo",
-					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
+		conf_ports_range_except(c, 'U', "auto", c->fwd[PIF_SPLICE],
+					NULL, "lo", 1, NUM_PORTS - 1, NULL, 1,
+					FWD_SCAN);
 	}
 
 	if (!c->quiet)
diff --git a/flow.c b/flow.c
index c84857b2..56a6c6d3 100644
--- a/flow.c
+++ b/flow.c
@@ -503,10 +503,10 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
 {
 	char estr[INANY_ADDRSTRLEN], ostr[INANY_ADDRSTRLEN];
 	struct flow_common *f = &flow->f;
+	const struct fwd_table *fwd = c->fwd[f->pif[INISIDE]];
 	const struct flowside *ini = &f->side[INISIDE];
 	struct flowside *tgt = &f->side[TGTSIDE];
 	const struct fwd_rule *rule = NULL;
-	const struct fwd_table *fwd;
 	uint8_t tgtpif = PIF_NONE;
 
 	assert(flow_new_entry == flow && f->state == FLOW_STATE_INI);
@@ -514,6 +514,11 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
 	assert(f->pif[INISIDE] != PIF_NONE && f->pif[TGTSIDE] == PIF_NONE);
 	assert(flow->f.state == FLOW_STATE_INI);
 
+	if (fwd) {
+		if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
+			goto norule;
+	}
+
 	switch (f->pif[INISIDE]) {
 	case PIF_TAP:
 		memcpy(f->tap_omac, MAC_UNDEF, ETH_ALEN);
@@ -521,20 +526,12 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
 		break;
 
 	case PIF_SPLICE:
-		fwd = &c->fwd_out;
-
-		if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
-			goto norule;
-
+		assert(rule);
 		tgtpif = fwd_nat_from_splice(rule, proto, ini, tgt);
 		break;
 
 	case PIF_HOST:
-		fwd = &c->fwd_in;
-
-		if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
-			goto norule;
-
+		assert(rule);
 		tgtpif = fwd_nat_from_host(c, rule, proto, ini, tgt);
 		fwd_neigh_mac_get(c, &tgt->oaddr, f->tap_omac);
 		break;
@@ -1014,8 +1011,7 @@ static int flow_migrate_source_rollback(struct ctx *c, unsigned bound, int ret)
 
 	debug("...roll back migration");
 
-	if (fwd_listen_sync(c, &c->fwd_in, PIF_HOST,
-			    &c->tcp.scan_in, &c->udp.scan_in) < 0)
+	if (fwd_listen_sync(c, PIF_HOST, &c->tcp.scan_in, &c->udp.scan_in) < 0)
 		die("Failed to re-establish listening sockets");
 
 	foreach_established_tcp_flow(flow) {
@@ -1148,7 +1144,7 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage,
 	 * fix that is to not allow local to local migration, which arguably we
 	 * should (use namespaces for testing instead). */
 	debug("Stop listen()s");
-	fwd_listen_close(&c->fwd_in);
+	fwd_listen_close(c->fwd[PIF_HOST]);
 
 	debug("Sending %u flows", count);
 
diff --git a/fwd.c b/fwd.c
index f47d666d..96957d9f 100644
--- a/fwd.c
+++ b/fwd.c
@@ -331,6 +331,21 @@ bool fwd_port_is_ephemeral(in_port_t port)
 	return (port >= fwd_ephemeral_min) && (port <= fwd_ephemeral_max);
 }
 
+/* Forwarding table storage, generally accessed via pointers in struct ctx */
+static struct fwd_table fwd_in;
+static struct fwd_table fwd_out;
+
+/**
+ * fwd_rule_init() - Initialise forwarding tables
+ * @c:		Execution context
+ */
+void fwd_rule_init(struct ctx *c)
+{
+	c->fwd[PIF_HOST] = &fwd_in;
+	if (c->mode == MODE_PASTA)
+		c->fwd[PIF_SPLICE] = &fwd_out;
+}
+
 /**
  * fwd_rule_add() - Add a rule to a forwarding table
  * @fwd:	Table to add to
@@ -505,19 +520,17 @@ void fwd_rules_print(const struct fwd_table *fwd)
 
 /** fwd_sync_one() - Create or remove listening sockets for a forward entry
  * @c:		Execution context
- * @fwd:	Forwarding table
- * @idx:	Rule index
  * @pif:	Interface to create listening sockets for
+ * @idx:	Rule index
  * @tcp:	Bitmap of TCP ports to listen for on FWD_SCAN entries
  * @udp:	Bitmap of UDP ports to listen for on FWD_SCAN entries
  *
  * Return: 0 on success, -1 on failure
  */
-static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
-			unsigned idx, uint8_t pif,
+static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
 			const uint8_t *tcp, const uint8_t *udp)
 {
-	const struct fwd_rule *rule = &fwd->rules[idx];
+	const struct fwd_rule *rule = &c->fwd[pif]->rules[idx];
 	const union inany_addr *addr = fwd_rule_addr(rule);
 	const char *ifname = rule->ifname;
 	const uint8_t *map = NULL;
@@ -598,7 +611,6 @@ static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
 
 /** struct fwd_listen_args - arguments for fwd_listen_init_()
  * @c:		Execution context
- * @fwd:	Forwarding table
  * @tcpmap:	Bitmap of TCP ports to auto-forward
  * @udpmap:	Bitmap of TCP ports to auto-forward
  * @pif:	Interface to create listening sockets for
@@ -606,7 +618,6 @@ static int fwd_sync_one(const struct ctx *c, const struct fwd_table *fwd,
  */
 struct fwd_listen_args {
 	const struct ctx *c;
-	const struct fwd_table *fwd;
 	const uint8_t *tcpmap, *udpmap;
 	uint8_t pif;
 	int ret;
@@ -625,9 +636,8 @@ static int fwd_listen_sync_(void *arg)
 	if (a->pif == PIF_SPLICE)
 		ns_enter(a->c);
 
-	for (i = 0; i < a->fwd->count; i++) {
-		a->ret = fwd_sync_one(a->c, a->fwd, i, a->pif,
-				      a->tcpmap, a->udpmap);
+	for (i = 0; i < a->c->fwd[a->pif]->count; i++) {
+		a->ret = fwd_sync_one(a->c, a->pif, i, a->tcpmap, a->udpmap);
 		if (a->ret < 0)
 			break;
 	}
@@ -637,21 +647,17 @@ static int fwd_listen_sync_(void *arg)
 
 /** fwd_listen_sync() - Call fwd_listen_sync_() in correct namespace
  * @c:		Execution context
- * @fwd:	Forwarding information
  * @pif:	Interface to create listening sockets for
  * @tcp:	Scanning state for TCP
  * @udp:	Scanning state for UDP
  *
  * Return: 0 on success, -1 on failure
  */
-int fwd_listen_sync(const struct ctx *c, const struct fwd_table *fwd,
-		    uint8_t pif,
+int fwd_listen_sync(const struct ctx *c, uint8_t pif,
 		    const struct fwd_scan *tcp, const struct fwd_scan *udp)
 {
 	struct fwd_listen_args a = {
-		.c = c, .fwd = fwd,
-		.tcpmap = tcp->map, .udpmap = udp->map,
-		.pif = pif,
+		.c = c, .tcpmap = tcp->map, .udpmap = udp->map, .pif = pif,
 	};
 
 	if (pif == PIF_SPLICE)
@@ -695,12 +701,11 @@ void fwd_listen_close(const struct fwd_table *fwd)
  */
 int fwd_listen_init(const struct ctx *c)
 {
-	if (fwd_listen_sync(c, &c->fwd_in, PIF_HOST,
-			    &c->tcp.scan_in, &c->udp.scan_in) < 0)
+	if (fwd_listen_sync(c, PIF_HOST, &c->tcp.scan_in, &c->udp.scan_in) < 0)
 		return -1;
 
 	if (c->mode == MODE_PASTA) {
-		if (fwd_listen_sync(c, &c->fwd_out, PIF_SPLICE,
+		if (fwd_listen_sync(c, PIF_SPLICE,
 				    &c->tcp.scan_out, &c->udp.scan_out) < 0)
 			return -1;
 	}
@@ -851,16 +856,16 @@ static void fwd_scan_ports(struct ctx *c)
 	uint8_t excl_tcp_out[PORT_BITMAP_SIZE], excl_udp_out[PORT_BITMAP_SIZE];
 	uint8_t excl_tcp_in[PORT_BITMAP_SIZE], excl_udp_in[PORT_BITMAP_SIZE];
 
-	current_listen_map(excl_tcp_out, &c->fwd_in, IPPROTO_TCP);
-	current_listen_map(excl_tcp_in, &c->fwd_out, IPPROTO_TCP);
-	current_listen_map(excl_udp_out, &c->fwd_in, IPPROTO_UDP);
-	current_listen_map(excl_udp_in, &c->fwd_out, IPPROTO_UDP);
+	current_listen_map(excl_tcp_out, c->fwd[PIF_HOST], IPPROTO_TCP);
+	current_listen_map(excl_tcp_in, c->fwd[PIF_SPLICE], IPPROTO_TCP);
+	current_listen_map(excl_udp_out, c->fwd[PIF_HOST], IPPROTO_UDP);
+	current_listen_map(excl_udp_in, c->fwd[PIF_SPLICE], IPPROTO_UDP);
 
-	fwd_scan_ports_tcp(&c->fwd_out, &c->tcp.scan_out, excl_tcp_out);
-	fwd_scan_ports_tcp(&c->fwd_in, &c->tcp.scan_in, excl_tcp_in);
-	fwd_scan_ports_udp(&c->fwd_out, &c->udp.scan_out,
+	fwd_scan_ports_tcp(c->fwd[PIF_SPLICE], &c->tcp.scan_out, excl_tcp_out);
+	fwd_scan_ports_tcp(c->fwd[PIF_HOST], &c->tcp.scan_in, excl_tcp_in);
+	fwd_scan_ports_udp(c->fwd[PIF_SPLICE], &c->udp.scan_out,
 			   &c->tcp.scan_out, excl_udp_out);
-	fwd_scan_ports_udp(&c->fwd_in, &c->udp.scan_in,
+	fwd_scan_ports_udp(c->fwd[PIF_HOST], &c->udp.scan_in,
 			   &c->tcp.scan_in, excl_udp_in);
 }
 
@@ -912,10 +917,8 @@ void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now)
 
 	fwd_scan_ports(c);
 
-	fwd_listen_sync(c, &c->fwd_in, PIF_HOST,
-			&c->tcp.scan_in, &c->udp.scan_in);
-	fwd_listen_sync(c, &c->fwd_out, PIF_SPLICE,
-			&c->tcp.scan_out, &c->udp.scan_out);
+	fwd_listen_sync(c, PIF_HOST, &c->tcp.scan_in, &c->udp.scan_in);
+	fwd_listen_sync(c, PIF_SPLICE, &c->tcp.scan_out, &c->udp.scan_out);
 }
 
 /**
diff --git a/fwd.h b/fwd.h
index 958eee25..b387d926 100644
--- a/fwd.h
+++ b/fwd.h
@@ -108,6 +108,7 @@ struct fwd_scan {
 
 #define FWD_PORT_SCAN_INTERVAL		1000	/* ms */
 
+void fwd_rule_init(struct ctx *c);
 void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 		  const union inany_addr *addr, const char *ifname,
 		  in_port_t first, in_port_t last, in_port_t to);
@@ -119,8 +120,7 @@ void fwd_rules_print(const struct fwd_table *fwd);
 void fwd_scan_ports_init(struct ctx *c);
 void fwd_scan_ports_timer(struct ctx * c, const struct timespec *now);
 
-int fwd_listen_sync(const struct ctx *c, const struct fwd_table *fwd,
-		    uint8_t pif,
+int fwd_listen_sync(const struct ctx *c, uint8_t pif,
 		    const struct fwd_scan *tcp, const struct fwd_scan *udp);
 void fwd_listen_close(const struct fwd_table *fwd);
 int fwd_listen_init(const struct ctx *c);
diff --git a/passt.h b/passt.h
index b614bdf0..62b8dcdf 100644
--- a/passt.h
+++ b/passt.h
@@ -184,8 +184,7 @@ struct ip6_ctx {
  * @pasta_ifn:		Name of namespace interface for pasta
  * @pasta_ifi:		Index of namespace interface for pasta
  * @pasta_conf_ns:	Configure namespace after creating it
- * @fwd_in:		Forwarding table for inbound flows
- * @fwd_out:		Forwarding table for outbound flows
+ * @fwd:		Forwarding tables
  * @no_tcp:		Disable TCP operation
  * @tcp:		Context for TCP protocol handler
  * @no_udp:		Disable UDP operation
@@ -264,8 +263,7 @@ struct ctx {
 	unsigned int pasta_ifi;
 	int pasta_conf_ns;
 
-	struct fwd_table fwd_in;
-	struct fwd_table fwd_out;
+	struct fwd_table *fwd[PIF_NUM_TYPES];
 
 	int no_tcp;
 	struct tcp_ctx tcp;
-- 
2.53.0

[PATCH 09/18] fwd: Allow FWD_DUAL_STACK_ANY flag to be passed directly to fwd_rule_add()

[David Gibson]

fwd_rule_add() takes a flags parameter, but it only allows the FWD_WEAK
and FWD_SCAN flags to be specified there.  It doesn't allow the
FWD_DUAL_STACK_ANY flag to be set, instead expecting a [*] address to be
indicated by passing NULL as @addr.

However, for upcoming dynamic rule updates, it's more convenient to be able
to explicitly pass FWD_DUAL_STACK_ANY along with an address of ::.  Allow
that mode of calling.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fwd.c b/fwd.c
index 96957d9f..03652b23 100644
--- a/fwd.c
+++ b/fwd.c
@@ -362,12 +362,15 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 		  in_port_t first, in_port_t last, in_port_t to)
 {
 	/* Flags which can be set from the caller */
-	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN;
+	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
 	unsigned num = (unsigned)last - first + 1;
 	struct fwd_rule *new;
 	unsigned i, port;
 
 	assert(!(flags & ~allowed_flags));
+	/* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */
+	assert(!(flags & FWD_DUAL_STACK_ANY) || !addr ||
+	       inany_equals(addr, &inany_any6));
 
 	if (fwd->count >= ARRAY_SIZE(fwd->rules))
 		die("Too many port forwarding ranges");
-- 
2.53.0

[PATCH 10/18] fwd, conf: Expose ephemeral ports as bitmap rather than function

[David Gibson]

It turns out the only callers of fwd_port_is_ephemeral() use it to build a
bitmap of ephemeral ports.  So, replace it with fwd_port_map_ephemeral(),
which directly builds that bitmap.  As a bonus this allows a slightly
cheaper implementation of building the map, since inside fwd.c we know that
the ephemeral ports form a single range.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c |  8 ++------
 fwd.c  | 15 +++++++--------
 fwd.h  |  2 +-
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/conf.c b/conf.c
index 6ca61b74..7a3085b7 100644
--- a/conf.c
+++ b/conf.c
@@ -282,9 +282,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		*mode = FWD_MODE_ALL;
 
 		/* Exclude ephemeral ports */
-		for (i = 0; i < NUM_PORTS; i++)
-			if (fwd_port_is_ephemeral(i))
-				bitmap_set(exclude, i);
+		fwd_port_map_ephemeral(exclude);
 
 		conf_ports_range_except(c, optname, optarg, fwd,
 					NULL, NULL,
@@ -376,9 +374,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 
 	if (exclude_only) {
 		/* Exclude ephemeral ports */
-		for (i = 0; i < NUM_PORTS; i++)
-			if (fwd_port_is_ephemeral(i))
-				bitmap_set(exclude, i);
+		fwd_port_map_ephemeral(exclude);
 
 		conf_ports_range_except(c, optname, optarg, fwd,
 					addr, ifname,
diff --git a/fwd.c b/fwd.c
index 03652b23..62d344c3 100644
--- a/fwd.c
+++ b/fwd.c
@@ -319,16 +319,15 @@ static const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule)
 }
 
 /**
- * fwd_port_is_ephemeral() - Is port number ephemeral?
- * @port:	Port number
- *
- * Return: true if @port is ephemeral, that is may be allocated by the kernel as
- *         a local port for outgoing connections or datagrams, but should not be
- *         used for binding services to.
+ * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap
+ * @map:	Bitmap to update
  */
-bool fwd_port_is_ephemeral(in_port_t port)
+void fwd_port_map_ephemeral(uint8_t *map)
 {
-	return (port >= fwd_ephemeral_min) && (port <= fwd_ephemeral_max);
+	unsigned port;
+
+	for (port = fwd_ephemeral_min; port <= fwd_ephemeral_max; port++)
+		bitmap_set(map, port);
 }
 
 /* Forwarding table storage, generally accessed via pointers in struct ctx */
diff --git a/fwd.h b/fwd.h
index b387d926..f111e139 100644
--- a/fwd.h
+++ b/fwd.h
@@ -22,7 +22,7 @@ struct flowside;
 #define	NUM_PORTS	(1U << 16)
 
 void fwd_probe_ephemeral(void);
-bool fwd_port_is_ephemeral(in_port_t port);
+void fwd_port_map_ephemeral(uint8_t *map);
 
 /**
  * struct fwd_rule - Forwarding rule governing a range of ports
-- 
2.53.0

[PATCH 11/18] conf: Don't bother complaining about overlapping excluded ranges

[David Gibson]

In conf_ports() we die with an error if a port specification includes
overlapping excluded ranges.  This is contrary to our usual convention of
handling conflicting options (last option wins, rather than error).  Plus,
these options don't even conflict, they're just redundant.

This behaviour potentially makes life harder for scripts or other tools
invoking pasta - if they might have the same port excluded for multiple
different reasons, they have to explicitly deduplicate the list, rather
than just list everything on the command line.  So, don't give this error,
let a port be excluded as many times as you like.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/conf.c b/conf.c
index 7a3085b7..e5baf8ee 100644
--- a/conf.c
+++ b/conf.c
@@ -355,12 +355,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		if ((*p != '\0')  && (*p != ',')) /* Garbage after the range */
 			goto bad;
 
-		for (i = xrange.first; i <= xrange.last; i++) {
-			if (bitmap_isset(exclude, i))
-				die("Overlapping excluded ranges %s", optarg);
-
+		for (i = xrange.first; i <= xrange.last; i++)
 			bitmap_set(exclude, i);
-		}
 	} while ((p = next_chunk(p, ',')));
 
 	if (ifname && c->no_bindtodevice) {
-- 
2.53.0

[PATCH 12/18] conf: Move check for mapping port 0 to caller

[David Gibson]

conf_ports_range_except() checks that it hasn't been asked to map port 0,
which generally won't work.  However, there's only one callsite that
doesn't statically pass a non-zero value here.  Change the check to an
assert() and move the actual error message to that callsite.  This will
allow further cleanups later.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/conf.c b/conf.c
index e5baf8ee..dc9468c2 100644
--- a/conf.c
+++ b/conf.c
@@ -151,10 +151,7 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 	unsigned base, i;
 	uint8_t proto;
 
-	if (first == 0) {
-		die("Can't forward port 0 for option '-%c %s'",
-		    optname, optarg);
-	}
+	assert(first != 0);
 
 	if (optname == 't' || optname == 'T')
 		proto = IPPROTO_TCP;
@@ -404,6 +401,11 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		if ((*p != '\0')  && (*p != ',')) /* Garbage after the ranges */
 			goto bad;
 
+		if (orig_range.first == 0) {
+			die("Can't forward port 0 for option '-%c %s'",
+			    optname, optarg);
+		}
+
 		conf_ports_range_except(c, optname, optarg, fwd,
 					addr, ifname,
 					orig_range.first, orig_range.last,
-- 
2.53.0

[PATCH 13/18] conf: Move check for disabled interfaces earlier

[David Gibson]

conf_ports_range_except() generates errors if asked to map an address of
a family that's not available.  This is quite late to perform the check,
most callsites pass a NULL address, making the test moot, the one remaining
can check the address earlier.  Move the check there, simplifying the
lower level function.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/conf.c b/conf.c
index dc9468c2..d4c2a013 100644
--- a/conf.c
+++ b/conf.c
@@ -160,16 +160,6 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 	else
 		assert(0);
 
-	if (addr) {
-		if (!c->ifi4 && inany_v4(addr)) {
-			die("IPv4 is disabled, can't use -%c %s",
-			    optname, optarg);
-		} else if (!c->ifi6 && !inany_v4(addr)) {
-			die("IPv6 is disabled, can't use -%c %s",
-			    optname, optarg);
-		}
-	}
-
 	for (base = first; base <= last; base++) {
 		if (exclude && bitmap_isset(exclude, base))
 			continue;
@@ -335,6 +325,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		addr = NULL;
 	}
 
+	if (addr) {
+		if (!c->ifi4 && inany_v4(addr)) {
+			die("IPv4 is disabled, can't use -%c %s",
+			    optname, optarg);
+		} else if (!c->ifi6 && !inany_v4(addr)) {
+			die("IPv6 is disabled, can't use -%c %s",
+			    optname, optarg);
+		}
+	}
+
 	/* Mark all exclusions first, they might be given after base ranges */
 	p = spec;
 	do {
-- 
2.53.0

[PATCH 14/18] conf: Remove redundant warning when SO_BINDTODEVICE is unavailable

[David Gibson]

conf_ports() has logic to warn if -[TU] auto is specified but we can't use
SO_BINDTODEVICE.  However, this is redundant with similar logic in
conf_ports_range_except().  The latter will be triggered both for an
explicit -[TU] auto and when it's invoked as a default option, so keep that
one and drop the one in conf_ports().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/conf.c b/conf.c
index d4c2a013..5639ef03 100644
--- a/conf.c
+++ b/conf.c
@@ -249,12 +249,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		if (c->mode != MODE_PASTA)
 			die("'auto' port forwarding is only allowed for pasta");
 
-		if ((optname == 'T' || optname == 'U') && c->no_bindtodevice) {
-			warn(
-"'-%c auto' enabled without unprivileged SO_BINDTODEVICE", optname);
-			warn(
-"Forwarding from addresses other than 127.0.0.1 will not work");
-		}
 		*mode = FWD_MODE_AUTO;
 		return;
 	}
-- 
2.53.0

[PATCH 15/18] pif: Limit pif names to IFNAMSIZ (16) bytes

[David Gibson]

All current pif names are quite short, and we expect them to remain short
when/if we allow arbitrary pifs.  However, because of the structure of
the current code we don't enforce any limit on the length.

This will become more important with dynamic configuration updates, so
start enforcing a length limit.  Specifically we allow pif names to be up
to IFNAMSIZ bytes, including the terminating \0.  This is semi-arbitrary -
there's no particular reason we have to use the same length limit as
kernel netif names.  However, when we do allow arbitrary pifs, we expect
that we might support a similar number to the number of kernel interfaces.
It might make sense to use names matching kernel interface names in that
future.  So, re-use IFNAMSIZ to avoid surprise.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 pif.c | 2 +-
 pif.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/pif.c b/pif.c
index 1e807247..4e25e3fa 100644
--- a/pif.c
+++ b/pif.c
@@ -17,7 +17,7 @@
 #include "inany.h"
 #include "epoll_ctl.h"
 
-const char *pif_type_str[] = {
+const char pif_type_str[][IFNAMSIZ] = {
 	[PIF_NONE]		= "<none>",
 	[PIF_HOST]		= "HOST",
 	[PIF_TAP]		= "TAP",
diff --git a/pif.h b/pif.h
index 7bb58e5c..c467a941 100644
--- a/pif.h
+++ b/pif.h
@@ -35,7 +35,8 @@ enum pif_type {
 	PIF_NUM_TYPES,
 };
 
-extern const char *pif_type_str[];
+/* Limit pif names to the same length as kernel interface names */
+extern const char pif_type_str[][IFNAMSIZ];
 
 static inline const char *pif_type(enum pif_type pt)
 {
@@ -43,6 +44,7 @@ static inline const char *pif_type(enum pif_type pt)
 		return pif_type_str[pt];
 	else
 		return "?";
+	static_assert(sizeof("?") <= IFNAMSIZ);
 }
 
 static inline const char *pif_name(uint8_t pif)
-- 
2.53.0

[PATCH 16/18] ip: Define a bound for the string returned by ipproto_name()

[David Gibson]

ipproto_name() returns a static string of theoretically unbounded length.
That's going to be inconvenient in future, so introduce IPPROTO_STRLEN
giving an explicit bound on the length.  Use static_assert() and some
macros to ensure nothing we return can exceed this.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 ip.c | 18 ++++++++++++------
 ip.h |  2 ++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/ip.c b/ip.c
index 0ea62998..25fa4073 100644
--- a/ip.c
+++ b/ip.c
@@ -12,6 +12,7 @@
  * Author: Stefano Brivio <sbrivio@redhat.com>
  */
 
+#include <assert.h>
 #include <stddef.h>
 #include <netinet/in.h>
 
@@ -74,7 +75,7 @@ found:
  * ipproto_name() - Get IP protocol name from number
  * @proto:	IP protocol number
  *
- * Return: pointer to name of protocol @proto
+ * Return: pointer to name of protocol @proto (<= IPPROTO_STRLEN bytes)
  *
  * Usually this would be done with getprotobynumber(3) but that reads
  * /etc/protocols and might allocate, which isn't possible for us once
@@ -83,16 +84,21 @@ found:
 const char *ipproto_name(uint8_t proto)
 {
 	switch (proto) {
+#define CASE(s)								\
+		static_assert(sizeof(s) <= IPPROTO_STRLEN,		\
+			      "Increase IPPROTO_STRLEN to contain " #s); \
+		return s;
 	case IPPROTO_ICMP:
-		return "ICMP";
+		CASE("ICMP");
 	case IPPROTO_TCP:
-		return "TCP";
+		CASE("TCP");
 	case IPPROTO_UDP:
-		return "UDP";
+		CASE("UDP");
 	case IPPROTO_ICMPV6:
-		return "ICMPv6";
+		CASE("ICMPv6");
 	default:
-		return "<unknown protocol>";
+		CASE("<unknown protocol>");
+#undef CASE
 	}
 }
 
diff --git a/ip.h b/ip.h
index d0de6c8d..fb4119a7 100644
--- a/ip.h
+++ b/ip.h
@@ -118,6 +118,8 @@ static inline uint32_t ip6_get_flow_lbl(const struct ipv6hdr *ip6h)
 }
 
 bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen);
+
+#define IPPROTO_STRLEN		(sizeof("<unknown protocol>"))
 const char *ipproto_name(uint8_t proto);
 
 /* IPv6 link-local all-nodes multicast address, ff02::1 */
-- 
2.53.0

[PATCH 17/18] bitmap: Split bitmap helper functions into their own module

[David Gibson]

Currently bitmap functions are in util.[ch] along with a lot of other
stuff.  In preparation for sharing them with a configuration client, move
these out into their own files.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile | 17 +++++-----
 bitmap.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 bitmap.h | 24 ++++++++++++++
 conf.c   |  1 +
 fwd.h    |  1 +
 util.c   | 84 -----------------------------------------------
 util.h   | 10 ------
 7 files changed, 134 insertions(+), 102 deletions(-)
 create mode 100644 bitmap.c
 create mode 100644 bitmap.h

diff --git a/Makefile b/Makefile
index 5b6891d7..d6ced328 100644
--- a/Makefile
+++ b/Makefile
@@ -37,20 +37,21 @@ FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
 FLAGS += -DVERSION=\"$(VERSION)\"
 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 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
+PASST_SRCS = arch.c arp.c bitmap.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 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)
 
 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_HEADERS = arch.h arp.h bitmap.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 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 \
diff --git a/bitmap.c b/bitmap.c
new file mode 100644
index 00000000..b3b204b2
--- /dev/null
+++ b/bitmap.c
@@ -0,0 +1,99 @@
+// 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
+ *
+ * bitmap.c - bitmap handling
+ *
+ * Copyright Red Hat
+ * Author: Stefano Brivio <sbrivio@redhat.com>
+ */
+
+#include "bitmap.h"
+
+/**
+ * bitmap_set() - Set single bit in bitmap
+ * @map:	Pointer to bitmap
+ * @bit:	Bit number to set
+ */
+void bitmap_set(uint8_t *map, unsigned bit)
+{
+	unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit);
+
+	*word |= BITMAP_BIT(bit);
+}
+
+/**
+ * bitmap_clear() - Clear single bit in bitmap
+ * @map:	Pointer to bitmap
+ * @bit:	Bit number to clear
+ */
+/* cppcheck-suppress unusedFunction */
+void bitmap_clear(uint8_t *map, unsigned bit)
+{
+	unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit);
+
+	*word &= ~BITMAP_BIT(bit);
+}
+
+/**
+ * bitmap_isset() - Check for set bit in bitmap
+ * @map:	Pointer to bitmap
+ * @bit:	Bit number to check
+ *
+ * Return: true if given bit is set, false if it's not
+ */
+bool bitmap_isset(const uint8_t *map, unsigned bit)
+{
+	const unsigned long *word
+		= (const unsigned long *)map + BITMAP_WORD(bit);
+
+	return !!(*word & BITMAP_BIT(bit));
+}
+
+/**
+ * bitmap_or() - Logical disjunction (OR) of two bitmaps
+ * @dst:	Pointer to result bitmap
+ * @size:	Size of bitmaps, in bytes
+ * @a:		First operand
+ * @b:		Second operand
+ */
+/* cppcheck-suppress unusedFunction */
+void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
+{
+	unsigned long *dw = (unsigned long *)dst;
+	unsigned long *aw = (unsigned long *)a;
+	unsigned long *bw = (unsigned long *)b;
+	size_t i;
+
+	for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++)
+		*dw = *aw | *bw;
+
+	for (i = size / sizeof(long) * sizeof(long); i < size; i++)
+		dst[i] = a[i] | b[i];
+}
+
+/**
+ * bitmap_and_not() - Logical conjunction with complement (AND NOT) of bitmap
+ * @dst:	Pointer to result bitmap
+ * @size:	Size of bitmaps, in bytes
+ * @a:		First operand
+ * @b:		Second operand
+ */
+void bitmap_and_not(uint8_t *dst, size_t size,
+		   const uint8_t *a, const uint8_t *b)
+{
+	unsigned long *dw = (unsigned long *)dst;
+	unsigned long *aw = (unsigned long *)a;
+	unsigned long *bw = (unsigned long *)b;
+	size_t i;
+
+	for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++)
+		*dw = *aw & ~*bw;
+
+	for (i = size / sizeof(long) * sizeof(long); i < size; i++)
+		dst[i] = a[i] & ~b[i];
+}
diff --git a/bitmap.h b/bitmap.h
new file mode 100644
index 00000000..b6e91c02
--- /dev/null
+++ b/bitmap.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright Red Hat
+ * Author: Stefano Brivio <sbrivio@redhat.com>
+ */
+
+#ifndef BITMAP_H
+#define BITMAP_H
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+
+#define BIT(n)			(1UL << (n))
+#define BITMAP_BIT(n)		(BIT((n) % (sizeof(long) * 8)))
+#define BITMAP_WORD(n)		(n / (sizeof(long) * 8))
+
+void bitmap_set(uint8_t *map, unsigned bit);
+void bitmap_clear(uint8_t *map, unsigned bit);
+bool bitmap_isset(const uint8_t *map, unsigned bit);
+void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b);
+void bitmap_and_not(uint8_t *dst, size_t size,
+		    const uint8_t *a, const uint8_t *b);
+
+#endif /* BITMAP_H */
diff --git a/conf.c b/conf.c
index 5639ef03..ae37bf96 100644
--- a/conf.c
+++ b/conf.c
@@ -36,6 +36,7 @@
 #include <netinet/if_ether.h>
 
 #include "util.h"
+#include "bitmap.h"
 #include "ip.h"
 #include "passt.h"
 #include "netlink.h"
diff --git a/fwd.h b/fwd.h
index f111e139..beba0bf5 100644
--- a/fwd.h
+++ b/fwd.h
@@ -14,6 +14,7 @@
 
 #include <netinet/in.h>
 
+#include "bitmap.h"
 #include "inany.h"
 
 struct flowside;
diff --git a/util.c b/util.c
index faa2c6a4..73c9d51d 100644
--- a/util.c
+++ b/util.c
@@ -367,90 +367,6 @@ long timespec_diff_ms(const struct timespec *a, const struct timespec *b)
 	return timespec_diff_us(a, b) / 1000;
 }
 
-/**
- * bitmap_set() - Set single bit in bitmap
- * @map:	Pointer to bitmap
- * @bit:	Bit number to set
- */
-void bitmap_set(uint8_t *map, unsigned bit)
-{
-	unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit);
-
-	*word |= BITMAP_BIT(bit);
-}
-
-/**
- * bitmap_clear() - Clear single bit in bitmap
- * @map:	Pointer to bitmap
- * @bit:	Bit number to clear
- */
-/* cppcheck-suppress unusedFunction */
-void bitmap_clear(uint8_t *map, unsigned bit)
-{
-	unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit);
-
-	*word &= ~BITMAP_BIT(bit);
-}
-
-/**
- * bitmap_isset() - Check for set bit in bitmap
- * @map:	Pointer to bitmap
- * @bit:	Bit number to check
- *
- * Return: true if given bit is set, false if it's not
- */
-bool bitmap_isset(const uint8_t *map, unsigned bit)
-{
-	const unsigned long *word
-		= (const unsigned long *)map + BITMAP_WORD(bit);
-
-	return !!(*word & BITMAP_BIT(bit));
-}
-
-/**
- * bitmap_or() - Logical disjunction (OR) of two bitmaps
- * @dst:	Pointer to result bitmap
- * @size:	Size of bitmaps, in bytes
- * @a:		First operand
- * @b:		Second operand
- */
-/* cppcheck-suppress unusedFunction */
-void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
-{
-	unsigned long *dw = (unsigned long *)dst;
-	unsigned long *aw = (unsigned long *)a;
-	unsigned long *bw = (unsigned long *)b;
-	size_t i;
-
-	for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++)
-		*dw = *aw | *bw;
-
-	for (i = size / sizeof(long) * sizeof(long); i < size; i++)
-		dst[i] = a[i] | b[i];
-}
-
-/**
- * bitmap_and_not() - Logical conjunction with complement (AND NOT) of bitmap
- * @dst:	Pointer to result bitmap
- * @size:	Size of bitmaps, in bytes
- * @a:		First operand
- * @b:		Second operand
- */
-void bitmap_and_not(uint8_t *dst, size_t size,
-		   const uint8_t *a, const uint8_t *b)
-{
-	unsigned long *dw = (unsigned long *)dst;
-	unsigned long *aw = (unsigned long *)a;
-	unsigned long *bw = (unsigned long *)b;
-	size_t i;
-
-	for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++)
-		*dw = *aw & ~*bw;
-
-	for (i = size / sizeof(long) * sizeof(long); i < size; i++)
-		dst[i] = a[i] & ~b[i];
-}
-
 /**
  * ns_enter() - Enter configured user (unless already joined) and network ns
  * @c:		Execution context
diff --git a/util.h b/util.h
index cb669105..92aeabc8 100644
--- a/util.h
+++ b/util.h
@@ -50,10 +50,6 @@
 
 #define MAX_FROM_BITS(n)	(((1U << (n)) - 1))
 
-#define BIT(n)			(1UL << (n))
-#define BITMAP_BIT(n)		(BIT((n) % (sizeof(long) * 8)))
-#define BITMAP_WORD(n)		(n / (sizeof(long) * 8))
-
 #define SWAP(a, b)							\
 	do {								\
 		__typeof__(a) __x = (a); (a) = (b); (b) = __x;		\
@@ -228,12 +224,6 @@ int sock_unix(char *sock_path);
 void sock_probe_features(struct ctx *c);
 long timespec_diff_ms(const struct timespec *a, const struct timespec *b);
 int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b);
-void bitmap_set(uint8_t *map, unsigned bit);
-void bitmap_clear(uint8_t *map, unsigned bit);
-bool bitmap_isset(const uint8_t *map, unsigned bit);
-void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b);
-void bitmap_and_not(uint8_t *dst, size_t size,
-		    const uint8_t *a, const uint8_t *b);
 char *line_read(char *buf, size_t len, int fd);
 void ns_enter(const struct ctx *c);
 bool ns_is_init(void);
-- 
2.53.0

[PATCH 18/18] fwd: Split forwading rule specification from its implementation state

[David Gibson]

Most of the fields in struct fwd_rule give the parameters of the forwarding
rule itself.  The @socks field gives the list of listening sockets we use
to implement it.  In order to share code with the configuraiton update tool
pesto, we want to split these.  Move the rule specification itself into
fwd_rule.h, which can be shared with pesto.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile   | 12 +++++-----
 fwd.c      | 66 ++++++++++++++++++++++++++++--------------------------
 fwd.h      | 31 +++++--------------------
 fwd_rule.h | 44 ++++++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 63 deletions(-)
 create mode 100644 fwd_rule.h

diff --git a/Makefile b/Makefile
index d6ced328..dd647b10 100644
--- a/Makefile
+++ b/Makefile
@@ -50,12 +50,12 @@ SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS)
 MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1
 
 PASST_HEADERS = arch.h arp.h bitmap.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 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
+	epoll_ctl.h flow.h fwd.h fwd_rule.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 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/fwd.c b/fwd.c
index 62d344c3..e09b42fe 100644
--- a/fwd.c
+++ b/fwd.c
@@ -363,7 +363,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 	/* Flags which can be set from the caller */
 	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
 	unsigned num = (unsigned)last - first + 1;
-	struct fwd_rule *new;
+	struct fwd_rule_state *new;
 	unsigned i, port;
 
 	assert(!(flags & ~allowed_flags));
@@ -379,7 +379,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 	/* Check for any conflicting entries */
 	for (i = 0; i < fwd->count; i++) {
 		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
-		const struct fwd_rule *rule = &fwd->rules[i];
+		const struct fwd_rule *rule = &fwd->rules[i].rule;
 
 		if (proto != rule->proto)
 			/* Non-conflicting protocols */
@@ -400,36 +400,37 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags,
 	}
 
 	new = &fwd->rules[fwd->count++];
-	new->proto = proto;
-	new->flags = flags;
+	new->rule.proto = proto;
+	new->rule.flags = flags;
 
 	if (addr) {
-		new->addr = *addr;
+		new->rule.addr = *addr;
 	} else {
-		new->addr = inany_any6;
-		new->flags |= FWD_DUAL_STACK_ANY;
+		new->rule.addr = inany_any6;
+		new->rule.flags |= FWD_DUAL_STACK_ANY;
 	}
 
-	memset(new->ifname, 0, sizeof(new->ifname));
+	memset(new->rule.ifname, 0, sizeof(new->rule.ifname));
 	if (ifname) {
 		int ret;
 
-		ret = snprintf(new->ifname, sizeof(new->ifname), "%s", ifname);
-		if (ret <= 0 || (size_t)ret >= sizeof(new->ifname))
+		ret = snprintf(new->rule.ifname, sizeof(new->rule.ifname),
+			       "%s", ifname);
+		if (ret <= 0 || (size_t)ret >= sizeof(new->rule.ifname))
 			die("Invalid interface name: %s", ifname);
 	}
 
 	assert(first <= last);
-	new->first = first;
-	new->last = last;
+	new->rule.first = first;
+	new->rule.last = last;
 
-	new->to = to;
+	new->rule.to = to;
 
 	new->socks = &fwd->socks[fwd->sock_count];
 	fwd->sock_count += num;
 
-	for (port = new->first; port <= new->last; port++)
-		new->socks[port - new->first] = -1;
+	for (port = new->rule.first; port <= new->rule.last; port++)
+		new->socks[port - new->rule.first] = -1;
 }
 
 /**
@@ -465,7 +466,7 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
 
 	if (hint >= 0) {
 		char ostr[INANY_ADDRSTRLEN], rstr[INANY_ADDRSTRLEN];
-		const struct fwd_rule *rule = &fwd->rules[hint];
+		const struct fwd_rule *rule = &fwd->rules[hint].rule;
 
 		assert((unsigned)hint < fwd->count);
 		if (fwd_rule_match(rule, ini, proto))
@@ -479,8 +480,8 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd,
 	}
 
 	for (i = 0; i < fwd->count; i++) {
-		if (fwd_rule_match(&fwd->rules[i], ini, proto))
-			return &fwd->rules[i];
+		if (fwd_rule_match(&fwd->rules[i].rule, ini, proto))
+			return &fwd->rules[i].rule;
 	}
 
 	return NULL;
@@ -495,7 +496,7 @@ void fwd_rules_print(const struct fwd_table *fwd)
 	unsigned i;
 
 	for (i = 0; i < fwd->count; i++) {
-		const struct fwd_rule *rule = &fwd->rules[i];
+		const struct fwd_rule *rule = &fwd->rules[i].rule;
 		const char *percent = *rule->ifname ? "%" : "";
 		const char *weak = "", *scan = "";
 		char addr[INANY_ADDRSTRLEN];
@@ -532,7 +533,8 @@ void fwd_rules_print(const struct fwd_table *fwd)
 static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
 			const uint8_t *tcp, const uint8_t *udp)
 {
-	const struct fwd_rule *rule = &c->fwd[pif]->rules[idx];
+	const struct fwd_rule_state *rs = &c->fwd[pif]->rules[idx];
+	const struct fwd_rule *rule = &rs->rule;
 	const union inany_addr *addr = fwd_rule_addr(rule);
 	const char *ifname = rule->ifname;
 	const uint8_t *map = NULL;
@@ -553,7 +555,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
 	}
 
 	for (port = rule->first; port <= rule->last; port++) {
-		int fd = rule->socks[port - rule->first];
+		int fd = rs->socks[port - rule->first];
 
 		if (map && !bitmap_isset(map, port)) {
 			/* We don't want to listen on this port */
@@ -561,7 +563,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
 				/* We already are, so stop */
 				epoll_del(c->epollfd, fd);
 				close(fd);
-				rule->socks[port - rule->first] = -1;
+				rs->socks[port - rule->first] = -1;
 			}
 			continue;
 		}
@@ -593,7 +595,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx,
 			continue;
 		}
 
-		rule->socks[port - rule->first] = fd;
+		rs->socks[port - rule->first] = fd;
 		bound_one = true;
 	}
 
@@ -683,11 +685,11 @@ void fwd_listen_close(const struct fwd_table *fwd)
 	unsigned i;
 
 	for (i = 0; i < fwd->count; i++) {
-		const struct fwd_rule *rule = &fwd->rules[i];
+		const struct fwd_rule_state *rs = &fwd->rules[i];
 		unsigned port;
 
-		for (port = rule->first; port <= rule->last; port++) {
-			int *fdp = &rule->socks[port - rule->first];
+		for (port = rs->rule.first; port <= rs->rule.last; port++) {
+			int *fdp = &rs->socks[port - rs->rule.first];
 			if (*fdp >= 0) {
 				close(*fdp);
 				*fdp = -1;
@@ -767,8 +769,8 @@ static bool has_scan_rules(const struct fwd_table *fwd, uint8_t proto)
 	unsigned i;
 
 	for (i = 0; i < fwd->count; i++) {
-		if (fwd->rules[i].proto == proto &&
-		    fwd->rules[i].flags & FWD_SCAN)
+		if (fwd->rules[i].rule.proto == proto &&
+		    fwd->rules[i].rule.flags & FWD_SCAN)
 			return true;
 	}
 	return false;
@@ -836,14 +838,14 @@ static void current_listen_map(uint8_t *map, const struct fwd_table *fwd,
 	memset(map, 0, PORT_BITMAP_SIZE);
 
 	for (i = 0; i < fwd->count; i++) {
-		const struct fwd_rule *rule = &fwd->rules[i];
+		const struct fwd_rule_state *rs = &fwd->rules[i];
 		unsigned port;
 
-		if (rule->proto != proto)
+		if (rs->rule.proto != proto)
 			continue;
 
-		for (port = rule->first; port <= rule->last; port++) {
-			if (rule->socks[port - rule->first] >= 0)
+		for (port = rs->rule.first; port <= rs->rule.last; port++) {
+			if (rs->socks[port - rs->rule.first] >= 0)
 				bitmap_set(map, port);
 		}
 	}
diff --git a/fwd.h b/fwd.h
index beba0bf5..33600cbf 100644
--- a/fwd.h
+++ b/fwd.h
@@ -16,6 +16,7 @@
 
 #include "bitmap.h"
 #include "inany.h"
+#include "fwd_rule.h"
 
 struct flowside;
 
@@ -26,32 +27,12 @@ void fwd_probe_ephemeral(void);
 void fwd_port_map_ephemeral(uint8_t *map);
 
 /**
- * struct fwd_rule - Forwarding rule governing a range of ports
- * @addr:	Address to forward from
- * @ifname:	Interface to forward from
- * @first:	First port number to forward
- * @last:	Last port number to forward
- * @to:		Target port for @first, port n goes to @to + (n - @first)
- * @proto:	Protocol to forward
- * @flags:	Flag mask
- * 	FWD_DUAL_STACK_ANY - match any IPv4 or IPv6 address (@addr should be ::)
- *	FWD_WEAK - Don't give an error if binds fail for some forwards
- *	FWD_SCAN - Only forward if the matching port in the target is listening
+ * struct fwd_rule_state - Forwarding rule and associated state
+ * @rule:	Rule specification
  * @socks:	Array of listening sockets for this entry
- *
- * FIXME: @addr and @ifname currently ignored for outbound tables
  */
-struct fwd_rule {
-	union inany_addr addr;
-	char ifname[IFNAMSIZ];
-	in_port_t first;
-	in_port_t last;
-	in_port_t to;
-	uint8_t proto;
-#define FWD_DUAL_STACK_ANY	BIT(0)
-#define FWD_WEAK		BIT(1)
-#define FWD_SCAN		BIT(2)
-	uint8_t flags;
+struct fwd_rule_state {
+	struct fwd_rule rule;
 	int *socks;
 };
 
@@ -88,7 +69,7 @@ struct fwd_listen_ref {
  */
 struct fwd_table {
 	unsigned count;
-	struct fwd_rule rules[MAX_FWD_RULES];
+	struct fwd_rule_state rules[MAX_FWD_RULES];
 	unsigned sock_count;
 	int socks[MAX_LISTEN_SOCKS];
 };
diff --git a/fwd_rule.h b/fwd_rule.h
new file mode 100644
index 00000000..ae0e1d61
--- /dev/null
+++ b/fwd_rule.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright Red Hat
+ * Author: Stefano Brivio <sbrivio@redhat.com>
+ * Author: David Gibson <david@gibson.dropbear.id.au>
+ *
+ * Forwarding rule definitions shared between passt/pasta and pesto
+ */
+
+#ifndef FWD_RULE_H
+#define FWD_RULE_H
+
+#include <stdint.h>
+#include <net/if.h>
+#include <netinet/in.h>
+
+#include "inany.h"
+
+/**
+ * struct fwd_rule - Forwarding rule governing a range of ports
+ * @addr:	Address to forward from
+ * @ifname:	Interface to forward from
+ * @first:	First port number to forward
+ * @last:	Last port number to forward
+ * @to:		Target port for @first, port n goes to @to + (n - @first)
+ * @proto:	Protocol to forward
+ * @flags:	Flag mask
+ * 	FWD_DUAL_STACK_ANY - match any IPv4 or IPv6 address (@addr should be ::)
+ *	FWD_WEAK - Don't give an error if binds fail for some forwards
+ *	FWD_SCAN - Only forward if the matching port in the target is listening
+ */
+struct fwd_rule {
+	union inany_addr addr;
+	char ifname[IFNAMSIZ];
+	in_port_t first;
+	in_port_t last;
+	in_port_t to;
+	uint8_t proto;
+#define FWD_DUAL_STACK_ANY	BIT(0)
+#define FWD_WEAK		BIT(1)
+#define FWD_SCAN		BIT(2)
+	uint8_t flags;
+};
+
+#endif /* FWD_RULE_H */
-- 
2.53.0

Re: [PATCH 15/18] pif: Limit pif names to IFNAMSIZ (16) bytes

[Stefano Brivio]

On Fri, 27 Mar 2026 15:34:27 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> All current pif names are quite short, and we expect them to remain short
> when/if we allow arbitrary pifs.  However, because of the structure of
> the current code we don't enforce any limit on the length.
> 
> This will become more important with dynamic configuration updates, so
> start enforcing a length limit.  Specifically we allow pif names to be up
> to IFNAMSIZ bytes, including the terminating \0.  This is semi-arbitrary -
> there's no particular reason we have to use the same length limit as
> kernel netif names.  However, when we do allow arbitrary pifs, we expect
> that we might support a similar number to the number of kernel interfaces.
> It might make sense to use names matching kernel interface names in that
> future.  So, re-use IFNAMSIZ to avoid surprise.

And what if... we used 128 instead, which is reasonably longer than
UNIX_PATH_MAX (108, which despite the application usage in POSIX 2024.1:
https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/sys_un.h.html
is still commonly used a path length limit, also by passt itself)?

At that point we could embed UNIX domain socket paths as PIF name
(possibly with some additional specifier) which _might_ be useful to
forward UNIX sockets in the https://bugs.passt.top/show_bug.cgi?id=200
sense.

It's not the only way to implement it, but perhaps it's one possibility
that might make sense for what we know now? What do you think?

It also has the advantage of being sufficiently longer than IFNAMSIZ,
so that should we ever need to have stuff like "container_A:eth0" in a
PIF name, we could have it as well.

-- 
Stefano

Re: [PATCH 00/18] More pesto preliminaries

[Stefano Brivio]

On Fri, 27 Mar 2026 15:34:12 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> The number of preliminary patches in the pesto series has grown to the
> point that I think it's worth trying to get a bunch merged, while we
> polish the rest.
> 
> Changes from pesto series v3
>  * Extracted just the preliminary patches, not introducing pesto
>    itself
>  * Several small revisions based on Stefano's feedback
>  * Added several extra bits of preliminary rework.

Applied, except for 15/18 (see pending question), with two small typos
in 18/18's message fixed (forwarding, configuration).

-- 
Stefano