Re: [PATCH v9 11/23] pesto: Expose list of pifs to pesto and display them

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

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

On Wed, May 06, 2026 at 11:22:29AM +0200, Stefano Brivio wrote:
> From: David Gibson <david@gibson.dropbear.id.au>
> 
> 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 prints it out.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> [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]
> [sbrivio: Fix minor nits reported by Laurent]
> [sbrivio: Initialise struct pesto_pif_info in conf_send_rules() with
>  zeroes, otherwise the pif name might be seen as not terminated, and
>  we'll expose memory from the back-end]

Oops, good catch.

> [sbrivio: Fix conflicts in Makefile]
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  common.h    |   2 +
>  conf.c      |  41 ++++++++++++++++
>  pesto.c     | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  pesto.h     |  18 ++++++-
>  pif.h       |   5 +-
>  serialise.c |   4 ++
>  serialise.h |   1 +
>  util.h      |   2 -
>  8 files changed, 200 insertions(+), 7 deletions(-)
> 
> diff --git a/common.h b/common.h
> index 4251781..68573b4 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 5ec0072..f8c2134 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1927,6 +1927,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 = { 0 };
> +		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
> @@ -1970,6 +2007,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 };
> @@ -2010,6 +2048,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 ab476c5..b33492a 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 ")",
> +		    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 3c93d3e..fda0ef6 100644
> --- a/pesto.h
> +++ b/pesto.h
> @@ -17,18 +17,32 @@
>  /* Version 0 is reserved for unreleased / unsupported experimental versions */
>  #define PESTO_PROTOCOL_VERSION	1
>  
> +/* Maximum size of a pif name, including \0 */
> +#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 553c742..48d4919 100644
> --- a/pif.h
> +++ b/pif.h
> @@ -11,6 +11,7 @@
>  
>  #include <netinet/in.h>
>  
> +#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 */
> @@ -35,8 +36,6 @@ enum pif_type {
>  	PIF_NUM_TYPES,
>  };
>  
> -/* Maximum size of a pif name, including \0 */
> -#define	PIF_NAME_SIZE	(128)
>  extern const char pif_type_str[][PIF_NAME_SIZE];
>  
>  static inline const char *pif_type(enum pif_type pt)
> diff --git a/serialise.c b/serialise.c
> index 346df99..e083112 100644
> --- a/serialise.c
> +++ b/serialise.c
> @@ -121,6 +121,10 @@ int write_all_buf(int fd, const void *buf, size_t len)
>  		return write_all_buf(fd, &beval, sizeof(beval));	\
>  	}
>  
> +#define	be8toh(x)	(x)
> +#define	htobe8(x)	(x)
> +
> +SERIALISE_UINT(8)
>  SERIALISE_UINT(32)
>  
>  #undef SERIALISE_UINT
> diff --git a/serialise.h b/serialise.h
> index a88f3de..4714f4c 100644
> --- a/serialise.h
> +++ b/serialise.h
> @@ -16,6 +16,7 @@ int write_all_buf(int fd, const void *buf, size_t len);
>  	int read_u##bits(int fd, uint##bits##_t *val);			\
>  	int write_u##bits(int fd, uint##bits##_t val);
>  
> +SERIALISE_UINT_DECL(8)
>  SERIALISE_UINT_DECL(32)
>  
>  #endif /* SERIALISE_H */
> diff --git a/util.h b/util.h
> index e90be47..c788382 100644
> --- a/util.h
> +++ b/util.h
> @@ -87,8 +87,6 @@ void abort_with_msg(const char *fmt, ...)
>  #define V6		1
>  #define IP_VERSIONS	2
>  
> -#define ARRAY_SIZE(a)		((int)(sizeof(a) / sizeof((a)[0])))
> -
>  #define foreach(item, array)						\
>  	for ((item) = (array); (item) - (array) < ARRAY_SIZE(array); (item)++)
>  
> -- 
> 2.43.0
> 

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