From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson To: passt-dev@passt.top Subject: Re: [PATCH 1/8] Move logging functions to a new file, log.c Date: Fri, 07 Oct 2022 17:13:45 +1100 Message-ID: In-Reply-To: <20221007004742.1188933-2-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6179265401547495347==" --===============6179265401547495347== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Fri, Oct 07, 2022 at 02:47:35AM +0200, Stefano Brivio wrote: > Logging to file is going to add some further complexity that we don't > want to squeeze into util.c. >=20 > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > 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 >=20 > diff --git a/Makefile b/Makefile > index d4b623f..74bbfeb 100644 > --- a/Makefile > +++ b/Makefile > @@ -33,8 +33,8 @@ FLAGS +=3D -DRLIMIT_STACK_VAL=3D$(RLIMIT_STACK_VAL) > FLAGS +=3D -DARCH=3D\"$(TARGET_ARCH)\" > =20 > PASST_SRCS =3D 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 =3D qrap.c > SRCS =3D $(PASST_SRCS) $(QRAP_SRCS) > =20 > 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" > =20 > /** > * 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" > =20 > /** > 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" > =20 > /** > * 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" > =20 > #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" > =20 > /** > 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 > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "log.h" > + > +/* For __openlog() and __setlogmask() wrappers, and passt_vsyslog() */ > +static int log_mask; > +static int log_sock =3D -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) =3D=3D LOG_MASK(LOG_EMERG)) { \ > + va_start(args, format); \ > + (void)vfprintf(stderr, format, args); \ > + va_end(args); \ > + if (format[strlen(format)] !=3D '\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 =3D 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 =3D tp.tv_sec; > + > + if (log_sock < 0) { > + struct sockaddr_un a =3D { .sun_family =3D AF_UNIX, }; > + > + log_sock =3D 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 =3D -1; > + return; > + } > + } > + > + log_mask |=3D facility; > + strncpy(log_ident, ident, sizeof(log_ident) - 1); > + log_opt =3D option; > + > + openlog(ident, option, facility); > +} > + > +/** > + * __setlogmask() - setlogmask() wrapper, to allow custom vsyslog() > + * @mask: Same as setlogmask() mask > + */ > +void __setlogmask(int mask) > +{ > + log_mask =3D 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 =3D snprintf(buf, BUFSIZ, "<%i> ", pri); > + > + n +=3D vsnprintf(buf + n, BUFSIZ - n, format, ap); > + > + if (format[strlen(format)] !=3D '\n') > + n +=3D snprintf(buf + n, BUFSIZ - n, "\n"); > + > + if (log_opt & LOG_PERROR) > + fprintf(stderr, "%s", buf + sizeof("<0>")); > + > + if (send(log_sock, buf, n, 0) !=3D 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 > + */ > + > +#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" > =20 > #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 @@ > =20 > #include "util.h" > #include "passt.h" > +#include "log.h" > #include "netlink.h" > =20 > /* Socket in init, in target namespace, sequence (just needs to be monoton= ic) */ > diff --git a/packet.c b/packet.c > index 6d10ec1..3f82e84 100644 > --- a/packet.c > +++ b/packet.c > @@ -20,6 +20,7 @@ > =20 > #include "packet.h" > #include "util.h" > +#include "log.h" > =20 > /** > * 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" > =20 > #define EPOLL_EVENTS 8 > =20 > 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" > =20 > /* 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 @@ > =20 > #include "util.h" > #include "passt.h" > +#include "log.h" > =20 > #define PCAP_VERSION_MINOR 4 > =20 > 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" > =20 > /* 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" > =20 > #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 @@ > =20 > #include "util.h" > #include "passt.h" > +#include "log.h" > =20 > #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" > =20 > #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 > #include > #include > -#include > -#include > #include > #include > #include > @@ -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 =3D -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) =3D=3D LOG_MASK(LOG_EMERG)) { \ > - va_start(args, format); \ > - (void)vfprintf(stderr, format, args); \ > - va_end(args); \ > - if (format[strlen(format)] !=3D '\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 =3D 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 =3D tp.tv_sec; > - > - if (log_sock < 0) { > - struct sockaddr_un a =3D { .sun_family =3D AF_UNIX, }; > - > - log_sock =3D 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 =3D -1; > - return; > - } > - } > - > - log_mask |=3D facility; > - strncpy(log_ident, ident, sizeof(log_ident) - 1); > - log_opt =3D option; > - > - openlog(ident, option, facility); > -} > - > -/** > - * __setlogmask() - setlogmask() wrapper, to allow custom vsyslog() > - * @mask: Same as setlogmask() mask > - */ > -void __setlogmask(int mask) > -{ > - log_mask =3D 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 =3D snprintf(buf, BUFSIZ, "<%i> ", pri); > - > - n +=3D vsnprintf(buf + n, BUFSIZ - n, format, ap); > - > - if (format[strlen(format)] !=3D '\n') > - n +=3D snprintf(buf + n, BUFSIZ - n, "\n"); > - > - if (log_opt & LOG_PERROR) > - fprintf(stderr, "%s", buf + sizeof("<0>")); > - > - if (send(log_sock, buf, n, 0) !=3D n) > - fprintf(stderr, "Failed to send %i bytes to syslog\n", n); > -} > +#include "log.h" > =20 > #define IPV6_NH_OPT(nh) \ > ((nh) =3D=3D 0 || (nh) =3D=3D 43 || (nh) =3D=3D 44 || (nh) =3D=3D 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 > =20 > -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 { > =20 > /* 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, --=20 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 --===============6179265401547495347== Content-Type: application/pgp-signature Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="signature.asc" MIME-Version: 1.0 LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUVCQ0FBZEZpRUVvVUx4V3U0L1dz MGRCK1h0Z3lwWTRnRXdZU0lGQW1NL3c1SUFDZ2tRZ3lwWTRnRXcKWVNKZmJSQUF6ZXo3V3Z1WGxl a1lYRXlrNlZ2eW9QSy9sSXJxd2pKQUp4WFFnbXZ3V3orM0Q4VGs3czR1YWFwRApDajhqV2xBcWxv Q2RURTZEM21rYTRvbk1IQ1JMWldTTlh5REcxUHJkL3JNQ1VxVGJ4MkUwRHljT1daM3JXbWpDCjlL RCtYalJBc2pzTmkxcDQ3Wm50U0MxRjhac0lsWlVUMCsrWkJjTnU3ZUU0V29KTGF0djV5SkFMQXNP cmpabkoKcGVuQ3UwalIvVjNzOXhCZElxL09LUGxlL0FHMkNWMTQvOEZnci9uV2pGZWZxdnZWdTRS T3kvTHNmTVJCZlhjQQpESTQxZHFTMWk1VVhaa2xSektabmZsbjlBTmtPTDBBdWVYOFRKSWRCNVlP cEUwOHp1enMxSld1L0VNb3J3RTJ2CjhTR3hySW9ld09FYk9XcXNUU2lIMmEzaGtLOHVvSm5TQm41 WUQ0TnVxUWsveHFhRkpWQ2JralVqRUMyK054WmEKVGJ0TDRobGRXd2gyb0JHMTdvVDJhekV2Vlp5 aFRWUmI0RHdSUVJadHBMWGM1VXhrQklpU3o3cFQ1eUFDVW56eAp6aytxTVdSbEI2YWZaZG85RHYv b2Y0TmN4TTNUdzNaNXpPL0xQbDRlN0dyeXNDbi9BWkNac0lrQXN5bTVzczNqClJJMTlQMFQ0aEJo aFhjYzBEWlJBMmkyM0Q2ZXBTQmFjdXVUdzZJUlFMMWlhWk5lcUd3K3JQWFZoQ0NBbDBzd1YKM3Mr VDV2UFdwMGFPSXA3cVhPTTFMT2phY3htUVYyTVp1Q01FcmRjQktxYnVnanFubGNrY3hxL3NoZjdF SElzNQpQQ3hsZ3pIWTZiZnc1Z3UwYStKazc2aXp1R01aREhWRFMrK296RUQ2TUg3ZTFoUUg2cFU9 Cj1MWjVJCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo= --===============6179265401547495347==--