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

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

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

On Sun, Apr 26, 2026 at 09:45:14AM -0400, Jon Maloy wrote:
> 
> 
> On 2026-04-21 02:25, David Gibson wrote:
> > 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.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >   Makefile    |   1 +
> >   common.h    |   2 +
> >   conf.c      |  41 ++++++++++++++++
> >   pesto.c     | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   pesto.h     |  19 +++++++-
> >   pif.h       |   4 +-
> >   serialise.c |   4 ++
> >   serialise.h |   1 +
> >   util.h      |   2 -
> >   9 files changed, 200 insertions(+), 6 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/pesto.h b/pesto.h
> > index 92d4df3a..ac4c2b58 100644
> > --- a/pesto.h
> > +++ b/pesto.h
> > @@ -17,18 +17,33 @@
> >   /* Version 0 is reserved for unreleased / unsupported experimental versions */
> >   #define PESTO_PROTOCOL_VERSION	0
> > +/* Maxmimum 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
> > + * @resv_:	Alignment gap (must be 0)
> 
> You forgot to remove this one.

Oops, yes.

> > + * @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 90dd3a32..d7708603 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,
> 
> This looks a bit weird.

Yes, but it seemed less weird that the other approaches I could think
of to do what I needed.  The key point here is that the specific value
of PIF_NONE is locked as part of the configuration protocol.  The
values of all other pif indices are (intentionally) passt internal and
should not be known by the client.

> As fara as I can see this one is never used.

Yes, that's correct.

> If it is important that PIF_NONE is zero and PIF_HOST is 1,

It's important that PIF_NONE is zero.  It's not important that
PIF_HOST is 1.

> you could just do:
> 
> enum pif_type {
> 
>       PIF_HOST = 1,
> 
>       PIF_TAP,
>       PIF_SPLICE,
> 
>       PIF_NUM_TYPES,
>   };

I could, but I think this makes the connection between the public
PIF_NONE and the private PIF_* less obvious.

> 
> or
> #define PESTO_PIF_END  0
> in pesto.h
> and leave this one as is.

I could - indeed an earlier draft did essentially this.  Hoewver, that
wouldn't enforce that PIF_NONE is equal to the terminator value, which
it should be.

> 
> /jon
> 
> [...]
> 
> item) - (array) < ARRAY_SIZE(array); (item)++)
> 

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