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

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 29 Apr 2026 15:17:34 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

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

Fixed in v6.

> > > + * @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.

Left as it was in v6.

-- 
Stefano