On Wed, May 19, 2021 at 10:32:25AM -0700, Song Liu wrote:

On Tue, May 18, 2021 at 11:05 PM Dmitrii Banshchikov [off-list ref] wrote:

quoted

There are three logging levels for messages: FATAL, NOTICE and DEBUG.
When a message is logged with FATAL level it results in bpfilter
usermode helper termination.

Could you please explain why we choose to have 3 levels? Will we need
more levels,
like WARNING, ERROR, etc.?

I found that I need one level for development - to trace what
goes rignt and wrong. At the same time as those messages go to
dmesg this level is too verbose to be used under normal
circumstances. That is why another level is introduced. And the
last one exists to verify invariants or error condintions from
which there is no right way to recover and they result in
bpfilter termination.

Probably we may have just two levels - DEBUG and NOTICE and some
analogue of BUG_ON/WARN_ON/runtime assert that results in a
message on NOTICE level and program termination if the checked
condition is false.

I don't think that we will need more levels - until we decide to
utilize syslog facility. Even in that case I don't know how to
differntiate between e.g. NOTICE and INFO messages.

quoted

Introduce struct context to avoid use of global objects and store there
the logging parameters: log level and log sink.

Signed-off-by: Dmitrii Banshchikov <redacted>
---
 net/bpfilter/Makefile  |  2 +-
 net/bpfilter/bflog.c   | 29 +++++++++++++++++++++++++++++
 net/bpfilter/bflog.h   | 24 ++++++++++++++++++++++++
 net/bpfilter/context.h | 16 ++++++++++++++++

Maybe combine bflog.h and context.h into one file? And bflog() can
probably fit in
that file too.

Sure.

Thanks,
Song

quoted

 4 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 net/bpfilter/bflog.c
 create mode 100644 net/bpfilter/bflog.h
 create mode 100644 net/bpfilter/context.h

diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
index cdac82b8c53a..874d5ef6237d 100644
--- a/net/bpfilter/Makefile
+++ b/net/bpfilter/Makefile

@@ -4,7 +4,7 @@
 #

 userprogs := bpfilter_umh
-bpfilter_umh-objs := main.o
+bpfilter_umh-objs := main.o bflog.o
 userccflags += -I $(srctree)/tools/include/ -I $(srctree)/tools/include/uapi

 ifeq ($(CONFIG_BPFILTER_UMH), y)

diff --git a/net/bpfilter/bflog.c b/net/bpfilter/bflog.c
new file mode 100644
index 000000000000..2752e39060e4
--- /dev/null
+++ b/net/bpfilter/bflog.c

@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021 Telegram FZ-LLC
+ */
+
+#define _GNU_SOURCE
+
+#include "bflog.h"
+
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "context.h"
+
+void bflog(struct context *ctx, int level, const char *fmt, ...)
+{
+       if (ctx->log_file &&
+           (level == BFLOG_LEVEL_FATAL || (level & ctx->log_level))) {
+               va_list va;
+
+               va_start(va, fmt);
+               vfprintf(ctx->log_file, fmt, va);
+               va_end(va);
+       }
+
+       if (level == BFLOG_LEVEL_FATAL)
+               exit(EXIT_FAILURE);
+}

diff --git a/net/bpfilter/bflog.h b/net/bpfilter/bflog.h
new file mode 100644
index 000000000000..4ed12791cfa1
--- /dev/null
+++ b/net/bpfilter/bflog.h

@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Telegram FZ-LLC
+ */
+
+#ifndef NET_BPFILTER_BFLOG_H
+#define NET_BPFILTER_BFLOG_H
+
+struct context;
+
+#define BFLOG_IMPL(ctx, level, fmt, ...) bflog(ctx, level, "bpfilter: " fmt, ##__VA_ARGS__)
+
+#define BFLOG_LEVEL_FATAL (0)
+#define BFLOG_LEVEL_NOTICE (1)
+#define BFLOG_LEVEL_DEBUG (2)
+
+#define BFLOG_FATAL(ctx, fmt, ...)                                                                 \
+       BFLOG_IMPL(ctx, BFLOG_LEVEL_FATAL, "fatal error: " fmt, ##__VA_ARGS__)
+#define BFLOG_NOTICE(ctx, fmt, ...) BFLOG_IMPL(ctx, BFLOG_LEVEL_NOTICE, fmt, ##__VA_ARGS__)
+#define BFLOG_DEBUG(ctx, fmt, ...) BFLOG_IMPL(ctx, BFLOG_LEVEL_DEBUG, fmt, ##__VA_ARGS__)
+
+void bflog(struct context *ctx, int level, const char *fmt, ...);
+
+#endif // NET_BPFILTER_BFLOG_H

diff --git a/net/bpfilter/context.h b/net/bpfilter/context.h
new file mode 100644
index 000000000000..e85c97c3d010
--- /dev/null
+++ b/net/bpfilter/context.h

@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Telegram FZ-LLC
+ */
+
+#ifndef NET_BPFILTER_CONTEXT_H
+#define NET_BPFILTER_CONTEXT_H
+
+#include <stdio.h>
+
+struct context {
+       FILE *log_file;
+       int log_level;
+};
+
+#endif // NET_BPFILTER_CONTEXT_H
--

2.25.1

-- 

Dmitrii Banshchikov