[PATCH] fwd, conf: Roll back to old rules if switching to new ones fails

[PATCH] fwd, conf: Roll back to old rules if switching to new ones fails

[Hayato Kiwata]

If fwd_listen_init() fails while switching to new forwarding rules
received from pesto, we currently close the old listening sockets,
report an error and keep the (partially bound) new rules. That leaves
pasta in a state the user didn't ask for.

Make fwd_listen_switch() return the result of fwd_listen_init(), and
call it a second time from conf_handler() on failure to switch back
to the old rules.

Don't reset the count of the old table after closing its sockets, so
that the old rules stay in c->fwd_pending and can be restored. The
pending tables are cleared by conf_handler() before receiving new
rules anyway.

Note that reporting the failure to the client is left for a later patch.

Link: https://archives.passt.top/passt-dev/20261025153025.600a4fe9@elisabeth/
Signed-off-by: Hayato Kiwata <dev@haytok.jp>
---
This is a follow-up to the discussion on the dropped patch linked above.

Tested with:

  $ pasta --ns-ifname=tap0 --mtu=65520 --config-net --address=10.0.2.100 \
      --netmask=24 --gateway=10.0.2.2 --dns-forward=10.0.2.3 --no-map-gw \
      --tcp-ns=none --udp-ns=none --ipv4-only --tcp-ports=none \
      --udp-ports=none --conf-path=/tmp/.pasta.sock \
      -- python3 -m http.server 80

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

pasta logs:

  Listen failed for HOST TCP port 192.0.2.1/8080: Cannot assign requested address
  Couldn't listen on requested ports
  Error switching to new forwarding rules

and the rule for port 8085 keeps working. Adding another valid rule
afterwards works as before.

 conf.c |  7 ++++++-
 fwd.c  | 10 +++++-----
 fwd.h  |  2 +-
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/conf.c b/conf.c
index 149a4c7..0d10ea4 100644
--- a/conf.c
+++ b/conf.c
@@ -2222,7 +2222,12 @@ void conf_handler(struct ctx *c, uint32_t events)
 				       "    ", "");
 		}
 
-		fwd_listen_switch(c);
+		if (fwd_listen_switch(c) < 0) {
+			err("Error switching to new forwarding rules");
+			/* Switch back to old rules (still in c->fwd_pending) */
+			if (fwd_listen_switch(c) < 0)
+				err("Couldn't restore old forwarding rules");
+		}
 	}
 
 	if (events & EPOLLHUP) {
diff --git a/fwd.c b/fwd.c
index 5679a3d..a081664 100644
--- a/fwd.c
+++ b/fwd.c
@@ -536,8 +536,10 @@ int fwd_listen_init(const struct ctx *c)
 /**
  * fwd_listen_switch() - Switch from current to pending rules table
  * @c:		Execution context
+ *
+ * Return: 0 on success, -1 on failure
  */
-void fwd_listen_switch(struct ctx *c)
+int fwd_listen_switch(struct ctx *c)
 {
 	struct fwd_table *tmp[PIF_NUM_TYPES];
 	unsigned i;
@@ -549,9 +551,8 @@ void fwd_listen_switch(struct ctx *c)
 		if (!fwd)
 			continue;
 
-		debug("Flushing %u old %s rules", fwd->count, pif_name(i));
+		debug("Flushing %u %s rules", fwd->count, pif_name(i));
 		fwd_listen_close(fwd);
-		fwd->count = fwd->sock_count = 0;
 	}
 
 	/* Swap active and pending tables */
@@ -563,8 +564,7 @@ void fwd_listen_switch(struct ctx *c)
 	memcpy((void *)c->fwd_pending, &tmp, sizeof(c->fwd_pending));
 
 	/* Start listening on the new tables */
-	if (fwd_listen_init(c) < 0)
-		err("Error switching to new forwarding rules");
+	return fwd_listen_init(c);
 }
 
 /* See enum in kernel's include/net/tcp_states.h */
diff --git a/fwd.h b/fwd.h
index b60697d..a6c7bd8 100644
--- a/fwd.h
+++ b/fwd.h
@@ -61,7 +61,7 @@ int fwd_listen_sync(const struct ctx *c, uint8_t pif,
 		    const struct fwd_scan *tcp, const struct fwd_scan *udp);
 void fwd_listen_close(const struct fwd_table *fwd);
 int fwd_listen_init(const struct ctx *c);
-void fwd_listen_switch(struct ctx *c);
+int fwd_listen_switch(struct ctx *c);
 
 bool nat_inbound(const struct ctx *c, const union inany_addr *addr,
 		 union inany_addr *translated);
-- 
2.50.1 (Apple Git-155)