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

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 6 May 2026 15:38:30 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, May 06, 2026 at 01:47:10AM +0200, Stefano Brivio wrote:
> > From: David Gibson <david@gibson.dropbear.id.au>
> > 
> > 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 <david@gibson.dropbear.id.au>
> > [sbrivio: Avoid potential recursive calling between conf_accept() and
> >  conf_close(), reported by clang-tidy]  
> 
> Huh.  For some reason that warning didn't trip for me.  Although it's
> technically true they can mutually recurse, I believe they're both
> tail calls, so it shouldn't eat the stack.
> 
> > [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()]  
> 
> Making the new (accepted) socket non-blocking was the intended
> behaviour here.  We also want non-blocking for the listening socket,
> but that was already done in feab892c7 ("tap, repair: Use
> SOCK_NONBLOCK and SOCK_CLOEXEC on Unix sockets").

Oops, now that Laurent mentioned it, I realised I dropped it
accidentally while / after debugging things on v6, and:

> WIth the current design, I guess we don't want non-blocking on the
> accepted socket, although I don't think it actually matters very much.

...this is the issue I was trying to fix: if the accepted socket is
non-blocking, messages are cut short sometimes, and in general things
don't work.

I don't remember if this was while testing things on Fedora or Debian,
it only happened in one of the two environments.

So, while it was accidental (I really didn't leave any note for a cover
letter, so I'm almost certain there was no other reason for me to drop
it), I think it's actually a good idea to drop it for the following
reasons:

- O_NONBLOCK on the accepted socket breaks things

- the rest looks correct to me but fairly out of scope, and I have very
  limited time for testing things in detail right now, so I'd rather
  keep that patch for later

Without that patch, and my follow-up change to this patch, we're just
adding two lines for this specific behaviour, instead of 18.

> We will want non-blocking it when we change this to read out the
> updated rules incrementally, rather than all at once.

Right, so maybe we can keep that patch for that moment. Or even before,
again, I think it's all correct and desirable, just out of scope (this
isn't the reason why I dropped it though, I would have written a note).

-- 
Stefano