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=cRRN31Qr; dkim-atps=neutral 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 1757D5A004E for ; Wed, 11 Feb 2026 10:09:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1770800996; 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=LPfcnHJj8dOlz+jKTJEVsLutqlbjAVanph2V1BtU4eI=; b=cRRN31QrdKPIZXErpttCIlb3m8OsUJKzcu4K3YrYtUiKu+ZOmVjB2Z7ABGyB6TISiUqVpU vS/8vriG9f0O0n71XlyPrWfOhaplwmHSWLzqL4zXkAt5nNUWESlRiJFcGrpfyZoVNGZXxM GUiNv2dskoJGB8YH5AifAb+Vm8qQD7k= 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-452-OyRXv0aiMwyH2QVxsr67vg-1; Wed, 11 Feb 2026 04:09:55 -0500 X-MC-Unique: OyRXv0aiMwyH2QVxsr67vg-1 X-Mimecast-MFC-AGG-ID: OyRXv0aiMwyH2QVxsr67vg_1770800994 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-43768e2aa4dso2940140f8f.0 for ; Wed, 11 Feb 2026 01:09:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770800994; x=1771405794; h=date:content-transfer-encoding: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; bh=LPfcnHJj8dOlz+jKTJEVsLutqlbjAVanph2V1BtU4eI=; b=r3qxXzKead2GFKcVoLQFNxip5WcgRcdFPeF5E/mv+XGShEEXfdHOFVe4pdkqSaFS1G Ke4fOaNRw9vYAXLn2uXrBmvOZD3ObFr4Xtxz52woF0y3qRasUCIgFwBK/xxfilXfI8PM YFAzLFX73V5pIP3bz44RpyUVbz5o/4A15vC92YmbgwmZevvnIDppD0yXoQQsjGrE6jbU S8R6cdYlufOa+4Y1y+0RIzr1BVenSAxh3A3Yx7cfEx48Jrn8Lb6UIcOmnqIWKvUIzaym r0AUKkzfCJbeBnts1bh2pJVoidyA5IxC5gmvwvQ9bxXX1IP/Ov4SnussememhcvZblTW brsg== X-Gm-Message-State: AOJu0YzQU6oewAa9f1WDgcX3/VMv4H6pmJbrqWIWIfYzCRUMrSnguSEk 2ue93icEe2TRXiFqWyVWaKnsqRxOwskqpUcqwkjZ4EZ489Iy8mObhNXzRX/eXsInyKpTGXcZ48O DSwfVu9E1P3VLFPiox6LJT6zlDPOuUgZQWZ0wjTkXD4sfI8NMsNaumQ== X-Gm-Gg: AZuq6aLeg29C1ElIYKkxRCAp8c0lCcRET3sav/q8D3sVJvCtNo6GX6TL76WkC4Wk8TV dW6viUGCW8WGDhE8w7Zuls0NZmrOlgHbM5aApUss8qkEbolZD1cx/PUQT12Kd34AiABVoqjfJ5m Y5CtCc8fkOGnQkd52OlCcTDev+4PoU9W6Pg9ORXKOmhmthf6BJ30aSS9FxjsgfpFlAa7/IDvrhs VKQ0CJ3DU2MRudLyxqe4JWtQ2dF29z1KuB0r2cMdhvflKU+zE/DNEj0BxRBBri6Q3As3tUnIPZr e+/Xs3c8q2xUiwbe79gBXZNWQhK8p1KQ7yeuaI38Ir8IEnBiXguuybvc7uoN0+/bXtGpTmyA59g MY53dzU6frHArpQeQGYH3izO8ziTtM8qQ X-Received: by 2002:a05:6000:2083:b0:436:18e5:48 with SMTP id ffacd0b85a97d-4377a5243e7mr8231922f8f.15.1770800993973; Wed, 11 Feb 2026 01:09:53 -0800 (PST) X-Received: by 2002:a05:6000:2083:b0:436:18e5:48 with SMTP id ffacd0b85a97d-4377a5243e7mr8231855f8f.15.1770800993363; Wed, 11 Feb 2026 01:09:53 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43783e5c93asm2789801f8f.39.2026.02.11.01.09.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Feb 2026 01:09:52 -0800 (PST) From: Stefano Brivio To: David Gibson Subject: Re: [WIP PATCH] Introduce configuration interface and configuration tool (pesto) Message-ID: <20260211100941.1d606ebb@elisabeth> In-Reply-To: References: <20260204234209.455262-1-sbrivio@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Wed, 11 Feb 2026 10:09:52 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: A9SLKpTBVWzcbGdwr_mJDHUAgTehyXKeY6zFBPq2yxI_1770800994 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: DTNELLWHMH6MFIJCVXCK6GMKSAU2F5Z4 X-Message-ID-Hash: DTNELLWHMH6MFIJCVXCK6GMKSAU2F5Z4 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: On Tue, 10 Feb 2026 15:15:36 +1100 David Gibson wrote: > On Thu, Feb 05, 2026 at 12:42:09AM +0100, Stefano Brivio wrote: > > Draft, very rudimentary. Notably lacking: > > > > - any form of error checking, documentation > > > > - support for anything that's not ports > > > > - delete/insert operations > > > > - proper handling for EPOLLHUP / EPOLLERR > > > > - command responses > > > > - headers, versioning, etc. > > > > Example: > > > > ./pasta --debug --config-net -c /tmp/pasta.conf -t none > > > > ./pesto /tmp/pasta.conf add 8889 > > > > Signed-off-by: Stefano Brivio > > Have some time to look at this, while Bass Strait glides past outside. > > For the most part, LGTM, as far as it goes. Just note that I shared this as early as I could not so much to get a review (see above as to why it will get half rewritten anyway), rather so that you could use it to sketch delete operations in the new table, as we discussed. I will go through your comments anyway once it's more or less complete, but many might be irrelevant at that point. Just a couple of examples of that, and some replies which might make vaguely sense: > [...] > > > + if (!getsockopt(c->fd_conf, SOL_SOCKET, SO_PEERCRED, &uc, &len)) > > + info("Accepted configuration client, PID %i", uc.pid); > > Is displaying the client's PID actually going to be useful? I'm not sure, but this is mostly copied and pasted from passt-repair code for the sake of time. > By the > time this happens, we'll have isolated our own pidns, but the client > will likely be outside our pidns. The client's pid as translated into > our pidns will therefore not really be meaningful (probably 0 or -1 or > something). Correct, it's usually 0, unless we run in foreground. In that case it was actually helpful (more like debug() stuff though) with passt-repair. > > + ref.fd = c->fd_conf; > > + > > + rc = epoll_add(c->epollfd, EPOLLHUP | EPOLLET, ref); > > EPOLLHUP is implicitly included. Surely we need EPOLLIN as well, no? That was my intention and I forgot, but actually it works pretty well with just EPOLLHUP because I just get one event, read everything... probably a bit too "risky" for actual non-prototyping operation though. > Since we're already assuming we get the config ops in aligned chunks, > can we avoid the global buffer and read ops one at a time into a local > buffer? It depends on how this is implemented on the server side: the current idea (as we discussed) would be to have a shadow table and replace it in a single transaction, so in that case we would need just another copy of the table, but not a permanent list of operations. So, yes, this will *probably* go away. > > + } > > + > > +close: > > + for (i = 0; i < conf_ops_bytes_read / sizeof(conf_ops[0]); i++) { > > + struct fwd_rule *r = &conf_ops[i].r; > > This is probably already on your plan, but as with migration, I think > we should explicitly translate from the control stream format, rather > than embedding struct fwd_rule, so that we decouple the internal rule > format from the control message format. Also like migration, I think > we should make sure the control format is the same, regardless of > platform specific padding, alignment, and endianness. Maybe, even though the case is much weaker here. Sure, the client could run on another machine eventually and we'd need to translate ports... I'm not sure. > > + > > + debug("%s rule %i, ports %i-%i:%i", > > + conf_ops[i].op == RULE_ADD ? "adding" : "deleting", i, > > + r->first, r->last, r->to); > > + > > + if (conf_ops[i].op == RULE_ADD) { > > + fwd_rule_add(&c->tcp.fwd_in, r->flags, &r->addr, > > + /* r->ifname */ NULL, > > Why ignoring r->ifname? Because it would have taken me a few minutes minutes to take care of it. > Also probably in your plan, but we want to update fwd_rule_add() to > make errors non-fatal as a preliminary to this. Unless it uses another table (see above) and in that case errors might need to be fatal. > > + r->first, r->last, r->to); > > + } else { > > + ; /* TODO */ > > + } > > + } > > + > > + fwd_listen_sync(c, &c->tcp.fwd_in, PIF_HOST, IPPROTO_TCP); > > + > > + debug("Closing configuration socket"); > > + epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_conf, NULL); > > + close(c->fd_conf); > > Why always close() after a single batch of ops? Easier to sketch, no need to keep track of 'count'. > [...] > > > + prog.len = (unsigned short)sizeof(filter_pesto) / > > + sizeof(filter_pesto[0]); > > + prog.filter = filter_pesto; > > + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) || > > + prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) { > > + fprintf(stderr, "Failed to apply seccomp filter\n"); > > + _exit(1); > > The case for seccomp also seems weaker for this client (it's not > exposed to guest or peer traffic).... ...it might be long lived, we don't know yet. It's exposed to some bits of data / information from passt. > > + } > > + > > + if (argc < 4) { > > + fprintf(stderr, > > + "Usage: %s PATH add|del SPEC [auto] [weak] \\" > > + "[add|del SPEC [auto] [weak]] ...\n", > > + argv[0]); > > IIUC, the code below allows multiple add/del on the same command line, > which this usage() doesn't reflect. It does in my opinion, hence the ..., but I don't remember how it's commonly represented in usage strings. > > + _exit(2); > > ... and especially the case for excluding malloc() (and the subsequent > complications with exit() and various other things) seems pretty weak > for this client. ...unless it's long lived. -- Stefano