[PATCH] conf, pesto: Notify client of the result of rule updates

[PATCH] conf, pesto: Notify client of the result of rule updates

[Hayato Kiwata]

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 <dev@haytok.jp>

---
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);
+		}
+	}
+
 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
+#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+)
+ *
+ * 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
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] conf, pesto: Notify client of the result of rule updates

[Stefano Brivio]

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 <dev@haytok.jp> 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 <dev@haytok.jp>
> 
> ---
> 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