public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH v2] treewide: Flush pcap and log files, if used, before exiting
@ 2025-08-15 16:18 Stefano Brivio
  2025-08-19  4:21 ` David Gibson
  0 siblings, 1 reply; 2+ messages in thread
From: Stefano Brivio @ 2025-08-15 16:18 UTC (permalink / raw)
  To: passt-dev; +Cc: Paul Holzinger, David Gibson

I didn't imagine that occasionally truncated pcap and log files, as a
result of commit d0006fa784a7 ("treewide: use _exit() over exit()"),
would be such a big deal, until I tried to debug TCP issues with this
beauty:

  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 <sbrivio@redhat.com>
---
v2: Fix commit reference in commit message

 README.md |  2 +-
 log.c     |  2 +-
 log.h     |  1 +
 passt.c   |  1 +
 pasta.c   |  2 ++
 pcap.c    |  2 +-
 pcap.h    |  2 ++
 tap.c     |  4 +++-
 util.c    | 15 +++++++++++++++
 util.h    |  1 +
 10 files changed, 28 insertions(+), 4 deletions(-)

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 <sys/random.h>
 #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
-- 
@@ -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
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] treewide: Flush pcap and log files, if used, before exiting
  2025-08-15 16:18 [PATCH v2] treewide: Flush pcap and log files, if used, before exiting Stefano Brivio
@ 2025-08-19  4:21 ` David Gibson
  0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2025-08-19  4:21 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, Paul Holzinger

[-- Attachment #1: Type: text/plain, Size: 5992 bytes --]

On Fri, Aug 15, 2025 at 06:18:46PM +0200, Stefano Brivio wrote:
> I didn't imagine that occasionally truncated pcap and log files, as a
> result of commit d0006fa784a7 ("treewide: use _exit() over exit()"),
> would be such a big deal, until I tried to debug TCP issues with this
> beauty:
> 
>   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 <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2: Fix commit reference in commit message
> 
>  README.md |  2 +-
>  log.c     |  2 +-
>  log.h     |  1 +
>  passt.c   |  1 +
>  pasta.c   |  2 ++
>  pcap.c    |  2 +-
>  pcap.h    |  2 ++
>  tap.c     |  4 +++-
>  util.c    | 15 +++++++++++++++
>  util.h    |  1 +
>  10 files changed, 28 insertions(+), 4 deletions(-)
> 
> 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 <sys/random.h>
>  #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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-08-19  4:22 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-15 16:18 [PATCH v2] treewide: Flush pcap and log files, if used, before exiting Stefano Brivio
2025-08-19  4:21 ` David Gibson

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).