From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=fail (p=quarantine dis=none) header.from=redhat.com Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id AC7D35A0265 for ; Sun, 15 Nov 2026 01:29:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1794702569; 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=OSk5K2n19YmkmcRg9RiC6sJ4hOG8ltiKT5ZFgHnf/Q4=; b=ihVRvjB+XBnNszf2QeSnSZ7xZGjuIZk32DazGUQjnK51MZZSn1nn8Z3znMLustHeLLjsej X8KOY4eUJwkVMvIFkPzEQoiIrdEH7q5nnNKsNwl3qzxIV/7iwfBoEX+ICnMfWW0nSCZcUl qVC12L2B8zarhJiM5SDF0VnmCh8Jt0A= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-274-f4IaVaHON9WKI2KIm_3BNg-1; Sat, 14 Nov 2026 19:29:28 -0500 X-MC-Unique: f4IaVaHON9WKI2KIm_3BNg-1 X-Mimecast-MFC-AGG-ID: f4IaVaHON9WKI2KIm_3BNg_1794702567 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-4bb64c99bb8so5216835e9.1 for ; Sat, 14 Nov 2026 16:29:27 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1794702567; x=1795307367; h=date:content-transfer-encoding:content-type: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:content-type; bh=OSk5K2n19YmkmcRg9RiC6sJ4hOG8ltiKT5ZFgHnf/Q4=; b=1bv4OevlC99wRpAnSQFEhidgCP8h20i6XFZlnxj5PToaKwnuGRLJt+3YZHPIJ3vV1r 55vbp/K6/QDFo6dw1acfebZer+rCVWgY5QByuAQG0cvhmhJnvwUlYa+Rf8NROvFSLlN2 Ek0mMP8LbO+OQaDKrRmDDLQSgauuOqeSfawOnmMI9gC4zkDfq2lRnE3r7+QnzT3TWx7k EIE3l6ZL4JsYypbtLXM9/dpNEKKv+UbrxaT0Vq4MGZDZUw9yj9dHm3q7pq9lODaF+0Co kCGWlWJ2aWtxSmiEpt7jZcoN8W6VZIoR9Kl5iXoAZ4Kwe3qM7wZYhaa1IoyFdi3MiXnx h23Q== X-Gm-Message-State: APuU4Z+pwyqgeUQckjfbp2+YUTFqxpaSl318ZyYSQvXebNedd7b1c7hw koZm0TPz4J0mYf4WOpLkooKnEGzUJ23dBdU1bnxVleLBdDstdlq5zfgTps+qlfNRtG+zfs4iU6N 84JLWMdhp2O/dg92Ee0LrkCetTaA9C4xg2hlfiiBxxX2AN5pspExdxg== X-Gm-Gg: AYBFou31FTkbE76A4OYhUIU9hVkPOsjtzA6JdbwNlS1PLcYbMWvlU1yLr7cxO3TSgcE 3TZLA2ixbvplDaYfP6RS4JD3+yE32DYOWPDPMpcuPypwuB3J32oZWoftkzE5CGikGpFRjuB0o7h VHH7zrS8AOc3dlbdpuNjpLROYxQO6bisFbp6XE0IZddiT0DBI+HQqX5LaO41CTUVS5i5t8Z4ImE YGhT1ER4PNfCbeKv0LaL6vYlaVmj1c9y3KuUAXcp0cs5o+4wuNmjWJ8zPdXv7vpjpGS0nrVr6l7 29IQotFTEyqQnCpCsxXuHKkAH6sbskztXKGn/AlCkgwzH4zCkS9ROEtKqv3lfOjjwIkEYWuD9Vc q8La+ZFdTod2bk04X3X6iIDKW7S4shJlrgkUR1HmuKWzyuwBbAMv+m9vsEREsiclAyIk= X-Received: by 2002:a05:600c:6090:b0:4bb:6687:5386 with SMTP id 5b1f17b1804b1-4bb668755b4mr56856755e9.19.1794702566765; Sat, 14 Nov 2026 16:29:26 -0800 (PST) X-Received: by 2002:a05:600c:6090:b0:4bb:6687:5386 with SMTP id 5b1f17b1804b1-4bb668755b4mr56856375e9.19.1794702566149; Sat, 14 Nov 2026 16:29:26 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4bb55ae501bsm190438495e9.10.2026.11.14.16.29.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 14 Nov 2026 16:29:25 -0800 (PST) From: Stefano Brivio To: Hayato Kiwata Subject: Re: [PATCH] conf, pesto: Notify client of the result of rule updates Message-ID: <20261115012923.2502e272@elisabeth> In-Reply-To: <20261113085505.43460-1-dev@haytok.jp> References: <20261113085505.43460-1-dev@haytok.jp> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Sun, 15 Nov 2026 01:29:25 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: GKEKRtyDL_l6ODpeCuTVaMuBbiucuzP4Kl8cFWlgFk0_1794702567 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: E3TR5Z7D57VM6PJO4R7NQDAKIUXTFV7Z X-Message-ID-Hash: E3TR5Z7D57VM6PJO4R7NQDAKIUXTFV7Z 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: Thanks, it looks good to me, and I also tested it briefly. Just two minor details (which I can also fix up on merge if you prefer): On Fri, 13 Nov 2026 17:55:05 +0900 Hayato Kiwata wrote: > Since commit 1776dcb7317e ("fwd, conf: Roll back to old rules if > switching to new ones fails"), pasta tries to restore the old rules when > switching to new rules fails. However, pesto has no way to know that, > and exits successfully as if the new rules were applied. > > In conf_handler(), write a one-byte status to the control socket after > trying to apply the new rules. The status has three values: > PESTO_STATUS_OK, PESTO_STATUS_ROLLBACK, and PESTO_STATUS_FAILED. In > pesto, read the status after send_conf(), and if the update failed, > print what happened and exit with a non-zero status. > > Note that we bump PESTO_PROTOCOL_VERSION to 3 because this adds a > message from the server. If the server version is lower than that, > pesto doesn't read the status. Also, if the server fails to receive > rules from the client, it still closes the connection without sending a > status. > > Link: https://archives.passt.top/passt-dev/20261025153025.600a4fe9@elisabeth/ > Signed-off-by: Hayato Kiwata > > --- > Tested with: > > $ pesto -A --tcp-ports=127.0.0.1/8085:80 /tmp/.pasta.sock > > $ pesto -A --tcp-ports=192.0.2.1/8080:80 /tmp/.pasta.sock > Failed to apply new rules, old rules restored > > $ echo $? > 1 > > $ pesto -s /tmp/.pasta.sock > passt/pasta configuration (/tmp/.pasta.sock) > HOST > TCP [127.0.0.1]:8085 => 80 > SPLICE > > $ curl -sI localhost:8085 | head -1 > HTTP/1.0 200 OK > --- > conf.c | 10 +++++++++- > pesto.c | 19 +++++++++++++++++++ > pesto.h | 20 +++++++++++++++++++- > 3 files changed, 47 insertions(+), 2 deletions(-) > > diff --git a/conf.c b/conf.c > index 7be0c79..f0fc606 100644 > --- a/conf.c > +++ b/conf.c > @@ -2273,6 +2273,7 @@ fail: > void conf_handler(struct ctx *c, uint32_t events) > { > if (events & EPOLLIN) { > + enum pesto_status status = PESTO_STATUS_OK; > unsigned pif; > > /* Clear pending tables */ > @@ -2297,11 +2298,18 @@ void conf_handler(struct ctx *c, uint32_t events) > } > > if (fwd_listen_switch(c) < 0) { > + status = PESTO_STATUS_ROLLBACK; > err("Error switching to new forwarding rules"); > + > /* Switch back to old rules (still in c->fwd_pending) */ > - if (fwd_listen_switch(c) < 0) > + if (fwd_listen_switch(c) < 0) { > + status = PESTO_STATUS_FAILED; > err("Couldn't restore old forwarding rules"); > + } > } > + > + if (write_u8(c->fd_control, status) < 0) > + warn_perror("Failed to write status on control socket"); > } > > if (events & EPOLLHUP) { > diff --git a/pesto.c b/pesto.c > index eeaf274..89ece5d 100644 > --- a/pesto.c > +++ b/pesto.c > @@ -513,6 +513,25 @@ int main(int argc, char **argv) > > send_conf(s, &conf); > > + if (s_version >= PESTO_VERSION_STATUS) { > + uint8_t status; > + > + if (read_u8(s, &status) < 0) > + die_perror("Couldn't read configuration status"); > + > + switch (status) { > + case PESTO_STATUS_OK: > + break; > + case PESTO_STATUS_ROLLBACK: > + die("Failed to apply new rules, old rules restored"); > + case PESTO_STATUS_FAILED: > + die( > +"Failed to apply new rules, couldn't restore old ones"); > + default: > + die("Unknown configuration status %"PRIu8, status); It doesn't impact functionality at least for the moment, but if we just exit on error without shutdown() this becomes inconsistent with the non-error path for no particular reason. It might become relevant the day we decide to export the configuration over TCP as well, or if we implement any kind of "shell" mode for pesto. I'd rather turn those to warn() calls (we don't export err(), it could be done but I don't think we really need it) and add break statements for the PESTO_STATUS_ROLLBACK and PESTO_STATUS_FAILED cases. > + } > + } > + > noupdate: > if (shutdown(s, SHUT_RDWR) < 0 || close(s) < 0) > die_perror("Error shutting down control socket"); > diff --git a/pesto.h b/pesto.h > index 8db701b..5065530 100644 > --- a/pesto.h > +++ b/pesto.h > @@ -19,12 +19,30 @@ > * but was little enough used that we decided not to implement backwards > * compatiblity code (i.e. a v2 pesto will not work with a v1 pasta) > */ > -#define PESTO_PROTOCOL_VERSION 2 > +/* Version 3 added a status byte, sent by the server after trying to apply the > + * new rules (see enum pesto_status) > + */ > +#define PESTO_VERSION_STATUS 3 By the way, I think this is pretty neat. It didn't occur to me that we could (also) define version numbers with specific features. > +#define PESTO_PROTOCOL_VERSION 3 > > /* Maximum size of a pif name, including \0 */ > #define PIF_NAME_SIZE (128) > #define PIF_NONE 0 > > +/** > + * enum pesto_status - Result of a configuration update (protocol version 3+) > + * Nit: we're trying to make all the documentation kerneldoc compatible (https://docs.kernel.org/doc-guide/kernel-doc.html), mostly for the sake of https://bugs.passt.top/show_bug.cgi?id=35, and enums should have their values documented in the header rather than inline, say: * @PESTO_STATUS_OK: New rules applied see also: $ grep -B5 "enum [a-z0-9_]* {" *.c ...I know, we haven't been very consistent with that so far. > + * Sent by the server as a single byte after trying to apply the new rules. > + */ > +enum pesto_status { > + /* New rules applied */ > + PESTO_STATUS_OK = 0, > + /* New rules failed, old rules restored */ > + PESTO_STATUS_ROLLBACK = 1, > + /* New rules failed, old rules couldn't be restored either */ > + PESTO_STATUS_FAILED = 2, > +}; > + > /** > * struct pesto_hello - Server introduction message > * @magic: PESTO_SERVER_MAGIC -- Stefano