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 CE7C85A0265 for ; Sun, 25 Oct 2026 15:30:31 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1792938630; 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=HD/wqtyLGGRZV25wRxg5VLTshqqhFzvhjYue0iKSob8=; b=H7s1bxMoM+xVMCXw36P1xd5UY4kQzX/JADEMPKNlkzLZlh/BCChIHbSRL/p30+wIVAFjWi MJkDrYd6rxpC2HaiiOcbfE8ZE24/Snz6F7kVPLGCPmz2CZjz0qg/w4yE4KrzLWfF3IZLIp Ps1PeYTWT5mpdMqgITqyeyRQYvE5XVo= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-304-m2Bbr1kZPRutQAqxf5YS7A-1; Sun, 25 Oct 2026 10:30:29 -0400 X-MC-Unique: m2Bbr1kZPRutQAqxf5YS7A-1 X-Mimecast-MFC-AGG-ID: m2Bbr1kZPRutQAqxf5YS7A_1792938628 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-48c7fe9736eso1934671f8f.0 for ; Sun, 25 Oct 2026 07:30:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1792938628; x=1793543428; 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=HD/wqtyLGGRZV25wRxg5VLTshqqhFzvhjYue0iKSob8=; b=If4unS6Q4rsad3l54LtUSx602LogGwm2MjBg3s16BjMX/mFweT/skddZ28onG+81t2 9C0zwnsmdZQ/qvqj053IMM1be2geXRiPNugz4FhXJ9nOUQYzMIAU3BOrnGITMZI62pcw w/q+QNSzfpwPYXpeRKpYJRSD0OiRzxgCX3dOtonOGiyHBBErSde5YmpaN5Zg6nZSSKTw uWIU5zXOKslohhfzXgeSHHmLK2VT0UgcOPXGwXW24drXtRNwN+EDbH/jv5XKV+3he+O7 hmOtH+S1wON1QKzhcl+RHVjLBYk78FBKBB6TYSgyvRkLYZ5/A20v7lp1rVQN6mXXbePy SeTA== X-Gm-Message-State: AFq9FYJUIz+P6rjwqfJH2TCIvVAp4AZu7AeFqN6QcKOk0QBQw4qg8VbG zLCO+aEWtq/XzsMgqoEebyOUtPlAxYNllyOMLAnYGEgMveJKnKCVWX3YuwjJ/wqEah5yEFNe2K+ 3ynH8TyU8x8kFaZ0ooG9ef8Mu1QJYUAvLLRD2uj/hO5gtSpHy4h90mHYeMs4Nng== X-Gm-Gg: AYBFou1fHGhxqyMgRxsFdxn0NguiDw6jDIxjO3DX1YXaalz0vpKI0CN8vM/p6LeDkCw rgl2BrjX33dSuw0OY/DWmZppczECAQdJ5E7RpeM4NmTtAL/U7LwVauymZLVctkzJSzSYgyJR5cm ASmEadUCTZBLK53xL1Ekl1q8GeBa/h5rLG9XErqyHCoivBnXuNORqbaKsO+9EoQ4N8Q2H3jhJ6K ybVRfVaxW7tTDN3ApzmkT/hNorvMY14CTyb8/nFhHStfsSvHdYjbVH6D/48NdgEvHyIVr63Bcnj R2dfTgQHvYvUWyefl+JjqgbOiWSDDreJKkghV2C8/AVra21aVMF9CalhmRDyy6Wji0N3TMqmBZp eHPdtYqHAYA== X-Received: by 2002:a05:6000:2312:b0:48d:faa4:5b66 with SMTP id ffacd0b85a97d-48e3eba7bd7mr7191219f8f.36.1792938627699; Sun, 25 Oct 2026 07:30:27 -0700 (PDT) X-Received: by 2002:a05:6000:2312:b0:48d:faa4:5b66 with SMTP id ffacd0b85a97d-48e3eba7bd7mr7191174f8f.36.1792938627113; Sun, 25 Oct 2026 07:30:27 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-48e3ebc39a8sm11272214f8f.11.2026.10.25.07.30.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 25 Oct 2026 07:30:26 -0700 (PDT) From: Stefano Brivio To: Hayato Kiwata , David Gibson Subject: Re: [PATCH] fwd: Keep listening on remaining rules if one of them fails to bind Message-ID: <20261025153025.600a4fe9@elisabeth> In-Reply-To: <20261018154225.56640-1-dev@haytok.jp> References: <20261018154225.56640-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, 25 Oct 2026 15:30:26 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: lEjBcJvfjApl75zrP0nsldSNx0TrkGiGPFqImJP7mnw_1792938628 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 3OONA3N5KF4ARZWRBPIQHFACCBAS6THM X-Message-ID-Hash: 3OONA3N5KF4ARZWRBPIQHFACCBAS6THM 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: Hi Hayato, On Mon, 19 Oct 2026 00:42:25 +0900 Hayato Kiwata 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 > --- > 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