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

On Wed, 6 May 2026 15:38:30 +1000 David Gibson wrote:

On Wed, May 06, 2026 at 01:47:10AM +0200, Stefano Brivio wrote:

...

From: David Gibson

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