Re: [PATCH v6 11/18] pesto: Expose list of pifs to pesto and optionally display

On Mon, 4 May 2026 16:34:40 +0200 Laurent Vivier wrote:

On 5/3/26 23:55, Stefano Brivio wrote:

...

From: David Gibson

Extend the dynamic update protocol to expose the pif indices and names from a running passt/pasta to the pesto tool. pesto records that data and, if requested with a new --show flag, prints it out.

There is no --show flag

Ah, right, that comes later in the series now. I dropped that from the commit message, and reworded the title.

...

Signed-off-by: David Gibson [sbrivio: In read_pif_conf(), force a redundant termination of the interface name, the existing check isn't obvious enough for static checkers] [sbrivio: Drop @resv_ left-over in description of struct pesto_pif_info, reported by Jon Maloy] Signed-off-by: Stefano Brivio

Reviewed-by: Laurent Vivier

But I've noticed 3 minor cosmetic flaws below.

...

--- Makefile | 1 + common.h | 2 + conf.c | 41 ++++++++++++++++ pesto.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++ pesto.h | 18 ++++++- pif.h | 4 +- serialise.c | 4 ++ serialise.h | 1 + util.h | 2 - 9 files changed, 201 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile index 1718ddb..6da76b4 100644 --- a/Makefile +++ b/Makefile @@ -223,6 +223,7 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^

passt.cppcheck: BASE_CPPFLAGS += -UPESTO +passt.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h

passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h diff --git a/common.h b/common.h index 2f2e6f1..45f66ea 100644 --- a/common.h +++ b/common.h @@ -53,4 +53,6 @@ static inline const char *strerror_(int errnum)

#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead"

+#define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) + #endif /* _COMMON_H */ diff --git a/conf.c b/conf.c index 823e08d..3b2fe42 100644 --- a/conf.c +++ b/conf.c @@ -1925,6 +1925,43 @@ void conf(struct ctx *c, int argc, char **argv)

static void conf_accept(struct ctx *c);

+/** + * conf_send_rules() - Send current forwarding rules to config client (pesto) + * @c: Execution context + * @fd: Socket to the client + * + * Return: 0 on success, -1 on failure + * + * FIXME: So far only sends pif ids and names + */ +static int conf_send_rules(const struct ctx *c, int fd) +{ + unsigned pif; + + for (pif = 0; pif < PIF_NUM_TYPES; pif++) { + struct pesto_pif_info info; + int rc; + + if (!c->fwd[pif]) + continue; + + assert(pif != PIF_NONE); + + rc = snprintf(info.name, sizeof(info.name), "%s", pif_name(pif)); + assert(rc >= 0 && (size_t)rc < sizeof(info.name)); + + if (write_u8(fd, pif) < 0) + return -1; + if (write_all_buf(fd, &info, sizeof(info)) < 0) + return -1; + } + + if (write_u8(fd, PIF_NONE) < 0) + return -1; + + return 0; +} + /** * conf_close() - Close configuration / control socket and clean up * @c: Execution context @@ -1968,6 +2005,7 @@ static void conf_accept(struct ctx *c) struct pesto_hello hello = { .magic = PESTO_SERVER_MAGIC, .version = htonl(PESTO_PROTOCOL_VERSION), + .pif_name_size = htonl(PIF_NAME_SIZE), }; union epoll_ref ref = { .type = EPOLL_TYPE_CONF }; struct ucred uc = { 0 }; @@ -2009,6 +2047,9 @@ retry: "Warning: Using experimental unsupported configuration protocol"); }

+ if (conf_send_rules(c, fd) < 0) + goto fail; + return;

fail: diff --git a/pesto.c b/pesto.c index 762cfe9..77244b3 100644 --- a/pesto.c +++ b/pesto.c @@ -60,6 +60,127 @@ static void usage(const char *name, FILE *f, int status) exit(status); }

+/* Maximum number of pifs with rule tables */ +#define MAX_PIFS 3 + +struct pif_configuration { + uint8_t pif; + char name[PIF_NAME_SIZE]; +}; + +struct configuration { + uint32_t npifs; + struct pif_configuration pif[MAX_PIFS]; +}; + +/** + * pif_conf_by_num() - Find a pif's configuration by pif id + * @conf: Configuration description + * @pif: pif id + * + * Return: pointer to the pif_configuration for @pif, or NULL if not found + */ +static struct pif_configuration *pif_conf_by_num(struct configuration *conf, + uint8_t pif) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + if (conf->pif[i].pif == pif) + return &conf->pif[i]; + } + + return NULL; +} + +/** + * pif_conf_by_name() - Find a pif's configuration by name + * @conf: Configuration description + * @name: Interface name + * + * Return: pif_configuration for pif named @name, or NULL if not found + */ +static struct pif_configuration *pif_conf_by_name(struct configuration *conf, + const char *name) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + if (strcmp(conf->pif[i].name, name) == 0) + return &conf->pif[i]; + } + + return NULL; +} + +/** + * pesto_read_rules() - Read rulestate from passt/pasta + * @fd: Control socket + * @conf: Configuration description to update + */ +static bool read_pif_conf(int fd, struct configuration *conf) +{ + struct pif_configuration *pc; + struct pesto_pif_info info; + uint8_t pif; + + if (read_u8(fd, &pif) < 0) + die("Error reading from control socket"); + + if (pif == PIF_NONE) + return false; + + debug("Receiving config for PIF %"PRIu8, pif); + + if (conf->npifs >= ARRAY_SIZE(conf->pif)) { + die("passt has more pifs than pesto can manage (max %d)", + ARRAY_SIZE(conf->pif)); + } + + pc = &conf->pif[conf->npifs]; + pc->pif = pif; + + if (read_all_buf(fd, &info, sizeof(info)) < 0) + die("Error reading from control socket"); + + if (info.name[sizeof(info.name)-1]) + die("Interface name was not NULL terminated"); + /* Redundant, to make static checkers happy */ + info.name[sizeof(info.name) - 1] = '\0'; + + static_assert(sizeof(info.name) == sizeof(pc->name), + "Mismatching pif name lengths"); + memcpy(pc->name, info.name, sizeof(pc->name)); + + debug("PIF %"PRIu8": %s", pc->pif, pc->name); + + /* O(n^2), but n is bounded by MAX_PIFS */ + if (pif_conf_by_num(conf, pc->pif)) + die("Received duplicate interface identifier"); + + /* O(n^2), but n is bounded by MAX_PIFS */ + if (pif_conf_by_name(conf, pc->name)) + die("Received duplicate interface name"); + + conf->npifs++; + return true; +} + +/** + * show_conf() - Show current configuration obtained from passt/pasta + * @conf: Configuration description + */ +static void show_conf(const struct configuration *conf) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + const struct pif_configuration *pc = &conf->pif[i]; + printf(" %s\n", pc->name); + printf(" TBD\n"); + } +} + /** * main() - Dynamic reconfiguration client main program * @argc: Argument count @@ -80,6 +201,7 @@ int main(int argc, char **argv) { 0 }, }; struct sockaddr_un a = { AF_UNIX, "" }; + struct configuration conf = { 0 }; const char *optstring = "dh"; struct pesto_hello hello; struct sock_fprog prog; @@ -162,6 +284,18 @@ int main(int argc, char **argv) "Warning: Using experimental protocol version, client and server must match\n"); }

+ if (ntohl(hello.pif_name_size) != PIF_NAME_SIZE) { + die("Server has unexpected pif name size (%" + PRIu32" not %"PRIu32"\n",

trailing '\n'

Fixed in v7.

...

+ ntohl(hello.pif_name_size), PIF_NAME_SIZE); + } + + while (read_pif_conf(s, &conf)) + ; + + printf("passt/pasta configuration (%s)\n", a.sun_path); + show_conf(&conf); + if (shutdown(s, SHUT_RDWR) < 0 || close(s) < 0) die_perror("Error shutting down control socket");

diff --git a/pesto.h b/pesto.h index 92d4df3..1879759 100644 --- a/pesto.h +++ b/pesto.h @@ -17,18 +17,32 @@ /* Version 0 is reserved for unreleased / unsupported experimental versions */ #define PESTO_PROTOCOL_VERSION 0

+/* Maxmimum size of a pif name, including \0 */

Typo copied from pif.h, should be "Maximum"

Fixed in v7.

...

+#define PIF_NAME_SIZE (128) +#define PIF_NONE 0 + /** * struct pesto_hello - Server introduction message - * @magic: PESTO_SERVER_MAGIC - * @version: Version number + * @magic: PESTO_SERVER_MAGIC + * @version: Version number + * @pif_name_size: Server's value for PIF_NAME_SIZE */ struct pesto_hello { char magic[8]; uint32_t version; + uint32_t pif_name_size; } __attribute__ ((__packed__));

static_assert(sizeof(PESTO_SERVER_MAGIC) == sizeof(((struct pesto_hello *)0)->magic), "PESTO_SERVER_MAGIC has wrong size");

+/** + * struct pesto_pif_info - Message with basic metadata about a pif + * @name: Name (\0 terminated) + */ +struct pesto_pif_info { + char name[PIF_NAME_SIZE]; +} __attribute__ ((__packed__)); + #endif /* PESTO_H */ diff --git a/pif.h b/pif.h index 90dd3a3..d770860 100644 --- a/pif.h +++ b/pif.h @@ -11,6 +11,7 @@

#include

+#include "pesto.h" #include "epoll_type.h"

union inany_addr; @@ -24,7 +25,7 @@ union sockaddr_inany; */ enum pif_type { /* Invalid or not present pif */ - PIF_NONE = 0, + PIF_NONE_ = PIF_NONE, /* Host socket interface */ PIF_HOST, /* Qemu socket or namespace tuntap interface */ @@ -36,7 +37,6 @@ enum pif_type { };

/* Maxmimum size of a pif name, including \0 */

Perhaps you could remove the above comment too?

Fixed in v7.

...

-#define PIF_NAME_SIZE (128) extern const char pif_type_str[][PIF_NAME_SIZE];

-- Stefano