[PATCH v2 0/8] Cleanups to auto port scanning

[PATCH v2 0/8] 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.

v2:
 * Rename bitmap_andc() to bitmap_and_not() (4/8)
 * Fixed comment formatting error (4/8)
 * Updated commit message based on further information from Stefano
   (7,8/8)
 * All other patches unchanged, except for trivial rebase fixes

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  |  24 +++++++++++++
 util.h  |   2 ++
 10 files changed, 131 insertions(+), 91 deletions(-)

-- 
2.51.0

[PATCH v2 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 dc3e1141..5d3ad55c 100644
--- a/passt.c
+++ b/passt.c
@@ -57,8 +57,7 @@
 
 #define NUM_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 v2 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 539b2901..6e248fe8 100644
--- a/fwd.c
+++ b/fwd.c
@@ -442,6 +442,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 352f3b54..ec7bb3bb 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 5d3ad55c..4964427d 100644
--- a/passt.c
+++ b/passt.c
@@ -57,7 +57,7 @@
 
 #define NUM_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)));
@@ -119,11 +119,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 8cf44590..fc1a2610 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2884,25 +2884,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 c1b8385f..9cd736d7 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);
diff --git a/udp.c b/udp.c
index f052a0c8..d95ef2f6 100644
--- a/udp.c
+++ b/udp.c
@@ -1252,28 +1252,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 dd6e5ad6..e3fb78ba 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);
 
 /**
-- 
2.51.0

[PATCH v2 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 6e248fe8..523097b8 100644
--- a/fwd.c
+++ b/fwd.c
@@ -365,7 +365,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);
@@ -379,9 +380,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];
 
@@ -460,10 +462,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 ec7bb3bb..77925822 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 fc1a2610..c35c1c3f 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2884,22 +2884,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 9cd736d7..00823867 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 d95ef2f6..8cff8809 100644
--- a/udp.c
+++ b/udp.c
@@ -1252,24 +1252,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 e3fb78ba..f1d83f38 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);
 
 /**
-- 
2.51.0

[PATCH v2 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_and_not()
helper.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c  | 33 ++++++++++++++-------------------
 util.c | 24 ++++++++++++++++++++++++
 util.h |  2 ++
 3 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/fwd.c b/fwd.c
index 523097b8..279b7ddd 100644
--- a/fwd.c
+++ b/fwd.c
@@ -322,13 +322,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;
@@ -353,10 +351,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);
 	}
 }
 
@@ -369,8 +364,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_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
 }
 
 /**
@@ -385,26 +381,25 @@ 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.
-	 *
-	 * This means we need to skip numbers of TCP ports bound on the other
+	 */
+	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_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
+	bitmap_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map);
 }
 
 /**
diff --git a/util.c b/util.c
index 707ab338..44c21a3e 100644
--- a/util.c
+++ b/util.c
@@ -309,6 +309,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);
@@ -338,6 +339,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;
@@ -352,6 +354,28 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
 		dst[i] = a[i] | b[i];
 }
 
+/**
+ * bitmap_and_not() - Logical conjunction with complement (AND NOT) of bitmap
+ * @dst:	Pointer to result bitmap
+ * @size:	Size of bitmaps, in bytes
+ * @a:		First operand
+ * @b:		Second operand
+ */
+void bitmap_and_not(uint8_t *dst, size_t size,
+		   const uint8_t *a, const uint8_t *b)
+{
+	unsigned long *dw = (unsigned long *)dst;
+	unsigned long *aw = (unsigned long *)a;
+	unsigned long *bw = (unsigned long *)b;
+	size_t i;
+
+	for (i = 0; i < size / sizeof(long); i++, dw++, aw++, bw++)
+		*dw = *aw & ~*bw;
+
+	for (i = size / sizeof(long) * sizeof(long); i < size; i++)
+		dst[i] = a[i] & ~b[i];
+}
+
 /**
  * ns_enter() - Enter configured user (unless already joined) and network ns
  * @c:		Execution context
diff --git a/util.h b/util.h
index 689b76a6..a0b2ada6 100644
--- a/util.h
+++ b/util.h
@@ -218,6 +218,8 @@ void bitmap_set(uint8_t *map, unsigned bit);
 void bitmap_clear(uint8_t *map, unsigned bit);
 bool bitmap_isset(const uint8_t *map, unsigned bit);
 void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b);
+void bitmap_and_not(uint8_t *dst, size_t size,
+		    const uint8_t *a, const uint8_t *b);
 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 v2 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 279b7ddd..40453086 100644
--- a/fwd.c
+++ b/fwd.c
@@ -402,6 +402,26 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
 	bitmap_and_not(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
@@ -418,25 +438,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 */
@@ -457,18 +472,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 v2 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 40453086..7b6c40fb 100644
--- a/fwd.c
+++ b/fwd.c
@@ -363,6 +363,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);
@@ -381,6 +384,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);
@@ -408,18 +414,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 v2 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.  This 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 | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/fwd.c b/fwd.c
index 7b6c40fb..68bb1166 100644
--- a/fwd.c
+++ b/fwd.c
@@ -358,10 +358,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;
@@ -369,20 +367,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_and_not(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;
@@ -398,14 +391,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_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
-	bitmap_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map);
 }
 
 /**
@@ -414,12 +399,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_and_not(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_and_not(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_and_not(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_and_not(c->udp.fwd_in.map, PORT_BITMAP_SIZE,
+			       c->udp.fwd_in.map, c->udp.fwd_out.map);
+	}
 }
 
 /**
-- 
2.51.0

[PATCH v2 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.  This was needed previously, because our reverse maps might have
been one cycle out of date, so could be missing a newly appeared port.

We've now rearranged 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 c35c1c3f..e91c0cf5 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2832,7 +2832,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;
 
@@ -2851,10 +2850,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 8cff8809..9c009502 100644
--- a/udp.c
+++ b/udp.c
@@ -1204,8 +1204,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++) {
@@ -1223,10 +1221,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

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

[Stefano Brivio]

On Fri, 31 Oct 2025 15:19:22 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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.
> 
> v2:
>  * Rename bitmap_andc() to bitmap_and_not() (4/8)
>  * Fixed comment formatting error (4/8)
>  * Updated commit message based on further information from Stefano
>    (7,8/8)
>  * All other patches unchanged, except for trivial rebase fixes

Applied.

-- 
Stefano