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

[Laurent Vivier <lvivier@redhat.com>]

On 5/4/26 14:01, Laurent Vivier wrote:
> On 5/3/26 23:55, 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]
>> [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]
>> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> 
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>

but...

> 
>> ---
>>   Makefile     |   8 ++-
>>   conf.c       | 183 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   conf.h       |   2 +
>>   epoll_type.h |   4 ++
>>   passt.1      |   5 ++
>>   passt.c      |   8 +++
>>   passt.h      |   6 ++
>>   pesto.c      |  47 ++++++++++++-
>>   pesto.h      |  22 +++++++
>>   serialise.c  |   3 +
>>   10 files changed, 282 insertions(+), 6 deletions(-)
>>
...
>> diff --git a/pesto.c b/pesto.c
>> index f0916e8..762cfe9 100644
>> --- a/pesto.c
>> +++ b/pesto.c
>> @@ -33,6 +33,7 @@
>>   #include "common.h"
>>   #include "seccomp_pesto.h"
>> +#include "serialise.h"
>>   #include "pesto.h"
>>   #include "log.h"
>> @@ -66,6 +67,8 @@ static void usage(const char *name, FILE *f, int status)
>>    *
>>    * Return: 0 on success, won't return on failure
>>    *
>> + * #syscalls:pesto socket s390x:socketcall i686:socketcall
>> + * #syscalls:pesto connect shutdown close
>>    * #syscalls:pesto exit_group fstat read write
>>    */
>>   int main(int argc, char **argv)
>> @@ -76,9 +79,12 @@ int main(int argc, char **argv)
>>           {"version",    no_argument,        NULL,        1 },
>>           { 0 },
>>       };
>> +    struct sockaddr_un a = { AF_UNIX, "" };
>>       const char *optstring = "dh";
>> +    struct pesto_hello hello;
>>       struct sock_fprog prog;
>> -    int optname;
>> +    int optname, ret, s;
>> +    uint32_t s_version;
>>       prctl(PR_SET_DUMPABLE, 0);
>> @@ -122,5 +128,42 @@ int main(int argc, char **argv)
>>       debug("debug_flag=%d, path=\"%s\"", debug_flag, argv[optind]);
>> -    die("pesto is not implemented yet");
>> +    if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0)
>> +        die_perror("Failed to create AF_UNIX socket");
>> +
>> +    ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[optind]);
>> +    if (ret <= 0 || ret >= (int)sizeof(a.sun_path))
>> +        die("Invalid socket path \"%s\"", argv[optind]);
>> +
>> +    ret = connect(s, (struct sockaddr *)&a, sizeof(a));
>> +    if (ret < 0) {
>> +        die_perror("Failed to connect to %s", a.sun_path);
>> +    }
>> +
>> +    ret = read_all_buf(s, &hello, sizeof(hello));
>> +    if (ret < 0)
>> +        die_perror("Couldn't read server greeting");
>> +
>> +    if (memcmp(hello.magic, PESTO_SERVER_MAGIC, sizeof(hello.magic)))
>> +        die("Bad magic number from server");
>> +
>> +    s_version = ntohl(hello.version);
>> +
>> +    if (s_version > PESTO_PROTOCOL_VERSION) {
>> +        die("Unknown server protocol version %"PRIu32" > %"PRIu32"\n",

The trailing '\n' is not needed.

>> +            s_version, PESTO_PROTOCOL_VERSION);
>> +    }
>> +
...