On Wed, Aug 13, 2025 at 06:45:05PM +0200, Stefano Brivio wrote:
I didn't imagine that occasionally truncated pcap and log files, as a result of commit a9d63f91a59a ("passt-repair: use _exit() over return"), would be such a big deal, until I tried to debug TCP issues with this beauty:
I think the problems are more introduced by the previous patch d0006fa784a7 ("treewide: use _exit() over exit()").
while true; do ./pasta --trace -l /tmp/pasta.log -p /tmp/pasta.pcap --config-net -t 5555 -- socat TCP-LISTEN:5555 OPEN:/tmp/large.rcv,trunc & (sleep 0.3; socat -T2 OPEN:large.bin TCP:88.198.0.164:5555; ); wait; diff large.bin /tmp/large.rcv || break; done
...flush files and pcap if we're using them. Ignore fsync() errors for the log file as we obviously can't reliably log them.
Signed-off-by: Stefano Brivio
Hmmmmm. I mean, yes, AFAICT this patch will address the immediate problem. But between this and 081df67d1fb2 ("conf: flush stdout before early exit") it seems more and more to me that _exit() just isn't what we want. Basically the assertion in d0006fa784a7 that "no exit handlers are needed" doesn't really seem to be true. Here we're adding a new syscall to work around the problems with _exit(). In which case, why don't we add futex() to the syscall list and go back to exit(3). With Laurent working on multi-threading we might well want futexes anyhow.
diff --git a/README.md b/README.md index 54fed07..8f188f4 100644 --- a/README.md +++ b/README.md @@ -291,7 +291,7 @@ speeding up local connections, and usually requiring NAT. _pasta_: * ✅ all capabilities dropped, other than `CAP_NET_BIND_SERVICE` (if granted) * ✅ with default options, user, mount, IPC, UTS, PID namespaces are detached * ✅ no external dependencies (other than a standard C library) -* ✅ restrictive seccomp profiles (30 syscalls allowed for _passt_, 41 for +* ✅ restrictive seccomp profiles (33 syscalls allowed for _passt_, 43 for _pasta_ on x86_64) * ✅ examples of [AppArmor](/passt/tree/contrib/apparmor) and [SELinux](/passt/tree/contrib/selinux) profiles available diff --git a/log.c b/log.c index 33b89fc..21e3673 100644 --- a/log.c +++ b/log.c @@ -35,7 +35,7 @@ static int log_sock = -1; /* Optional socket to system logger */ static char log_ident[BUFSIZ]; /* Identifier string for openlog() */ static int log_mask; /* Current log priority mask */
-static int log_file = -1; /* Optional log file descriptor */ +int log_file = -1; /* Optional log file descriptor */ static size_t log_size; /* Maximum log file size in bytes */ static size_t log_written; /* Currently used bytes in log file */ static size_t log_cut_size; /* Bytes to cut at start on rotation */ diff --git a/log.h b/log.h index 08aa88c..c8473c0 100644 --- a/log.h +++ b/log.h @@ -41,6 +41,7 @@ void logmsg_perror(int pri, const char *format, ...) _exit(EXIT_FAILURE); \ } while (0)
+extern int log_file; extern int log_trace; extern bool log_conf_parsed; extern bool log_stderr; diff --git a/passt.c b/passt.c index 388d10f..a4ec115 100644 --- a/passt.c +++ b/passt.c @@ -170,6 +170,7 @@ static void exit_handler(int signal) { (void)signal;
+ fsync_pcap_and_log(); _exit(EXIT_SUCCESS); }
diff --git a/pasta.c b/pasta.c index c207692..687406b 100644 --- a/pasta.c +++ b/pasta.c @@ -70,6 +70,8 @@ void pasta_child_handler(int signal) if (pasta_child_pid && !waitid(P_PID, pasta_child_pid, &infop, WEXITED | WNOHANG)) { if (infop.si_pid == pasta_child_pid) { + fsync_pcap_and_log(); + if (infop.si_code == CLD_EXITED) _exit(infop.si_status);
diff --git a/pcap.c b/pcap.c index 46d11a2..e9d8d80 100644 --- a/pcap.c +++ b/pcap.c @@ -37,7 +37,7 @@
#define PCAP_VERSION_MINOR 4
-static int pcap_fd = -1; +int pcap_fd = -1;
struct pcap_pkthdr { uint32_t tv_sec; diff --git a/pcap.h b/pcap.h index 9795f2e..2aeb53e 100644 --- a/pcap.h +++ b/pcap.h @@ -6,6 +6,8 @@ #ifndef PCAP_H #define PCAP_H
+extern int pcap_fd; + void pcap(const char *pkt, size_t l2len); void pcap_multiple(const struct iovec *iov, size_t frame_parts, unsigned int n, size_t offset); diff --git a/tap.c b/tap.c index 6db5d88..cddc164 100644 --- a/tap.c +++ b/tap.c @@ -1117,8 +1117,10 @@ void tap_sock_reset(struct ctx *c) { info("Client connection closed%s", c->one_off ? ", exiting" : "");
- if (c->one_off) + if (c->one_off) { + fsync_pcap_and_log(); _exit(EXIT_SUCCESS); + }
/* Close the connected socket, wait for a new connection */ epoll_del(c, c->fd_tap); diff --git a/util.c b/util.c index 3e93d47..c492f90 100644 --- a/util.c +++ b/util.c @@ -34,6 +34,7 @@ #include "passt.h" #include "packet.h" #include "log.h" +#include "pcap.h" #ifdef HAS_GETRANDOM #include
#endif @@ -1045,3 +1046,17 @@ void abort_with_msg(const char *fmt, ...) */ abort(); } + +/** + * fsync_pcap_and_log() - Flush pcap and log files as needed + * + * #syscalls fsync + */ +void fsync_pcap_and_log(void) +{ + if (pcap_fd != -1 && fsync(pcap_fd)) + warn_perror("Failed to flush pcap file, it might be truncated"); + + if (log_file != -1) + (void)fsync(log_file); +} diff --git a/util.h b/util.h index b5b1b31..2a8c38f 100644 --- a/util.h +++ b/util.h @@ -226,6 +226,7 @@ int read_all_buf(int fd, void *buf, size_t len); int read_remainder(int fd, const struct iovec *iov, size_t cnt, size_t skip); void close_open_files(int argc, char **argv); bool snprintf_check(char *str, size_t size, const char *format, ...); +void fsync_pcap_and_log(void); /** * af_name() - Return name of an address family
-- 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