Re: [PATCH v7 15/18] pesto: Parse and add new rules from command line

[Laurent Vivier <lvivier@redhat.com>]

On 5/5/26 01:11, Stefano Brivio wrote:
> From: David Gibson <david@gibson.dropbear.id.au>
> 
> This adds parsing of options using fwd_rule_parse(), validates them and
> adds them to the existing rules. It doesn't yet send those rules back to
> passt or pasta.
> 
> Message-ID: <20260322141843.4095972-3-sbrivio@redhat.com>
> [dwg: Based on an early draft by Stefano]
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> [sbrivio: Recycled usage messages for -T and -U from conf.c as
>   suggested by Laurent, dropped unrelated whitespace change]
> [sbrivio: Add description of -t, -u, -T, -U to pesto.1]

-T, -U, -s are still missing in pesto.1

otherwise:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Thanks,
Laurent

> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>   Makefile   |   1 +
>   fwd_rule.c |   2 +-
>   fwd_rule.h |   1 +
>   pesto.1    | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   pesto.c    | 111 ++++++++++++++++++++++++++++++++++++++++++++++++--
>   5 files changed, 227 insertions(+), 5 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9e99dd1..c746b55 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -226,6 +226,7 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck
>   passt.cppcheck: BASE_CPPFLAGS += -UPESTO
>   passt.cppcheck: CPPCHECK_FLAGS += \
>   	--suppress=unusedFunction:fwd_rule.c \
> +	--suppress=staticFunction:fwd_rule.c \
>   	--suppress=unusedFunction:serialise.c
>   passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h
>   
> diff --git a/fwd_rule.c b/fwd_rule.c
> index c2824d5..b55e4df 100644
> --- a/fwd_rule.c
> +++ b/fwd_rule.c
> @@ -187,7 +187,7 @@ static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule *
>    *
>    * Return: 0 on success, negative error code on failure
>    */
> -static int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
> +int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
>   {
>   	/* Flags which can be set from the caller */
>   	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
> diff --git a/fwd_rule.h b/fwd_rule.h
> index 330d49e..f43b37d 100644
> --- a/fwd_rule.h
> +++ b/fwd_rule.h
> @@ -103,6 +103,7 @@ const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size);
>   void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd);
>   int fwd_rule_read(int fd, struct fwd_rule *rule);
>   int fwd_rule_write(int fd, const struct fwd_rule *rule);
> +int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new);
>   
>   /**
>    * fwd_rules_dump() - Dump forwarding rules
> diff --git a/pesto.1 b/pesto.1
> index b06433d..32d6ed1 100644
> --- a/pesto.1
> +++ b/pesto.1
> @@ -31,6 +31,123 @@ Be verbose.
>   .BR \-h ", " \-\-help
>   Display a help message and exit.
>   
> +.TP
> +.BR \-t ", " \-\-tcp-ports " " \fIspec
> +Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of:
> +.RS
> +
> +.TP
> +.BR none
> +Don't forward any ports
> +
> +.TP
> +[\fIaddress\fR[\fB%\fR\fIinterface\fR]\fB/\fR]\fIports\fR ...
> +Specific ports to forward.  Optionally, a specific listening address
> +and interface name (since Linux 5.7) can be specified.  \fIports\fR
> +may be either:
> +.RS
> +.TP
> +\fBall\fR
> +Forward all unbound, non-ephemeral ports, as permitted by current capabilities.
> +No failures are reported for unavailable ports, unless no ports could be
> +forwarded at all.
> +.RE
> +
> +.RS
> +or a comma-separated list of entries which may be any of:
> +.TP
> +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItofirst\fR[\fB-\fR\fItolast\fR]]
> +Include range. Forward port numbers between \fIfirst\fR and \fIlast\fR
> +(inclusive) to ports between \fItofirst\fR and \fItolast\fR.  If
> +\fItofirst\fR and \fItolast\fR are omitted, assume the same as
> +\fIfirst\fR and \fIlast\fR.  If \fIlast\fR is omitted, assume the same
> +as \fIfirst\fR.
> +
> +.TP
> +\fB~\fR\fIfirst\fR[\fB-\fR\fIlast\fR]
> +Exclude range.  Don't forward port numbers between \fIfirst\fR and
> +\fIlast\fR.  This takes precedences over include ranges.
> +
> +.TP
> +.BR auto
> +\fBpasta\fR only.  Only forward ports in the specified set if the
> +target ports are bound in the namespace. The list of ports is
> +periodically derived (every second) from listening sockets reported by
> +\fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5).
> +.RE
> +
> +Specifying excluded ranges only implies that all other non-ephemeral
> +ports are forwarded. Specifying no ranges at all implies forwarding
> +all non-ephemeral ports permitted by current capabilities.  In this
> +case, no failures are reported for unavailable ports, unless no ports
> +could be forwarded at all.
> +
> +Examples:
> +.RS
> +.TP
> +-t all
> +Forward all unbound, non-ephemeral ports as permitted by current
> +capabilities to the corresponding port on the guest or namespace
> +.TP
> +-t ::1/all
> +For the local address ::1, forward all unbound, non-ephemeral ports as
> +permitted by current capabilities
> +.TP
> +-t 22
> +Forward local port 22 to port 22 on the guest or namespace
> +.TP
> +-t 22:23
> +Forward local port 22 to port 23 on the guest or namespace
> +.TP
> +-t 22,25
> +Forward local ports 22 and 25 to ports 22 and 25 on the guest or namespace
> +.TP
> +-t 22-80
> +Forward local ports between 22 and 80 to corresponding ports on the guest or
> +namespace
> +.TP
> +-t 22-80:32-90
> +Forward local ports between 22 and 80 to ports between 32 and 90 on the guest or
> +namespace
> +.TP
> +-t 192.0.2.1/22
> +Forward local port 22, bound to 192.0.2.1, to port 22 on the guest or namespace
> +.TP
> +-t 192.0.2.1%eth0/22
> +Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22
> +.TP
> +-t %eth0/22
> +Forward local port 22, bound to any address on interface eth0, to port 22
> +.TP
> +-t 2000-5000,~3000-3010
> +Forward local ports between 2000 and 5000, except for those between 3000 and
> +3010
> +.TP
> +-t 192.0.2.1/20-30,~25
> +For the local address 192.0.2.1, forward ports between 20 and 24 and between 26
> +and 30
> +.TP
> +-t ~20000-20010
> +Forward all ports to the guest, except for the range from 20000 to 20010
> +.TP
> +-t auto
> +Automatically forward any ports which are bound in the namespace
> +.TP
> +-t ::1/auto
> +Automatically forward any ports which are bound in the namespace,
> +listening only on local port ::1
> +.TP
> +-t 8000-8010,auto
> +Forward ports in the range 8000-8010 if and only if they are bound in
> +the namespace
> +.RE
> +.RE
> +
> +.TP
> +.BR \-u ", " \-\-udp-ports " " \fIspec
> +Configure UDP port forwarding to guest. \fIspec\fR is as described for TCP
> +above.
> +
>   .TP
>   .BR \-\-version
>   Show version and exit.
> diff --git a/pesto.c b/pesto.c
> index 92a8cb2..16b3a5a 100644
> --- a/pesto.c
> +++ b/pesto.c
> @@ -55,6 +55,43 @@ static void usage(const char *name, FILE *f, int status)
>   	FPRINTF(f, "Usage: %s [OPTION]... PATH\n", name);
>   	FPRINTF(f,
>   		"\n"
> +		"  -t, --tcp-ports SPEC	TCP inbound port forwarding\n"
> +		"    can be specified multiple times\n"
> +		"    SPEC can be:\n"
> +		"      'none': don't forward any ports\n"
> +		"      [ADDR[%%IFACE]/]PORTS: forward specific ports\n"
> +		"        PORTS is either 'all' (forward all unbound, non-ephemeral\n"
> +		"        ports), or a comma-separated list of ports, optionally\n"
> +		"        ranged with '-' and optional target ports after ':'.\n"
> +		"        Ranges can be reduced by excluding ports or ranges\n"
> +		"        prefixed by '~'.\n"
> +		"        The 'auto' keyword may be given to only forward\n"
> +		"        ports which are bound in the target namespace\n"
> +		"        Examples:\n"
> +		"        -t all         Forward all ports\n"
> +		"        -t 127.0.0.1/all Forward all ports from local address\n"
> +		"                         127.0.0.1\n"
> +		"        -t 22		Forward local port 22 to 22\n"
> +		"        -t 22:23	Forward local port 22 to 23\n"
> +		"        -t 22,25	Forward ports 22, 25 to ports 22, 25\n"
> +		"        -t 22-80  	Forward ports 22 to 80\n"
> +		"        -t 22-80:32-90	Forward ports 22 to 80 to\n"
> +		"			corresponding port numbers plus 10\n"
> +		"        -t 192.0.2.1/5	Bind port 5 of 192.0.2.1\n"
> +		"        -t 5-25,~10-20	Forward ports 5 to 9, and 21 to 25\n"
> +		"        -t ~25		Forward all ports except for 25\n"
> +		"        -t auto	Forward all ports bound in namespace\n"
> +		"        -t 192.0.2.2/auto Forward ports from 192.0.2.2 if\n"
> +		"                          they are bound in the namespace\n"
> +		"        -t 8000-8010,auto Forward ports 8000-8010 if they\n"
> +		"                          are bound in the namespace\n"
> +		"  -u, --udp-ports SPEC	UDP inbound port forwarding\n"
> +		"    SPEC is as described for TCP above\n"
> +		"  -T, --tcp-ns SPEC	TCP port forwarding to init namespace\n"
> +		"    SPEC is as described above\n"
> +		"  -U, --udp-ns SPEC	UDP port forwarding to init namespace\n"
> +		"    SPEC is as described above\n"
> +		"  -s, --show		Show configuration before and after\n"
>   		"  -d, --debug		Print debugging messages\n"
>   		"  -h, --help		Display this help message and exit\n"
>   		"  --version		Show version and exit\n");
> @@ -207,6 +244,8 @@ static void show_conf(const struct configuration *conf)
>   		fwd_rules_dump(printf, pc->fwd.rules, pc->fwd.count,
>   			       "    ", "\n");
>   	}
> +	/* Flush stdout, so this doesn't get misordered with later debug()s */
> +	(void)fflush(stdout);
>   }
>   
>   /**
> @@ -218,7 +257,7 @@ static void show_conf(const struct configuration *conf)
>    *
>    * #syscalls:pesto socket s390x:socketcall i686:socketcall
>    * #syscalls:pesto connect shutdown close
> - * #syscalls:pesto exit_group fstat read write
> + * #syscalls:pesto exit_group fstat read write openat
>    */
>   int main(int argc, char **argv)
>   {
> @@ -226,11 +265,18 @@ int main(int argc, char **argv)
>   		{"debug",	no_argument,		NULL,		'd' },
>   		{"help",	no_argument,		NULL,		'h' },
>   		{"version",	no_argument,		NULL,		1 },
> +		{"tcp-ports",	required_argument,	NULL,		't' },
> +		{"udp-ports",	required_argument,	NULL,		'u' },
> +		{"tcp-ns",	required_argument,	NULL,		'T' },
> +		{"udp-ns",	required_argument,	NULL,		'U' },
> +		{"show",	no_argument,		NULL,		's' },
>   		{ 0 },
>   	};
> +	struct pif_configuration *inbound, *outbound;
>   	struct sockaddr_un a = { AF_UNIX, "" };
> +	const char *optstring = "dht:u:T:U:s";
>   	struct configuration conf = { 0 };
> -	const char *optstring = "dh";
> +	bool update = false, show = false;
>   	struct pesto_hello hello;
>   	struct sock_fprog prog;
>   	int optname, ret, s;
> @@ -251,6 +297,8 @@ int main(int argc, char **argv)
>   	if (setvbuf(stdout, stdout_buf, _IOFBF, sizeof(stdout_buf)))
>   		die_perror("Failed to set stdout buffer");
>   
> +	fwd_probe_ephemeral();
> +
>   	do {
>   		optname = getopt_long(argc, argv, optstring, options, NULL);
>   
> @@ -258,6 +306,16 @@ int main(int argc, char **argv)
>   		case -1:
>   		case 0:
>   			break;
> +		case 't':
> +		case 'u':
> +		case 'T':
> +		case 'U':
> +			/* Parse these options after we've read state from passt/pasta */
> +			update = true;
> +			break;
> +		case 's':
> +			show = true;
> +			break;
>   		case 'h':
>   			usage(argv[0], stdout, EXIT_SUCCESS);
>   			break;
> @@ -290,6 +348,8 @@ int main(int argc, char **argv)
>   		die_perror("Failed to connect to %s", a.sun_path);
>   	}
>   
> +	debug("Connected to passt/pasta control socket");
> +
>   	ret = read_all_buf(s, &hello, sizeof(hello));
>   	if (ret < 0)
>   		die_perror("Couldn't read server greeting");
> @@ -327,9 +387,52 @@ int main(int argc, char **argv)
>   	while (read_pif_conf(s, &conf))
>   		;
>   
> -	printf("passt/pasta configuration (%s)\n", a.sun_path);
> -	show_conf(&conf);
> +	if (!update) {
> +		printf("passt/pasta configuration (%s)\n", a.sun_path);
> +		show_conf(&conf);
> +		goto noupdate;
> +	}
> +
> +	if (show) {
> +		printf("Previous configuration (%s)\n", a.sun_path);
> +		show_conf(&conf);
> +	}
> +
> +	inbound = pif_conf_by_name(&conf, "HOST");
> +	outbound = pif_conf_by_name(&conf, "SPLICE");
> +
> +	optind = 0;
> +	do {
> +		optname = getopt_long(argc, argv, optstring, options, NULL);
> +
> +		switch (optname) {
> +		case 't':
> +		case 'u':
> +			if (!inbound) {
> +				die("Can't use -%c, no inbound interface",
> +				    optname);
> +			}
> +			fwd_rule_parse(optname, optarg, &inbound->fwd);
> +			break;
> +		case 'T':
> +		case 'U':
> +			if (!outbound) {
> +				die("Can't use -%c, no outbound interface",
> +				    optname);
> +			}
> +			fwd_rule_parse(optname, optarg, &outbound->fwd);
> +			break;
> +		default:
> +			continue;
> +		}
> +	} while (optname != -1);
> +
> +	if (show) {
> +		printf("Updated configuration (%s)\n", a.sun_path);
> +		show_conf(&conf);
> +	}
>   
> +noupdate:
>   	if (shutdown(s, SHUT_RDWR) < 0 || close(s) < 0)
>   		die_perror("Error shutting down control socket");
>