Thank you for sharing your thoughts, I really appreciate it. I do really like the idea of having a regular frontend section with "mode log" in the future. Considering this, I fully agree on the approach that you suggest.
Notes: * The new two options take two bits in the proxy->options2. * I replicate the loop to read options as it is in cfgparse-listen.c (only over options2). I added an explicit initialization px->options2 = 0. I'm attaching the new patch. I you prefer me to split it (or any other change), just let me know. Thx! Rober --- Roberto Moreda Allenta Consulting<http://www.allenta.com> (+34 881922600) Privacidad / Privacy<http://allenta.com/mail-privacy> On Mar 3, 2025, at 16:28, Aurelien DARRAGON <adarra...@haproxy.com> wrote: 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. Well that's a good point. I was trying to avoid eating those additional bytes for non-log proxies but if it ends up being more complicated I'm not sure it's worth the trouble you're right. As I thought about this a bit more, I think it doesn't really make sense to add dedicated log_options field if we expect such options to be relevant for both frontend log proxies (log-forward) and backend log proxies (backend with "mode log"). While frontend log-proxies (log-forward) have a dedicated config section for historical reason, which makes it obvious that the proxy type is different during option parsing, it isn't the case for backend log proxies (backend with "mode log"). Indeed, at parsing time we don't know the type of the proxy yet, so we can only parse the config on the fly, storing all possible options even if they are not compatible with the final proxy type, and try to resolve options after all the config was parsed to detect potential configuration errors. What I'm trying to tell is that, either we go the simplest route, which is to add a way to configure options for log-forward section specifically, and we don't try to anticipate the future nor consider that the options may be used for log backends or regular proxies. Or we try to integrate log-oriented options in the existing "generic" API (options+options2). While doing so is not very elegant due to the log oriented options being mixed with http/tcp oriented options (although we can always add comments to specify that some options are only relevant under a certain context in addition to the PR_MODE_SYSLOG which indicates that they are to be used with log proxies), the main advantage is that some log-oriented options could be exposed for log frontends and log backends at the same time. We even thought of the possibility to configure log frontends (log-forward section) in a more generic way in the future: a regular frontend section with "mode log" set, like we already have for log backends, and this latter approach would remain compatible with this, while the first one is not. So all things considered I'm tempted to say that we should stick to the "ugly" option/option2 flags, having the flag declared as usual for the PR_MODE_SYSLOG with a note or name that suggests that they are only relevant for log oriented proxies. For now since we cannot configure a log-forward section with a simple frontend, we would still require the "for" loop you implemented in cfg_parse_log_forward() to iterate over options with PR_MODE_SYSLOG mode + FRONTEND capability to evaluate available options, at least as a transition solution. Ideally it should be done in a preliminary patch (just have the options evaluating logic under cfg_parse_log_forward() for options with PR_MODE_SYSLOG + FRONTEND cap in a dedicated patch, before the patch that adds prepare_log_message() and before the one that effectively adds the 2 options + their handling in syslog functions) Once this is done I think we're ready for a merge, please let me know if it looks ok to you :) Thanks!
add-dontparselog-and-assume-rfc6587-ntf.patch
Description: add-dontparselog-and-assume-rfc6587-ntf.patch
