public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH 5/5] pesto: Introduce stub configuration interface and tool
Date: Tue, 17 Mar 2026 11:48:50 +1100	[thread overview]
Message-ID: <abik8onHZ6_iIWU-@zatzit> (raw)
In-Reply-To: <20260317010245.4f5a8f33@elisabeth>

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

On Tue, Mar 17, 2026 at 01:02:46AM +0100, Stefano Brivio wrote:
> On Mon, 16 Mar 2026 16:46:29 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Add a control socket to passt/pasta for updating configuration at runtime.
> > Add a tool (pesto) for communicating with the control socket.
> > 
> > For now this is a stub implementation that forms the connection and sends
> > some version information, but nothing more.
> > Example:
> >   ./pasta --debug --config-net -c /tmp/pasta.conf -t none
> >   ./pesto /tmp/pasta.conf
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > [dwg: Based on an early draft from Stefano]
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  .gitignore   |   2 +
> >  Makefile     |  32 +++++++----
> >  conf.c       | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  conf.h       |   2 +
> >  epoll_type.h |   4 ++
> >  passt.1      |   5 ++
> >  passt.c      |   8 +++
> >  passt.h      |   6 +++
> >  pesto.1      |  47 ++++++++++++++++
> >  pesto.c      | 111 ++++++++++++++++++++++++++++++++++++++
> >  pesto.h      |  34 ++++++++++++
> >  pesto_util.c |  62 +++++++++++++++++++++
> >  pesto_util.h |  19 +++++++
> >  util.c       |  38 -------------
> >  util.h       |   5 +-
> >  15 files changed, 469 insertions(+), 54 deletions(-)
> >  create mode 100644 pesto.1
> >  create mode 100644 pesto.c
> >  create mode 100644 pesto.h
> >  create mode 100644 pesto_util.c
> >  create mode 100644 pesto_util.h
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 3c16adc7..3e40d9f7 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -4,9 +4,11 @@
> >  /pasta
> >  /pasta.avx2
> >  /passt-repair
> > +/pesto
> >  /qrap
> >  /pasta.1
> >  /seccomp.h
> > +/seccomp_pesto.h
> >  /seccomp_repair.h
> >  /c*.json
> >  README.plain.md
> > diff --git a/Makefile b/Makefile
> > index fe016f30..a528b34a 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -39,21 +39,25 @@ FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
> >  
> >  PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c \
> >  	flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c \
> > -	log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c \
> > -	pif.c repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c \
> > -	udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c
> > +	log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c \
> > +	pesto_util.c pcap.c pif.c repair.c tap.c tcp.c tcp_buf.c tcp_splice.c \
> > +	tcp_vu.c udp.c udp_flow.c udp_vu.c util.c vhost_user.c virtio.c \
> > +	vu_common.c
> >  QRAP_SRCS = qrap.c
> >  PASST_REPAIR_SRCS = passt-repair.c
> > -SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS)
> > +PESTO_SRCS = pesto.c pesto_util.c
> > +SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_SRCS)
> >  
> > -MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1
> > +MANPAGES = passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1
> >  
> > +PESTO_HEADERS = pesto.h pesto_util.h
> >  PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h epoll_ctl.h \
> >  	flow.h fwd.h flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h \
> >  	isolation.h lineread.h log.h migrate.h ndp.h netlink.h packet.h \
> >  	passt.h pasta.h pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h \
> >  	tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h \
> > -	udp_internal.h udp_vu.h util.h vhost_user.h virtio.h vu_common.h
> > +	udp_internal.h udp_vu.h util.h vhost_user.h virtio.h vu_common.h \
> > +	$(PESTO_HEADERS)
> >  HEADERS = $(PASST_HEADERS) seccomp.h
> >  
> >  C := \#include <sys/random.h>\nint main(){int a=getrandom(0, 0, 0);}
> > @@ -74,9 +78,9 @@ mandir		?= $(datarootdir)/man
> >  man1dir		?= $(mandir)/man1
> >  
> >  ifeq ($(TARGET_ARCH),x86_64)
> > -BIN := passt passt.avx2 pasta pasta.avx2 qrap passt-repair
> > +BIN := passt passt.avx2 pasta pasta.avx2 qrap passt-repair pesto
> >  else
> > -BIN := passt pasta qrap passt-repair
> > +BIN := passt pasta qrap passt-repair pesto
> >  endif
> >  
> >  all: $(BIN) $(MANPAGES) docs
> > @@ -90,6 +94,9 @@ seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
> >  seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS)
> >  	@ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS)
> >  
> > +seccomp_pesto.h: seccomp.sh $(PESTO_SRCS)
> > +	@ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_pesto.h $(PESTO_SRCS)
> > +
> >  passt: $(PASST_SRCS) $(HEADERS)
> >  	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)
> >  
> > @@ -109,6 +116,9 @@ qrap: $(QRAP_SRCS) passt.h
> >  passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h
> >  	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS)
> >  
> > +pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h
> > +	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PESTO_SRCS) -o pesto $(LDFLAGS)
> > +
> >  valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction	\
> >  			    rt_sigreturn getpid gettid kill clock_gettime \
> >  			    mmap|mmap2 munmap open unlink gettimeofday futex \
> > @@ -118,7 +128,7 @@ valgrind: all
> >  
> >  .PHONY: clean
> >  clean:
> > -	$(RM) $(BIN) *~ *.o seccomp.h seccomp_repair.h pasta.1 \
> > +	$(RM) $(BIN) *~ *.o seccomp.h seccomp_repair.h seccomp_pesto.h pasta.1 \
> >  		passt.tar passt.tar.gz *.deb *.rpm \
> >  		passt.pid README.plain.md
> >  
> > @@ -172,11 +182,11 @@ docs: README.md
> >  		done < README.md;					\
> >  	) > README.plain.md
> >  
> > -clang-tidy: $(PASST_SRCS)
> > +clang-tidy: $(PASST_SRCS) $(PESTO_SRCS)
> >  	clang-tidy $^ -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) \
> >  	           -DCLANG_TIDY_58992
> >  
> > -cppcheck: $(PASST_SRCS) $(HEADERS)
> > +cppcheck: $(PASST_SRCS) $(PESTO_SRCS) $(HEADERS)
> >  	if cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; then \
> >  		CPPCHECK_EXHAUSTIVE="--check-level=exhaustive";		\
> >  	else								\
> > diff --git a/conf.c b/conf.c
> > index 940fb9e9..04f65b30 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -47,6 +47,9 @@
> >  #include "isolation.h"
> >  #include "log.h"
> >  #include "vhost_user.h"
> > +#include "epoll_ctl.h"
> > +#include "conf.h"
> > +#include "pesto.h"
> >  
> >  #define NETNS_RUN_DIR	"/run/netns"
> >  
> > @@ -894,6 +897,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"
> 
> --conf-path

Oops, fixed.
> 
> >  		"  -h, --help		Display this help message and exit\n"
> >  		"  --version		Show version and exit\n");
> >  
> > @@ -1425,6 +1429,17 @@ 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_conf = -1;
> 
> Note: we should remember to tackle your own comments from
> <aYqw6M2G9n-tv9Qm@zatzit>, eventually (not necessarily now, but fd_conf
> just reminded me).

Uh, don't remember what that mail was and I'm not immediately sure how
to search for it based on message-id.

> > +	if (*c->conf_path) {
> > +		c->fd_conf_listen = sock_unix(c->conf_path);
> > +		if (c->fd_conf_listen < 0) {
> > +			die_perror("Couldn't open control socket %s",
> > +				   c->conf_path);
> > +		}
> > +	} else {
> > +		c->fd_conf_listen = -1;
> > +	}
> >  }
> >  
> >  /**
> > @@ -1460,6 +1475,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_conf_listen < 0)
> > +		return;
> > +
> > +	if (listen(c->fd_conf_listen, 0))
> > +		die_perror("Couldn't listen on configuration socket");
> > +
> > +	ref.fd = c->fd_conf_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
> > @@ -1542,9 +1576,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";
> >  	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
> >  	bool copy_addrs_opt = false, copy_routes_opt = false;
> > @@ -1808,6 +1843,13 @@ void conf(struct ctx *c, int argc, char **argv)
> >  
> >  			c->fd_tap = -1;
> >  			break;
> > +		case 'c':
> > +			ret = snprintf(c->conf_path, sizeof(c->conf_path), "%s",
> > +				       optarg);
> > +			if (ret <= 0 || ret >= (int)sizeof(c->sock_path))
> > +				die("Invalid configuration path: %s", optarg);
> > +			c->fd_conf_listen = c->fd_conf = -1;
> > +			break;
> >  		case 'F':
> >  			errno = 0;
> >  			fd_tap_opt = strtol(optarg, NULL, 0);
> > @@ -2242,4 +2284,108 @@ void conf(struct ctx *c, int argc, char **argv)
> >  
> >  	if (!c->quiet)
> >  		conf_print(c);
> > +
> > +	conf_sock_listen(c);
> > +}
> > +
> > +/**
> > + * 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)
> > +{
> > +	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;
> > +
> > +	if (events != EPOLLIN) {
> > +		err("Unexpected event 0x%04x on configuration socket", events);
> > +		return;
> > +	}
> > +
> > +	fd = accept4(c->fd_conf_listen, NULL, NULL, SOCK_NONBLOCK);
> > +	if (fd < 0) {
> > +		warn_perror("accept4() on configuration listening socket");
> > +		return;
> > +	}
> > +
> > +	if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &uc, &len) < 0)
> > +		warn_perror("Can't get configuration client credentials");
> > +
> > +	/* Another client is already connected: accept and close right away. */
> > +	if (c->fd_conf != -1) {
> > +		info("Discarding configuration client, PID %i", uc.pid);
> > +		goto fail;
> > +	}
> > +
> > +	ref.fd = fd;
> > +	rc = epoll_add(c->epollfd, EPOLLIN | EPOLLET, ref);
> > +	if (rc < 0) {
> > +		warn_perror("epoll_ctl() on configuration socket");
> > +		goto fail;
> > +	}
> > +
> > +	rc = write_all_buf(fd, &hello, sizeof(hello));
> > +	if (rc < 0) {
> > +		warn_perror("Error writing configuration protocol hello");
> > +		goto fail;
> > +	}
> > +
> > +	c->fd_conf = fd;
> > +	info("Accepted configuration client, PID %i", uc.pid);
> > +	if (!PESTO_PROTOCOL_VERSION) {
> > +		warn(
> > +"Warning: Using experimental unsupported configuration protocol");
> > +	}
> > +
> > +	return;
> > +
> > +fail:
> > +	close(fd);
> > +}
> > +
> > +/**
> > + * conf_handler() - Handle events on configuration socket
> > + * @c:		Execution context
> > + * @events:	epoll events
> > + */
> > +void conf_handler(struct ctx *c, uint32_t events)
> > +{
> > +	if (events & EPOLLIN) {
> > +		char discard[BUFSIZ];
> > +		ssize_t n;
> > +
> > +		do {
> > +			n = read(c->fd_conf, discard, sizeof(discard));
> > +			if (n > 0)
> > +				debug("Discarded %zd bytes of config data", n);
> > +		} while (n > 0);
> > +		if (n == 0) {
> > +			debug("Configuration client EOF");
> > +			goto close;
> > +		}
> > +		if (errno != EAGAIN && errno != EWOULDBLOCK) {
> > +			err_perror("Error reading config data");
> > +			goto close;
> > +		}
> > +	}
> > +
> > +	if (events & EPOLLHUP) {
> > +		debug("Configuration client hangup");
> > +		goto close;
> > +	}
> > +
> > +	return;
> > +
> > +close:
> > +	debug("Closing configuration socket");
> > +	epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_conf, NULL);
> > +	close(c->fd_conf);
> > +	c->fd_conf = -1;
> >  }
> > diff --git a/conf.h b/conf.h
> > index b45ad746..16f97189 100644
> > --- a/conf.h
> > +++ b/conf.h
> > @@ -8,5 +8,7 @@
> >  
> >  enum passt_modes conf_mode(int argc, char *argv[]);
> >  void conf(struct ctx *c, int argc, char **argv);
> > +void conf_listen_handler(struct ctx *c, uint32_t events);
> > +void conf_handler(struct ctx *c, uint32_t events);
> >  
> >  #endif /* CONF_H */
> > diff --git a/epoll_type.h b/epoll_type.h
> > index a90ffb67..061325aa 100644
> > --- a/epoll_type.h
> > +++ b/epoll_type.h
> > @@ -46,6 +46,10 @@ enum epoll_type {
> >  	EPOLL_TYPE_REPAIR,
> >  	/* Netlink neighbour subscription socket */
> >  	EPOLL_TYPE_NL_NEIGH,
> > +	/* Configuration listening socket */
> > +	EPOLL_TYPE_CONF_LISTEN,
> > +	/* Configuration socket */
> > +	EPOLL_TYPE_CONF,
> >  
> >  	EPOLL_NUM_TYPES,
> >  };
> > diff --git a/passt.1 b/passt.1
> > index 13e8df9d..32d70c8d 100644
> > --- a/passt.1
> > +++ b/passt.1
> > @@ -127,6 +127,11 @@ login name and group name can be passed. This requires privileges (either
> >  initial effective UID 0 or CAP_SETUID capability) to work.
> >  Default is to change to user \fInobody\fR if started as root.
> >  
> > +.TP
> > +.BR \-c ", " \-\-conf-path " " \fIpath " " (EXPERIMENTAL)
> 
> I wouldn't even mention (EXPERIMENTAL). Pretty much all the options we
> added so far were.

Well, my plan was to remove that once we had a v1 of the update protocol.

> > +Path for configuration and control socket used by \fBpesto\fR(1) to
> > +dynamically update passt or pasta's configuration.
> > +
> >  .TP
> >  .BR \-h ", " \-\-help
> >  Display a help message and exit.
> > diff --git a/passt.c b/passt.c
> > index f84419c7..bc42ea33 100644
> > --- a/passt.c
> > +++ b/passt.c
> > @@ -80,6 +80,8 @@ char *epoll_type_str[] = {
> >  	[EPOLL_TYPE_REPAIR_LISTEN]	= "TCP_REPAIR helper listening socket",
> >  	[EPOLL_TYPE_REPAIR]		= "TCP_REPAIR helper socket",
> >  	[EPOLL_TYPE_NL_NEIGH]		= "netlink neighbour notifier socket",
> > +	[EPOLL_TYPE_CONF_LISTEN]	= "configuration listening socket",
> > +	[EPOLL_TYPE_CONF]		= "configuration socket",
> >  };
> >  static_assert(ARRAY_SIZE(epoll_type_str) == EPOLL_NUM_TYPES,
> >  	      "epoll_type_str[] doesn't match enum epoll_type");
> > @@ -303,6 +305,12 @@ static void passt_worker(void *opaque, int nfds, struct epoll_event *events)
> >  		case EPOLL_TYPE_NL_NEIGH:
> >  			nl_neigh_notify_handler(c);
> >  			break;
> > +		case EPOLL_TYPE_CONF_LISTEN:
> > +			conf_listen_handler(c, eventmask);
> > +			break;
> > +		case EPOLL_TYPE_CONF:
> > +			conf_handler(c, eventmask);
> > +			break;
> >  		default:
> >  			/* Can't happen */
> >  			assert(0);
> > diff --git a/passt.h b/passt.h
> > index b614bdf0..a49fbe1d 100644
> > --- a/passt.h
> > +++ b/passt.h
> > @@ -158,6 +158,7 @@ struct ip6_ctx {
> >   * @foreground:		Run in foreground, don't log to stderr by default
> >   * @nofile:		Maximum number of open files (ulimit -n)
> >   * @sock_path:		Path for UNIX domain socket
> > + * @conf_path:		Path for configuration UNIX domain socket
> >   * @repair_path:	TCP_REPAIR helper path, can be "none", empty for default
> >   * @pcap:		Path for packet capture file
> >   * @pidfile:		Path to PID file, empty string if not configured
> > @@ -169,6 +170,8 @@ struct ip6_ctx {
> >   * @epollfd:		File descriptor for epoll instance
> >   * @fd_tap_listen:	File descriptor for listening AF_UNIX socket, if any
> >   * @fd_tap:		AF_UNIX socket, tuntap device, or pre-opened socket
> > + * @fd_conf_listen:	Listening configuration socket, if any
> > + * @fd_conf:		Configuration socket, if any
> >   * @fd_repair_listen:	File descriptor for listening TCP_REPAIR socket, if any
> >   * @fd_repair:		Connected AF_UNIX socket for TCP_REPAIR helper
> >   * @our_tap_mac:	Pasta/passt's MAC on the tap link
> > @@ -224,6 +227,7 @@ struct ctx {
> >  	int foreground;
> >  	int nofile;
> >  	char sock_path[UNIX_PATH_MAX];
> > +	char conf_path[UNIX_PATH_MAX];
> >  	char repair_path[UNIX_PATH_MAX];
> >  	char pcap[PATH_MAX];
> >  
> > @@ -241,6 +245,8 @@ struct ctx {
> >  	int epollfd;
> >  	int fd_tap_listen;
> >  	int fd_tap;
> > +	int fd_conf_listen;
> > +	int fd_conf;
> >  	int fd_repair_listen;
> >  	int fd_repair;
> >  	unsigned char our_tap_mac[ETH_ALEN];
> > diff --git a/pesto.1 b/pesto.1
> > new file mode 100644
> > index 00000000..fe2072f5
> > --- /dev/null
> > +++ b/pesto.1
> > @@ -0,0 +1,47 @@
> > +.\" SPDX-License-Identifier: GPL-2.0-or-later
> > +.\" Copyright Red Hat
> > +.\" Author: David Gibson <david@gibson.dropbear.id.au>
> > +.TH pesto 1
> > +
> > +.SH NAME
> > +.B pesto
> > +\- View or alter configuration of a running \fBpasst\fR(1) or
> > +\fBpasta\fR(1).
> 
> ...process/instance?

I had that initially, but I was trying to keep it to one line.

> > +
> > +.SH SYNOPSIS
> > +.B pesto
> > +\fIPATH\fR
> > +
> > +.SH DESCRIPTION
> > +
> > +.B pesto
> > +is an experimental client to view and update the port forwarding
> 
> I'd drop "experimental" from here as well, otherwise we'll never drop
> it until somebody asks if there's a "stable" solution in two years.

Again, my plan was to remove that once we have a series that
implements a suitable-for-release version of the protocol.  This man
page to require a bunch of updates as we add options, so I'm hoping
that will be enough not to forget.

> > +configuration of a running \fBpasst\fR(1) or \fBpasta\fR(1) instance.
> > +
> > +\fIPATH\fR gives the path to the UNIX domain socket created by
> > +\fBpasst\fR or \fBpasta\fR.  It should match the \fB-c\fR command line
> > +option given to that instance.
> > +
> > +.SH AUTHOR
> 
> Should be AUTHORS (just like in passt(1) -- I checked, back then, both
> section names seem to be common and they aren't really standardised
> anywhere).

Fixed.

> 
> > +
> > +Stefano Brivio <sbrivio@redhat.com>,
> > +David Gibson <david@gibson.dropbear.id.au>.
> > +
> > +.SH REPORTING BUGS
> > +
> > +Please report issues on the bug tracker at https://bugs.passt.top/, or
> > +send a message to the passt-user@passt.top mailing list, see
> > +https://lists.passt.top/.
> > +
> > +.SH COPYRIGHT
> > +
> > +Copyright Red Hat
> > +
> > +\fBpesto\fR is free software: you can redistribute them and/or modify
> > +them under the terms of the GNU General Public License as published by
> > +the Free Software Foundation, either version 2 of the License, or (at
> > +your option) any later version.
> > +
> > +.SH SEE ALSO
> > +
> > +\fBpasst\fR(1), \fBpasta\fR(1), \fBunix\fR(7).
> > diff --git a/pesto.c b/pesto.c
> > new file mode 100644
> > index 00000000..a158eadf
> > --- /dev/null
> > +++ b/pesto.c
> > @@ -0,0 +1,111 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +/* PESTO - Programmable Extensible Socket Translation Orchestrator
> > + *  front-end for passt(1) and pasta(1) forwarding configuration
> > + *
> > + * pesto.c - Main program (it's not actually extensible)
> > + *
> > + * Copyright (c) 2026 Red Hat GmbH
> > + * Author: Stefano Brivio <sbrivio@redhat.com>
> > + */
> > +
> > +#include <arpa/inet.h>
> > +#include <sys/prctl.h>
> > +#include <sys/types.h>
> > +#include <sys/socket.h>
> > +#include <sys/un.h>
> > +#include <errno.h>
> > +#include <inttypes.h>
> > +#include <stdbool.h>
> > +#include <stddef.h>
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <limits.h>
> > +#include <unistd.h>
> > +
> > +#include <linux/audit.h>
> > +#include <linux/capability.h>
> > +#include <linux/filter.h>
> > +#include <linux/seccomp.h>
> > +
> > +#include "seccomp_pesto.h"
> > +#include "pesto_util.h"
> > +#include "pesto.h"
> > +
> > +#define die(...)							\
> > +	do {								\
> > +		FPRINTF(stderr, __VA_ARGS__);				\
> > +		FPRINTF(stderr, "\n");					\
> > +		exit(EXIT_FAILURE);					\
> > +	} while (0)
> > +
> > +/**
> > + * main() - Entry point and whole program with loop
> > + * @argc:	Argument count
> > + * @argv:	Arguments: socket path, operation, port specifiers
> > + *
> > + * Return: 0 on success, won't return on failure
> > + *
> > + * #syscalls:pesto connect write close exit_group fstat brk
> > + * #syscalls:pesto socket s390x:socketcall i686:socketcall
> > + * #syscalls:pesto recvfrom recvmsg arm:recv ppc64le:recv
> > + * #syscalls:pesto sendto sendmsg arm:send ppc64le:send
> > + */
> > +int main(int argc, char **argv)
> > +{
> > +	struct sockaddr_un a = { AF_UNIX, "" };
> > +	struct pesto_hello hello;
> > +	struct sock_fprog prog;
> > +	uint32_t s_version;
> > +	int ret, s;
> > +
> > +	prctl(PR_SET_DUMPABLE, 0);
> > +
> > +	prog.len = (unsigned short)sizeof(filter_pesto) /
> > +				   sizeof(filter_pesto[0]);
> > +	prog.filter = filter_pesto;
> > +	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
> > +	    prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog))
> > +		die("Failed to apply seccomp filter");
> > +
> > +	if (argc < 2)
> > +		die("Usage: %s CONTROLPATH", argv[0]);
> 
> While CONTROLPATH is a bit more descriptive, I think it's also harder
> to read compared to PATH, and after all it's not adding much.

Fair enough, done.

> > +
> > +	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0)
> > +		die("Failed to create AF_UNIX socket: %s", strerror(errno));
> > +
> > +	ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
> > +	if (ret <= 0 || ret >= (int)sizeof(a.sun_path))
> > +		die("Invalid socket path \"%s\"", argv[1]);
> > +
> > +	ret = connect(s, (struct sockaddr *)&a, sizeof(a));
> > +	if (ret < 0) {
> > +		die("Failed to connect to %s: %s",
> > +		    a.sun_path, strerror(errno));
> > +	}
> > +
> > +	ret = read_all_buf(s, &hello, sizeof(hello));
> > +	if (ret < 0)
> > +		die("Couldn't read server greeting: %s", strerror(errno));
> > +
> > +	if (memcmp(hello.magic, PESTO_SERVER_MAGIC, sizeof(hello.magic)))
> > +		die("Bad magic number from server");
> > +
> > +	s_version = ntohl(hello.version);
> > +
> > +	if (s_version > PESTO_PROTOCOL_VERSION) {
> > +		die("Unknown server protocol version %"PRIu32" > %"PRIu32"\n",
> > +		    s_version, PESTO_PROTOCOL_VERSION);
> > +	}
> > +
> > +	if (!s_version) {
> > +		if (PESTO_PROTOCOL_VERSION)
> > +			die("Unsupported experimental server protocol");
> > +		fprintf(stderr,
> > +"Warning: Using experimental protocol version, client and server must match\n");
> > +	}
> > +
> > +	return 0;
> > +}
> > diff --git a/pesto.h b/pesto.h
> > new file mode 100644
> > index 00000000..92d4df3a
> > --- /dev/null
> > +++ b/pesto.h
> > @@ -0,0 +1,34 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later
> > + * Copyright Red Hat
> > + * Author: David Gibson <david@gibson.dropbear.id.au>
> > + *
> > + * Definitions and functions used by both client and server of the configuration
> > + * update protocol (pesto).
> 
> The answer is probably not obvious now, but eventually we should figure
> out what we want to call "pesto": for me it was just the client, from
> this description it sounds like the protocol as well (or Bad Acronym
> Sounds Indeed Logical?). Maybe we'll never need to name the protocol
> though.

Yeah, I've been using it a bit sloppily for both.  Or "pesto" (client)
versus "pesto protocol".

> > + */
> > +
> > +#ifndef PESTO_H
> > +#define PESTO_H
> > +
> > +#include <assert.h>
> > +#include <stdint.h>
> > +
> > +#define PESTO_SERVER_MAGIC	"pesto:s"
> 
> Regardless of the consideration above, shouldn't this be "basil:s"?

I really don't care :).

> > +
> > +/* Version 0 is reserved for unreleased / unsupported experimental versions */
> > +#define PESTO_PROTOCOL_VERSION	0
> > +
> > +/**
> > + * struct pesto_hello - Server introduction message
> > + * @magic:	PESTO_SERVER_MAGIC
> > + * @version:	Version number
> > + */
> > +struct pesto_hello {
> > +	char magic[8];
> > +	uint32_t version;
> > +} __attribute__ ((__packed__));
> > +
> > +static_assert(sizeof(PESTO_SERVER_MAGIC)
> > +	      == sizeof(((struct pesto_hello *)0)->magic),
> > +	      "PESTO_SERVER_MAGIC has wrong size");
> > +
> > +#endif /* PESTO_H */
> > diff --git a/pesto_util.c b/pesto_util.c
> > new file mode 100644
> > index 00000000..8e6b43d4
> > --- /dev/null
> > +++ b/pesto_util.c
> > @@ -0,0 +1,62 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +/* ASST - Plug A Simple Socket Transport
> 
> :)

Oops, fixed.

> > + *  for qemu/UNIX domain socket mode
> > + *
> > + * PASTA - Pack A Subtle Tap Abstraction
> > + *  for network namespace/tap device mode
> > + *
> > + * PESTO - Programmable Extensible Socket Translation Orchestrator
> > + *  front-end for passt(1) and pasta(1) forwarding configuration
> > + *
> > + * pesto_util.c - Helpers used by both passt/pasta and pesto
> 
> Maybe we should call this common.c? It's a bit counter-intuitive to
> look for read_all_buf() as used by passt into pesto_util.c. I'm not
> fond of common.c either, it's less descriptive, but also potentially
> less misleading.

Maybe?  I thought about common.c, but I wan't to distinguish (a) that
this is shared specifically between pesto and passt, not with,
e.g. passt-repair and (b) these are shared utility functions, rather
than shared logic specifically regarding the protocol (that's in
pesto.h).

I'll keep it for now, but I'm open to better suggestions if either of
us comes up with one.

> > + * Copyright Red Hat
> > + * Author: Stefano Brivio <sbrivio@redhat.com>
> > + * Author: David Gibson <david@gibson.dropbear.id.au>
> > + */
> > +
> > +#include <assert.h>
> > +#include <errno.h>
> > +#include <unistd.h>
> > +#include <sys/types.h>
> > +
> > +#include "pesto_util.h"
> > +
> > +/**
> > + * read_all_buf() - Fill a whole buffer from a file descriptor
> > + * @fd:		File descriptor
> > + * @buf:	Pointer to base of buffer
> > + * @len:	Length of buffer
> > + *
> > + * Return: 0 on success, -1 on error (with errno set)
> > + *
> > + * #syscalls read
> > + */
> > +int read_all_buf(int fd, void *buf, size_t len)
> > +{
> > +	size_t left = len;
> > +	char *p = buf;
> > +
> > +	while (left) {
> > +		ssize_t rc;
> > +
> > +		assert(left <= len);
> > +
> > +		do
> > +			rc = read(fd, p, left);
> > +		while ((rc < 0) && errno == EINTR);
> > +
> > +		if (rc < 0)
> > +			return -1;
> > +
> > +		if (rc == 0) {
> > +			errno = ENODATA;
> > +			return -1;
> > +		}
> > +
> > +		p += rc;
> > +		left -= rc;
> > +	}
> > +	return 0;
> > +}
> > diff --git a/pesto_util.h b/pesto_util.h
> > new file mode 100644
> > index 00000000..1fc5d9fd
> > --- /dev/null
> > +++ b/pesto_util.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later
> > + * Copyright Red Hat
> > + * Author: David Gibson <david@gibson.dropbear.id.au>
> > + *
> > + * Helper functions used by both passt/pasta and pesto (the configuration update
> > + * client).
> > + */
> > +
> > +#ifndef PESTO_UTIL_H
> > +#define PESTO_UTIL_H
> > +
> > +#include <stddef.h>
> > +
> > +/* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */
> > +#define FPRINTF(f, ...)	(void)fprintf(f, __VA_ARGS__)
> > +
> > +int read_all_buf(int fd, void *buf, size_t len);
> > +
> > +#endif /* PESTO_UTIL_H */
> > diff --git a/util.c b/util.c
> > index 22318c00..1571850b 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -866,44 +866,6 @@ int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip)
> >  	return 0;
> >  }
> >  
> > -/**
> > - * read_all_buf() - Fill a whole buffer from a file descriptor
> > - * @fd:		File descriptor
> > - * @buf:	Pointer to base of buffer
> > - * @len:	Length of buffer
> > - *
> > - * Return: 0 on success, -1 on error (with errno set)
> > - *
> > - * #syscalls read
> > - */
> > -int read_all_buf(int fd, void *buf, size_t len)
> > -{
> > -	size_t left = len;
> > -	char *p = buf;
> > -
> > -	while (left) {
> > -		ssize_t rc;
> > -
> > -		assert(left <= len);
> > -
> > -		do
> > -			rc = read(fd, p, left);
> > -		while ((rc < 0) && errno == EINTR);
> > -
> > -		if (rc < 0)
> > -			return -1;
> > -
> > -		if (rc == 0) {
> > -			errno = ENODATA;
> > -			return -1;
> > -		}
> > -
> > -		p += rc;
> > -		left -= rc;
> > -	}
> > -	return 0;
> > -}
> > -
> >  /**
> >   * read_remainder() - Read the tail of an IO vector from a file descriptor
> >   * @fd:		File descriptor
> > diff --git a/util.h b/util.h
> > index dcb79afe..74227a68 100644
> > --- a/util.h
> > +++ b/util.h
> > @@ -20,6 +20,7 @@
> >  #include <net/ethernet.h>
> >  
> >  #include "log.h"
> > +#include "pesto_util.h"
> >  
> >  #define VERSION_BLOB							       \
> >  	VERSION "\n"							       \
> > @@ -242,7 +243,6 @@ int write_file(const char *path, const char *buf);
> >  intmax_t read_file_integer(const char *path, intmax_t fallback);
> >  int write_all_buf(int fd, const void *buf, size_t len);
> >  int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip);
> > -int read_all_buf(int fd, void *buf, size_t len);
> >  int read_remainder(int fd, const struct iovec *iov, size_t cnt, size_t skip);
> >  void close_open_files(int argc, char **argv);
> >  bool snprintf_check(char *str, size_t size, const char *format, ...);
> > @@ -314,9 +314,6 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m)
> >  	return mod_sub(x, i, m) < mod_sub(j, i, m);
> >  }
> >  
> > -/* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */
> > -#define FPRINTF(f, ...)	(void)fprintf(f, __VA_ARGS__)
> > -
> >  void raw_random(void *buf, size_t buflen);
> >  
> >  /*
> 
> The rest looks good to me, maybe we could just keep it like it is while
> building stuff on top of this patch and address the comments later, no
> particular preference (not a big effort either way).
> 
> -- 
> Stefano
> 

-- 
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 --]

  reply	other threads:[~2026-03-17  0:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-16  5:46 [PATCH 0/5] RFC: Stub dynamic update implementation David Gibson
2026-03-16  5:46 ` [PATCH 1/5] Makefile: Use $^ to avoid duplication in static checker rules David Gibson
2026-03-16  5:46 ` [PATCH 2/5] doc: Fix formatting of (DEPRECATED) notes in man page David Gibson
2026-03-16  5:46 ` [PATCH 3/5] pif: Remove unused PIF_NAMELEN David Gibson
2026-03-16  5:46 ` [PATCH 4/5] treewide: Spell ASSERT() as assert() David Gibson
2026-03-17  0:02   ` Stefano Brivio
2026-03-17  0:39     ` David Gibson
2026-03-17  9:36       ` Stefano Brivio
2026-03-16  5:46 ` [PATCH 5/5] pesto: Introduce stub configuration interface and tool David Gibson
2026-03-17  0:02   ` Stefano Brivio
2026-03-17  0:48     ` David Gibson [this message]
2026-03-17  9:36       ` Stefano Brivio
2026-03-17  0:02 ` [PATCH 0/5] RFC: Stub dynamic update implementation 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=abik8onHZ6_iIWU-@zatzit \
    --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).