[PATCH 0/8] RFC: Cleanups to auto port scanning

[PATCH 0/8] RFC: Cleanups to auto port scanning

[David Gibson]

One of the trickiest parts of implementing the improved forwarding
table is integrating it properly with automatic forwarding (-t auto
etc.).  Here are some preliminary cleanups to the automatic port
scanning that will make the job a bit easier.

David Gibson (8):
  icmp: Remove vestiges of ICMP timer
  tcp, udp, fwd: Run all port scanning from a single timer
  fwd: Consolidate scans (not rebinds) in fwd.c
  fwd: Move port exclusion handling from procfs_scan_listen() to callers
  fwd: Share port scanning logic between init and timer cases
  fwd: Check forwarding mode in fwd_scan_ports_*() rather than caller
  fwd: Update all port maps before applying exclusions
  tcp, udp: Don't exclude ports in {tcp,udp}_port_rebind()

 fwd.c   | 107 ++++++++++++++++++++++++++++++++++++++------------------
 fwd.h   |   7 ++--
 icmp.h  |   2 --
 passt.c |   8 ++---
 tcp.c   |  34 +++++++++---------
 tcp.h   |   3 +-
 udp.c   |  31 ++++------------
 udp.h   |   4 +--
 util.c  |  23 ++++++++++++
 util.h  |   1 +
 10 files changed, 129 insertions(+), 91 deletions(-)

-- 
2.51.0

[PATCH 1/8] icmp: Remove vestiges of ICMP timer

[David Gibson]

We no longer have a global ICMP timer (timers for individual flows are
handled through the flow timer).  We still have an ICMP_TIMER_INTERVAL
define, though.  Remove it.

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

diff --git a/icmp.h b/icmp.h
index d1cecb20..1a0e6205 100644
--- a/icmp.h
+++ b/icmp.h
@@ -6,8 +6,6 @@
 #ifndef ICMP_H
 #define ICMP_H
 
-#define ICMP_TIMER_INTERVAL		10000 /* ms */
-
 struct ctx;
 struct icmp_ping_flow;
 
diff --git a/passt.c b/passt.c
index 31fbb75b..b877659e 100644
--- a/passt.c
+++ b/passt.c
@@ -56,8 +56,7 @@
 
 #define EPOLL_EVENTS		8
 
-#define TIMER_INTERVAL__	MIN(TCP_TIMER_INTERVAL, UDP_TIMER_INTERVAL)
-#define TIMER_INTERVAL_		MIN(TIMER_INTERVAL__, ICMP_TIMER_INTERVAL)
+#define TIMER_INTERVAL_		MIN(TCP_TIMER_INTERVAL, UDP_TIMER_INTERVAL)
 #define TIMER_INTERVAL		MIN(TIMER_INTERVAL_, FLOW_TIMER_INTERVAL)
 
 char pkt_buf[PKT_BUF_BYTES]	__attribute__ ((aligned(PAGE_SIZE)));
-- 
2.51.0

[PATCH 2/8] tcp, udp, fwd: Run all port scanning from a single timer

[David Gibson]

With -t auto and similar options we need to periodically scan /proc for
listening ports.  Currently we do this separately for TCP and UDP, from
tcp_timer() and udp_timer().

For upcoming changes (leading eventually to a more general forwarding
table), it's awkward to have these separate.  Move them to a single common
timer.  For now this just calls new tcp_scan_ports() and udp_scan_ports()
functions, but we'll consolidate more thoroughly in later patches.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c   | 24 ++++++++++++++++++++++++
 fwd.h   |  3 +++
 passt.c |  7 +++----
 tcp.c   | 33 ++++++++++++++++++++-------------
 tcp.h   |  3 ++-
 udp.c   | 29 ++++++++++++-----------------
 udp.h   |  4 +---
 7 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/fwd.c b/fwd.c
index 250cf564..bd89a94a 100644
--- a/fwd.c
+++ b/fwd.c
@@ -230,6 +230,30 @@ void fwd_scan_ports_init(struct ctx *c)
 	}
 }
 
+/* Last time we scanned for open ports */
+static struct timespec scan_ports_run;
+
+/**
+ * fwd_scan_ports_timer() - Rescan open port information when necessary
+ * @c:		Execution context
+ * @now:	Current (monotonic) time
+ */
+void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now)
+{
+	if (c->mode != MODE_PASTA)
+		return;
+
+	if (timespec_diff_ms(now, &scan_ports_run) < FWD_PORT_SCAN_INTERVAL)
+		return;
+
+	scan_ports_run = *now;
+
+	if (!c->no_tcp)
+		tcp_scan_ports(c);
+	if (!c->no_udp)
+		udp_scan_ports(c);
+}
+
 /**
  * is_dns_flow() - Determine if flow appears to be a DNS request
  * @proto:	Protocol (IP L4 protocol number)
diff --git a/fwd.h b/fwd.h
index 65c7c964..385e5bd8 100644
--- a/fwd.h
+++ b/fwd.h
@@ -42,11 +42,14 @@ struct fwd_ports {
 	in_port_t delta[NUM_PORTS];
 };
 
+#define FWD_PORT_SCAN_INTERVAL		1000	/* ms */
+
 void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev);
 void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *rev,
 			const struct fwd_ports *tcp_fwd,
 			const struct fwd_ports *tcp_rev);
 void fwd_scan_ports_init(struct ctx *c);
+void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now);
 
 bool nat_inbound(const struct ctx *c, const union inany_addr *addr,
 		 union inany_addr *translated);
diff --git a/passt.c b/passt.c
index b877659e..28ce5711 100644
--- a/passt.c
+++ b/passt.c
@@ -56,7 +56,7 @@
 
 #define EPOLL_EVENTS		8
 
-#define TIMER_INTERVAL_		MIN(TCP_TIMER_INTERVAL, UDP_TIMER_INTERVAL)
+#define TIMER_INTERVAL_		MIN(TCP_TIMER_INTERVAL, FWD_PORT_SCAN_INTERVAL)
 #define TIMER_INTERVAL		MIN(TIMER_INTERVAL_, FLOW_TIMER_INTERVAL)
 
 char pkt_buf[PKT_BUF_BYTES]	__attribute__ ((aligned(PAGE_SIZE)));
@@ -117,11 +117,10 @@ static void post_handler(struct ctx *c, const struct timespec *now)
 
 	/* NOLINTNEXTLINE(bugprone-branch-clone): intervals can be the same */
 	CALL_PROTO_HANDLER(tcp, TCP);
-	/* NOLINTNEXTLINE(bugprone-branch-clone): intervals can be the same */
-	CALL_PROTO_HANDLER(udp, UDP);
+#undef CALL_PROTO_HANDLER
 
 	flow_defer_handler(c, now);
-#undef CALL_PROTO_HANDLER
+	fwd_scan_ports_timer(c, now);
 
 	if (!c->no_ndp)
 		ndp_timer(c, now);
diff --git a/tcp.c b/tcp.c
index 0f9e9b3f..dbbb00c4 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2869,25 +2869,32 @@ static int tcp_port_rebind_outbound(void *arg)
 }
 
 /**
- * tcp_timer() - Periodic tasks: port detection, closed connections, pool refill
+ * tcp_scan_ports() - Update forwarding maps based on scan of listening ports
  * @c:		Execution context
- * @now:	Current timestamp
  */
-void tcp_timer(struct ctx *c, const struct timespec *now)
+void tcp_scan_ports(struct ctx *c)
 {
-	(void)now;
+	ASSERT(c->mode == MODE_PASTA && !c->no_tcp);
 
-	if (c->mode == MODE_PASTA) {
-		if (c->tcp.fwd_out.mode == FWD_AUTO) {
-			fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
-			NS_CALL(tcp_port_rebind_outbound, c);
-		}
+	if (c->tcp.fwd_out.mode == FWD_AUTO) {
+		fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
+		NS_CALL(tcp_port_rebind_outbound, c);
+	}
 
-		if (c->tcp.fwd_in.mode == FWD_AUTO) {
-			fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
-			tcp_port_rebind(c, false);
-		}
+	if (c->tcp.fwd_in.mode == FWD_AUTO) {
+		fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
+		tcp_port_rebind(c, false);
 	}
+}
+
+/**
+ * tcp_timer() - Periodic tasks: port detection, closed connections, pool refill
+ * @c:		Execution context
+ * @now:	Current timestamp
+ */
+void tcp_timer(const struct ctx *c, const struct timespec *now)
+{
+	(void)now;
 
 	tcp_sock_refill_init(c);
 	if (c->mode == MODE_PASTA)
diff --git a/tcp.h b/tcp.h
index 234a8033..43ab0655 100644
--- a/tcp.h
+++ b/tcp.h
@@ -21,7 +21,8 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 int tcp_sock_init(const struct ctx *c, const union inany_addr *addr,
 		  const char *ifname, in_port_t port);
 int tcp_init(struct ctx *c);
-void tcp_timer(struct ctx *c, const struct timespec *now);
+void tcp_scan_ports(struct ctx *c);
+void tcp_timer(const struct ctx *c, const struct timespec *now);
 void tcp_defer_handler(struct ctx *c);
 
 void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s);
diff --git a/udp.c b/udp.c
index 86585b7e..d15f03dc 100644
--- a/udp.c
+++ b/udp.c
@@ -1239,28 +1239,23 @@ static int udp_port_rebind_outbound(void *arg)
 }
 
 /**
- * udp_timer() - Scan activity bitmaps for ports with associated timed events
+ * udp_scan_ports() - Update forwarding maps based on scan of listening ports
  * @c:		Execution context
- * @now:	Current timestamp
  */
-void udp_timer(struct ctx *c, const struct timespec *now)
+void udp_scan_ports(struct ctx *c)
 {
-	(void)now;
-
-	ASSERT(!c->no_udp);
+	ASSERT(c->mode == MODE_PASTA && !c->no_udp);
 
-	if (c->mode == MODE_PASTA) {
-		if (c->udp.fwd_out.mode == FWD_AUTO) {
-			fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
-					   &c->tcp.fwd_out, &c->tcp.fwd_in);
-			NS_CALL(udp_port_rebind_outbound, c);
-		}
+	if (c->udp.fwd_out.mode == FWD_AUTO) {
+		fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
+				   &c->tcp.fwd_out, &c->tcp.fwd_in);
+		NS_CALL(udp_port_rebind_outbound, c);
+	}
 
-		if (c->udp.fwd_in.mode == FWD_AUTO) {
-			fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
-					   &c->tcp.fwd_in, &c->tcp.fwd_out);
-			udp_port_rebind(c, false);
-		}
+	if (c->udp.fwd_in.mode == FWD_AUTO) {
+		fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
+				   &c->tcp.fwd_in, &c->tcp.fwd_out);
+		udp_port_rebind(c, false);
 	}
 }
 
diff --git a/udp.h b/udp.h
index 8f8531ad..a6de1f1c 100644
--- a/udp.h
+++ b/udp.h
@@ -6,8 +6,6 @@
 #ifndef UDP_H
 #define UDP_H
 
-#define UDP_TIMER_INTERVAL		1000 /* ms */
-
 void udp_portmap_clear(void);
 void udp_listen_sock_handler(const struct ctx *c, union epoll_ref ref,
 			     uint32_t events, const struct timespec *now);
@@ -20,7 +18,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
 int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr,
 		  const char *ifname, in_port_t port);
 int udp_init(struct ctx *c);
-void udp_timer(struct ctx *c, const struct timespec *now);
+void udp_scan_ports(struct ctx *c);
 void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s);
 
 /**
-- 
2.51.0

[PATCH 3/8] fwd: Consolidate scans (not rebinds) in fwd.c

[David Gibson]

fwd_scan_ports_timer(), via the things it calls, goes through all the auto
forwarding cases (tcp, udp, inbound, outbound) and for each one first scans
for listening ports, then rebinds - that is, closes or opens our own
listening ports to match.

Rearrange to do all the scans first, then all the rebinds after.  This lets
us consolidate all the scans into fwd.c, and will enable further cleanups.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c | 27 +++++++++++++++++++++------
 fwd.h |  4 ----
 tcp.c | 12 ++++--------
 tcp.h |  2 +-
 udp.c | 14 ++++----------
 udp.h |  2 +-
 6 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/fwd.c b/fwd.c
index bd89a94a..19309f14 100644
--- a/fwd.c
+++ b/fwd.c
@@ -153,7 +153,8 @@ static void procfs_scan_listen(int fd, unsigned int lstate,
  * @fwd:	Forwarding information to update
  * @rev:	Forwarding information for the reverse direction
  */
-void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev)
+static void fwd_scan_ports_tcp(struct fwd_ports *fwd,
+			       const struct fwd_ports *rev)
 {
 	memset(fwd->map, 0, PORT_BITMAP_SIZE);
 	procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map, rev->map);
@@ -167,9 +168,10 @@ void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev)
  * @tcp_fwd:	Corresponding TCP forwarding information
  * @tcp_rev:	TCP forwarding information for the reverse direction
  */
-void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *rev,
-			const struct fwd_ports *tcp_fwd,
-			const struct fwd_ports *tcp_rev)
+static void fwd_scan_ports_udp(struct fwd_ports *fwd,
+			       const struct fwd_ports *rev,
+			       const struct fwd_ports *tcp_fwd,
+			       const struct fwd_ports *tcp_rev)
 {
 	uint8_t exclude[PORT_BITMAP_SIZE];
 
@@ -248,10 +250,23 @@ void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now)
 
 	scan_ports_run = *now;
 
+	if (c->tcp.fwd_out.mode == FWD_AUTO)
+		fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
+	if (c->tcp.fwd_in.mode == FWD_AUTO)
+		fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
+	if (c->udp.fwd_out.mode == FWD_AUTO) {
+		fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
+				   &c->tcp.fwd_out, &c->tcp.fwd_in);
+	}
+	if (c->udp.fwd_in.mode == FWD_AUTO) {
+		fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
+				   &c->tcp.fwd_in, &c->tcp.fwd_out);
+	}
+
 	if (!c->no_tcp)
-		tcp_scan_ports(c);
+		tcp_port_rebind_all(c);
 	if (!c->no_udp)
-		udp_scan_ports(c);
+		udp_port_rebind_all(c);
 }
 
 /**
diff --git a/fwd.h b/fwd.h
index 385e5bd8..8fe4119a 100644
--- a/fwd.h
+++ b/fwd.h
@@ -44,10 +44,6 @@ struct fwd_ports {
 
 #define FWD_PORT_SCAN_INTERVAL		1000	/* ms */
 
-void fwd_scan_ports_tcp(struct fwd_ports *fwd, const struct fwd_ports *rev);
-void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *rev,
-			const struct fwd_ports *tcp_fwd,
-			const struct fwd_ports *tcp_rev);
 void fwd_scan_ports_init(struct ctx *c);
 void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now);
 
diff --git a/tcp.c b/tcp.c
index dbbb00c4..a330538d 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2869,22 +2869,18 @@ static int tcp_port_rebind_outbound(void *arg)
 }
 
 /**
- * tcp_scan_ports() - Update forwarding maps based on scan of listening ports
+ * tcp_port_rebind_all() - Rebind ports to match forward maps (in host & ns)
  * @c:		Execution context
  */
-void tcp_scan_ports(struct ctx *c)
+void tcp_port_rebind_all(struct ctx *c)
 {
 	ASSERT(c->mode == MODE_PASTA && !c->no_tcp);
 
-	if (c->tcp.fwd_out.mode == FWD_AUTO) {
-		fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
+	if (c->tcp.fwd_out.mode == FWD_AUTO)
 		NS_CALL(tcp_port_rebind_outbound, c);
-	}
 
-	if (c->tcp.fwd_in.mode == FWD_AUTO) {
-		fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
+	if (c->tcp.fwd_in.mode == FWD_AUTO)
 		tcp_port_rebind(c, false);
-	}
 }
 
 /**
diff --git a/tcp.h b/tcp.h
index 43ab0655..69b6c5dc 100644
--- a/tcp.h
+++ b/tcp.h
@@ -21,7 +21,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 int tcp_sock_init(const struct ctx *c, const union inany_addr *addr,
 		  const char *ifname, in_port_t port);
 int tcp_init(struct ctx *c);
-void tcp_scan_ports(struct ctx *c);
+void tcp_port_rebind_all(struct ctx *c);
 void tcp_timer(const struct ctx *c, const struct timespec *now);
 void tcp_defer_handler(struct ctx *c);
 
diff --git a/udp.c b/udp.c
index d15f03dc..e27b2219 100644
--- a/udp.c
+++ b/udp.c
@@ -1239,24 +1239,18 @@ static int udp_port_rebind_outbound(void *arg)
 }
 
 /**
- * udp_scan_ports() - Update forwarding maps based on scan of listening ports
+ * udp_port_rebind_all() - Rebind ports to match forward maps (in host & ns)
  * @c:		Execution context
  */
-void udp_scan_ports(struct ctx *c)
+void udp_port_rebind_all(struct ctx *c)
 {
 	ASSERT(c->mode == MODE_PASTA && !c->no_udp);
 
-	if (c->udp.fwd_out.mode == FWD_AUTO) {
-		fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
-				   &c->tcp.fwd_out, &c->tcp.fwd_in);
+	if (c->udp.fwd_out.mode == FWD_AUTO)
 		NS_CALL(udp_port_rebind_outbound, c);
-	}
 
-	if (c->udp.fwd_in.mode == FWD_AUTO) {
-		fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
-				   &c->tcp.fwd_in, &c->tcp.fwd_out);
+	if (c->udp.fwd_in.mode == FWD_AUTO)
 		udp_port_rebind(c, false);
-	}
 }
 
 /**
diff --git a/udp.h b/udp.h
index a6de1f1c..a2bf2720 100644
--- a/udp.h
+++ b/udp.h
@@ -18,7 +18,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
 int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr,
 		  const char *ifname, in_port_t port);
 int udp_init(struct ctx *c);
-void udp_scan_ports(struct ctx *c);
+void udp_port_rebind_all(struct ctx *c);
 void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s);
 
 /**
-- 
2.51.0

[PATCH 4/8] fwd: Move port exclusion handling from procfs_scan_listen() to callers

[David Gibson]

To avoid forwarding loops, we need to exclude certain ports from being
auto-forwarded.  To accomplish this, procfs_scan_listen() takes a bitmap
of exclusions.  As it detects each port, it checks against that bitmap.
This is a complicated way of accomplishing what we need.  We can instead
mask out the excluded ports in the callers using a new bitmap_andc()
helper.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c  | 32 ++++++++++++++------------------
 util.c | 23 +++++++++++++++++++++++
 util.h |  1 +
 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/fwd.c b/fwd.c
index 19309f14..c7b768d5 100644
--- a/fwd.c
+++ b/fwd.c
@@ -110,13 +110,11 @@ bool fwd_port_is_ephemeral(in_port_t port)
  * @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)
  *
  * #syscalls:pasta lseek
  * #syscalls:pasta ppc64le:_llseek ppc64:_llseek arm:_llseek
  */
-static void procfs_scan_listen(int fd, unsigned int lstate,
-			       uint8_t *map, const uint8_t *exclude)
+static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map)
 {
 	struct lineread lr;
 	unsigned long port;
@@ -141,10 +139,7 @@ static void procfs_scan_listen(int fd, unsigned int lstate,
 		if (state != lstate)
 			continue;
 
-		if (bitmap_isset(exclude, port))
-			bitmap_clear(map, port);
-		else
-			bitmap_set(map, port);
+		bitmap_set(map, port);
 	}
 }
 
@@ -157,8 +152,9 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd,
 			       const struct fwd_ports *rev)
 {
 	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);
+	procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map);
+	procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map);
+	bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
 }
 
 /**
@@ -173,26 +169,26 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
 			       const struct fwd_ports *tcp_fwd,
 			       const struct fwd_ports *tcp_rev)
 {
-	uint8_t exclude[PORT_BITMAP_SIZE];
-
-	bitmap_or(exclude, PORT_BITMAP_SIZE, rev->map, tcp_rev->map);
-
 	memset(fwd->map, 0, PORT_BITMAP_SIZE);
-	procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map, exclude);
-	procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map, exclude);
+	procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map);
+	procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->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(tcp_fwd->scan4, TCP_LISTEN, fwd->map);
+	procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map);
+
+	/*
 	 * This means we need to skip numbers of TCP ports bound on the other
 	 * side, too. Otherwise, we would detect corresponding UDP ports as
 	 * bound and try to forward them from the opposite side, but it's
 	 * already us handling them.
 	 */
-	procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map, exclude);
-	procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map, exclude);
+	bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
+	bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map);
 }
 
 /**
diff --git a/util.c b/util.c
index c492f904..eabadf7d 100644
--- a/util.c
+++ b/util.c
@@ -322,6 +322,7 @@ void bitmap_set(uint8_t *map, unsigned bit)
  * @map:	Pointer to bitmap
  * @bit:	Bit number to clear
  */
+/* cppcheck-suppress unusedFunction */
 void bitmap_clear(uint8_t *map, unsigned bit)
 {
 	unsigned long *word = (unsigned long *)map + BITMAP_WORD(bit);
@@ -351,6 +352,7 @@ bool bitmap_isset(const uint8_t *map, unsigned bit)
  * @a:		First operand
  * @b:		Second operand
  */
+/* cppcheck-suppress unusedFunction */
 void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
 {
 	unsigned long *dw = (unsigned long *)dst;
@@ -365,6 +367,27 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
 		dst[i] = a[i] | b[i];
 }
 
+/**
+ * bitmap_andc() - Logical conjunction with complement (AND NOT) of bitmap
+ * @dst:	Pointer to result bitmap
+ * @size:	Size of bitmaps, in bytes
+ * @a:		First operand
+ * @b:		Second operand
+ */
+void bitmap_andc(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
+{
+	unsigned long *dw = (unsigned long *)dst;
+	unsigned long *aw = (unsigned long *)a;
+	unsigned long *bw = (unsigned long *)b;
+	size_t i;
+
+	for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++)
+		*dw = *aw & ~*bw;
+
+	for (i = size / sizeof(long) * sizeof(long); i < size; i++)
+		dst[i] = a[i] & ~b[i];
+}
+
 /**
  * ns_enter() - Enter configured user (unless already joined) and network ns
  * @c:		Execution context
diff --git a/util.h b/util.h
index 22eaac56..ac8339a3 100644
--- a/util.h
+++ b/util.h
@@ -213,6 +213,7 @@ void bitmap_set(uint8_t *map, unsigned bit);
 void bitmap_clear(uint8_t *map, unsigned bit);
 bool bitmap_isset(const uint8_t *map, unsigned bit);
 void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b);
+void bitmap_andc(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b);
 char *line_read(char *buf, size_t len, int fd);
 void ns_enter(const struct ctx *c);
 bool ns_is_init(void);
-- 
2.51.0

[PATCH 5/8] fwd: Share port scanning logic between init and timer cases

[David Gibson]

When using -t auto and the like, we scan for listening ports once at
startup, then repeatedly on a timer.  With previous rearrangements the
logic for each of these cases is very nearly repeated.  Factor it out into
a fwd_scan_ports() function.

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

diff --git a/fwd.c b/fwd.c
index c7b768d5..6f89acf5 100644
--- a/fwd.c
+++ b/fwd.c
@@ -191,6 +191,26 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
 	bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map);
 }
 
+/**
+ * fwd_scan_ports() - Scan automatic port forwarding information
+ * @c:		Execution context
+ */
+static void fwd_scan_ports(struct ctx *c)
+{
+	if (c->tcp.fwd_out.mode == FWD_AUTO)
+		fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
+	if (c->tcp.fwd_in.mode == FWD_AUTO)
+		fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
+	if (c->udp.fwd_out.mode == FWD_AUTO) {
+		fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
+				   &c->tcp.fwd_out, &c->tcp.fwd_in);
+	}
+	if (c->udp.fwd_in.mode == FWD_AUTO) {
+		fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
+				   &c->tcp.fwd_in, &c->tcp.fwd_out);
+	}
+}
+
 /**
  * fwd_scan_ports_init() - Initial setup for automatic port forwarding
  * @c:		Execution context
@@ -207,25 +227,20 @@ void fwd_scan_ports_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);
-		fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
 	}
 	if (c->udp.fwd_in.mode == FWD_AUTO) {
 		c->udp.fwd_in.scan4 = open_in_ns(c, "/proc/net/udp", flags);
 		c->udp.fwd_in.scan6 = open_in_ns(c, "/proc/net/udp6", flags);
-		fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
-				   &c->tcp.fwd_in, &c->tcp.fwd_out);
 	}
 	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);
-		fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
 	}
 	if (c->udp.fwd_out.mode == FWD_AUTO) {
 		c->udp.fwd_out.scan4 = open("/proc/net/udp", flags);
 		c->udp.fwd_out.scan6 = open("/proc/net/udp6", flags);
-		fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
-				   &c->tcp.fwd_out, &c->tcp.fwd_in);
 	}
+	fwd_scan_ports(c);
 }
 
 /* Last time we scanned for open ports */
@@ -246,18 +261,7 @@ void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now)
 
 	scan_ports_run = *now;
 
-	if (c->tcp.fwd_out.mode == FWD_AUTO)
-		fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
-	if (c->tcp.fwd_in.mode == FWD_AUTO)
-		fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
-	if (c->udp.fwd_out.mode == FWD_AUTO) {
-		fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
-				   &c->tcp.fwd_out, &c->tcp.fwd_in);
-	}
-	if (c->udp.fwd_in.mode == FWD_AUTO) {
-		fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
-				   &c->tcp.fwd_in, &c->tcp.fwd_out);
-	}
+	fwd_scan_ports(c);
 
 	if (!c->no_tcp)
 		tcp_port_rebind_all(c);
-- 
2.51.0

[PATCH 6/8] fwd: Check forwarding mode in fwd_scan_ports_*() rather than caller

[David Gibson]

fwd_scan_ports() needs to check for FWD_AUTO mode before calling each
scan function - otherwise it would clobber the forwarding bitmap which
should retain the user's fixed configuration.

Make this slightly cleaner and safer by moving the mode check into
fwd_scan_ports_{tcp,udp}().

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

diff --git a/fwd.c b/fwd.c
index 6f89acf5..57941759 100644
--- a/fwd.c
+++ b/fwd.c
@@ -151,6 +151,9 @@ static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map)
 static void fwd_scan_ports_tcp(struct fwd_ports *fwd,
 			       const struct fwd_ports *rev)
 {
+	if (fwd->mode != FWD_AUTO)
+		return;
+
 	memset(fwd->map, 0, PORT_BITMAP_SIZE);
 	procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map);
 	procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map);
@@ -169,6 +172,9 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
 			       const struct fwd_ports *tcp_fwd,
 			       const struct fwd_ports *tcp_rev)
 {
+	if (fwd->mode != FWD_AUTO)
+		return;
+
 	memset(fwd->map, 0, PORT_BITMAP_SIZE);
 	procfs_scan_listen(fwd->scan4, UDP_LISTEN, fwd->map);
 	procfs_scan_listen(fwd->scan6, UDP_LISTEN, fwd->map);
@@ -197,18 +203,12 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
  */
 static void fwd_scan_ports(struct ctx *c)
 {
-	if (c->tcp.fwd_out.mode == FWD_AUTO)
-		fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
-	if (c->tcp.fwd_in.mode == FWD_AUTO)
-		fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
-	if (c->udp.fwd_out.mode == FWD_AUTO) {
-		fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
-				   &c->tcp.fwd_out, &c->tcp.fwd_in);
-	}
-	if (c->udp.fwd_in.mode == FWD_AUTO) {
-		fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
-				   &c->tcp.fwd_in, &c->tcp.fwd_out);
-	}
+	fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
+	fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
+	fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
+			   &c->tcp.fwd_out, &c->tcp.fwd_in);
+	fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
+			   &c->tcp.fwd_in, &c->tcp.fwd_out);
 }
 
 /**
-- 
2.51.0

[PATCH 7/8] fwd: Update all port maps before applying exclusions

[David Gibson]

In fwd_scan_ports() we go through each of the automatic forwarding cases
(tcp, udp, inbound and outbound) in turn, scanning and calculating the
new forwarding map.  However, to avoid avoid circular forwarding, some of
these maps affect each other.  This has the odd effect that the ones
handled earlier are based on the previous scan of other maps, whereas
the later ones are based on the latest scan.

That's not generally harmful, but it is counter-intuitive and results in a
few odd edge cases.  Avoid this by performing all the scans first, without
regard to other maps, then applying the exclusions afterwards.

One case has an extra wrinkle: for UDP we forwarded not just ports that
were listening on UDP but ones listening on TCP as well, for the benefit of
protocols like iperf3.  We therefore also excluded listening ports from
both UDP and TCP from the other direction to avoid circular forwarding.

This doesn't really make sense, though.  To avoid circular forwarding, we
don't care *why* the other side is listening on UDP, just that it *is*
listening.  I believe the explicit handling of the reverse TCP map was only
needed because the reverse map might have been one cycle out of date and
therefore not included a port opened because of the corresponding TCP port.

Now that we avoid that out of date map possibility, it's sufficient to
just mask out UDP listening ports in the other direction.

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

diff --git a/fwd.c b/fwd.c
index 57941759..395b0def 100644
--- a/fwd.c
+++ b/fwd.c
@@ -146,10 +146,8 @@ static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map)
 /**
  * fwd_scan_ports_tcp() - Scan /proc to update TCP forwarding map
  * @fwd:	Forwarding information to update
- * @rev:	Forwarding information for the reverse direction
  */
-static void fwd_scan_ports_tcp(struct fwd_ports *fwd,
-			       const struct fwd_ports *rev)
+static void fwd_scan_ports_tcp(struct fwd_ports *fwd)
 {
 	if (fwd->mode != FWD_AUTO)
 		return;
@@ -157,20 +155,15 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd,
 	memset(fwd->map, 0, PORT_BITMAP_SIZE);
 	procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map);
 	procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map);
-	bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
 }
 
 /**
  * fwd_scan_ports_udp() - Scan /proc to update UDP forwarding map
  * @fwd:	Forwarding information to update
- * @rev:	Forwarding information for the reverse direction
  * @tcp_fwd:	Corresponding TCP forwarding information
- * @tcp_rev:	TCP forwarding information for the reverse direction
  */
 static void fwd_scan_ports_udp(struct fwd_ports *fwd,
-			       const struct fwd_ports *rev,
-			       const struct fwd_ports *tcp_fwd,
-			       const struct fwd_ports *tcp_rev)
+			       const struct fwd_ports *tcp_fwd)
 {
 	if (fwd->mode != FWD_AUTO)
 		return;
@@ -186,15 +179,6 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
 	 */
 	procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map);
 	procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map);
-
-	/*
-	 * This means we need to skip numbers of TCP ports bound on the other
-	 * side, too. Otherwise, we would detect corresponding UDP ports as
-	 * bound and try to forward them from the opposite side, but it's
-	 * already us handling them.
-	 */
-	bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
-	bitmap_andc(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map);
 }
 
 /**
@@ -203,12 +187,28 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
  */
 static void fwd_scan_ports(struct ctx *c)
 {
-	fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
-	fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
-	fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
-			   &c->tcp.fwd_out, &c->tcp.fwd_in);
-	fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
-			   &c->tcp.fwd_in, &c->tcp.fwd_out);
+	fwd_scan_ports_tcp(&c->tcp.fwd_out);
+	fwd_scan_ports_tcp(&c->tcp.fwd_in);
+	fwd_scan_ports_udp(&c->udp.fwd_out, &c->tcp.fwd_out);
+	fwd_scan_ports_udp(&c->udp.fwd_in, &c->tcp.fwd_in);
+
+	if (c->tcp.fwd_out.mode == FWD_AUTO) {
+		bitmap_andc(c->tcp.fwd_out.map, PORT_BITMAP_SIZE,
+			    c->tcp.fwd_out.map, c->tcp.fwd_in.map);
+	}
+	if (c->tcp.fwd_in.mode == FWD_AUTO) {
+		bitmap_andc(c->tcp.fwd_in.map, PORT_BITMAP_SIZE,
+			    c->tcp.fwd_in.map, c->tcp.fwd_out.map);
+	}
+
+	if (c->udp.fwd_out.mode == FWD_AUTO) {
+		bitmap_andc(c->udp.fwd_out.map, PORT_BITMAP_SIZE,
+			    c->udp.fwd_out.map, c->udp.fwd_in.map);
+	}
+	if (c->udp.fwd_in.mode == FWD_AUTO) {
+		bitmap_andc(c->udp.fwd_in.map, PORT_BITMAP_SIZE,
+			    c->udp.fwd_in.map, c->udp.fwd_out.map);
+	}
 }
 
 /**
-- 
2.51.0

[PATCH 8/8] tcp, udp: Don't exclude ports in {tcp,udp}_port_rebind()

[David Gibson]

To avoid circular forwarding, {tcp,udp}_port_rebind() refuse to listen on
ports that we're already listening on in the reverse direction.  This is
redundant, because we already remove such ports from the forward map when
we scan.  I believe this was needed, because previously our reverse maps
might be one cycle out of date, so could be missing a newly appeared port.

We've now rearrange the port scanning code to avoid that, so we don't need
the check in tcp_port_rebind() any more.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c | 5 -----
 udp.c | 6 ------
 2 files changed, 11 deletions(-)

diff --git a/tcp.c b/tcp.c
index a330538d..99a8a4c0 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2817,7 +2817,6 @@ int tcp_init(struct ctx *c)
 static void tcp_port_rebind(struct ctx *c, bool outbound)
 {
 	const uint8_t *fmap = outbound ? c->tcp.fwd_out.map : c->tcp.fwd_in.map;
-	const uint8_t *rmap = outbound ? c->tcp.fwd_in.map : c->tcp.fwd_out.map;
 	int (*socks)[IP_VERSIONS] = outbound ? tcp_sock_ns : tcp_sock_init_ext;
 	unsigned port;
 
@@ -2836,10 +2835,6 @@ static void tcp_port_rebind(struct ctx *c, bool outbound)
 			continue;
 		}
 
-		/* Don't loop back our own ports */
-		if (bitmap_isset(rmap, port))
-			continue;
-
 		if ((c->ifi4 && socks[port][V4] == -1) ||
 		    (c->ifi6 && socks[port][V6] == -1)) {
 			if (outbound)
diff --git a/udp.c b/udp.c
index e27b2219..4af98503 100644
--- a/udp.c
+++ b/udp.c
@@ -1191,8 +1191,6 @@ static void udp_port_rebind(struct ctx *c, bool outbound)
 	int (*socks)[NUM_PORTS] = outbound ? udp_splice_ns : udp_splice_init;
 	const uint8_t *fmap
 		= outbound ? c->udp.fwd_out.map : c->udp.fwd_in.map;
-	const uint8_t *rmap
-		= outbound ? c->udp.fwd_in.map : c->udp.fwd_out.map;
 	unsigned port;
 
 	for (port = 0; port < NUM_PORTS; port++) {
@@ -1210,10 +1208,6 @@ static void udp_port_rebind(struct ctx *c, bool outbound)
 			continue;
 		}
 
-		/* Don't loop back our own ports */
-		if (bitmap_isset(rmap, port))
-			continue;
-
 		if ((c->ifi4 && socks[V4][port] == -1) ||
 		    (c->ifi6 && socks[V6][port] == -1))
 			udp_sock_init(c, outbound, NULL, NULL, port);
-- 
2.51.0