[PATCH v2 1/7] Move logging functions to a new file, log.c

[Stefano Brivio <sbrivio@redhat.com>]

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

Logging to file is going to add some further complexity that we don't
want to squeeze into util.c.

Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 Makefile     |   4 +-
 conf.c       |   1 +
 dhcp.c       |   1 +
 dhcpv6.c     |   1 +
 icmp.c       |   1 +
 isolation.c  |   1 +
 log.c        | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++
 log.h        |  26 ++++++++++
 ndp.c        |   1 +
 netlink.c    |   1 +
 packet.c     |   1 +
 passt.c      |   1 +
 pasta.c      |   1 +
 pcap.c       |   1 +
 tap.c        |   1 +
 tcp.c        |   1 +
 tcp_splice.c |   1 +
 udp.c        |   1 +
 util.c       | 121 +------------------------------------------
 util.h       |  16 ------
 20 files changed, 187 insertions(+), 138 deletions(-)
 create mode 100644 log.c
 create mode 100644 log.h

diff --git a/Makefile b/Makefile
index d4b623f..74bbfeb 100644
--- a/Makefile
+++ b/Makefile
@@ -33,8 +33,8 @@ FLAGS += -DRLIMIT_STACK_VAL=$(RLIMIT_STACK_VAL)
 FLAGS += -DARCH=\"$(TARGET_ARCH)\"
 
 PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
-	isolation.c lineread.c mld.c ndp.c netlink.c packet.c passt.c pasta.c \
-	pcap.c siphash.c tap.c tcp.c tcp_splice.c udp.c util.c
+	isolation.c lineread.c log.c mld.c ndp.c netlink.c packet.c passt.c \
+	pasta.c pcap.c siphash.c tap.c tcp.c tcp_splice.c udp.c util.c
 QRAP_SRCS = qrap.c
 SRCS = $(PASST_SRCS) $(QRAP_SRCS)
 
diff --git a/conf.c b/conf.c
index 1537dbf..ba1d8de 100644
--- a/conf.c
+++ b/conf.c
@@ -40,6 +40,7 @@
 #include "pasta.h"
 #include "lineread.h"
 #include "isolation.h"
+#include "log.h"
 
 /**
  * get_bound_ports() - Get maps of ports with bound sockets
diff --git a/dhcp.c b/dhcp.c
index 7ad1319..7f0cc0b 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -29,6 +29,7 @@
 #include "packet.h"
 #include "passt.h"
 #include "tap.h"
+#include "log.h"
 #include "dhcp.h"
 
 /**
diff --git a/dhcpv6.c b/dhcpv6.c
index 8a1d4a6..e7640ce 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -30,6 +30,7 @@
 #include "util.h"
 #include "passt.h"
 #include "tap.h"
+#include "log.h"
 
 /**
  * struct opt_hdr - DHCPv6 option header
diff --git a/icmp.c b/icmp.c
index 29170aa..f02f89f 100644
--- a/icmp.c
+++ b/icmp.c
@@ -35,6 +35,7 @@
 #include "util.h"
 #include "passt.h"
 #include "tap.h"
+#include "log.h"
 #include "icmp.h"
 
 #define ICMP_ECHO_TIMEOUT	60 /* s, timeout for ICMP socket activity */
diff --git a/isolation.c b/isolation.c
index 124dea4..b54c325 100644
--- a/isolation.c
+++ b/isolation.c
@@ -42,6 +42,7 @@
 #include "util.h"
 #include "seccomp.h"
 #include "passt.h"
+#include "log.h"
 #include "isolation.h"
 
 /**
diff --git a/log.c b/log.c
new file mode 100644
index 0000000..54a7cbb
--- /dev/null
+++ b/log.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+/* PASST - Plug A Simple Socket Transport
+ *  for qemu/UNIX domain socket mode
+ *
+ * PASTA - Pack A Subtle Tap Abstraction
+ *  for network namespace/tap device mode
+ *
+ * log.c - Logging functions
+ *
+ * Copyright (c) 2020-2022 Red Hat GmbH
+ * Author: Stefano Brivio <sbrivio(a)redhat.com>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <time.h>
+#include <syslog.h>
+#include <stdarg.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+
+#include "log.h"
+
+/* For __openlog() and __setlogmask() wrappers, and passt_vsyslog() */
+static int	log_mask;
+static int	log_sock = -1;
+static char	log_ident[BUFSIZ];
+static int	log_opt;
+static time_t	log_debug_start;
+int		log_trace;
+
+#define logfn(name, level)						\
+void name(const char *format, ...) {					\
+	struct timespec tp;						\
+	va_list args;							\
+									\
+	if (setlogmask(0) & LOG_MASK(LOG_DEBUG)) {			\
+		clock_gettime(CLOCK_REALTIME, &tp);			\
+		fprintf(stderr, "%li.%04li: ",				\
+			tp.tv_sec - log_debug_start,			\
+			tp.tv_nsec / (100L * 1000));			\
+	} else {							\
+		va_start(args, format);					\
+		passt_vsyslog(level, format, args);			\
+		va_end(args);						\
+	}								\
+									\
+	if (setlogmask(0) & LOG_MASK(LOG_DEBUG) ||			\
+	    setlogmask(0) == LOG_MASK(LOG_EMERG)) {			\
+		va_start(args, format);					\
+		(void)vfprintf(stderr, format, args); 			\
+		va_end(args);						\
+		if (format[strlen(format)] != '\n')			\
+			fprintf(stderr, "\n");				\
+	}								\
+}
+
+logfn(err,   LOG_ERR)
+logfn(warn,  LOG_WARNING)
+logfn(info,  LOG_INFO)
+logfn(debug, LOG_DEBUG)
+
+void trace_init(int enable)
+{
+	log_trace = enable;
+}
+
+/**
+ * __openlog() - Non-optional openlog() wrapper, to allow custom vsyslog()
+ * @ident:	openlog() identity (program name)
+ * @option:	openlog() options
+ * @facility:	openlog() facility (LOG_DAEMON)
+ */
+void __openlog(const char *ident, int option, int facility)
+{
+	struct timespec tp;
+
+	clock_gettime(CLOCK_REALTIME, &tp);
+	log_debug_start = tp.tv_sec;
+
+	if (log_sock < 0) {
+		struct sockaddr_un a = { .sun_family = AF_UNIX, };
+
+		log_sock = socket(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+		if (log_sock < 0)
+			return;
+
+		strncpy(a.sun_path, _PATH_LOG, sizeof(a.sun_path));
+		if (connect(log_sock, (const struct sockaddr *)&a, sizeof(a))) {
+			close(log_sock);
+			log_sock = -1;
+			return;
+		}
+	}
+
+	log_mask |= facility;
+	strncpy(log_ident, ident, sizeof(log_ident) - 1);
+	log_opt = option;
+
+	openlog(ident, option, facility);
+}
+
+/**
+ * __setlogmask() - setlogmask() wrapper, to allow custom vsyslog()
+ * @mask:	Same as setlogmask() mask
+ */
+void __setlogmask(int mask)
+{
+	log_mask = mask;
+	setlogmask(mask);
+}
+
+/**
+ * passt_vsyslog() - vsyslog() implementation not using heap memory
+ * @pri:	Facility and level map, same as priority for vsyslog()
+ * @format:	Same as vsyslog() format
+ * @ap:		Same as vsyslog() ap
+ */
+void passt_vsyslog(int pri, const char *format, va_list ap)
+{
+	char buf[BUFSIZ];
+	int n;
+
+	if (!(LOG_MASK(LOG_PRI(pri)) & log_mask))
+		return;
+
+	/* Send without name and timestamp, the system logger should add them */
+	n = snprintf(buf, BUFSIZ, "<%i> ", pri);
+
+	n += vsnprintf(buf + n, BUFSIZ - n, format, ap);
+
+	if (format[strlen(format)] != '\n')
+		n += snprintf(buf + n, BUFSIZ - n, "\n");
+
+	if (log_opt & LOG_PERROR)
+		fprintf(stderr, "%s", buf + sizeof("<0>"));
+
+	if (send(log_sock, buf, n, 0) != n)
+		fprintf(stderr, "Failed to send %i bytes to syslog\n", n);
+}
diff --git a/log.h b/log.h
new file mode 100644
index 0000000..70cad6f
--- /dev/null
+++ b/log.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: AGPL-3.0-or-later
+ * Copyright (c) 2022 Red Hat GmbH
+ * Author: Stefano Brivio <sbrivio(a)redhat.com>
+ */
+
+#ifndef LOG_H
+#define LOG_H
+
+void err(const char *format, ...);
+void warn(const char *format, ...);
+void info(const char *format, ...);
+void debug(const char *format, ...);
+
+extern int log_trace;
+void trace_init(int enable);
+#define trace(format, ...)						\
+	do {								\
+		if (log_trace)						\
+			debug(format, ##__VA_ARGS__);			\
+	} while (0)
+
+void __openlog(const char *ident, int option, int facility);
+void passt_vsyslog(int pri, const char *format, va_list ap);
+void __setlogmask(int mask);
+
+#endif /* LOG_H */
diff --git a/ndp.c b/ndp.c
index 29c4b14..dec36a9 100644
--- a/ndp.c
+++ b/ndp.c
@@ -30,6 +30,7 @@
 #include "util.h"
 #include "passt.h"
 #include "tap.h"
+#include "log.h"
 
 #define RS	133
 #define RA	134
diff --git a/netlink.c b/netlink.c
index 15ce213..9719e91 100644
--- a/netlink.c
+++ b/netlink.c
@@ -30,6 +30,7 @@
 
 #include "util.h"
 #include "passt.h"
+#include "log.h"
 #include "netlink.h"
 
 /* Socket in init, in target namespace, sequence (just needs to be monotonic) */
diff --git a/packet.c b/packet.c
index 6d10ec1..3f82e84 100644
--- a/packet.c
+++ b/packet.c
@@ -20,6 +20,7 @@
 
 #include "packet.h"
 #include "util.h"
+#include "log.h"
 
 /**
  * packet_add_do() - Add data as packet descriptor to given pool
diff --git a/passt.c b/passt.c
index 2c4a986..0c561b2 100644
--- a/passt.c
+++ b/passt.c
@@ -44,6 +44,7 @@
 #include "conf.h"
 #include "pasta.h"
 #include "arch.h"
+#include "log.h"
 
 #define EPOLL_EVENTS		8
 
diff --git a/pasta.c b/pasta.c
index 1dd8267..1887dd2 100644
--- a/pasta.c
+++ b/pasta.c
@@ -44,6 +44,7 @@
 #include "passt.h"
 #include "isolation.h"
 #include "netlink.h"
+#include "log.h"
 
 /* PID of child, in case we created a namespace */
 static int pasta_child_pid;
diff --git a/pcap.c b/pcap.c
index 7a48baf..836688d 100644
--- a/pcap.c
+++ b/pcap.c
@@ -30,6 +30,7 @@
 
 #include "util.h"
 #include "passt.h"
+#include "log.h"
 
 #define PCAP_VERSION_MINOR 4
 
diff --git a/tap.c b/tap.c
index 5540c18..bdcc161 100644
--- a/tap.c
+++ b/tap.c
@@ -52,6 +52,7 @@
 #include "netlink.h"
 #include "pasta.h"
 #include "packet.h"
+#include "log.h"
 
 /* IPv4 (plus ARP) and IPv6 message batches from tap/guest to IP handlers */
 static PACKET_POOL_NOINIT(pool_tap4, TAP_MSGS, pkt_buf);
diff --git a/tcp.c b/tcp.c
index 8ec991c..830dc88 100644
--- a/tcp.c
+++ b/tcp.c
@@ -298,6 +298,7 @@
 #include "pcap.h"
 #include "conf.h"
 #include "tcp_splice.h"
+#include "log.h"
 
 #define TCP_FRAMES_MEM			128
 #define TCP_FRAMES							\
diff --git a/tcp_splice.c b/tcp_splice.c
index edbcfd4..4a015d0 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -47,6 +47,7 @@
 
 #include "util.h"
 #include "passt.h"
+#include "log.h"
 
 #define MAX_PIPE_SIZE			(8UL * 1024 * 1024)
 #define TCP_SPLICE_MAX_CONNS		(128 * 1024)
diff --git a/udp.c b/udp.c
index d1be622..5422fdd 100644
--- a/udp.c
+++ b/udp.c
@@ -116,6 +116,7 @@
 #include "passt.h"
 #include "tap.h"
 #include "pcap.h"
+#include "log.h"
 
 #define UDP_CONN_TIMEOUT	180 /* s, timeout for ephemeral or local bind */
 #define UDP_SPLICE_FRAMES	32
diff --git a/util.c b/util.c
index 6b86ead..b366167 100644
--- a/util.c
+++ b/util.c
@@ -19,8 +19,6 @@
 #include <net/ethernet.h>
 #include <sys/epoll.h>
 #include <fcntl.h>
-#include <syslog.h>
-#include <stdarg.h>
 #include <string.h>
 #include <time.h>
 #include <errno.h>
@@ -29,124 +27,7 @@
 #include "passt.h"
 #include "packet.h"
 #include "lineread.h"
-
-/* For __openlog() and __setlogmask() wrappers, and passt_vsyslog() */
-static int	log_mask;
-static int	log_sock = -1;
-static char	log_ident[BUFSIZ];
-static int	log_opt;
-static time_t	log_debug_start;
-int		log_trace;
-
-#define logfn(name, level)						\
-void name(const char *format, ...) {					\
-	struct timespec tp;						\
-	va_list args;							\
-									\
-	if (setlogmask(0) & LOG_MASK(LOG_DEBUG)) {			\
-		clock_gettime(CLOCK_REALTIME, &tp);			\
-		fprintf(stderr, "%li.%04li: ",				\
-			tp.tv_sec - log_debug_start,			\
-			tp.tv_nsec / (100L * 1000));			\
-	} else {							\
-		va_start(args, format);					\
-		passt_vsyslog(level, format, args);			\
-		va_end(args);						\
-	}								\
-									\
-	if (setlogmask(0) & LOG_MASK(LOG_DEBUG) ||			\
-	    setlogmask(0) == LOG_MASK(LOG_EMERG)) {			\
-		va_start(args, format);					\
-		(void)vfprintf(stderr, format, args); 			\
-		va_end(args);						\
-		if (format[strlen(format)] != '\n')			\
-			fprintf(stderr, "\n");				\
-	}								\
-}
-
-logfn(err,   LOG_ERR)
-logfn(warn,  LOG_WARNING)
-logfn(info,  LOG_INFO)
-logfn(debug, LOG_DEBUG)
-
-void trace_init(int enable)
-{
-	log_trace = enable;
-}
-
-/**
- * __openlog() - Non-optional openlog() wrapper, to allow custom vsyslog()
- * @ident:	openlog() identity (program name)
- * @option:	openlog() options
- * @facility:	openlog() facility (LOG_DAEMON)
- */
-void __openlog(const char *ident, int option, int facility)
-{
-	struct timespec tp;
-
-	clock_gettime(CLOCK_REALTIME, &tp);
-	log_debug_start = tp.tv_sec;
-
-	if (log_sock < 0) {
-		struct sockaddr_un a = { .sun_family = AF_UNIX, };
-
-		log_sock = socket(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0);
-		if (log_sock < 0)
-			return;
-
-		strncpy(a.sun_path, _PATH_LOG, sizeof(a.sun_path));
-		if (connect(log_sock, (const struct sockaddr *)&a, sizeof(a))) {
-			close(log_sock);
-			log_sock = -1;
-			return;
-		}
-	}
-
-	log_mask |= facility;
-	strncpy(log_ident, ident, sizeof(log_ident) - 1);
-	log_opt = option;
-
-	openlog(ident, option, facility);
-}
-
-/**
- * __setlogmask() - setlogmask() wrapper, to allow custom vsyslog()
- * @mask:	Same as setlogmask() mask
- */
-void __setlogmask(int mask)
-{
-	log_mask = mask;
-	setlogmask(mask);
-}
-
-/**
- * passt_vsyslog() - vsyslog() implementation not using heap memory
- * @pri:	Facility and level map, same as priority for vsyslog()
- * @format:	Same as vsyslog() format
- * @ap:		Same as vsyslog() ap
- */
-void passt_vsyslog(int pri, const char *format, va_list ap)
-{
-	char buf[BUFSIZ];
-	int n;
-
-	if (!(LOG_MASK(LOG_PRI(pri)) & log_mask))
-		return;
-
-	/* Send without name and timestamp, the system logger should add them */
-	n = snprintf(buf, BUFSIZ, "<%i> ", pri);
-
-	n += vsnprintf(buf + n, BUFSIZ - n, format, ap);
-
-	if (format[strlen(format)] != '\n')
-		n += snprintf(buf + n, BUFSIZ - n, "\n");
-
-	if (log_opt & LOG_PERROR)
-		fprintf(stderr, "%s", buf + sizeof("<0>"));
-
-	if (send(log_sock, buf, n, 0) != n)
-		fprintf(stderr, "Failed to send %i bytes to syslog\n", n);
-}
+#include "log.h"
 
 #define IPV6_NH_OPT(nh)							\
 	((nh) == 0   || (nh) == 43  || (nh) == 44  || (nh) == 50  ||	\
diff --git a/util.h b/util.h
index 0c06e34..1adbf04 100644
--- a/util.h
+++ b/util.h
@@ -6,19 +6,6 @@
 #ifndef UTIL_H
 #define UTIL_H
 
-void err(const char *format, ...);
-void warn(const char *format, ...);
-void info(const char *format, ...);
-void debug(const char *format, ...);
-
-extern int log_trace;
-void trace_init(int enable);
-#define trace(format, ...)						\
-	do {								\
-		if (log_trace)						\
-			debug(format, ##__VA_ARGS__);			\
-	} while (0)
-
 #ifndef SECCOMP_RET_KILL_PROCESS
 #define SECCOMP_RET_KILL_PROCESS	SECCOMP_RET_KILL
 #endif
@@ -196,9 +183,6 @@ struct ipv6_opt_hdr {
 
 /* cppcheck-suppress funcArgNamesDifferent */
 __attribute__ ((weak)) int ffsl(long int i) { return __builtin_ffsl(i); }
-void __openlog(const char *ident, int option, int facility);
-void passt_vsyslog(int pri, const char *format, va_list ap);
-void __setlogmask(int mask);
 char *ipv6_l4hdr(const struct pool *p, int index, size_t offset, uint8_t *proto,
 		 size_t *dlen);
 int sock_l4(const struct ctx *c, int af, uint8_t proto,
-- 
@@ -6,19 +6,6 @@
 #ifndef UTIL_H
 #define UTIL_H
 
-void err(const char *format, ...);
-void warn(const char *format, ...);
-void info(const char *format, ...);
-void debug(const char *format, ...);
-
-extern int log_trace;
-void trace_init(int enable);
-#define trace(format, ...)						\
-	do {								\
-		if (log_trace)						\
-			debug(format, ##__VA_ARGS__);			\
-	} while (0)
-
 #ifndef SECCOMP_RET_KILL_PROCESS
 #define SECCOMP_RET_KILL_PROCESS	SECCOMP_RET_KILL
 #endif
@@ -196,9 +183,6 @@ struct ipv6_opt_hdr {
 
 /* cppcheck-suppress funcArgNamesDifferent */
 __attribute__ ((weak)) int ffsl(long int i) { return __builtin_ffsl(i); }
-void __openlog(const char *ident, int option, int facility);
-void passt_vsyslog(int pri, const char *format, va_list ap);
-void __setlogmask(int mask);
 char *ipv6_l4hdr(const struct pool *p, int index, size_t offset, uint8_t *proto,
 		 size_t *dlen);
 int sock_l4(const struct ctx *c, int af, uint8_t proto,
-- 
2.35.1