Re: [PATCH v8 10/19] pesto, conf: Have pesto connect to passt and check versions

[David Gibson <david@gibson.dropbear.id.au>]

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

On Wed, May 06, 2026 at 01:47:10AM +0200, Stefano Brivio wrote:
> From: David Gibson <david@gibson.dropbear.id.au>
> 
> Start implementing pesto in earnest.  Create a control/configuration
> socket in passt.  Have pesto connect to it and retrieve a server greeting
> Perform some basic version checking.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> [sbrivio: Avoid potential recursive calling between conf_accept() and
>  conf_close(), reported by clang-tidy]

Huh.  For some reason that warning didn't trip for me.  Although it's
technically true they can mutually recurse, I believe they're both
tail calls, so it shouldn't eat the stack.

> [sbrivio: In conf(), check we're not exceeding sizeof(c->control_path)
>  instead of sizeof(c->socket_path), and, in pesto's main(), print
>  argv[optind] instead of argv[1] to indicate an invalid socket path,
>  both reported by Jon Maloy]
> [sbrivio: In pesto's main(), drop unnecessary newline from error
>  message, reported by Laurent]
> [sbrivio: Don't use SOCK_NONBLOCK on accept4(), as that only applies
>  to the *new* file descriptor, which we don't want -- set O_NONBLOCK
>  on the listening file descriptor using fcntl()]

Making the new (accepted) socket non-blocking was the intended
behaviour here.  We also want non-blocking for the listening socket,
but that was already done in feab892c7 ("tap, repair: Use
SOCK_NONBLOCK and SOCK_CLOEXEC on Unix sockets").

WIth the current design, I guess we don't want non-blocking on the
accepted socket, although I don't think it actually matters very much.
We will want non-blocking it when we change this to read out the
updated rules incrementally, rather than all at once.

[snip]
> @@ -1072,6 +1080,19 @@ static void conf_open_files(struct ctx *c)
>  		if (c->pidfile_fd < 0)
>  			die_perror("Couldn't open PID file %s", c->pidfile);
>  	}
> +
> +	c->fd_control = -1;
> +	if (*c->control_path) {
> +		c->fd_control_listen = sock_unix(c->control_path);
> +		if (c->fd_control_listen < 0) {
> +			die_perror("Couldn't open control socket %s",
> +				   c->control_path);
> +		}
> +		if (fcntl(c->fd_control_listen, F_SETFL, O_NONBLOCK))
> +			die_perror("Couldn't set O_NONBLOCK on control socket");

So, this is unneccessary because of feab892c7.

-- 
David Gibson (he or they)	| 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 --]