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=KsO+1QaZ; 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 EE53F5A0265 for ; Wed, 25 Mar 2026 01:55:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1774400112; 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=R8wHm93gIkqmTUwSVicf7jA74NFuxIDTjdt1trtayvQ=; b=KsO+1QaZD1SwVhnRf5anmuzBA6caSAacBpx2E3yzGO3W3fNWlvl+HKKsknDmSLkGrunWg/ hra7+HVyYotHQrHKD17YX8mzHQ8ZKdWT43ysPNC5aD2a+iJmGYsEIRMVty/PS/OqYS1lRc E40980ucc5sv7QSM+c1zNbc0vi6XQ/E= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-148-soo410LCO3aPPw_ZiLv6VQ-1; Tue, 24 Mar 2026 20:55:11 -0400 X-MC-Unique: soo410LCO3aPPw_ZiLv6VQ-1 X-Mimecast-MFC-AGG-ID: soo410LCO3aPPw_ZiLv6VQ_1774400110 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-485345e2fdfso25963715e9.2 for ; Tue, 24 Mar 2026 17:55:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774400109; x=1775004909; 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=R8wHm93gIkqmTUwSVicf7jA74NFuxIDTjdt1trtayvQ=; b=UcmMwZ8Lt0qh9jFKDShwQ+LqxcSwQJLYkuc2F6Ar638+Jyqw383y4l5iNEhLyzVSLx WEmdEvvT+ANWoUq6kT1c0cRt+Fbx3EaiBpuHyjfUMKs6U0Y4Jp5KY3yJzM6qjSIqEAup HadilBvmYf/e+SI34QQfNLs8E0bhiYal6WF868ufmY1Kn5+ImuLa+2+zkx6o5Vdj1GmK vNVNrmPJI2bLzjHhfC+2JlzsbqKDWjAYEiwa3M9zmJqZgEEBN5woghXmkD7ssMOUOBHs e+/x5q40ia0c02yPtu1epDWGla6L7Gd68VjwohipgMz27mXmZfhhAy9ctloclq5jqylQ CuaQ== X-Gm-Message-State: AOJu0Yw2j0UZPA0G31URx2gsXz8yL19HrXuRigRN9hH9FbhSM+UoARfr +6sKSL8TGhm2dU/TGl5SkpXCmYyJKp2Dye01Hf5Q7GJGcmxdWfckwY5rH/3cnGidXdGO84eInGw RnjL3AhbRwFoZMU6eB839TFyhNCeeX+uvYzyygw+555GHRXqeD3vu/ZMoBXHeGa07 X-Gm-Gg: ATEYQzw395Dllk+vaUL0nOAT9cHWCE2ftGqvN13eOxtUXiJ4QSbGuAQ9ryqiKBIwb+1 iRVwAjJ2EKhebVvcdvHMJF0lnNqWVdJM+MgR1upHR7EXadzYzcxGsaoRLko/kiakNhuaKG8FBuc 3uXkg92V8e9li8yOYtBJA3+yl7aAGnzkV1lkaT/VLyz7iNI6mZWl19HGU21F0loa6lAHMRb0f2w hT/FbRhO68jqfkx1kQ/i5vr7LF+txuNJA/kJJ3Ozl8p6WpUxDtFgropp5xaFOE337M7cDywFBkN WfASmnuUb68UbR5aR9QvH8HyoJOiO6tPD8GtLZRx6ib4uRr5odfjHhSDTPtHhb/C5ig2GP34iS5 7zq8o7VQQrWNJqP+ZLWOQH+kMxQYPiBC8dyo6rtG3wyX4HSADTQ== X-Received: by 2002:a05:600c:c16e:b0:485:3dfc:569 with SMTP id 5b1f17b1804b1-48716042b0emr21470285e9.16.1774400109038; Tue, 24 Mar 2026 17:55:09 -0700 (PDT) X-Received: by 2002:a05:600c:c16e:b0:485:3dfc:569 with SMTP id 5b1f17b1804b1-48716042b0emr21470065e9.16.1774400108465; Tue, 24 Mar 2026 17:55:08 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48717390c4csm6214235e9.31.2026.03.24.17.55.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Mar 2026 17:55:07 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v3 14/25] pesto: Add command line option parsing and debug messages Message-ID: <20260325015506.201d9ca0@elisabeth> In-Reply-To: <20260323073732.3158468-15-david@gibson.dropbear.id.au> References: <20260323073732.3158468-1-david@gibson.dropbear.id.au> <20260323073732.3158468-15-david@gibson.dropbear.id.au> 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, 25 Mar 2026 01:55:07 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: Kx2ab-KqxXysq4VEbzyimaX3AZCNNFjFiNHwxzIk8EI_1774400110 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 4RBNHLHY2WNSSZ6ZWFPNFUWXLAJOIQLV X-Message-ID-Hash: 4RBNHLHY2WNSSZ6ZWFPNFUWXLAJOIQLV 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 Mon, 23 Mar 2026 18:37:21 +1100 David Gibson wrote: > Add basic command line option parsing using getopt_long() to pesto. > Implement --help, --version, --quiet and --verbose options, along with a Two questions: - do we really need --quiet? - shouldn't we stick to the passt(1) convention where we avoided -v altogether to avoid confusion, and we just have --version and -d / --debug? > debug() macro to print debugging messages when given the verbose option. > > Signed-off-by: David Gibson > --- > common.h | 8 ++++++ > conf.c | 45 +++++++++++++++++---------------- > passt.h | 12 ++++----- > pesto.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++------ > util.h | 8 ------ > 5 files changed, 108 insertions(+), 42 deletions(-) > > diff --git a/common.h b/common.h > index 76a95609..927d20a4 100644 > --- a/common.h > +++ b/common.h > @@ -8,6 +8,14 @@ > #ifndef COMMON_H > #define COMMON_H > > +#define VERSION_BLOB \ > + VERSION "\n" \ > + "Copyright Red Hat\n" \ > + "GNU General Public License, version 2 or later\n" \ > + " \n" \ > + "This is free software: you are free to change and redistribute it.\n" \ > + "There is NO WARRANTY, to the extent permitted by law.\n\n" > + > /* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */ > #define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__) > > diff --git a/conf.c b/conf.c > index 5e8bc665..7b960fe9 100644 > --- a/conf.c > +++ b/conf.c > @@ -1140,6 +1140,9 @@ static void conf_print(const struct ctx *c) > char bufmac[ETH_ADDRSTRLEN], ifn[IFNAMSIZ]; > int i; > > + if (c->fd_control_listen >= 0) > + info("Configuration socket: %s", c->control_path); > + > if (c->ifi4 > 0 || c->ifi6 > 0) { > info("Template interface: %s%s%s%s%s", > c->ifi4 > 0 ? if_indextoname(c->ifi4, ifn) : "", > @@ -1432,15 +1435,15 @@ static void conf_open_files(struct ctx *c) > die_perror("Couldn't open PID file %s", c->pidfile); > } > > - c->fd_conf = -1; > - if (*c->conf_path) { > - c->fd_conf_listen = sock_unix(c->conf_path); > - if (c->fd_conf_listen < 0) { > + c->fd_control = -1; > + if (*c->control_path) { For later: maybe I guess we should squash the renaming of this file descriptor with the earlier patch adding it. > + c->fd_control_listen = sock_unix(c->control_path); > + if (c->fd_control_listen < 0) { > die_perror("Couldn't open control socket %s", > - c->conf_path); > + c->control_path); > } > } else { > - c->fd_conf_listen = -1; > + c->fd_control_listen = -1; > } > } > > @@ -1485,13 +1488,13 @@ static void conf_sock_listen(const struct ctx *c) > { > union epoll_ref ref = { .type = EPOLL_TYPE_CONF_LISTEN }; > > - if (c->fd_conf_listen < 0) > + if (c->fd_control_listen < 0) > return; > > - if (listen(c->fd_conf_listen, 0)) > + if (listen(c->fd_control_listen, 0)) > die_perror("Couldn't listen on configuration socket"); > > - ref.fd = c->fd_conf_listen; > + ref.fd = c->fd_control_listen; > if (epoll_add(c->epollfd, EPOLLIN | EPOLLET, ref)) > die_perror("Couldn't add configuration socket to epoll"); > } > @@ -1846,11 +1849,11 @@ void conf(struct ctx *c, int argc, char **argv) > c->fd_tap = -1; > break; > case 'c': > - ret = snprintf(c->conf_path, sizeof(c->conf_path), "%s", > - optarg); > + ret = snprintf(c->control_path, sizeof(c->control_path), > + "%s", optarg); > if (ret <= 0 || ret >= (int)sizeof(c->sock_path)) > die("Invalid configuration path: %s", optarg); > - c->fd_conf_listen = c->fd_conf = -1; > + c->fd_control_listen = c->fd_control = -1; > break; > case 'F': > errno = 0; > @@ -2294,10 +2297,10 @@ void conf(struct ctx *c, int argc, char **argv) > FWD_SCAN); > } > > + conf_sock_listen(c); > + > if (!c->quiet) > conf_print(c); > - > - conf_sock_listen(c); > } > > /** > @@ -2321,7 +2324,7 @@ void conf_listen_handler(struct ctx *c, uint32_t events) > return; > } > > - fd = accept4(c->fd_conf_listen, NULL, NULL, SOCK_NONBLOCK); > + fd = accept4(c->fd_control_listen, NULL, NULL, SOCK_NONBLOCK); > if (fd < 0) { > warn_perror("accept4() on configuration listening socket"); > return; > @@ -2331,7 +2334,7 @@ void conf_listen_handler(struct ctx *c, uint32_t events) > warn_perror("Can't get configuration client credentials"); > > /* Another client is already connected: accept and close right away. */ > - if (c->fd_conf != -1) { > + if (c->fd_control != -1) { > info("Discarding configuration client, PID %i", uc.pid); > goto fail; > } > @@ -2349,7 +2352,7 @@ void conf_listen_handler(struct ctx *c, uint32_t events) > goto fail; > } > > - c->fd_conf = fd; > + c->fd_control = fd; > info("Accepted configuration client, PID %i", uc.pid); > if (!PESTO_PROTOCOL_VERSION) { > warn( > @@ -2374,7 +2377,7 @@ void conf_handler(struct ctx *c, uint32_t events) > ssize_t n; > > do { > - n = read(c->fd_conf, discard, sizeof(discard)); > + n = read(c->fd_control, discard, sizeof(discard)); > if (n > 0) > debug("Discarded %zd bytes of config data", n); > } while (n > 0); > @@ -2397,7 +2400,7 @@ void conf_handler(struct ctx *c, uint32_t events) > > close: > debug("Closing configuration socket"); > - epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_conf, NULL); > - close(c->fd_conf); > - c->fd_conf = -1; > + epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_control, NULL); > + close(c->fd_control); > + c->fd_control = -1; > } > diff --git a/passt.h b/passt.h > index c38bb5ae..b3f049de 100644 > --- a/passt.h > +++ b/passt.h > @@ -158,7 +158,7 @@ struct ip6_ctx { > * @foreground: Run in foreground, don't log to stderr by default > * @nofile: Maximum number of open files (ulimit -n) > * @sock_path: Path for UNIX domain socket > - * @conf_path: Path for configuration UNIX domain socket > + * @control_path: Path for control/configuration UNIX domain socket > * @repair_path: TCP_REPAIR helper path, can be "none", empty for default > * @pcap: Path for packet capture file > * @pidfile: Path to PID file, empty string if not configured > @@ -170,8 +170,8 @@ struct ip6_ctx { > * @epollfd: File descriptor for epoll instance > * @fd_tap_listen: File descriptor for listening AF_UNIX socket, if any > * @fd_tap: AF_UNIX socket, tuntap device, or pre-opened socket > - * @fd_conf_listen: Listening configuration socket, if any > - * @fd_conf: Configuration socket, if any > + * @fd_control_listen: Listening control/configuration socket, if any > + * @fd_control: Control/configuration socket, if any > * @fd_repair_listen: File descriptor for listening TCP_REPAIR socket, if any > * @fd_repair: Connected AF_UNIX socket for TCP_REPAIR helper > * @our_tap_mac: Pasta/passt's MAC on the tap link > @@ -226,7 +226,7 @@ struct ctx { > int foreground; > int nofile; > char sock_path[UNIX_PATH_MAX]; > - char conf_path[UNIX_PATH_MAX]; > + char control_path[UNIX_PATH_MAX]; > char repair_path[UNIX_PATH_MAX]; > char pcap[PATH_MAX]; > > @@ -244,8 +244,8 @@ struct ctx { > int epollfd; > int fd_tap_listen; > int fd_tap; > - int fd_conf_listen; > - int fd_conf; > + int fd_control_listen; > + int fd_control; > int fd_repair_listen; > int fd_repair; > unsigned char our_tap_mac[ETH_ALEN]; > diff --git a/pesto.c b/pesto.c > index 6d066084..f8c9e01d 100644 > --- a/pesto.c > +++ b/pesto.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -42,6 +43,32 @@ > exit(EXIT_FAILURE); \ > } while (0) > > +#define debug(...) \ > + do { \ > + if (verbosity > 1) { \ > + FPRINTF(stderr, __VA_ARGS__); \ > + FPRINTF(stderr, "\n"); \ > + } \ > + } while (0) > + > +/** > + * usage() - Print usage, exit with given status code > + * @name: Executable name > + * @f: Stream to print usage info to > + * @status: Status code for exit(2) > + */ > +static void usage(const char *name, FILE *f, int status) > +{ > + FPRINTF(f, "Usage: %s [OPTION]... PATH\n", name); > + FPRINTF(f, > + "\n" > + " -v, --verbose Be more verbose\n" > + " -q, --quiet Be less verbose\n" > + " -h, --help Display this help message and exit\n" > + " --version Show version and exit\n"); > + exit(status); > +} > + > /** > * main() - Entry point and whole program with loop > * @argc: Argument count > @@ -56,13 +83,20 @@ > */ > int main(int argc, char **argv) > { > + const struct option options[] = { > + {"quiet", no_argument, NULL, 'q' }, > + {"verbose", no_argument, NULL, 'v' }, > + {"help", no_argument, NULL, 'h' }, > + {"version", no_argument, NULL, 1 }, > + { 0 }, > + }; > struct sockaddr_un a = { AF_UNIX, "" }; > + const char *optstring = "vh"; > struct pesto_hello hello; > struct sock_fprog prog; > + int optname, ret, s; > uint32_t s_version; > - int ret, s; > - > - prctl(PR_SET_DUMPABLE, 0); I would, at least by default, keep this exactly in line with the paranoid stuff I added to passt, for consistency and ease of auditing: what will happen if we run *this* tool as root? If we choose to drop to nobody in that case, removing this will be a problem. And we'll probably not remember to add it back. > + int verbosity = 1; If we skip --quiet, this can be simplified. > > prog.len = (unsigned short)sizeof(filter_pesto) / > sizeof(filter_pesto[0]); > @@ -71,15 +105,40 @@ int main(int argc, char **argv) > prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) > die("Failed to apply seccomp filter"); > > - if (argc < 2) > - die("Usage: %s CONTROLPATH", argv[0]); I was suggesting to use PATH instead of CONTROLPATH in the earlier patch, but this goes away here and we'll probably want to squash this anyway so it shouldn't matter. > + do { > + optname = getopt_long(argc, argv, optstring, options, NULL); > + > + switch (optname) { > + case -1: > + case 0: > + break; > + case 'h': > + usage(argv[0], stdout, EXIT_SUCCESS); > + break; > + case 'q': > + verbosity--; > + break; > + case 'v': > + verbosity++; > + break; > + case 1: > + FPRINTF(stdout, "pesto "); > + FPRINTF(stdout, VERSION_BLOB); > + exit(EXIT_SUCCESS); > + default: > + usage(argv[0], stderr, EXIT_FAILURE); > + } > + } while (optname != -1); > + > + if (argc - optind != 1) > + usage(argv[0], stderr, EXIT_FAILURE); > > if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) > die("Failed to create AF_UNIX socket: %s", strerror(errno)); > > - ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]); > + ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[optind]); > if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) > - die("Invalid socket path \"%s\"", argv[1]); > + die("Invalid socket path \"%s\"", argv[optind]); > > ret = connect(s, (struct sockaddr *)&a, sizeof(a)); > if (ret < 0) { > @@ -87,6 +146,8 @@ int main(int argc, char **argv) > a.sun_path, strerror(errno)); > } > > + debug("Connected to passt/pasta control socket"); > + > ret = read_all_buf(s, &hello, sizeof(hello)); > if (ret < 0) > die("Couldn't read server greeting: %s", strerror(errno)); > @@ -96,6 +157,8 @@ int main(int argc, char **argv) > > s_version = ntohl(hello.version); > > + debug("Server protocol version: %"PRIu32, s_version); > + > if (s_version > PESTO_PROTOCOL_VERSION) { > die("Unknown server protocol version %"PRIu32" > %"PRIu32"\n", > s_version, PESTO_PROTOCOL_VERSION); > diff --git a/util.h b/util.h > index e7993f4d..d7c397f6 100644 > --- a/util.h > +++ b/util.h > @@ -21,14 +21,6 @@ > > #include "log.h" > > -#define VERSION_BLOB \ > - VERSION "\n" \ > - "Copyright Red Hat\n" \ > - "GNU General Public License, version 2 or later\n" \ > - " \n" \ > - "This is free software: you are free to change and redistribute it.\n" \ > - "There is NO WARRANTY, to the extent permitted by law.\n\n" > - > #ifndef SECCOMP_RET_KILL_PROCESS > #define SECCOMP_RET_KILL_PROCESS SECCOMP_RET_KILL > #endif -- Stefano