[PATCH v2 0/9] Clean ups to automatic port forwarding

[PATCH v2 0/9] Clean ups to automatic port forwarding

[David Gibson]

This contains an assortment of cleanups to the code supporting the
automatic port forwarding more for past - specifically
get_bound_ports() and related functions.  This shouldn't mae any
functional changes.

Based on the series with fixes for new cppcheck-2.12 warnings.

Changes since v1:
 * Some comment tweaks based on Stefano's feedback.

David Gibson (9):
  conf: Cleaner initialisation of default forwarding modes
  port_fwd: Move automatic port forwarding code to port_fwd.[ch]
  port_fwd: Better parameterise procfs_scan_listen()
  util: Add open_in_ns() helper
  port_fwd: Pre-open /proc/net/* files rather than on-demand
  port_fwd: Don't NS_CALL get_bound_ports()
  port_fwd: Split TCP and UDP cases for get_bound_ports()
  port_fwd: Move port scanning /proc fds into struct port_fwd
  port_fwd: Simplify get_bound_ports_*() to port_fwd_scan_*()

 Makefile   |   2 +-
 conf.c     | 113 +++++--------------------------------------
 conf.h     |   1 -
 passt.h    |   5 --
 port_fwd.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 port_fwd.h |   9 ++++
 tcp.c      |  39 +--------------
 util.c     | 118 +++++++++++++++++++++------------------------
 util.h     |   3 +-
 9 files changed, 218 insertions(+), 211 deletions(-)
 create mode 100644 port_fwd.c

-- 
2.41.0

[PATCH v2 1/9] conf: Cleaner initialisation of default forwarding modes

[David Gibson]

Initialisation of the forwarding mode variables is complicated a bit by the
fact that we have different defaults for passt and pasta modes.  This leads
to some debateably duplicated code between those two cases.

More significantly, however, currently the final setting of the mode
variable is interleaved with the code to actually start automatic scanning
when that's selected.  This essentially mingles "policy" code (setting the
default mode), with implementation code (making that happen).  That's a bit
inflexible if we want to change policies in future.

Disentangle these two pieces, and use a slightly different construction to
make things briefer as well.

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

diff --git a/conf.c b/conf.c
index a235b31..4d37af1 100644
--- a/conf.c
+++ b/conf.c
@@ -1238,6 +1238,7 @@ void conf(struct ctx *c, int argc, char **argv)
 	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
 	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
 	bool copy_addrs_opt = false, copy_routes_opt = false;
+	enum port_fwd_mode fwd_default = FWD_NONE;
 	bool v4_only = false, v6_only = false;
 	char *runas = NULL, *logfile = NULL;
 	struct in6_addr *dns6 = c->ip6.dns;
@@ -1252,6 +1253,7 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	if (c->mode == MODE_PASTA) {
 		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
+		fwd_default = FWD_AUTO;
 		optstring = "dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:46t:u:T:U:";
 	} else {
 		optstring = "dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:461t:u:";
@@ -1803,40 +1805,32 @@ void conf(struct ctx *c, int argc, char **argv)
 			if_indextoname(c->ifi6, c->pasta_ifn);
 	}
 
-	if (c->mode == MODE_PASTA) {
-		c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
-		c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
-		c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1;
-		c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
-
-		if (!c->tcp.fwd_in.mode || c->tcp.fwd_in.mode == FWD_AUTO) {
-			c->tcp.fwd_in.mode = FWD_AUTO;
-			ns_ports_arg.proto = IPPROTO_TCP;
-			NS_CALL(get_bound_ports_ns, &ns_ports_arg);
-		}
-		if (!c->udp.fwd_in.f.mode || c->udp.fwd_in.f.mode == FWD_AUTO) {
-			c->udp.fwd_in.f.mode = FWD_AUTO;
-			ns_ports_arg.proto = IPPROTO_UDP;
-			NS_CALL(get_bound_ports_ns, &ns_ports_arg);
-		}
-		if (!c->tcp.fwd_out.mode || c->tcp.fwd_out.mode == FWD_AUTO) {
-			c->tcp.fwd_out.mode = FWD_AUTO;
-			get_bound_ports(c, 0, IPPROTO_TCP);
-		}
-		if (!c->udp.fwd_out.f.mode || c->udp.fwd_out.f.mode == FWD_AUTO) {
-			c->udp.fwd_out.f.mode = FWD_AUTO;
-			get_bound_ports(c, 0, IPPROTO_UDP);
-		}
-	} else {
-		if (!c->tcp.fwd_in.mode)
-			c->tcp.fwd_in.mode = FWD_NONE;
-		if (!c->tcp.fwd_out.mode)
-			c->tcp.fwd_out.mode = FWD_NONE;
-		if (!c->udp.fwd_in.f.mode)
-			c->udp.fwd_in.f.mode = FWD_NONE;
-		if (!c->udp.fwd_out.f.mode)
-			c->udp.fwd_out.f.mode = FWD_NONE;
+	if (!c->tcp.fwd_in.mode)
+		c->tcp.fwd_in.mode = fwd_default;
+	if (!c->tcp.fwd_out.mode)
+		c->tcp.fwd_out.mode = fwd_default;
+	if (!c->udp.fwd_in.f.mode)
+		c->udp.fwd_in.f.mode = fwd_default;
+	if (!c->udp.fwd_out.f.mode)
+		c->udp.fwd_out.f.mode = fwd_default;
+
+	c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
+	c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
+	c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1;
+	c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
+
+	if (c->tcp.fwd_in.mode == FWD_AUTO) {
+		ns_ports_arg.proto = IPPROTO_TCP;
+		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
+	}
+	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
+		ns_ports_arg.proto = IPPROTO_UDP;
+		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
 	}
+	if (c->tcp.fwd_out.mode == FWD_AUTO)
+		get_bound_ports(c, 0, IPPROTO_TCP);
+	if (c->udp.fwd_out.f.mode == FWD_AUTO)
+		get_bound_ports(c, 0, IPPROTO_UDP);
 
 	if (!c->quiet)
 		conf_print(c);
-- 
@@ -1238,6 +1238,7 @@ void conf(struct ctx *c, int argc, char **argv)
 	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
 	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
 	bool copy_addrs_opt = false, copy_routes_opt = false;
+	enum port_fwd_mode fwd_default = FWD_NONE;
 	bool v4_only = false, v6_only = false;
 	char *runas = NULL, *logfile = NULL;
 	struct in6_addr *dns6 = c->ip6.dns;
@@ -1252,6 +1253,7 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	if (c->mode == MODE_PASTA) {
 		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
+		fwd_default = FWD_AUTO;
 		optstring = "dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:46t:u:T:U:";
 	} else {
 		optstring = "dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:461t:u:";
@@ -1803,40 +1805,32 @@ void conf(struct ctx *c, int argc, char **argv)
 			if_indextoname(c->ifi6, c->pasta_ifn);
 	}
 
-	if (c->mode == MODE_PASTA) {
-		c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
-		c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
-		c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1;
-		c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
-
-		if (!c->tcp.fwd_in.mode || c->tcp.fwd_in.mode == FWD_AUTO) {
-			c->tcp.fwd_in.mode = FWD_AUTO;
-			ns_ports_arg.proto = IPPROTO_TCP;
-			NS_CALL(get_bound_ports_ns, &ns_ports_arg);
-		}
-		if (!c->udp.fwd_in.f.mode || c->udp.fwd_in.f.mode == FWD_AUTO) {
-			c->udp.fwd_in.f.mode = FWD_AUTO;
-			ns_ports_arg.proto = IPPROTO_UDP;
-			NS_CALL(get_bound_ports_ns, &ns_ports_arg);
-		}
-		if (!c->tcp.fwd_out.mode || c->tcp.fwd_out.mode == FWD_AUTO) {
-			c->tcp.fwd_out.mode = FWD_AUTO;
-			get_bound_ports(c, 0, IPPROTO_TCP);
-		}
-		if (!c->udp.fwd_out.f.mode || c->udp.fwd_out.f.mode == FWD_AUTO) {
-			c->udp.fwd_out.f.mode = FWD_AUTO;
-			get_bound_ports(c, 0, IPPROTO_UDP);
-		}
-	} else {
-		if (!c->tcp.fwd_in.mode)
-			c->tcp.fwd_in.mode = FWD_NONE;
-		if (!c->tcp.fwd_out.mode)
-			c->tcp.fwd_out.mode = FWD_NONE;
-		if (!c->udp.fwd_in.f.mode)
-			c->udp.fwd_in.f.mode = FWD_NONE;
-		if (!c->udp.fwd_out.f.mode)
-			c->udp.fwd_out.f.mode = FWD_NONE;
+	if (!c->tcp.fwd_in.mode)
+		c->tcp.fwd_in.mode = fwd_default;
+	if (!c->tcp.fwd_out.mode)
+		c->tcp.fwd_out.mode = fwd_default;
+	if (!c->udp.fwd_in.f.mode)
+		c->udp.fwd_in.f.mode = fwd_default;
+	if (!c->udp.fwd_out.f.mode)
+		c->udp.fwd_out.f.mode = fwd_default;
+
+	c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
+	c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
+	c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1;
+	c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
+
+	if (c->tcp.fwd_in.mode == FWD_AUTO) {
+		ns_ports_arg.proto = IPPROTO_TCP;
+		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
+	}
+	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
+		ns_ports_arg.proto = IPPROTO_UDP;
+		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
 	}
+	if (c->tcp.fwd_out.mode == FWD_AUTO)
+		get_bound_ports(c, 0, IPPROTO_TCP);
+	if (c->udp.fwd_out.f.mode == FWD_AUTO)
+		get_bound_ports(c, 0, IPPROTO_UDP);
 
 	if (!c->quiet)
 		conf_print(c);
-- 
2.41.0

[PATCH v2 2/9] port_fwd: Move automatic port forwarding code to port_fwd.[ch]

[David Gibson]

The implementation of scanning /proc files to do automatic port forwarding
is a bit awkwardly split between procfs_scan_listen() in util.c,
get_bound_ports() and related functions in conf.c and the initial setup
code in conf().

Consolidate all of this into port_fwd.h, which already has some related
definitions, and a new port_fwd.c.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile   |   2 +-
 conf.c     |  85 +------------------------
 conf.h     |   1 -
 port_fwd.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 port_fwd.h |   3 +
 tcp.c      |   1 -
 util.c     |  65 +------------------
 util.h     |   2 -
 8 files changed, 185 insertions(+), 153 deletions(-)
 create mode 100644 port_fwd.c

diff --git a/Makefile b/Makefile
index 941086a..0324fdd 100644
--- a/Makefile
+++ b/Makefile
@@ -45,7 +45,7 @@ FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
 
 PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
 	isolation.c lineread.c log.c mld.c ndp.c netlink.c packet.c passt.c \
-	pasta.c pcap.c tap.c tcp.c tcp_splice.c udp.c util.c
+	pasta.c pcap.c port_fwd.c tap.c tcp.c tcp_splice.c udp.c util.c
 QRAP_SRCS = qrap.c
 SRCS = $(PASST_SRCS) $(QRAP_SRCS)
 
diff --git a/conf.c b/conf.c
index 4d37af1..d3e6eb2 100644
--- a/conf.c
+++ b/conf.c
@@ -44,72 +44,6 @@
 #include "isolation.h"
 #include "log.h"
 
-/**
- * get_bound_ports() - Get maps of ports with bound sockets
- * @c:		Execution context
- * @ns:		If set, set bitmaps for ports to tap/ns -- to init otherwise
- * @proto:	Protocol number (IPPROTO_TCP or IPPROTO_UDP)
- */
-void get_bound_ports(struct ctx *c, int ns, uint8_t proto)
-{
-	uint8_t *udp_map, *udp_excl, *tcp_map, *tcp_excl;
-
-	if (ns) {
-		udp_map = c->udp.fwd_in.f.map;
-		udp_excl = c->udp.fwd_out.f.map;
-		tcp_map = c->tcp.fwd_in.map;
-		tcp_excl = c->tcp.fwd_out.map;
-	} else {
-		udp_map = c->udp.fwd_out.f.map;
-		udp_excl = c->udp.fwd_in.f.map;
-		tcp_map = c->tcp.fwd_out.map;
-		tcp_excl = c->tcp.fwd_in.map;
-	}
-
-	if (proto == IPPROTO_UDP) {
-		memset(udp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(c, IPPROTO_UDP, V4, ns, udp_map, udp_excl);
-		procfs_scan_listen(c, IPPROTO_UDP, V6, ns, udp_map, udp_excl);
-
-		procfs_scan_listen(c, IPPROTO_TCP, V4, ns, udp_map, udp_excl);
-		procfs_scan_listen(c, IPPROTO_TCP, V6, ns, udp_map, udp_excl);
-	} else if (proto == IPPROTO_TCP) {
-		memset(tcp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(c, IPPROTO_TCP, V4, ns, tcp_map, tcp_excl);
-		procfs_scan_listen(c, IPPROTO_TCP, V6, ns, tcp_map, tcp_excl);
-	}
-}
-
-/**
- * struct get_bound_ports_ns_arg - Arguments for get_bound_ports_ns()
- * @c:		Execution context
- * @proto:	Protocol number (IPPROTO_TCP or IPPROTO_UDP)
- */
-struct get_bound_ports_ns_arg {
-	struct ctx *c;
-	uint8_t proto;
-};
-
-/**
- * get_bound_ports_ns() - Get maps of ports in namespace with bound sockets
- * @arg:	See struct get_bound_ports_ns_arg
- *
- * Return: 0
- */
-static int get_bound_ports_ns(void *arg)
-{
-	struct get_bound_ports_ns_arg *a = (struct get_bound_ports_ns_arg *)arg;
-	struct ctx *c = a->c;
-
-	if (!c->pasta_netns_fd)
-		return 0;
-
-	ns_enter(c);
-	get_bound_ports(c, 1, a->proto);
-
-	return 0;
-}
-
 /**
  * next_chunk - Return the next piece of a string delimited by a character
  * @s:		String to search
@@ -1235,7 +1169,6 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"no-copy-addrs", no_argument,		NULL,		19 },
 		{ 0 },
 	};
-	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
 	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
 	bool copy_addrs_opt = false, copy_routes_opt = false;
 	enum port_fwd_mode fwd_default = FWD_NONE;
@@ -1814,23 +1747,7 @@ void conf(struct ctx *c, int argc, char **argv)
 	if (!c->udp.fwd_out.f.mode)
 		c->udp.fwd_out.f.mode = fwd_default;
 
-	c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
-	c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
-	c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1;
-	c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
-
-	if (c->tcp.fwd_in.mode == FWD_AUTO) {
-		ns_ports_arg.proto = IPPROTO_TCP;
-		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
-	}
-	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
-		ns_ports_arg.proto = IPPROTO_UDP;
-		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
-	}
-	if (c->tcp.fwd_out.mode == FWD_AUTO)
-		get_bound_ports(c, 0, IPPROTO_TCP);
-	if (c->udp.fwd_out.f.mode == FWD_AUTO)
-		get_bound_ports(c, 0, IPPROTO_UDP);
+	port_fwd_init(c);
 
 	if (!c->quiet)
 		conf_print(c);
diff --git a/conf.h b/conf.h
index ce7b8c5..9d2143d 100644
--- a/conf.h
+++ b/conf.h
@@ -7,6 +7,5 @@
 #define CONF_H
 
 void conf(struct ctx *c, int argc, char **argv);
-void get_bound_ports(struct ctx *c, int ns, uint8_t proto);
 
 #endif /* CONF_H */
diff --git a/port_fwd.c b/port_fwd.c
new file mode 100644
index 0000000..136a450
--- /dev/null
+++ b/port_fwd.c
@@ -0,0 +1,179 @@
+// 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
+ *
+ * port_fwd.c - Port forwarding helpers
+ *
+ * Copyright Red Hat
+ * Author: Stefano Brivio <sbrivio@redhat.com>
+ * Author: David Gibson <david@gibson.dropbear.id.au>
+ */
+
+#include <stdint.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+
+#include "util.h"
+#include "port_fwd.h"
+#include "passt.h"
+#include "lineread.h"
+
+/**
+ * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs
+ * @proto:	IPPROTO_TCP or IPPROTO_UDP
+ * @ip_version:	IP version, V4 or V6
+ * @ns:		Use saved file descriptors for namespace if set
+ * @map:	Bitmap where numbers of ports in listening state will be set
+ * @exclude:	Bitmap of ports to exclude from setting (and clear)
+ *
+ * #syscalls:pasta lseek
+ * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
+ */
+static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
+			       int ns, uint8_t *map, const uint8_t *exclude)
+{
+	char *path, *line;
+	struct lineread lr;
+	unsigned long port;
+	unsigned int state;
+	int *fd;
+
+	if (proto == IPPROTO_TCP) {
+		fd = &c->proc_net_tcp[ip_version][ns];
+		if (ip_version == V4)
+			path = "/proc/net/tcp";
+		else
+			path = "/proc/net/tcp6";
+	} else {
+		fd = &c->proc_net_udp[ip_version][ns];
+		if (ip_version == V4)
+			path = "/proc/net/udp";
+		else
+			path = "/proc/net/udp6";
+	}
+
+	if (*fd != -1) {
+		if (lseek(*fd, 0, SEEK_SET)) {
+			warn("lseek() failed on %s: %s", path, strerror(errno));
+			return;
+		}
+	} else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) {
+		return;
+	}
+
+	lineread_init(&lr, *fd);
+	lineread_get(&lr, &line); /* throw away header */
+	while (lineread_get(&lr, &line) > 0) {
+		/* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */
+		if (sscanf(line, "%*u: %*x:%lx %*x:%*x %x", &port, &state) != 2)
+			continue;
+
+		/* See enum in kernel's include/net/tcp_states.h */
+		if ((proto == IPPROTO_TCP && state != 0x0a) ||
+		    (proto == IPPROTO_UDP && state != 0x07))
+			continue;
+
+		if (bitmap_isset(exclude, port))
+			bitmap_clear(map, port);
+		else
+			bitmap_set(map, port);
+	}
+}
+
+/**
+ * get_bound_ports() - Get maps of ports with bound sockets
+ * @c:		Execution context
+ * @ns:		If set, set bitmaps for ports to tap/ns -- to init otherwise
+ * @proto:	Protocol number (IPPROTO_TCP or IPPROTO_UDP)
+ */
+void get_bound_ports(struct ctx *c, int ns, uint8_t proto)
+{
+	uint8_t *udp_map, *udp_excl, *tcp_map, *tcp_excl;
+
+	if (ns) {
+		udp_map = c->udp.fwd_in.f.map;
+		udp_excl = c->udp.fwd_out.f.map;
+		tcp_map = c->tcp.fwd_in.map;
+		tcp_excl = c->tcp.fwd_out.map;
+	} else {
+		udp_map = c->udp.fwd_out.f.map;
+		udp_excl = c->udp.fwd_in.f.map;
+		tcp_map = c->tcp.fwd_out.map;
+		tcp_excl = c->tcp.fwd_in.map;
+	}
+
+	if (proto == IPPROTO_UDP) {
+		memset(udp_map, 0, PORT_BITMAP_SIZE);
+		procfs_scan_listen(c, IPPROTO_UDP, V4, ns, udp_map, udp_excl);
+		procfs_scan_listen(c, IPPROTO_UDP, V6, ns, udp_map, udp_excl);
+
+		procfs_scan_listen(c, IPPROTO_TCP, V4, ns, udp_map, udp_excl);
+		procfs_scan_listen(c, IPPROTO_TCP, V6, ns, udp_map, udp_excl);
+	} else if (proto == IPPROTO_TCP) {
+		memset(tcp_map, 0, PORT_BITMAP_SIZE);
+		procfs_scan_listen(c, IPPROTO_TCP, V4, ns, tcp_map, tcp_excl);
+		procfs_scan_listen(c, IPPROTO_TCP, V6, ns, tcp_map, tcp_excl);
+	}
+}
+
+/**
+ * struct get_bound_ports_ns_arg - Arguments for get_bound_ports_ns()
+ * @c:		Execution context
+ * @proto:	Protocol number (IPPROTO_TCP or IPPROTO_UDP)
+ */
+struct get_bound_ports_ns_arg {
+	struct ctx *c;
+	uint8_t proto;
+};
+
+/**
+ * get_bound_ports_ns() - Get maps of ports in namespace with bound sockets
+ * @arg:	See struct get_bound_ports_ns_arg
+ *
+ * Return: 0
+ */
+static int get_bound_ports_ns(void *arg)
+{
+	struct get_bound_ports_ns_arg *a = (struct get_bound_ports_ns_arg *)arg;
+	struct ctx *c = a->c;
+
+	if (!c->pasta_netns_fd)
+		return 0;
+
+	ns_enter(c);
+	get_bound_ports(c, 1, a->proto);
+
+	return 0;
+}
+
+/**
+ * port_fwd_init() - Initial setup for port forwarding
+ * @c:		Execution context
+ */
+void port_fwd_init(struct ctx *c)
+{
+	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
+
+	c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
+	c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
+	c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1;
+	c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
+
+	if (c->tcp.fwd_in.mode == FWD_AUTO) {
+		ns_ports_arg.proto = IPPROTO_TCP;
+		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
+	}
+	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
+		ns_ports_arg.proto = IPPROTO_UDP;
+		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
+	}
+	if (c->tcp.fwd_out.mode == FWD_AUTO)
+		get_bound_ports(c, 0, IPPROTO_TCP);
+	if (c->udp.fwd_out.f.mode == FWD_AUTO)
+		get_bound_ports(c, 0, IPPROTO_UDP);
+}
diff --git a/port_fwd.h b/port_fwd.h
index 6ed3f14..ad8ed1f 100644
--- a/port_fwd.h
+++ b/port_fwd.h
@@ -31,4 +31,7 @@ struct port_fwd {
 	in_port_t delta[NUM_PORTS];
 };
 
+void get_bound_ports(struct ctx *c, int ns, uint8_t proto);
+void port_fwd_init(struct ctx *c);
+
 #endif /* PORT_FWD_H */
diff --git a/tcp.c b/tcp.c
index 945023c..c6cc020 100644
--- a/tcp.c
+++ b/tcp.c
@@ -298,7 +298,6 @@
 #include "tap.h"
 #include "siphash.h"
 #include "pcap.h"
-#include "conf.h"
 #include "tcp_splice.h"
 #include "log.h"
 #include "inany.h"
diff --git a/util.c b/util.c
index 1f35382..a8f3b35 100644
--- a/util.c
+++ b/util.c
@@ -28,7 +28,6 @@
 #include "util.h"
 #include "passt.h"
 #include "packet.h"
-#include "lineread.h"
 #include "log.h"
 
 #define IPV6_NH_OPT(nh)							\
@@ -326,69 +325,7 @@ int bitmap_isset(const uint8_t *map, int bit)
 	return !!(*word & BITMAP_BIT(bit));
 }
 
-/**
- * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs
- * @proto:	IPPROTO_TCP or IPPROTO_UDP
- * @ip_version:	IP version, V4 or V6
- * @ns:		Use saved file descriptors for namespace if set
- * @map:	Bitmap where numbers of ports in listening state will be set
- * @exclude:	Bitmap of ports to exclude from setting (and clear)
- *
- * #syscalls:pasta lseek
- * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
- */
-void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns,
-			uint8_t *map, const uint8_t *exclude)
-{
-	char *path, *line;
-	struct lineread lr;
-	unsigned long port;
-	unsigned int state;
-	int *fd;
-
-	if (proto == IPPROTO_TCP) {
-		fd = &c->proc_net_tcp[ip_version][ns];
-		if (ip_version == V4)
-			path = "/proc/net/tcp";
-		else
-			path = "/proc/net/tcp6";
-	} else {
-		fd = &c->proc_net_udp[ip_version][ns];
-		if (ip_version == V4)
-			path = "/proc/net/udp";
-		else
-			path = "/proc/net/udp6";
-	}
-
-	if (*fd != -1) {
-		if (lseek(*fd, 0, SEEK_SET)) {
-			warn("lseek() failed on %s: %s", path, strerror(errno));
-			return;
-		}
-	} else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) {
-		return;
-	}
-
-	lineread_init(&lr, *fd);
-	lineread_get(&lr, &line); /* throw away header */
-	while (lineread_get(&lr, &line) > 0) {
-		/* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */
-		if (sscanf(line, "%*u: %*x:%lx %*x:%*x %x", &port, &state) != 2)
-			continue;
-
-		/* See enum in kernel's include/net/tcp_states.h */
-		if ((proto == IPPROTO_TCP && state != 0x0a) ||
-		    (proto == IPPROTO_UDP && state != 0x07))
-			continue;
-
-		if (bitmap_isset(exclude, port))
-			bitmap_clear(map, port);
-		else
-			bitmap_set(map, port);
-	}
-}
-
-/**
+/*
  * ns_enter() - Enter configured user (unless already joined) and network ns
  * @c:		Execution context
  *
diff --git a/util.h b/util.h
index 60d6872..9effc54 100644
--- a/util.h
+++ b/util.h
@@ -217,8 +217,6 @@ void bitmap_set(uint8_t *map, int bit);
 void bitmap_clear(uint8_t *map, int bit);
 int bitmap_isset(const uint8_t *map, int bit);
 char *line_read(char *buf, size_t len, int fd);
-void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns,
-			uint8_t *map, const uint8_t *exclude);
 void ns_enter(const struct ctx *c);
 bool ns_is_init(void);
 void write_pidfile(int fd, pid_t pid);
-- 
@@ -217,8 +217,6 @@ void bitmap_set(uint8_t *map, int bit);
 void bitmap_clear(uint8_t *map, int bit);
 int bitmap_isset(const uint8_t *map, int bit);
 char *line_read(char *buf, size_t len, int fd);
-void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns,
-			uint8_t *map, const uint8_t *exclude);
 void ns_enter(const struct ctx *c);
 bool ns_is_init(void);
 void write_pidfile(int fd, pid_t pid);
-- 
2.41.0

[PATCH v2 3/9] port_fwd: Better parameterise procfs_scan_listen()

[David Gibson]

procfs_scan_listen() does some slightly clunky logic to deduce the fd it
wants to use, the path it wants to open and the state it's looking for
based on parameters for protocol, IP version and whether we're in the
namespace.

However, the caller already has to make choices based on similar parameters
so it can just pass in the things that procfs_scan_listen() needs directly.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 port_fwd.c | 55 ++++++++++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/port_fwd.c b/port_fwd.c
index 136a450..49b60b1 100644
--- a/port_fwd.c
+++ b/port_fwd.c
@@ -23,39 +23,28 @@
 #include "passt.h"
 #include "lineread.h"
 
+/* See enum in kernel's include/net/tcp_states.h */
+#define UDP_LISTEN	0x07
+#define TCP_LISTEN	0x0a
+
 /**
  * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs
- * @proto:	IPPROTO_TCP or IPPROTO_UDP
- * @ip_version:	IP version, V4 or V6
- * @ns:		Use saved file descriptors for namespace if set
+ * @fd:		Pointer to fd for relevant /proc/net file
+ * @path:	Path to /proc/net file to open (if fd is -1)
+ * @lstate:	Code for listening state to scan for
  * @map:	Bitmap where numbers of ports in listening state will be set
  * @exclude:	Bitmap of ports to exclude from setting (and clear)
  *
  * #syscalls:pasta lseek
  * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
  */
-static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
-			       int ns, uint8_t *map, const uint8_t *exclude)
+static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate,
+			       uint8_t *map, const uint8_t *exclude)
 {
-	char *path, *line;
 	struct lineread lr;
 	unsigned long port;
 	unsigned int state;
-	int *fd;
-
-	if (proto == IPPROTO_TCP) {
-		fd = &c->proc_net_tcp[ip_version][ns];
-		if (ip_version == V4)
-			path = "/proc/net/tcp";
-		else
-			path = "/proc/net/tcp6";
-	} else {
-		fd = &c->proc_net_udp[ip_version][ns];
-		if (ip_version == V4)
-			path = "/proc/net/udp";
-		else
-			path = "/proc/net/udp6";
-	}
+	char *line;
 
 	if (*fd != -1) {
 		if (lseek(*fd, 0, SEEK_SET)) {
@@ -73,9 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
 		if (sscanf(line, "%*u: %*x:%lx %*x:%*x %x", &port, &state) != 2)
 			continue;
 
-		/* See enum in kernel's include/net/tcp_states.h */
-		if ((proto == IPPROTO_TCP && state != 0x0a) ||
-		    (proto == IPPROTO_UDP && state != 0x07))
+		if (state != lstate)
 			continue;
 
 		if (bitmap_isset(exclude, port))
@@ -109,15 +96,21 @@ void get_bound_ports(struct ctx *c, int ns, uint8_t proto)
 
 	if (proto == IPPROTO_UDP) {
 		memset(udp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(c, IPPROTO_UDP, V4, ns, udp_map, udp_excl);
-		procfs_scan_listen(c, IPPROTO_UDP, V6, ns, udp_map, udp_excl);
-
-		procfs_scan_listen(c, IPPROTO_TCP, V4, ns, udp_map, udp_excl);
-		procfs_scan_listen(c, IPPROTO_TCP, V6, ns, udp_map, udp_excl);
+		procfs_scan_listen(&c->proc_net_udp[V4][ns], "/proc/net/udp",
+				   UDP_LISTEN, udp_map, udp_excl);
+		procfs_scan_listen(&c->proc_net_udp[V6][ns], "/proc/net/udp6",
+				   UDP_LISTEN, udp_map, udp_excl);
+
+		procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp",
+				   TCP_LISTEN, udp_map, udp_excl);
+		procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6",
+				   TCP_LISTEN, udp_map, udp_excl);
 	} else if (proto == IPPROTO_TCP) {
 		memset(tcp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(c, IPPROTO_TCP, V4, ns, tcp_map, tcp_excl);
-		procfs_scan_listen(c, IPPROTO_TCP, V6, ns, tcp_map, tcp_excl);
+		procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp",
+				   TCP_LISTEN, tcp_map, tcp_excl);
+		procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6",
+				   TCP_LISTEN, tcp_map, tcp_excl);
 	}
 }
 
-- 
@@ -23,39 +23,28 @@
 #include "passt.h"
 #include "lineread.h"
 
+/* See enum in kernel's include/net/tcp_states.h */
+#define UDP_LISTEN	0x07
+#define TCP_LISTEN	0x0a
+
 /**
  * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs
- * @proto:	IPPROTO_TCP or IPPROTO_UDP
- * @ip_version:	IP version, V4 or V6
- * @ns:		Use saved file descriptors for namespace if set
+ * @fd:		Pointer to fd for relevant /proc/net file
+ * @path:	Path to /proc/net file to open (if fd is -1)
+ * @lstate:	Code for listening state to scan for
  * @map:	Bitmap where numbers of ports in listening state will be set
  * @exclude:	Bitmap of ports to exclude from setting (and clear)
  *
  * #syscalls:pasta lseek
  * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
  */
-static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
-			       int ns, uint8_t *map, const uint8_t *exclude)
+static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate,
+			       uint8_t *map, const uint8_t *exclude)
 {
-	char *path, *line;
 	struct lineread lr;
 	unsigned long port;
 	unsigned int state;
-	int *fd;
-
-	if (proto == IPPROTO_TCP) {
-		fd = &c->proc_net_tcp[ip_version][ns];
-		if (ip_version == V4)
-			path = "/proc/net/tcp";
-		else
-			path = "/proc/net/tcp6";
-	} else {
-		fd = &c->proc_net_udp[ip_version][ns];
-		if (ip_version == V4)
-			path = "/proc/net/udp";
-		else
-			path = "/proc/net/udp6";
-	}
+	char *line;
 
 	if (*fd != -1) {
 		if (lseek(*fd, 0, SEEK_SET)) {
@@ -73,9 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
 		if (sscanf(line, "%*u: %*x:%lx %*x:%*x %x", &port, &state) != 2)
 			continue;
 
-		/* See enum in kernel's include/net/tcp_states.h */
-		if ((proto == IPPROTO_TCP && state != 0x0a) ||
-		    (proto == IPPROTO_UDP && state != 0x07))
+		if (state != lstate)
 			continue;
 
 		if (bitmap_isset(exclude, port))
@@ -109,15 +96,21 @@ void get_bound_ports(struct ctx *c, int ns, uint8_t proto)
 
 	if (proto == IPPROTO_UDP) {
 		memset(udp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(c, IPPROTO_UDP, V4, ns, udp_map, udp_excl);
-		procfs_scan_listen(c, IPPROTO_UDP, V6, ns, udp_map, udp_excl);
-
-		procfs_scan_listen(c, IPPROTO_TCP, V4, ns, udp_map, udp_excl);
-		procfs_scan_listen(c, IPPROTO_TCP, V6, ns, udp_map, udp_excl);
+		procfs_scan_listen(&c->proc_net_udp[V4][ns], "/proc/net/udp",
+				   UDP_LISTEN, udp_map, udp_excl);
+		procfs_scan_listen(&c->proc_net_udp[V6][ns], "/proc/net/udp6",
+				   UDP_LISTEN, udp_map, udp_excl);
+
+		procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp",
+				   TCP_LISTEN, udp_map, udp_excl);
+		procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6",
+				   TCP_LISTEN, udp_map, udp_excl);
 	} else if (proto == IPPROTO_TCP) {
 		memset(tcp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(c, IPPROTO_TCP, V4, ns, tcp_map, tcp_excl);
-		procfs_scan_listen(c, IPPROTO_TCP, V6, ns, tcp_map, tcp_excl);
+		procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp",
+				   TCP_LISTEN, tcp_map, tcp_excl);
+		procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6",
+				   TCP_LISTEN, tcp_map, tcp_excl);
 	}
 }
 
-- 
2.41.0

[PATCH v2 4/9] util: Add open_in_ns() helper

[David Gibson]

Most of our helpers which need to enter the pasta network namespace are
quite specialised.  Add one which is rather general - it just open()s a
given file in the namespace context and returns the fd back to the main
namespace.  This will have some future uses.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 util.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 util.h |  1 +
 2 files changed, 54 insertions(+)

diff --git a/util.c b/util.c
index a8f3b35..c38ab7e 100644
--- a/util.c
+++ b/util.c
@@ -364,6 +364,59 @@ bool ns_is_init(void)
 	return ret;
 }
 
+/**
+ * struct open_in_ns_args - Parameters for do_open_in_ns()
+ * @c:		Execution context
+ * @fd:		Filled in with return value from open()
+ * @err:	Filled in with errno if open() failed
+ * @path:	Path to open
+ * @flags:	open() flags
+ */
+struct open_in_ns_args {
+	const struct ctx *c;
+	int fd;
+	int err;
+	const char *path;
+	int flags;
+};
+
+/**
+ * do_open_in_ns() - Enter namespace and open a file
+ * @arg:	See struct open_in_ns_args
+ *
+ * Must be called via NS_CALL()
+ */
+static int do_open_in_ns(void *arg)
+{
+	struct open_in_ns_args *a = (struct open_in_ns_args *)arg;
+
+	ns_enter(a->c);
+
+	a->fd = open(a->path, a->flags);
+	a->err = errno;
+
+	return 0;
+}
+
+/**
+ * open_in_ns() - open() within the pasta namespace
+ * @c:		Execution context
+ * @path:	Path to open
+ * @flags:	open() flags
+ *
+ * Return: fd of open()ed file or -1 on error, errno is set to indicate error
+ */
+int open_in_ns(const struct ctx *c, const char *path, int flags)
+{
+	struct open_in_ns_args arg = {
+		.c = c, .path = path, .flags = flags,
+	};
+
+	NS_CALL(do_open_in_ns, &arg);
+	errno = arg.err;
+	return arg.fd;
+}
+
 /**
  * pid_file() - Write PID to file, if requested to do so, and close it
  * @fd:		Open PID file descriptor, closed on exit, -1 to skip writing it
diff --git a/util.h b/util.h
index 9effc54..78a8fb2 100644
--- a/util.h
+++ b/util.h
@@ -219,6 +219,7 @@ int bitmap_isset(const uint8_t *map, int bit);
 char *line_read(char *buf, size_t len, int fd);
 void ns_enter(const struct ctx *c);
 bool ns_is_init(void);
+int open_in_ns(const struct ctx *c, const char *path, int flags);
 void write_pidfile(int fd, pid_t pid);
 int __daemon(int pidfile_fd, int devnull_fd);
 int fls(unsigned long x);
-- 
@@ -219,6 +219,7 @@ int bitmap_isset(const uint8_t *map, int bit);
 char *line_read(char *buf, size_t len, int fd);
 void ns_enter(const struct ctx *c);
 bool ns_is_init(void);
+int open_in_ns(const struct ctx *c, const char *path, int flags);
 void write_pidfile(int fd, pid_t pid);
 int __daemon(int pidfile_fd, int devnull_fd);
 int fls(unsigned long x);
-- 
2.41.0

[PATCH v2 5/9] port_fwd: Pre-open /proc/net/* files rather than on-demand

[David Gibson]

procfs_scan_listen() can either use an already opened fd for a /proc/net
file, or it will open it.  So, effectively it will open the file on the
first call, then re-use the fd in subsequent calls.  However, it's not
possible to open the /proc/net files after we isolate our filesystem in
isolate_prefork().  That means that for each /proc/net file we must call
procfs_scan_listen() at least once before isolate_prefork(), or it won't
work afterwards.

That happens to be the case, but it's a pretty fragile requirement.  To
make this more robust, instead always pre-open the /proc files we will need
in get_bounds_port_init() and have procfs_scan_listen() just use those.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 port_fwd.c | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/port_fwd.c b/port_fwd.c
index 49b60b1..045ad7a 100644
--- a/port_fwd.c
+++ b/port_fwd.c
@@ -29,8 +29,7 @@
 
 /**
  * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs
- * @fd:		Pointer to fd for relevant /proc/net file
- * @path:	Path to /proc/net file to open (if fd is -1)
+ * @fd:		fd for relevant /proc/net file
  * @lstate:	Code for listening state to scan for
  * @map:	Bitmap where numbers of ports in listening state will be set
  * @exclude:	Bitmap of ports to exclude from setting (and clear)
@@ -38,7 +37,7 @@
  * #syscalls:pasta lseek
  * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
  */
-static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate,
+static void procfs_scan_listen(int fd, unsigned int lstate,
 			       uint8_t *map, const uint8_t *exclude)
 {
 	struct lineread lr;
@@ -46,16 +45,12 @@ static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate,
 	unsigned int state;
 	char *line;
 
-	if (*fd != -1) {
-		if (lseek(*fd, 0, SEEK_SET)) {
-			warn("lseek() failed on %s: %s", path, strerror(errno));
-			return;
-		}
-	} else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) {
+	if (lseek(fd, 0, SEEK_SET)) {
+		warn("lseek() failed on /proc/net file: %s", strerror(errno));
 		return;
 	}
 
-	lineread_init(&lr, *fd);
+	lineread_init(&lr, fd);
 	lineread_get(&lr, &line); /* throw away header */
 	while (lineread_get(&lr, &line) > 0) {
 		/* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */
@@ -96,20 +91,20 @@ void get_bound_ports(struct ctx *c, int ns, uint8_t proto)
 
 	if (proto == IPPROTO_UDP) {
 		memset(udp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(&c->proc_net_udp[V4][ns], "/proc/net/udp",
+		procfs_scan_listen(c->proc_net_udp[V4][ns],
 				   UDP_LISTEN, udp_map, udp_excl);
-		procfs_scan_listen(&c->proc_net_udp[V6][ns], "/proc/net/udp6",
+		procfs_scan_listen(c->proc_net_udp[V6][ns],
 				   UDP_LISTEN, udp_map, udp_excl);
 
-		procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp",
+		procfs_scan_listen(c->proc_net_tcp[V4][ns],
 				   TCP_LISTEN, udp_map, udp_excl);
-		procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6",
+		procfs_scan_listen(c->proc_net_tcp[V6][ns],
 				   TCP_LISTEN, udp_map, udp_excl);
 	} else if (proto == IPPROTO_TCP) {
 		memset(tcp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp",
+		procfs_scan_listen(c->proc_net_tcp[V4][ns],
 				   TCP_LISTEN, tcp_map, tcp_excl);
-		procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6",
+		procfs_scan_listen(c->proc_net_tcp[V6][ns],
 				   TCP_LISTEN, tcp_map, tcp_excl);
 	}
 }
@@ -151,6 +146,7 @@ static int get_bound_ports_ns(void *arg)
 void port_fwd_init(struct ctx *c)
 {
 	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
+	const int flags = O_RDONLY | O_CLOEXEC;
 
 	c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
 	c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
@@ -158,15 +154,25 @@ void port_fwd_init(struct ctx *c)
 	c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
 
 	if (c->tcp.fwd_in.mode == FWD_AUTO) {
+		c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags);
+		c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags);
 		ns_ports_arg.proto = IPPROTO_TCP;
 		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
 	}
 	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
+		c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags);
+		c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags);
 		ns_ports_arg.proto = IPPROTO_UDP;
 		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
 	}
-	if (c->tcp.fwd_out.mode == FWD_AUTO)
+	if (c->tcp.fwd_out.mode == FWD_AUTO) {
+		c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags);
+		c->proc_net_tcp[V6][0] = open("/proc/net/tcp6", flags);
 		get_bound_ports(c, 0, IPPROTO_TCP);
-	if (c->udp.fwd_out.f.mode == FWD_AUTO)
+	}
+	if (c->udp.fwd_out.f.mode == FWD_AUTO) {
+		c->proc_net_udp[V4][0] = open("/proc/net/udp", flags);
+		c->proc_net_udp[V6][0] = open("/proc/net/udp6", flags);
 		get_bound_ports(c, 0, IPPROTO_UDP);
+	}
 }
-- 
@@ -29,8 +29,7 @@
 
 /**
  * procfs_scan_listen() - Set bits for listening TCP or UDP sockets from procfs
- * @fd:		Pointer to fd for relevant /proc/net file
- * @path:	Path to /proc/net file to open (if fd is -1)
+ * @fd:		fd for relevant /proc/net file
  * @lstate:	Code for listening state to scan for
  * @map:	Bitmap where numbers of ports in listening state will be set
  * @exclude:	Bitmap of ports to exclude from setting (and clear)
@@ -38,7 +37,7 @@
  * #syscalls:pasta lseek
  * #syscalls:pasta ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
  */
-static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate,
+static void procfs_scan_listen(int fd, unsigned int lstate,
 			       uint8_t *map, const uint8_t *exclude)
 {
 	struct lineread lr;
@@ -46,16 +45,12 @@ static void procfs_scan_listen(int *fd, const char *path, unsigned int lstate,
 	unsigned int state;
 	char *line;
 
-	if (*fd != -1) {
-		if (lseek(*fd, 0, SEEK_SET)) {
-			warn("lseek() failed on %s: %s", path, strerror(errno));
-			return;
-		}
-	} else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) {
+	if (lseek(fd, 0, SEEK_SET)) {
+		warn("lseek() failed on /proc/net file: %s", strerror(errno));
 		return;
 	}
 
-	lineread_init(&lr, *fd);
+	lineread_init(&lr, fd);
 	lineread_get(&lr, &line); /* throw away header */
 	while (lineread_get(&lr, &line) > 0) {
 		/* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */
@@ -96,20 +91,20 @@ void get_bound_ports(struct ctx *c, int ns, uint8_t proto)
 
 	if (proto == IPPROTO_UDP) {
 		memset(udp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(&c->proc_net_udp[V4][ns], "/proc/net/udp",
+		procfs_scan_listen(c->proc_net_udp[V4][ns],
 				   UDP_LISTEN, udp_map, udp_excl);
-		procfs_scan_listen(&c->proc_net_udp[V6][ns], "/proc/net/udp6",
+		procfs_scan_listen(c->proc_net_udp[V6][ns],
 				   UDP_LISTEN, udp_map, udp_excl);
 
-		procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp",
+		procfs_scan_listen(c->proc_net_tcp[V4][ns],
 				   TCP_LISTEN, udp_map, udp_excl);
-		procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6",
+		procfs_scan_listen(c->proc_net_tcp[V6][ns],
 				   TCP_LISTEN, udp_map, udp_excl);
 	} else if (proto == IPPROTO_TCP) {
 		memset(tcp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(&c->proc_net_tcp[V4][ns], "/proc/net/tcp",
+		procfs_scan_listen(c->proc_net_tcp[V4][ns],
 				   TCP_LISTEN, tcp_map, tcp_excl);
-		procfs_scan_listen(&c->proc_net_tcp[V6][ns], "/proc/net/tcp6",
+		procfs_scan_listen(c->proc_net_tcp[V6][ns],
 				   TCP_LISTEN, tcp_map, tcp_excl);
 	}
 }
@@ -151,6 +146,7 @@ static int get_bound_ports_ns(void *arg)
 void port_fwd_init(struct ctx *c)
 {
 	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
+	const int flags = O_RDONLY | O_CLOEXEC;
 
 	c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
 	c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
@@ -158,15 +154,25 @@ void port_fwd_init(struct ctx *c)
 	c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
 
 	if (c->tcp.fwd_in.mode == FWD_AUTO) {
+		c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags);
+		c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags);
 		ns_ports_arg.proto = IPPROTO_TCP;
 		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
 	}
 	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
+		c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags);
+		c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags);
 		ns_ports_arg.proto = IPPROTO_UDP;
 		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
 	}
-	if (c->tcp.fwd_out.mode == FWD_AUTO)
+	if (c->tcp.fwd_out.mode == FWD_AUTO) {
+		c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags);
+		c->proc_net_tcp[V6][0] = open("/proc/net/tcp6", flags);
 		get_bound_ports(c, 0, IPPROTO_TCP);
-	if (c->udp.fwd_out.f.mode == FWD_AUTO)
+	}
+	if (c->udp.fwd_out.f.mode == FWD_AUTO) {
+		c->proc_net_udp[V4][0] = open("/proc/net/udp", flags);
+		c->proc_net_udp[V6][0] = open("/proc/net/udp6", flags);
 		get_bound_ports(c, 0, IPPROTO_UDP);
+	}
 }
-- 
2.41.0

[PATCH v2 6/9] port_fwd: Don't NS_CALL get_bound_ports()

[David Gibson]

When we want to scan for bound ports in the namespace we use NS_CALL() to
run get_bound_ports() in the namespace.  However, the only thing it
actually needed to be in the namespace for was to open the /proc/net file
it was scanning.  Since we now always pre-open those, we no longer need
to switch to the namespace for the actual get_bound_ports() calls.

That in turn means that tcp_port_detect() doesn't need to run in the ns
either, and we can just replace it with inline calls to get_bound_ports().

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

diff --git a/port_fwd.c b/port_fwd.c
index 045ad7a..f400766 100644
--- a/port_fwd.c
+++ b/port_fwd.c
@@ -109,43 +109,12 @@ void get_bound_ports(struct ctx *c, int ns, uint8_t proto)
 	}
 }
 
-/**
- * struct get_bound_ports_ns_arg - Arguments for get_bound_ports_ns()
- * @c:		Execution context
- * @proto:	Protocol number (IPPROTO_TCP or IPPROTO_UDP)
- */
-struct get_bound_ports_ns_arg {
-	struct ctx *c;
-	uint8_t proto;
-};
-
-/**
- * get_bound_ports_ns() - Get maps of ports in namespace with bound sockets
- * @arg:	See struct get_bound_ports_ns_arg
- *
- * Return: 0
- */
-static int get_bound_ports_ns(void *arg)
-{
-	struct get_bound_ports_ns_arg *a = (struct get_bound_ports_ns_arg *)arg;
-	struct ctx *c = a->c;
-
-	if (!c->pasta_netns_fd)
-		return 0;
-
-	ns_enter(c);
-	get_bound_ports(c, 1, a->proto);
-
-	return 0;
-}
-
 /**
  * port_fwd_init() - Initial setup for port forwarding
  * @c:		Execution context
  */
 void port_fwd_init(struct ctx *c)
 {
-	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
 	const int flags = O_RDONLY | O_CLOEXEC;
 
 	c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
@@ -156,14 +125,12 @@ void port_fwd_init(struct ctx *c)
 	if (c->tcp.fwd_in.mode == FWD_AUTO) {
 		c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags);
 		c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags);
-		ns_ports_arg.proto = IPPROTO_TCP;
-		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
+		get_bound_ports(c, 1, IPPROTO_TCP);
 	}
 	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
 		c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags);
 		c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags);
-		ns_ports_arg.proto = IPPROTO_UDP;
-		NS_CALL(get_bound_ports_ns, &ns_ports_arg);
+		get_bound_ports(c, 1, IPPROTO_UDP);
 	}
 	if (c->tcp.fwd_out.mode == FWD_AUTO) {
 		c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags);
diff --git a/tcp.c b/tcp.c
index c6cc020..6fe9cdd 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3196,37 +3196,6 @@ int tcp_init(struct ctx *c)
 	return 0;
 }
 
-/**
- * struct tcp_port_detect_arg - Arguments for tcp_port_detect()
- * @c:			Execution context
- * @detect_in_ns:	Detect ports bound in namespace, not in init
- */
-struct tcp_port_detect_arg {
-	struct ctx *c;
-	int detect_in_ns;
-};
-
-/**
- * tcp_port_detect() - Detect ports bound in namespace or init
- * @arg:		See struct tcp_port_detect_arg
- *
- * Return: 0
- */
-static int tcp_port_detect(void *arg)
-{
-	struct tcp_port_detect_arg *a = (struct tcp_port_detect_arg *)arg;
-
-	if (a->detect_in_ns) {
-		ns_enter(a->c);
-
-		get_bound_ports(a->c, 1, IPPROTO_TCP);
-	} else {
-		get_bound_ports(a->c, 0, IPPROTO_TCP);
-	}
-
-	return 0;
-}
-
 /**
  * struct tcp_port_rebind_arg - Arguments for tcp_port_rebind()
  * @c:			Execution context
@@ -3315,19 +3284,16 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
 	(void)ts;
 
 	if (c->mode == MODE_PASTA) {
-		struct tcp_port_detect_arg detect_arg = { c, 0 };
 		struct tcp_port_rebind_arg rebind_arg = { c, 0 };
 
 		if (c->tcp.fwd_out.mode == FWD_AUTO) {
-			detect_arg.detect_in_ns = 0;
-			tcp_port_detect(&detect_arg);
+			get_bound_ports(c, 0, IPPROTO_TCP);
 			rebind_arg.bind_in_ns = 1;
 			NS_CALL(tcp_port_rebind, &rebind_arg);
 		}
 
 		if (c->tcp.fwd_in.mode == FWD_AUTO) {
-			detect_arg.detect_in_ns = 1;
-			NS_CALL(tcp_port_detect, &detect_arg);
+			get_bound_ports(c, 1, IPPROTO_TCP);
 			rebind_arg.bind_in_ns = 0;
 			tcp_port_rebind(&rebind_arg);
 		}
-- 
@@ -3196,37 +3196,6 @@ int tcp_init(struct ctx *c)
 	return 0;
 }
 
-/**
- * struct tcp_port_detect_arg - Arguments for tcp_port_detect()
- * @c:			Execution context
- * @detect_in_ns:	Detect ports bound in namespace, not in init
- */
-struct tcp_port_detect_arg {
-	struct ctx *c;
-	int detect_in_ns;
-};
-
-/**
- * tcp_port_detect() - Detect ports bound in namespace or init
- * @arg:		See struct tcp_port_detect_arg
- *
- * Return: 0
- */
-static int tcp_port_detect(void *arg)
-{
-	struct tcp_port_detect_arg *a = (struct tcp_port_detect_arg *)arg;
-
-	if (a->detect_in_ns) {
-		ns_enter(a->c);
-
-		get_bound_ports(a->c, 1, IPPROTO_TCP);
-	} else {
-		get_bound_ports(a->c, 0, IPPROTO_TCP);
-	}
-
-	return 0;
-}
-
 /**
  * struct tcp_port_rebind_arg - Arguments for tcp_port_rebind()
  * @c:			Execution context
@@ -3315,19 +3284,16 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
 	(void)ts;
 
 	if (c->mode == MODE_PASTA) {
-		struct tcp_port_detect_arg detect_arg = { c, 0 };
 		struct tcp_port_rebind_arg rebind_arg = { c, 0 };
 
 		if (c->tcp.fwd_out.mode == FWD_AUTO) {
-			detect_arg.detect_in_ns = 0;
-			tcp_port_detect(&detect_arg);
+			get_bound_ports(c, 0, IPPROTO_TCP);
 			rebind_arg.bind_in_ns = 1;
 			NS_CALL(tcp_port_rebind, &rebind_arg);
 		}
 
 		if (c->tcp.fwd_in.mode == FWD_AUTO) {
-			detect_arg.detect_in_ns = 1;
-			NS_CALL(tcp_port_detect, &detect_arg);
+			get_bound_ports(c, 1, IPPROTO_TCP);
 			rebind_arg.bind_in_ns = 0;
 			tcp_port_rebind(&rebind_arg);
 		}
-- 
2.41.0

[PATCH v2 7/9] port_fwd: Split TCP and UDP cases for get_bound_ports()

[David Gibson]

Currently get_bound_ports() takes a parameter to determine if it scans for
UDP or TCP bound ports, but in fact there's almost nothing in common
between those two paths.  The parameter appears primarily to have been
a convenience for when we needed to invoke this function via NS_CALL().

Now that we don't need that, split it into separate TCP and UDP versions.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 port_fwd.c | 76 ++++++++++++++++++++++++++++++------------------------
 port_fwd.h |  3 ++-
 tcp.c      |  4 +--
 3 files changed, 47 insertions(+), 36 deletions(-)

diff --git a/port_fwd.c b/port_fwd.c
index f400766..40e633d 100644
--- a/port_fwd.c
+++ b/port_fwd.c
@@ -68,45 +68,55 @@ static void procfs_scan_listen(int fd, unsigned int lstate,
 }
 
 /**
- * get_bound_ports() - Get maps of ports with bound sockets
+ * get_bound_ports_tcp() - Get maps of TCP ports with bound sockets
  * @c:		Execution context
  * @ns:		If set, set bitmaps for ports to tap/ns -- to init otherwise
- * @proto:	Protocol number (IPPROTO_TCP or IPPROTO_UDP)
  */
-void get_bound_ports(struct ctx *c, int ns, uint8_t proto)
+void get_bound_ports_tcp(struct ctx *c, int ns)
 {
-	uint8_t *udp_map, *udp_excl, *tcp_map, *tcp_excl;
+	uint8_t *map, *excl;
 
 	if (ns) {
-		udp_map = c->udp.fwd_in.f.map;
-		udp_excl = c->udp.fwd_out.f.map;
-		tcp_map = c->tcp.fwd_in.map;
-		tcp_excl = c->tcp.fwd_out.map;
+		map = c->tcp.fwd_in.map;
+		excl = c->tcp.fwd_out.map;
 	} else {
-		udp_map = c->udp.fwd_out.f.map;
-		udp_excl = c->udp.fwd_in.f.map;
-		tcp_map = c->tcp.fwd_out.map;
-		tcp_excl = c->tcp.fwd_in.map;
+		map = c->tcp.fwd_out.map;
+		excl = c->tcp.fwd_in.map;
 	}
 
-	if (proto == IPPROTO_UDP) {
-		memset(udp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(c->proc_net_udp[V4][ns],
-				   UDP_LISTEN, udp_map, udp_excl);
-		procfs_scan_listen(c->proc_net_udp[V6][ns],
-				   UDP_LISTEN, udp_map, udp_excl);
-
-		procfs_scan_listen(c->proc_net_tcp[V4][ns],
-				   TCP_LISTEN, udp_map, udp_excl);
-		procfs_scan_listen(c->proc_net_tcp[V6][ns],
-				   TCP_LISTEN, udp_map, udp_excl);
-	} else if (proto == IPPROTO_TCP) {
-		memset(tcp_map, 0, PORT_BITMAP_SIZE);
-		procfs_scan_listen(c->proc_net_tcp[V4][ns],
-				   TCP_LISTEN, tcp_map, tcp_excl);
-		procfs_scan_listen(c->proc_net_tcp[V6][ns],
-				   TCP_LISTEN, tcp_map, tcp_excl);
+	memset(map, 0, PORT_BITMAP_SIZE);
+	procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, map, excl);
+	procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, map, excl);
+}
+
+/**
+ * get_bound_ports_udp() - Get maps of UDP ports with bound sockets
+ * @c:		Execution context
+ * @ns:		If set, set bitmaps for ports to tap/ns -- to init otherwise
+ */
+void get_bound_ports_udp(struct ctx *c, int ns)
+{
+	uint8_t *map, *excl;
+
+	if (ns) {
+		map = c->udp.fwd_in.f.map;
+		excl = c->udp.fwd_out.f.map;
+	} else {
+		map = c->udp.fwd_out.f.map;
+		excl = c->udp.fwd_in.f.map;
 	}
+
+	memset(map, 0, PORT_BITMAP_SIZE);
+	procfs_scan_listen(c->proc_net_udp[V4][ns], UDP_LISTEN, map, excl);
+	procfs_scan_listen(c->proc_net_udp[V6][ns], UDP_LISTEN, map, excl);
+
+	/* Also forward UDP ports with the same numbers as bound TCP ports.
+	 * This is useful for a handful of protocols (e.g. iperf3) where a TCP
+	 * control port is used to set up transfers on a corresponding UDP
+	 * port.
+	 */
+	procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, map, excl);
+	procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, map, excl);
 }
 
 /**
@@ -125,21 +135,21 @@ void port_fwd_init(struct ctx *c)
 	if (c->tcp.fwd_in.mode == FWD_AUTO) {
 		c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags);
 		c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags);
-		get_bound_ports(c, 1, IPPROTO_TCP);
+		get_bound_ports_tcp(c, 1);
 	}
 	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
 		c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags);
 		c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags);
-		get_bound_ports(c, 1, IPPROTO_UDP);
+		get_bound_ports_udp(c, 1);
 	}
 	if (c->tcp.fwd_out.mode == FWD_AUTO) {
 		c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags);
 		c->proc_net_tcp[V6][0] = open("/proc/net/tcp6", flags);
-		get_bound_ports(c, 0, IPPROTO_TCP);
+		get_bound_ports_tcp(c, 0);
 	}
 	if (c->udp.fwd_out.f.mode == FWD_AUTO) {
 		c->proc_net_udp[V4][0] = open("/proc/net/udp", flags);
 		c->proc_net_udp[V6][0] = open("/proc/net/udp6", flags);
-		get_bound_ports(c, 0, IPPROTO_UDP);
+		get_bound_ports_udp(c, 0);
 	}
 }
diff --git a/port_fwd.h b/port_fwd.h
index ad8ed1f..2f8f526 100644
--- a/port_fwd.h
+++ b/port_fwd.h
@@ -31,7 +31,8 @@ struct port_fwd {
 	in_port_t delta[NUM_PORTS];
 };
 
-void get_bound_ports(struct ctx *c, int ns, uint8_t proto);
+void get_bound_ports_tcp(struct ctx *c, int ns);
+void get_bound_ports_udp(struct ctx *c, int ns);
 void port_fwd_init(struct ctx *c);
 
 #endif /* PORT_FWD_H */
diff --git a/tcp.c b/tcp.c
index 6fe9cdd..5b41897 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3287,13 +3287,13 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
 		struct tcp_port_rebind_arg rebind_arg = { c, 0 };
 
 		if (c->tcp.fwd_out.mode == FWD_AUTO) {
-			get_bound_ports(c, 0, IPPROTO_TCP);
+			get_bound_ports_tcp(c, 0);
 			rebind_arg.bind_in_ns = 1;
 			NS_CALL(tcp_port_rebind, &rebind_arg);
 		}
 
 		if (c->tcp.fwd_in.mode == FWD_AUTO) {
-			get_bound_ports(c, 1, IPPROTO_TCP);
+			get_bound_ports_tcp(c, 1);
 			rebind_arg.bind_in_ns = 0;
 			tcp_port_rebind(&rebind_arg);
 		}
-- 
@@ -3287,13 +3287,13 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
 		struct tcp_port_rebind_arg rebind_arg = { c, 0 };
 
 		if (c->tcp.fwd_out.mode == FWD_AUTO) {
-			get_bound_ports(c, 0, IPPROTO_TCP);
+			get_bound_ports_tcp(c, 0);
 			rebind_arg.bind_in_ns = 1;
 			NS_CALL(tcp_port_rebind, &rebind_arg);
 		}
 
 		if (c->tcp.fwd_in.mode == FWD_AUTO) {
-			get_bound_ports(c, 1, IPPROTO_TCP);
+			get_bound_ports_tcp(c, 1);
 			rebind_arg.bind_in_ns = 0;
 			tcp_port_rebind(&rebind_arg);
 		}
-- 
2.41.0

[PATCH v2 8/9] port_fwd: Move port scanning /proc fds into struct port_fwd

[David Gibson]

Currently we store /proc/net fds used to implement automatic port
forwarding in the proc_net_{tcp,udp} fields of the main context structure.
However, in fact each of those is associated with a particular direction
of forwarding, and we already have struct port_fwd which collects all
other information related to a particular direction of port forwarding.

We can simplify things a bit by moving the /proc fds into struct port_fwd.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 passt.h    |  5 -----
 port_fwd.c | 62 ++++++++++++++++++++++++++++--------------------------
 port_fwd.h |  4 ++++
 3 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/passt.h b/passt.h
index 282bd1a..53defa4 100644
--- a/passt.h
+++ b/passt.h
@@ -203,8 +203,6 @@ struct ip6_ctx {
  * @no_netns_quit:	In pasta mode, don't exit if fs-bound namespace is gone
  * @netns_base:		Base name for fs-bound namespace, if any, in pasta mode
  * @netns_dir:		Directory of fs-bound namespace, if any, in pasta mode
- * @proc_net_tcp:	Stored handles for /proc/net/tcp{,6} in init and ns
- * @proc_net_udp:	Stored handles for /proc/net/udp{,6} in init and ns
  * @epollfd:		File descriptor for epoll instance
  * @fd_tap_listen:	File descriptor for listening AF_UNIX socket, if any
  * @fd_tap:		AF_UNIX socket, tuntap device, or pre-opened socket
@@ -258,9 +256,6 @@ struct ctx {
 	char netns_base[PATH_MAX];
 	char netns_dir[PATH_MAX];
 
-	int proc_net_tcp[IP_VERSIONS][2];
-	int proc_net_udp[IP_VERSIONS][2];
-
 	int epollfd;
 	int fd_tap_listen;
 	int fd_tap;
diff --git a/port_fwd.c b/port_fwd.c
index 40e633d..9502463 100644
--- a/port_fwd.c
+++ b/port_fwd.c
@@ -74,19 +74,19 @@ static void procfs_scan_listen(int fd, unsigned int lstate,
  */
 void get_bound_ports_tcp(struct ctx *c, int ns)
 {
-	uint8_t *map, *excl;
+	struct port_fwd *fwd, *rev;
 
 	if (ns) {
-		map = c->tcp.fwd_in.map;
-		excl = c->tcp.fwd_out.map;
+		fwd = &c->tcp.fwd_in;
+		rev = &c->tcp.fwd_out;
 	} else {
-		map = c->tcp.fwd_out.map;
-		excl = c->tcp.fwd_in.map;
+		fwd = &c->tcp.fwd_out;
+		rev = &c->tcp.fwd_in;
 	}
 
-	memset(map, 0, PORT_BITMAP_SIZE);
-	procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, map, excl);
-	procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, map, excl);
+	memset(fwd->map, 0, PORT_BITMAP_SIZE);
+	procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map, rev->map);
+	procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map, rev->map);
 }
 
 /**
@@ -96,27 +96,29 @@ void get_bound_ports_tcp(struct ctx *c, int ns)
  */
 void get_bound_ports_udp(struct ctx *c, int ns)
 {
-	uint8_t *map, *excl;
+	struct port_fwd *fwd, *rev, *tcp;
 
 	if (ns) {
-		map = c->udp.fwd_in.f.map;
-		excl = c->udp.fwd_out.f.map;
+		fwd = &c->udp.fwd_in.f;
+		rev = &c->udp.fwd_out.f;
+		tcp = &c->tcp.fwd_in;
 	} else {
-		map = c->udp.fwd_out.f.map;
-		excl = c->udp.fwd_in.f.map;
+		fwd = &c->udp.fwd_out.f;
+		rev = &c->udp.fwd_in.f;
+		tcp = &c->tcp.fwd_out;
 	}
 
-	memset(map, 0, PORT_BITMAP_SIZE);
-	procfs_scan_listen(c->proc_net_udp[V4][ns], UDP_LISTEN, map, excl);
-	procfs_scan_listen(c->proc_net_udp[V6][ns], UDP_LISTEN, map, excl);
+	memset(fwd->map, 0, PORT_BITMAP_SIZE);
+	procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, rev->map);
+	procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, rev->map);
 
 	/* Also forward UDP ports with the same numbers as bound TCP ports.
 	 * This is useful for a handful of protocols (e.g. iperf3) where a TCP
 	 * control port is used to set up transfers on a corresponding UDP
 	 * port.
 	 */
-	procfs_scan_listen(c->proc_net_tcp[V4][ns], TCP_LISTEN, map, excl);
-	procfs_scan_listen(c->proc_net_tcp[V6][ns], TCP_LISTEN, map, excl);
+	procfs_scan_listen(tcp->scan4, TCP_LISTEN, fwd->map, rev->map);
+	procfs_scan_listen(tcp->scan6, TCP_LISTEN, fwd->map, rev->map);
 }
 
 /**
@@ -127,29 +129,29 @@ void port_fwd_init(struct ctx *c)
 {
 	const int flags = O_RDONLY | O_CLOEXEC;
 
-	c->proc_net_tcp[V4][0] = c->proc_net_tcp[V4][1] = -1;
-	c->proc_net_tcp[V6][0] = c->proc_net_tcp[V6][1] = -1;
-	c->proc_net_udp[V4][0] = c->proc_net_udp[V4][1] = -1;
-	c->proc_net_udp[V6][0] = c->proc_net_udp[V6][1] = -1;
+	c->tcp.fwd_in.scan4 = c->tcp.fwd_in.scan6 = -1;
+	c->tcp.fwd_out.scan4 = c->tcp.fwd_out.scan6 = -1;
+	c->udp.fwd_in.f.scan4 = c->udp.fwd_in.f.scan6 = -1;
+	c->udp.fwd_out.f.scan4 = c->udp.fwd_out.f.scan6 = -1;
 
 	if (c->tcp.fwd_in.mode == FWD_AUTO) {
-		c->proc_net_tcp[V4][1] = open_in_ns(c, "/proc/net/tcp", flags);
-		c->proc_net_tcp[V6][1] = open_in_ns(c, "/proc/net/tcp6", flags);
+		c->tcp.fwd_in.scan4 = open_in_ns(c, "/proc/net/tcp", flags);
+		c->tcp.fwd_in.scan6 = open_in_ns(c, "/proc/net/tcp6", flags);
 		get_bound_ports_tcp(c, 1);
 	}
 	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
-		c->proc_net_udp[V4][1] = open_in_ns(c, "/proc/net/udp", flags);
-		c->proc_net_udp[V6][1] = open_in_ns(c, "/proc/net/udp6", flags);
+		c->udp.fwd_in.f.scan4 = open_in_ns(c, "/proc/net/udp", flags);
+		c->udp.fwd_in.f.scan6 = open_in_ns(c, "/proc/net/udp6", flags);
 		get_bound_ports_udp(c, 1);
 	}
 	if (c->tcp.fwd_out.mode == FWD_AUTO) {
-		c->proc_net_tcp[V4][0] = open("/proc/net/tcp", flags);
-		c->proc_net_tcp[V6][0] = open("/proc/net/tcp6", flags);
+		c->tcp.fwd_out.scan4 = open("/proc/net/tcp", flags);
+		c->tcp.fwd_out.scan6 = open("/proc/net/tcp6", flags);
 		get_bound_ports_tcp(c, 0);
 	}
 	if (c->udp.fwd_out.f.mode == FWD_AUTO) {
-		c->proc_net_udp[V4][0] = open("/proc/net/udp", flags);
-		c->proc_net_udp[V6][0] = open("/proc/net/udp6", flags);
+		c->udp.fwd_out.f.scan4 = open("/proc/net/udp", flags);
+		c->udp.fwd_out.f.scan6 = open("/proc/net/udp6", flags);
 		get_bound_ports_udp(c, 0);
 	}
 }
diff --git a/port_fwd.h b/port_fwd.h
index 2f8f526..8ab6b48 100644
--- a/port_fwd.h
+++ b/port_fwd.h
@@ -22,11 +22,15 @@ enum port_fwd_mode {
 /**
  * port_fwd - Describes port forwarding for one protocol and direction
  * @mode:	Overall forwarding mode (all, none, auto, specific ports)
+ * @scan4:	/proc/net fd to scan for IPv4 ports when in AUTO mode
+ * @scan6:	/proc/net fd to scan for IPv6 ports when in AUTO mode
  * @map:	Bitmap describing which ports are forwarded
  * @delta:	Offset between the original destination and mapped port number
  */
 struct port_fwd {
 	enum port_fwd_mode mode;
+	int scan4;
+	int scan6;
 	uint8_t map[PORT_BITMAP_SIZE];
 	in_port_t delta[NUM_PORTS];
 };
-- 
@@ -22,11 +22,15 @@ enum port_fwd_mode {
 /**
  * port_fwd - Describes port forwarding for one protocol and direction
  * @mode:	Overall forwarding mode (all, none, auto, specific ports)
+ * @scan4:	/proc/net fd to scan for IPv4 ports when in AUTO mode
+ * @scan6:	/proc/net fd to scan for IPv6 ports when in AUTO mode
  * @map:	Bitmap describing which ports are forwarded
  * @delta:	Offset between the original destination and mapped port number
  */
 struct port_fwd {
 	enum port_fwd_mode mode;
+	int scan4;
+	int scan6;
 	uint8_t map[PORT_BITMAP_SIZE];
 	in_port_t delta[NUM_PORTS];
 };
-- 
2.41.0

[PATCH v2 9/9] port_fwd: Simplify get_bound_ports_*() to port_fwd_scan_*()

[David Gibson]

get_bound_ports_*() now only use their context and ns parameters to
determine which forwarding maps they're operating on.  Each function needs
the map they're actually updating, as well as the map for the other
direction, to avoid creating forwarding loops.  The UDP function also
requires the corresponding TCP map, to implement the behaviour where we
forward UDP ports of the same number as bound TCP ports for tools like
iperf3.

Passing those maps directly as parameters simplifies the code without
making the callers life harder, because those already know the relevant
maps.  IMO, invoking these functions in terms of where they're looking for
updated forwarding also makes more logical sense than in terms of where
they're looking for bound ports.  Given that new way of looking at the
functions, also rename them to port_fwd_scan_*().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 port_fwd.c | 50 ++++++++++++++++----------------------------------
 port_fwd.h |  5 +++--
 tcp.c      |  4 ++--
 3 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/port_fwd.c b/port_fwd.c
index 9502463..fc4d5cb 100644
--- a/port_fwd.c
+++ b/port_fwd.c
@@ -68,46 +68,26 @@ static void procfs_scan_listen(int fd, unsigned int lstate,
 }
 
 /**
- * get_bound_ports_tcp() - Get maps of TCP ports with bound sockets
- * @c:		Execution context
- * @ns:		If set, set bitmaps for ports to tap/ns -- to init otherwise
+ * port_fwd_scan_tcp() - Scan /proc to update TCP forwarding map
+ * @fwd:	Forwarding information to update
+ * @rev:	Forwarding information for the reverse direction
  */
-void get_bound_ports_tcp(struct ctx *c, int ns)
+void port_fwd_scan_tcp(struct port_fwd *fwd, const struct port_fwd *rev)
 {
-	struct port_fwd *fwd, *rev;
-
-	if (ns) {
-		fwd = &c->tcp.fwd_in;
-		rev = &c->tcp.fwd_out;
-	} else {
-		fwd = &c->tcp.fwd_out;
-		rev = &c->tcp.fwd_in;
-	}
-
 	memset(fwd->map, 0, PORT_BITMAP_SIZE);
 	procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map, rev->map);
 	procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map, rev->map);
 }
 
 /**
- * get_bound_ports_udp() - Get maps of UDP ports with bound sockets
- * @c:		Execution context
- * @ns:		If set, set bitmaps for ports to tap/ns -- to init otherwise
+ * port_fwd_scan_tcp() - Scan /proc to update TCP forwarding map
+ * @fwd:	Forwarding information to update
+ * @rev:	Forwarding information for the reverse direction
+ * @tcp:	Corresponding TCP forwarding information
  */
-void get_bound_ports_udp(struct ctx *c, int ns)
+void port_fwd_scan_udp(struct port_fwd *fwd, const struct port_fwd *rev,
+		       const struct port_fwd *tcp)
 {
-	struct port_fwd *fwd, *rev, *tcp;
-
-	if (ns) {
-		fwd = &c->udp.fwd_in.f;
-		rev = &c->udp.fwd_out.f;
-		tcp = &c->tcp.fwd_in;
-	} else {
-		fwd = &c->udp.fwd_out.f;
-		rev = &c->udp.fwd_in.f;
-		tcp = &c->tcp.fwd_out;
-	}
-
 	memset(fwd->map, 0, PORT_BITMAP_SIZE);
 	procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, rev->map);
 	procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, rev->map);
@@ -137,21 +117,23 @@ void port_fwd_init(struct ctx *c)
 	if (c->tcp.fwd_in.mode == FWD_AUTO) {
 		c->tcp.fwd_in.scan4 = open_in_ns(c, "/proc/net/tcp", flags);
 		c->tcp.fwd_in.scan6 = open_in_ns(c, "/proc/net/tcp6", flags);
-		get_bound_ports_tcp(c, 1);
+		port_fwd_scan_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
 	}
 	if (c->udp.fwd_in.f.mode == FWD_AUTO) {
 		c->udp.fwd_in.f.scan4 = open_in_ns(c, "/proc/net/udp", flags);
 		c->udp.fwd_in.f.scan6 = open_in_ns(c, "/proc/net/udp6", flags);
-		get_bound_ports_udp(c, 1);
+		port_fwd_scan_udp(&c->udp.fwd_in.f, &c->udp.fwd_out.f,
+				  &c->tcp.fwd_in);
 	}
 	if (c->tcp.fwd_out.mode == FWD_AUTO) {
 		c->tcp.fwd_out.scan4 = open("/proc/net/tcp", flags);
 		c->tcp.fwd_out.scan6 = open("/proc/net/tcp6", flags);
-		get_bound_ports_tcp(c, 0);
+		port_fwd_scan_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
 	}
 	if (c->udp.fwd_out.f.mode == FWD_AUTO) {
 		c->udp.fwd_out.f.scan4 = open("/proc/net/udp", flags);
 		c->udp.fwd_out.f.scan6 = open("/proc/net/udp6", flags);
-		get_bound_ports_udp(c, 0);
+		port_fwd_scan_udp(&c->udp.fwd_out.f, &c->udp.fwd_in.f,
+				  &c->tcp.fwd_out);
 	}
 }
diff --git a/port_fwd.h b/port_fwd.h
index 8ab6b48..8a823d8 100644
--- a/port_fwd.h
+++ b/port_fwd.h
@@ -35,8 +35,9 @@ struct port_fwd {
 	in_port_t delta[NUM_PORTS];
 };
 
-void get_bound_ports_tcp(struct ctx *c, int ns);
-void get_bound_ports_udp(struct ctx *c, int ns);
+void port_fwd_scan_tcp(struct port_fwd *fwd, const struct port_fwd *rev);
+void port_fwd_scan_udp(struct port_fwd *fwd, const struct port_fwd *rev,
+		       const struct port_fwd *tcp);
 void port_fwd_init(struct ctx *c);
 
 #endif /* PORT_FWD_H */
diff --git a/tcp.c b/tcp.c
index 5b41897..c13b7de 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3287,13 +3287,13 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
 		struct tcp_port_rebind_arg rebind_arg = { c, 0 };
 
 		if (c->tcp.fwd_out.mode == FWD_AUTO) {
-			get_bound_ports_tcp(c, 0);
+			port_fwd_scan_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
 			rebind_arg.bind_in_ns = 1;
 			NS_CALL(tcp_port_rebind, &rebind_arg);
 		}
 
 		if (c->tcp.fwd_in.mode == FWD_AUTO) {
-			get_bound_ports_tcp(c, 1);
+			port_fwd_scan_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
 			rebind_arg.bind_in_ns = 0;
 			tcp_port_rebind(&rebind_arg);
 		}
-- 
@@ -3287,13 +3287,13 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
 		struct tcp_port_rebind_arg rebind_arg = { c, 0 };
 
 		if (c->tcp.fwd_out.mode == FWD_AUTO) {
-			get_bound_ports_tcp(c, 0);
+			port_fwd_scan_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
 			rebind_arg.bind_in_ns = 1;
 			NS_CALL(tcp_port_rebind, &rebind_arg);
 		}
 
 		if (c->tcp.fwd_in.mode == FWD_AUTO) {
-			get_bound_ports_tcp(c, 1);
+			port_fwd_scan_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
 			rebind_arg.bind_in_ns = 0;
 			tcp_port_rebind(&rebind_arg);
 		}
-- 
2.41.0

Re: [PATCH v2 0/9] Clean ups to automatic port forwarding

[Stefano Brivio]

On Fri,  3 Nov 2023 13:22:54 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> This contains an assortment of cleanups to the code supporting the
> automatic port forwarding more for past - specifically
> get_bound_ports() and related functions.  This shouldn't mae any
> functional changes.
> 
> Based on the series with fixes for new cppcheck-2.12 warnings.
> 
> Changes since v1:
>  * Some comment tweaks based on Stefano's feedback.
> 
> David Gibson (9):
>   conf: Cleaner initialisation of default forwarding modes
>   port_fwd: Move automatic port forwarding code to port_fwd.[ch]
>   port_fwd: Better parameterise procfs_scan_listen()
>   util: Add open_in_ns() helper
>   port_fwd: Pre-open /proc/net/* files rather than on-demand
>   port_fwd: Don't NS_CALL get_bound_ports()
>   port_fwd: Split TCP and UDP cases for get_bound_ports()
>   port_fwd: Move port scanning /proc fds into struct port_fwd
>   port_fwd: Simplify get_bound_ports_*() to port_fwd_scan_*()

Applied.

-- 
Stefano