On Tue, May 18, 2021 at 11:05 PM Dmitrii Banshchikov <me@xxxxxxxxxxxxx> wrote:
>
> 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.?
>
> 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 <me@xxxxxxxxxxxxx>
> ---
> 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.
Thanks,
Song
> 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
>
