[ANNOUNCE] haproxy-2.6-dev9
Hi, HAProxy 2.6-dev9 was released on 2022/05/08. It added 122 new commits after version 2.6-dev8. Among the changes in this version, the majority of visible ones concern the HTTP client, which can now support host name resolution using an automatically configured resolvers section named "default" if no such one already exists, and a few updates to QUIC (mostly fixes affecting POST). The rest are essentially bug fixes in various places covering CLI, help messages and reports of memory leaks on exit, which can be counted as cleanups. There's a long cleanup series touching the applets. It's not supposed to be user-visible, but it aims at simplifying the internal API to maintain a context used by applets by making each applet request some storage for its own structure instead of squatting existing fields here and there. And I tend to think this simplified API brings some value because the identification work in the current code that was required to drop the old API revealed something like 10 bugs... Special care was taken to make sure that those having their own local patches will still have something that builds and works, but the struct definitions for the old one will have to be removed for 2.7, for good. I just noticed that some of the "deprecated" attributes I placed on some of the previously abused enums to discourage from using them break the build on gcc 4.8 and older, so I'll have to relax this. The rest are some doc updates and a few cleanups (removal of unneded flags). There's still the protocol declaration mess on the "bind" lines that I want to address. I remember that we had a discussion about providing a global keyword to disable implicit IPv6 for load balancers that cannot reach IPv6 networks but that could face such resolutions, I don't know how many places need to be adjusted for that. There's this thing to discuss about regarding adding an option to re-allow GET/HEAD/DELETE + body. We've also had a demand for being able to block non-alnum/hyphen characters in header names because some application servers reportedly don't like them at all despite being part of the spec, so that will take a bit of time as well. Another point we must have a look at is that gcc 12 is finally out and I seem to remember some reports of bad jokes with earlier versions of it a few weeks ago, that's definitely something we need to check before the release. Everything seems to be nicely cooling down, so let's aim for a release by the end of this month. We could put a tentative date on the 30th to keep all efforts aligned (and I'll attend Kernel Recipes on 1st to 3rd of June and would prefer not to have to think about last minute trouble). This tentative date doesn't guarantee anything of course but that will help everyone adjust their changes based on the remaining time, and look at issues in an optimal priority order. If we see that it's already in perfect shape one week before we can decide to release earlier, and if on the contrary we discover a last-minute mess, we can postpone a bit as needed. As usual, please continue to test and report problems till they're easy to address. Please find the usual URLs below : Site index : http://www.haproxy.org/ Documentation: http://docs.haproxy.org/ Wiki : https://github.com/haproxy/wiki/wiki Discourse: http://discourse.haproxy.org/ Slack channel: https://slack.haproxy.org/ Issue tracker: https://github.com/haproxy/haproxy/issues Sources : http://www.haproxy.org/download/2.6/src/ Git repository : http://git.haproxy.org/git/haproxy.git/ Git Web browsing : http://git.haproxy.org/?p=haproxy.git Changelog: http://www.haproxy.org/download/2.6/src/CHANGELOG Pending bugs : http://www.haproxy.org/l/pending-bugs Reviewed bugs: http://www.haproxy.org/l/reviewed-bugs Code reports : http://www.haproxy.org/l/code-reports Latest builds: http://www.haproxy.org/l/dev-packages Willy --- Complete changelog : Amaury Denoyelle (2): MINOR: mux-quic: support full request channel buffer BUG/MINOR: h3: fix parsing of unknown frame type with null length Christopher Faulet (5): MINOR: conn-stream: Add mask from flags set by endpoint or app layer BUG/MEDIUM: conn-stream: Only keep app layer flags of the endpoint on reset BUG/MEDIUM: mux-fcgi: Be sure to never set EOM flag on an empty HTX message BUG/MEDIUM: mux-h1: Be able to handle trailers when C-L header was specified DOC: config: Update doc for PR/PH session states to warn about rewrite failures David CARLIER (1): MINOR: tcp: socket translate TCP_KEEPIDLE for macOs equivalent Frédéric Lécaille (4): CLEANUP: mux: Useless xprt_quic-t.h inclusion MINOR: quic: Make the quic_conn be aware of the number of streams BUG/MINOR: quic: Dropped retransmitted STREAM frames BUG/MINOR: mux_quic: Dropped packet upon retransmission for closed streams Ilya
Re: [PATCH 1/1: BUILD/MINOR: TCP_KEEPIDLE macos equivalence
On Sun, May 08, 2022 at 12:39:13PM +0200, Vincent Bernat wrote: > ? 8 May 2022 10:57 +02, Willy Tarreau: > > > After edition (still minimal and possibly inaccurate but the best I > > could do): > > > > On Linux the interval before starting to send TCP keep-alive packets > > is defined by TCP_KEEPIDLE. MacOS has an equivalent with TCP_KEEPIDLE, > > which also uses seconds as a unit, so it's possible to simply remap the > > definition of TCP_KEEPIDLE to TCP_KEEPALIVE there and get it to > > seamlessly > > work. The other settings (interval and count) are not present, > > though. > > First instance of TCP_KEEPIDLE should be TCP_KEEPALIVE. ;-) Oops, thanks, that's exactly why I hate having to document others' work myself. It's so easy to get something wrong :-/ Willy
Re: Latest http/3 info
On 5/8/2022 3:16 AM, Willy Tarreau wrote: There's no good solution to this, except by forcing the exact address yourself. The BSD socket API doesn't permit to send UDP packets from a specific source, so the commonly used approach for clients is to bind while sending the first packet, but that doesn't work for a server that faces many clients, as it would restrict the traffic to the first IP used. Thanks for that info. I got it working. I set the wildcard entry in my internal DNS to the VIP, configured a specific name to point to the machine's primary address, and then bound quic directly to the VIP address only. TCP bindings are still 0.0.0.0. Then I changed the port forwarding in my router to point ports 22/tcp, 80/tcp, 443/tcp, and 443/udp to the VIP. Adding documentation about this quirk of UDP sounds like an excellent idea. The doc for QUIC should point the user to the doc for UDP for details. Note that in order for the two haproxy nodes to bind to the virtual address you'll likely have to enable ip_nonlocal_bind, but I guess you already have it. When I had two haproxy instances, I didn't need ip_nonlocal_bind. Probably because I used 0.0.0.0 for all bindings and the VIP didn't exist at the time. The dev version has proven stable enough for my purposes that I eliminated the second instance. If I have a problem with it in the near future, I can roll back to a prior commit and rebuild. Thanks, Shawn
Re: [PATCH 1/1: BUILD/MINOR: TCP_KEEPIDLE macos equivalence
❦ 8 May 2022 10:57 +02, Willy Tarreau: > After edition (still minimal and possibly inaccurate but the best I > could do): > > On Linux the interval before starting to send TCP keep-alive packets > is defined by TCP_KEEPIDLE. MacOS has an equivalent with TCP_KEEPIDLE, > which also uses seconds as a unit, so it's possible to simply remap the > definition of TCP_KEEPIDLE to TCP_KEEPALIVE there and get it to seamlessly > work. The other settings (interval and count) are not present, > though. First instance of TCP_KEEPIDLE should be TCP_KEEPALIVE. ;-) -- Treat end of file conditions in a uniform manner. - The Elements of Programming Style (Kernighan & Plauger)
Re: [PATCH 1/1: BUILD/MINOR: TCP_KEEPIDLE macos equivalence
On Sun, May 08, 2022 at 10:21:28AM +0100, David CARLIER wrote: > On Sun, 8 May 2022 at 09:57, Willy Tarreau wrote: > > > > On Sun, May 01, 2022 at 03:33:17PM +0100, David CARLIER wrote: > > > Hi here a little patch to set idle time for SO_KEEPALIVE socket option. > > > > Now merged, thanks. > > > > David, one comment though, your commit messages keep missing a lot of > > crucial information for reviewers and debuggers, and I had to spend time > > documenting myself on keep-alive on MacOS to figure the differences and/or > > what the impacts or limitations of the patch could be. I finally found and > > put that information into the commit message, but it would be much easier > > if you could put it yourself after your development since you actually > > have access to the information I had to seek, and know the reasoning > > behind your choice. > > > > Thanks, fair point :-) I ll take it in account Thanks ;-) > even tough I was certain for this one it would not break anything. Quite frankly, experience tells us we really never know. If MacOS in the future version would define the TCP_KEEPIDLE macro, that would be sufficient to break the build for example. > > The new choice was explained again. Finally, two weeks later I got a > > report of breakage when using external code linked at run time, the > > keyword registration didn't work anymore due to a mistake in this last > > patch. Fortunately, the design decision was explained and I could > > restart from this, figure my mistake and make sure that the 3rd attempt > > at a fix this time addressed all 3 identified use cases: > > Do you have a reasonable numbers of macOs users or is it a rare occurrence ? I really don't know, here it was on the CI. Outside of the CI, breakage is usually reported in this order: - Linux - FreeBSD - MacOS - others, in random order Thus I guess it translates the number of occurrences in field. I suspect that a number of admins have macs and are used to build it locally to test their configs, and that it's probably a non-negligible part of the MacOS reports. But as usual, I could be totally wrong ;-) Willy
Re: 2.5: Possibility to upgrade http/1.0 clients to http/1.1?
Hello Dominik, On Thu, May 05, 2022 at 07:55:06AM +, Froehlich, Dominik wrote: > Hello everyone, > > We recently bumped our HAproxy deployment to 2.5 and are now getting hit by > this fix: > > MEDIUM: mux-h1: Reject HTTP/1.0 GET/HEAD/DELETE requests with a payload > > > http://git.haproxy.org/?p=haproxy-2.5.git;a=blob_plain;f=CHANGELOG > > The issue is we have many legacy customers using very old systems and we > can't tell all of them to rewrite their clients to http/1.1. Of course... Do you have an idea how they ended up sending a body with such requests ? Maybe adding a comment on the issue below for posterity could be useful for future versions of the HTTP spec: https://github.com/httpwg/http-core/issues/904 > I get the security fix to prevent request smuggling where some servers ignore > the body and treat it as another request, I'm not arguing that. > > However, I was wondering if it was possible to intercept HTTP/1.0 client > requests and upgrade them to HTTP/1.1 without hitting the rejection code of > the commit here: > https://github.com/haproxy/haproxy/commit/e136bd12a32970bc90d862d5fe09ea1952b62974 "Upgrading" the version must really never ever be done, as a lot of HTTP semantics changed between 1.0 and 1.1, and by telling a server that the client is 1.1, the server will be allowed to use chunking, trailers, 100-continue and a lot of other stuff that 1.0 clients are unable to understand. For your use case, I guess the best solution would be to add an option (possibly a global one) to relax that rule. It's self-contained inside an "if" statement so it should be quite easy to add such an option and be done with it, because when you need it, you definitely know that you need it, you'll find the keyword in the doc and the accompanying warning about the security impacts. Also the HTTP spec now says "unless support is indicated via other means", so the intent clearly is to make this configurable on a case-by-case basis. > This way we would not have to downgrade to HAproxy 2.4 again - which would be > very unfortunate as we need many of the nice features of 2.5. We'll discuss this with Christopher tomorrow, but I'm not worried about this for now. Thanks! Willy
Re: [PATCH 1/1: BUILD/MINOR: TCP_KEEPIDLE macos equivalence
On Sun, 8 May 2022 at 09:57, Willy Tarreau wrote:
>
> On Sun, May 01, 2022 at 03:33:17PM +0100, David CARLIER wrote:
> > Hi here a little patch to set idle time for SO_KEEPALIVE socket option.
>
> Now merged, thanks.
>
> David, one comment though, your commit messages keep missing a lot of
> crucial information for reviewers and debuggers, and I had to spend time
> documenting myself on keep-alive on MacOS to figure the differences and/or
> what the impacts or limitations of the patch could be. I finally found and
> put that information into the commit message, but it would be much easier
> if you could put it yourself after your development since you actually
> have access to the information I had to seek, and know the reasoning
> behind your choice.
>
Thanks, fair point :-) I ll take it in account even tough I was
certain for this one it would not break anything.
...
> The new choice was explained again. Finally, two weeks later I got a
> report of breakage when using external code linked at run time, the
> keyword registration didn't work anymore due to a mistake in this last
> patch. Fortunately, the design decision was explained and I could
> restart from this, figure my mistake and make sure that the 3rd attempt
> at a fix this time addressed all 3 identified use cases:
Do you have a reasonable numbers of macOs users or is it a rare occurrence ?
>
> commit 65d9f83794e00e136335348de531167f31d2f39b
> Author: Willy Tarreau
> Date: Tue Apr 26 19:35:38 2022 +0200
>
> BUILD: compiler: properly distinguish weak and global symbols
>
> While weak symbols were finally fixed with commit fb1b6f5bc ("BUILD:
> compiler: use a more portable set of asm(".weak") statements"), it
> was an error to think that initcall symbols were also weak. They must
> not be and they're only global. The reason is that any externally
> linked code loaded as a .so would drop its weak symbols when being
> loaded, hence its initcalls that may contain various function
> registration calls.
>
> The ambiguity came from the fact that we initially reused the initcall's
> HA_GLOBL macro for OSX then generalized it, then turned it to a choice
> between .globl and .weak based on the OS, while in fact we needed a
> macro to define weak symbols.
>
> Let's rename the macro to HA_WEAK() to make it clear it's only for weak
> symbols, and redefine HA_GLOBL() that initcall needs.
>
> This will need to be backported wherever the commit above is backported
> (at least 2.5 for now).
>
I did not notice really those mem stats until now, good to know !
> For sure it's not perfect, and anything can always be improved. But
> that's not the point, the point here is to put info about the intent and
> the approach so that these ones can be preserved or corrected later. And
> that's valid for any patch, even a one-liner.
>
> The impact of your patch is that tcp-keep-alive will start to work on
> MacOS and if someone starts to rely on it and someone else later reverts
> your patch because it causes a build issue on a specific platform and the
> person thinks "oh we don't need this one there, we already have something
> else", by just reverting it they could break the other user's deployment.
>
> That's why it's important to put enough explanation to ensure the patch
> remains firmly stuck to the code base and cannot vanish without an extremly
> good argument.
>
Alrighty :-)
> Hoping this helps!
>
> Cheers
> Willy
Re: Latest http/3 info
On Sat, May 07, 2022 at 09:11:30AM -0600, Shawn Heisey wrote: > If you look closely at the tcpdump output, you'll notice that when haproxy > replies, it replies from the actual IP address of the machine (.200) rather > than the ucarp VIP (.170) where it received the request. Is this something > that can be fixed? Oh that's an excellent point! I remember having faced that in the past on other services (e.g. DNS). That's why most UDP-based services require that you enforce the exact address on the interface (e.g. 10.2.3.4 instead of 0.0.0.0). There's no good solution to this, except by forcing the exact address yourself. The BSD socket API doesn't permit to send UDP packets from a specific source, so the commonly used approach for clients is to bind while sending the first packet, but that doesn't work for a server that faces many clients, as it would restrict the traffic to the first IP used. Note that in order for the two haproxy nodes to bind to the virtual address you'll likely have to enable ip_nonlocal_bind, but I guess you already have it. I think that for the short term we could forbid binding on 0.0.0.0 but that would annoy most users by complicating config deployments. Thus maybe the best we can do is document this limitation. Thanks for reporting this! Willy
Re: [PATCH] CI: dynamically determine actual h2spec version
On Thu, May 05, 2022 at 03:17:07PM +0500, ??? wrote: > Hi, > > small improvement, no need to use hardcoded version. Merged, thank you Ilya Willy
Re: Fwd: Set environment variables
Hi Valerio, On Fri, May 06, 2022 at 04:25:23PM +0200, Valerio Pachera wrote: > Hi, I have several backend configuration that make use of a custom script: > > external-check command 'custom-script.sh' > > The script read uses the environment variables such as $HAPROXY_PROXY_NAME. > I would like to be able to set and environment variable in the backend > declaration, before running the external check. > This environment variable will change the behavior of custom-script.sh. > > Is it possible to declare environment variables in haproxy 1.9 or later? > > What I need is to make custom-script.sh aware if SSL is used or not. > If there's another way to achieve that, please tell me. You can only set variables in the global section using "setenv" and "presetenv", and they are in effect at the moment they're parsed (thus they're even usable later in the config file). But there's nothing to set per-backend variables that would be used by the external-check commands. If you know that your whole haproxy process is either with or without SSL, that might still work for you. Otherwise maybe you should use two different scripts for SSL a clear servers. You could possibly also rely on the server's name and/or port as a hint to your script. With that said, I admit that it could be useful to have another variable such as HAPROXY_SERVER_SSL to indicate if SSL is being used, and possibly another one such as HAPROXY_SERVER_PROTO to indicate when a protocol is being forced (e.g. h2). If you think your only solution would be to have such variables, feel free to have a look at the code to see how to add them. That should be simple enough for a first contribution. That's something we can merge late in the cycle, and possibly even backport to 2.5. Willy
Re: DOC/MINOR: Typo in INSTALL doc
On Mon, May 02, 2022 at 11:02:11PM +, Tom?s Zubiri wrote: > Line 227/581 Col 53/75 char 9913/27467 > > Section 4.5 cryptography > "is known to build ant work with branches" > > Release Branch 2.5.0 Now fixed, thank you Tomas :-) Willy
Re: [PATCH 1/1: BUILD/MINOR: TCP_KEEPIDLE macos equivalence
On Sun, May 01, 2022 at 03:33:17PM +0100, David CARLIER wrote:
> Hi here a little patch to set idle time for SO_KEEPALIVE socket option.
Now merged, thanks.
David, one comment though, your commit messages keep missing a lot of
crucial information for reviewers and debuggers, and I had to spend time
documenting myself on keep-alive on MacOS to figure the differences and/or
what the impacts or limitations of the patch could be. I finally found and
put that information into the commit message, but it would be much easier
if you could put it yourself after your development since you actually
have access to the information I had to seek, and know the reasoning
behind your choice.
Just an indication, your patch mentioned this:
TCP_KEEPALIVE has the same semantic.
After edition (still minimal and possibly inaccurate but the best I
could do):
On Linux the interval before starting to send TCP keep-alive packets
is defined by TCP_KEEPIDLE. MacOS has an equivalent with TCP_KEEPIDLE,
which also uses seconds as a unit, so it's possible to simply remap the
definition of TCP_KEEPIDLE to TCP_KEEPALIVE there and get it to seamlessly
work. The other settings (interval and count) are not present, though.
A good rule of thumb to write a useful commit message is to think about
these 3 steps:
- first, convince a project maintainer that your work is worth being
merged and is very unlikely to cause trouble elsewhere ;
- second, provide helpful information about why it's done like this and
what it provides, to someone who would see a bisect session finish on
this patch, so that this person doesn't simply think "what's this mess,
let's revert it" without figuring what use case a revert could break ;
- third, by explaining the design decisions, choices and tradeoffs (and
sometimes even alternate solutions that were ditched), you'll help the
person that discovers breakage find a different solution that will
still preserve the original intent while addressing the problem.
For example recently I broke MacOS build while trying to fix a clang
warning. The two patches that caused the breakage were these ones:
commit b12966af1006be8d4438ee1ca39c2541a1f2a4f9
Author: Willy Tarreau
Date: Wed Apr 13 17:09:45 2022 +0200
BUILD: debug: mark the __start_mem_stats/__stop_mem_stats symbols as weak
Building with clang and DEBUG_MEM_STATS shows the following warnings:
warning: __start_mem_stats changed binding to STB_WEAK [-Wsource-mgr]
warning: __stop_mem_stats changed binding to STB_WEAK [-Wsource-mgr]
The reason is that the symbols are declared using ".globl" while they
are also referenced as __attribute__((weak)) elsewhere. It turns out
that a weak symbol is implicitly a global one and that the two classes
are exclusive, thus it may confuse the linker. Better fix this.
This may be backported where the patch applies.
commit 2a06e248f5c8b7c86c7dd48eed7f6d5e87288457
Author: Willy Tarreau
Date: Wed Apr 13 17:12:20 2022 +0200
BUILD: initcall: mark the __start_i_* symbols as weak, not global
Just like for previous fix, these symbols are marked ".globl" during
their declaration, but their later mention uses __attribute__((weak)),
so it's better to only use ".weak" during the declaration so that the
symbol's class does not change.
No need to backport this unless someone reports build issues.
There's an explanation of the problem, when it's encountered, and the
reasoning behind the proposed solution. The follwing day the CI broke
on MacOS, I could restart from the elements above to try to design
another solution that still follows the same spirit (and in the worst
case it could possibly have been accepted to just revert and keep that
warning when debugging):
commit fb1b6f5bc0e8eac116e2cafe8716a7f16d95b58e
Author: Willy Tarreau
Date: Thu Apr 14 16:57:12 2022 +0200
BUILD: compiler: use a more portable set of asm(".weak") statements
The two recent patches b12966af1 ("BUILD: debug: mark the
__start_mem_stats/__stop_mem_stats symbols as weak") and 2a06e248f
("BUILD: initcall: mark the __start_i_* symbols as weak, not global")
aimed at fixing a build warning and resulted in a build breakage on
MacOS which doesn't have a ".weak" asm statement.
We've already had MacOS-specific asm() statements for section names, so
this patch continues on this trend by moving HA_GLOBL() to compiler.h
and using ".globl" on MacOS since apparently nobody complains there.
It is debatable whether to expose this only when !USE_OBSOLETE_LINKER
or all the time, but since these are just macroes it's no big deal to
let them be available when needed and let the caller decide on the
build conditions.
If any of the patches above is backported, this one will need to as
well.
The new choice was explained again. Finally,
