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

On Wed, 6 May 2026 19:52:27 +0200 Paul Holzinger wrote:

Hi,

so I was testing these patches and found one small "problem".

On 06/05/2026 15:23, Stefano Brivio wrote:

...

From: David Gibson

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 [sbrivio: Avoid potential recursive calling between conf_accept() and conf_close(), reported by clang-tidy] [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()] [sbrivio: Switch to protocol version 1, and reflect the true magic behind pesto, i.e. basil, into the magic string] [sbrivio: Fix conflicts in the Makefile caused by the fact that I'm not merging a previous series reworking it] Signed-off-by: Stefano Brivio Reviewed-by: Laurent Vivier --- Makefile | 2 +- conf.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++- conf.h | 2 + epoll_type.h | 4 ++ passt.1 | 5 ++ passt.c | 8 +++ passt.h | 6 ++ pesto.c | 47 ++++++++++++- pesto.h | 22 ++++++ serialise.c | 3 + 10 files changed, 279 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile index 2639472..b1003d8 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,7 @@ PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ vhost_user.c virtio.c vu_common.c QRAP_SRCS = qrap.c PASST_REPAIR_SRCS = passt-repair.c -PESTO_SRCS = pesto.c +PESTO_SRCS = pesto.c serialise.c SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_SRCS)

MANPAGES = passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1 diff --git a/conf.c b/conf.c index 27aded8..9eed1ec 100644 --- a/conf.c +++ b/conf.c @@ -48,6 +48,10 @@ #include "isolation.h" #include "log.h" #include "vhost_user.h" +#include "epoll_ctl.h" +#include "conf.h" +#include "pesto.h" +#include "serialise.h"

#define NETNS_RUN_DIR "/run/netns"

@@ -541,6 +545,7 @@ static void usage(const char *name, FILE *f, int status) " --runas UID|UID:GID Run as given UID, GID, which can be\n" " numeric, or login and group names\n" " default: drop to user \"nobody\"\n" + " -c, --conf-path PATH Configuration socket path\n" " -h, --help Display this help message and exit\n" " --version Show version and exit\n");

@@ -779,6 +784,9 @@ static void conf_print(const struct ctx *c) char buf[INANY_ADDRSTRLEN]; int i;

+ if (c->fd_control_listen >= 0) + info("Configuration socket: %s", c->control_path); + if (c->ifi4 > 0 || c->ifi6 > 0) { char ifn[IFNAMSIZ];

@@ -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"); + } else { + c->fd_control_listen = -1; + } }

/** @@ -1107,6 +1128,25 @@ fail: die("Invalid MAC address: %s", str); }

+/** + * conf_sock_listen() - Start listening for connections on configuration socket + * @c: Execution context + */ +static void conf_sock_listen(const struct ctx *c) +{ + union epoll_ref ref = { .type = EPOLL_TYPE_CONF_LISTEN }; + + if (c->fd_control_listen < 0) + return; + + if (listen(c->fd_control_listen, 0)) + die_perror("Couldn't listen on configuration socket"); + + ref.fd = c->fd_control_listen; + if (epoll_add(c->epollfd, EPOLLIN | EPOLLET, ref)) + die_perror("Couldn't add configuration socket to epoll"); +} + /** * conf() - Process command-line arguments and set configuration * @c: Execution context @@ -1189,9 +1229,10 @@ void conf(struct ctx *c, int argc, char **argv) {"migrate-exit", no_argument, NULL, 29 }, {"migrate-no-linger", no_argument, NULL, 30 }, {"stats", required_argument, NULL, 31 }, + {"conf-path", required_argument, NULL, 'c' }, { 0 }, }; - const char *optstring = "+dqfel:hs:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; + const char *optstring = "+dqfel:hs:c:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; const char *logname = (c->mode == MODE_PASTA) ? "pasta" : "passt"; bool opt_t = false, opt_T = false, opt_u = false, opt_U = false; char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 }; @@ -1449,6 +1490,13 @@ void conf(struct ctx *c, int argc, char **argv)

c->fd_tap = -1; break; + case 'c': + ret = snprintf(c->control_path, sizeof(c->control_path), + "%s", optarg); + if (ret <= 0 || ret >= (int)sizeof(c->control_path)) + die("Invalid configuration path: %s", optarg); + c->fd_control_listen = c->fd_control = -1; + break; case 'F': errno = 0; fd_tap_opt = strtol(optarg, NULL, 0); @@ -1871,6 +1919,140 @@ void conf(struct ctx *c, int argc, char **argv) fwd_rule_parse('U', "auto", c->fwd[PIF_SPLICE]); }

+ conf_sock_listen(c); + if (!c->quiet) conf_print(c); } + +static void conf_accept(struct ctx *c); + +/** + * conf_close() - Close configuration / control socket and clean up + * @c: Execution context + */ +static void conf_close(struct ctx *c) +{ + debug("Closing configuration socket"); + epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_control, NULL); + close(c->fd_control); + c->fd_control = -1; +} + +/** + * conf_listen_handler() - Handle events on configuration listening socket + * @c: Execution context + * @events: epoll events + */ +void conf_listen_handler(struct ctx *c, uint32_t events) +{ + if (events != EPOLLIN) { + err("Unexpected event 0x%04x on configuration socket", events); + return; + } + + if (c->fd_control >= 0) { + /* Ignore the new connection for now, blocking it until the + * current one finishes. + */ + return; + } + + conf_accept(c); +} + +/** + * conf_accept() - Accept a new control connection + * @c: Execution context + */ +static void conf_accept(struct ctx *c) +{ + struct pesto_hello hello = { + .magic = PESTO_SERVER_MAGIC, + .version = htonl(PESTO_PROTOCOL_VERSION), + }; + union epoll_ref ref = { .type = EPOLL_TYPE_CONF }; + struct ucred uc = { 0 }; + socklen_t len = sizeof(uc); + int fd, rc; + +retry: + err("%s: %i", __func__, __LINE__); + fd = accept4(c->fd_control_listen, NULL, NULL, SOCK_CLOEXEC); + if (fd < 0) { + err("%s: %i", __func__, __LINE__); + if (errno != EAGAIN) + warn_perror("accept4() on configuration listening socket"); + return; + } + + err("%s: %i", __func__, __LINE__); I assume the three err() calls are debug leftovers? I was wondering why my journal was getting spammed with "conf_accept: XXX".

Oops, definitely left-overs, I spotted those as well and I was fairly sure I dropped them but here they are again... fixing in v11. -- Stefano