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

On 5/6/26 11:22, 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]

But the real value is in pine nuts (110 €/kg here). (I make mine with bear's garlic and almonds)

[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 0586107..5ec0072 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__); + + if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &uc, &len) < 0) + warn_perror("Can't get configuration client credentials"); + + c->fd_control = 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; + } + + info("Accepted configuration client, PID %i", uc.pid); + if (!PESTO_PROTOCOL_VERSION) { + warn( +"Warning: Using experimental unsupported configuration protocol"); + } + + return; + +fail: + conf_close(c); + goto retry; +} + +/** + * 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_control, 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: + conf_close(c); + + /* Check if any other clients are waiting to connect */ + conf_accept(c); +} diff --git a/conf.h b/conf.h index b45ad74..16f9718 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 a90ffb6..061325a 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 6303aeb..908fd4a 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) +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 f84419c..bc42ea3 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 62b8dcd..b3f049d 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 + * @control_path: Path for control/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_control_listen: Listening control/configuration socket, if any + * @fd_control: Control/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 @@ -223,6 +226,7 @@ struct ctx { int foreground; int nofile; char sock_path[UNIX_PATH_MAX]; + char control_path[UNIX_PATH_MAX]; char repair_path[UNIX_PATH_MAX]; char pcap[PATH_MAX];

@@ -240,6 +244,8 @@ struct ctx { int epollfd; int fd_tap_listen; int fd_tap; + int fd_control_listen; + int fd_control; int fd_repair_listen; int fd_repair; unsigned char our_tap_mac[ETH_ALEN]; diff --git a/pesto.c b/pesto.c index f0916e8..ab476c5 100644 --- a/pesto.c +++ b/pesto.c @@ -33,6 +33,7 @@

#include "common.h" #include "seccomp_pesto.h" +#include "serialise.h" #include "pesto.h" #include "log.h"

@@ -66,6 +67,8 @@ static void usage(const char *name, FILE *f, int status) * * Return: 0 on success, won't return on failure * + * #syscalls:pesto socket s390x:socketcall i686:socketcall + * #syscalls:pesto connect shutdown close * #syscalls:pesto exit_group fstat read write */ int main(int argc, char **argv) @@ -76,9 +79,12 @@ int main(int argc, char **argv) {"version", no_argument, NULL, 1 }, { 0 }, }; + struct sockaddr_un a = { AF_UNIX, "" }; const char *optstring = "dh"; + struct pesto_hello hello; struct sock_fprog prog; - int optname; + int optname, ret, s; + uint32_t s_version;

prctl(PR_SET_DUMPABLE, 0);

@@ -122,5 +128,42 @@ int main(int argc, char **argv)

debug("debug_flag=%d, path=\"%s\"", debug_flag, argv[optind]);

- die("pesto is not implemented yet"); + if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) + die_perror("Failed to create AF_UNIX socket"); + + ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[optind]); + if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) + die("Invalid socket path \"%s\"", argv[optind]); + + ret = connect(s, (struct sockaddr *)&a, sizeof(a)); + if (ret < 0) { + die_perror("Failed to connect to %s", a.sun_path); + } + + ret = read_all_buf(s, &hello, sizeof(hello)); + if (ret < 0) + die_perror("Couldn't read server greeting"); + + 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, + s_version, PESTO_PROTOCOL_VERSION); + } + + /* cppcheck-suppress knownConditionTrueFalse */ + 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"); + } + + if (shutdown(s, SHUT_RDWR) < 0 || close(s) < 0) + die_perror("Error shutting down control socket"); + + exit(0); } diff --git a/pesto.h b/pesto.h index e9b329f..3c93d3e 100644 --- a/pesto.h +++ b/pesto.h @@ -9,4 +9,26 @@ #ifndef PESTO_H #define PESTO_H

+#include +#include + +#define PESTO_SERVER_MAGIC "basil:s" + +/* Version 0 is reserved for unreleased / unsupported experimental versions */ +#define PESTO_PROTOCOL_VERSION 1 + +/** + * 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/serialise.c b/serialise.c index 944e741..346df99 100644 --- a/serialise.c +++ b/serialise.c @@ -6,6 +6,9 @@ * 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 + * * serialise.c - Serialisation of data structures over bytestreams * * Copyright Red Hat