* [PATCH v3 1/7] Move logging functions to a new file, log.c
2022-10-10 8:35 [PATCH v3 0/7] Add support for log file and version display Stefano Brivio
@ 2022-10-10 8:35 ` Stefano Brivio
2022-10-10 8:35 ` [PATCH v3 2/7] conf: Drop duplicate, diverging optstring assignments Stefano Brivio
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Stefano Brivio @ 2022-10-10 8:35 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson
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@redhat.com>
Reviewed-by: David Gibson <david@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@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@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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 2/7] conf: Drop duplicate, diverging optstring assignments
2022-10-10 8:35 [PATCH v3 0/7] Add support for log file and version display Stefano Brivio
2022-10-10 8:35 ` [PATCH v3 1/7] Move logging functions to a new file, log.c Stefano Brivio
@ 2022-10-10 8:35 ` Stefano Brivio
2022-10-10 8:35 ` [PATCH v3 3/7] passt.h: Include netinet/if_ether.h before struct ctx declaration Stefano Brivio
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Stefano Brivio @ 2022-10-10 8:35 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson
This originated as a result of copy and paste to introduce a second
stage for processing options related to port forwarding, has already
bitten David in the past, and just gave me hours of fun.
As a matter of fact, the second set of optstring assignments was
already incorrect, but it didn't matter because the first one was
more restrictive, not allowing optional arguments for -P, -D, -S.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/conf.c b/conf.c
index ba1d8de..e9cd4a7 100644
--- a/conf.c
+++ b/conf.c
@@ -1043,25 +1043,23 @@ void conf(struct ctx *c, int argc, char **argv)
struct fqdn *dnss = c->dns_search;
uint32_t *dns4 = c->ip4.dns;
int name, ret, mask, b, i;
+ const char *optstring;
unsigned int ifi = 0;
char *runas = NULL;
uid_t uid;
gid_t gid;
- if (c->mode == MODE_PASTA)
+ if (c->mode == MODE_PASTA) {
c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
+ optstring = "dqfehI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
+ } else {
+ optstring = "dqfehs:p:P:m:a:n:M:g:i:D:S:46t:u:";
+ }
c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0;
c->udp.fwd_in.f.mode = c->udp.fwd_out.f.mode = 0;
do {
- const char *optstring;
-
- if (c->mode == MODE_PASST)
- optstring = "dqfehs:p:P:m:a:n:M:g:i:D:S:46t:u:";
- else
- optstring = "dqfehI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
-
name = getopt_long(argc, argv, optstring, options, NULL);
switch (name) {
@@ -1505,12 +1503,6 @@ void conf(struct ctx *c, int argc, char **argv)
optind = 1;
do {
struct port_fwd *fwd = NULL;
- const char *optstring;
-
- if (c->mode == MODE_PASST)
- optstring = "dqfehs:p::P:m:a:n:M:g:i:D::S::46t:u:";
- else
- optstring = "dqfehI:p::P:m:a:n:M:g:i:D::S::46t:u:T:U:";
name = getopt_long(argc, argv, optstring, options, NULL);
switch (name) {
--
@@ -1043,25 +1043,23 @@ void conf(struct ctx *c, int argc, char **argv)
struct fqdn *dnss = c->dns_search;
uint32_t *dns4 = c->ip4.dns;
int name, ret, mask, b, i;
+ const char *optstring;
unsigned int ifi = 0;
char *runas = NULL;
uid_t uid;
gid_t gid;
- if (c->mode == MODE_PASTA)
+ if (c->mode == MODE_PASTA) {
c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
+ optstring = "dqfehI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
+ } else {
+ optstring = "dqfehs:p:P:m:a:n:M:g:i:D:S:46t:u:";
+ }
c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0;
c->udp.fwd_in.f.mode = c->udp.fwd_out.f.mode = 0;
do {
- const char *optstring;
-
- if (c->mode == MODE_PASST)
- optstring = "dqfehs:p:P:m:a:n:M:g:i:D:S:46t:u:";
- else
- optstring = "dqfehI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
-
name = getopt_long(argc, argv, optstring, options, NULL);
switch (name) {
@@ -1505,12 +1503,6 @@ void conf(struct ctx *c, int argc, char **argv)
optind = 1;
do {
struct port_fwd *fwd = NULL;
- const char *optstring;
-
- if (c->mode == MODE_PASST)
- optstring = "dqfehs:p::P:m:a:n:M:g:i:D::S::46t:u:";
- else
- optstring = "dqfehI:p::P:m:a:n:M:g:i:D::S::46t:u:T:U:";
name = getopt_long(argc, argv, optstring, options, NULL);
switch (name) {
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 3/7] passt.h: Include netinet/if_ether.h before struct ctx declaration
2022-10-10 8:35 [PATCH v3 0/7] Add support for log file and version display Stefano Brivio
2022-10-10 8:35 ` [PATCH v3 1/7] Move logging functions to a new file, log.c Stefano Brivio
2022-10-10 8:35 ` [PATCH v3 2/7] conf: Drop duplicate, diverging optstring assignments Stefano Brivio
@ 2022-10-10 8:35 ` Stefano Brivio
2022-10-10 8:35 ` [PATCH v3 4/7] log, conf: Add support for logging to file Stefano Brivio
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Stefano Brivio @ 2022-10-10 8:35 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson
This saves some hassle when including passt.h, as we need ETH_ALEN
there.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
passt.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/passt.h b/passt.h
index 48e1096..de79e7b 100644
--- a/passt.h
+++ b/passt.h
@@ -133,6 +133,8 @@ struct ip6_ctx {
struct in6_addr dns_fwd;
};
+#include <netinet/if_ether.h>
+
/**
* struct ctx - Execution context
* @mode: Operation mode, qemu/UNIX domain socket or namespace/tap
--
@@ -133,6 +133,8 @@ struct ip6_ctx {
struct in6_addr dns_fwd;
};
+#include <netinet/if_ether.h>
+
/**
* struct ctx - Execution context
* @mode: Operation mode, qemu/UNIX domain socket or namespace/tap
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 4/7] log, conf: Add support for logging to file
2022-10-10 8:35 [PATCH v3 0/7] Add support for log file and version display Stefano Brivio
` (2 preceding siblings ...)
2022-10-10 8:35 ` [PATCH v3 3/7] passt.h: Include netinet/if_ether.h before struct ctx declaration Stefano Brivio
@ 2022-10-10 8:35 ` Stefano Brivio
2022-10-11 0:11 ` David Gibson
2022-10-10 8:35 ` [PATCH v3 5/7] log: Add missing function comment for trace_init() Stefano Brivio
` (2 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Stefano Brivio @ 2022-10-10 8:35 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson
In some environments, such as KubeVirt pods, we might not have a
system logger available. We could choose to run in foreground, but
this takes away the convenient synchronisation mechanism derived from
forking to background when interfaces are ready.
Add optional logging to file with -l/--log-file and --log-size.
Unfortunately, this means we need to duplicate features that are more
appropriately implemented by a system logger, such as rotation. Keep
that reasonably simple, by using fallocate() with range collapsing
where supported (Linux kernel >= 3.15, extent-based ext4 and XFS) and
falling back to an unsophisticated block-by-block moving of entries
toward the beginning of the file once we reach the (mandatory) size
limit.
While at it, clarify the role of LOG_EMERG in passt.c.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
v3:
- don't use FALLOC_FL_COLLAPSE_RANGE if not defined in headers (e.g.
glibc < 2.18), and add fallocate() to syscall list optionally using
EXTRA_SYSCALLS in Makefile, depending on availability of that flag
- reported by clang-tidy: perform multiplication for
LOGFILE_SIZE_DEFAULT as unsigned long
v2:
- Make truncation on different failure modes in logfile_rotate_move()
consistent (David Gibson)
- Actually define default maximum log size as 1 MiB, not 1 MB (David
Gibson)
Makefile | 15 ++--
README.md | 2 +-
conf.c | 49 ++++++++++-
log.c | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++----
log.h | 6 ++
passt.1 | 14 ++-
passt.c | 1 +
7 files changed, 316 insertions(+), 29 deletions(-)
diff --git a/Makefile b/Makefile
index 74bbfeb..e4c64fe 100644
--- a/Makefile
+++ b/Makefile
@@ -41,7 +41,7 @@ SRCS = $(PASST_SRCS) $(QRAP_SRCS)
MANPAGES = passt.1 pasta.1 qrap.1
PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
- isolation.h lineread.h ndp.h netlink.h packet.h passt.h pasta.h \
+ isolation.h lineread.h log.h ndp.h netlink.h packet.h passt.h pasta.h \
pcap.h port_fwd.h siphash.h tap.h tcp.h tcp_splice.h udp.h util.h
HEADERS = $(PASST_HEADERS) seccomp.h
@@ -90,6 +90,11 @@ ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; ec
FLAGS += -fstack-protector-strong
endif
+C := \#define _GNU_SOURCE\n\#include <fcntl.h>\nx = FALLOC_FL_COLLAPSE_RANGE;
+ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
+ EXTRA_SYSCALLS += fallocate
+endif
+
prefix ?= /usr/local
exec_prefix ?= $(prefix)
bindir ?= $(exec_prefix)/bin
@@ -110,7 +115,7 @@ static: FLAGS += -static -DGLIBC_NO_STATIC_NSS
static: clean all
seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
- @ EXTRA_SYSCALLS=$(EXTRA_SYSCALLS) ./seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
+ @ EXTRA_SYSCALLS="$(EXTRA_SYSCALLS)" ./seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
passt: $(PASST_SRCS) $(HEADERS)
$(CC) $(FLAGS) $(CFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)
@@ -128,9 +133,9 @@ pasta.avx2 pasta.1 pasta: pasta%: passt%
qrap: $(QRAP_SRCS) passt.h
$(CC) $(FLAGS) $(CFLAGS) $(QRAP_SRCS) -o qrap $(LDFLAGS)
-valgrind: EXTRA_SYSCALLS="rt_sigprocmask rt_sigtimedwait rt_sigaction \
- getpid gettid kill clock_gettime mmap munmap open \
- unlink gettimeofday futex"
+valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \
+ getpid gettid kill clock_gettime mmap \
+ munmap open unlink gettimeofday futex
valgrind: FLAGS:=-g -O0 $(filter-out -O%,$(FLAGS))
valgrind: all
diff --git a/README.md b/README.md
index 2c522b2..3dc4fc5 100644
--- a/README.md
+++ b/README.md
@@ -289,7 +289,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 (26 syscalls allowed for _passt_, 40 for
+* ✅ restrictive seccomp profiles (30 syscalls allowed for _passt_, 41 for
_pasta_ on x86_64)
* ✅ examples of [AppArmor](/passt/tree/contrib/apparmor) and
[SELinux](/passt/tree/contrib/selinux) profiles available
diff --git a/conf.c b/conf.c
index e9cd4a7..f22940b 100644
--- a/conf.c
+++ b/conf.c
@@ -633,6 +633,9 @@ static void usage(const char *name)
info( " default: run in background if started from a TTY");
info( " -e, --stderr Log to stderr too");
info( " default: log to system logger only if started from a TTY");
+ info( " -l, --log-file PATH Log (only) to given file");
+ info( " --log-size BYTES Maximum size of log file");
+ info( " default: 1 MiB");
info( " --runas UID|UID:GID Run as given UID, GID, which can be");
info( " numeric, or login and group names");
info( " default: drop to user \"nobody\"");
@@ -994,6 +997,7 @@ void conf(struct ctx *c, int argc, char **argv)
{"quiet", no_argument, NULL, 'q' },
{"foreground", no_argument, NULL, 'f' },
{"stderr", no_argument, NULL, 'e' },
+ {"log-file", required_argument, NULL, 'l' },
{"help", no_argument, NULL, 'h' },
{"socket", required_argument, NULL, 's' },
{"ns-ifname", required_argument, NULL, 'I' },
@@ -1034,26 +1038,28 @@ void conf(struct ctx *c, int argc, char **argv)
{"no-netns-quit", no_argument, NULL, 10 },
{"trace", no_argument, NULL, 11 },
{"runas", required_argument, NULL, 12 },
+ {"log-size", required_argument, NULL, 13 },
{ 0 },
};
struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
bool v4_only = false, v6_only = false;
+ char *runas = NULL, *logfile = NULL;
struct in6_addr *dns6 = c->ip6.dns;
struct fqdn *dnss = c->dns_search;
uint32_t *dns4 = c->ip4.dns;
int name, ret, mask, b, i;
const char *optstring;
unsigned int ifi = 0;
- char *runas = NULL;
+ size_t logsize = 0;
uid_t uid;
gid_t gid;
if (c->mode == MODE_PASTA) {
c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
- optstring = "dqfehI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
+ optstring = "dqfel:hI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
} else {
- optstring = "dqfehs:p:P:m:a:n:M:g:i:D:S:46t:u:";
+ optstring = "dqfel:hs:p:P:m:a:n:M:g:i:D:S:46t:u:";
}
c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0;
@@ -1177,6 +1183,20 @@ void conf(struct ctx *c, int argc, char **argv)
runas = optarg;
break;
+ case 13:
+ if (logsize) {
+ err("Multiple --log-size options given");
+ usage(argv[0]);
+ }
+
+ errno = 0;
+ logsize = strtol(optarg, NULL, 0);
+
+ if (logsize < LOGFILE_SIZE_MIN || errno) {
+ err("Invalid --log-size: %s", optarg);
+ usage(argv[0]);
+ }
+ break;
case 'd':
if (c->debug) {
err("Multiple --debug options given");
@@ -1192,6 +1212,11 @@ void conf(struct ctx *c, int argc, char **argv)
c->foreground = 1;
break;
case 'e':
+ if (logfile) {
+ err("Can't log to both file and stderr");
+ usage(argv[0]);
+ }
+
if (c->stderr) {
err("Multiple --stderr options given");
usage(argv[0]);
@@ -1199,6 +1224,19 @@ void conf(struct ctx *c, int argc, char **argv)
c->stderr = 1;
break;
+ case 'l':
+ if (c->stderr) {
+ err("Can't log to both stderr and file");
+ usage(argv[0]);
+ }
+
+ if (logfile) {
+ err("Multiple --log-file options given");
+ usage(argv[0]);
+ }
+
+ logfile = optarg;
+ break;
case 'q':
if (c->quiet) {
err("Multiple --quiet options given");
@@ -1460,6 +1498,11 @@ void conf(struct ctx *c, int argc, char **argv)
if (ret)
usage(argv[0]);
+ if (logfile) {
+ logfile_init(c->mode == MODE_PASST ? "passt" : "pasta",
+ logfile, logsize);
+ }
+
if (c->mode == MODE_PASTA) {
if (conf_pasta_ns(&netns_only, userns, netns,
optind, argc, argv) < 0)
diff --git a/log.c b/log.c
index 54a7cbb..d769a33 100644
--- a/log.c
+++ b/log.c
@@ -12,7 +12,12 @@
* Author: Stefano Brivio <sbrivio@redhat.com>
*/
+#include <arpa/inet.h>
+#include <limits.h>
+#include <errno.h>
+#include <fcntl.h>
#include <stdio.h>
+#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
@@ -20,36 +25,49 @@
#include <syslog.h>
#include <stdarg.h>
#include <sys/socket.h>
-#include <sys/un.h>
#include "log.h"
+#include "util.h"
+#include "passt.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;
+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_opt; /* Options for openlog() */
+
+static 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 */
+static char log_header[BUFSIZ]; /* File header, written back on cuts */
+
+static time_t log_start; /* Start timestamp */
+int log_trace; /* --trace mode enabled */
#define logfn(name, level) \
void name(const char *format, ...) { \
struct timespec tp; \
va_list args; \
\
- if (setlogmask(0) & LOG_MASK(LOG_DEBUG)) { \
+ if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { \
clock_gettime(CLOCK_REALTIME, &tp); \
fprintf(stderr, "%li.%04li: ", \
- tp.tv_sec - log_debug_start, \
+ tp.tv_sec - log_start, \
tp.tv_nsec / (100L * 1000)); \
- } else { \
+ } \
+ \
+ if ((LOG_MASK(LOG_PRI(level)) & log_mask) || \
+ setlogmask(0) == LOG_MASK(LOG_EMERG)) { \
va_start(args, format); \
- passt_vsyslog(level, format, args); \
+ if (log_file != -1) \
+ logfile_write(level, format, args); \
+ else if (!(setlogmask(0) & LOG_MASK(LOG_DEBUG))) \
+ passt_vsyslog(level, format, args); \
va_end(args); \
} \
\
- if (setlogmask(0) & LOG_MASK(LOG_DEBUG) || \
- setlogmask(0) == LOG_MASK(LOG_EMERG)) { \
+ if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) || \
+ setlogmask(0) == LOG_MASK(LOG_EMERG)) && log_file == -1) { \
va_start(args, format); \
(void)vfprintf(stderr, format, args); \
va_end(args); \
@@ -58,6 +76,16 @@ void name(const char *format, ...) { \
} \
}
+/* Prefixes for log file messages, indexed by priority */
+const char *logfile_prefix[] = {
+ NULL, NULL, NULL, /* Unused: LOG_EMERG, LOG_ALERT, LOG_CRIT */
+ "ERROR: ",
+ "WARNING: ",
+ NULL, /* Unused: LOG_NOTICE */
+ "info: ",
+ " ", /* LOG_DEBUG */
+};
+
logfn(err, LOG_ERR)
logfn(warn, LOG_WARNING)
logfn(info, LOG_INFO)
@@ -79,7 +107,7 @@ void __openlog(const char *ident, int option, int facility)
struct timespec tp;
clock_gettime(CLOCK_REALTIME, &tp);
- log_debug_start = tp.tv_sec;
+ log_start = tp.tv_sec;
if (log_sock < 0) {
struct sockaddr_un a = { .sun_family = AF_UNIX, };
@@ -124,9 +152,6 @@ 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);
@@ -141,3 +166,200 @@ void passt_vsyslog(int pri, const char *format, va_list ap)
if (send(log_sock, buf, n, 0) != n)
fprintf(stderr, "Failed to send %i bytes to syslog\n", n);
}
+
+/**
+ * logfile_init() - Open log file and write header with PID and path
+ * @name: Identifier for header: passt or pasta
+ * @path: Path to log file
+ * @size: Maximum size of log file: log_cut_size is calculatd here
+ */
+void logfile_init(const char *name, const char *path, size_t size)
+{
+ char nl = '\n', exe[PATH_MAX] = { 0 };
+ int n;
+
+ if (readlink("/proc/self/exe", exe, PATH_MAX - 1) < 0) {
+ perror("readlink /proc/self/exe");
+ exit(EXIT_FAILURE);
+ }
+
+ log_file = open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEXEC,
+ S_IRUSR | S_IWUSR);
+ if (log_file == -1) {
+ err("Couldn't open log file %s: %s", path, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ log_size = size ? size : LOGFILE_SIZE_DEFAULT;
+
+ n = snprintf(log_header, sizeof(log_header), "%s: %s (%i)",
+ name, exe, getpid());
+
+ if (write(log_file, log_header, n) <= 0 ||
+ write(log_file, &nl, 1) <= 0) {
+ perror("Couldn't write to log file\n");
+ exit(EXIT_FAILURE);
+ }
+
+ /* For FALLOC_FL_COLLAPSE_RANGE: VFS block size can be up to one page */
+ log_cut_size = ROUND_UP(log_size * LOGFILE_CUT_RATIO / 100, PAGE_SIZE);
+}
+
+#ifdef FALLOC_FL_COLLAPSE_RANGE
+/**
+ * logfile_rotate_fallocate() - Write header, set log_written after fallocate()
+ * @fd: Log file descriptor
+ * @ts: Current timestamp
+ *
+ * #syscalls lseek ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
+ */
+static void logfile_rotate_fallocate(int fd, struct timespec *ts)
+{
+ char buf[BUFSIZ], *nl;
+ int n;
+
+ if (lseek(fd, 0, SEEK_SET) == -1)
+ return;
+ if (read(fd, buf, BUFSIZ) == -1)
+ return;
+
+ n = snprintf(buf, BUFSIZ,
+ "%s - log truncated at %li.%04li", log_header,
+ ts->tv_sec - log_start, ts->tv_nsec / (100L * 1000));
+
+ /* Avoid partial lines by padding the header with spaces */
+ nl = memchr(buf + n + 1, '\n', BUFSIZ - n - 1);
+ if (nl)
+ memset(buf + n, ' ', nl - (buf + n));
+
+ if (lseek(fd, 0, SEEK_SET) == -1)
+ return;
+ if (write(fd, buf, BUFSIZ) == -1)
+ return;
+
+ log_written -= log_cut_size;
+}
+#endif /* FALLOC_FL_COLLAPSE_RANGE */
+
+/**
+ * logfile_rotate_move() - Fallback: move recent entries toward start, then cut
+ * @fd: Log file descriptor
+ * @ts: Current timestamp
+ *
+ * #syscalls lseek ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
+ * #syscalls ftruncate
+ */
+static void logfile_rotate_move(int fd, struct timespec *ts)
+{
+ int header_len, write_offset, end, discard, n;
+ char buf[BUFSIZ], *nl;
+
+ header_len = snprintf(buf, BUFSIZ,
+ "%s - log truncated at %li.%04li\n", log_header,
+ ts->tv_sec - log_start,
+ ts->tv_nsec / (100L * 1000));
+ if (lseek(fd, 0, SEEK_SET) == -1)
+ return;
+ if (write(fd, buf, header_len) == -1)
+ return;
+
+ end = write_offset = header_len;
+ discard = log_cut_size + header_len;
+
+ /* Try to cut cleanly at newline */
+ if (lseek(fd, discard, SEEK_SET) == -1)
+ goto out;
+ if ((n = read(fd, buf, BUFSIZ)) <= 0)
+ goto out;
+ if ((nl = memchr(buf, '\n', n)))
+ discard += (nl - buf) + 1;
+
+ /* Go to first block to be moved */
+ if (lseek(fd, discard, SEEK_SET) == -1)
+ goto out;
+
+ while ((n = read(fd, buf, BUFSIZ)) > 0) {
+ end = header_len;
+
+ if (lseek(fd, write_offset, SEEK_SET) == -1)
+ goto out;
+ if ((n = write(fd, buf, n)) == -1)
+ goto out;
+ write_offset += n;
+
+ if ((n = lseek(fd, 0, SEEK_CUR)) == -1)
+ goto out;
+
+ if (lseek(fd, discard - header_len, SEEK_CUR) == -1)
+ goto out;
+
+ end = n;
+ }
+
+out:
+ if (ftruncate(fd, end))
+ return;
+
+ log_written = end;
+}
+
+/**
+ * logfile_rotate() - "Rotate" log file once it's full
+ * @fd: Log file descriptor
+ * @ts: Current timestamp
+ *
+ * Return: 0 on success, negative error code on failure
+ *
+ * #syscalls fcntl
+ *
+ * fallocate() passed as EXTRA_SYSCALL only if FALLOC_FL_COLLAPSE_RANGE is there
+ */
+static int logfile_rotate(int fd, struct timespec *ts)
+{
+ if (fcntl(fd, F_SETFL, O_RDWR /* Drop O_APPEND: explicit lseek() */))
+ return -errno;
+
+#ifdef FALLOC_FL_COLLAPSE_RANGE
+ /* Only for Linux >= 3.15, extent-based ext4 or XFS, glibc >= 2.18 */
+ if (!fallocate(fd, FALLOC_FL_COLLAPSE_RANGE, 0, log_cut_size))
+ logfile_rotate_fallocate(fd, ts);
+ else
+#endif
+ logfile_rotate_move(fd, ts);
+
+ if (fcntl(fd, F_SETFL, O_RDWR | O_APPEND))
+ return -errno;
+
+ return 0;
+}
+
+/**
+ * logfile_write() - Write entry to log file, trigger rotation if full
+ * @pri: Facility and level map, same as priority for vsyslog()
+ * @format: Same as vsyslog() format
+ * @ap: Same as vsyslog() ap
+ */
+void logfile_write(int pri, const char *format, va_list ap)
+{
+ struct timespec ts;
+ char buf[BUFSIZ];
+ int n;
+
+ if (clock_gettime(CLOCK_REALTIME, &ts))
+ return;
+
+ n = snprintf(buf, BUFSIZ, "%li.%04li: %s",
+ ts.tv_sec - log_start, ts.tv_nsec / (100L * 1000),
+ logfile_prefix[pri]);
+
+ n += vsnprintf(buf + n, BUFSIZ - n, format, ap);
+
+ if (format[strlen(format)] != '\n')
+ n += snprintf(buf + n, BUFSIZ - n, "\n");
+
+ if ((log_written + n >= log_size) && logfile_rotate(log_file, &ts))
+ return;
+
+ if ((n = write(log_file, buf, n)) >= 0)
+ log_written += n;
+}
diff --git a/log.h b/log.h
index 70cad6f..f92394c 100644
--- a/log.h
+++ b/log.h
@@ -6,6 +6,10 @@
#ifndef LOG_H
#define LOG_H
+#define LOGFILE_SIZE_DEFAULT (1024 * 1024UL)
+#define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */
+#define LOGFILE_SIZE_MIN (5UL * MAX(BUFSIZ, PAGE_SIZE))
+
void err(const char *format, ...);
void warn(const char *format, ...);
void info(const char *format, ...);
@@ -20,7 +24,9 @@ void trace_init(int enable);
} while (0)
void __openlog(const char *ident, int option, int facility);
+void logfile_init(const char *name, const char *path, size_t size);
void passt_vsyslog(int pri, const char *format, va_list ap);
+void logfile_write(int pri, const char *format, va_list ap);
void __setlogmask(int mask);
#endif /* LOG_H */
diff --git a/passt.1 b/passt.1
index 83395bc..64236b6 100644
--- a/passt.1
+++ b/passt.1
@@ -79,7 +79,7 @@ for performance reasons.
.TP
.BR \-d ", " \-\-debug
-Be verbose, don't run in background.
+Be verbose, don't run in background, don't log to the system logger.
.TP
.BR \-\-trace
@@ -99,9 +99,19 @@ Default is to fork into background.
.TP
.BR \-e ", " \-\-stderr
Log to standard error too.
-Default is to log to system logger only, if started from an interactive
+Default is to log to the system logger only, if started from an interactive
terminal, and to both system logger and standard error otherwise.
+.TP
+.BR \-l ", " \-\-log-file " " \fIPATH\fR
+Log to file \fIPATH\fR, not to standard error, and not to the system logger.
+
+.TP
+.BR \-\-log-size " " \fISIZE\fR
+Limit log file size to \fISIZE\fR bytes. When the log file is full, make room
+for new entries by removing old ones at the beginning. This limit is mandatory.
+Default is 1048576 (1 MiB).
+
.TP
.BR \-\-runas " " \fIUID\fR|\fIUID:GID\fR|\fILOGIN\fR|\fILOGIN:GROUP\fR
Attempt to change to given UID and corresponding group if UID is given,
diff --git a/passt.c b/passt.c
index 0c561b2..7589b05 100644
--- a/passt.c
+++ b/passt.c
@@ -220,6 +220,7 @@ int main(int argc, char **argv)
__openlog(log_name, 0, LOG_DAEMON);
+ /* Meaning we don't know yet: log everything. LOG_EMERG is unused */
__setlogmask(LOG_MASK(LOG_EMERG));
c.epollfd = epoll_create1(EPOLL_CLOEXEC);
--
@@ -220,6 +220,7 @@ int main(int argc, char **argv)
__openlog(log_name, 0, LOG_DAEMON);
+ /* Meaning we don't know yet: log everything. LOG_EMERG is unused */
__setlogmask(LOG_MASK(LOG_EMERG));
c.epollfd = epoll_create1(EPOLL_CLOEXEC);
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 4/7] log, conf: Add support for logging to file
2022-10-10 8:35 ` [PATCH v3 4/7] log, conf: Add support for logging to file Stefano Brivio
@ 2022-10-11 0:11 ` David Gibson
0 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2022-10-11 0:11 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 21210 bytes --]
On Mon, Oct 10, 2022 at 10:35:45AM +0200, Stefano Brivio wrote:
> In some environments, such as KubeVirt pods, we might not have a
> system logger available. We could choose to run in foreground, but
> this takes away the convenient synchronisation mechanism derived from
> forking to background when interfaces are ready.
>
> Add optional logging to file with -l/--log-file and --log-size.
>
> Unfortunately, this means we need to duplicate features that are more
> appropriately implemented by a system logger, such as rotation. Keep
> that reasonably simple, by using fallocate() with range collapsing
> where supported (Linux kernel >= 3.15, extent-based ext4 and XFS) and
> falling back to an unsophisticated block-by-block moving of entries
> toward the beginning of the file once we reach the (mandatory) size
> limit.
>
> While at it, clarify the role of LOG_EMERG in passt.c.
>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> v3:
> - don't use FALLOC_FL_COLLAPSE_RANGE if not defined in headers (e.g.
> glibc < 2.18), and add fallocate() to syscall list optionally using
> EXTRA_SYSCALLS in Makefile, depending on availability of that flag
> - reported by clang-tidy: perform multiplication for
> LOGFILE_SIZE_DEFAULT as unsigned long
>
> v2:
> - Make truncation on different failure modes in logfile_rotate_move()
> consistent (David Gibson)
> - Actually define default maximum log size as 1 MiB, not 1 MB (David
> Gibson)
>
> Makefile | 15 ++--
> README.md | 2 +-
> conf.c | 49 ++++++++++-
> log.c | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> log.h | 6 ++
> passt.1 | 14 ++-
> passt.c | 1 +
> 7 files changed, 316 insertions(+), 29 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 74bbfeb..e4c64fe 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -41,7 +41,7 @@ SRCS = $(PASST_SRCS) $(QRAP_SRCS)
> MANPAGES = passt.1 pasta.1 qrap.1
>
> PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
> - isolation.h lineread.h ndp.h netlink.h packet.h passt.h pasta.h \
> + isolation.h lineread.h log.h ndp.h netlink.h packet.h passt.h pasta.h \
> pcap.h port_fwd.h siphash.h tap.h tcp.h tcp_splice.h udp.h util.h
> HEADERS = $(PASST_HEADERS) seccomp.h
>
> @@ -90,6 +90,11 @@ ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; ec
> FLAGS += -fstack-protector-strong
> endif
>
> +C := \#define _GNU_SOURCE\n\#include <fcntl.h>\nx = FALLOC_FL_COLLAPSE_RANGE;
> +ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
> + EXTRA_SYSCALLS += fallocate
> +endif
> +
> prefix ?= /usr/local
> exec_prefix ?= $(prefix)
> bindir ?= $(exec_prefix)/bin
> @@ -110,7 +115,7 @@ static: FLAGS += -static -DGLIBC_NO_STATIC_NSS
> static: clean all
>
> seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
> - @ EXTRA_SYSCALLS=$(EXTRA_SYSCALLS) ./seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
> + @ EXTRA_SYSCALLS="$(EXTRA_SYSCALLS)" ./seccomp.sh $(PASST_SRCS) $(PASST_HEADERS)
>
> passt: $(PASST_SRCS) $(HEADERS)
> $(CC) $(FLAGS) $(CFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)
> @@ -128,9 +133,9 @@ pasta.avx2 pasta.1 pasta: pasta%: passt%
> qrap: $(QRAP_SRCS) passt.h
> $(CC) $(FLAGS) $(CFLAGS) $(QRAP_SRCS) -o qrap $(LDFLAGS)
>
> -valgrind: EXTRA_SYSCALLS="rt_sigprocmask rt_sigtimedwait rt_sigaction \
> - getpid gettid kill clock_gettime mmap munmap open \
> - unlink gettimeofday futex"
> +valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \
> + getpid gettid kill clock_gettime mmap \
> + munmap open unlink gettimeofday futex
> valgrind: FLAGS:=-g -O0 $(filter-out -O%,$(FLAGS))
> valgrind: all
>
> diff --git a/README.md b/README.md
> index 2c522b2..3dc4fc5 100644
> --- a/README.md
> +++ b/README.md
> @@ -289,7 +289,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 (26 syscalls allowed for _passt_, 40 for
> +* ✅ restrictive seccomp profiles (30 syscalls allowed for _passt_, 41 for
> _pasta_ on x86_64)
> * ✅ examples of [AppArmor](/passt/tree/contrib/apparmor) and
> [SELinux](/passt/tree/contrib/selinux) profiles available
> diff --git a/conf.c b/conf.c
> index e9cd4a7..f22940b 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -633,6 +633,9 @@ static void usage(const char *name)
> info( " default: run in background if started from a TTY");
> info( " -e, --stderr Log to stderr too");
> info( " default: log to system logger only if started from a TTY");
> + info( " -l, --log-file PATH Log (only) to given file");
> + info( " --log-size BYTES Maximum size of log file");
> + info( " default: 1 MiB");
> info( " --runas UID|UID:GID Run as given UID, GID, which can be");
> info( " numeric, or login and group names");
> info( " default: drop to user \"nobody\"");
> @@ -994,6 +997,7 @@ void conf(struct ctx *c, int argc, char **argv)
> {"quiet", no_argument, NULL, 'q' },
> {"foreground", no_argument, NULL, 'f' },
> {"stderr", no_argument, NULL, 'e' },
> + {"log-file", required_argument, NULL, 'l' },
> {"help", no_argument, NULL, 'h' },
> {"socket", required_argument, NULL, 's' },
> {"ns-ifname", required_argument, NULL, 'I' },
> @@ -1034,26 +1038,28 @@ void conf(struct ctx *c, int argc, char **argv)
> {"no-netns-quit", no_argument, NULL, 10 },
> {"trace", no_argument, NULL, 11 },
> {"runas", required_argument, NULL, 12 },
> + {"log-size", required_argument, NULL, 13 },
> { 0 },
> };
> struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
> char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
> bool v4_only = false, v6_only = false;
> + char *runas = NULL, *logfile = NULL;
> struct in6_addr *dns6 = c->ip6.dns;
> struct fqdn *dnss = c->dns_search;
> uint32_t *dns4 = c->ip4.dns;
> int name, ret, mask, b, i;
> const char *optstring;
> unsigned int ifi = 0;
> - char *runas = NULL;
> + size_t logsize = 0;
> uid_t uid;
> gid_t gid;
>
> if (c->mode == MODE_PASTA) {
> c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
> - optstring = "dqfehI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
> + optstring = "dqfel:hI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
> } else {
> - optstring = "dqfehs:p:P:m:a:n:M:g:i:D:S:46t:u:";
> + optstring = "dqfel:hs:p:P:m:a:n:M:g:i:D:S:46t:u:";
> }
>
> c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0;
> @@ -1177,6 +1183,20 @@ void conf(struct ctx *c, int argc, char **argv)
>
> runas = optarg;
> break;
> + case 13:
> + if (logsize) {
> + err("Multiple --log-size options given");
> + usage(argv[0]);
> + }
> +
> + errno = 0;
> + logsize = strtol(optarg, NULL, 0);
> +
> + if (logsize < LOGFILE_SIZE_MIN || errno) {
> + err("Invalid --log-size: %s", optarg);
> + usage(argv[0]);
> + }
> + break;
> case 'd':
> if (c->debug) {
> err("Multiple --debug options given");
> @@ -1192,6 +1212,11 @@ void conf(struct ctx *c, int argc, char **argv)
> c->foreground = 1;
> break;
> case 'e':
> + if (logfile) {
> + err("Can't log to both file and stderr");
> + usage(argv[0]);
> + }
> +
> if (c->stderr) {
> err("Multiple --stderr options given");
> usage(argv[0]);
> @@ -1199,6 +1224,19 @@ void conf(struct ctx *c, int argc, char **argv)
>
> c->stderr = 1;
> break;
> + case 'l':
> + if (c->stderr) {
> + err("Can't log to both stderr and file");
> + usage(argv[0]);
> + }
> +
> + if (logfile) {
> + err("Multiple --log-file options given");
> + usage(argv[0]);
> + }
> +
> + logfile = optarg;
> + break;
> case 'q':
> if (c->quiet) {
> err("Multiple --quiet options given");
> @@ -1460,6 +1498,11 @@ void conf(struct ctx *c, int argc, char **argv)
> if (ret)
> usage(argv[0]);
>
> + if (logfile) {
> + logfile_init(c->mode == MODE_PASST ? "passt" : "pasta",
> + logfile, logsize);
> + }
> +
> if (c->mode == MODE_PASTA) {
> if (conf_pasta_ns(&netns_only, userns, netns,
> optind, argc, argv) < 0)
> diff --git a/log.c b/log.c
> index 54a7cbb..d769a33 100644
> --- a/log.c
> +++ b/log.c
> @@ -12,7 +12,12 @@
> * Author: Stefano Brivio <sbrivio@redhat.com>
> */
>
> +#include <arpa/inet.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include <fcntl.h>
> #include <stdio.h>
> +#include <stdint.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <string.h>
> @@ -20,36 +25,49 @@
> #include <syslog.h>
> #include <stdarg.h>
> #include <sys/socket.h>
> -#include <sys/un.h>
>
> #include "log.h"
> +#include "util.h"
> +#include "passt.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;
> +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_opt; /* Options for openlog() */
> +
> +static 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 */
> +static char log_header[BUFSIZ]; /* File header, written back on cuts */
> +
> +static time_t log_start; /* Start timestamp */
> +int log_trace; /* --trace mode enabled */
>
> #define logfn(name, level) \
> void name(const char *format, ...) { \
> struct timespec tp; \
> va_list args; \
> \
> - if (setlogmask(0) & LOG_MASK(LOG_DEBUG)) { \
> + if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { \
> clock_gettime(CLOCK_REALTIME, &tp); \
> fprintf(stderr, "%li.%04li: ", \
> - tp.tv_sec - log_debug_start, \
> + tp.tv_sec - log_start, \
> tp.tv_nsec / (100L * 1000)); \
> - } else { \
> + } \
> + \
> + if ((LOG_MASK(LOG_PRI(level)) & log_mask) || \
> + setlogmask(0) == LOG_MASK(LOG_EMERG)) { \
> va_start(args, format); \
> - passt_vsyslog(level, format, args); \
> + if (log_file != -1) \
> + logfile_write(level, format, args); \
> + else if (!(setlogmask(0) & LOG_MASK(LOG_DEBUG))) \
> + passt_vsyslog(level, format, args); \
> va_end(args); \
> } \
> \
> - if (setlogmask(0) & LOG_MASK(LOG_DEBUG) || \
> - setlogmask(0) == LOG_MASK(LOG_EMERG)) { \
> + if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) || \
> + setlogmask(0) == LOG_MASK(LOG_EMERG)) && log_file == -1) { \
> va_start(args, format); \
> (void)vfprintf(stderr, format, args); \
> va_end(args); \
> @@ -58,6 +76,16 @@ void name(const char *format, ...) { \
> } \
> }
>
> +/* Prefixes for log file messages, indexed by priority */
> +const char *logfile_prefix[] = {
> + NULL, NULL, NULL, /* Unused: LOG_EMERG, LOG_ALERT, LOG_CRIT */
> + "ERROR: ",
> + "WARNING: ",
> + NULL, /* Unused: LOG_NOTICE */
> + "info: ",
> + " ", /* LOG_DEBUG */
> +};
> +
> logfn(err, LOG_ERR)
> logfn(warn, LOG_WARNING)
> logfn(info, LOG_INFO)
> @@ -79,7 +107,7 @@ void __openlog(const char *ident, int option, int facility)
> struct timespec tp;
>
> clock_gettime(CLOCK_REALTIME, &tp);
> - log_debug_start = tp.tv_sec;
> + log_start = tp.tv_sec;
>
> if (log_sock < 0) {
> struct sockaddr_un a = { .sun_family = AF_UNIX, };
> @@ -124,9 +152,6 @@ 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);
>
> @@ -141,3 +166,200 @@ void passt_vsyslog(int pri, const char *format, va_list ap)
> if (send(log_sock, buf, n, 0) != n)
> fprintf(stderr, "Failed to send %i bytes to syslog\n", n);
> }
> +
> +/**
> + * logfile_init() - Open log file and write header with PID and path
> + * @name: Identifier for header: passt or pasta
> + * @path: Path to log file
> + * @size: Maximum size of log file: log_cut_size is calculatd here
> + */
> +void logfile_init(const char *name, const char *path, size_t size)
> +{
> + char nl = '\n', exe[PATH_MAX] = { 0 };
> + int n;
> +
> + if (readlink("/proc/self/exe", exe, PATH_MAX - 1) < 0) {
> + perror("readlink /proc/self/exe");
> + exit(EXIT_FAILURE);
> + }
> +
> + log_file = open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEXEC,
> + S_IRUSR | S_IWUSR);
> + if (log_file == -1) {
> + err("Couldn't open log file %s: %s", path, strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> +
> + log_size = size ? size : LOGFILE_SIZE_DEFAULT;
> +
> + n = snprintf(log_header, sizeof(log_header), "%s: %s (%i)",
> + name, exe, getpid());
> +
> + if (write(log_file, log_header, n) <= 0 ||
> + write(log_file, &nl, 1) <= 0) {
> + perror("Couldn't write to log file\n");
> + exit(EXIT_FAILURE);
> + }
> +
> + /* For FALLOC_FL_COLLAPSE_RANGE: VFS block size can be up to one page */
> + log_cut_size = ROUND_UP(log_size * LOGFILE_CUT_RATIO / 100, PAGE_SIZE);
> +}
> +
> +#ifdef FALLOC_FL_COLLAPSE_RANGE
> +/**
> + * logfile_rotate_fallocate() - Write header, set log_written after fallocate()
> + * @fd: Log file descriptor
> + * @ts: Current timestamp
> + *
> + * #syscalls lseek ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
> + */
> +static void logfile_rotate_fallocate(int fd, struct timespec *ts)
> +{
> + char buf[BUFSIZ], *nl;
> + int n;
> +
> + if (lseek(fd, 0, SEEK_SET) == -1)
> + return;
> + if (read(fd, buf, BUFSIZ) == -1)
> + return;
> +
> + n = snprintf(buf, BUFSIZ,
> + "%s - log truncated at %li.%04li", log_header,
> + ts->tv_sec - log_start, ts->tv_nsec / (100L * 1000));
> +
> + /* Avoid partial lines by padding the header with spaces */
> + nl = memchr(buf + n + 1, '\n', BUFSIZ - n - 1);
> + if (nl)
> + memset(buf + n, ' ', nl - (buf + n));
> +
> + if (lseek(fd, 0, SEEK_SET) == -1)
> + return;
> + if (write(fd, buf, BUFSIZ) == -1)
> + return;
> +
> + log_written -= log_cut_size;
> +}
> +#endif /* FALLOC_FL_COLLAPSE_RANGE */
> +
> +/**
> + * logfile_rotate_move() - Fallback: move recent entries toward start, then cut
> + * @fd: Log file descriptor
> + * @ts: Current timestamp
> + *
> + * #syscalls lseek ppc64le:_llseek ppc64:_llseek armv6l:_llseek armv7l:_llseek
> + * #syscalls ftruncate
> + */
> +static void logfile_rotate_move(int fd, struct timespec *ts)
> +{
> + int header_len, write_offset, end, discard, n;
> + char buf[BUFSIZ], *nl;
> +
> + header_len = snprintf(buf, BUFSIZ,
> + "%s - log truncated at %li.%04li\n", log_header,
> + ts->tv_sec - log_start,
> + ts->tv_nsec / (100L * 1000));
> + if (lseek(fd, 0, SEEK_SET) == -1)
> + return;
> + if (write(fd, buf, header_len) == -1)
> + return;
> +
> + end = write_offset = header_len;
> + discard = log_cut_size + header_len;
> +
> + /* Try to cut cleanly at newline */
> + if (lseek(fd, discard, SEEK_SET) == -1)
> + goto out;
> + if ((n = read(fd, buf, BUFSIZ)) <= 0)
> + goto out;
> + if ((nl = memchr(buf, '\n', n)))
> + discard += (nl - buf) + 1;
> +
> + /* Go to first block to be moved */
> + if (lseek(fd, discard, SEEK_SET) == -1)
> + goto out;
> +
> + while ((n = read(fd, buf, BUFSIZ)) > 0) {
> + end = header_len;
> +
> + if (lseek(fd, write_offset, SEEK_SET) == -1)
> + goto out;
> + if ((n = write(fd, buf, n)) == -1)
> + goto out;
> + write_offset += n;
> +
> + if ((n = lseek(fd, 0, SEEK_CUR)) == -1)
> + goto out;
> +
> + if (lseek(fd, discard - header_len, SEEK_CUR) == -1)
> + goto out;
> +
> + end = n;
> + }
> +
> +out:
> + if (ftruncate(fd, end))
> + return;
> +
> + log_written = end;
> +}
> +
> +/**
> + * logfile_rotate() - "Rotate" log file once it's full
> + * @fd: Log file descriptor
> + * @ts: Current timestamp
> + *
> + * Return: 0 on success, negative error code on failure
> + *
> + * #syscalls fcntl
> + *
> + * fallocate() passed as EXTRA_SYSCALL only if FALLOC_FL_COLLAPSE_RANGE is there
> + */
> +static int logfile_rotate(int fd, struct timespec *ts)
> +{
> + if (fcntl(fd, F_SETFL, O_RDWR /* Drop O_APPEND: explicit lseek() */))
> + return -errno;
> +
> +#ifdef FALLOC_FL_COLLAPSE_RANGE
> + /* Only for Linux >= 3.15, extent-based ext4 or XFS, glibc >= 2.18 */
> + if (!fallocate(fd, FALLOC_FL_COLLAPSE_RANGE, 0, log_cut_size))
> + logfile_rotate_fallocate(fd, ts);
> + else
> +#endif
> + logfile_rotate_move(fd, ts);
> +
> + if (fcntl(fd, F_SETFL, O_RDWR | O_APPEND))
> + return -errno;
> +
> + return 0;
> +}
> +
> +/**
> + * logfile_write() - Write entry to log file, trigger rotation if full
> + * @pri: Facility and level map, same as priority for vsyslog()
> + * @format: Same as vsyslog() format
> + * @ap: Same as vsyslog() ap
> + */
> +void logfile_write(int pri, const char *format, va_list ap)
> +{
> + struct timespec ts;
> + char buf[BUFSIZ];
> + int n;
> +
> + if (clock_gettime(CLOCK_REALTIME, &ts))
> + return;
> +
> + n = snprintf(buf, BUFSIZ, "%li.%04li: %s",
> + ts.tv_sec - log_start, ts.tv_nsec / (100L * 1000),
> + logfile_prefix[pri]);
> +
> + n += vsnprintf(buf + n, BUFSIZ - n, format, ap);
> +
> + if (format[strlen(format)] != '\n')
> + n += snprintf(buf + n, BUFSIZ - n, "\n");
> +
> + if ((log_written + n >= log_size) && logfile_rotate(log_file, &ts))
> + return;
> +
> + if ((n = write(log_file, buf, n)) >= 0)
> + log_written += n;
> +}
> diff --git a/log.h b/log.h
> index 70cad6f..f92394c 100644
> --- a/log.h
> +++ b/log.h
> @@ -6,6 +6,10 @@
> #ifndef LOG_H
> #define LOG_H
>
> +#define LOGFILE_SIZE_DEFAULT (1024 * 1024UL)
> +#define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */
> +#define LOGFILE_SIZE_MIN (5UL * MAX(BUFSIZ, PAGE_SIZE))
> +
> void err(const char *format, ...);
> void warn(const char *format, ...);
> void info(const char *format, ...);
> @@ -20,7 +24,9 @@ void trace_init(int enable);
> } while (0)
>
> void __openlog(const char *ident, int option, int facility);
> +void logfile_init(const char *name, const char *path, size_t size);
> void passt_vsyslog(int pri, const char *format, va_list ap);
> +void logfile_write(int pri, const char *format, va_list ap);
> void __setlogmask(int mask);
>
> #endif /* LOG_H */
> diff --git a/passt.1 b/passt.1
> index 83395bc..64236b6 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -79,7 +79,7 @@ for performance reasons.
>
> .TP
> .BR \-d ", " \-\-debug
> -Be verbose, don't run in background.
> +Be verbose, don't run in background, don't log to the system logger.
>
> .TP
> .BR \-\-trace
> @@ -99,9 +99,19 @@ Default is to fork into background.
> .TP
> .BR \-e ", " \-\-stderr
> Log to standard error too.
> -Default is to log to system logger only, if started from an interactive
> +Default is to log to the system logger only, if started from an interactive
> terminal, and to both system logger and standard error otherwise.
>
> +.TP
> +.BR \-l ", " \-\-log-file " " \fIPATH\fR
> +Log to file \fIPATH\fR, not to standard error, and not to the system logger.
> +
> +.TP
> +.BR \-\-log-size " " \fISIZE\fR
> +Limit log file size to \fISIZE\fR bytes. When the log file is full, make room
> +for new entries by removing old ones at the beginning. This limit is mandatory.
> +Default is 1048576 (1 MiB).
> +
> .TP
> .BR \-\-runas " " \fIUID\fR|\fIUID:GID\fR|\fILOGIN\fR|\fILOGIN:GROUP\fR
> Attempt to change to given UID and corresponding group if UID is given,
> diff --git a/passt.c b/passt.c
> index 0c561b2..7589b05 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -220,6 +220,7 @@ int main(int argc, char **argv)
>
> __openlog(log_name, 0, LOG_DAEMON);
>
> + /* Meaning we don't know yet: log everything. LOG_EMERG is unused */
> __setlogmask(LOG_MASK(LOG_EMERG));
>
> c.epollfd = epoll_create1(EPOLL_CLOEXEC);
--
David Gibson | 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] 10+ messages in thread
* [PATCH v3 5/7] log: Add missing function comment for trace_init()
2022-10-10 8:35 [PATCH v3 0/7] Add support for log file and version display Stefano Brivio
` (3 preceding siblings ...)
2022-10-10 8:35 ` [PATCH v3 4/7] log, conf: Add support for logging to file Stefano Brivio
@ 2022-10-10 8:35 ` Stefano Brivio
2022-10-10 8:35 ` [PATCH v3 6/7] conf, log, Makefile: Add versioning information Stefano Brivio
2022-10-10 8:35 ` [PATCH v3 7/7] util: Check return value of lseek() while reading bound ports from procfs Stefano Brivio
6 siblings, 0 replies; 10+ messages in thread
From: Stefano Brivio @ 2022-10-10 8:35 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
log.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/log.c b/log.c
index d769a33..993c617 100644
--- a/log.c
+++ b/log.c
@@ -91,6 +91,10 @@ logfn(warn, LOG_WARNING)
logfn(info, LOG_INFO)
logfn(debug, LOG_DEBUG)
+/**
+ * trace_init() - Set log_trace depending on trace (debug) mode
+ * @enable: Tracing debug mode enabled if non-zero
+ */
void trace_init(int enable)
{
log_trace = enable;
--
@@ -91,6 +91,10 @@ logfn(warn, LOG_WARNING)
logfn(info, LOG_INFO)
logfn(debug, LOG_DEBUG)
+/**
+ * trace_init() - Set log_trace depending on trace (debug) mode
+ * @enable: Tracing debug mode enabled if non-zero
+ */
void trace_init(int enable)
{
log_trace = enable;
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 6/7] conf, log, Makefile: Add versioning information
2022-10-10 8:35 [PATCH v3 0/7] Add support for log file and version display Stefano Brivio
` (4 preceding siblings ...)
2022-10-10 8:35 ` [PATCH v3 5/7] log: Add missing function comment for trace_init() Stefano Brivio
@ 2022-10-10 8:35 ` Stefano Brivio
2022-10-11 0:12 ` David Gibson
2022-10-10 8:35 ` [PATCH v3 7/7] util: Check return value of lseek() while reading bound ports from procfs Stefano Brivio
6 siblings, 1 reply; 10+ messages in thread
From: Stefano Brivio @ 2022-10-10 8:35 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson
Add a --version option displaying that, and also include this
information in the log files.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
Makefile | 3 +++
conf.c | 8 ++++++++
contrib/fedora/passt.spec | 1 +
log.c | 4 ++--
passt.1 | 4 ++++
util.h | 8 ++++++++
6 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index e4c64fe..6b22408 100644
--- a/Makefile
+++ b/Makefile
@@ -9,6 +9,8 @@
# Copyright (c) 2021 Red Hat GmbH
# Author: Stefano Brivio <sbrivio@redhat.com>
+VERSION ?= $(shell git describe --tags HEAD 2>/dev/null || echo "unknown\ version")
+
RLIMIT_STACK_VAL := $(shell /bin/sh -c 'ulimit -s')
ifeq ($(RLIMIT_STACK_VAL),unlimited)
RLIMIT_STACK_VAL := 1024
@@ -31,6 +33,7 @@ FLAGS += -DNETNS_RUN_DIR=\"/run/netns\"
FLAGS += -DPASST_AUDIT_ARCH=AUDIT_ARCH_$(AUDIT_ARCH)
FLAGS += -DRLIMIT_STACK_VAL=$(RLIMIT_STACK_VAL)
FLAGS += -DARCH=\"$(TARGET_ARCH)\"
+FLAGS += -DVERSION=\"$(VERSION)\"
PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
isolation.c lineread.c log.c mld.c ndp.c netlink.c packet.c passt.c \
diff --git a/conf.c b/conf.c
index f22940b..7c3e346 100644
--- a/conf.c
+++ b/conf.c
@@ -626,6 +626,7 @@ static void usage(const char *name)
}
info("");
+
info( " -d, --debug Be verbose, don't run in background");
info( " --trace Be extra verbose, implies --debug");
info( " -q, --quiet Don't print informational messages");
@@ -640,6 +641,7 @@ static void usage(const char *name)
info( " numeric, or login and group names");
info( " default: drop to user \"nobody\"");
info( " -h, --help Display this help message and exit");
+ info( " --version Show version and exit");
if (strstr(name, "pasta")) {
info( " -I, --ns-ifname NAME namespace interface name");
@@ -1039,6 +1041,7 @@ void conf(struct ctx *c, int argc, char **argv)
{"trace", no_argument, NULL, 11 },
{"runas", required_argument, NULL, 12 },
{"log-size", required_argument, NULL, 13 },
+ {"version", no_argument, NULL, 14 },
{ 0 },
};
struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
@@ -1197,6 +1200,11 @@ void conf(struct ctx *c, int argc, char **argv)
usage(argv[0]);
}
break;
+ case 14:
+ fprintf(stdout,
+ c->mode == MODE_PASST ? "passt " : "pasta ");
+ fprintf(stdout, VERSION_BLOB);
+ exit(EXIT_SUCCESS);
case 'd':
if (c->debug) {
err("Multiple --debug options given");
diff --git a/contrib/fedora/passt.spec b/contrib/fedora/passt.spec
index bd60650..f441f5c 100644
--- a/contrib/fedora/passt.spec
+++ b/contrib/fedora/passt.spec
@@ -48,6 +48,7 @@ This package adds SELinux enforcement to passt(1) and pasta(1).
%setup -q -n passt-%{git_hash}
%build
+%global optflags %{optflags} -DVERSION=\"%{version}-%{release}.%{_arch}\"
%set_build_flags
%make_build
diff --git a/log.c b/log.c
index 993c617..468c730 100644
--- a/log.c
+++ b/log.c
@@ -172,7 +172,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap)
}
/**
- * logfile_init() - Open log file and write header with PID and path
+ * logfile_init() - Open log file and write header with PID, version, path
* @name: Identifier for header: passt or pasta
* @path: Path to log file
* @size: Maximum size of log file: log_cut_size is calculatd here
@@ -196,7 +196,7 @@ void logfile_init(const char *name, const char *path, size_t size)
log_size = size ? size : LOGFILE_SIZE_DEFAULT;
- n = snprintf(log_header, sizeof(log_header), "%s: %s (%i)",
+ n = snprintf(log_header, sizeof(log_header), "%s " VERSION ": %s (%i)",
name, exe, getpid());
if (write(log_file, log_header, n) <= 0 ||
diff --git a/passt.1 b/passt.1
index 64236b6..667c1bc 100644
--- a/passt.1
+++ b/passt.1
@@ -124,6 +124,10 @@ Default is to change to user \fInobody\fR if started as root.
.BR \-h ", " \-\-help
Display a help message and exit.
+.TP
+.BR \-\-version
+Show version and exit.
+
.TP
.BR \-p ", " \-\-pcap " " \fIfile
Capture tap-facing (that is, guest-side or namespace-side) network packets to
diff --git a/util.h b/util.h
index 1adbf04..f9a8ec6 100644
--- a/util.h
+++ b/util.h
@@ -6,6 +6,14 @@
#ifndef UTIL_H
#define UTIL_H
+#define VERSION_BLOB \
+ VERSION "\n" \
+ "Copyright Red Hat\n" \
+ "GNU Affero GPL version 3 or later " \
+ "<https://www.gnu.org/licenses/agpl-3.0.html>\n" \
+ "This is free software: you are free to change and redistribute it.\n" \
+ "There is NO WARRANTY, to the extent permitted by law.\n\n"
+
#ifndef SECCOMP_RET_KILL_PROCESS
#define SECCOMP_RET_KILL_PROCESS SECCOMP_RET_KILL
#endif
--
@@ -6,6 +6,14 @@
#ifndef UTIL_H
#define UTIL_H
+#define VERSION_BLOB \
+ VERSION "\n" \
+ "Copyright Red Hat\n" \
+ "GNU Affero GPL version 3 or later " \
+ "<https://www.gnu.org/licenses/agpl-3.0.html>\n" \
+ "This is free software: you are free to change and redistribute it.\n" \
+ "There is NO WARRANTY, to the extent permitted by law.\n\n"
+
#ifndef SECCOMP_RET_KILL_PROCESS
#define SECCOMP_RET_KILL_PROCESS SECCOMP_RET_KILL
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 6/7] conf, log, Makefile: Add versioning information
2022-10-10 8:35 ` [PATCH v3 6/7] conf, log, Makefile: Add versioning information Stefano Brivio
@ 2022-10-11 0:12 ` David Gibson
0 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2022-10-11 0:12 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 5506 bytes --]
On Mon, Oct 10, 2022 at 10:35:47AM +0200, Stefano Brivio wrote:
> Add a --version option displaying that, and also include this
> information in the log files.
>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> Makefile | 3 +++
> conf.c | 8 ++++++++
> contrib/fedora/passt.spec | 1 +
> log.c | 4 ++--
> passt.1 | 4 ++++
> util.h | 8 ++++++++
> 6 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index e4c64fe..6b22408 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -9,6 +9,8 @@
> # Copyright (c) 2021 Red Hat GmbH
> # Author: Stefano Brivio <sbrivio@redhat.com>
>
> +VERSION ?= $(shell git describe --tags HEAD 2>/dev/null || echo "unknown\ version")
> +
> RLIMIT_STACK_VAL := $(shell /bin/sh -c 'ulimit -s')
> ifeq ($(RLIMIT_STACK_VAL),unlimited)
> RLIMIT_STACK_VAL := 1024
> @@ -31,6 +33,7 @@ FLAGS += -DNETNS_RUN_DIR=\"/run/netns\"
> FLAGS += -DPASST_AUDIT_ARCH=AUDIT_ARCH_$(AUDIT_ARCH)
> FLAGS += -DRLIMIT_STACK_VAL=$(RLIMIT_STACK_VAL)
> FLAGS += -DARCH=\"$(TARGET_ARCH)\"
> +FLAGS += -DVERSION=\"$(VERSION)\"
>
> PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
> isolation.c lineread.c log.c mld.c ndp.c netlink.c packet.c passt.c \
> diff --git a/conf.c b/conf.c
> index f22940b..7c3e346 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -626,6 +626,7 @@ static void usage(const char *name)
> }
> info("");
>
> +
> info( " -d, --debug Be verbose, don't run in background");
> info( " --trace Be extra verbose, implies --debug");
> info( " -q, --quiet Don't print informational messages");
> @@ -640,6 +641,7 @@ static void usage(const char *name)
> info( " numeric, or login and group names");
> info( " default: drop to user \"nobody\"");
> info( " -h, --help Display this help message and exit");
> + info( " --version Show version and exit");
>
> if (strstr(name, "pasta")) {
> info( " -I, --ns-ifname NAME namespace interface name");
> @@ -1039,6 +1041,7 @@ void conf(struct ctx *c, int argc, char **argv)
> {"trace", no_argument, NULL, 11 },
> {"runas", required_argument, NULL, 12 },
> {"log-size", required_argument, NULL, 13 },
> + {"version", no_argument, NULL, 14 },
> { 0 },
> };
> struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
> @@ -1197,6 +1200,11 @@ void conf(struct ctx *c, int argc, char **argv)
> usage(argv[0]);
> }
> break;
> + case 14:
> + fprintf(stdout,
> + c->mode == MODE_PASST ? "passt " : "pasta ");
> + fprintf(stdout, VERSION_BLOB);
> + exit(EXIT_SUCCESS);
> case 'd':
> if (c->debug) {
> err("Multiple --debug options given");
> diff --git a/contrib/fedora/passt.spec b/contrib/fedora/passt.spec
> index bd60650..f441f5c 100644
> --- a/contrib/fedora/passt.spec
> +++ b/contrib/fedora/passt.spec
> @@ -48,6 +48,7 @@ This package adds SELinux enforcement to passt(1) and pasta(1).
> %setup -q -n passt-%{git_hash}
>
> %build
> +%global optflags %{optflags} -DVERSION=\"%{version}-%{release}.%{_arch}\"
> %set_build_flags
> %make_build
>
> diff --git a/log.c b/log.c
> index 993c617..468c730 100644
> --- a/log.c
> +++ b/log.c
> @@ -172,7 +172,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap)
> }
>
> /**
> - * logfile_init() - Open log file and write header with PID and path
> + * logfile_init() - Open log file and write header with PID, version, path
> * @name: Identifier for header: passt or pasta
> * @path: Path to log file
> * @size: Maximum size of log file: log_cut_size is calculatd here
> @@ -196,7 +196,7 @@ void logfile_init(const char *name, const char *path, size_t size)
>
> log_size = size ? size : LOGFILE_SIZE_DEFAULT;
>
> - n = snprintf(log_header, sizeof(log_header), "%s: %s (%i)",
> + n = snprintf(log_header, sizeof(log_header), "%s " VERSION ": %s (%i)",
> name, exe, getpid());
>
> if (write(log_file, log_header, n) <= 0 ||
> diff --git a/passt.1 b/passt.1
> index 64236b6..667c1bc 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -124,6 +124,10 @@ Default is to change to user \fInobody\fR if started as root.
> .BR \-h ", " \-\-help
> Display a help message and exit.
>
> +.TP
> +.BR \-\-version
> +Show version and exit.
> +
> .TP
> .BR \-p ", " \-\-pcap " " \fIfile
> Capture tap-facing (that is, guest-side or namespace-side) network packets to
> diff --git a/util.h b/util.h
> index 1adbf04..f9a8ec6 100644
> --- a/util.h
> +++ b/util.h
> @@ -6,6 +6,14 @@
> #ifndef UTIL_H
> #define UTIL_H
>
> +#define VERSION_BLOB \
> + VERSION "\n" \
> + "Copyright Red Hat\n" \
> + "GNU Affero GPL version 3 or later " \
> + "<https://www.gnu.org/licenses/agpl-3.0.html>\n" \
> + "This is free software: you are free to change and redistribute it.\n" \
> + "There is NO WARRANTY, to the extent permitted by law.\n\n"
> +
> #ifndef SECCOMP_RET_KILL_PROCESS
> #define SECCOMP_RET_KILL_PROCESS SECCOMP_RET_KILL
> #endif
--
David Gibson | 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] 10+ messages in thread
* [PATCH v3 7/7] util: Check return value of lseek() while reading bound ports from procfs
2022-10-10 8:35 [PATCH v3 0/7] Add support for log file and version display Stefano Brivio
` (5 preceding siblings ...)
2022-10-10 8:35 ` [PATCH v3 6/7] conf, log, Makefile: Add versioning information Stefano Brivio
@ 2022-10-10 8:35 ` Stefano Brivio
6 siblings, 0 replies; 10+ messages in thread
From: Stefano Brivio @ 2022-10-10 8:35 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson
Coverity now noticed we're checking most lseek() return values, but
not this. Not really relevant, but it doesn't hurt to check we can
actually seek before reading lines.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
util.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/util.c b/util.c
index b366167..5b1e08a 100644
--- a/util.c
+++ b/util.c
@@ -311,10 +311,14 @@ void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns,
path = "/proc/net/udp6";
}
- if (*fd != -1)
- lseek(*fd, 0, SEEK_SET);
- else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0)
+ if (*fd != -1) {
+ if (lseek(*fd, 0, SEEK_SET)) {
+ warn("lseek() failed on %s: %s", path, strerror(errno));
+ return;
+ }
+ } else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) {
return;
+ }
lineread_init(&lr, *fd);
lineread_get(&lr, &line); /* throw away header */
--
@@ -311,10 +311,14 @@ void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns,
path = "/proc/net/udp6";
}
- if (*fd != -1)
- lseek(*fd, 0, SEEK_SET);
- else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0)
+ if (*fd != -1) {
+ if (lseek(*fd, 0, SEEK_SET)) {
+ warn("lseek() failed on %s: %s", path, strerror(errno));
+ return;
+ }
+ } else if ((*fd = open(path, O_RDONLY | O_CLOEXEC)) < 0) {
return;
+ }
lineread_init(&lr, *fd);
lineread_get(&lr, &line); /* throw away header */
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread