From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=hPrIQa1f; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 314E25A0265 for ; Wed, 25 Mar 2026 01:56:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1774400187; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bHPiSHPz8U+/Vo8Llh1ZxKR9SfIZFA56/Mxc1qsdcOM=; b=hPrIQa1f0uGL+FumO3197PeAtKEcJEtqSmVMqXK/tssvrM6bEPyLSiP6yl+deWV5wg6Ffm ZNLG3istZsRQvj6XYd0CLlHD79TZGSKmHqTw94/Io4mA3Aa65RegIJ8Ni/uqh8KCrmXGtS UDbKjxfs7OOizmyVGOASQOq7xkk4AY4= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-265-RFO25M0QPCmoDMAs6uPjTg-1; Tue, 24 Mar 2026 20:56:25 -0400 X-MC-Unique: RFO25M0QPCmoDMAs6uPjTg-1 X-Mimecast-MFC-AGG-ID: RFO25M0QPCmoDMAs6uPjTg_1774400184 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-486fe3b9441so36435155e9.3 for ; Tue, 24 Mar 2026 17:56:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774400184; x=1775004984; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bHPiSHPz8U+/Vo8Llh1ZxKR9SfIZFA56/Mxc1qsdcOM=; b=rDKFZrSF5Ra9t6PwQ8OVP6dFeREQYSfndSX7dTQj8TCbBKNX98peZcdiy5AxEDDduR j0XzUbiFzvfKQOmh1m81LHaGHDzuIJO1qdaUWgsaCGRhCI1Exv6wFQ2CHLKIUVro28zY ezSwVNn+vDE1dTDehGghqowdxbcu9/6gwFfIRqb6XAstacI8R+w3ghdE+e/0zHGQCHw/ JCpE+WQ1O/lHbx2z3e6FqFHAyVJvprNb8/k3msQ1W0JJKHA8dkAcV0RcpUt5QkK25vJJ gL+zJZcSts880Neg0bmZTprvu2602R2PTWTLGjjevcYVUodLuRasRHtQsQisQ+zZ/Bqz d7SA== X-Gm-Message-State: AOJu0Yy2WkuLAAcedZW35oVLyvdgLxT8Obt/fk3rvlAno0eDN4zfnN1A 5c+hHY5T498SHj/YY+scvqrr4sokDbeYhx9JdX90k5sI2iXG4TOhtBMnQzcrfuh79TdQ7sUM6e1 qhu536IE8UJUuHKQ995vdbxSU/1qPDX30E2cCgBkFzcRC9XYTQ3bqymMIm3kVmSID X-Gm-Gg: ATEYQzwysWtJDfSgkemMz/rT8HzXCYlyx5lbvUqyfz5Ty2JsWCrUlpJRnE487ZUQtZK SHBs2oxsKkh5QvS6j+zEF+LkbIWTn++ukUqDQQ2pexpJQrpdU9qlXaZqyU4I9GC6wkWc6iG99vm G4QG4JJ1Vmeu+MakhtppS0TioNl3aJ08ZzdsMa8nTDHcVczVkYOf20/w73TAuVYSCBmxRONhRzC soP+e9pOc57zC6T1/p4oB5brAlJScKdXXuFTLPcH22F49CrO+m4abI2q6QUul0bO/G7PUvGg9AF 2VVvnO/FDwlz0wnzjJ8jErEzOjnIAVXgtVikBGlT9qYkXZn6fm4DT4OXJJLh4pfAOaZseYqviwI 5AFk9S5MkdIzPDD6eZpS1ONoCkCtWtzhX X-Received: by 2002:a05:600c:8b53:b0:485:469f:5320 with SMTP id 5b1f17b1804b1-4871606de9emr22544385e9.30.1774400183630; Tue, 24 Mar 2026 17:56:23 -0700 (PDT) X-Received: by 2002:a05:600c:8b53:b0:485:469f:5320 with SMTP id 5b1f17b1804b1-4871606de9emr22544125e9.30.1774400183038; Tue, 24 Mar 2026 17:56:23 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-487116abe8esm92471115e9.4.2026.03.24.17.56.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Mar 2026 17:56:22 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v3 15/25] pesto: Expose list of pifs to pesto Message-ID: <20260325015621.6f595f6a@elisabeth> In-Reply-To: <20260323073732.3158468-16-david@gibson.dropbear.id.au> References: <20260323073732.3158468-1-david@gibson.dropbear.id.au> <20260323073732.3158468-16-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Wed, 25 Mar 2026 01:56:22 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: ACxdKS85G6lS56BTc9CMxSHo5P9o7MNXBzUkrqlUIXw_1774400184 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: MU2DO45RXHG26FCA7EVXEJTSF6PLRLIE X-Message-ID-Hash: MU2DO45RXHG26FCA7EVXEJTSF6PLRLIE X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Mon, 23 Mar 2026 18:37:22 +1100 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 (for now) prints it out. > > Signed-off-by: David Gibson > --- > conf.c | 38 +++++++++++++++++++++ > pesto.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++- > serialise.c | 21 ++++++++++++ > serialise.h | 3 ++ > 4 files changed, 159 insertions(+), 1 deletion(-) > > diff --git a/conf.c b/conf.c > index 7b960fe9..b878db94 100644 > --- a/conf.c > +++ b/conf.c > @@ -2303,6 +2303,41 @@ void conf(struct ctx *c, int argc, char **argv) > conf_print(c); > } > > +/** > + * conf_send_pifs() - Send list of pifs to dynamic update client (pesto) > + * @c: Execution context > + * @fd: Socket to the client > + * > + * Return: 0 on success, -1 on failure > + */ > +static int conf_send_pifs(const struct ctx *c, int fd) > +{ > + uint32_t num = 0; > + unsigned pif; > + > + /* First count the number of pifs with tables */ > + for (pif = 0; pif < PIF_NUM_TYPES; pif++) { > + if (c->fwd[pif]) > + num++; > + } > + > + if (write_u32(fd, num)) > + return -1; > + > + for (pif = 0; pif < PIF_NUM_TYPES; pif++) { > + if (!c->fwd[pif]) > + continue; > + > + if (write_u8(fd, pif)) > + return -1; > + > + if (write_str(fd, pif_name(pif)) < 0) > + return -1; > + } > + > + return 0; > +} > + > /** > * conf_listen_handler() - Handle events on configuration listening socket > * @c: Execution context > @@ -2359,6 +2394,9 @@ void conf_listen_handler(struct ctx *c, uint32_t events) > "Warning: Using experimental unsupported configuration protocol"); > } > > + if (conf_send_pifs(c, fd) < 0) > + goto fail; > + > return; > > fail: > diff --git a/pesto.c b/pesto.c > index f8c9e01d..ed4f2aab 100644 > --- a/pesto.c > +++ b/pesto.c > @@ -36,6 +36,8 @@ > #include "serialise.h" > #include "pesto.h" > > +static int verbosity = 1; > + > #define die(...) \ > do { \ > FPRINTF(stderr, __VA_ARGS__); \ > @@ -51,6 +53,19 @@ > } \ > } while (0) > > +/** > + * xmalloc() - Allocate memory, with fatal error on failure > + * @size: Number of bytes to allocate > + */ > +static void *xmalloc(size_t size) > +{ > + void *p = malloc(size); I would really really prefer to skip this unless it's absolutely necessary, for a number of reasons: 1. this thing is talking to passt which talks to untrusted workloads, and there's a risk of arbitrary code execution, which implies the possibility of specially crafted messages that might make us allocate: a. in specific regions and build a heap spraying attack based on that b. a lot, and cause a denial of service ...and, while this process should be short lived, we can't assume that, if somebody achieves arbitrary code execution in passt and malicious messages, because we risk looping on rules, or having passt send us information very slowly (minutes / hours), etc. 2. besides, we might want to add a "daemon" mode one day, and then it's not short lived anymore, which opens the door to leaks and double-frees (adding to a. and b. above) 3. we might need to reuse functions from passt and not notice right away that they call this xmalloc(), or have to eventually adapt them anyway 4. consistency, in messaging ("passt doesn't allocate dynamic memory", without caveats) and for auditing reasons Strings can be 32 bytes, or 1024 if we want to splurge. Unless we need to pile a lot of them on the stack (but it's not the case in pesto) a malloc() doesn't really bring any advantage compared to static buffers here and there. It's not megabytes. > + > + if (!p) > + die("Memory allocation failure"); > + return p; > +} > + > /** > * usage() - Print usage, exit with given status code > * @name: Executable name > @@ -69,6 +84,83 @@ static void usage(const char *name, FILE *f, int status) > exit(status); > } > > +/** > + * pesto_recv_str() - Receive a string from passt/pasta > + * @fd: Control socket > + * > + * Return: pointer to malloc()ed string > + */ > +static const char *pesto_recv_str(int fd) > +{ > + uint32_t len; > + char *buf; > + > + if (read_u32(fd, &len) < 0) > + die("Error reading from control socket"); > + > + buf = xmalloc(len); > + if (read_all_buf(fd, buf, len) < 0) > + die("Error reading from control socket"); > + > + return buf; > +} > + > +struct pif_state { > + uint8_t pif; > + const char *name; > +}; > + > +struct conf_state { More as a to-do list item rather than as a review comment: we need documentation for those structs. > + uint32_t npifs; > + struct pif_state pif[]; > +}; > + > +/** > + * pesto_read_pifs() - Read pif names and IDs from passt/pasta > + * @fd: Control socket > + */ > +static const struct conf_state *pesto_read_pifs(int fd) > +{ > + uint32_t num; > + struct conf_state *state; > + unsigned i; > + > + if (read_u32(fd, &num) < 0) > + die("Error reading from control socket"); > + > + debug("Receiving %"PRIu32" interface names", num); > + > + state = xmalloc(sizeof(*state) + num * sizeof(struct pif_state)); > + state->npifs = num; > + > + for (i = 0; i < num; i++) { > + struct pif_state *ps = &state->pif[i]; > + > + if (read_u8(fd, &ps->pif) < 0) > + die("Error reading from control socket"); > + ps->name = pesto_recv_str(fd); > + > + debug("%u: %s", ps->pif, ps->name); > + } > + > + return state; > +} > + > +/** > + * show_state() - Show current rule state obtained from passt/pasta > + * @pifs: PIF name information > + */ > +static void show_state(const struct conf_state *state) > +{ > + unsigned i; > + > + for (i = 0; i < state->npifs; i++) { > + const struct pif_state *ps = &state->pif[i]; > + printf("Forwarding rules for %s interface\n", ps->name); > + printf("\tTBD\n"); > + } > +} > + > /** > * main() - Entry point and whole program with loop > * @argc: Argument count > @@ -91,12 +183,12 @@ int main(int argc, char **argv) > { 0 }, > }; > struct sockaddr_un a = { AF_UNIX, "" }; > + const struct conf_state *state; > const char *optstring = "vh"; > struct pesto_hello hello; > struct sock_fprog prog; > int optname, ret, s; > uint32_t s_version; > - int verbosity = 1; > > prog.len = (unsigned short)sizeof(filter_pesto) / > sizeof(filter_pesto[0]); > @@ -172,5 +264,9 @@ int main(int argc, char **argv) > "Warning: Using experimental protocol version, client and server must match\n"); > } > > + state = pesto_read_pifs(s); > + > + show_state(state); > + > exit(0); > } > diff --git a/serialise.c b/serialise.c > index e3ea86e3..94c34d3c 100644 > --- a/serialise.c > +++ b/serialise.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > > #include "serialise.h" > @@ -121,6 +122,26 @@ int write_all_buf(int fd, const void *buf, size_t len) > return write_all_buf(fd, &beval, sizeof(beval)); \ > } > > +#define be8toh(x) (x) > +#define htobe8(x) (x) > + > +SERIALISE_UINT(8) > SERIALISE_UINT(32) > > #undef SERIALISE_UNIT > + > +/** > + * write_str() - Write a string to an fd in length/value format > + * @fd: Socket to the client > + * @s: String to send > + * > + * Return: 0 on success, -1 on error > + */ > +int write_str(int fd, const char *s) > +{ > + uint32_t len = strlen(s) + 1; /* Include \0 */ > + > + if (write_u32(fd, len) < 0) > + return -1; Even if we don't need to allocate here, I would feel more comfortable if we passed fixed-size strings only to passt to have a slightly lower risk of buffer overflows, in general. > + return write_all_buf(fd, s, len); > +} > diff --git a/serialise.h b/serialise.h > index 0a4ed086..7ef35786 100644 > --- a/serialise.h > +++ b/serialise.h > @@ -16,6 +16,9 @@ int write_all_buf(int fd, const void *buf, size_t len); > int read_u##bits(int fd, uint##bits##_t *val); \ > int write_u##bits(int fd, uint##bits##_t val); > > +SERIALISE_UINT_DECL(8) > SERIALISE_UINT_DECL(32) > > +int write_str(int fd, const char *s); > + > #endif /* _SERIALISE_H */ -- Stefano