From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Dnzm64oA; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 587CA5A0265 for ; Fri, 17 Jul 2026 13:14:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1784286848; 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=+1XE42TofolNiRc7kr271YEk0AVX5AaLAPsDReI8Svc=; b=Dnzm64oAcFR6AlEBm+wDWuVxqS0flb2zuZJuUxeTLhcZ8JrWZ0+EqQXSN/m6swPkbleHDS G/OsbdPUfK4O2AfuOU4nKnCktyBUEf/iKD1OickiletPICuqm1jQHSkyISqjug/XeYdfdd zZbI808FnqNgkcCgdVDhd1HhqFwtpT8= 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-31-ODHPe9z5NrCEChxPexxmZQ-1; Fri, 17 Jul 2026 07:14:06 -0400 X-MC-Unique: ODHPe9z5NrCEChxPexxmZQ-1 X-Mimecast-MFC-AGG-ID: ODHPe9z5NrCEChxPexxmZQ_1784286845 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-47e81aca584so4079233f8f.2 for ; Fri, 17 Jul 2026 04:14:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784286845; x=1784891645; 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=+1XE42TofolNiRc7kr271YEk0AVX5AaLAPsDReI8Svc=; b=quy7pZ8G2lomtBO5YOdlGUFSyi1TzNFXQKx6N6cwYKzL79bRCGW/LdHINIWiqDCbJc /7A7vwZANdBtrAjLPwQeIpUgNwu7ISgUNUjCwSHVLo3KRUqd9TjHRBmJedb1BxxqqXke KDNPd2Wwz5BH29o1nhDShYWyNIyS/xkhsDtWt3fnZEkPmvHI2kphbo9Divy4uJJkYwXL yVPJQNQUDzXU7W0SN1NaXk2alOxkPeWrV0Vv65ysJTNudx/EFCyv8V2ZAukfczlVLTK0 XHh24+v19f3/yqd/XsPiMwuxAr7LGZPcEgJzuaQmVbfBQqHIPd6cDrcKQBwxgDuSZMx/ WWEg== X-Forwarded-Encrypted: i=1; AHgh+RreBYKFSRvNqjis4KhOzagGJ9yr6VHPbeo4A3RVZDH/Qkj86xqxrBS9NjmILvU8hPhPe7KiA0Ty9vI=@passt.top X-Gm-Message-State: AOJu0YwmywRRTcFNc6iJy+W8igfneYBLIFgRiopLiwGnygZ6qJNsZV+k H6axAtXjkaND/Uf/rSGFrFiz+VtJV2BdNq/2dLiMJGZIs7HaEh+QpzmKolf1iueVC/YL19KqY5F 3U99Mz9UFOzzWC0mBdjPFiwrCRUpe5YWXDYgV8bGWp8ROBTH6KHATC6K+h7QDGw== X-Gm-Gg: AfdE7ckBCCEve6vArsnC6mylIz8DZaJkmVtp/8cPp+xynLXhE69IAd+dRZ8zyZH2hcq /WJ5G7va2cOFJAuo4zdZGR/NLp9onQNPQNR5teyBXJKe1u8f+exaawYTl2u1MVuPeEcm6mgQVWO 2kTh9IA4Dc+YcWSOATt4QZ7XgNK9ljjK37TbrkXpP50ViY6HS7QmYvK8OerM0JDFpw9ln33PK7o EZlRKeahivCHRjk+Ju2+T2X6OM22Z38K9AKBqveeK7Drr6QkhQRPrb4O3HctAzsFX2HjlwDO4pC YfhGXuSAhc4IfbrzWBeJICMshytilfh3DzGDc3uUaN8R7csQvUSwwYbhD6DFHUOnYL7/bItXNGq 6iiJ2RjEDZl4= X-Received: by 2002:a05:6000:2083:b0:472:3090:d878 with SMTP id ffacd0b85a97d-47f623075abmr2742790f8f.4.1784286845101; Fri, 17 Jul 2026 04:14:05 -0700 (PDT) X-Received: by 2002:a05:6000:2083:b0:472:3090:d878 with SMTP id ffacd0b85a97d-47f623075abmr2742723f8f.4.1784286844392; Fri, 17 Jul 2026 04:14:04 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47f63ec7e49sm2836695f8f.19.2026.07.17.04.14.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Jul 2026 04:14:03 -0700 (PDT) From: Stefano Brivio To: Richard Lawrence Subject: Re: [PATCH] pasta: Add --pass-fds option to preserve specific file descriptors Message-ID: <20260717131402.43168eb8@elisabeth> In-Reply-To: References: <20260716190300.57944-1-rlawrence@tamu.edu> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Fri, 17 Jul 2026 13:14:03 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: FGx__dytO4DckQqqX4Zh3FdCKPRWjn-_KRc877cBqOQ_1784286845 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: I4AH7DWM7UTPI5K6RROOHHMOU64SOLG3 X-Message-ID-Hash: I4AH7DWM7UTPI5K6RROOHHMOU64SOLG3 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: David Gibson , passt-dev@passt.top, jbash@jbash.com 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: On Fri, 17 Jul 2026 16:14:06 +1000 David Gibson wrote: > On Thu, Jul 16, 2026 at 02:03:00PM -0500, Richard Lawrence wrote: > > Introduce a new long-only command line option '--pass-fds' for > > pasta mode. This option accepts a comma-separated list of file > > descriptor numbers. It parses, validates, sorts, and deduplicates > > these file descriptors, and then skips closing them by calling > > close_range() on the gaps between them inside close_open_files(). > > > > The option is documented in the usage help and the passt.1 man page. > > This commit message really needs to say *why* this is a desirable > change. It needs to make the case for this, not just say what the > patch does. > > I'm guessing this is for the case of trying to pass fds not to pasta > per se, but to the command running within the namespace pasta creates. Right, I'm also guessing this is about: https://bugs.passt.top/show_bug.cgi?id=204 but it definitely needs to be stated in the commit message. > If that's the case, there's already a workaround that's good enough, > I'm not (so far) convinced there's a case for an option like this. > Specifically, you can manually create a namespace with unshare, and > pass whatever fds you wish to it, then attach pasta to that existing > namespace, rather than having pasta create its own. > > Even if I accept the need for passing arbitrary fds to a command > spawned by pasta, I think this is an overly complex way of > accomplishing it: the alternate approach would be to delay the closing > of extra fds until after the pasta command is spawned. That comes with the issue that we would still pass those file descriptors on to the command we spawn, which is what we want to avoid with close_open_files(), or isolate_fds() after the series mentioned below. I think we should still close those as soon as possible, but I agree that this is an overly complex way to implement the option. For example, we don't actually need to sort the array, we could actually scan the array while closing ranges directly, that is: 1. close the first open interval between standard error, or the file descriptor passed as --fd (which is now moved to index 3), and the file descriptor with the lowest index in the array. Remove this index, i, from the array 2. as long as there is any file descriptor left in the array, close the range between i and the lowest index (open interval), and set i to this index that should yield a significantly more compact implementation. Also have a look at CONTRIBUTING.md for coding style references. In general this patch seems to hammer together several general-purpose bits until they work for this specific option, instead of directly implement what we need (and nothing more). I would suggest giving a try at _writing_ that implementation instead of somehow generating it. It's probably going to be shorter and easier to audit. > Less importantly, this patch will definitely conflict with some > patches I've just posted, so will likely need reworking and rebasing > for that. I guess / hope this implementation should become simpler on top of those patches (which fixes some actual issues so we would need that series first anyway). Richard, if you need a reference, this would be David's series at: https://archives.passt.top/passt-dev/20260717054634.1293553-1-david@gibson.dropbear.id.au/ which you can apply to your local tree with: b4 shazam https://archives.passt.top/passt-dev/20260717054634.1293553-1-david@gibson.dropbear.id.au/ > > Co-authored-by: rarensu <40872193+rarensu@users.noreply.github.com> > > Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> > > I'm not clear to me if these are people, or LLMs. We haven't devised > a formal policy for AI assisted contributions, but for that reason > amongst others I would certainly argue against using Co-authored-by as > the way of indicating it. By the way, the first one seems to be Richard himself, the second one is a language model, but indeed, please don't use those tags, because it's quite debatable whether google-labs-jules[bot] is actually an author. Just add the specific information in plain English I would say. > > Signed-off-by: Richard Lawrence > > --- > > conf.c | 9 +++++- > > passt.1 | 5 ++++ > > util.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++------- > > 3 files changed, 96 insertions(+), 11 deletions(-) > > > > diff --git a/conf.c b/conf.c > > index 5b6cc2b..0032d02 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -732,7 +732,9 @@ pasta_opts: > > " Don't copy all addresses to namespace\n" > > " --ns-mac-addr ADDR Set MAC address on tap interface\n" > > " --no-splice Disable inbound socket splicing\n" > > - " --splice-only Only enable loopback forwarding\n"); > > + " --splice-only Only enable loopback forwarding\n" > > + " --pass-fds FDS Comma-separated list of fds to pass to\n" > > + " the spawned command\n"); > > > > passt_exit(status); > > } > > @@ -1316,6 +1318,7 @@ void conf(struct ctx *c, int argc, char **argv) > > {"stats", required_argument, NULL, 31 }, > > {"conf-path", required_argument, NULL, 'c' }, > > {"chroot-fallback", no_argument, NULL, 32 }, > > + {"pass-fds", required_argument, NULL, 33 }, > > { 0 }, > > }; > > const char *optstring = "+dqfel:hs:c:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; > > @@ -1557,6 +1560,10 @@ void conf(struct ctx *c, int argc, char **argv) > > case 32: > > c->chroot_fallback = true; > > break; > > + case 33: > > + if (c->mode != MODE_PASTA) > > + die("--pass-fds is for pasta mode only"); > > + break; > > case 'd': > > c->debug = 1; > > c->quiet = 0; > > diff --git a/passt.1 b/passt.1 > > index 1570f6a..93c9b24 100644 > > --- a/passt.1 > > +++ b/passt.1 > > @@ -755,6 +755,11 @@ of local traffic in pasta\fR in the \fBNOTES\fR for more details. > > Do not create a tap device in the namespace. In this mode, \fIpasta\fR only > > forwards loopback traffic between namespaces. > > > > +.TP > > +.BR \-\-pass\-fds " " \fIfds\fR > > +Pass a comma-separated list of file descriptors to the spawned command. > > +These file descriptors will be kept open. > > + > > .SH EXAMPLES > > > > .SS \fBpasta > > diff --git a/util.c b/util.c > > index 4bc5d6f..c73fd74 100644 > > --- a/util.c > > +++ b/util.c > > @@ -932,29 +932,102 @@ const char *str_ee_origin(const struct sock_extended_err *ee) > > void close_open_files(int argc, char **argv) > > { > > const struct option optfd[] = { { "fd", required_argument, NULL, 'F' }, > > + { "pass-fds", required_argument, NULL, 33 }, > > { 0 }, > > }; > > long fd = -1; > > - int name, rc; > > + const char *pass_fds_arg = NULL; > > + int name, rc = 0; > > + int old_opterr = opterr; > > > > + opterr = 0; > > do { > > name = getopt_long(argc, argv, "-:F:", optfd, NULL); > > > > if (name == 'F') > > fd = conf_tap_fd(optarg); > > + else if (name == 33) > > + pass_fds_arg = optarg; > > } while (name != -1); > > + opterr = old_opterr; > > > > - if (fd == -1) { > > - rc = close_range(STDERR_FILENO + 1, ~0U, CLOSE_RANGE_UNSHARE); > > - } else if (fd == STDERR_FILENO + 1) { /* Still a single range */ > > - rc = close_range(STDERR_FILENO + 2, ~0U, CLOSE_RANGE_UNSHARE); > > - } else { > > - rc = close_range(STDERR_FILENO + 1, fd - 1, > > - CLOSE_RANGE_UNSHARE); > > - if (!rc) > > - rc = close_range(fd + 1, ~0U, CLOSE_RANGE_UNSHARE); > > + #define MAX_PASS_FDS 1024 > > + unsigned int keep_fds[MAX_PASS_FDS]; > > + int keep_fds_cnt = 0; > > + > > + if (fd != -1 && fd > STDERR_FILENO) { > > + keep_fds[keep_fds_cnt++] = (unsigned int)fd; > > + } > > + > > + if (pass_fds_arg) { > > + const char *p = pass_fds_arg; > > + while (*p) { > > + char *endptr; > > + unsigned long val = strtoul(p, &endptr, 10); > > + if (p == endptr) { > > + die("Invalid --pass-fds option: %s", pass_fds_arg); > > + } > > + if (val > INT_MAX || val <= STDERR_FILENO) { > > + die("Invalid file descriptor in --pass-fds: %lu", val); > > + } > > + if (keep_fds_cnt >= MAX_PASS_FDS) { > > + die("Too many file descriptors in --pass-fds"); > > + } > > + keep_fds[keep_fds_cnt++] = (unsigned int)val; > > + if (*endptr == ',') { > > + p = endptr + 1; > > + if (*p == '\0') { > > + die("Invalid --pass-fds option: trailing comma"); > > + } > > + } else if (*endptr == '\0') { > > + p = endptr; > > + } else { > > + die("Invalid character in --pass-fds option: %s", pass_fds_arg); > > + } > > + } > > } > > > > + /* Sort keep_fds in ascending order */ > > + for (int i = 1; i < keep_fds_cnt; i++) { > > + unsigned int key = keep_fds[i]; > > + int j = i - 1; > > + while (j >= 0 && keep_fds[j] > key) { > > + keep_fds[j + 1] = keep_fds[j]; > > + j--; > > + } > > + keep_fds[j + 1] = key; > > + } > > + > > + /* Remove duplicates */ > > + int write_idx = 0; > > + for (int i = 0; i < keep_fds_cnt; i++) { > > + if (i == 0 || keep_fds[i] != keep_fds[i - 1]) { > > + keep_fds[write_idx++] = keep_fds[i]; > > + } > > + } > > + keep_fds_cnt = write_idx; > > + > > + /* Close ranges */ > > + unsigned int current = STDERR_FILENO + 1; > > + for (int i = 0; i < keep_fds_cnt; i++) { > > + unsigned int f = keep_fds[i]; > > + if (current < f) { > > + int call_rc = close_range(current, f - 1, CLOSE_RANGE_UNSHARE); > > + if (call_rc != 0) > > + rc = call_rc; > > + } > > + current = f + 1; > > + } > > + if (keep_fds_cnt == 0 || keep_fds[keep_fds_cnt - 1] < ~0U) { > > + int call_rc = close_range(current, ~0U, CLOSE_RANGE_UNSHARE); > > + if (call_rc != 0) > > + rc = call_rc; > > + } > > + > > + /* Note: We assume that the parent process did not set FD_CLOEXEC on the > > + * passed file descriptors, so we do not explicitly clear it here. > > + */ > > + > > if (rc) { > > if (errno == ENOSYS || errno == EINVAL) { > > /* This probably means close_range() or the > > -- > > 2.52.0 > > >