Hi, Aurelien. Sorry for the indentation issues. See the corrected patch attached. Also I declared a pointer to frontend at the top of syslog_fd_handler() to have the same access pattern as in syslog_io_handler().
Regarding your extra notes: I don't have an opinion regarding the "proxy->options_log" options dedicated to log-forward proxies. Since log-forward section only supports keywords explicitly handled in cfg_parse_log_forward() (so proxy->options and proxy->options2 are currently unused), I'm wondering if we could re-use the same memory area, either have PR_O_* flags that have a different meaning when set on log-forward proxy, or share the same memory area by leveraging an union for options_log member. My rationale was to separate clearly the set of options. As I saw already two cluttered set of flags in options and options2, I guessed it would be clearer to create a new one and avoid unions or context-dependent meanings to save a few bytes per config. Also I saw in struct proxy that there are two declarations (no_options and no_options2) marked specifically as "used only during configuration parsing", so I guessed that it shouldn't be a big deal adding a new one. Truth is that I can't imagine right now a situation in the future where proxy->options and proxy->options2 could be useful in log-forward section (let's go crazy; transforming log messages into payloads to be sent using HTTP?), but it would be a PITA if that happens. Also it looks like cfg_opts_log[] is a bit overkill because only the name->flag value association is used. I'll let other give their opinion on that point. My approach is that this way we can use the same scheme of things as with proxy->options and proxy->options2, including the possibility of using PR_CAP_NONE to disable options at build time. Again, I can't see the future, but what about the possibility of options that could cross boundaries from log-forward to other type of proxies? Some type of stickiness at log message level? IDK, but I think that following a well established pattern is a good thing (like declaring a frontend pointer for syslog_fd_handler as in syslog_io_handler). Anyway, I'm more that willing to try other approaches if that makes easier for this to be included as a feature. Thanks a lot in advance. Rober --- Roberto Moreda Allenta Consulting<http://www.allenta.com> (+34 881922600) Privacidad / Privacy<http://allenta.com/mail-privacy> On Mar 3, 2025, at 10:35, Aurelien DARRAGON <adarra...@haproxy.com> wrote: Hi! As discussed few weeks ago in issue #2856 , this is a PR that adds two useful options to the log-forward section. ``` option dontparselog Enables HAProxy to relay syslog messages without attempting to parse and restructure them, useful for forwarding messages that may not conform to traditional formats. This option should be used with the format raw setting on destination log targets to ensure the original message content is preserved. option assume-rfc6587-ntf Directs HAProxy to treat incoming TCP log streams always as using non-transparent framing. This option simplifies the framing logic and ensures consistent handling of messages, particularly useful when dealing with improperly formed starting characters. ``` Thanks a lot in advance for taking this into consideration. Best, Rober Hi Rober, Thanks for your contribution. Here are some remarks: diff --git a/src/log.c b/src/log.c index 90b5645cb2e6..234f83e2426f 100644 --- a/src/log.c +++ b/src/log.c @@ -5376,6 +5376,24 @@ void app_log(struct list *loggers, struct buffer *tag, int level, const char *fo __send_log(NULL, loggers, tag, level, logline, data_len, default_rfc5424_sd_log_format, 2); } + +/* + * This function sets up the initial state for a log message by preparing + * the buffer, setting default values for the log level and facility, and + * initializing metadata fields. It is used before parsing or constructing + * a log message to ensure all fields are in a known state. + */ +void prepare_log_message(char *buf, size_t buflen, int *level, int *facility, idendation issue below + struct ist *metadata, char **message, size_t *size) +{ + *level = *facility = -1; + + *message = buf; + *size = buflen; + + memset(metadata, 0, LOG_META_FIELDS*sizeof(struct ist)); +} @@ -6203,6 +6206,34 @@ int cfg_parse_log_forward(const char *file, int linenum, char **args, int kwm) } cfg_log_forward->timeout.client = MS_TO_TICKS(timeout); } + else if (strcmp(args[0], "option") == 0) { + int optnum; + + if (*(args[1]) == '\0') { + ha_alert("parsing [%s:%d]: '%s' expects an option name.\n", + file, linenum, args[0]); + err_code |= ERR_ALERT | ERR_FATAL; identation issue below + goto out; + } + + + for (optnum = 0; cfg_opts_log[optnum].name; optnum++) { + if (strcmp(args[1], cfg_opts_log[optnum].name) == 0) { + if (cfg_opts_log[optnum].cap == PR_CAP_NONE) { + ha_alert("parsing [%s:%d]: option '%s' is not supported due to build options.\n", identation issue below + file, linenum, cfg_opts2[optnum].name); + err_code |= ERR_ALERT | ERR_FATAL; + goto out; + } + if (alertif_too_many_args_idx(0, 1, file, linenum, args, &err_code)) + goto out; + if (warnifnotcap(cfg_log_forward, cfg_opts_log[optnum].cap, file, linenum, args[1], NULL)) { + err_code |= ERR_WARN; + goto out; + } + cfg_log_forward->options_log |= cfg_opts_log[optnum].val; + } + } + } diff --git a/src/proxy.c b/src/proxy.c index a1143fc68631..9000f63b751e 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -134,6 +134,13 @@ const struct cfg_opt cfg_opts2[] = { NULL, 0, 0, 0 } }; +/* proxy->options_log */ +const struct cfg_opt cfg_opts_log[] = { identation issue below + {"dontparselog", PR_OL_DONTPARSELOG, PR_CAP_FE, 0, PR_MODE_SYSLOG }, + {"assume-rfc6587-ntf", PR_OL_ASSUME_RFC6587_NTF, PR_CAP_FE, 0, PR_MODE_SYSLOG }, + { NULL, 0, 0, 0 } +}; + /* Helper function to resolve a single sticking rule after config parsing. * Returns 1 for success and 0 for failure */ diff --git a/src/log.c b/src/log.c index 234f83e2426f..af9c4c704e94 100644 --- a/src/log.c +++ b/src/log.c @@ -5759,7 +5759,8 @@ void syslog_fd_handler(int fd) prepare_log_message(buf->area, buf->data, &level, &facility, metadata, &message, &size); - parse_log_message(buf->area, buf->data, &level, &facility, metadata, &message, &size); + if (!(l->bind_conf->frontend->options_log & PR_OL_DONTPARSELOG)) I would rather have "struct proxy *frontend = strm_fe(s);" and use that frontend pointer to access options instead of l->bind_conf->frontend. + parse_log_message(buf->area, buf->data, &level, &facility, metadata, &message, &size); Well, aside from indentation issues, I'm fine with the first patch. Regarding the second one, the doc and behavior look good to me, However I don't have an opinion regarding the "proxy->options_log" options dedicated to log-forward proxies. Since log-forward section only supports keywords explicitly handled in cfg_parse_log_forward() (so proxy->options and proxy->options2 are currently unused), I'm wondering if we could re-use the same memory area, either have PR_O_* flags that have a different meaning when set on log-forward proxy, or share the same memory area by leveraging an union for options_log member. Also it looks like cfg_opts_log[] is a bit overkill because only the name->flag value association is used. I'll let other give their opinion on that point. Thanks :) Aurelien
add-dontparselog-and-assume-rfc6587-ntf.patch
Description: add-dontparselog-and-assume-rfc6587-ntf.patch
