[PATCH] conf, tap: Add option to quit once the client closes the connection

[PATCH] conf, tap: Add option to quit once the client closes the connection

[Stefano Brivio]

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

This is practical to avoid explicit lifecycle management in users,
e.g. libvirtd, and is trivial to implement.

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
 conf.c  | 17 ++++++++++++++++-
 passt.1 |  5 +++++
 passt.h |  1 +
 tap.c   |  5 +++++
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/conf.c b/conf.c
index 4ec3153..53a8dcf 100644
--- a/conf.c
+++ b/conf.c
@@ -712,6 +712,7 @@ static void usage(const char *name)
 	if (strstr(name, "pasta"))
 		goto pasta_opts;
 
+	info(   "  -1, --one-off	Quit after handling one single client");
 	info(   "  -t, --tcp-ports SPEC	TCP port forwarding to guest");
 	info(   "    can be specified multiple times");
 	info(   "    SPEC can be:");
@@ -1024,6 +1025,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"no-map-gw",	no_argument,		&c->no_map_gw,	1 },
 		{"ipv4-only",	no_argument,		NULL,		'4' },
 		{"ipv6-only",	no_argument,		NULL,		'6' },
+		{"one-off",	no_argument,		NULL,		'1' },
 		{"tcp-ports",	required_argument,	NULL,		't' },
 		{"udp-ports",	required_argument,	NULL,		'u' },
 		{"tcp-ns",	required_argument,	NULL,		'T' },
@@ -1062,7 +1064,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
 		optstring = "vdqfel:hI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
 	} else {
-		optstring = "vdqfel:hs:p:P:m:a:n:M:g:i:D:S:46t:u:";
+		optstring = "vdqfel:hs:p:P:m:a:n:M:g:i:D:S:461t:u:";
 	}
 
 	c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0;
@@ -1488,6 +1490,19 @@ void conf(struct ctx *c, int argc, char **argv)
 		case '6':
 			v6_only = true;
 			break;
+		case '1':
+			if (c->mode != MODE_PASST) {
+				err("--one-off is for passt mode only");
+				usage(argv[0]);
+			}
+
+			if (c->one_off) {
+				err("Redundant --one-off option");
+				usage(argv[0]);
+			}
+
+			c->one_off = true;
+			break;
 		case 't':
 		case 'u':
 		case 'T':
diff --git a/passt.1 b/passt.1
index c63a439..41f8836 100644
--- a/passt.1
+++ b/passt.1
@@ -302,6 +302,11 @@ Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to
 Default is to probe a free socket, not accepting connections, starting from
 \fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR.
 
+.TP
+.BR \-1 ", " \-\-one-off
+Quit after handling a single client connection, that is, once the client closes
+the socket, or once we get a socket error.
+
 .TP
 .BR \-t ", " \-\-tcp-ports " " \fIspec
 Configure TCP port forwarding to guest. \fIspec\fR can be one of:
diff --git a/passt.h b/passt.h
index de79e7b..67281db 100644
--- a/passt.h
+++ b/passt.h
@@ -196,6 +196,7 @@ struct ctx {
 	char sock_path[UNIX_PATH_MAX];
 	char pcap[PATH_MAX];
 	char pid_file[PATH_MAX];
+	int one_off;
 
 	int pasta_netns_fd;
 
diff --git a/tap.c b/tap.c
index bdcc161..78de42c 100644
--- a/tap.c
+++ b/tap.c
@@ -1011,5 +1011,10 @@ void tap_handler(struct ctx *c, int fd, uint32_t events,
 
 	return;
 reinit:
+	if (c->one_off) {
+		info("Client closed connection, exiting");
+		exit(EXIT_SUCCESS);
+	}
+
 	tap_sock_init(c);
 }
-- 
@@ -1011,5 +1011,10 @@ void tap_handler(struct ctx *c, int fd, uint32_t events,
 
 	return;
 reinit:
+	if (c->one_off) {
+		info("Client closed connection, exiting");
+		exit(EXIT_SUCCESS);
+	}
+
 	tap_sock_init(c);
 }
-- 
2.35.1

Re: [PATCH] conf, tap: Add option to quit once the client closes the connection

[David Gibson]

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

On Fri, Oct 07, 2022 at 09:16:41AM +0200, Stefano Brivio wrote:
> This is practical to avoid explicit lifecycle management in users,
> e.g. libvirtd, and is trivial to implement.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

I actually wonder if this should be the default/only behaviour.

> ---
>  conf.c  | 17 ++++++++++++++++-
>  passt.1 |  5 +++++
>  passt.h |  1 +
>  tap.c   |  5 +++++
>  4 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/conf.c b/conf.c
> index 4ec3153..53a8dcf 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -712,6 +712,7 @@ static void usage(const char *name)
>  	if (strstr(name, "pasta"))
>  		goto pasta_opts;
>  
> +	info(   "  -1, --one-off	Quit after handling one single client");
>  	info(   "  -t, --tcp-ports SPEC	TCP port forwarding to guest");
>  	info(   "    can be specified multiple times");
>  	info(   "    SPEC can be:");
> @@ -1024,6 +1025,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  		{"no-map-gw",	no_argument,		&c->no_map_gw,	1 },
>  		{"ipv4-only",	no_argument,		NULL,		'4' },
>  		{"ipv6-only",	no_argument,		NULL,		'6' },
> +		{"one-off",	no_argument,		NULL,		'1' },
>  		{"tcp-ports",	required_argument,	NULL,		't' },
>  		{"udp-ports",	required_argument,	NULL,		'u' },
>  		{"tcp-ns",	required_argument,	NULL,		'T' },
> @@ -1062,7 +1064,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
>  		optstring = "vdqfel:hI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
>  	} else {
> -		optstring = "vdqfel:hs:p:P:m:a:n:M:g:i:D:S:46t:u:";
> +		optstring = "vdqfel:hs:p:P:m:a:n:M:g:i:D:S:461t:u:";
>  	}
>  
>  	c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0;
> @@ -1488,6 +1490,19 @@ void conf(struct ctx *c, int argc, char **argv)
>  		case '6':
>  			v6_only = true;
>  			break;
> +		case '1':
> +			if (c->mode != MODE_PASST) {
> +				err("--one-off is for passt mode only");
> +				usage(argv[0]);
> +			}
> +
> +			if (c->one_off) {
> +				err("Redundant --one-off option");
> +				usage(argv[0]);
> +			}
> +
> +			c->one_off = true;
> +			break;
>  		case 't':
>  		case 'u':
>  		case 'T':
> diff --git a/passt.1 b/passt.1
> index c63a439..41f8836 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -302,6 +302,11 @@ Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to
>  Default is to probe a free socket, not accepting connections, starting from
>  \fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR.
>  
> +.TP
> +.BR \-1 ", " \-\-one-off
> +Quit after handling a single client connection, that is, once the client closes
> +the socket, or once we get a socket error.
> +
>  .TP
>  .BR \-t ", " \-\-tcp-ports " " \fIspec
>  Configure TCP port forwarding to guest. \fIspec\fR can be one of:
> diff --git a/passt.h b/passt.h
> index de79e7b..67281db 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -196,6 +196,7 @@ struct ctx {
>  	char sock_path[UNIX_PATH_MAX];
>  	char pcap[PATH_MAX];
>  	char pid_file[PATH_MAX];
> +	int one_off;
>  
>  	int pasta_netns_fd;
>  
> diff --git a/tap.c b/tap.c
> index bdcc161..78de42c 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1011,5 +1011,10 @@ void tap_handler(struct ctx *c, int fd, uint32_t events,
>  
>  	return;
>  reinit:
> +	if (c->one_off) {
> +		info("Client closed connection, exiting");
> +		exit(EXIT_SUCCESS);
> +	}
> +
>  	tap_sock_init(c);
>  }

-- 
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] conf, tap: Add option to quit once the client closes the connection

[Stefano Brivio]

On Sat, 8 Oct 2022 14:55:53 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Oct 07, 2022 at 09:16:41AM +0200, Stefano Brivio wrote:
> > This is practical to avoid explicit lifecycle management in users,
> > e.g. libvirtd, and is trivial to implement.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> I actually wonder if this should be the default/only behaviour.

I also think it might make sense to have it as default behaviour: if
one runs another guest, quite likely they won't want to touch that
dirty, filthy process from the guest before.

On the other hand, what if one day we try and add the capability to
qemu to reconnect? I would check back later on this topic once we make
more progress with libvirt integration -- we should have more clarity
then.

-- 
Stefano