From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202512 header.b=Ogt+Re6m; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id C76635A0626 for ; Fri, 12 Dec 2025 03:32:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1765506727; bh=zXPHvRHmnEibZObQg6xNg8PJ3mU22CuSl4gu4iGM++E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ogt+Re6mm2EqQDHWSGRGzi7AWaKNTBFMaqo1VnAGWD3LeE9l2sQwiM0K3qMUKzHWb ht63Co6RDA75ts+mdKHQ8ZRXgQWZ2ln2czZn/Q54dVkUDd07j8DbY/Adu2Vwe5DKAT MbVhRRUB11r5aCsv449w6m2U5rYkn36If3j06aOeLJ12iK0VfoxItXdcayRuf5EZG/ xezEjWlKgeRXRIQYOomAyxuZ98AdlJeqB/HRicKIrM6ZYL+BA0Woou69Um6tuY+ghK Zt46et6vIOYJ1zaggkIPGf+GwISAwQfpaopwldmFvwTn2lZGde90px9237+i8sSZrx 5kwFfsc43BSuw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dSD4R6r7jz4wQb; Fri, 12 Dec 2025 13:32:07 +1100 (AEDT) Date: Fri, 12 Dec 2025 13:32:01 +1100 From: David Gibson To: Paul Holzinger Subject: Re: [PATCH 2/2] pasta: Clean up waiting pasta child on failures Message-ID: References: <20251210051552.1157177-1-david@gibson.dropbear.id.au> <20251210051552.1157177-3-david@gibson.dropbear.id.au> <3b553775-1ee8-4b87-8b9c-ff0dec281576@redhat.com> <3151ef6f-c170-4d83-b562-8785f7fb5c67@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="2L3SqFtXdVLoCOJi" Content-Disposition: inline In-Reply-To: <3151ef6f-c170-4d83-b562-8785f7fb5c67@redhat.com> Message-ID-Hash: ZVVUWK3X4FAEUKFRRC4TXBPWCUD4JMQB X-Message-ID-Hash: ZVVUWK3X4FAEUKFRRC4TXBPWCUD4JMQB X-MailFrom: dgibson@gandalf.ozlabs.org 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: Stefano Brivio , 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: --2L3SqFtXdVLoCOJi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 11, 2025 at 02:49:50PM +0100, Paul Holzinger wrote: >=20 > On 11/12/2025 00:45, David Gibson wrote: > > On Wed, Dec 10, 2025 at 01:05:10PM +0100, Paul Holzinger wrote: > > > On 10/12/2025 06:15, David Gibson wrote: > > > > When pasta is invoked with a command rather than an existing namesp= ace to > > > > attach to, it spawns a child process to run a shell or other comman= d. We > > > > create that process during conf(), since we need the namespace to e= xist for > > > > much of our setup. However, we don't want the specified command to= run > > > > until the pasta network interface is ready for use. Therefore, > > > > pasta_spawn_cmd() executing in the child waits before exec()ing. m= ain() > > > > signals the child to continue with SIGUSR1 shortly before entering = the > > > > main forwarding loop. > > > >=20 > > > > This has the downside that if we exit due to any kind of failure be= tween > > > > conf() and the SIGUSR1, the child process will be around waiting > > > > indefinitely. The user must manually clean this up. > > > >=20 > > > > Make this cleaner, by having passt_exit() terminate the child, when= called > > > > during this window. > > > >=20 > > > > Signed-off-by: David Gibson > > > > --- > > > > passt.c | 1 + > > > > pasta.c | 2 ++ > > > > pasta.h | 1 + > > > > util.c | 5 +++++ > > > > 4 files changed, 9 insertions(+) > > > >=20 > > > > diff --git a/passt.c b/passt.c > > > > index cf38822f..955c7091 100644 > > > > --- a/passt.c > > > > +++ b/passt.c > > > > @@ -431,6 +431,7 @@ int main(int argc, char **argv) > > > > if (pasta_child_pid) { > > > > kill(pasta_child_pid, SIGUSR1); > > > > log_stderr =3D false; > > > > + pasta_child_signalled =3D true; > > > > } > > > > isolate_postfork(&c); > > > > diff --git a/pasta.c b/pasta.c > > > > index 5c693de1..8ac4511f 100644 > > > > --- a/pasta.c > > > > +++ b/pasta.c > > > > @@ -54,6 +54,8 @@ > > > > /* PID of child, in case we created a namespace */ > > > > int pasta_child_pid; > > > > +/* Has the child been signalled to start a shell or command */ > > > > +bool pasta_child_signalled; > > > > /** > > > > * pasta_child_handler() - Exit once shell exits (if we started = it), reap clones > > > > diff --git a/pasta.h b/pasta.h > > > > index 4b063d13..55028c74 100644 > > > > --- a/pasta.h > > > > +++ b/pasta.h > > > > @@ -7,6 +7,7 @@ > > > > #define PASTA_H > > > > extern int pasta_child_pid; > > > > +extern bool pasta_child_signalled; > > > > void pasta_open_ns(struct ctx *c, const char *netns); > > > > void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, > > > > diff --git a/util.c b/util.c > > > > index e266c396..4744f09e 100644 > > > > --- a/util.c > > > > +++ b/util.c > > > > @@ -35,6 +35,7 @@ > > > > #include "log.h" > > > > #include "pcap.h" > > > > #include "epoll_ctl.h" > > > > +#include "pasta.h" > > > > #ifdef HAS_GETRANDOM > > > > #include > > > > #endif > > > > @@ -1235,6 +1236,10 @@ void abort_with_msg(const char *fmt, ...) > > > > */ > > > > void passt_exit(int status) > > > > { > > > > + /* If we're starting our own namespace, don't leave it in limbo */ > > > > + if (pasta_child_pid && !pasta_child_signalled) > > > > + kill(pasta_child_pid, SIGTERM); > > > Any reason not to use SIGKILL? Then there is no doubt if it might be = ignored > > > or not. > > Eh, only that some log might show KILLs on the assumption that they > > indicate a bad thing happening, which isn't really the case here. > >=20 > > > Another thing I assume the goal here is to only kill the process if we > > > didn't exec yet. > > Yes, that was the goal. > >=20 > > > I wonder how much value there is to have the child process > > > outlive pasta. > > That's a good question, and I don't have a strong opinion either way. > > I leant this way based on two factors: > >=20 > > - If this happens later, once the child is established, it's possible > > the user could have started running something there that remains > > valuable even if it loses its network > >=20 > > - If pasta dies once the child has exec()ed, the symptoms are both > > more obvious and easier to clean up: the shell (or whatever) is > > right there, running, and can be exited in the normal way > > (e.g. running to completion or ^C). Before the exec() this leaves > > a process running non-obviously which has to be spotted and killed > > explicitly by the user. > >=20 > > (also note that if we do kill the child after exec(), we definitely > > want to use SIGTERM not SIGKILL to allow it to clean up gracefully). >=20 > sure, I agree though there is bit of a common issue here. The process is = in > a new pid namespace so it acts as pid 1. That means unless the process ad= ds > a signal handler explicitly a signal like SIGTERM will get ignored by > default. When running `sleep` as pid 1 it will ignore SIGTERM and you must > kill it. We see that all the time with podman containers. Ah, good point. I guess my earlier draft only worked because we do, in fact have a SIGTERM handler. --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --2L3SqFtXdVLoCOJi Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmk7fqAACgkQzQJF27ox 2GcRCg/+NdKUfuE85tMduwUXyIbp+A1+X3WX9YwUEBe9QbtCCmKw+HKHIrWPAFoF mmckQCQ7TP6fjOFHndcOS/dmKDmTAl+fFHIvkg2SSp8eDW8CWlJ9kN9KVMS4uFGA 3sz2i62h20fYGEGea4rlMCS4bRVVhIC0GNk/Sn9p07BdZh4Y52VtRNEcjIQ2BPvI 4rZPmFfq+VkHNHKYn2t7n9PBM9eX73ILOB7SLi2YNLJZ8SiryK7m4H0+Z7xlt5gN aqxW4UvOJnnz1fZ/PDnQEXJuIlzMNHy17OpTVoPQYZSKc4Oa+IZNCUDvkPWB+5Io Jzqi3aVrhhnYVcv+A5hdecjKE/mO2jW7H7i8WZIHEWEkcgcIwTW+Jb/T2Nbh0jZC cG9k8gB668NYCnLNHpfSECdrlmlu9h1UQRt9GuzKvmYQLb77bcgVnf7dmdMQopJ8 LCjJGWaeEFZq5pV14bU1kBfxGUixE7h9H2sjIXuj6gJO77sIMJGdpQASHtVQCR0H qd7kvdXt+PEvK6cIHi+T/ksk3asel8npM7PBjZNxhlRDGM/Y8rJjrWq6sotDwbhf UGERK/GAbo0844hou3ItmVYjQhcs4DsSh2Rm85u1IuqyaGfr5XTUmKmgB2gqd1sS wrFMQa6iXM3rEWgpDLWdGiUrhMKmisOoaJU08kNGeZcJnVevA0c= =PJY/ -----END PGP SIGNATURE----- --2L3SqFtXdVLoCOJi--