On Thu, 30 Jan 2025 19:02:45 +0100,
Martijn van Duren <[email protected]> wrote:
>
> Hello misc@,
>
> The diff below adds support for 4 different loglevels to smtpd filters:
> warn, info, debug, trace. These for levels are in line with the
> loglevels of smtpd itself and are emitted on said levels.
>
> To keep backwards compatibility and to keep things simpler for coders
> who don't care about loglevels, loglines without prefix will be logged
> at the warn (brief) level.
>
I generally like it.
But I feel that one pice is missed.
If I make a filter with debug and trace logging, it can be quite noisy. I
not sure that it worth to send all debug and trace level logs to smtpd which
is ignored due to wrong log level.
I suggest to extend handshake protocol and to add a configured log-level,
which allows to adjust log level in filter and skip too noisy logs.
We also can make it dynamic, but here the catch:
1. we may restart filters after changing a log level,
2. or to send an update for handshake.
I think that (1) is safer solution, but I may be wrong.
> martijn@
>
> diff bfd8327fa29c9064ad80b6cdb8235722bbf18877
> acdf9909bf270b4d19c207f705c0e1bef68c8cc1
> commit - bfd8327fa29c9064ad80b6cdb8235722bbf18877
> commit + acdf9909bf270b4d19c207f705c0e1bef68c8cc1
> blob - b9c5d55c15f9052bc726f1a09484f0e32f75e2f3
> blob + cbd8657110f1cac373d7964c20b9fca57fc17ec1
> --- usr.sbin/smtpd/lka_filter.c
> +++ usr.sbin/smtpd/lka_filter.c
> @@ -317,8 +317,26 @@ processor_errfd(struct io *io, int evt, void *arg)
>
> switch (evt) {
> case IO_DATAIN:
> - while ((line = io_getline(io, &len)) != NULL)
> - log_warnx("%s: %s", name, line);
> + while ((line = io_getline(io, &len)) != NULL) {
> + if (strncasecmp(line, "WARN|",
> + sizeof("WARN|") - 1) == 0)
> + log_warnx("%s: %s", name,
> + line + sizeof("WARN|") - 1);
> + else if (strncasecmp(line, "INFO|",
> + sizeof("INFO|") - 1) == 0)
> + log_info("%s: %s", name,
> + line + sizeof("INFO|") - 1);
> + else if (strncasecmp(line, "DEBUG|",
> + sizeof("DEBUG|") - 1) == 0)
> + log_debug("%s: %s", name,
> + line + sizeof("DEBUG|") - 1);
> + else if (strncasecmp(line, "TRACE|",
> + sizeof("TRACE|") - 1) == 0)
> + log_trace(TRACE_FILTERS, "%s: %s", name,
> + line + sizeof("TRACE|") - 1);
> + else
> + log_warnx("%s: %s", name, line);
> + }
> }
> }
>
> blob - 5a1bcbb39e2380f90adc754d4849b1ac7385c1fe
> blob + d5636b55abfd4bc86cd865851f4b9aa0cb9f2759
> --- usr.sbin/smtpd/smtpd-filters.7
> +++ usr.sbin/smtpd/smtpd-filters.7
> @@ -74,6 +74,12 @@ and may be written in any language.
> must not use blocking I/O,
> they must support answering asynchronously to
> .Xr smtpd 8 .
> +.Pp
> +Loglines to
> +.Xr stderr 4
> +can be prefixed with WARN|, INFO|, DEBUG|, or TRACE| to specify the level at
> +which they need to be logged.
> +Lines without a prefix will be logged as warning.
> .Sh REPORT AND FILTER
> The API relies on two streams,
> report and filter.
>
>
--
wbr, Kirill
