public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v2 1/9] conf: Cleaner initialisation of default forwarding modes
Date: Fri,  3 Nov 2023 13:22:55 +1100	[thread overview]
Message-ID: <20231103022303.968638-2-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20231103022303.968638-1-david@gibson.dropbear.id.au>

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


  reply	other threads:[~2023-11-03  2:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-03  2:22 [PATCH v2 0/9] Clean ups to automatic port forwarding David Gibson
2023-11-03  2:22 ` David Gibson [this message]
2023-11-03  2:22 ` [PATCH v2 2/9] port_fwd: Move automatic port forwarding code to port_fwd.[ch] David Gibson
2023-11-03  2:22 ` [PATCH v2 3/9] port_fwd: Better parameterise procfs_scan_listen() David Gibson
2023-11-03  2:22 ` [PATCH v2 4/9] util: Add open_in_ns() helper David Gibson
2023-11-03  2:22 ` [PATCH v2 5/9] port_fwd: Pre-open /proc/net/* files rather than on-demand David Gibson
2023-11-03  2:23 ` [PATCH v2 6/9] port_fwd: Don't NS_CALL get_bound_ports() David Gibson
2023-11-03  2:23 ` [PATCH v2 7/9] port_fwd: Split TCP and UDP cases for get_bound_ports() David Gibson
2023-11-03  2:23 ` [PATCH v2 8/9] port_fwd: Move port scanning /proc fds into struct port_fwd David Gibson
2023-11-03  2:23 ` [PATCH v2 9/9] port_fwd: Simplify get_bound_ports_*() to port_fwd_scan_*() David Gibson
2023-11-07 12:44 ` [PATCH v2 0/9] Clean ups to automatic port forwarding Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231103022303.968638-2-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).