* Re: [PATCH] fwd: Keep listening on remaining rules if one of them fails to bind
2026-10-18 15:42 [PATCH] fwd: Keep listening on remaining rules if one of them fails to bind Hayato Kiwata
@ 2026-10-25 14:30 ` Stefano Brivio
0 siblings, 0 replies; 2+ messages in thread
From: Stefano Brivio @ 2026-10-25 14:30 UTC (permalink / raw)
To: Hayato Kiwata, David Gibson; +Cc: passt-dev
Hi Hayato,
On Mon, 19 Oct 2026 00:42:25 +0900
Hayato Kiwata <dev@haytok.jp> wrote:
> fwd_listen_sync_() breaks out of the loop as soon as fwd_sync_one()
> fails. So, if a single rule fails to bind(), for example because the
> address isn't assigned to the host, no rule after it in the table is
> even tried, and those forwards don't work.
>
> Reproducer:
>
> $ 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
> UNIX domain socket bound at /tmp/.pasta.sock
> Serving HTTP on 0.0.0.0 port 80 (http://0.0.0.0:80/) ...
>
> $ pesto -A --tcp-ports=192.0.2.1/8080:80 /tmp/.pasta.sock
> $ pesto -A --tcp-ports=127.0.0.1/8085:80 /tmp/.pasta.sock
> $ pesto -s /tmp/.pasta.sock
> passt/pasta configuration (/tmp/.pasta.sock)
> HOST
> TCP [192.0.2.1]:8080 => 80
> TCP [127.0.0.1]:8085 => 80
> SPLICE
> $ curl -sS -o /dev/null localhost:8085
> curl: (7) Failed to connect to localhost port 8085 after 0 ms: Could not connect to server
>
> pasta only reports the failure for port 8080, and never tries to
> listen on port 8085:
>
> 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
This was actually intended, at some point in the design phase, because
the original idea was to revert to the previous configuration in these
two cases:
1. the new rules are invalid. This is relatively simple to do:
conf_recv_rules() detects this and conf_handler() returns early,
skipping the call to fwd_listen_switch(), so the switch to the new
tables of forwarding rules doesn't happen at all, before we apply
any change
2. the new rules are generally valid but not (entirely) working in the
setup (like in your example).
Here we call fwd_listen_switch() which closes the listening sockets
for the old table, swaps the tables, and calls fwd_listen_init(),
which returns failure, but we don't handle that because this kind of
rollback feature isn't implemented (yet).
I think we should decide how to handle 2. and make this consistent. As
you showed, we're not handling this consistently.
We're not sure whether a rollback after 2. would work: forwarding rules
that might have worked earlier might not work now (for example because
host addresses changed).
However, in a general case, I guess that the existing (old) rules are
still more likely to work than not. That is, given:
a. new forwarding rules don't work because of an unintended
configuration error
b. old forwarding rules wouldn't work anymore if re-applied because of
a setup change
I think a. is much more likely, so a rollback would be beneficial and
give the user the chance to look at possible configuration errors
without switching to a possibly unintended state. What do you think?
If we decide to go ahead and complete the rollback feature, I guess the
missing steps are roughly these:
1. abort on any failure of fwd_listen_init() (maintaining the current
behaviour of fwd_listen_sync_()), and:
2. close any listening sockets (first part of fwd_listen_switch()) that
we might have just opened
3. swap the rules again (as you can see, we already preserve the old
rules, because we planned to complete the rollback implementation at
some point)
4. call fwd_listen_init() on the old rules
5. make sure we report failure to the client
...given that we need 2., 3. and 4., we could probably generalise
fwd_listen_switch() and use it in reverse once we detect a failure.
About your patch itself:
> Keep going through the remaining rules if fwd_sync_one() fails, and
> still return -1 if any of them failed, as before.
>
> Signed-off-by: Hayato Kiwata <dev@haytok.jp>
> ---
> fwd.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fwd.c b/fwd.c
> index 5679a3d..6c9df01 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -456,9 +456,8 @@ static int fwd_listen_sync_(void *arg)
> ns_enter(a->c);
>
> for (i = 0; i < a->c->fwd[a->pif]->count; i++) {
> - a->ret = fwd_sync_one(a->c, a->pif, i, a->tcpmap, a->udpmap);
> - if (a->ret < 0)
> - break;
> + if (fwd_sync_one(a->c, a->pif, i, a->tcpmap, a->udpmap) < 0)
> + a->ret = -1;
...this looks reasonable to me and better than the existing behaviour,
but it's pointless to do this if we want to finish things up and add
the rollback for this case.
> }
>
> return 0;
--
Stefano
^ permalink raw reply [flat|nested] 2+ messages in thread