From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202508 header.b=Vo/VC3hX; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 4A50D5A027A for ; Thu, 14 Aug 2025 06:57:49 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1755147466; bh=DVGtcKGOCZThCzNWwhRqpZS+uodDL9FheSIQMP9Wp/s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Vo/VC3hX77bNUF2lWVRLtXovFXprl6rhI2MauuW6SIgRSpbWZXyAGf3N2YtkdftyL fNCUrFapRLhy+fPDe67wz5teuKFhbNr8W4Sj4MR3V1XTMOHs8uLZyqePd890jf+gUK oP9nBosWr/tLtXR6a5wKfLAT0T4vW6oD9AerETPOmVbOEmVioAMwNTFpF2vDdaz4MP XZAWlpP6zptnQX20Br+fdXzggIs3rp8JHKHLsjyTAtxY1hivEFeNO72bhzx+QFz8IH wzSOdXCN6WkfLP26AeST3gzMlfw3AhFqnxBHCTPOUKRhL+EHvk30wHa9t+3pybwb2J qRhw+H8OTjTtA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4c2Xzt0T00z4x5q; Thu, 14 Aug 2025 14:57:46 +1000 (AEST) Date: Thu, 14 Aug 2025 14:10:20 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] treewide: Flush pcap and log files, if used, before exiting Message-ID: References: <20250813164510.3382756-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="AMfUCTGMLgzWHx2q" Content-Disposition: inline In-Reply-To: <20250813164510.3382756-1-sbrivio@redhat.com> Message-ID-Hash: YMVXUBGDGN5WKZS3JCQPAWBP3QCGBATN X-Message-ID-Hash: YMVXUBGDGN5WKZS3JCQPAWBP3QCGBATN X-MailFrom: dgibson@gandalf.ozlabs.org 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, Paul Holzinger 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: --AMfUCTGMLgzWHx2q Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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()"). >=20 > while true; do ./pasta --trace -l /tmp/pasta.log -p /tmp/pasta.pcap --c= onfig-net -t 5555 -- socat TCP-LISTEN:5555 OPEN:/tmp/large.rcv,trunc & (sle= ep 0.3; socat -T2 OPEN:large.bin TCP:88.198.0.164:5555; ); wait; diff large= =2Ebin /tmp/large.rcv || break; done >=20 > ...flush files and pcap if we're using them. Ignore fsync() errors for > the log file as we obviously can't reliably log them. >=20 > 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_: > * =E2=9C=85 all capabilities dropped, other than `CAP_NET_BIND_SERVICE` = (if granted) > * =E2=9C=85 with default options, user, mount, IPC, UTS, PID namespaces = are detached > * =E2=9C=85 no external dependencies (other than a standard C library) > -* =E2=9C=85 restrictive seccomp profiles (30 syscalls allowed for _passt= _, 41 for > +* =E2=9C=85 restrictive seccomp profiles (33 syscalls allowed for _passt= _, 43 for > _pasta_ on x86_64) > * =E2=9C=85 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 =3D -1; /* Optional socket to syst= em logger */ > static char log_ident[BUFSIZ]; /* Identifier string for openlog() */ > static int log_mask; /* Current log priority mask */ > =20 > -static int log_file =3D -1; /* Optional log file descriptor */ > +int log_file =3D -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) > =20 > +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; > =20 > + fsync_pcap_and_log(); > _exit(EXIT_SUCCESS); > } > =20 > 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 =3D=3D pasta_child_pid) { > + fsync_pcap_and_log(); > + > if (infop.si_code =3D=3D CLD_EXITED) > _exit(infop.si_status); > =20 > diff --git a/pcap.c b/pcap.c > index 46d11a2..e9d8d80 100644 > --- a/pcap.c > +++ b/pcap.c > @@ -37,7 +37,7 @@ > =20 > #define PCAP_VERSION_MINOR 4 > =20 > -static int pcap_fd =3D -1; > +int pcap_fd =3D -1; > =20 > 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 > =20 > +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" : ""); > =20 > - if (c->one_off) > + if (c->one_off) { > + fsync_pcap_and_log(); > _exit(EXIT_SUCCESS); > + } > =20 > /* 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 !=3D -1 && fsync(pcap_fd)) > + warn_perror("Failed to flush pcap file, it might be truncated"); > + > + if (log_file !=3D -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 s= kip); > 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); > =20 > /** > * af_name() - Return name of an address family --=20 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 --AMfUCTGMLgzWHx2q Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmidYYUACgkQzQJF27ox 2Ge75hAAl5ZE3i8+dG6iNH2/YGMmm/h10T0eCSUwFo+osXNAuBQPwMe1jz6NX6yy lzUGOjvEgxxDk2XC/IWFPvzxykbANHbeKhh5jX9mxOfBJQpK/pyqNN5ZSUMVlM5l zz3kcV4Xl3RnvwBgQNSytSHQTVaphZEZwS0MwOOyEmyaLha8CCWRZhIOX7bCgVCa ADbAq0s77A9l9cw1O5NrohsgHjPFz6jwOYJ8hWmZlepr03zHPFuqrDrdnF2ta2we yVYd+VhTdg+JHyF/bhsALkqyxqn5FZEUR4jaZqkf9YULYoNSW0GxVF0En3lWG/U4 hnrtD71nJ6vjW05BreHbl+isTfbuGJDnKLmn8U3Ku63zqxNZsKlUifEjljTP+tYl TMup8R0AMFZe/OEYEpfOHm8exqY6JSy/YuhKVudjFg0J1WiZ8rIg6cLyLZvs1Xz/ O9H25uWX2a7si85CwIa0ZEH/rIPVbrqcGtYVcgeJ/NvtPUSOuFBpT5qVtJ88r/1P mS6rGbcqIKVRkpPn4h+fMQvrmSfXjvnQwuaxz/ym8su7yg8zADgm3ztS1eS6MIxZ Ixzo4mlYu57WYnZAPDLjxN++7tIG/bt8AnZvofWpjcXCBS5r8bu8ov8+7/yavUAN TASqv4TAqNwDsQC0ylxlcpukVL5GTVSKdx29mEOhPXw2C0bx3s8= =kcvq -----END PGP SIGNATURE----- --AMfUCTGMLgzWHx2q--