Re: [PATCH] Add --stats option to display event statistics

On Thu, 18 Sep 2025 12:59:10 +0200 Laurent Vivier wrote:

Introduce a new --stats DELAY option that displays event statistics tables showing counts by epoll event type. Statistics are printed to stderr with a minimum delay between updates, and only when events occur.

I guess we'll need more and more of these quirks in the future, so maybe *at some point* we should think of optionally exporting those to a machine-readable format. Given that we'll probably need a small binary tool to dynamically configure port forwarding, and address/port translations, maybe we could, that day, have this also handled by... passtctl? But meanwhile:

Signed-off-by: Laurent Vivier --- conf.c | 6 ++++++ passt.1 | 6 ++++++ passt.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ passt.h | 1 + 4 files changed, 77 insertions(+)

diff --git a/conf.c b/conf.c index 02e903b5cf70..9aba27cef806 100644 --- a/conf.c +++ b/conf.c @@ -835,6 +835,8 @@ static void usage(const char *name, FILE *f, int status) "\n" " -d, --debug Be verbose\n" " --trace Be extra verbose, implies --debug\n" + " --stats DELAY Display events statistics\n" + " minimum DELAY seconds between updates\n" " -q, --quiet Don't print informational messages\n" " -f, --foreground Don't run in background\n" " default: run in background\n" @@ -1480,6 +1482,7 @@ void conf(struct ctx *c, int argc, char **argv) {"repair-path", required_argument, NULL, 28 }, {"migrate-exit", no_argument, NULL, 29 }, {"migrate-no-linger", no_argument, NULL, 30 }, + {"stats", required_argument, NULL, 31 }, { 0 }, }; const char *optstring = "+dqfel:hs:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; @@ -1708,6 +1711,9 @@ void conf(struct ctx *c, int argc, char **argv) die("--migrate-no-linger is for vhost-user mode only"); c->migrate_no_linger = true;

+ break; + case 31: + c->stats = strtol(optarg, NULL, 0);

We should make sure that we print to standard error only if it's there, and not to, say, a TCP socket that happened to be number 2. See also commit b74801645c23 ("conf, passt: Don't try to log to stderr after we close it") for how badly I broke this in the past. We could probably go the same way as that commit for c->stats as well.

break; case 'd': c->debug = 1; diff --git a/passt.1 b/passt.1 index af5726a7f1c5..d5cde2c424d0 100644 --- a/passt.1 +++ b/passt.1 @@ -84,6 +84,12 @@ Be verbose, don't log to the system logger. .BR \-\-trace Be extra verbose, show single packets. Implies \fB--debug\fR.

+.TP +.BR \-\-stats " " \fIDELAY\fR +Display events statistics with a +minimum \fIDELAY\fR seconds between updates. If there is no event, +statistics are not displayed.

Unnecessary padding/wrapping I guess? This could be two lines.

+ .TP .BR \-q ", " \-\-quiet Don't print informational messages. diff --git a/passt.c b/passt.c index a4ec115d1784..afd7f709e58a 100644 --- a/passt.c +++ b/passt.c @@ -83,6 +83,31 @@ char *epoll_type_str[] = { static_assert(ARRAY_SIZE(epoll_type_str) == EPOLL_NUM_TYPES, "epoll_type_str[] doesn't match enum epoll_type");

+char *epoll_type_tag[] = {

This could be const, and I'm a bit surprised none of the static checkers suggests that, and that's the same for epoll_type_str[], actually. But I find those names a bit hard to decode, so, instead of having this array, what about my proposal below? See comment to print_stats().

+ [EPOLL_TYPE_TCP] = "TCP", + [EPOLL_TYPE_TCP_SPLICE] = "TCPSPL", + [EPOLL_TYPE_TCP_LISTEN] = "TCPLSN", + [EPOLL_TYPE_TCP_TIMER] = "TCPTMR", + [EPOLL_TYPE_UDP_LISTEN] = "UDPLSN", + [EPOLL_TYPE_UDP] = "UDP", + [EPOLL_TYPE_PING] = "PING", + [EPOLL_TYPE_NSQUIT_INOTIFY] = "INOTF", + [EPOLL_TYPE_NSQUIT_TIMER] = "TIMER", + [EPOLL_TYPE_TAP_PASTA] = "PASTA", + [EPOLL_TYPE_TAP_PASST] = "PASST", + [EPOLL_TYPE_TAP_LISTEN] = "TAPLSN", + [EPOLL_TYPE_VHOST_CMD] = "VUCMD", + [EPOLL_TYPE_VHOST_KICK] = "VUKICK", + [EPOLL_TYPE_REPAIR_LISTEN] = "REPLSN", + [EPOLL_TYPE_REPAIR] = "REPAIR", +}; +static_assert(ARRAY_SIZE(epoll_type_tag) == EPOLL_NUM_TYPES, + "epoll_type_tag[] doesn't match enum epoll_type"); +

This should have the usual kerneldoc-style description.

+struct passt_stats { + unsigned long events[EPOLL_NUM_TYPES]; +}; + /** * post_handler() - Run periodic and deferred tasks for L4 protocol handlers * @c: Execution context @@ -174,6 +199,42 @@ static void exit_handler(int signal) _exit(EXIT_SUCCESS); }

+/** + * print_stats() - Print event statistics table to stderr + * @c: Execution context + * @stats: Event counters + * @now: Current timestamp + */ +static void print_stats(const struct ctx *c, const struct passt_stats *stats, + const struct timespec *now) +{ + int i; + static struct timespec before; + long long elapsed_ns; + + if (!c->stats) + return; + + elapsed_ns = (now->tv_sec - before.tv_sec) * 1000000000LL + + (now->tv_nsec - before.tv_nsec); + + if (elapsed_ns < c->stats * 1000000000LL) + return; + + before = *now; + + /* table header */ + FPRINTF(stderr, "\t"); + for (i = 1; i < EPOLL_NUM_TYPES; i++) + FPRINTF(stderr, "%6s\t", epoll_type_tag[i]);

Instead of being forced to mangle header fields to fit, what about having a multi-line header that's only printed every now and then, say, every slightly less than the canonical 22 rows? That is (quickly tested): --- ... static int lines_printed; long long elapsed_ns; if (!c->stats) return; elapsed_ns = (now->tv_sec - before.tv_sec) * 1E9 + (now->tv_nsec - before.tv_nsec); if (elapsed_ns < c->stats * 1E9) return; before = *now; if (!(lines_printed % 20)) { /* Table header */ for (i = 1; i < EPOLL_NUM_TYPES; i++) { int j; for (j = 0; j < i * (6 + 1); j++) { if (j && !(j % (6 + 1))) FPRINTF(stderr, "|"); else FPRINTF(stderr, " "); } FPRINTF(stderr, "%s\n", epoll_type_str[i]); } } FPRINTF(stderr, " "); for (i = 1; i < EPOLL_NUM_TYPES; i++) FPRINTF(stderr, " %6lu", stats->events[i]); FPRINTF(stderr, "\n"); lines_printed++; ... --- ?

+ FPRINTF(stderr, "\n"); + /* main loop */ + FPRINTF(stderr, "MAIN\t"); + for (i = 1; i < EPOLL_NUM_TYPES; i++) + FPRINTF(stderr, "%6lu\t", stats->events[i]); + FPRINTF(stderr, "\n"); +} + /** * main() - Entry point and main loop * @argc: Argument count @@ -196,6 +257,7 @@ int main(int argc, char **argv) struct rlimit limit; struct timespec now; struct sigaction sa; + struct passt_stats stats = { 0 };

This should go a bit above.

if (clock_gettime(CLOCK_MONOTONIC, &log_start)) die_perror("Failed to get CLOCK_MONOTONIC time"); @@ -362,6 +424,8 @@ loop: /* Can't happen */ ASSERT(0); } + stats.events[ref.type]++; + print_stats(&c, &stats, &now); }

post_handler(&c, &now); diff --git a/passt.h b/passt.h index 3ffc19fdae05..283e7c8a832c 100644 --- a/passt.h +++ b/passt.h @@ -248,6 +248,7 @@ struct ctx { enum passt_modes mode; int debug; int trace; + int stats;

And this should be documented in the struct comment.

int quiet; int foreground; int nofile;

The rest looks good and quite useful to me (I guess it's much more useful to you, of course ;)). -- Stefano