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

[PATCH 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.

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     | 104 +++++++++++++++------------------------
 util.h     |   3 +-
 9 files changed, 204 insertions(+), 211 deletions(-)
 create mode 100644 port_fwd.c

-- 
2.41.0

[PATCH 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 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 a9a6f2a..a2418ae 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 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 | 53 +++++++++++++++++++++++------------------------------
 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/port_fwd.c b/port_fwd.c
index 136a450..4b54877 100644
--- a/port_fwd.c
+++ b/port_fwd.c
@@ -23,39 +23,27 @@
 #include "passt.h"
 #include "lineread.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)) {
@@ -74,8 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
 			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,27 @@
 #include "passt.h"
 #include "lineread.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)) {
@@ -74,8 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
 			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 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 | 39 +++++++++++++++++++++++++++++++++++++++
 util.h |  1 +
 2 files changed, 40 insertions(+)

diff --git a/util.c b/util.c
index a8f3b35..92ad375 100644
--- a/util.c
+++ b/util.c
@@ -364,6 +364,45 @@ bool ns_is_init(void)
 	return ret;
 }
 
+struct open_in_ns_args {
+	const struct ctx *c;
+	int fd;
+	int err;
+	const char *path;
+	int flags;
+};
+
+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 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 4b54877..a3f69dd 100644
--- a/port_fwd.c
+++ b/port_fwd.c
@@ -28,8 +28,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)
@@ -37,7 +36,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;
@@ -45,16 +44,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);
+	}
 }
-- 
@@ -28,8 +28,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)
@@ -37,7 +36,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;
@@ -45,16 +44,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 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 a3f69dd..b91eafe 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 a2418ae..63a3c64 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3149,37 +3149,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
@@ -3268,19 +3237,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);
 		}
-- 
@@ -3149,37 +3149,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
@@ -3268,19 +3237,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 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 b91eafe..e6b3b14 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 63a3c64..921bd2a 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3240,13 +3240,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);
 		}
-- 
@@ -3240,13 +3240,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 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 e6b3b14..5308620 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 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 5308620..b6e256a 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 921bd2a..1c3af76 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3240,13 +3240,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);
 		}
-- 
@@ -3240,13 +3240,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 3/9] port_fwd: Better parameterise procfs_scan_listen()

[Stefano Brivio]

On Thu,  5 Oct 2023 14:44:39 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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 | 53 +++++++++++++++++++++++------------------------------
>  1 file changed, 23 insertions(+), 30 deletions(-)
> 
> diff --git a/port_fwd.c b/port_fwd.c
> index 136a450..4b54877 100644
> --- a/port_fwd.c
> +++ b/port_fwd.c
> @@ -23,39 +23,27 @@
>  #include "passt.h"
>  #include "lineread.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)) {
> @@ -74,8 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
>  			continue;
>  
>  		/* See enum in kernel's include/net/tcp_states.h */

This comment should now be moved before #define UDP_LISTEN and
TCP_LISTEN, as it's not otherwise clear where they're coming from.

Given how late I'm reviewing all this, and that presumably you have a
bunch of series/patches based on it, I'm also fine to apply this and
patch it in a separate commit, or also fix this up on merge myself --
let me know.

Same for the comments to the next patches/series.

-- 
Stefano

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

[Stefano Brivio]

On Thu,  5 Oct 2023 14:44:40 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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 | 39 +++++++++++++++++++++++++++++++++++++++
>  util.h |  1 +
>  2 files changed, 40 insertions(+)
> 
> diff --git a/util.c b/util.c
> index a8f3b35..92ad375 100644
> --- a/util.c
> +++ b/util.c
> @@ -364,6 +364,45 @@ bool ns_is_init(void)
>  	return ret;
>  }
>  
> +struct open_in_ns_args {

It would be nice to have the usual kerneldoc-style comment to this
(at a first reading: are "flags" the flags for open(2) or something
specialised for internal use?).

> +	const struct ctx *c;
> +	int fd;
> +	int err;
> +	const char *path;
> +	int flags;
> +};
> +
> +static int do_open_in_ns(void *arg)

Same for this one.

The rest of the series makes sense and looks good to me.

-- 
Stefano

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3571 bytes --]

On Thu, Nov 02, 2023 at 07:07:19PM +0100, Stefano Brivio wrote:
> On Thu,  5 Oct 2023 14:44:39 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > 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 | 53 +++++++++++++++++++++++------------------------------
> >  1 file changed, 23 insertions(+), 30 deletions(-)
> > 
> > diff --git a/port_fwd.c b/port_fwd.c
> > index 136a450..4b54877 100644
> > --- a/port_fwd.c
> > +++ b/port_fwd.c
> > @@ -23,39 +23,27 @@
> >  #include "passt.h"
> >  #include "lineread.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)) {
> > @@ -74,8 +62,7 @@ static void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
> >  			continue;
> >  
> >  		/* See enum in kernel's include/net/tcp_states.h */
> 
> This comment should now be moved before #define UDP_LISTEN and
> TCP_LISTEN, as it's not otherwise clear where they're coming from.

Good point, fixed.

> Given how late I'm reviewing all this, and that presumably you have a
> bunch of series/patches based on it, I'm also fine to apply this and
> patch it in a separate commit, or also fix this up on merge myself --
> let me know.
> 
> Same for the comments to the next patches/series.

Eh, I don't think the rebasing should be too hard, so I'm happy to
sort that out.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

On Thu, Nov 02, 2023 at 07:07:25PM +0100, Stefano Brivio wrote:
> On Thu,  5 Oct 2023 14:44:40 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > 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 | 39 +++++++++++++++++++++++++++++++++++++++
> >  util.h |  1 +
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/util.c b/util.c
> > index a8f3b35..92ad375 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -364,6 +364,45 @@ bool ns_is_init(void)
> >  	return ret;
> >  }
> >  
> > +struct open_in_ns_args {
> 
> It would be nice to have the usual kerneldoc-style comment to this
> (at a first reading: are "flags" the flags for open(2) or something
> specialised for internal use?).
> 
> > +	const struct ctx *c;
> > +	int fd;
> > +	int err;
> > +	const char *path;
> > +	int flags;
> > +};
> > +
> > +static int do_open_in_ns(void *arg)
> 
> Same for this one.

Eh, ok.  I left it out originally, because they both seemed like they
were essentially internals of open_in_ns() itself, but I've added them
now.

> 
> The rest of the series makes sense and looks good to me.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]